Hi guys, On Mon, Mar 25, 2013 at 06:54:24AM -0400, David Coulson wrote: > > On 3/13/13 7:59 AM, Cyril Bonté wrote: > > > >For now, I don't know where to look but maybe it can be useful to find > >and fix the issue. > > > >I also tried with : > >v1.5-dev8 : it works > >v1.5-dev9 : segfault > >v1.5-dev10 : segfault > >v1.5-dev11 : couldn't compile > >v1.5-dev12 : couldn't compile > >v1.5-dev13 : it doesn't work anymore > > > >Maybe we'll have to study the commits between dev8 and dev9 ? > Does anyone have any suggestions how to further troubleshoot this bug, > or a potential workaround?
Yes, please use this patch, that I'm about to merge into master. And thanks to Cyril for pinging me again on this subject which I initially missed! Cheers, Willy
>From c8fc8216c84a9cb4c42e7932e9931c0e91ba96ba Mon Sep 17 00:00:00 2001 From: Willy Tarreau <[email protected]> Date: Tue, 26 Mar 2013 01:08:21 +0100 Subject: BUG/MEDIUM: http: fix another issue caused by http-send-name-header MIME-Version: 1.0 Content-Type: text/plain; charset=latin1 Content-Transfer-Encoding: 8bit An issue reported by David Coulson is that when using http-send-name-header, the response processing would randomly be performed. The issue was first diagnosed by Cyril Bonté as being related to a time race when processing the closing of the response. In practice, the issue is a bit trickier. It happens that http_send_name_header() did not update msg->sol after a rewrite. This counter is supposed to point to the beginning of the message's body once headers are scheduled for being forwarded. And not updating it means that the first forwarding of the request headers in http_request_forward_body() does not send the correct count, leaving some bytes in chn->to_forward. Then if the server sends its response in a single packet with the close, the stream interface switches to state SI_ST_DIS which in turn moves to SI_ST_CLO in process_session(), and to close the outgoing connection. This is detected by http_request_forward_body(), which then switches the request message to the error state, and syncs all FSMs and removes any response analyser. The response analyser being removed, no processing is performed on the response buffer, which is tunnelled as-is to the client. Of course, the correct fix consists in having http_send_name_header() update msg->sol. Normally this ought not to have been needed, but it is an abuse to modify data already scheduled for being forwarded, so it is expected that such specific handling has to be done there. Better not have generic functions deal with such cases, so that it does not become the standard. --- src/proto_http.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/proto_http.c b/src/proto_http.c index 2fa7598..0f14016 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -4162,8 +4162,11 @@ int http_send_name_header(struct http_txn *txn, struct proxy* be, const char* sr if (old_o) { /* If this was a forwarded request, we must readjust the amount of * data to be forwarded in order to take into account the size - * variations. + * variations. Note that if the request was already scheduled for + * forwarding, it had its req->sol pointing to the body, which + * must then be updated too. */ + txn->req.sol += chn->buf->i - old_i; b_adv(chn->buf, old_o + chn->buf->i - old_i); } -- 1.7.12.2.21.g234cd45.dirty
