Merge branch 'pr-642'

test/__init__.py

@@ -2,6 +2,7 @@
 import sys
 import errno
 import functools
+import logging
 import socket
 
 from nose.plugins.skip import SkipTest
@@ -91,3 +92,37 @@ def wrapper(*args, **kwargs):
                 raise SkipTest(msg)
             raise
     return wrapper
+
+
+class _ListHandler(logging.Handler):
+    def __init__(self):
+        super(_ListHandler, self).__init__()
+        self.records = []
+
+    def emit(self, record):
+        self.records.append(record)
+
+
+class LogRecorder(object):
+    def __init__(self, target=logging.root):
+        super(LogRecorder, self).__init__()
+        self._target = target
+        self._handler = _ListHandler()
+
+    @property
+    def records(self):
+        return self._handler.records
+
+    def install(self):
+        self._target.addHandler(self._handler)
+
+    def uninstall(self):
+        self._target.removeHandler(self._handler)
+
+    def __enter__(self):
+        self.install()
+        return self.records
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        self.uninstall()
+        return False

test/test_connectionpool.py

@@ -223,6 +223,12 @@ def test_contextmanager(self):
         self.assertRaises(ClosedPoolError, pool._get_conn)
         self.assertRaises(Empty, old_pool_queue.get, block=False)
 
+    def test_absolute_url(self):
+        c = connection_from_url('http://google.com:80')
+        self.assertEqual(
+                'http://google.com:80/path?query=foo',
+                c._absolute_url('path?query=foo'))
+
 
 if __name__ == '__main__':
     unittest.main()

test/test_exceptions.py

@@ -4,7 +4,7 @@
 from urllib3.exceptions import (HTTPError, MaxRetryError, LocationParseError,
                                 ClosedPoolError, EmptyPoolError,
                                 HostChangedError, ReadTimeoutError,
-                                ConnectTimeoutError)
+                                ConnectTimeoutError, HeaderParsingError)
 from urllib3.connectionpool import HTTPConnectionPool
 
 
@@ -44,3 +44,11 @@ def test_exceptions_with_objects(self):
 
         assert self.verify_pickling(
             ReadTimeoutError(HTTPConnectionPool('localhost'), '/', None))
+
+
+class TestFormat(unittest.TestCase):
+    def test_header_parsing_errors(self):
+        hpe = HeaderParsingError('defects', 'unparsed_data')
+
+        self.assertTrue('defects' in str(hpe))
+        self.assertTrue('unparsed_data' in str(hpe))

test/with_dummyserver/test_socketlevel.py

@@ -10,6 +10,7 @@
         SSLError,
         ProtocolError,
 )
+from urllib3.response import httplib
 from urllib3.util.ssl_ import HAS_SNI
 from urllib3.util.timeout import Timeout
 from urllib3.util.retry import Retry
@@ -18,9 +19,14 @@
 from dummyserver.server import (
     DEFAULT_CERTS, DEFAULT_CA, get_unreachable_address)
 
-from .. import onlyPy3
+from .. import onlyPy3, LogRecorder
 
 from nose.plugins.skip import SkipTest
+try:
+    from mimetools import Message as MimeToolMessage
+except ImportError:
+    class MimeToolMessage(object):
+        pass
 from threading import Event
 import socket
 import ssl
@@ -621,6 +627,52 @@ def test_httplib_headers_case_insensitive(self):
         self.assertEqual(HEADERS, dict(r.headers.items())) # to preserve case sensitivity
 
 
