Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Template confused by data containing percent ('%') #333

Closed
JamesNewton opened this issue Mar 5, 2018 · 21 comments
Closed

Template confused by data containing percent ('%') #333

JamesNewton opened this issue Mar 5, 2018 · 21 comments
Labels

Comments

@JamesNewton
Copy link

Setup: Web page for user configuration of device settings located in PROGMEM with placeholders. User function replacing placeholders with configuration data. The configuration data is a char array and one of the characters is a %.
Observed behavior: When requested via HTTP 1.0 everything is as expected. Via HTTP 1.1, all of the web page from the % in the data, to the % in the last placeholder is replaced with the names of the remaining placeholders. No idea why the 1.0 vs the 1.1 would make a difference. Is 1.0 not chunked?

Code to reproduce: (I hope you don't mind an excerpt here, I think it gets the idea across very well). In setup:

  server.on ( "/device.html", HTTP_GET, send_device_html  ); //in Pages.h
  server.on ( "/device.html", HTTP_POST, send_device_html  ); //in Pages.h

and in the included Pages.h file:

const char PAGE_AdminDeviceSettings[] PROGMEM =  R"=====(
<!DOCTYPE html>
<HTML>
<HEAD>
  <meta name="viewport" content="width=device-width, initial-scale=1" />
  <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
  </HEAD>
<BODY>
  <a href="admin.html"  class="btn btn--s">&lt;</a>&nbsp;&nbsp;<strong>Connected Device Settings</strong>
  <hr>
  <form action="" method="post">
    <table border="0"  cellspacing="0" cellpadding="3" >
      <tr>
        <td align="right">Baud rate</td>
        <td><input type="text" id="baud" name="baud" value="%baud%"></td>
        </tr>
      <tr>
        <td align="right"> Enable Connection:</td>
        <td><input type="checkbox" id="connect" name="connect" %Connect%></td>
        </tr>
      <tr>
        <td align="right"> Blink?:</td>
        <td><div id="blinked">%blinked%</div></td>
        </tr>
      <tr><td align="left" colspan="2"><hr></td></tr>
      <tr><td align="left" colspan="2">Extracted Data Reading:</td></tr>
      <tr>
        <td align="right"> Data Pattern:</td>
        <td><input type="text" id="dataregexp1" name="dataregexp1" value="%dataregexp1%"></td>
        </tr>
      <tr>
        <td align="right"> Slope:</td>
        <td><input type="text" id="dataslope1" name="dataslope1" size="6" value="%dataslope1%"></td>
        </tr>
      <tr>
        <td align="right"> Offset:</td>
        <td><input type="text" id="dataoffset1" name="dataoffset1" size="6" value="%dataoffset1%"></td>
        </tr>
      <tr><td colspan="2" align="center"><input id="save" type="submit" style="width:150px" class="btn btn--m btn--blue" value="Save"></td></tr>
      </table>
    </form>
  </BODY>
</HTML>
)=====";

String send_device_values_html(const String& tag) {
  if (tag == "baud") return (String) config.baud;
  if (tag == "Connect") return (String) (config.Connect ? "checked" : "");
  if (tag == "blinked") return (String) (digitalRead(WAS_BLINK)? "YES": "NO");
  if (tag == "dataregexp1") return (String) config.dataregexp1;
  if (tag == "dataslope1") return (String) config.dataslope1;
  if (tag == "dataoffset1") return (String) config.dataoffset1;

}

