me-no-dev/ESPAsyncWebServer

Template confused by data containing percent ('%')

JamesNewton opened this issue ยท 21 comments

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   

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 '`'

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 :)

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.

LOL you are absolutely right :D My apologies :D

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 commented

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 commented

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

stale commented

[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 commented

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

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?

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

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

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

stale commented

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

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.

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 commented

[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 commented

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

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

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

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

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.