Skip to content

Enumerating parameters - class httpsserver::ResourceParameters' has no member named 'beginQueryParameters' #81

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

Closed
Twilight-Logic opened this issue Apr 11, 2020 · 19 comments

Comments

@Twilight-Logic
Copy link

Describe Your Goal
Enumerate parameters returned to the webserver from a submitted page

What Does Your Project Look Like
ESP32 library version 1.0.4
Hessel ESP_HTTPS_Server library version 0.3.1

Hessel HTTPS_Server downloaded via Manage Libraries in the Arduino IDE.

If you can boil your question down to a minimal working example, please do so and share the code here:

void showArgs(HTTPRequest * req) {
  ResourceParameters * params = req->getParams();
 
  for(auto idx = params->beginQueryParameters(); idx != params->endQueryParameters(); ++idx) {
    Serial.print((*idx).first + ":\t");
    Serial.println((*idx).second);
  }

// Original ESP8266 code:
//  for (uint8_t i = 0; i < acnt; i++) {
//    AsyncWebParameter* p = request->getParam(i);
//    Serial.print(p->name() + ":\t");
//    Serial.println(p->value());
//  }
}

ESP32 Module
Please provide specifications of your module

ESP32 - NodeMCU-32S with ESP-WROOM-32

Software (please complete the following information if applicable)

Arduino IDE version 1.8.12
Linux Mint 19.2

Additional context
On the ESP8266 I had a fairly simple routine to show all the submitted parameters for diagnostic purposes. I am trying to write the equivalent routine for the ESP32 and following the example here:

https://github.com./fhessel/esp32_https_server/blob/master/examples/Parameters/Parameters.ino

See lines 282 to 293. However, it seems that beginQueryParameters() and probably endQueryParameters() is not available ? Having had a look at the "documentation" I could not find such a class member in the header file ?

@fhessel
Copy link
Owner

fhessel commented Apr 11, 2020

Hi,
version 0.3.1 of the library couldn't enumerate over the parameters (see #63), but that has changed in v1.0.0, which has been released yesterday and maybe hasn't been picked up by the library registry, yet. The missing functions are part of that release.

If you're using v0.3.1 of the library, you must use the functions from the older version of the example.

If you want to work with HTML forms, you may want also to have a look at the new example (also v1.0.0 only).

@Twilight-Logic
Copy link
Author

Twilight-Logic commented Apr 11, 2020

Ok, so looking at that example it would seem that no enumeration is possible with version 0.3.1 so question answered. I checked the Library Manager in the Arduinio IDE again and there was an update button, so I hit this and it has updated to version 1.0.0.

I don't know whether it works yet because I have now ran into further complications such as:

class httpsserver::ResourceParameters' has no member named 'getRequestParameter'

I then find this has been replaced with getQueryParameter(std::string &name, std::string &val) so the code has to be reworked. I do have a question about isQueryParameterSet(std::string): does bool getQueryParameter(std::string &name, std::string &val) serve the same purpose as it would seem that the bool response would indicate whether the query parameter exists (is set) or not?

Then I also read this:

ResourceParameters::getRequestParameterInt() does no longer exist. Use std::atoi(str.c_str())

So I now have to convert all of my instances of getRequestParameterInt() as well!

I appreciate you are working hard to develop the library, and I thank you for making it available, but
since this library is already considerably more complex to use than the original ESP8266Webserver library, the last thing it presumably needed was to make it even more complicated to use?

Given these significant changes it will take me some time to update my sketch and determine whether the enumeration works or not, but I will try to let you know as soon as I can.

@fhessel
Copy link
Owner

fhessel commented Apr 11, 2020

Seems a bit like change for the sake of change?

The change of the signature was necessary, as there was no option to differentiate between a missing parameter or an empty string. As I had to change it anyway, I also renamed the functions, because I often got feedback that the names were confusing / different from the usual wording. I queued all these changes in a single (but indeed breaking) release to require adoption just once. To make this transition as smooth as possible, the changelog lists all breaking changes and suggests what you can do. If that is a lot of work, maybe consider switching from the Arduino IDE to a more sophisticated IDE like PlatformIO with VSCode, because there you get support for such changes.

The getSomethingInt(...) variants do no longer exist because using the standard library offers the same/even more functionality in a much more stable way. Most importantly, you can select the data type you need (signedness, length, ...). atoi() is only a suggestion (and probably not the best one) that would roughly do the same as the functions before.

Using the old versions of the functions could easily lead to unsafe code, because if you used the oneliner before and didn't check the values, you cannot be sure that they are in the expected range.

If you need a workaround, you can always define something like:

