diff --git a/changelog.d/294.removal.rst b/changelog.d/294.removal.rst new file mode 100644 index 00000000..deac5dfb --- /dev/null +++ b/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. diff --git a/src/treq/client.py b/src/treq/client.py index 113d6bb6..ba4f0a1c 100644 --- a/src/treq/client.py +++ b/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: diff --git a/src/treq/test/test_client.py b/src/treq/test/test_client.py index 51a316d2..a4dc9577 100644 --- a/src/treq/test/test_client.py +++ b/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