diff --git a/Lib/httplib.py b/Lib/httplib.py index 9c8c5eedccb04a3..6271c76f741dcaf 100644 --- a/Lib/httplib.py +++ b/Lib/httplib.py @@ -261,6 +261,11 @@ # to prevent http header injection. _contains_disallowed_method_pchar_re = re.compile('[\x00-\x1f]') +# RFC 9112: Content-Length = 1*DIGIT and chunk-size = 1*HEXDIG. +# int() is more permissive, so we match against the grammar before calling it. +_is_legal_content_length = re.compile(r'[0-9]+\Z').match +_is_legal_chunk_size = re.compile(r'[0-9a-fA-F]+\Z').match + # We always set the Content-Length header for these methods because some # servers will otherwise respond with a 411 _METHODS_EXPECTING_BODY = {'PATCH', 'POST', 'PUT'} @@ -503,14 +508,12 @@ def begin(self): # do we have a Content-Length? # NOTE: RFC 2616, S4.4, #3 says we ignore this if tr_enc is "chunked" length = self.msg.getheader('content-length') - if length and not self.chunked: - try: - self.length = int(length) - except ValueError: - self.length = None - else: - if self.length < 0: # ignore nonsensical negative lengths - self.length = None + if length is not None: + # RFC 9112, section 5.1: optional whitespace (SP / HTAB) around + # the field value is not part of the value. + length = length.strip(' \t') + if length and not self.chunked and _is_legal_content_length(length): + self.length = int(length) else: self.length = None @@ -632,7 +635,10 @@ def _read_chunked(self, amt): i = line.find(';') if i >= 0: line = line[:i] # strip chunk-extensions + line = line.rstrip() try: + if not _is_legal_chunk_size(line): + raise ValueError("invalid chunk size") chunk_left = int(line, 16) except ValueError: # close the connection as protocol synchronisation is diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index e7ee76b0226d096..dfd0e79075eed07 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -622,6 +622,46 @@ def test_negative_content_length(self): self.assertEqual(resp.read(), 'Hello\r\n') self.assertTrue(resp.isclosed()) + def test_malformed_content_length(self): + # RFC 9112: Content-Length = 1*DIGIT. Values that int() accepts but + # the grammar forbids must not be used to frame the body. + for value in ('+5', '5_0', '5 0', '0x5'): + sock = FakeSocket( + 'HTTP/1.1 200 OK\r\nContent-Length: %s\r\n\r\nHello\r\n' % value) + resp = httplib.HTTPResponse(sock, method="GET") + resp.begin() + self.assertIsNone(resp.length, value) + self.assertEqual(resp.read(), 'Hello\r\n') + resp.close() + + def test_content_length_with_whitespace(self): + # RFC 9112, section 5.1: optional whitespace (SP / HTAB) around + # the field value is not part of the value, so it must not + # prevent the Content-Length from being used to frame the body. + for value in ('7 ', '7\t', '7 \t ', ' 7 '): + sock = FakeSocket( + 'HTTP/1.1 200 OK\r\nContent-Length: %s\r\n\r\n' + 'Hello\r\nextra' % value) + resp = httplib.HTTPResponse(sock, method="GET") + resp.begin() + self.assertEqual(resp.length, 7, repr(value)) + self.assertEqual(resp.read(), 'Hello\r\n') + self.assertTrue(resp.isclosed()) + resp.close() + + def test_malformed_chunk_size(self): + # RFC 9112: chunk-size = 1*HEXDIG. Reject sizes that int(_, 16) accepts + # but the grammar forbids (a sign, an "0x" prefix or leading + # whitespace). + start = 'HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n' + for size in ('-5', '+5', '0x5', ' 5'): + sock = FakeSocket(start + '%s\r\nHELLO\r\n0\r\n\r\n' % size) + resp = httplib.HTTPResponse(sock, method="GET") + resp.begin() + self.assertRaises(httplib.IncompleteRead, resp.read) + self.assertTrue(resp.isclosed(), size) + resp.close() + def test_incomplete_read(self): sock = FakeSocket('HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\nHello\r\n') resp = httplib.HTTPResponse(sock, method="GET")