void send_device_html(AsyncWebServerRequest *request){
  if (request->args() > 0 )  { // Save Settings 
    config.Connect = false; //guess
    for ( uint8_t i = 0; i < request->args(); i++ ) {
      if (request->argName(i) == "baud") config.baud = request->arg(i).toInt(); 
      else if (request->argName(i) == "connect") config.Connect = true; 
      else if (request->argName(i) == "dataregexp1") strlcpy( config.dataregexp1, request->arg(i).c_str(), sizeof(config.dataregexp1)); 
      else if (request->argName(i) == "dataslope1") config.dataslope1 = request->arg(i).toInt(); 
      else if (request->argName(i) == "dataoffset1") config.dataoffset1 = request->arg(i).toInt(); 
      }
    WriteConfig();
    if (config.Connect) {
        digitalWrite(SERIAL_ENABLE_PIN, HIGH);
      } else {
        digitalWrite(SERIAL_ENABLE_PIN, LOW);
        }
    }
  request->send_P ( 200, "text/html", PAGE_AdminDeviceSettings, send_device_values_html ); 
  debugbuf+=__FUNCTION__;
}


The problem is when the config.dataregexp1 is set to a value containing a '%' character. In this case, it is set to r_0000_%3x.

Using a little utility called web bug that allows me to control the http data request and see the raw returned values, I am testing this and when using HTTP 1.0, I get this:

GET /device.html HTTP/1.0
Accept: */*
User-Agent: WebBug/5.0

HTTP/1.0 200 OK
Content-Type: text/html
Connection: close

3e9 

<!DOCTYPE html>
<HTML>
<HEAD>
  <meta name="viewport" content="width=device-width, initial-scale=1" />
  <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
  </HEAD>
<BODY>
  <a href="admin.html"  class="btn btn--s">&lt;</a>&nbsp;&nbsp;<strong>Connected Device Settings</strong>
  <hr>
  <form action="" method="post">
    <table border="0"  cellspacing="0" cellpadding="3" >
      <tr><td align="right">Baud rate</td><td><input type="text" id="baud" name="baud" value="9600"></td></tr>
      <tr><td align="right"> Enable Connection:</td><td><input type="checkbox" id="connect" name="connect" ></td></tr>
      <tr><td align="right"> Blink?:</td><td><div id="blinked">YES</div></td></tr>
      <tr><td align="left" colspan="2"><hr></td></tr>
      <tr><td align="left" colspan="2">Extracted Data Reading:</td></tr>
      <tr><td align="right"> Data Pattern:</td><td><input type="text" id="dataregexp1" name="dataregexp1" value="r_0000_%3x"></td></tr>
      <tr><td align="right"> 
1a2 
Slope:</td><td><input type="text" id="dataslope1" name="dataslope1" size="6" value="1"></td></tr>
      <tr><td align="right"> Offset:</td><td><input type="text" id="dataoffset1" name="dataoffset1" size="6" value="0"></td></tr>
      <tr><td colspan="2" align="center"><input id="save" type="submit" style="width:150px" class="btn btn--m btn--blue" value="Save"></td></tr>
      </table>
    </form>
  </BODY>
</HTML>

0   

But when using HTTP/1.1, as Chrome and other browsers will do, I get:

GET /device.html HTTP/1.1
Accept: */*

HTTP/1.1 200 OK
Content-Type: text/html
Connection: close
Accept-Ranges: none
Transfer-Encoding: chunked

3b8 

<!DOCTYPE html>
<HTML>
<HEAD>
  <meta name="viewport" content="width=device-width, initial-scale=1" />
  <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
  </HEAD>
<BODY>
  <a href="admin.html"  class="btn btn--s">&lt;</a>&nbsp;&nbsp;<strong>Connected Device Settings</strong>
  <hr>
  <form action="" method="post">
    <table border="0"  cellspacing="0" cellpadding="3" >
      <tr><td align="right">Baud rate</td><td><input type="text" id="baud" name="baud" value="9600"></td></tr>
      <tr><td align="right"> Enable Connection:</td><td><input type="checkbox" id="connect" name="connect" ></td></tr>
      <tr><td align="right"> Blink?:</td><td><div id="blinked">YES</div></td></tr>
      <tr><td align="left" colspan="2"><hr></td></tr>
      <tr><td align="left" colspan="2">Extracted Data Reading:</td></tr>
      <tr><td align="right"> Data Pattern:</td><td><input type="text" id="dataregexp1" name="dataregexp1" value="r_0
