Merge pull request #298 from twisted/294-non-string-headers

Deprecate ignoring non-string headers

changelog.d/294.removal.rst

@@ -0,0 +1 @@
+Deprecate tolerance of non-string values when passing headers as a dict. They have historically been silently dropped, but will raise TypeError in the next treq release. Also deprecate passing headers other than :class:`dict`, :class:`~twisted.web.http_headers.Headers`, or ``None``. Historically falsy values like ``[]`` or ``()`` were accepted.

src/treq/client.py

@@ -171,27 +171,13 @@ def request(self, method, url, **kwargs):
 
         url = parsed_url.to_uri().to_text().encode('ascii')
 
-        # Convert headers dictionary to
-        # twisted raw headers format.
-        headers = kwargs.pop('headers', None)
-        if headers:
-            if isinstance(headers, dict):
-                h = Headers({})
-                for k, v in headers.items():
-                    if isinstance(v, (bytes, six.text_type)):
-                        h.addRawHeader(k, v)
-                    elif isinstance(v, list):
-                        h.setRawHeaders(k, v)
-
-                headers = h
-        else:
-            headers = Headers({})
+        headers = self._request_headers(kwargs.pop('headers', None), stacklevel + 1)
 
         bodyProducer, contentType = self._request_body(
             data=kwargs.pop('data', None),
             files=kwargs.pop('files', None),
             json=kwargs.pop('json', _NOTHING),
-            stacklevel=stacklevel,
+            stacklevel=stacklevel + 1,
         )
         if contentType is not None:
             headers.setRawHeaders(b'Content-Type', [contentType])
@@ -252,6 +238,47 @@ def gotResult(result):
 
         return d.addCallback(_Response, cookies)
 
+    def _request_headers(self, headers, stacklevel):
+        """
+        Convert the *headers* argument to a :class:`Headers` instance
+
+        :returns:
+            :class:`twisted.web.http_headers.Headers`
+        """
+        if isinstance(headers, dict):
+            h = Headers({})
+            for k, v in headers.items():
+                if isinstance(v, (bytes, six.text_type)):
+                    h.addRawHeader(k, v)
+                elif isinstance(v, list):
+                    h.setRawHeaders(k, v)
+                else:
+                    warnings.warn(
+                        (
+                            "The value of headers key {!r} has non-string type {}"
+                            " and will be dropped."
+                            " This will raise TypeError in the next treq release."
+                        ).format(k, type(v)),
+                        DeprecationWarning,
+                        stacklevel=stacklevel,
+                    )
+            return h
+        if isinstance(headers, Headers):
+            return headers
+        if headers is None:
+            return Headers({})
+
+        warnings.warn(
+            (
+                "headers must be a dict, twisted.web.http_headers.Headers, or None,"
+                " but found {}, which will be ignored."
+                " This will raise TypeError in the next treq release."
+            ).format(type(headers)),
+            DeprecationWarning,
+            stacklevel=stacklevel,
+        )
+        return Headers({})
+
     def _request_body(self, data, files, json, stacklevel):
         """
         Here we choose a right producer based on the parameters passed in.
@@ -287,7 +314,7 @@ def _request_body(self, data, files, json, stacklevel):
                     " This will raise TypeError in the next treq release."
                 ).format("data" if data else "files"),
                 DeprecationWarning,
-                stacklevel=stacklevel + 1,
+                stacklevel=stacklevel,
             )
 
         if files:

src/treq/test/test_client.py

@@ -1,4 +1,5 @@
 # -*- encoding: utf-8 -*-
+from collections import OrderedDict
 from io import BytesIO
 
 import mock
@@ -517,6 +518,43 @@ def test_request_dict_headers(self):
                      b'Accept': [b'application/json', b'text/plain']}),
             None)
 
+    def test_request_headers_invalid_type(self):
+        """
+        `HTTPClient.request()` warns that headers of an unexpected type are
+        invalid and that this behavior is deprecated.
+        """
+        self.client.request('GET', 'http://example.com', headers=[])
+
+        [w] = self.flushWarnings([self.test_request_headers_invalid_type])
+        self.assertEqual(DeprecationWarning, w['category'])
+        self.assertIn(
+            "headers must be a dict, twisted.web.http_headers.Headers, or None,",
+            w['message'],
+        )
+
+    def test_request_dict_headers_invalid_values(self):
+        """
+        `HTTPClient.request()` warns that non-string header values are dropped
+        and that this behavior is deprecated.
+        """
+        self.client.request('GET', 'http://example.com', headers=OrderedDict([
+            ('none', None),
+            ('one', 1),
+            ('ok', 'string'),
+        ]))
+
+        [w1, w2] = self.flushWarnings([self.test_request_dict_headers_invalid_values])
+        self.assertEqual(DeprecationWarning, w1['category'])
+        self.assertEqual(DeprecationWarning, w2['category'])
+        self.assertIn(
+            "The value of headers key 'none' has non-string type",
+            w1['message'],
+        )
+        self.assertIn(
+            "The value of headers key 'one' has non-string type",
+            w2['message'],
+        )
+
     def test_request_invalid_param(self):
         """
         `HTTPClient.request()` warns that invalid parameters are ignored and