From cfc47d43bd9de20cfad3cf4a7cfea52a10503003 Mon Sep 17 00:00:00 2001 From: Delta Regeer Date: Sun, 2 Aug 2026 18:18:06 -0600 Subject: [PATCH] Tighten four RFC 9112 rules that Waitress did not enforce Validate the HTTP version. RFC 9112 section 2.3 asks a server to answer 505 (HTTP Version Not Supported) when the major version is one it does not support, and to process a higher minor version within a major it does implement as the highest minor it is conformant with. Waitress accepted any [0-9].[0-9] and kept it verbatim. That is more than untidiness: everything version dependent is keyed on the version being exactly "1.0" or "1.1", so an unrecognised one fell through all of it and a request claiming HTTP/2.0 had its Connection and Expect header fields silently ignored. Reject obsolete line folding. RFC 9112 section 5.2 requires a server to either reject an obs-fold or replace it with SP before interpreting the field value; Waitress did neither, joining the continuation onto the line before it and keeping the original whitespace. That leaves us reading a field one way while something in front of us that rejects folding, or unfolds differently, reads it another. Folding has been deprecated since RFC 7230 in 2014. Drop the header fields named in a request's Connection field before building the environ. RFC 9110 section 7.6.1 makes them connection specific, applying to a single hop and not to be forwarded. Waitress is the end of the connection, so handing them to the application amounts to forwarding: a client can name a header there that a proxy in front of us believes only it controls, and the application cannot tell the two apart. Skip only empty lines before the request-line. RFC 9112 section 2.2 sanctions ignoring a CRLF there, which lets a client send a spare one after a request when pipelining. A bare lstrip() also ate spaces, tabs, vertical tabs, form feeds and lone CR or LF octets, none of which may appear before a request-line, and swallowing them hid them from the checks below. The request-line version in tests/test_parser.py was HTTP/8.4 in 25 places, chosen to show the parser did not care about it. It does now, so those use a version we support. --- CHANGES.txt | 38 +++++++++- src/waitress/parser.py | 78 +++++++++++++++++--- src/waitress/rfc7230.py | 20 +++++ src/waitress/task.py | 19 +++++ src/waitress/utilities.py | 5 ++ tests/test_parser.py | 149 ++++++++++++++++++++++++++------------ tests/test_task.py | 27 +++++++ 7 files changed, 276 insertions(+), 60 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index c7f32ea4..c07a78e3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,39 @@ -Unreleased ----------- +4.0.0 (unreleased) +------------------ + +Backward Incompatibilities +~~~~~~~~~~~~~~~~~~~~~~~~~~ + +- A request whose HTTP version has a major version other than 1 is now + rejected with a 505 (HTTP Version Not Supported), and one with a higher + minor version than Waitress implements, such as ``HTTP/1.9``, is processed + as ``HTTP/1.1``. RFC 9112 section 2.3 asks for both. Previously any + ``[0-9].[0-9]`` was accepted and kept verbatim, and because everything + version dependent is keyed on the version being exactly ``1.0`` or ``1.1``, + an unrecognised one fell through all of it: a request claiming ``HTTP/2.0`` + had its ``Connection`` and ``Expect`` header fields silently ignored. + +- Obsolete line folding in a request header field is now rejected with a 400 + (Bad Request). RFC 9112 section 5.2 requires a server to either reject an + obs-fold or replace it with SP before interpreting the field value, and + folding has been deprecated since RFC 7230 in 2014. Waitress used to join + the continuation onto the preceding line, which left it reading a field one + way while something in front of it that rejects folding, or unfolds it + differently, read it another. + +- Header fields named in the ``Connection`` header field of a request are no + longer passed to the WSGI application. RFC 9110 section 7.6.1 makes them + connection specific, applying to a single hop and not to be forwarded. + Waitress is the end of the connection, so handing them to the application + amounts to forwarding them: a client could name a header there that a proxy + in front of Waitress believes only it controls, and the application has no + way to tell the two apart. + +- Only empty lines are now skipped before the request-line. RFC 9112 section + 2.2 sanctions ignoring a CRLF received before a request-line, which lets a + client send a spare one after a request when pipelining. Waitress stripped + any leading whitespace, which also covered spaces, tabs, vertical tabs, form + feeds and lone CR or LF octets, none of which may appear there. Bugfix ~~~~~~ diff --git a/src/waitress/parser.py b/src/waitress/parser.py index 1af4594d..279a0bae 100644 --- a/src/waitress/parser.py +++ b/src/waitress/parser.py @@ -27,6 +27,7 @@ from waitress.rfc7230 import HEADER_FIELD_RE, ONLY_DIGIT_RE from waitress.utilities import ( BadRequest, + HTTPVersionNotSupported, RequestEntityTooLarge, RequestHeaderFieldsTooLarge, ServerNotImplemented, @@ -49,6 +50,10 @@ class TransferEncodingNotImplemented(Exception): pass +class HTTPVersionNotSupportedError(Exception): + pass + + class HTTPRequestParser: """A structure that collects the HTTP request. @@ -130,11 +135,20 @@ def received(self, data): # Header finished. header_plus = s[:index] - # Remove preceding blank lines. This is suggested by - # https://tools.ietf.org/html/rfc7230#section-3.5 to support - # clients sending an extra CR LF after another request when - # using HTTP pipelining - header_plus = header_plus.lstrip() + # Remove preceding blank lines. RFC 9112 section 2.2 says a + # server that is expecting to read a start-line SHOULD ignore + # at least one empty line received before it, which supports + # clients sending an extra CRLF after a request when they are + # pipelining. + # + # NB: only whole CRLF pairs are skipped. A bare lstrip() also + # eats spaces, tabs, vertical tabs, form feeds and lone CR or + # LF octets, none of which RFC 9112 allows before a + # request-line, and swallowing them here would hide them from + # the checks further down. + + while header_plus.startswith(b"\r\n"): + header_plus = header_plus[2:] if not header_plus: self.empty = True @@ -148,6 +162,9 @@ def received(self, data): except TransferEncodingNotImplemented as e: self.error = ServerNotImplemented(e.args[0]) self.completed = True + except HTTPVersionNotSupportedError as e: + self.error = HTTPVersionNotSupported(e.args[0]) + self.completed = True else: if self.body_rcv is None: # no content-length header and not a t-e: chunked @@ -265,6 +282,30 @@ def parse_header(self, header_plus): version = version.decode("latin-1") command = command.decode("latin-1") self.command = command + + if version: + # RFC 9112 section 2.3: a recipient that receives a message with a + # major version it implements but a higher minor version than it + # implements SHOULD process the message as if it were in the + # highest minor version it is conformant with, and a server SHOULD + # respond with a 505 (HTTP Version Not Supported) when the major + # version is one it does not support. + # + # This matters beyond tidiness: everything version dependent below + # is keyed on the version being exactly "1.0" or "1.1", so an + # unrecognised one used to fall through all of it. A request + # claiming HTTP/2.0 had its Connection and Expect header fields + # ignored, and nothing else in Waitress noticed either. + major, _, minor = version.partition(".") + + if major != "1": + raise HTTPVersionNotSupportedError( + "HTTP version %s is not supported" % version + ) + + if minor > "1": + version = "1.1" + self.version = version ( self.proxy_scheme, @@ -408,7 +449,7 @@ def split_uri(uri): def get_header_lines(header): """ - Splits the header into lines, putting multi-line headers together. + Splits the header into lines. """ r = [] lines = header.split(b"\r\n") @@ -423,12 +464,25 @@ def get_header_lines(header): ) if line.startswith((b" ", b"\t")): - if not r: - # https://corte.si/posts/code/pathod/pythonservers/index.html - raise ParsingError('Malformed header line "%s"' % str(line, "latin-1")) - r[-1] += line - else: - r.append(line) + # RFC 9112 section 5.2: a server that receives an obs-fold in a + # request message that is not within a message/http container MUST + # either reject the message with a 400 (Bad Request), preferably + # with a representation explaining that obsolete line folding is + # unacceptable, or replace each obs-fold with one or more SP + # octets before interpreting the field value. + # + # We reject. Folding has been deprecated since RFC 7230 in 2014, + # and unfolding leaves us interpreting a field one way while + # something in front of us that rejects folding, or unfolds it + # differently, interprets it another. + # + # NB: a folded line arriving first has nothing to fold onto, which + # used to be the only case rejected here. See + # https://corte.si/posts/code/pathod/pythonservers/index.html + raise ParsingError( + 'Obsolete line folding is not allowed "%s"' % str(line, "latin-1") + ) + r.append(line) return r diff --git a/src/waitress/rfc7230.py b/src/waitress/rfc7230.py index 26e64260..dcb39f58 100644 --- a/src/waitress/rfc7230.py +++ b/src/waitress/rfc7230.py @@ -73,3 +73,23 @@ QUOTED_PAIR_RE = re.compile(QUOTED_PAIR) QUOTED_STRING_RE = re.compile(QUOTED_STRING) CHUNK_EXT_RE = re.compile(("^" + CHUNK_EXT + "$").encode("latin-1")) + + +def connection_options(value): + """ + Return the set of connection options in a Connection header field value. + + RFC 9112 Section 9.1 defines the field as a comma separated list of + options, so an option has to be looked for as a member of that list rather + than by comparing the whole field value. + + Options are case insensitive, and a field that appeared more than once has + already been joined with commas by the parser, which is exactly the same + grammar, so this handles that as well. + """ + + return { + option.strip(" \t").lower() + for option in value.split(",") + if option.strip(" \t") + } diff --git a/src/waitress/task.py b/src/waitress/task.py index bda3ae50..3898794e 100644 --- a/src/waitress/task.py +++ b/src/waitress/task.py @@ -18,6 +18,7 @@ import time from .buffers import ReadOnlyFileBasedBuffer +from .rfc7230 import connection_options from .utilities import build_http_date, logger, queue_logger rename_headers = { # or keep them without the HTTP_ prefix added @@ -558,7 +559,25 @@ def get_environment(self): "wsgi.input_terminated": True, # wsgi.input is EOF terminated } + # RFC 9110 section 7.6.1: the field names listed in the Connection + # header field are connection specific, they apply to this hop only + # and a recipient must not forward them. Waitress is the end of the + # line for the connection, so handing them to the application is the + # equivalent of forwarding: a client can name a header there that the + # proxy in front of us believes only it controls, and the application + # has no way to tell the difference. + # + # The field names Connection itself may carry are dropped along with + # the ones it names, since they describe the connection rather than + # the request. + hop_by_hop_fields = { + option.upper().replace("-", "_") + for option in connection_options(request.headers.get("CONNECTION", "")) + } + for key, value in dict(request.headers).items(): + if key in hop_by_hop_fields: + continue mykey = rename_headers.get(key, None) if mykey is None: mykey = "HTTP_" + key diff --git a/src/waitress/utilities.py b/src/waitress/utilities.py index ae94e4cd..22a0b56f 100644 --- a/src/waitress/utilities.py +++ b/src/waitress/utilities.py @@ -298,3 +298,8 @@ class InternalServerError(Error): class ServerNotImplemented(Error): code = 501 reason = "Not Implemented" + + +class HTTPVersionNotSupported(Error): + code = 505 + reason = "HTTP Version Not Supported" diff --git a/tests/test_parser.py b/tests/test_parser.py index 5f341ae9..67cdfe57 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -27,6 +27,7 @@ ) from waitress.utilities import ( BadRequest, + HTTPVersionNotSupported, RequestEntityTooLarge, RequestHeaderFieldsTooLarge, ServerNotImplemented, @@ -111,7 +112,7 @@ def test_received_nonsense_nothing(self): self.assertDictEqual(self.parser.headers, {}) def test_received_no_doublecr(self): - data = b"GET /foobar HTTP/8.4\r\n" + data = b"GET /foobar HTTP/1.1\r\n" result = self.parser.received(data) self.assertEqual(result, 22) self.assertFalse(self.parser.completed) @@ -124,14 +125,14 @@ def test_received_already_completed(self): def test_received_cl_too_large(self): self.parser.adj.max_request_body_size = 2 - data = b"GET /foobar HTTP/8.4\r\nContent-Length: 10\r\n\r\n" + data = b"GET /foobar HTTP/1.1\r\nContent-Length: 10\r\n\r\n" result = self.parser.received(data) self.assertEqual(result, 44) self.assertTrue(self.parser.completed) self.assertIsInstance(self.parser.error, RequestEntityTooLarge) def test_received_headers_not_too_large_multiple_chunks(self): - data = b"GET /foobar HTTP/8.4\r\nX-Foo: 1\r\n" + data = b"GET /foobar HTTP/1.1\r\nX-Foo: 1\r\n" data2 = b"X-Foo-Other: 3\r\n\r\n" self.parser.adj.max_request_header_size = len(data) + len(data2) + 1 result = self.parser.received(data) @@ -143,7 +144,7 @@ def test_received_headers_not_too_large_multiple_chunks(self): def test_received_headers_too_large(self): self.parser.adj.max_request_header_size = 2 - data = b"GET /foobar HTTP/8.4\r\nX-Foo: 1\r\n\r\n" + data = b"GET /foobar HTTP/1.1\r\nX-Foo: 1\r\n\r\n" result = self.parser.received(data) self.assertEqual(result, 34) self.assertTrue(self.parser.completed) @@ -201,14 +202,67 @@ def test_received_chunked_completed_sets_content_length(self): self.assertIsNone(self.parser.error) self.assertEqual(self.parser.headers["CONTENT_LENGTH"], "29") + def test_received_unsupported_http_version(self): + # RFC 9112 section 2.3: a server SHOULD respond with a 505 (HTTP + # Version Not Supported) when the major version is one it does not + # support + for version in (b"0.9", b"2.0", b"3.0", b"9.9"): + with self.subTest(version=version): + parser = HTTPRequestParser(Adjustments()) + data = b"GET /foobar HTTP/" + version + b"\r\nHost: localhost\r\n\r\n" + parser.received(data) + self.assertTrue(parser.completed) + self.assertIsInstance(parser.error, HTTPVersionNotSupported) + self.assertEqual(parser.error.code, 505) + + def test_received_higher_minor_version_treated_as_1_1(self): + # RFC 9112 section 2.3: a recipient that receives a higher minor + # version within a major version it implements SHOULD process the + # message as the highest minor version it is conformant with + for version in (b"1.2", b"1.9"): + with self.subTest(version=version): + parser = HTTPRequestParser(Adjustments()) + data = b"GET /foobar HTTP/" + version + b"\r\nHost: localhost\r\n\r\n" + parser.received(data) + self.assertIsNone(parser.error) + self.assertEqual(parser.version, "1.1") + + def test_received_higher_minor_version_honours_connection(self): + # the point of normalising the version: everything version dependent + # is keyed on it being exactly "1.0" or "1.1", so an unrecognised + # version used to fall through all of it + parser = HTTPRequestParser(Adjustments()) + parser.received( + b"GET /foobar HTTP/1.2\r\nHost: localhost\r\nConnection: close\r\n\r\n" + ) + self.assertIsNone(parser.error) + self.assertTrue(parser.connection_close) + + def test_received_only_crlf_skipped_before_request_line(self): + # RFC 9112 section 2.2 sanctions ignoring empty lines before the + # request-line, and nothing else. A bare lstrip() also ate spaces, + # tabs, vertical tabs and form feeds. + parser = HTTPRequestParser(Adjustments()) + parser.received(b"\r\n\r\nGET /foobar HTTP/1.1\r\nHost: localhost\r\n\r\n") + self.assertIsNone(parser.error) + + for prefix in (b" ", b"\t", b"\x0b", b"\x0c"): + with self.subTest(prefix=prefix): + parser = HTTPRequestParser(Adjustments()) + parser.received( + prefix + b"GET /foobar HTTP/1.1\r\nHost: localhost\r\n\r\n" + ) + self.assertTrue(parser.completed) + self.assertIsInstance(parser.error, BadRequest) + def test_parse_header_gardenpath(self): - data = b"GET /foobar HTTP/8.4\r\nfoo: bar\r\n" + data = b"GET /foobar HTTP/1.1\r\nfoo: bar\r\n" self.parser.parse_header(data) - self.assertEqual(self.parser.first_line, b"GET /foobar HTTP/8.4") + self.assertEqual(self.parser.first_line, b"GET /foobar HTTP/1.1") self.assertEqual(self.parser.headers["FOO"], "bar") def test_parse_header_no_cr_in_headerplus(self): - data = b"GET /foobar HTTP/8.4" + data = b"GET /foobar HTTP/1.1" try: self.parser.parse_header(data) @@ -218,7 +272,7 @@ def test_parse_header_no_cr_in_headerplus(self): self.assertTrue(False) def test_parse_header_bad_content_length(self): - data = b"GET /foobar HTTP/8.4\r\ncontent-length: abc\r\n" + data = b"GET /foobar HTTP/1.1\r\ncontent-length: abc\r\n" try: self.parser.parse_header(data) @@ -228,7 +282,7 @@ def test_parse_header_bad_content_length(self): self.assertTrue(False) def test_parse_header_bad_content_length_plus(self): - data = b"GET /foobar HTTP/8.4\r\ncontent-length: +10\r\n" + data = b"GET /foobar HTTP/1.1\r\ncontent-length: +10\r\n" try: self.parser.parse_header(data) @@ -238,7 +292,7 @@ def test_parse_header_bad_content_length_plus(self): self.assertTrue(False) def test_parse_header_bad_content_length_minus(self): - data = b"GET /foobar HTTP/8.4\r\ncontent-length: -10\r\n" + data = b"GET /foobar HTTP/1.1\r\ncontent-length: -10\r\n" try: self.parser.parse_header(data) @@ -248,7 +302,7 @@ def test_parse_header_bad_content_length_minus(self): self.assertTrue(False) def test_parse_header_multiple_content_length(self): - data = b"GET /foobar HTTP/8.4\r\ncontent-length: 10\r\ncontent-length: 20\r\n" + data = b"GET /foobar HTTP/1.1\r\ncontent-length: 10\r\ncontent-length: 20\r\n" try: self.parser.parse_header(data) @@ -349,7 +403,7 @@ def test_close_with_no_body_rcv(self): self.parser.close() # doesn't raise def test_parse_header_lf_only(self): - data = b"GET /foobar HTTP/8.4\nfoo: bar" + data = b"GET /foobar HTTP/1.1\nfoo: bar" try: self.parser.parse_header(data) @@ -359,7 +413,7 @@ def test_parse_header_lf_only(self): self.assertTrue(False) def test_parse_header_cr_only(self): - data = b"GET /foobar HTTP/8.4\rfoo: bar" + data = b"GET /foobar HTTP/1.1\rfoo: bar" try: self.parser.parse_header(data) except ParsingError: @@ -368,7 +422,7 @@ def test_parse_header_cr_only(self): self.assertTrue(False) def test_parse_header_extra_lf_in_header(self): - data = b"GET /foobar HTTP/8.4\r\nfoo: \nbar\r\n" + data = b"GET /foobar HTTP/1.1\r\nfoo: \nbar\r\n" try: self.parser.parse_header(data) except ParsingError as e: @@ -377,7 +431,7 @@ def test_parse_header_extra_lf_in_header(self): self.assertTrue(False) def test_parse_header_extra_lf_in_first_line(self): - data = b"GET /foobar\n HTTP/8.4\r\n" + data = b"GET /foobar\n HTTP/1.1\r\n" try: self.parser.parse_header(data) except ParsingError as e: @@ -386,7 +440,7 @@ def test_parse_header_extra_lf_in_first_line(self): self.assertTrue(False) def test_parse_header_invalid_whitespace(self): - data = b"GET /foobar HTTP/8.4\r\nfoo : bar\r\n" + data = b"GET /foobar HTTP/1.1\r\nfoo : bar\r\n" try: self.parser.parse_header(data) except ParsingError as e: @@ -417,7 +471,7 @@ def test_parse_header_invalid_folding_spacing(self): try: self.parser.parse_header(data) except ParsingError as e: - self.assertIn("Invalid header", e.args[0]) + self.assertIn("Obsolete line folding", e.args[0]) else: # pragma: nocover self.assertTrue(False) @@ -452,18 +506,24 @@ def test_parse_header_multiple_values(self): self.assertEqual(self.parser.headers["FOO"], "bar, whatever, more, please, yes") def test_parse_header_multiple_values_header_folded(self): + # RFC 9112 section 5.2: a server that receives an obs-fold MUST either + # reject the message or replace the fold with SP. We reject. data = b"GET /foobar HTTP/1.1\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") + try: + self.parser.parse_header(data) + except ParsingError as e: + self.assertIn("Obsolete line folding", e.args[0]) + else: # pragma: nocover + self.assertTrue(False) 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" - self.parser.parse_header(data) - - self.assertIn("FOO", self.parser.headers) - self.assertEqual(self.parser.headers["FOO"], "bar, whatever, more, please, yes") + try: + self.parser.parse_header(data) + except ParsingError as e: + self.assertIn("Obsolete line folding", e.args[0]) + else: # pragma: nocover + self.assertTrue(False) def test_parse_header_multiple_values_extra_space(self): # Tests errata from: https://www.rfc-editor.org/errata_search.php?rfc=7230&eid=4189 @@ -575,20 +635,17 @@ def test_get_header_lines(self): self.assertListEqual(result, [b"slam", b"slim"]) def test_get_header_lines_folded(self): - # From RFC2616: - # HTTP/1.1 header field values can be folded onto multiple lines if the - # continuation line begins with a space or horizontal tab. All linear - # white space, including folding, has the same semantics as SP. A - # recipient MAY replace any linear white space with a single SP before - # interpreting the field value or forwarding the message downstream. - - # We are just preserving the whitespace that indicates folding. - result = self._callFUT(b"slim\r\n slam") - self.assertListEqual(result, [b"slim slam"]) + # RFC 9112 section 5.2: a server that receives an obs-fold in a request + # message that is not within a message/http container MUST either + # reject the message with a 400 (Bad Request) or replace each obs-fold + # with one or more SP octets before interpreting the field value. We + # reject: folding was deprecated by RFC 7230 in 2014, and unfolding + # leaves us reading a field one way while something in front of us + # that rejects folding, or unfolds differently, reads it another. + self.assertRaises(ParsingError, self._callFUT, b"slim\r\n slam") def test_get_header_lines_tabbed(self): - result = self._callFUT(b"slam\r\n\tslim") - self.assertListEqual(result, [b"slam\tslim"]) + self.assertRaises(ParsingError, self._callFUT, b"slam\r\n\tslim") def test_get_header_lines_malformed(self): # https://corte.si/posts/code/pathod/pythonservers/index.html @@ -618,7 +675,7 @@ def test_crack_first_line_missing_version(self): self.assertTupleEqual(result, (b"GET", b"/", b"")) def test_crack_first_line_bad_method(self): - result = self._callFUT(b"GE\x00 /foobar HTTP/8.4") + result = self._callFUT(b"GE\x00 /foobar HTTP/1.1") self.assertTupleEqual(result, (b"", b"", b"")) def test_crack_first_line_bad_version(self): @@ -644,7 +701,7 @@ def feed(self, data): def testSimpleGET(self): data = ( - b"GET /foobar HTTP/8.4\r\n" + b"GET /foobar HTTP/1.1\r\n" b"FirstName: mickey\r\n" b"lastname: Mouse\r\n" b"content-length: 6\r\n" @@ -654,7 +711,7 @@ def testSimpleGET(self): parser = self.parser self.feed(data) self.assertTrue(parser.completed) - self.assertEqual(parser.version, "8.4") + self.assertEqual(parser.version, "1.1") self.assertFalse(parser.empty) self.assertDictEqual( parser.headers, @@ -673,7 +730,7 @@ def testSimpleGET(self): def testComplexGET(self): data = ( - b"GET /foo/a+%2B%2F%C3%A4%3D%26a%3Aint?d=b+%2B%2F%3D%26b%3Aint&c+%2B%2F%3D%26c%3Aint=6 HTTP/8.4\r\n" + b"GET /foo/a+%2B%2F%C3%A4%3D%26a%3Aint?d=b+%2B%2F%3D%26b%3Aint&c+%2B%2F%3D%26c%3Aint=6 HTTP/1.1\r\n" b"FirstName: mickey\r\n" b"lastname: Mouse\r\n" b"content-length: 10\r\n" @@ -683,7 +740,7 @@ def testComplexGET(self): parser = self.parser self.feed(data) self.assertEqual(parser.command, "GET") - self.assertEqual(parser.version, "8.4") + self.assertEqual(parser.version, "1.1") self.assertFalse(parser.empty) self.assertDictEqual( parser.headers, @@ -706,7 +763,7 @@ def testComplexGET(self): def testProxyGET(self): data = ( - b"GET https://example.com:8080/foobar HTTP/8.4\r\n" + b"GET https://example.com:8080/foobar HTTP/1.1\r\n" b"content-length: 6\r\n" b"\r\n" b"Hello." @@ -714,7 +771,7 @@ def testProxyGET(self): parser = self.parser self.feed(data) self.assertTrue(parser.completed) - self.assertEqual(parser.version, "8.4") + self.assertEqual(parser.version, "1.1") self.assertFalse(parser.empty) self.assertEqual(parser.headers, {"CONTENT_LENGTH": "6"}) self.assertEqual(parser.path, "/foobar") @@ -729,7 +786,7 @@ def testDuplicateHeaders(self): # Ensure that headers with the same key get concatenated as per # RFC2616. data = ( - b"GET /foobar HTTP/8.4\r\n" + b"GET /foobar HTTP/1.1\r\n" b"x-forwarded-for: 10.11.12.13\r\n" b"x-forwarded-for: unknown,127.0.0.1\r\n" b"X-Forwarded_for: 255.255.255.255\r\n" @@ -749,7 +806,7 @@ def testDuplicateHeaders(self): def testSpoofedHeadersDropped(self): data = ( - b"GET /foobar HTTP/8.4\r\n" + b"GET /foobar HTTP/1.1\r\n" b"x-auth_user: bob\r\n" b"content-length: 6\r\n" b"\r\n" diff --git a/tests/test_task.py b/tests/test_task.py index 278ab538..1739a0b9 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -768,6 +768,33 @@ def test_get_environ_with_url_prefix_empty_path(self): self.assertEqual(environ["PATH_INFO"], "") self.assertEqual(environ["SCRIPT_NAME"], "/foo") + def test_get_environment_drops_connection_named_fields(self): + # RFC 9110 section 7.6.1: the field names listed in Connection are + # connection specific and must not be forwarded. Handing them to the + # application is the equivalent of forwarding: a client could name a + # header there that the proxy in front of us believes it controls. + inst = self._makeOne() + request = DummyParser() + request.headers = { + "CONNECTION": "keep-alive, X-Secret", + "X_SECRET": "spoofed", + "X_KEPT": "fine", + "HOST": "localhost", + } + inst.request = request + environ = inst.get_environment() + self.assertNotIn("HTTP_X_SECRET", environ) + self.assertEqual(environ["HTTP_X_KEPT"], "fine") + self.assertEqual(environ["HTTP_HOST"], "localhost") + + def test_get_environment_keeps_fields_when_no_connection(self): + inst = self._makeOne() + request = DummyParser() + request.headers = {"X_SECRET": "fine", "HOST": "localhost"} + inst.request = request + environ = inst.get_environment() + self.assertEqual(environ["HTTP_X_SECRET"], "fine") + def test_get_environment_values(self): import sys