From 96b49c789f8f378c5b488c9ddc74da943e9842fc Mon Sep 17 00:00:00 2001 From: Delta Regeer Date: Sun, 2 Aug 2026 00:22:08 -0600 Subject: [PATCH 1/6] Add changelog entries for the duplicate header field fixes The fixes themselves landed in #484 and #488, but neither added a CHANGES.txt entry, so a reader of the changelog had no way to find out that duplicate Host, Content-Length and Content-Type header fields are now rejected. --- CHANGES.txt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index c7f32ea4..54158236 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -19,6 +19,19 @@ Bugfix https://github.com/Pylons/waitress/pull/475 and https://github.com/Pylons/waitress/issues/464 +- A request that contains more than one Host header field is now rejected with + a 400 (Bad Request), rather than being passed to the WSGI application with + the values joined by a comma. See + https://github.com/Pylons/waitress/issues/462 and + https://github.com/Pylons/waitress/pull/484 + +- A request that contains more than one Content-Type header field is now + rejected with a 400 (Bad Request). Content-Type is a singleton field, and + recipients differ in which of the values they pick when it is sent more than + once. The same applies to Content-Length. See + https://github.com/Pylons/waitress/issues/466 and + https://github.com/Pylons/waitress/pull/488 + 3.0.2 (2024-11-16) ------------------ From f3224f746010947a25019cd5915e4338ed448167 Mon Sep 17 00:00:00 2001 From: Delta Regeer Date: Sun, 2 Aug 2026 00:23:17 -0600 Subject: [PATCH 2/6] Require a valid Host header field on HTTP/1.1 requests RFC 9112 section 3.2 requires a 400 (Bad Request) response to any HTTP/1.1 request that lacks a Host header field, or that carries one with an invalid field value. Waitress rejected duplicate Host header fields already, but accepted a request with none at all, and never looked at the value. The value is now checked against the "uri-host [ ':' port ]" grammar of RFC 3986 section 3.2.2 whatever the version of the request, so that a value that isn't a host can't reach the WSGI application and be interpreted differently there than it was here. Note that this means an internationalised domain name has to be punycoded by the client, as it always should have been. The existing tests that sent an HTTP/1.1 request without a Host header field have been updated to send one. See https://github.com/Pylons/waitress/issues/462 --- CHANGES.txt | 10 ++++ src/waitress/parser.py | 27 ++++++++- src/waitress/rfc7230.py | 32 ++++++++++ tests/test_functional.py | 126 ++++++++++++++++++++++++++++----------- tests/test_parser.py | 117 ++++++++++++++++++++++++++++-------- 5 files changed, 252 insertions(+), 60 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 54158236..8aacbf32 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,16 @@ Unreleased ---------- +Backward Incompatibilities +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +- An HTTP/1.1 request that does not contain a Host header field is now rejected + with a 400 (Bad Request), as required by RFC 9112 section 3.2. A Host header + field whose value is not a valid ``uri-host [ ":" port ]`` is rejected the + same way, whatever the HTTP version of the request. Note that this means an + internationalised domain name has to be punycoded by the client, as it always + should have been. See https://github.com/Pylons/waitress/issues/462 + Bugfix ~~~~~~ diff --git a/src/waitress/parser.py b/src/waitress/parser.py index 1af4594d..85c2f758 100644 --- a/src/waitress/parser.py +++ b/src/waitress/parser.py @@ -24,7 +24,7 @@ from waitress.buffers import OverflowableBuffer from waitress.receiver import ChunkedReceiver, FixedStreamReceiver -from waitress.rfc7230 import HEADER_FIELD_RE, ONLY_DIGIT_RE +from waitress.rfc7230 import HEADER_FIELD_RE, HOST_RE, ONLY_DIGIT_RE from waitress.utilities import ( BadRequest, RequestEntityTooLarge, @@ -273,6 +273,19 @@ def parse_header(self, header_plus): self.query, self.fragment, ) = split_uri(uri) + + # RFC 9112 section 3.2 requires a 400 (Bad Request) response to any + # HTTP/1.1 request that lacks a Host header field, or that has a Host + # header field with an invalid field value. Duplicate Host headers are + # already rejected as part of SINGLETON_FIELDS above. + host = headers.get("HOST") + + if host is None: + if version == "1.1": + raise ParsingError("HTTP/1.1 request does not contain a Host header") + else: + validate_uri_host(host, "Host header") + self.url_scheme = self.adj.url_scheme connection = headers.get("CONNECTION", "") @@ -370,6 +383,18 @@ def close(self): body_rcv.getbuf().close() +def validate_uri_host(value, what): + """ + Validate that ``value`` is a "uri-host [ ':' port ]" as required by RFC 7230 + section 5.4 for the Host header field. + + ``what`` names the thing being validated, for use in the error message. + """ + + if not HOST_RE.match(value.encode("latin-1")): + raise ParsingError(f"Invalid {what}") + + def split_uri(uri): # urlsplit handles byte input by returning bytes on py3, so # scheme, netloc, path, query, and fragment are bytes diff --git a/src/waitress/rfc7230.py b/src/waitress/rfc7230.py index 26e64260..6d138262 100644 --- a/src/waitress/rfc7230.py +++ b/src/waitress/rfc7230.py @@ -62,6 +62,38 @@ "(?:;(?P" + CHUNK_EXT_NAME + ")(?:=(?P" + CHUNK_EXT_VAL + "))?)*" ) +# RFC 3986 Section 3.2.2 "Host", which is what RFC 7230 Section 5.4 uses to +# define the value of the Host header field: +# +# host = IP-literal / IPv4address / reg-name +# IP-literal = "[" ( IPv6address / IPvFuture ) "]" +# IPvFuture = "v" 1*HEXDIG "." 1*( unreserved / sub-delims / ":" ) +# reg-name = *( unreserved / pct-encoded / sub-delims ) +# unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" +# sub-delims = "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "=" +# pct-encoded = "%" HEXDIG HEXDIG +# +# IPv4address is a strict subset of reg-name, so it doesn't need to be matched +# separately. The contents of an IP-literal are not validated any further than +# the set of characters that may appear inside of one. +UNRESERVED = r"[A-Za-z0-9\-._~]" +SUB_DELIMS = r"[!$&'()*+,;=]" +PCT_ENCODED = "%" + HEXDIG + HEXDIG +IPV6_LITERAL = r"\[[A-Fa-f0-9:.]+\]" +IPVFUTURE_LITERAL = ( + r"\[[vV]" + HEXDIG + r"+\.(?:" + UNRESERVED + "|" + SUB_DELIMS + "|:)+\\]" +) +IP_LITERAL = "(?:" + IPV6_LITERAL + "|" + IPVFUTURE_LITERAL + ")" +# NB: the alternatives here are disjoint (no character may start more than one +# of them), so this can't backtrack catastrophically +REG_NAME = "(?:" + UNRESERVED + "|" + PCT_ENCODED + "|" + SUB_DELIMS + ")*" +URI_HOST = "(?:" + IP_LITERAL + "|" + REG_NAME + ")" + +# RFC 7230 Section 5.4: Host = uri-host [ ":" port ], where port = *DIGIT. +# Both the uri-host and the port may be empty as far as the grammar goes; the +# callers apply the stricter rules that their context requires. +HOST_RE = re.compile(("^" + URI_HOST + "(?::" + DIGIT + "*)?$").encode("latin-1")) + # Pre-compiled regular expressions for use elsewhere ONLY_HEXDIG_RE = re.compile(("^" + HEXDIG + "+$").encode("latin-1")) ONLY_DIGIT_RE = re.compile(("^" + DIGIT + "+$").encode("latin-1")) diff --git a/tests/test_functional.py b/tests/test_functional.py index fae412c5..11bc9517 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -232,6 +232,36 @@ def test_bad_host_header(self): self.assertEqual(headers.get("server"), "waitress") self.assertTrue(headers.get("date")) + def test_missing_host_header_http11(self): + # RFC 9112 section 3.2 + to_send = b"POST / HTTP/1.1\r\nContent-Length: 0\r\n\r\n" + self.connect() + self.sock.send(to_send) + with self.sock.makefile("rb", 0) as fp: + line, headers, response_body = read_http(fp) + self.assertline(line, "400", "Bad Request", "HTTP/1.1") + + def test_duplicate_host_header_http11(self): + # RFC 9112 section 3.2 + to_send = b"POST / HTTP/1.1\r\nHost: victim1.com\r\nHost: victim2.com\r\n\r\n" + self.connect() + self.sock.send(to_send) + with self.sock.makefile("rb", 0) as fp: + line, headers, response_body = read_http(fp) + # NB: duplicate headers are rejected before the first line of the + # request has been cracked, so the version we know about at that + # point is still the default of HTTP/1.0 + self.assertline(line, "400", "Bad Request", "HTTP/1.0") + + def test_invalid_host_header_http11(self): + # RFC 9112 section 3.2 + to_send = b"POST / HTTP/1.1\r\nHost: victim.com:notaport\r\n\r\n" + self.connect() + self.sock.send(to_send) + with self.sock.makefile("rb", 0) as fp: + line, headers, response_body = read_http(fp) + self.assertline(line, "400", "Bad Request", "HTTP/1.1") + def test_send_with_body(self): to_send = b"GET / HTTP/1.0\r\nContent-Length: 5\r\n\r\n" to_send += b"hello" @@ -322,7 +352,9 @@ def test_many_clients(self): h.close() def test_chunking_request_without_content(self): - header = b"GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + header = ( + b"GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\n\r\n" + ) self.connect() self.sock.send(header) self.sock.send(b"0\r\n\r\n") @@ -337,7 +369,9 @@ def test_chunking_request_with_content(self): control_line = b"20\r\n" # 20 hex = 32 dec s = b"This string has 32 characters.\r\n" expected = s * 12 - header = b"GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + header = ( + b"GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\n\r\n" + ) self.connect() self.sock.send(header) with self.sock.makefile("rb", 0) as fp: @@ -355,7 +389,9 @@ def test_chunking_request_with_content(self): def test_broken_chunked_encoding(self): control_line = b"20\r\n" # 20 hex = 32 dec s = b"This string has 32 characters.\r\n" - to_send = b"GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + to_send = ( + b"GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\n\r\n" + ) to_send += control_line + s + b"\r\n" # garbage in input to_send += b"garbage\r\n" @@ -379,7 +415,9 @@ def test_broken_chunked_encoding(self): def test_broken_chunked_encoding_invalid_hex(self): control_line = b"0x20\r\n" # 20 hex = 32 dec s = b"This string has 32 characters.\r\n" - to_send = b"GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + to_send = ( + b"GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\n\r\n" + ) to_send += control_line + s + b"\r\n" self.connect() self.sock.send(to_send) @@ -401,7 +439,9 @@ def test_broken_chunked_encoding_invalid_hex(self): def test_broken_chunked_encoding_invalid_extension(self): control_line = b"20;invalid=\r\n" # 20 hex = 32 dec s = b"This string has 32 characters.\r\n" - to_send = b"GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + to_send = ( + b"GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\n\r\n" + ) to_send += control_line + s + b"\r\n" self.connect() self.sock.send(to_send) @@ -423,7 +463,9 @@ def test_broken_chunked_encoding_invalid_extension(self): def test_broken_chunked_encoding_missing_chunk_end(self): control_line = b"20\r\n" # 20 hex = 32 dec s = b"This string has 32 characters.\r\n" - to_send = b"GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + to_send = ( + b"GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\n\r\n" + ) to_send += control_line + s # garbage in input to_send += b"garbage" @@ -484,7 +526,10 @@ def test_keepalive_http_11(self): # All connections are kept alive, unless stated otherwise data = b"Default: Keep me alive" - s = b"GET / HTTP/1.1\r\nContent-Length: %d\r\n\r\n%s" % (len(data), data) + s = b"GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: %d\r\n\r\n%s" % ( + len(data), + data, + ) self.connect() self.sock.send(s) response = httplib.HTTPResponse(self.sock) @@ -496,7 +541,7 @@ def test_keepalive_http11_explicit(self): # Explicitly set keep-alive data = b"Default: Keep me alive" s = ( - b"GET / HTTP/1.1\r\n" + b"GET / HTTP/1.1\r\nHost: localhost\r\n" b"Connection: keep-alive\r\n" b"Content-Length: %d\r\n" b"\r\n" @@ -513,7 +558,7 @@ def test_keepalive_http11_connclose(self): # specifying Connection: close explicitly data = b"Don't keep me alive" s = ( - b"GET / HTTP/1.1\r\n" + b"GET / HTTP/1.1\r\nHost: localhost\r\n" b"Connection: close\r\n" b"Content-Length: %d\r\n" b"\r\n" @@ -606,7 +651,7 @@ def test_expect_continue(self): # specifying Connection: close explicitly data = b"I have expectations" to_send = ( - b"GET / HTTP/1.1\r\n" + b"GET / HTTP/1.1\r\nHost: localhost\r\n" b"Connection: close\r\n" b"Content-Length: %d\r\n" b"Expect: 100-continue\r\n" @@ -774,7 +819,10 @@ def test_http10_listlentwo(self): def test_http11_generator(self): body = string.ascii_letters body = body.encode("latin-1") - to_send = b"GET / HTTP/1.1\r\nContent-Length: %d\r\n\r\n" % len(body) + to_send = ( + b"GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: %d\r\n\r\n" + % len(body) + ) to_send += body self.connect() self.sock.send(to_send) @@ -796,7 +844,10 @@ def test_http11_generator(self): def test_http11_list(self): body = string.ascii_letters.encode("latin-1") - to_send = b"GET /list HTTP/1.1\r\nContent-Length: %d\r\n\r\n" % len(body) + to_send = ( + b"GET /list HTTP/1.1\r\nHost: localhost\r\nContent-Length: %d\r\n\r\n" + % len(body) + ) to_send += body self.connect() self.sock.send(to_send) @@ -813,7 +864,10 @@ def test_http11_list(self): def test_http11_listlentwo(self): body = string.ascii_letters.encode("latin-1") - to_send = b"GET /list_lentwo HTTP/1.1\r\nContent-Length: %d\r\n\r\n" % len(body) + to_send = ( + b"GET /list_lentwo HTTP/1.1\r\nHost: localhost\r\nContent-Length: %d\r\n\r\n" + % len(body) + ) to_send += body self.connect() self.sock.send(to_send) @@ -951,7 +1005,7 @@ def tearDown(self): def test_request_headers_too_large_http11(self): body = b"" bad_headers = b"X-Random-Header: 100\r\n" * int(self.toobig / 20) - to_send = b"GET / HTTP/1.1\r\nContent-Length: 0\r\n" + to_send = b"GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 0\r\n" to_send += bad_headers to_send += b"\r\n\r\n" to_send += body @@ -1043,7 +1097,7 @@ def test_request_body_too_large_with_no_cl_http10_keepalive(self): def test_request_body_too_large_with_wrong_cl_http11(self): body = b"a" * self.toobig - to_send = b"GET / HTTP/1.1\r\nContent-Length: 5\r\n\r\n" + to_send = b"GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 5\r\n\r\n" to_send += body self.connect() self.sock.send(to_send) @@ -1064,7 +1118,7 @@ def test_request_body_too_large_with_wrong_cl_http11(self): def test_request_body_too_large_with_wrong_cl_http11_connclose(self): body = b"a" * self.toobig - to_send = b"GET / HTTP/1.1\r\nContent-Length: 5\r\nConnection: close\r\n\r\n" + to_send = b"GET / HTTP/1.1\r\nHost: localhost\r\nContent-Length: 5\r\nConnection: close\r\n\r\n" to_send += body self.connect() self.sock.send(to_send) @@ -1080,7 +1134,7 @@ def test_request_body_too_large_with_wrong_cl_http11_connclose(self): def test_request_body_too_large_with_no_cl_http11(self): body = b"a" * self.toobig - to_send = b"GET / HTTP/1.1\r\n\r\n" + to_send = b"GET / HTTP/1.1\r\nHost: localhost\r\n\r\n" to_send += body self.connect() self.sock.send(to_send) @@ -1104,7 +1158,7 @@ def test_request_body_too_large_with_no_cl_http11(self): def test_request_body_too_large_with_no_cl_http11_connclose(self): body = b"a" * self.toobig - to_send = b"GET / HTTP/1.1\r\nConnection: close\r\n\r\n" + to_send = b"GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n" to_send += body self.connect() self.sock.send(to_send) @@ -1121,7 +1175,9 @@ def test_request_body_too_large_with_no_cl_http11_connclose(self): def test_request_body_too_large_chunked_encoding(self): control_line = b"20;\r\n" # 20 hex = 32 dec s = b"This string has 32 characters.\r\n" - to_send = b"GET / HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n" + to_send = ( + b"GET / HTTP/1.1\r\nHost: localhost\r\nTransfer-Encoding: chunked\r\n\r\n" + ) repeat = control_line + s to_send += repeat * ((self.toobig // len(repeat)) + 1) self.connect() @@ -1163,7 +1219,7 @@ def test_before_start_response_http_10(self): self.assertRaises(ConnectionClosed, read_http, fp) def test_before_start_response_http_11(self): - to_send = b"GET /before_start_response HTTP/1.1\r\n\r\n" + to_send = b"GET /before_start_response HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() self.sock.send(to_send) with self.sock.makefile("rb", 0) as fp: @@ -1181,7 +1237,7 @@ def test_before_start_response_http_11(self): self.assertRaises(ConnectionClosed, read_http, fp) def test_before_start_response_http_11_close(self): - to_send = b"GET /before_start_response HTTP/1.1\r\nConnection: close\r\n\r\n" + to_send = b"GET /before_start_response HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n" self.connect() self.sock.send(to_send) with self.sock.makefile("rb", 0) as fp: @@ -1219,7 +1275,7 @@ def test_after_start_response_http10(self): self.assertRaises(ConnectionClosed, read_http, fp) def test_after_start_response_http11(self): - to_send = b"GET /after_start_response HTTP/1.1\r\n\r\n" + to_send = b"GET /after_start_response HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() self.sock.send(to_send) with self.sock.makefile("rb", 0) as fp: @@ -1237,7 +1293,7 @@ def test_after_start_response_http11(self): self.assertRaises(ConnectionClosed, read_http, fp) def test_after_start_response_http11_close(self): - to_send = b"GET /after_start_response HTTP/1.1\r\nConnection: close\r\n\r\n" + to_send = b"GET /after_start_response HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n" self.connect() self.sock.send(to_send) with self.sock.makefile("rb", 0) as fp: @@ -1256,7 +1312,7 @@ def test_after_start_response_http11_close(self): self.assertRaises(ConnectionClosed, read_http, fp) def test_after_write_cb(self): - to_send = b"GET /after_write_cb HTTP/1.1\r\n\r\n" + to_send = b"GET /after_write_cb HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() self.sock.send(to_send) with self.sock.makefile("rb", 0) as fp: @@ -1268,7 +1324,7 @@ def test_after_write_cb(self): self.assertRaises(ConnectionClosed, read_http, fp) def test_in_generator(self): - to_send = b"GET /in_generator HTTP/1.1\r\n\r\n" + to_send = b"GET /in_generator HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() self.sock.send(to_send) with self.sock.makefile("rb", 0) as fp: @@ -1305,7 +1361,7 @@ def test_expose_tracebacks_http_10(self): self.assertRaises(ConnectionClosed, read_http, fp) def test_expose_tracebacks_http_11(self): - to_send = b"GET / HTTP/1.1\r\n\r\n" + to_send = b"GET / HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() self.sock.send(to_send) with self.sock.makefile("rb", 0) as fp: @@ -1333,7 +1389,7 @@ def tearDown(self): self.stop_subprocess() def test_filelike_http11(self): - to_send = b"GET /filelike HTTP/1.1\r\n\r\n" + to_send = b"GET /filelike HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() @@ -1350,7 +1406,7 @@ def test_filelike_http11(self): # connection has not been closed def test_filelike_nocl_http11(self): - to_send = b"GET /filelike_nocl HTTP/1.1\r\n\r\n" + to_send = b"GET /filelike_nocl HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() @@ -1367,7 +1423,7 @@ def test_filelike_nocl_http11(self): # connection has not been closed def test_filelike_shortcl_http11(self): - to_send = b"GET /filelike_shortcl HTTP/1.1\r\n\r\n" + to_send = b"GET /filelike_shortcl HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() @@ -1385,7 +1441,7 @@ def test_filelike_shortcl_http11(self): # connection has not been closed def test_filelike_longcl_http11(self): - to_send = b"GET /filelike_longcl HTTP/1.1\r\n\r\n" + to_send = b"GET /filelike_longcl HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() @@ -1402,7 +1458,7 @@ def test_filelike_longcl_http11(self): # connection has not been closed def test_notfilelike_http11(self): - to_send = b"GET /notfilelike HTTP/1.1\r\n\r\n" + to_send = b"GET /notfilelike HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() @@ -1419,7 +1475,7 @@ def test_notfilelike_http11(self): # connection has not been closed def test_notfilelike_iobase_http11(self): - to_send = b"GET /notfilelike_iobase HTTP/1.1\r\n\r\n" + to_send = b"GET /notfilelike_iobase HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() @@ -1436,7 +1492,7 @@ def test_notfilelike_iobase_http11(self): # connection has not been closed def test_notfilelike_nocl_http11(self): - to_send = b"GET /notfilelike_nocl HTTP/1.1\r\n\r\n" + to_send = b"GET /notfilelike_nocl HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() @@ -1452,7 +1508,7 @@ def test_notfilelike_nocl_http11(self): self.assertRaises(ConnectionClosed, read_http, fp) def test_notfilelike_shortcl_http11(self): - to_send = b"GET /notfilelike_shortcl HTTP/1.1\r\n\r\n" + to_send = b"GET /notfilelike_shortcl HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() @@ -1470,7 +1526,7 @@ def test_notfilelike_shortcl_http11(self): # connection has not been closed def test_notfilelike_longcl_http11(self): - to_send = b"GET /notfilelike_longcl HTTP/1.1\r\n\r\n" + to_send = b"GET /notfilelike_longcl HTTP/1.1\r\nHost: localhost\r\n\r\n" self.connect() diff --git a/tests/test_parser.py b/tests/test_parser.py index 5f341ae9..8e103fd2 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -89,9 +89,69 @@ def test_received_duplicate_content_type_header(self): self.assertIsInstance(self.parser.error, BadRequest) self.assertTrue(self.parser.error.body.startswith("Duplicate header:")) + def test_received_missing_host_header_11(self): + # RFC 9112 section 3.2: MUST reject an HTTP/1.1 request that lacks a + # Host header field + data = b"POST / HTTP/1.1\r\nContent-Length: 0\r\n\r\n" + result = self.parser.received(data) + self.assertEqual(result, len(data)) + self.assertTrue(self.parser.completed) + self.assertIsInstance(self.parser.error, BadRequest) + self.assertIn("does not contain a Host header", self.parser.error.body) + + def test_received_missing_host_header_10(self): + # the Host header is only required from HTTP/1.1 onwards + data = b"POST / HTTP/1.0\r\nContent-Length: 0\r\n\r\n" + result = self.parser.received(data) + self.assertEqual(result, len(data)) + self.assertTrue(self.parser.completed) + self.assertIsNone(self.parser.error) + + def test_received_valid_host_headers(self): + for host in ( + b"example.com", + b"example.com:8080", + b"example.com.", + b"under_score.example.com", + b"127.0.0.1:80", + b"[::1]", + b"[::1]:8080", + b"[v7.host:in-the:future]", + b"xn--n3h.example", + b"", + ): + with self.subTest(host=host): + parser = HTTPRequestParser(Adjustments()) + data = b"GET / HTTP/1.1\r\nHost: " + host + b"\r\n\r\n" + parser.received(data) + self.assertIsNone(parser.error) + self.assertEqual(parser.headers["HOST"], host.decode("latin-1")) + + def test_received_invalid_host_headers(self): + # RFC 9112 section 3.2: MUST reject a Host header field with an invalid + # field value + for host in ( + b"exa mple.com", # whitespace is not allowed in a uri-host + b"example.com:80x", # the port must be numeric + b"example.com:80:80", + b"user@example.com", # a userinfo subcomponent is not an uri-host + b"example.com/path", + b"example.com?query", + b"[::1", # unterminated IP-literal + b"exampl\xc3\xa9.com", # IDNs need to be punycoded by the client + ): + with self.subTest(host=host): + parser = HTTPRequestParser(Adjustments()) + data = b"GET / HTTP/1.1\r\nHost: " + host + b"\r\n\r\n" + parser.received(data) + self.assertTrue(parser.completed) + self.assertIsInstance(parser.error, BadRequest) + self.assertIn("Invalid Host header", parser.error.body) + def test_received_bad_transfer_encoding(self): data = ( b"GET /foobar HTTP/1.1\r\n" + b"Host: example.com\r\n" b"Transfer-Encoding: foo\r\n" b"\r\n" b"1d;\r\n" @@ -99,7 +159,7 @@ def test_received_bad_transfer_encoding(self): b"0\r\n\r\n" ) result = self.parser.received(data) - self.assertEqual(result, 48) + self.assertEqual(result, 67) self.assertTrue(self.parser.completed) self.assertIsInstance(self.parser.error, ServerNotImplemented) @@ -153,6 +213,7 @@ def test_received_body_too_large(self): self.parser.adj.max_request_body_size = 2 data = ( b"GET /foobar HTTP/1.1\r\n" + b"Host: example.com\r\n" b"Transfer-Encoding: chunked\r\n" b"X-Foo: 1\r\n" b"\r\n" @@ -162,7 +223,7 @@ def test_received_body_too_large(self): ) result = self.parser.received(data) - self.assertEqual(result, 62) + self.assertEqual(result, 81) self.parser.received(data[result:]) self.assertTrue(self.parser.completed) self.assertIsInstance(self.parser.error, RequestEntityTooLarge) @@ -170,6 +231,7 @@ def test_received_body_too_large(self): def test_received_error_from_parser(self): data = ( b"GET /foobar HTTP/1.1\r\n" + b"Host: example.com\r\n" b"Transfer-Encoding: chunked\r\n" b"X-Foo: 1\r\n" b"\r\n" @@ -186,6 +248,7 @@ def test_received_error_from_parser(self): def test_received_chunked_completed_sets_content_length(self): data = ( b"GET /foobar HTTP/1.1\r\n" + b"Host: example.com\r\n" b"Transfer-Encoding: chunked\r\n" b"X-Foo: 1\r\n" b"\r\n" @@ -194,7 +257,7 @@ def test_received_chunked_completed_sets_content_length(self): b"0\r\n\r\n" ) result = self.parser.received(data) - self.assertEqual(result, 62) + self.assertEqual(result, 81) data = data[result:] result = self.parser.received(data) self.assertTrue(self.parser.completed) @@ -259,19 +322,21 @@ def test_parse_header_multiple_content_length(self): def test_parse_header_11_te_chunked(self): # NB: test that capitalization of header value is unimportant - data = b"GET /foobar HTTP/1.1\r\ntransfer-encoding: ChUnKed\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\ntransfer-encoding: ChUnKed\r\n" self.parser.parse_header(data) self.assertEqual(self.parser.body_rcv.__class__.__name__, "ChunkedReceiver") def test_parse_header_11_te_chunked_with_cl_close_connection(self): # NB: test that capitalization of header value is unimportant - data = b"GET /foobar HTTP/1.1\r\ntransfer-encoding: chunked\r\ncontent-length: 10\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\ntransfer-encoding: chunked\r\ncontent-length: 10\r\n" self.parser.parse_header(data) self.assertEqual(self.parser.body_rcv.__class__.__name__, "ChunkedReceiver") self.assertEqual(self.parser.connection_close, True) def test_parse_header_transfer_encoding_invalid(self): - data = b"GET /foobar HTTP/1.1\r\ntransfer-encoding: gzip\r\n" + data = ( + b"GET /foobar HTTP/1.1\r\nHost: example.com\r\ntransfer-encoding: gzip\r\n" + ) try: self.parser.parse_header(data) @@ -281,7 +346,7 @@ def test_parse_header_transfer_encoding_invalid(self): self.assertTrue(False) def test_parse_header_transfer_encoding_invalid_multiple(self): - data = b"GET /foobar HTTP/1.1\r\ntransfer-encoding: gzip\r\ntransfer-encoding: chunked\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\ntransfer-encoding: gzip\r\ntransfer-encoding: chunked\r\n" try: self.parser.parse_header(data) @@ -291,7 +356,7 @@ def test_parse_header_transfer_encoding_invalid_multiple(self): self.assertTrue(False) def test_parse_header_transfer_encoding_invalid_multiple_chunked(self): - data = b"GET /foobar HTTP/1.1\r\ntransfer-encoding: chunked\r\ntransfer-encoding: chunked\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\ntransfer-encoding: chunked\r\ntransfer-encoding: chunked\r\n" try: self.parser.parse_header(data) @@ -304,7 +369,7 @@ def test_parse_header_transfer_encoding_invalid_multiple_chunked(self): self.assertTrue(False) def test_parse_header_transfer_encoding_invalid_whitespace(self): - data = b"GET /foobar HTTP/1.1\r\nTransfer-Encoding:\x85chunked\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nTransfer-Encoding:\x85chunked\r\n" try: self.parser.parse_header(data) @@ -319,7 +384,7 @@ def test_parse_header_transfer_encoding_invalid_unicode(self): # which if waitress were to accidentally do the wrong thing get # lowercased to just the ascii "k" due to unicode collisions during # transformation - data = b"GET /foobar HTTP/1.1\r\nTransfer-Encoding: chun\xe2\x84\xaaed\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nTransfer-Encoding: chun\xe2\x84\xaaed\r\n" try: self.parser.parse_header(data) @@ -329,12 +394,12 @@ def test_parse_header_transfer_encoding_invalid_unicode(self): self.assertTrue(False) def test_parse_header_11_expect_continue(self): - data = b"GET /foobar HTTP/1.1\r\nexpect: 100-continue\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nexpect: 100-continue\r\n" self.parser.parse_header(data) self.assertTrue(self.parser.expect_continue) def test_parse_header_connection_close(self): - data = b"GET /foobar HTTP/1.1\r\nConnection: close\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nConnection: close\r\n" self.parser.parse_header(data) self.assertTrue(self.parser.connection_close) @@ -395,7 +460,7 @@ def test_parse_header_invalid_whitespace(self): self.assertTrue(False) def test_parse_header_invalid_whitespace_vtab(self): - data = b"GET /foobar HTTP/1.1\r\nfoo:\x0bbar\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo:\x0bbar\r\n" try: self.parser.parse_header(data) except ParsingError as e: @@ -404,7 +469,7 @@ def test_parse_header_invalid_whitespace_vtab(self): self.assertTrue(False) def test_parse_header_invalid_no_colon(self): - data = b"GET /foobar HTTP/1.1\r\nfoo: bar\r\nnotvalid\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: bar\r\nnotvalid\r\n" try: self.parser.parse_header(data) except ParsingError as e: @@ -413,7 +478,7 @@ def test_parse_header_invalid_no_colon(self): self.assertTrue(False) def test_parse_header_invalid_folding_spacing(self): - data = b"GET /foobar HTTP/1.1\r\nfoo: bar\r\n\t\x0bbaz\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: bar\r\n\t\x0bbaz\r\n" try: self.parser.parse_header(data) except ParsingError as e: @@ -422,7 +487,9 @@ def test_parse_header_invalid_folding_spacing(self): self.assertTrue(False) def test_parse_header_invalid_chars(self): - data = b"GET /foobar HTTP/1.1\r\nfoo: bar\r\nfoo: \x0bbaz\r\n" + data = ( + b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: bar\r\nfoo: \x0bbaz\r\n" + ) try: self.parser.parse_header(data) except ParsingError as e: @@ -431,12 +498,14 @@ def test_parse_header_invalid_chars(self): self.assertTrue(False) def test_parse_header_other_whitespace(self): - data = b"GET /foobar HTTP/1.1\r\nfoo: \xa0something\x85\r\n" + data = ( + b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: \xa0something\x85\r\n" + ) self.parser.parse_header(data) self.assertEqual(self.parser.headers["FOO"], "\xa0something\x85") def test_parse_header_empty(self): - data = b"GET /foobar HTTP/1.1\r\nfoo: bar\r\nempty:\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: bar\r\nempty:\r\n" self.parser.parse_header(data) self.assertIn("EMPTY", self.parser.headers) @@ -445,21 +514,21 @@ def test_parse_header_empty(self): self.assertEqual(self.parser.headers["FOO"], "bar") def test_parse_header_multiple_values(self): - data = b"GET /foobar HTTP/1.1\r\nfoo: bar, whatever, more, please, yes\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: bar, whatever, more, please, yes\r\n" self.parser.parse_header(data) self.assertIn("FOO", self.parser.headers) self.assertEqual(self.parser.headers["FOO"], "bar, whatever, more, please, yes") def test_parse_header_multiple_values_header_folded(self): - data = b"GET /foobar HTTP/1.1\r\nfoo: bar, whatever,\r\n more, please, yes\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: bar, whatever,\r\n more, please, yes\r\n" self.parser.parse_header(data) self.assertIn("FOO", self.parser.headers) self.assertEqual(self.parser.headers["FOO"], "bar, whatever, more, please, yes") def test_parse_header_multiple_values_header_folded_multiple(self): - data = b"GET /foobar HTTP/1.1\r\nfoo: bar, whatever,\r\n more\r\nfoo: please, yes\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: bar, whatever,\r\n more\r\nfoo: please, yes\r\n" self.parser.parse_header(data) self.assertIn("FOO", self.parser.headers) @@ -467,14 +536,14 @@ def test_parse_header_multiple_values_header_folded_multiple(self): def test_parse_header_multiple_values_extra_space(self): # Tests errata from: https://www.rfc-editor.org/errata_search.php?rfc=7230&eid=4189 - data = b"GET /foobar HTTP/1.1\r\nfoo: abrowser/0.001 (C O M M E N T)\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: abrowser/0.001 (C O M M E N T)\r\n" self.parser.parse_header(data) self.assertIn("FOO", self.parser.headers) self.assertEqual(self.parser.headers["FOO"], "abrowser/0.001 (C O M M E N T)") def test_parse_header_invalid_backtrack_bad(self): - data = b"GET /foobar HTTP/1.1\r\nfoo: bar\r\nfoo: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\x10\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\nfoo: bar\r\nfoo: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\x10\r\n" try: self.parser.parse_header(data) except ParsingError as e: @@ -483,7 +552,7 @@ def test_parse_header_invalid_backtrack_bad(self): self.assertTrue(False) def test_parse_header_short_values(self): - data = b"GET /foobar HTTP/1.1\r\none: 1\r\ntwo: 22\r\n" + data = b"GET /foobar HTTP/1.1\r\nHost: example.com\r\none: 1\r\ntwo: 22\r\n" self.parser.parse_header(data) self.assertIn("ONE", self.parser.headers) From 2db35ac7d4e3034d2ad138a120ea7a6c9f10190e Mon Sep 17 00:00:00 2001 From: Delta Regeer Date: Sun, 2 Aug 2026 00:58:06 -0600 Subject: [PATCH 3/6] Use the absolute-form request-target authority as the Host RFC 9112 section 3.2.2 requires that an origin server receiving a request with an absolute-form request-target ignore the Host header field and use the host information from the request-target instead. Waitress did the opposite: it parsed the authority out of the request-target into proxy_netloc, then never looked at it again, so "GET http://evil.com/page HTTP/1.1" with "Host: victim.com" reached the application as victim.com. Anything in front of Waitress that follows the RFC would have routed that request to evil.com, and a disagreement of that shape is the basis of a host confusion attack. The authority now replaces the Host header field value, and is checked the same way a Host header field value is. That rejects a userinfo subcomponent, since "@" may not appear in a uri-host, and RFC 9110 section 4.2.1 separately requires that an "http" URI with an empty host be rejected as invalid. A CONNECT is excluded, because it uses the authority-form of request-target and urlsplit() reads that as a scheme followed by a path, which would otherwise make "CONNECT example.com:443" look like an absolute-form with an empty authority. Waitress leaves it to the WSGI application to decide what to do with a CONNECT, so its request-target is passed through untouched. A request-target that starts with "//" still parses as a path rather than as an authority, which keeps the behaviour asked for in #260. See https://github.com/Pylons/waitress/issues/467 --- CHANGES.txt | 63 +++++++++++++++++++++++++++++++++++++ src/waitress/parser.py | 28 ++++++++++++++++- tests/test_functional.py | 12 +++++++ tests/test_parser.py | 68 +++++++++++++++++++++++++++++++++++++++- 4 files changed, 169 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 8aacbf32..1a41f9db 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -11,6 +11,69 @@ Backward Incompatibilities internationalised domain name has to be punycoded by the client, as it always should have been. See https://github.com/Pylons/waitress/issues/462 +- When a request uses the absolute-form of request-target, the authority from + the request-target is now used as the value of ``HTTP_HOST`` in the environ, + and the Host header field of the request is ignored. RFC 9112 section 3.2.2 + requires this of an origin server, so that a request such as + ``GET http://evil.com/ HTTP/1.1`` with ``Host: victim.com`` can't be + interpreted one way by Waitress and another way by anything in front of it. + An absolute-form request-target with an empty authority, or with a userinfo + subcomponent, is rejected with a 400 (Bad Request). See + https://github.com/Pylons/waitress/issues/467 + +Bugfix +~~~~~~ + +- Renamed the HTTP header "Trailers" to "Trailer" to fix a typo and comply with + the correct header name as specified in RFC 7230. + +- Waitress will now drop a request if the Transer-Encoding is set twice in the + request, previously it would decode the chunks and pass it along to the WSGI + application with an appropriate content length. See + https://github.com/Pylons/waitress/issues/465 and + https://github.com/Pylons/waitress/pull/474 + +- When encountering a request that has both Content-Length set and + Transfer-Encoding of chunked we now close the connection after it is + completed to comply with the requirements of RFC9112. See + https://github.com/Pylons/waitress/pull/475 and + https://github.com/Pylons/waitress/issues/464 + +- A request that contains more than one Host header field is now rejected with + a 400 (Bad Request), rather than being passed to the WSGI application with + the values joined by a comma. See + https://github.com/Pylons/waitress/issues/462 and + https://github.com/Pylons/waitress/pull/484 + +- A request that contains more than one Content-Type header field is now + rejected with a 400 (Bad Request). Content-Type is a singleton field, and + recipients differ in which of the values they pick when it is sent more than + once. The same applies to Content-Length. See + https://github.com/Pylons/waitress/issues/466 and + https://github.com/Pylons/waitress/pull/488 + +3.0.2 (2024-11-16) +------------------ + +Security +~~~~~~~~ + +- When using Waitress to process trusted proxy headers, Waitress will now + update the headers to drop any untrusted values, thereby making sure that + WSGI apps only get trusted and validated values that Waitress itself used to + update the environ. See https://github.com/Pylons/waitress/pull/452 and + https://github.com/Pylons/waitress/issues/451 + + +3.0.1 (2024-10-28) +------------------ + +Backward Incompatibilities +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +- Python 3.8 is no longer supported. + See https://github.com/Pylons/waitress/pull/445. + Bugfix ~~~~~~ diff --git a/src/waitress/parser.py b/src/waitress/parser.py index 85c2f758..a7e52c66 100644 --- a/src/waitress/parser.py +++ b/src/waitress/parser.py @@ -274,6 +274,31 @@ def parse_header(self, header_plus): self.fragment, ) = split_uri(uri) + # NB: the check for CONNECT here is not an attempt to validate the + # request, Waitress leaves it to the WSGI application to decide what to + # do with a CONNECT. It is that a CONNECT uses the authority-form of + # request-target, which urlsplit() reads as a scheme followed by a path + # ("example.com:443" comes back as the scheme "example.com" and the + # path "443"), and an authority-form is not an absolute-form. + if self.proxy_scheme and command != "CONNECT": + # This is an absolute-form request-target. RFC 9112 section 3.2.2 + # says that an origin server MUST ignore the received Host header + # field and use the host information from the request-target + # instead, so that the two can't disagree about which host the + # request was meant for. + # + # validate_uri_host() rejects a userinfo subcomponent along with + # everything else that isn't a valid uri-host, since "@" may not + # appear in one. RFC 9110 section 4.2.1 requires that an "http" + # URI with an empty host be rejected as invalid, so an authority + # is required here rather than merely allowed. + if not self.proxy_netloc: + raise ParsingError("Empty authority in absolute-form request-target") + + validate_uri_host(self.proxy_netloc, "request-target authority") + + headers["HOST"] = self.proxy_netloc + # RFC 9112 section 3.2 requires a 400 (Bad Request) response to any # HTTP/1.1 request that lacks a Host header field, or that has a Host # header field with an invalid field value. Duplicate Host headers are @@ -386,7 +411,8 @@ def close(self): def validate_uri_host(value, what): """ Validate that ``value`` is a "uri-host [ ':' port ]" as required by RFC 7230 - section 5.4 for the Host header field. + section 5.4 for the Host header field, and by RFC 3986 section 3.2 for the + authority of an absolute-form request-target. ``what`` names the thing being validated, for use in the error message. """ diff --git a/tests/test_functional.py b/tests/test_functional.py index 11bc9517..03156b55 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -262,6 +262,18 @@ def test_invalid_host_header_http11(self): line, headers, response_body = read_http(fp) self.assertline(line, "400", "Bad Request", "HTTP/1.1") + def test_absolute_form_request_target(self): + # RFC 9112 section 3.2.2: the authority of the request-target wins over + # the Host header + to_send = b"GET http://evil.com/page HTTP/1.1\r\nHost: victim.com\r\n\r\n" + self.connect() + self.sock.send(to_send) + with self.sock.makefile("rb", 0) as fp: + line, headers, echo = self._read_echo(fp) + self.assertline(line, "200", "OK", "HTTP/1.1") + self.assertEqual(echo.headers["HOST"], "evil.com") + self.assertEqual(echo.path_info, "/page") + def test_send_with_body(self): to_send = b"GET / HTTP/1.0\r\nContent-Length: 5\r\n\r\n" to_send += b"hello" diff --git a/tests/test_parser.py b/tests/test_parser.py index 8e103fd2..8cceb54c 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -148,6 +148,69 @@ def test_received_invalid_host_headers(self): self.assertIsInstance(parser.error, BadRequest) self.assertIn("Invalid Host header", parser.error.body) + def test_received_absolute_form_overrides_host_header(self): + # RFC 9112 section 3.2.2: an origin server MUST ignore the Host header + # field of a request with an absolute-form request-target and use the + # host information from the request-target instead + data = b"GET http://evil.com/page HTTP/1.1\r\nHost: victim.com\r\n\r\n" + result = self.parser.received(data) + self.assertEqual(result, len(data)) + self.assertTrue(self.parser.completed) + self.assertIsNone(self.parser.error) + self.assertEqual(self.parser.headers["HOST"], "evil.com") + self.assertEqual(self.parser.path, "/page") + + def test_received_absolute_form_without_host_header(self): + data = b"GET http://evil.com:8080/page HTTP/1.1\r\n\r\n" + self.parser.received(data) + self.assertTrue(self.parser.completed) + self.assertIsNone(self.parser.error) + self.assertEqual(self.parser.headers["HOST"], "evil.com:8080") + + def test_received_absolute_form_invalid_authority(self): + for uri in ( + b"http:///page", # RFC 9110 4.2.1: MUST reject an empty host + b"http://user@evil.com/page", # userinfo is not allowed + b"http://evil.com:80x/page", + ): + with self.subTest(uri=uri): + parser = HTTPRequestParser(Adjustments()) + data = b"GET " + uri + b" HTTP/1.1\r\nHost: victim.com\r\n\r\n" + parser.received(data) + self.assertTrue(parser.completed) + self.assertIsInstance(parser.error, BadRequest) + + def test_received_network_path_reference_is_a_path(self): + # a request-target that starts with "//" has no scheme, so it stays a + # path and the Host header is left alone. See + # https://github.com/Pylons/waitress/issues/260 + data = b"GET //testing/whatever HTTP/1.1\r\nHost: victim.com\r\n\r\n" + self.parser.received(data) + self.assertIsNone(self.parser.error) + self.assertEqual(self.parser.headers["HOST"], "victim.com") + self.assertEqual(self.parser.path, "//testing/whatever") + + def test_received_connect_is_not_an_absolute_form(self): + # a CONNECT uses the authority-form of request-target, which urlsplit() + # reads as a scheme followed by a path. Waitress passes a CONNECT + # through for the WSGI application to deal with, so the request-target + # must not be mistaken for an absolute-form and rejected. + for uri, path in ( + (b"example.com:443", "443"), + (b"example.com", "example.com"), + (b"[::1]:443", "[::1]:443"), + ): + with self.subTest(uri=uri): + parser = HTTPRequestParser(Adjustments()) + data = b"CONNECT " + uri + b" HTTP/1.1\r\nHost: example.com\r\n\r\n" + parser.received(data) + self.assertIsNone(parser.error) + self.assertEqual(parser.command, "CONNECT") + self.assertEqual(parser.request_uri, uri.decode("latin-1")) + self.assertEqual(parser.path, path) + # the Host header field is left exactly as the client sent it + self.assertEqual(parser.headers["HOST"], "example.com") + def test_received_bad_transfer_encoding(self): data = ( b"GET /foobar HTTP/1.1\r\n" @@ -785,7 +848,10 @@ def testProxyGET(self): self.assertTrue(parser.completed) self.assertEqual(parser.version, "8.4") self.assertFalse(parser.empty) - self.assertEqual(parser.headers, {"CONTENT_LENGTH": "6"}) + # the authority of the absolute-form request-target is used as the Host + self.assertEqual( + parser.headers, {"CONTENT_LENGTH": "6", "HOST": "example.com:8080"} + ) self.assertEqual(parser.path, "/foobar") self.assertEqual(parser.command, "GET") self.assertEqual(parser.proxy_scheme, "https") From b5f6325003e61da617e8e129a988c48e323c1423 Mon Sep 17 00:00:00 2001 From: Delta Regeer Date: Sun, 2 Aug 2026 01:03:58 -0600 Subject: [PATCH 4/6] Validate the authority-form request-target of a CONNECT RFC 9112 section 3.2.3 requires that the request-target of a CONNECT be an authority-form, and that a server reject a CONNECT that targets an empty or invalid port number. Waitress still leaves it to the WSGI application to decide what to do with a CONNECT, up to and including implementing a proxy with it. This does not reject the method, it validates the shape of the request-target so that an application splitting a host from a port is working with something well formed rather than having to re-derive that itself. A CONNECT whose request-target is not a uri-host followed by an in-range port is now answered with a 400 (Bad Request). Note that this includes a target with no port at all, such as "CONNECT example.com", which used to be passed through. The validation runs before split_uri() sees the request-target, because urlsplit() reads "example.com:443" as the scheme "example.com" followed by the path "443". See https://github.com/Pylons/waitress/issues/463 --- CHANGES.txt | 7 ++++++ src/waitress/parser.py | 52 +++++++++++++++++++++++++++++++++++----- src/waitress/rfc7230.py | 9 ++++++- tests/test_functional.py | 9 +++++++ tests/test_parser.py | 24 ++++++++++++++++++- 5 files changed, 93 insertions(+), 8 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 1a41f9db..7226e2c5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -21,6 +21,13 @@ Backward Incompatibilities subcomponent, is rejected with a 400 (Bad Request). See https://github.com/Pylons/waitress/issues/467 +- The request-target of a CONNECT request is now validated as the + authority-form that RFC 9112 section 3.2.3 requires it to be, and a CONNECT + that targets an empty or invalid port number is rejected with a 400 (Bad + Request). Waitress still leaves it to the WSGI application to decide what to + do with a CONNECT, this only makes sure the request-target it is handed is + well formed. See https://github.com/Pylons/waitress/issues/463 + Bugfix ~~~~~~ diff --git a/src/waitress/parser.py b/src/waitress/parser.py index a7e52c66..dcf2dc06 100644 --- a/src/waitress/parser.py +++ b/src/waitress/parser.py @@ -24,7 +24,7 @@ from waitress.buffers import OverflowableBuffer from waitress.receiver import ChunkedReceiver, FixedStreamReceiver -from waitress.rfc7230 import HEADER_FIELD_RE, HOST_RE, ONLY_DIGIT_RE +from waitress.rfc7230 import AUTHORITY_FORM_RE, HEADER_FIELD_RE, HOST_RE, ONLY_DIGIT_RE from waitress.utilities import ( BadRequest, RequestEntityTooLarge, @@ -266,6 +266,24 @@ def parse_header(self, header_plus): command = command.decode("latin-1") self.command = command self.version = version + + if command == "CONNECT": + # RFC 9112 section 3.2.3: the request-target of a CONNECT is an + # authority-form, and a server MUST reject a CONNECT that targets + # an empty or invalid port number. + # + # Waitress leaves it to the WSGI application to decide what to do + # with a CONNECT, up to and including implementing a proxy with it, + # but the application can only do that safely if the target it is + # handed is well formed. This validates the shape of the + # request-target, it does not reject the method. + # + # NB: this has to happen before split_uri() is given a chance to + # look at the request-target, as urlsplit() parses the + # authority-form "example.com:443" as the scheme "example.com" + # followed by the path "443". + validate_authority_form(uri) + ( self.proxy_scheme, self.proxy_netloc, @@ -274,12 +292,11 @@ def parse_header(self, header_plus): self.fragment, ) = split_uri(uri) - # NB: the check for CONNECT here is not an attempt to validate the - # request, Waitress leaves it to the WSGI application to decide what to - # do with a CONNECT. It is that a CONNECT uses the authority-form of - # request-target, which urlsplit() reads as a scheme followed by a path + # NB: a CONNECT is excluded here because its request-target is an + # authority-form, which urlsplit() reads as a scheme followed by a path # ("example.com:443" comes back as the scheme "example.com" and the - # path "443"), and an authority-form is not an absolute-form. + # path "443"). An authority-form is not an absolute-form, and it has + # been validated as one above already. if self.proxy_scheme and command != "CONNECT": # This is an absolute-form request-target. RFC 9112 section 3.2.2 # says that an origin server MUST ignore the received Host header @@ -421,6 +438,29 @@ def validate_uri_host(value, what): raise ParsingError(f"Invalid {what}") +def validate_authority_form(uri): + """ + Validate that ``uri`` is an "authority-form" request-target as defined by + RFC 9112 section 3.2.3, that is a uri-host followed by a non-empty and + in-range port number. + """ + + m = AUTHORITY_FORM_RE.match(uri) + + if m is None: + raise ParsingError("Request-target is not in the authority-form") + + host, port = m.group("host", "port") + + if not host: + raise ParsingError("Request-target does not contain a host") + + # The regular expression bounds the port to at most 5 digits, so int() is + # safe to use on it here + if not port or not 0 < int(port) <= 65535: + raise ParsingError("Request-target contains an empty or invalid port") + + def split_uri(uri): # urlsplit handles byte input by returning bytes on py3, so # scheme, netloc, path, query, and fragment are bytes diff --git a/src/waitress/rfc7230.py b/src/waitress/rfc7230.py index 6d138262..3f8dee99 100644 --- a/src/waitress/rfc7230.py +++ b/src/waitress/rfc7230.py @@ -63,7 +63,8 @@ ) # RFC 3986 Section 3.2.2 "Host", which is what RFC 7230 Section 5.4 uses to -# define the value of the Host header field: +# define the value of the Host header field, and what RFC 9112 Section 3.2.3 +# uses for the authority-form of a request-target: # # host = IP-literal / IPv4address / reg-name # IP-literal = "[" ( IPv6address / IPvFuture ) "]" @@ -94,6 +95,12 @@ # callers apply the stricter rules that their context requires. HOST_RE = re.compile(("^" + URI_HOST + "(?::" + DIGIT + "*)?$").encode("latin-1")) +# RFC 9112 Section 3.2.3: authority-form = uri-host ":" port. It is used only +# for CONNECT, where the port is required to be present and valid. +AUTHORITY_FORM_RE = re.compile( + ("^(?P" + URI_HOST + "):(?P" + DIGIT + "{0,5})$").encode("latin-1") +) + # Pre-compiled regular expressions for use elsewhere ONLY_HEXDIG_RE = re.compile(("^" + HEXDIG + "+$").encode("latin-1")) ONLY_DIGIT_RE = re.compile(("^" + DIGIT + "+$").encode("latin-1")) diff --git a/tests/test_functional.py b/tests/test_functional.py index 03156b55..55d993c3 100644 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -262,6 +262,15 @@ def test_invalid_host_header_http11(self): line, headers, response_body = read_http(fp) self.assertline(line, "400", "Bad Request", "HTTP/1.1") + def test_connect_method_invalid_port(self): + # RFC 9112 section 3.2.3 + to_send = b"CONNECT victim.com HTTP/1.1\r\nHost: victim.com\r\n\r\n" + self.connect() + self.sock.send(to_send) + with self.sock.makefile("rb", 0) as fp: + line, headers, response_body = read_http(fp) + self.assertline(line, "400", "Bad Request", "HTTP/1.1") + def test_absolute_form_request_target(self): # RFC 9112 section 3.2.2: the authority of the request-target wins over # the Host header diff --git a/tests/test_parser.py b/tests/test_parser.py index 8cceb54c..14523dc6 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -197,7 +197,6 @@ def test_received_connect_is_not_an_absolute_form(self): # must not be mistaken for an absolute-form and rejected. for uri, path in ( (b"example.com:443", "443"), - (b"example.com", "example.com"), (b"[::1]:443", "[::1]:443"), ): with self.subTest(uri=uri): @@ -211,6 +210,29 @@ def test_received_connect_is_not_an_absolute_form(self): # the Host header field is left exactly as the client sent it self.assertEqual(parser.headers["HOST"], "example.com") + def test_received_connect_invalid_authority_form(self): + # RFC 9112 section 3.2.3: MUST reject a CONNECT request that targets an + # empty or invalid port number + for uri in ( + b"example.com", # no port at all + b"example.com:", # empty port + b"example.com:0", # port zero is not a valid port number + b"example.com:65536", # out of range + b"example.com:999999", + b"example.com:https", # the port must be numeric + b":443", # no host + b"http://example.com:443", + b"/", + b"*", + ): + with self.subTest(uri=uri): + parser = HTTPRequestParser(Adjustments()) + data = b"CONNECT " + uri + b" HTTP/1.1\r\nHost: example.com\r\n\r\n" + parser.received(data) + self.assertTrue(parser.completed) + self.assertIsInstance(parser.error, BadRequest) + self.assertIn("Request-target", parser.error.body) + def test_received_bad_transfer_encoding(self): data = ( b"GET /foobar HTTP/1.1\r\n" From 7901005bafa744f10a2578c65ae45ef50bdfa470 Mon Sep 17 00:00:00 2001 From: Delta Regeer Date: Sun, 2 Aug 2026 16:59:19 -0600 Subject: [PATCH 5/6] Remove the duplicated release sections from the changelog The absolute-form commit re-added a copy of the Unreleased Bugfix entries along with the whole 3.0.2 and 3.0.1 sections, so each of those releases appeared twice in the file and the 3.0.1 section grew a Bugfix block it never shipped with. Drop the duplicate. What is left is the new Unreleased section followed by the released sections exactly as they stand on main, so the branch only ever adds to the changelog. --- CHANGES.txt | 53 ----------------------------------------------------- 1 file changed, 53 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 7226e2c5..da88929d 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -72,59 +72,6 @@ Security https://github.com/Pylons/waitress/issues/451 -3.0.1 (2024-10-28) ------------------- - -Backward Incompatibilities -~~~~~~~~~~~~~~~~~~~~~~~~~~ - -- Python 3.8 is no longer supported. - See https://github.com/Pylons/waitress/pull/445. - -Bugfix -~~~~~~ - -- Renamed the HTTP header "Trailers" to "Trailer" to fix a typo and comply with - the correct header name as specified in RFC 7230. - -- Waitress will now drop a request if the Transer-Encoding is set twice in the - request, previously it would decode the chunks and pass it along to the WSGI - application with an appropriate content length. See - https://github.com/Pylons/waitress/issues/465 and - https://github.com/Pylons/waitress/pull/474 - -- When encountering a request that has both Content-Length set and - Transfer-Encoding of chunked we now close the connection after it is - completed to comply with the requirements of RFC9112. See - https://github.com/Pylons/waitress/pull/475 and - https://github.com/Pylons/waitress/issues/464 - -- A request that contains more than one Host header field is now rejected with - a 400 (Bad Request), rather than being passed to the WSGI application with - the values joined by a comma. See - https://github.com/Pylons/waitress/issues/462 and - https://github.com/Pylons/waitress/pull/484 - -- A request that contains more than one Content-Type header field is now - rejected with a 400 (Bad Request). Content-Type is a singleton field, and - recipients differ in which of the values they pick when it is sent more than - once. The same applies to Content-Length. See - https://github.com/Pylons/waitress/issues/466 and - https://github.com/Pylons/waitress/pull/488 - -3.0.2 (2024-11-16) ------------------- - -Security -~~~~~~~~ - -- When using Waitress to process trusted proxy headers, Waitress will now - update the headers to drop any untrusted values, thereby making sure that - WSGI apps only get trusted and validated values that Waitress itself used to - update the environ. See https://github.com/Pylons/waitress/pull/452 and - https://github.com/Pylons/waitress/issues/451 - - 3.0.1 (2024-10-28) ------------------ From 194ba7f62ca15707b069208815288183a6a8e53d Mon Sep 17 00:00:00 2001 From: Delta Regeer Date: Sun, 2 Aug 2026 17:23:20 -0600 Subject: [PATCH 6/6] Validate the origin-form and asterisk-form request-targets RFC 9112 section 3.2 defines exactly four forms of request-target. The absolute-form and the authority-form of a CONNECT are checked by the preceding commits; the remaining two were not checked at all, so a request-target that is none of the four still reached the application. The asterisk-form is restricted by section 3.2.4 to a server-wide OPTIONS, so it is now rejected for any other method. "GET * HTTP/1.1" used to be served with a PATH_INFO of "*" for the application to puzzle over. Everything else has to be an origin-form, which section 3.2.1 defines as an absolute-path optionally followed by a query, and an absolute-path begins with a "/". "GET foo/bar HTTP/1.1" used to be served with a PATH_INFO of "foo/bar", which PEP 3333 does not allow either: a non-empty PATH_INFO starts with a slash. The check looks at the request-target as it arrived rather than at the decoded path, so a percent encoded "/" cannot stand in for a real one. A request-target beginning with "//" has no scheme and stays a path, as it already did for https://github.com/Pylons/waitress/issues/260. Also retarget the unreleased section at 4.0.0, since everything in it is backward incompatible. --- CHANGES.txt | 15 +++++++++++-- src/waitress/parser.py | 26 +++++++++++++++++++++ tests/test_parser.py | 51 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index da88929d..e1263655 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,5 @@ -Unreleased ----------- +4.0.0 (unreleased) +------------------ Backward Incompatibilities ~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -28,6 +28,17 @@ Backward Incompatibilities do with a CONNECT, this only makes sure the request-target it is handed is well formed. See https://github.com/Pylons/waitress/issues/463 +- The two remaining forms of request-target described by RFC 9112 section 3.2 + are now validated as well. The asterisk-form ``*`` is only accepted for an + OPTIONS request, as section 3.2.4 restricts it to a server-wide OPTIONS; + sending it with any other method is rejected with a 400 (Bad Request). + Anything that is not an absolute-form, an authority-form or the asterisk-form + has to be an origin-form, which section 3.2.1 defines as an absolute-path, + and a request-target that does not begin with a ``/`` is now rejected the + same way rather than reaching the application as a ``PATH_INFO`` that PEP + 3333 does not allow. A request-target beginning with ``//`` is still treated + as a path, as before. + Bugfix ~~~~~~ diff --git a/src/waitress/parser.py b/src/waitress/parser.py index dcf2dc06..f4a46598 100644 --- a/src/waitress/parser.py +++ b/src/waitress/parser.py @@ -316,6 +316,32 @@ def parse_header(self, header_plus): headers["HOST"] = self.proxy_netloc + elif command != "CONNECT": + # Not an absolute-form, and not the authority-form of a CONNECT, + # so RFC 9112 section 3.2 leaves only two forms this can be. A + # request-target that is neither is malformed and does not name + # anything we could route. + + if uri == b"*": + # RFC 9112 section 3.2.4: the asterisk-form is only used for a + # server-wide OPTIONS request. For any other method it is not + # a request-target at all, and passing it through would hand + # the application a PATH_INFO of "*" to make sense of. + + if command != "OPTIONS": + raise ParsingError( + "Asterisk-form request-target is only valid for OPTIONS" + ) + elif not uri.startswith(b"/"): + # RFC 9112 section 3.2.1: an origin-form is an absolute-path + # optionally followed by a query, and an absolute-path begins + # with a "/". PEP 3333 wants the same of PATH_INFO. + # + # NB: this looks at the request-target as it arrived rather + # than at the decoded path, so that a percent encoded "/" + # can not stand in for the real one. + raise ParsingError("Request-target is not in the origin-form") + # RFC 9112 section 3.2 requires a 400 (Bad Request) response to any # HTTP/1.1 request that lacks a Host header field, or that has a Host # header field with an invalid field value. Duplicate Host headers are diff --git a/tests/test_parser.py b/tests/test_parser.py index 14523dc6..122d79e8 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -190,6 +190,57 @@ def test_received_network_path_reference_is_a_path(self): self.assertEqual(self.parser.headers["HOST"], "victim.com") self.assertEqual(self.parser.path, "//testing/whatever") + def test_received_asterisk_form_options(self): + # RFC 9112 section 3.2.4: the asterisk-form is used for a server-wide + # OPTIONS request + data = b"OPTIONS * HTTP/1.1\r\nHost: localhost\r\n\r\n" + self.parser.received(data) + self.assertTrue(self.parser.completed) + self.assertIsNone(self.parser.error) + self.assertEqual(self.parser.command, "OPTIONS") + self.assertEqual(self.parser.path, "*") + + def test_received_asterisk_form_rejected_for_other_methods(self): + # the asterisk-form is only a request-target for OPTIONS; for anything + # else it names nothing that could be routed + for command in (b"GET", b"POST", b"HEAD", b"DELETE"): + with self.subTest(command=command): + parser = HTTPRequestParser(Adjustments()) + data = command + b" * HTTP/1.1\r\nHost: localhost\r\n\r\n" + parser.received(data) + self.assertTrue(parser.completed) + self.assertIsInstance(parser.error, BadRequest) + + def test_received_origin_form_must_be_an_absolute_path(self): + # RFC 9112 section 3.2.1: an origin-form is an absolute-path, which + # begins with a "/". PEP 3333 wants the same of PATH_INFO. + for uri in ( + b"foo/bar", + b"1.2.3.4:80/x", + b"%2Ffoo", # a percent encoded "/" does not make an absolute-path + b".", + ): + with self.subTest(uri=uri): + parser = HTTPRequestParser(Adjustments()) + data = b"GET " + uri + b" HTTP/1.1\r\nHost: localhost\r\n\r\n" + parser.received(data) + self.assertTrue(parser.completed) + self.assertIsInstance(parser.error, BadRequest) + + def test_received_origin_form_accepted(self): + for uri, path in ( + (b"/", "/"), + (b"/foo", "/foo"), + (b"/foo?a=b", "/foo"), + (b"/foo%2Fbar", "/foo/bar"), + ): + with self.subTest(uri=uri): + parser = HTTPRequestParser(Adjustments()) + data = b"GET " + uri + b" HTTP/1.1\r\nHost: localhost\r\n\r\n" + parser.received(data) + self.assertIsNone(parser.error) + self.assertEqual(parser.path, path) + def test_received_connect_is_not_an_absolute_form(self): # a CONNECT uses the authority-form of request-target, which urlsplit() # reads as a scheme followed by a path. Waitress passes a CONNECT