Bugs item #3214223, was opened at 2011-03-15 21:06
Message generated for change (Comment added) made by
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3214223&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: hang
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Chris Smowton ()
Assigned to: Daniel Stenberg (bagder)
Summary: Pipelined HTTP requests with a zero-length body broken

Initial Comment:
The scenario:

cURL is running pipelined. It's trying to get a series of three documents from the server, call them http://server/one, two and three. 'one' is zero-length.

What happens:

1. cURL sends all three requests
2. Server sends two replies in quick succession.
3. cURL enters Curl_readwrite --> readwrite_data --> Curl_read
4. We're at the HTTP-read-headers stage, so we don't have an expected length. We read too much, saving the buffer in conn->master_buffer.
5. It's still header-time, so we enter Curl_http_readwrite_headers.
6. The first response's headers indicate Content-Length: 0, so we set "stop_reading"
7. ...and consequently bail out of the do-while loop.

The problem: that buffer saved in conn->master_buffer contains not only the zero-length request's headers, but also the headers and body of its successor requests. However it will never yield them unless we set conn->read_pos using read_rewind, but we only do that if we read a short body, *not* if we set stop_reading, which bails out immediately.

Consequently the responses which were in that buffer are silently dropped, and subsequent requests get paired with replies which are not their own! Example: Requests 1, 2 and 3 meet reply-packets [1, 2] and [3]. Request 1 is zero-length, triggering the bug and silently discarding the rest of its packet. Request 2 is then answered using reply [3]... curse HTTP's lack of a transaction-identifier field!

Solution: read_rewind whenever we've read too much data, not just in the special cases dealt with further down readwrite.

I attach as always a pair of test cases, somewhat tricky this time: a server and a client. They'll prompt you to press enter a few times: the drill should be:

1. Start server ("dumb_server 9000")
2. Start client, no params. We immediately spin up a pipelining, maxconnects=1 multi-context and submit 4 requests, "init", "zero_length", "nonzero_length_one", "nonzero_length_two".
3. We'll immediately submit the first request and the server will answer. So,e purpose is to convince cURL it's dealing with HTTP/1.1 and can pipeline the rest of them.
4. cURL will automatically send the other three as a pipeline and quiesce.
5. Now press return at the server: it'll send a single-packet (hopefully!) reply to the first two requests
6. Press return at cURL's console: it'll consume the first reply and silently eat the second one.
7. Press return at the server: it'll reply to the third request.
8. Press return at the client: it'll consume the third reply (but match it to the second request!) and end.

To check, see /tmp/curl_*_out.

If it works right, you'll get:

curl_zero_out: "init"
curl_first_out: ""
curl_second_out: "nz1"
curl_third_out: "nz2"

If the bug manifests, instead you'll get:

curl_zero_out: "init"
curl_first_out: ""
curl_second_out: "nz2"
curl_third_out: ""

Note that the third request has not returned, whilst the second has been given the value that rightfully belongs to the third.

I'd really appreciate a fix to this one, as I can't find a workaround that isn't disabling pipelining.

----------------------------------------------------------------------

>Comment By: Chris Smowton ()
Date: 2011-04-06 10:41

Message:
I finally got around to making a proper patch for this -- the attached
includes a test case which fails in trunk but works with this patch
applied. It doesn't cause trouble with any other test cases, and I *think*
it's fairly safe, as it only does anything against a pipelined handle.

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2011-03-16 07:44

Message:
Thanks for the report and analysis. I agree that we want this fixed, but
the reality is that we're heavily undermanned in the debugging department
of the curl project, as it consists mostly of just me. Pipelining is
unfortunately not very widely used by libcurl users so it has this kind of
quirks left to smooth away.

This is just a long way of saying: it may take a little while until I can
get to looking into this.

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=3214223&group_id=976
Received on 2011-04-06