e5  
000_dataslope1dataoffset1%"></td></tr>
      <tr><td colspan="2" align="center"><input id="save" type="submit" style="width:150px" class="btn btn--m btn--blue" value="Save"></td></tr>
      </table>
    </form>
  </BODY>
</HTML>

0   

@JamesNewton
Copy link
Author

This seems to be different from #300 where the percent in a CSS string is being mistaken for a placeholder, but the workaround for that,
#300 (comment)
https://github.com/me-no-dev/ESPAsyncWebServer/blob/master/src/WebResponseImpl.h#L62
changing the placeholder, is a valid workaround for this, given a marker that will never ever be entered by a user. Yeah. So I'ma use a back tick for now and hope that never comes up in the application.

I'll try to take a look at the code and see if I can see a fix when time allows. For now, I would recommend the back tick as a less likely to be an issue option. In /src/WebResponseImpl.h

#define TEMPLATE_PLACEHOLDER '`'

@aguaviva
Copy link

aguaviva commented Mar 6, 2018

Hey, this sounds like an easy fix where you dont need to have any deep knowledge of the libraries. Remember you can do a pull request and contribute yourself with fixes :)

@JamesNewton
Copy link
Author

JamesNewton commented Mar 7, 2018

Take a look at the code:
https://github.com/me-no-dev/ESPAsyncWebServer/blob/master/src/WebResponses.cpp#L366
and tell me again how it's an easy fix. LOL.

In general I'm sure the issue is that it's not skipping past the inserted data before looking for another % but where it's going wrong on that is unclear. There are 4 or 5 different places where len is set.

If I could watch it in a debugger, I could figure it out, but I'm not aware of a way to debug the ESP.

@aguaviva
Copy link

aguaviva commented Mar 8, 2018

LOL you are absolutely right :D My apologies :D

@drschlaumeier
Copy link

