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 471ef21..c24216a 100644
--- a/include/types/proto_http.h
+++ b/include/types/proto_http.h
@@ -187,6 +187,11 @@
#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 52f626e..06a0372 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -4883,8 +4883,17 @@
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 @@
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 5458f1b..ef0f63d 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -863,6 +863,15 @@
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;