From 80a92c02f478dc1b836e0c97c891875437fc54da Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 12 Mar 2014 10:41:13 +0100 Subject: [PATCH] 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. --- include/types/proto_http.h | 5 +++++ src/proto_http.c | 16 ++++++++++++++-- src/proxy.c | 9 +++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/include/types/proto_http.h b/include/types/proto_http.h index 471ef210f..c24216a4b 100644 --- a/include/types/proto_http.h +++ b/include/types/proto_http.h @@ -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 */ diff --git a/src/proto_http.c b/src/proto_http.c index 52f626eba..06a037237 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -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; diff --git a/src/proxy.c b/src/proxy.c index 5458f1bc8..ef0f63de1 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -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;