Skip to content

Commit

Permalink
Fix for parsing Content-Type: message/* Responses without warnings (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
timb07 authored and sethmlarson committed Sep 19, 2018
1 parent 285889d commit f4efcca
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 10 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Expand Up @@ -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 #)


Expand Down
3 changes: 3 additions & 0 deletions CONTRIBUTORS.txt
Expand Up @@ -266,6 +266,9 @@ In chronological order:
* Hugo van Kemenade <https://github.com/hugovk>
* Drop support for EOL Python 2.6

* Tim Bell <https://github.com/timb07>
* Bugfix for responses with Content-Type: message/* logging warnings

* Justin Bramley <https://github.com/jbramleycl>
* Add ability to handle multiple Content-Encodings

Expand Down
10 changes: 8 additions & 2 deletions src/urllib3/util/response.py
Expand Up @@ -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)
Expand Down
42 changes: 34 additions & 8 deletions test/with_dummyserver/test_socketlevel.py
Expand Up @@ -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)
Expand All @@ -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):
Expand Down

0 comments on commit f4efcca

Please sign in to comment.