+class TestBrokenHeaders(SocketDummyServerTestCase):
+    def setUp(self):
+        if issubclass(httplib.HTTPMessage, MimeToolMessage):
+            raise SkipTest('Header parsing errors not available')
+
+        super(TestBrokenHeaders, self).setUp()
+
+    def _test_broken_header_parsing(self, headers):
+        handler = create_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'
+        )
+
+        self._start_server(handler)
+        pool = HTTPConnectionPool(self.host, self.port, retries=False)
+
+        with LogRecorder() as logs:
+            pool.request('GET', '/')
+
+        for record in logs:
+            if 'Failed to parse headers' in record.msg and \
+                    pool._absolute_url('/') == record.args[0]:
+                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',
+        ])
+
+    def test_header_without_name_or_value(self):
+        self._test_broken_header_parsing([
+            b':\r\n',
+            b'Another: Header\r\n',
+        ])
+
+    def test_header_without_colon_or_value(self):
+        self._test_broken_header_parsing([
+            b'Broken Header',
+            b'Another: Header',
+        ])
+
+
 class TestHEAD(SocketDummyServerTestCase):
     def test_chunked_head_response_does_not_hang(self):
         handler = create_response_handler(

urllib3/connectionpool.py

@@ -17,6 +17,7 @@
     ClosedPoolError,
     ProtocolError,
     EmptyPoolError,
+    HeaderParsingError,
     HostChangedError,
     LocationValueError,
     MaxRetryError,
@@ -38,9 +39,10 @@
 from .response import HTTPResponse
 
 from .util.connection import is_connection_dropped
+from .util.response import assert_header_parsing
 from .util.retry import Retry
 from .util.timeout import Timeout
-from .util.url import get_host
+from .util.url import get_host, Url
 
 
 xrange = six.moves.xrange
@@ -381,8 +383,19 @@ def _make_request(self, conn, method, url, timeout=_Default,
         log.debug("\"%s %s %s\" %s %s" % (method, url, http_version,
                                           httplib_response.status,
                                           httplib_response.length))
+
+        try:
+            assert_header_parsing(httplib_response.msg)
+        except HeaderParsingError as hpe:  # Platform-specific: Python 3
+            log.warning(
+                'Failed to parse headers (url=%s): %s',
+                self._absolute_url(url), hpe, exc_info=True)
+
         return httplib_response
 
+    def _absolute_url(self, path):
+        return Url(scheme=self.scheme, host=self.host, port=self.port, path=path).url
+
     def close(self):
         """
         Close all pooled connections and disable the pool.

urllib3/exceptions.py

@@ -176,3 +176,10 @@ class ProxySchemeUnknown(AssertionError, ValueError):
     def __init__(self, scheme):
         message = "Not supported proxy scheme %s" % scheme
         super(ProxySchemeUnknown, self).__init__(message)
+
+
+class HeaderParsingError(HTTPError):
+    "Raised by assert_header_parsing, but we convert it to a log.warning statement."
+    def __init__(self, defects, unparsed_data):
+        message = '%s, unparsed data: %r' % (defects or 'Unknown', unparsed_data)
+        super(HeaderParsingError, self).__init__(message)

urllib3/response.py

@@ -319,6 +319,7 @@ def from_httplib(ResponseCls, r, **response_kw):
         with ``original_response=r``.
         """
         headers = r.msg
+
         if not isinstance(headers, HTTPHeaderDict):
             if PY3: # Python 3
                 headers = HTTPHeaderDict(headers.items())

urllib3/util/response.py

@@ -1,3 +1,11 @@
+try:
+    import http.client as httplib
+except ImportError:
+    import httplib
+
+from ..exceptions import HeaderParsingError
+
+
 def is_fp_closed(obj):
     """
     Checks whether a given file-like object is closed.
@@ -20,3 +28,34 @@ def is_fp_closed(obj):
         pass
 
     raise ValueError("Unable to determine whether fp is closed.")
+
+
+def assert_header_parsing(headers):
+    """
+    Asserts whether all headers have been successfully parsed.
+    Extracts encountered errors from the result of parsing headers.
+
+    Only works on Python 3.
+
+    :param headers: Headers to verify.
+    :type headers: `httplib.HTTPMessage`.
+
+    :raises urllib3.exceptions.HeaderParsingError:
+        If parsing errors are found.
+    """
+
+    # This will fail silently if we pass in the wrong kind of parameter.
+    # To make debugging easier add an explicit check.
+    if not isinstance(headers, httplib.HTTPMessage):
+        raise TypeError('expected httplib.Message, got {}.'.format(
+            type(headers)))
+
+    defects = getattr(headers, 'defects', None)
+    get_payload = getattr(headers, 'get_payload', None)
+
+    unparsed_data = None
+    if get_payload:  # Platform-specific: Python 3.
+        unparsed_data = get_payload()
+
+    if defects or unparsed_data:
+        raise HeaderParsingError(defects=defects, unparsed_data=unparsed_data)