Process all method like POST (with body).

[IgorGalarraga]

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

[codecov-io]

Codecov Report

Merging #165 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #165      +/-   ##
=========================================
- Coverage   95.31%   95.3%   -0.02%     
=========================================
  Files          35      35              
  Lines        3225    3196      -29     
=========================================
- Hits         3074    3046      -28     
+ Misses        151     150       -1

Impacted Files	Coverage Δ
src/httpserver/details/modded_request.hpp	100% <100%> (ø)	⬆️
src/webserver.cpp	90.47% <100%> (-0.03%)	⬇️
test/integ/basic.cpp	99.67% <0%> (-0.02%)	⬇️
src/httpserver/http_request.hpp	95.23% <0%> (+2.38%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a74f9e...73104a4. Read the comment docs.

[etr]

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!

[IgorGalarraga]

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,

[etr]

I have to say that I disagree. Mostly on two points:

1. 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).

2. [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?

[IgorGalarraga]

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.