BUG/MEDIUM: h1: don't rewrite the Upgrade header when not cleaning headers

Since commit 8dd49dfab ("BUG/MEDIUM: h1: Skip all h2c values from Upgrade
headers during parsing"), h1_parse_upgrade_header() always rewrites the
Upgrade header value in place to remove the "h2c" and "h2" tokens. This is
desired for the H1 mux, which converts the message to HTX, but not for the
other users of h1_headers_to_hdr_list(). In particular, in TCP mode,
smp_prefetch_http() parses the raw request directly in the channel buffer to
evaluate HTTP sample fetches. There, the request bytes are forwarded as-is
to the server, so the client's Upgrade header was modified in flight.

The rewrite is now only performed when H1_MF_CLEAN_CONN_HDR is set, which is
the flag already used to clean the Connection header up and that is set by
the H1 and FCGI muxes. Without this flag, the value is left untouched and
only the H1_MF_UPG_WEBSOCKET/H1_MF_UPG_HDR flags are computed. The flag's
description was updated accordingly.

This must be backported wherever commit 8dd49dfab was backported, so to all
supported versions.

diff --git a/include/haproxy/h1.h b/include/haproxy/h1.h
index caf9a8e42d5347702a4818680fda09cf516fd8a5..7ae60a00337118c0104912bab00d9e87f9059e60 100644 (file)
--- a/include/haproxy/h1.h
+++ b/include/haproxy/h1.h
@@ -92,7 +92,7 @@ enum h1m_state {
 #define H1_MF_XFER_ENC          0x00000200 // transfer-encoding is present
 #define H1_MF_NO_PHDR           0x00000400 // don't add pseudo-headers in the header list
 #define H1_MF_HDRS_ONLY         0x00000800 // parse headers only
-#define H1_MF_CLEAN_CONN_HDR    0x00001000 // skip close/keep-alive values of connection headers during parsing
+#define H1_MF_CLEAN_CONN_HDR    0x00001000 // skip close/keep-alive values of connection headers and h2c/h2 values of upgrade headers during parsing (rewrites the buffer)
 #define H1_MF_METH_CONNECT      0x00002000 // Set for a response to a CONNECT request
 #define H1_MF_METH_HEAD         0x00004000 // Set for a response to a HEAD request
 #define H1_MF_UPG_WEBSOCKET     0x00008000 // Set for a Websocket upgrade handshake

diff --git a/src/h1.c b/src/h1.c
index 8d23cf9a5234f3d13f47ff4b35813211d4342770..2d33bd31682d8107ff94a2e9965b1a6de3e0484c 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -283,9 +283,11 @@ void h1_parse_connection_header(struct h1m *h1m, struct ist *value)
 
 /* Parse the Upgrade: header of an HTTP/1 request.
  * If "websocket" is found, set H1_MF_UPG_WEBSOCKET flag
- * If "h2c" or "h2" found, the value is skipped. The header value is rewritten
- * in place and the bytes freed at the end of the original value are replaced
- * with spaces so that parsing the same area again gives the same value.
+ * If "h2c" or "h2" found and the flag H1_MF_CLEAN_CONN_HDR is set, the value is
+ * skipped. The header value is rewritten in place and the bytes freed at the
+ * end of the original value are replaced with spaces so that parsing the same
+ * area again gives the same value. Without H1_MF_CLEAN_CONN_HDR, the value is
+ * never modified.
  */
 void h1_parse_upgrade_header(struct h1m *h1m, struct ist *value)
 {
@@ -311,7 +313,8 @@ void h1_parse_upgrade_header(struct h1m *h1m, struct ist *value)
 
                if (isteqi(word, ist("websocket")))
                        h1m->flags |= H1_MF_UPG_WEBSOCKET;
-               else if (isteqi(word, ist("h2c")) || isteqi(word, ist("h2")))
+               else if ((h1m->flags & H1_MF_CLEAN_CONN_HDR) &&
+                        (isteqi(word, ist("h2c")) || isteqi(word, ist("h2"))))
                        goto skip_val;
 
                if (value->ptr + value->len == p) {
@@ -329,7 +332,7 @@ void h1_parse_upgrade_header(struct h1m *h1m, struct ist *value)
        }
 
        /* blank the freed area to make the operation idempotent */
-       if (value->ptr + value->len < e)
+       if ((h1m->flags & H1_MF_CLEAN_CONN_HDR) && value->ptr + value->len < e)
                memset(value->ptr + value->len, ' ', e - (value->ptr + value->len));
 
        if (istlen(*value))