int getQueryParameterInt(HTTPRequest *req, std::string &param) {
  ResourceParameters params = req->getParams();
  std::string val;
  if (params->getgetQueryParameter(param, val)) {
    return std::atoi(val.c_str()); // Careful: undefined behavior if range is unknown
  }
  return 0; // no other way to show the value doesn't exist
}

And then replace the calls by int myParam = getQueryParameterInt(req, "myParam").

@Twilight-Logic
Copy link
Author

Twilight-Logic commented Apr 11, 2020

Thank you for your reply. Ok, so do I understand correctly that isQueryParameterSet() checks whether the parameter is "missing", i.e. not be sent, whereas bool getQueryParameter(std::string, std::string) checks for an empty (i.e. NULL) string where a parameter exists and has been specified in the URL?

Thanks for the example function. It is an interesting idea and I take the point about atoi and bounds checking. I do find it interesting that you use params->getQueryParameter(param, val) to check whether the parameter exists?

@fhessel
Copy link
Owner

fhessel commented Apr 11, 2020

I do find it interesting that you use params->getQueryParameter(param, val) to check whether the parameter exists?

That's the point. The function will only modify the second argument if the parameter name does exist, and only in that case it will return true.

So for getQueryParameter("foo", val) you will get:

Request Return Value Second argument (val)
/route false unchanged
/route?foo true ""
/route?foo=bar true "bar"
/route?foo=bar&foo=baz true "bar"

You couldn't distinguish the first two rows before, and without iterating, you cannot get the the second value for foo in row 4.

isQueryParameterSet will check the same but only give you the return value. The point is that you cannot set an std::string to NULL without using pointers, and then it gets really messy. That's the exact reason for the change.

@Twilight-Logic
Copy link
Author

It took me a while but I managed to get my code updated to the point where I can get the sketch compiled without error or warning. I have the following as the enumeration routine:

void showArgs(HTTPRequest * req) {
ResourceParameters * params = req->getParams();
Serial.println(F("Parameters received =>"));
for(auto idx = params->beginQueryParameters(); idx != params->endQueryParameters(); ++idx) {
  Serial.print( (*idx).first.c_str()); Serial.print("\t");
  Serial.println( (*idx).second.c_str() );
}
Serial.println(F("<= parameters done."));
}

I simply call this from the page handler function like this:

void pageHandler(HTTPRequest * req, HTTPResponse * res) {
  ResourceParameters * params = req->getParams();
#ifdef DEBUG_2
  Serial.println(F("Received this page..."));
  showArgs(req);
#endif
some other page handling stuff.....
}

However, although I see the beginning and end markers, I am not getting any data returned from the for loop?

@fhessel
Copy link
Owner

fhessel commented Apr 12, 2020

I tried your code and it works fine for me, see the project file in Issue-81.zip.

GET https://my-esp32/?bar=bar&foo=bar&bar=baz gives me:

[HTTPS:I] Request: GET /?bar=bar&foo=bar&bar=baz (FID=56)
Parameters received =>
bar	bar
foo	bar
bar	baz
<= parameters done.

So you do see the Parameters received =>? Not that the issue is DEBUG_2 not being defined.

@Twilight-Logic
Copy link
Author

Thank you for confirming that my code actually works! I presume you tested in v1.0.0? I can confirm that DEBUG_2 is defined, but I see only:

Parameters received =>
<= parameters done.

There is no parameter enumeration in-between.

There is one thing different however. Your URL request used the GET method. Although I am using GET for the most part, including CSS and scripts, forms are submitted using POST and I am applying the showArgs() function to a form. To test this further I changed the following resource node definition line:

ResourceNode * nodeSetGen = new ResourceNode("/setGen", "POST", &setGen_h);

to this:

ResourceNode * nodeSetGen = new ResourceNode("/setGen", "GET", &setGen_h);

I then also amended the form submission method to 'GET' and now the parameters are indeed enumerated:

Parameters received =>
pass	on
webp	80
gpibp	8488
<= parameters done.

It would seem therefore, that for whatever reason, the enumeration works with the GET method, but not for the POST method. Any thoughts?

@fhessel
Copy link
Owner

fhessel commented Apr 12, 2020

Works for me with POST as well, as long as the parameters are in the URL.

Are you using HTML? Is your setup like this:

<form method="POST" action="/setGen?pass=on&webp=80&gpibp=8488">
<!-- some other form elements -->
</form>

Or are you trying to parse your form fields, like this:

<form method="POST" action="/setGEN">
<input type="text" name="pass" value="on" />
<input type="text" name="webp" value="80" />
<input type="text" name="gpibp" value="8488" />
<!-- some other form elements -->
</form>

