From ce8dccfff45c0bc2bb01a3b3ceb6cf8da805a904 Mon Sep 17 00:00:00 2001 From: Tim Bell Date: Wed, 19 Sep 2018 11:47:11 +1000 Subject: [PATCH] Fix for parsing Content-Type: message/* Responses without warnings (#1439) --- CHANGES.rst | 3 ++ CONTRIBUTORS.txt | 3 ++ src/urllib3/util/response.py | 10 ++++-- test/with_dummyserver/test_socketlevel.py | 42 ++++++++++++++++++----- 4 files changed, 48 insertions(+), 10 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 5f2f65559c..0b912d0b37 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,6 +13,9 @@ dev (master) * Drop support for EOL Python 2.6 (Pull #1429 and Pull #1430) +* Fixed bug where responses with header Content-Type: message/* erroneously + raised HeaderParsingError, resulting in a warning being logged. (Pull #1439) + * ... [Short description of non-trivial change.] (Issue #) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 7d868a8f7d..71813f95f5 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -266,6 +266,9 @@ In chronological order: * Hugo van Kemenade * Drop support for EOL Python 2.6 +* Tim Bell + * Bugfix for responses with Content-Type: message/* logging warnings + * Justin Bramley * Add ability to handle multiple Content-Encodings diff --git a/src/urllib3/util/response.py b/src/urllib3/util/response.py index 67cf730ab0..3d5486485a 100644 --- a/src/urllib3/util/response.py +++ b/src/urllib3/util/response.py @@ -59,8 +59,14 @@ def assert_header_parsing(headers): get_payload = getattr(headers, 'get_payload', None) unparsed_data = None - if get_payload: # Platform-specific: Python 3. - unparsed_data = get_payload() + if get_payload: + # get_payload is actually email.message.Message.get_payload; + # we're only interested in the result if it's not a multipart message + if not headers.is_multipart(): + payload = get_payload() + + if isinstance(payload, (bytes, str)): + unparsed_data = payload if defects or unparsed_data: raise HeaderParsingError(defects=defects, unparsed_data=unparsed_data) diff --git a/test/with_dummyserver/test_socketlevel.py b/test/with_dummyserver/test_socketlevel.py index ec2f01d227..8f2cf9e957 100644 --- a/test/with_dummyserver/test_socketlevel.py +++ b/test/with_dummyserver/test_socketlevel.py @@ -1289,12 +1289,12 @@ def socket_handler(listener): ) class TestBrokenHeaders(SocketDummyServerTestCase): - def _test_broken_header_parsing(self, headers): + def _test_broken_header_parsing(self, headers, unparsed_data_check=None): self.start_response_handler(( b'HTTP/1.1 200 OK\r\n' b'Content-Length: 0\r\n' b'Content-type: text/plain\r\n' - ) + b'\r\n'.join(headers) + b'\r\n' + ) + b'\r\n'.join(headers) + b'\r\n\r\n' ) pool = HTTPConnectionPool(self.host, self.port, retries=False) @@ -1306,26 +1306,52 @@ def _test_broken_header_parsing(self, headers): for record in logs: if 'Failed to parse headers' in record.msg and \ pool._absolute_url('/') == record.args[0]: - return + if unparsed_data_check is None or unparsed_data_check in record.getMessage(): + return self.fail('Missing log about unparsed headers') def test_header_without_name(self): self._test_broken_header_parsing([ - b': Value\r\n', - b'Another: Header\r\n', + b': Value', + b'Another: Header', ]) def test_header_without_name_or_value(self): self._test_broken_header_parsing([ - b':\r\n', - b'Another: Header\r\n', + b':', + b'Another: Header', ]) def test_header_without_colon_or_value(self): self._test_broken_header_parsing([ b'Broken Header', b'Another: Header', - ]) + ], 'Broken Header') + + +class TestHeaderParsingContentType(SocketDummyServerTestCase): + + def _test_okay_header_parsing(self, header): + self.start_response_handler(( + b'HTTP/1.1 200 OK\r\n' + b'Content-Length: 0\r\n' + ) + header + b'\r\n\r\n' + ) + + pool = HTTPConnectionPool(self.host, self.port, retries=False) + self.addCleanup(pool.close) + + with LogRecorder() as logs: + pool.request('GET', '/') + + for record in logs: + assert 'Failed to parse headers' not in record.msg + + def test_header_text_plain(self): + self._test_okay_header_parsing(b'Content-type: text/plain') + + def test_header_message_rfc822(self): + self._test_okay_header_parsing(b'Content-type: message/rfc822') class TestHEAD(SocketDummyServerTestCase):