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 <[email protected]
> <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 <[email protected] <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 [email protected] <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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/fad38e25-526b-4958-b668-3edd0bb79dda%40googlegroups.com.