In the second case, you are not looking for query parameters but for form values. Have a look at the body parsers. The HTML Forms example is a good start.

@fhessel
Copy link
Owner

fhessel commented Apr 12, 2020

Short addition: You can have an HTML <form> that appends the parameters to the URL, but then you need to set method="GET". You should, however, do that only if your request does not have side effects (like configuring something), because browsers will refresh such URLs without a warning and its way more prone to request forgery.

@Twilight-Logic
Copy link
Author

Twilight-Logic commented Apr 13, 2020

Isn't the whole point of the POST method to NOT have parameters displayed in the URL?
The forms in this sketch are generally designed for configuring the device and although I have server side validation as well, preventing request forgery is one of the considerations. Besides, it is much neater if form values do not appear as parameters in the URL.

On that point, you are correct that I am looking for form values as per your second example. With GET these are appended as &this=that key pairs when the form is transmitted so I'm not clear on the distinction. Post handles things differently, but essentially key pairs are still transmitted in the background.

For now I had a quick look at the HTML Forms example and am uncertain of its relevance as I am not using mime types such as files, only basic text fields and form controls, however I will come back to it for a more detailed look in a while.

@fhessel
Copy link
Owner

fhessel commented Apr 13, 2020

On that point, you are correct that I am looking for form values as per your second example. With GET these are appended as &this=that key pairs when the form is transmitted so I'm not clear on the distinction. Post handles things differently, but essentially key pairs are sill transmitted in the background.

Then you are not using the query parameters but transmitting the fields in the body. That's why you need the body parsers. As you said, that's the point of using POST.

For now I had a quick look at the HTML Forms example and am uncertain of its relevance as I am not using mime types such as files, only basic text fields and form controls, however I will come back to it for a more detailed look in a while.

There are two flavors of body parsers, you should pick the right one, depending on what you want to do. They differ in what you put into the enctype attribute of your form tag. As one of them (the multipart one) requires passing of MIME types, this is in the interfaces of both, but the URL-encoded one will just give you always text/plain, so you don't need to check that field.

Variant 1: enctype="application/x-www-form-urlencoded"

How the form looks like:

<form method="POST" action="/target" enctype="application/x-www-form-urlencoded">
<input type="text" name="somefield" value="42" />
<input type="text" name="otherfield" value="23" />
</form>

How the request looks like (simplified):

POST /target HTTP/1.1
Content-Type: application/x-www-form-urlencoded

somefield=42&otherfield=23

Pro: Efficient encoding of the data

Con: Cannot handle file uploads

Bodyparser Type: HTTPURLEncodedBodyParser

Variant 2: enctype="multipart/form-data"

How the form looks like:

<form method="POST" action="/target" enctype="multipart/form-data">
<input type="text" name="somefield" value="42" />
<input type="text" name="otherfield" value="23" />
</form>

How the request looks like (simplified):

POST /target HTTP/1.1
Content-Type: multipart/form-data; boundary=----abcdef

----abcdef
Content-Disposition: form-data; name="somefield"

42
----abcdef
Content-Disposition: form-data; name="otherfield"

23
----abcdef--

Pros: Can handle file upload

Cons: Very much overhead for simple forms

Bodyparser type: HTTPMultipartBodyParser

@Twilight-Logic
Copy link
Author

Twilight-Logic commented Apr 13, 2020

Thank you for the above information. I had a more in-depth look at the parser objects and managed to come up with something that works for POST:

void showArgs(HTTPRequest * req) {

  size_t fieldLength;
  byte buf[128];

  HTTPURLEncodedBodyParser parser(req);

  while(parser.nextField()) {
    std::string fieldValue;
    std::string fieldName = parser.getFieldName();
    fieldLength = parser.read(buf, 128);
    fieldValue.append((char*)buf, fieldLength);
    Serial.print(fieldName.c_str()); Serial.print("\t");
    Serial.println(fieldValue.c_str());    
  }

  Serial.println(F("<= parameters done."));
}

I was hoping for something like getFieldValue() or getFieldText() method, but all three parser classes appear geared up to read files or encoded data and only provide the read() method which takes a byte buffer and a length attribute. The byte buffer then has then to be converted into a string type to perform string comparisons or output via Serial.println(). This does seem a lot of overhead just to read a handful of characters from a form field?

I guess this also means that I will have to replace all occurrences of params->getQueryParameter("fieldname", result) for forms submitted using the POST method with a similar function to the above in order to extract values for a specific fields.

BTW, can I ask what the relationship is between the esp_https_server library included in the Espressif ESP32 library and your library? Have they used your library in theirs?

@fhessel
Copy link
Owner

fhessel commented Apr 13, 2020

