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.

diff --git a/include/types/proto_http.h b/include/types/proto_http.h
index 471ef210fe4a83aa45d46e00680cf61e6b2f1353..c24216a4bbc5903a10372f852179c57dc4ee1f0d 100644 (file)
--- 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 52f626eba5d5ede97caec3ed32377029aee2d878..06a03723715baf43f72069aefb734eb6d95d6664 100644 (file)
--- 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 5458f1bc81df18050c55781157b8579225c075b2..ef0f63de1f466f9cecc94d24f96b5d9094e2717a 100644 (file)
--- 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;