Process all method like POST (with body). by IgorGalarraga · Pull Request #165 · etr/libhttpserver
Description of the Change
Originally this example:
curl -v -XGET --data 'test' 'http://localhost:8080/service'
(GET metoth + data on body of request)
does not finalize connection with client.
Quantitative Performance Benefits
With this change, connection closes and responds correctly.
I am not completely sure that it is the best solution.
Verification Process
Using examples/service:
curl -v -XGET --data 'test' 'http://localhost:8080/service'
Applicable Issues
Release Notes
Originally this example: curl -v -XGET --data 'test' 'http://localhost:8080/service' does not finalize connection with client. With this change, connection closes and responds correctly.
Thanks for notifying about this bug.
I still feel that GET (and GET-like) methods shouldn't support body provided in input.
See: https://tools.ietf.org/html/rfc7231#section-4.3.1
"A payload within a GET request message has no defined semantics; sending a payload body on a GET request might cause some existing implementations to reject the request."
This commit ( b8f6578 ) going through travis right now should fix the bug you report but enforces the correct semantic of GET.
It should be on head soon. Thanks again!
labels
I still feel that GET (and GET-like) methods shouldn't support body provided in input.
See: https://tools.ietf.org/html/rfc7231#section-4.3.1
"A payload within a GET request message has no defined semantics; sending a payload body on a GET request might cause some existing implementations to reject the request."
I agree with reject as malformed method..., but, this is the answer of a stardards committe (I will omit which one) about their recommendation to 'post' body content using GET method:
Regarding the use of and application content over a http GET method, 'XXX technical committee' is aware that it is not a common practice, but according to RFCs 7230-7237, the request body is independent of method semantics. It is “allowed” to send a body over HTTP GET method. This convention should be managed by the HTTP server of the unit providing the service.
So, I think the best solution is to support GETs with body AND have the functions to retrieve received data like in POST method:
Piece of code that I used to test into examples/service.cpp into POST and GET methods:
if (verbose) {
std::cout << req;
std::cout << " Body [ " << req.get_content() << " ]" << std::endl;
}
If I use POST I can request 'body' but I can't retreive that info if a GET is received.
What do you think? Is there another function to retrieve body content when a GET is received?
Best regards,
I have to say that I disagree. Mostly on two points:
-
There is a difference in this context between syntax and semantic. Whereas a GET with body might be syntactically correct, it is generally considered bad semantic. It interesting to note how older HTTP-1.1 RFC referred explicitly to how this should not be done (rather than it should be discouraged). I appreciate disagreement on this point, but the overall approach of this library (hopefully not failed too much) has been to push for a clean RESTful semantic.
In this sense, RESTful offers a very specific tool (POST) which cover those cases where the length of parameters cannot respect the requirements of a GET (with POST having a willingly more ambiguous semantic)Providing a block of data, such as the fields entered into an HTML form, to a data-handling process;
Even in this I understand how others have taken different approaches and consider my view - e.g. Elasticsearch allowing for body in GET because they felt that using POST might be inappropriate instead (mostly based on naming of the method).
- [minor] Processing the body has a non indifferent cost that would always be payed (e.g. by creating a post-processor) even when, as in the majority of cases, it wouldn't be needed.
For the above, I believe that syntactically the webserver should be immune from erroring when receiving a body (unless specified by the business logic above) but shouldn't support this use-case.
As a general case though, I am quite fond to allow for ideas in disagreement with my recommended view of the world and of how this API should behave. In general, I like to segregate these behaviors using preprocessor directives so that "non-standard" behaviors can be supported to allow for specific needs. I am going to push the current change I have on the separate branch I linked (see: #166). I will be happy to receive a follow-up change from you to support the body-processing use-case if that is kept isolated within preprocessor option. Would that work?
Thank you for your full explanation, I totally agree.
I didn't realize that processing the body has an extra cost, I thought you could look early if the body has any size or not to evaluate processing or not the body, dinamically but without extra complexity.
But after your explanation, I wouldn't make the decision to worsen all requests to support a few exceptions using not recommended semantics.
I think, the preprocessor option (adding a new parameter into 'configure') would be a sufficient and adequate solution. It should also be documented for its easy identification, when the special need arises.