Yeah, super :-)
So, I lost 3h searching why my new webserver code not working ... until I found the stupid % in CSS of my html :-(
Would be good to have TEMPLATE_PLACEHOLDER configurable from outside...

@me21
Copy link
Contributor

me21 commented May 18, 2018

Platformio IDE with Visual Studio Code now has the debugger, it works for ESP too. But, unfortunately, it is a paid feature.
I will make TEMPLATE_PLACEHOLDER configurable from outside, I have an idea how to do this.

PS My apologies to all of you, since I'm the author of that code. I don't like it either, but at that moment I thought that using memmove/memcpy/memchr would result in better performance than searching the string for templates byte-by-byte. Premature optimization is the root of all evil, as Donald Knuth said. Again, my apologies.
Perhaps I should prepare an alternative implementation at least to compare the performance. I'm not working with ESP at the moment, though.

@me21
Copy link
Contributor

me21 commented May 19, 2018

Try the PR #366. You will need to pass the argument to the compiler to #define TEMPLATE_PLACEHOLDER symbol with desired value during build.

JamesNewton added a commit to JamesNewton/esp8266WebSerial that referenced this issue Jul 2, 2018
-DEBUG levels: 1 for serial only, 2 for serial and display.
-User config device baudrate, poweron initialization string, data extraction and display:
-- New /device.html page to configure all connected device options
-- Send config.pwronstr to device pwrondelay seconds after startup
-- Extract up to 3 data items returned from device for local display via scanf code:
   http://www.cplusplus.com/reference/cstdio/scanf/
-- Extracted values multiplied by slope plus offset and shown with name on the display
-global.h config load/save simplified with EEPROM.get / EEPROM.put.
-CONFIG_VER version number added. This update will clear prior config data to default!
-txbuf to hold data waiting to be sent to the device so we can send this in a debug message
-Support "hidden" /update page to upload web/js/css/etc files into SPIFFS for web server.
 This allows customization and rebranding for your solution without the need for Arduino code development.
 Put your own html/javascript in the device for user interface.
-Default web page is index.htm from SPIFFS. /root is handle_root from Page_Root.h
  i.e. http://ip/ will display nothing until index.htm is uploaded via http://ip/update.
  If you want to use the built in homepage, go to http://ip/root
  TODO: serve root automatically if no index.htm in SPIFFS
-Built in /root page converted from jquery to raw javascript for increased reliability and smaller size.
   This is critical when working in poor RF environments while directly connected to the device.
-Use template processor in ESPAsyncWebServer for config pages. There are problems with this, see:
-- me-no-dev/ESPAsyncWebServer#333 Must change TEMPLATE_PLACEHOLDER to '`'
    in \libraries\ESPAsyncWebServer\src\WebResponseImpl.h and trigger library re-compile
-- me-no-dev/ESPAsyncWebServer#374 Must use "AI-THINKER" ESP-12E units
-- Network setup /config.html NOT updated to templates for this reason.
-Moving to a single general tag processor function "send_tag_values"
-Moving to standard linking to .css and .js resources from config pages instead of the javascript window.onload functions.
-Add debugq macro to append to debugbuf for later output. This avoids delays when debugging web responses
-Increase max string length of ReadStringFromEEPROM from 60 to 65
-urldecode, avoids Strings, returns char*, expects length.
-urlencode, expects char* vs String (still returns String). Simplified via nibble2hex.
-HTMLencode added inorder to support all strings for scanf codes.
-Optional support for NeoPixel LEDs. NOT enabled by default.
-Start on a command processor language from server response or user input
BUGFIX:
IO pin 10 is NOT usable as SERIAL_ENABLE_PIN, back to IO5 D1
parseServer p1,p2 had not been initialized. Data returned from server for device could be lost.
JamesNewton added a commit to JamesNewton/esp8266WebSerial that referenced this issue Jul 2, 2018
-DEBUG levels: 1 for serial only, 2 for serial and display.
-User config device baudrate, poweron initialization string, data extraction and display:
-- New /device.html page to configure all connected device options
-- Send config.pwronstr to device pwrondelay seconds after startup
-- Extract up to 3 data items returned from device for local display via scanf code:
   http://www.cplusplus.com/reference/cstdio/scanf/
-- Extracted values multiplied by slope plus offset and shown with name on the display
-global.h config load/save simplified with EEPROM.get / EEPROM.put.
-CONFIG_VER version number added. This update will clear prior config data to default!
-txbuf to hold data waiting to be sent to the device so we can send this in a debug message
-Support "hidden" /update page to upload web/js/css/etc files into SPIFFS for web server.
 This allows customization and rebranding for your solution without the need for Arduino code development.
 Put your own html/javascript in the device for user interface.
-Default web page is index.htm from SPIFFS. /root is handle_root from Page_Root.h
  i.e. http://ip/ will display nothing until index.htm is uploaded via http://ip/update.
  If you want to use the built in homepage, go to http://ip/root
  TODO: serve root automatically if no index.htm in SPIFFS
-Built in /root page converted from jquery to raw javascript for increased reliability and smaller size.
   This is critical when working in poor RF environments while directly connected to the device.
-Use template processor in ESPAsyncWebServer for config pages. There are problems with this, see:
-- me-no-dev/ESPAsyncWebServer#333 Must change TEMPLATE_PLACEHOLDER to '`'
    in \libraries\ESPAsyncWebServer\src\WebResponseImpl.h and trigger library re-compile
-- me-no-dev/ESPAsyncWebServer#374 Must use "AI-THINKER" ESP-12E units
-- Network setup /config.html NOT updated to templates for this reason.
-Moving to a single general tag processor function "send_tag_values"
-Moving to standard linking to .css and .js resources from config pages instead of the javascript window.onload functions.
-Add debugq macro to append to debugbuf for later output. This avoids delays when debugging web responses
-Increase max string length of ReadStringFromEEPROM from 60 to 65
-urldecode, avoids Strings, returns char*, expects length.
-urlencode, expects char* vs String (still returns String). Simplified via nibble2hex.
-HTMLencode added inorder to support all strings for scanf codes.
-Optional support for NeoPixel LEDs. NOT enabled by default.
-Start on a command processor language from server response or user input
BUGFIX:
IO pin 10 is NOT usable as SERIAL_ENABLE_PIN, back to IO5 D1
parseServer p1,p2 had not been initialized. Data returned from server for device could be lost.

Merge branch 'master' of https://github.com/JamesNewton/esp8266WebSerial

# Conflicts:
#	esp8266WebSerial.ino
#	global.h
@stale
Copy link

stale bot commented Sep 21, 2019

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 21, 2019
@stale
Copy link

stale bot commented Oct 5, 2019

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.

@stale stale bot closed this as completed Oct 5, 2019
@JamesNewton
Copy link
Author

JamesNewton commented Oct 5, 2019

This is an unresolved and important issue. Sadly, it doesn't look like I'm allowed to re-open it? I guess I have to copy the text and open a new issue?

@atanisoft
Copy link
Contributor

@JamesNewton you can reopen it if needed. But it needs to be kept active.

@JamesNewton
Copy link
Author

Again I do not appear to have permissions to reopen it. There is no reopen button available to me.

@atanisoft
Copy link
Contributor

Huh, that is very odd. @me-no-dev any ideas?

@me-no-dev me-no-dev reopened this Oct 6, 2019
@stale
Copy link

stale bot commented Oct 6, 2019

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

@stale stale bot removed the stale label Oct 6, 2019
@JamesNewton
Copy link
Author

Thanks for re-opening. Just to be clear, there is a workaround for this issue since the TEMPLATE_PLACEHOLDER is now a supported define. The issue is that you must pick one character to lose which you can never have in data placed in that field. Rather than parsing the template character out and replacing it with data and then moving forward from /after/ the inserted data, it is re-scanning the inserted data and finding template markers in it. I thought the fix would be easy, but having looked at the current code, I'll admit that I'm seriously confused by it and can't see how to apply that seemly simple idea. Hopefully someone smarter than I can do it.

@DrP2016
Copy link

DrP2016 commented Nov 4, 2019

I think i had the xame issue a while ago, that my StringProcessor replaced content between two "%" symbols e.g. in my css. You simply have to escape the Placeholder with another "%".
So for example i have an css block that uses width in percent:

		.container_content{
			min-width:800px;
			width:85%%;
			display: block;
			margin: 0px auto;
			text-align:center;
		}

@stale
Copy link

stale bot commented Jan 3, 2020

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 3, 2020
@stale
Copy link

stale bot commented Jan 17, 2020

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.

@stale stale bot closed this as completed Jan 17, 2020
@foxharp
Copy link

foxharp commented Mar 12, 2020

I can confirm that doubling the '%' character has the desired effect.

makerMcl pushed a commit to makerMcl/universalUi that referenced this issue Jan 24, 2021
@DeeEmm
Copy link

DeeEmm commented Nov 17, 2022

Doubling the %% creates invalid CSS so not a workable / usable solution.

Any solution that creates another issue is not a solution at all. ;)

@VeloSteve
Copy link

VeloSteve commented Jan 23, 2024

Edit: I am sure that what I wrote below worked briefly, but I had to edit the value in the library after all. The line is in WebResponseImpl.h at line 63 in the latest version. Easy to do, but not as easy to explain to the dozens of students who may need to download and use this in the future. I also changed from using "`" to "@". That may be a more common character, but in my case there was no conflict and the backtick is used in some javascript variable replacement code.

Just a note for users like me who have just run into this. As of 1.2.7 (and probably earlier) you can simply define what one character to use before including the header file. It will use what you provide. As others have said, you sacrifice that one character,
but in my opinion it is a lot easier to avoid the backtick as opposed to %, which is common in HTML, CSS, C, and so on.

#define TEMPLATE_PLACEHOLDER '`'
#include <ESPAsyncWebSrv.h>

Thanks to those who provided this information above. I have simply tried to make it obvious how to use it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants