From 2557f6a3e24ade33f4a367d3ffc8762f4b9305db Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Fri, 14 Sep 2018 16:34:47 +0200
Subject: [PATCH] MEDIUM: h1: better handle transfer-encoding vs content-length

The transfer-encoding header processing was a bit lenient in this part
because it was made to read messages already validated by haproxy. We
absolutely need to reinstate the strict processing defined in RFC7230
as is currently being done in proto_http.c. That is, transfer-encoding
presence alone is enough to cancel content-length, and must be
terminated by the "chunked" token, except in the response where we
can fall back to the close mode if it's not last.

For this we now use a specific parsing function which updates the
flags and we introduce a new flag H1_MF_XFER_ENC indicating that the
transfer-encoding header is present.

Last, if such a header is found, we delete all content-length header
fields found in the message.
---
 include/types/h1.h |  1 +
 src/h1.c           | 62 +++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/include/types/h1.h b/include/types/h1.h
index 2abf4723ca..c6645faa07 100644
--- a/include/types/h1.h
+++ b/include/types/h1.h
@@ -145,6 +145,7 @@ enum h1m_state {
 #define H1_MF_CONN_KAL          0x00000040 // message contains "connection: keep-alive"
 #define H1_MF_CONN_UPG          0x00000080 // message contains "connection: upgrade"
 #define H1_MF_XFER_LEN          0x00000100 // message xfer size can be determined
+#define H1_MF_XFER_ENC          0x00000200 // transfer-encoding is present
 
 /* Note: for a connection to be persistent, we need this for the request :
  *   - one of CLEN or CHNK
diff --git a/src/h1.c b/src/h1.c
index 65e7495afb..d94766132d 100644
--- a/src/h1.c
+++ b/src/h1.c
@@ -660,6 +660,44 @@ void http_msg_analyzer(struct http_msg *msg, struct hdr_idx *idx)
 }
 
 
+/* Parse the Transfer-Encoding: header field of an HTTP/1 request, looking for
+ * "chunked" being the last value, and setting H1_MF_CHNK in h1m->flags only in
+ * this case. Any other token found or any empty header field found will reset
+ * this flag, so that it accurately represents the token's presence at the last
+ * position. The H1_MF_XFER_ENC flag is always set. Note that transfer codings
+ * are case-insensitive (cf RFC7230#4).
+ */
+void h1_parse_xfer_enc_header(struct h1m *h1m, struct ist value)
+{
+	char *e, *n;
+	struct ist word;
+
+	h1m->flags |= H1_MF_XFER_ENC;
+	h1m->flags &= ~H1_MF_CHNK;
+
+	word.ptr = value.ptr - 1; // -1 for next loop's pre-increment
+	e = value.ptr + value.len;
+
+	while (++word.ptr < e) {
+		/* skip leading delimitor and blanks */
+		if (HTTP_IS_LWS(*word.ptr))
+			continue;
+
+		n = http_find_hdr_value_end(word.ptr, e); // next comma or end of line
+		word.len = n - word.ptr;
+
+		/* trim trailing blanks */
+		while (word.len && HTTP_IS_LWS(word.ptr[word.len-1]))
+			word.len--;
+
+		h1m->flags &= ~H1_MF_CHNK;
+		if (isteqi(word, ist("chunked")))
+			h1m->flags |= H1_MF_CHNK;
+
+		word.ptr = n;
+	}
+}
+
 /* Parse the Connection: header of an HTTP/1 request, looking for "close",
  * "keep-alive", and "upgrade" values, and updating h1m->flags according to
  * what was found there. Note that flags are only added, not removed, so the
@@ -1272,8 +1310,7 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
 			}
 
 			if (isteqi(n, ist("transfer-encoding"))) {
-				h1m->flags &= ~H1_MF_CLEN;
-				h1m->flags |= H1_MF_CHNK;
+				h1_parse_xfer_enc_header(h1m, v);
 			}
 			else if (isteqi(n, ist("content-length")) && !(h1m->flags & H1_MF_CHNK)) {
 				h1m->flags |= H1_MF_CLEN;
@@ -1321,10 +1358,23 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
 		if (restarting)
 			goto restart;
 
-		if (h1m->flags & H1_MF_CHNK)
-			state = H1_MSG_CHUNK_SIZE;
-		else
-			state = H1_MSG_DATA;
+		state = H1_MSG_DATA;
+		if (h1m->flags & H1_MF_XFER_ENC) {
+			if (h1m->flags & H1_MF_CLEN) {
+				h1m->flags &= ~H1_MF_CLEN;
+				hdr_count = http_del_hdr(hdr, ist("content-length"));
+			}
+
+			if (h1m->flags & H1_MF_CHNK)
+				state = H1_MSG_CHUNK_SIZE;
+			else if (!(h1m->flags & H1_MF_RESP)) {
+				/* cf RFC7230#3.3.3 : transfer-encoding in
+				 * request without chunked encoding is invalid.
+				 */
+				goto http_msg_invalid;
+			}
+		}
+
 		break;
 
 	default: