RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2025-02-21 05:06:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MEDIUM: http/session: disable client-side expiration only after body

Browse Source 
For a very long time, back in the v1.3 days, we used to rely on a trick
to avoid expiring the client side while transferring a payload to the
server. The problem was that if a client was able to quickly fill the
buffers, and these buffers took some time to reach the server, the
client should not expire while not sending anything.

In order to cover this situation, the client-side timeout was disabled
once the connection to the server was OK, since it implied that we would
at least expire on the server if required.

But there is a drawback to this : if a client stops uploading data before
the end, its timeout is not enforced and we only expire on the server's
timeout, so the logs report a 504.

Since 1.4, we have message body analysers which ensure that we know whether
all the expected data was received or not (HTTP_MSG_DATA or HTTP_MSG_DONE).
So we can fix this problem by disabling the client-side or server-side
timeout at the end of the transfer for the respective side instead of
having it unconditionally in session.c during all the transfer.

With this, the logs now report the correct side for the timeout. Note that
this patch is not enough, because another issue remains : the HTTP body
forwarders do not abort upon timeout, they simply rely on the generic
handling from session.c. So for now, the session is still aborted when
reaching the server timeout, but the culprit is properly reported. A
subsequent patch will address this specific point.

This bug was tagged MEDIUM because of the changes performed. The issue
it fixes is minor however. After some cooling down, it may be backported
to 1.4.

It was reported by and discussed with Rachel Chavez and Patrick Hemmer
on the mailing list.
This commit is contained in:
Willy Tarreau 2014-05-06 22:57:53 +02:00
parent 07c8b24edb
commit b1982e27aa
2 changed files with 11 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/proto_http.c
View File

@ -5341,6 +5341,11 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit
channel_auto_read(req);
}
/* if we received everything, we don't want to expire anymore */
if (msg->msg_state == HTTP_MSG_DONE) {
req->flags |= CF_READ_NOEXP;
req->rex = TICK_ETERNITY;
}
return 0;
}
}
@ -6529,6 +6534,12 @@ int http_response_forward_body(struct session *s, struct channel *res, int an_bi
}
return 1;
}
/* if we received everything, we don't want to expire anymore */
if (msg->msg_state == HTTP_MSG_DONE) {
res->flags |= CF_READ_NOEXP;
res->rex = TICK_ETERNITY;
}
return 0;
}
}

14
src/session.c
View File

@ -2411,20 +2411,6 @@ struct task *process_session(struct task *t)
s->si[0].flags &= ~(SI_FL_ERR|SI_FL_EXP);
s->si[1].flags &= ~(SI_FL_ERR|SI_FL_EXP);
/* Trick: if a request is being waiting for the server to respond,
* and if we know the server can timeout, we don't want the timeout
* to expire on the client side first, but we're still interested
* in passing data from the client to the server (eg: POST). Thus,
* we can cancel the client's request timeout if the server's
* request timeout is set and the server has not yet sent a response.
*/
if ((s->rep->flags & (CF_AUTO_CLOSE|CF_SHUTR)) == 0 &&
(tick_isset(s->req->wex) || tick_isset(s->rep->rex))) {
s->req->flags |= CF_READ_NOEXP;
s->req->rex = TICK_ETERNITY;
}
/* When any of the stream interfaces is attached to an applet,
* we have to call it here. Note that this one may wake the
* task up again. If at least one applet was called, the current