I was hoping for something like getFieldValue() or getFieldText() method, but all three parser classes appear geared up to read files or encoded data and only provide the read() method which takes a byte buffer and a length attribute.

That kind of interface is the only one that can handle arbitrary lengths of the value field. It the library would create a buffer for you, you're limited to the largest chunk of memory that can be allocated. Furthermore, the library cannot know the length of the field value before, so allocating a sufficiently large buffer in advance is not possible. In addition, multiple reallocation would be required if the lib underestimates the size of the field, leading effectively to half of the free memory being the upper boundary for a single field's content size. However, I see that convenience methods like getFieldValue() make sense for handling forms with many short fields. I'll create an issue for that. I created Issue #83 for that.

guess this also means that I will have to replace all occurrences of params->getQueryParameter("fieldname", result) for forms submitted using the POST (...)

That's true, as with getQueryParameter, you cannot access the POSTed body. The reason why you can directly and randomly access the query parameters is because the length of the query string is quite limited by default, so that the memory wouldn't get exhausted from that.

BTW, can I ask what the relationship is between the esp_https_server library included in the Espressif ESP32 library and your library? Have they used your library in theirs?

There is no relation. When I started implementing this at the end of 2017, there was (to my knowledge) no useful HTTPS server implementation for the ESP32, neither for the IDF nor for the Arduino core. I didn't check their code, but as it's written in plain C, taking the C++ code from this library and converting it might not be very efficient.

@Twilight-Logic
Copy link
Author

Twilight-Logic commented Apr 13, 2020

That kind of interface is the only one that can handle arbitrary lengths of the value field. It the library would create a buffer for you, you're limited to the largest chunk of memory that can be allocated. Furthermore, the library cannot know the length of the field value before, so allocating a sufficiently large buffer in advance is not possible.

I kind of take the point with that. There is no way to know beforehand the size of the submitted field data.

In the meantime, it seems I have another problem. I created a function to read the contents of a specific field. It looks like this:

String getArg(HTTPRequest * req, std::string argname){
  
  HTTPURLEncodedBodyParser parser(req);

  size_t fieldLength;
  byte buf[128];
  String argValue = "";
  bool found = false;

  while(parser.nextField()) {
    std::string fieldName = parser.getFieldName();
    if (argname.compare(fieldName) == 0) {
      found = true;
      break;
    }
  }

  if (found) {
    fieldLength = parser.read(buf, 128);
    for (int i=0; i<fieldLength; i++) {
      argValue += (char)buf[i];
    }
  }
  return argValue;
}

It is similar to the showArgs() function earlier, except that it stops when a match is found and returns the results. I appreciate it can be tidied up further, but when tested it returned no data. After a little troubleshooting I discovered that the while() loop never runs so nothing is found. I am running showArgs() first to make sure that my page is returning data (which it is) and then getArg(req, "argname") to get the value of a specific field. You will note that each function has its own instance of the HTTPURLEncodedBodyParser object. Some further testing showed that when showArgs() is commented out, getArg() then returns results, but only the first time it is called. If I run getArg() again for another field, there are no results again. It would therefore seem that only one enumeration of the parser is possible even despite the fact that a new HTTPURLEncodedBodyParser object is being created each time. Not sure what's happening here, but I do need some way to access the value of more than one field in each form. For the present, since there is no method to directly select a given field, enumerating the field names and looking for a match seems the only way to find them.

BTW, I asked about the origins of the server because of this:

https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/protocols/esp_https_server.html#overview

Seems very similar but they don't mention your project and hence some confusion on my part thinking that perhaps that your library is included in the ESP32 library.

@fhessel
Copy link
Owner

fhessel commented Apr 15, 2020

It would therefore seem that only one enumeration of the parser is possible even despite the fact that a new HTTPURLEncodedBodyParser object is being created each time. Not sure what's happening here, but I do need some way to access the value of more than one field in each form. For the present, since there is no method to directly select a given field, enumerating the field names and looking for a match seems the only way to find them.

That is intended behavior, you can wrap only a single body parser around the body, which will then consume the whole request. See #84 for the reason and a workaround. Maybe that's an issue of lacking documentation how the parsers actually work.

@Twilight-Logic
Copy link
Author

Thank you. That does seem to make sense and I see the workaround descibed in #84 is as I imagined it might have to be.

Thank you for your help.

@fhessel
Copy link
Owner

fhessel commented Apr 17, 2020

Then I'll update the docs for the body parsers. Is there anything else or may I close the issue?

@Twilight-Logic
Copy link
Author

Twilight-Logic commented Apr 22, 2020

Nothing at present., Issue may be closed. Thanks.

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

No branches or pull requests

2 participants