BUG/MEDIUM: http: don't start to forward request data before the connect

Currently, "balance url_param check_post" randomly works. If the client
sends chunked data and there's another chunk after the one containing the
data, http_request_forward_body() will advance msg->sov and move the start
of data to the beginning of the last chunk, and get_server_ph_post() will
not find the data.

In order to avoid this, we add an HTTP_MSGF_WAIT_CONN flag whose goal is
to prevent the forwarding code from parsing until the connection is
confirmed, so that we're certain not to fail on a redispatch. Note that
we need to force channel_auto_connect() since the output buffer is empty
and a previous analyser might have stopped auto-connect.

The flag is currently set whenever some L7 POST analysis is needed for a
connect() so that it correctly addresses all corner cases involving a
possible rewind of the buffer, waiting for a better fix.

Note that this has been broken for a very long time. Even all 1.4 versions
seem broken but differently, with ->sov pointing to the end of the arguments.
So the fix should be considered for backporting to all stable releases,
possibly including 1.3 which works differently.
This commit is contained in:
Willy Tarreau 2014-03-12 10:41:13 +01:00
parent da5d4a5560
commit 80a92c02f4
3 changed files with 28 additions and 2 deletions

View File

@ -187,6 +187,11 @@ enum ht_state {
#define HTTP_MSGF_XFER_LEN 0x00000004 /* message xfer size can be determined */
#define HTTP_MSGF_VER_11 0x00000008 /* the message is HTTP/1.1 or above */
/* If this flag is set, we don't process the body until the connect() is confirmed.
* This is only used by the request forwarding function to protect the buffer
* contents if something needs them during a redispatch.
*/
#define HTTP_MSGF_WAIT_CONN 0x00000010 /* Wait for connect() to be confirmed before processing body */
/* Redirect flags */

View File

@ -4883,8 +4883,17 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit
return 1;
}
/* in most states, we should abort in case of early close */
channel_auto_close(req);
/* Some post-connect processing might want us to refrain from starting to
* forward data. Currently, the only reason for this is "balance url_param"
* whichs need to parse/process the request after we've enabled forwarding.
*/
if (unlikely(msg->flags & HTTP_MSGF_WAIT_CONN)) {
if (!(s->rep->flags & CF_READ_ATTACHED)) {
channel_auto_connect(req);
goto missing_data;
}
msg->flags &= ~HTTP_MSGF_WAIT_CONN;
}
/* Note that we don't have to send 100-continue back because we don't
* need the data to complete our job, and it's up to the server to
@ -4906,6 +4915,9 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit
msg->msg_state = HTTP_MSG_DATA;
}
/* in most states, we should abort in case of early close */
channel_auto_close(req);
while (1) {
unsigned int bytes;

View File

@ -863,6 +863,15 @@ int session_set_backend(struct session *s, struct proxy *be)
hdr_idx_init(&s->txn.hdr_idx);
}
/* If an LB algorithm needs to access some pre-parsed body contents,
* we must not start to forward anything until the connection is
* confirmed otherwise we'll lose the pointer to these data and
* prevent the hash from being doable again after a redispatch.
*/
if (be->mode == PR_MODE_HTTP &&
(be->lbprm.algo & (BE_LB_KIND | BE_LB_PARM)) == (BE_LB_KIND_HI | BE_LB_HASH_PRM))
s->txn.req.flags |= HTTP_MSGF_WAIT_CONN;
if (be->options2 & PR_O2_NODELAY) {
s->req->flags |= CF_NEVER_WAIT;
s->rep->flags |= CF_NEVER_WAIT;