On Tuesday, February 18, 2020 at 10:12:56 AM UTC-5, Nadav Har'El wrote:
>
>
> On Mon, Feb 17, 2020 at 11:58 PM Waldemar Kozaczuk <jwkoz...@gmail.com
> <javascript:>> wrote:
>
>> 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 <javascript:>>
>> ---
>> 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;
>> }
>>
>
> 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)
>
> 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 <javascript:>.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/osv-dev/20200217215801.8729-1-jwkozaczuk%40gmail.com
>> .
>>
>
--
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/fad38e25-526b-4958-b668-3edd0bb79dda%40googlegroups.com.
