[PATCH] httpserver: properly handle chunked POST requests with body

[Waldemar Kozaczuk]

This bug was discovered when running httpserver unit tests with python
scripts upgraded to version 3. The new version of the requests module chunks
POST requests with body so that they are sent over socket in two parts - the request
and the body.

Our httpserver had a bug in how it consumed such requests. Instead of
completing the request once the body chunk was fully received,
it would try to re-consume the same body chunk as next request and after
failing to, it would send back a 400 (bad request) response.

So this patch simply changes the connection logic to complete handling
request in such scenario and proceed to the next request.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>
---
modules/httpserver-api/connection.cc | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/modules/httpserver-api/connection.cc b/modules/httpserver-api/connection.cc
index bb88eab6..e669a5f5 100644
--- a/modules/httpserver-api/connection.cc
+++ b/modules/httpserver-api/connection.cc
@@ -246,8 +246,11 @@ void connection::do_read()
request_.content.append(buffer_.data(), buffer_.data() + bytes_transferred);
if (request_.content.size() < request_.content_length) {
do_read();
- return;
+ } else {
+ request_handler_.handle_request(request_, reply_);
+ do_write();
}
+ return;
}

auto r = request_parser_.parse(
--
2.20.1

[Nadav Har'El]

I'm afraid I don't understand this change.... In the if(true) case, you just moved the return a line down.

In the if(false) case, the previous code simply continued (did not "return") and ran exactly the same

two commands - request_handler_.handle_request(request_, reply_) and do_write() - that you now

have it do explicitly in the else(). So what changed? (I'm probably missing something, but I can't

figure out what)

             auto r = request_parser_.parse(
--
2.20.1

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20200217215801.8729-1-jwkozaczuk%40gmail.com.

[Waldek Kozaczuk]

I think you may need to look in the context of full do_read() method code. Before the patch if (request_.content.size() < request_.content_length)" was false (full data body was received on a socket at this point), the code would continue to the next line:

            auto r = request_parser_.parse(

                         request_, buffer_.data(), buffer_.data() +

                         bytes_transferred);

which would try to parse the body as a new request (method line, etc) and result in 400 - bad request. So instead we need to terminate handling of THIS request and its body and return. This is why we have this change.

             auto r = request_parser_.parse(
--
2.20.1

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.

To unsubscribe from this group and stop receiving emails from it, send an email to osv...@googlegroups.com.

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

httpserver: properly handle chunked POST requests with body

This bug was discovered when running httpserver unit tests with python
scripts upgraded to version 3. The new version of the requests module chunks
POST requests with body so that they are sent over socket in two parts - the request
and the body.

Our httpserver had a bug in how it consumed such requests. Instead of
completing the request once the body chunk was fully received,
it would try to re-consume the same body chunk as next request and after
failing to, it would send back a 400 (bad request) response.

So this patch simply changes the connection logic to complete handling
request in such scenario and proceed to the next request.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

---

diff --git a/modules/httpserver-api/connection.cc b/modules/httpserver-api/connection.cc

--- a/modules/httpserver-api/connection.cc
+++ b/modules/httpserver-api/connection.cc
@@ -246,8 +246,11 @@ void connection::do_read()
request_.content.append(buffer_.data(), buffer_.data() + bytes_transferred);
if (request_.content.size() < request_.content_length) {
do_read();
- return;
+ } else {
+ request_handler_.handle_request(request_, reply_);
+ do_write();
}
+ return;
}

auto r = request_parser_.parse(