From da56e1bac6449daef9aeab8d076d2594d9fd5b44 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Wed, 24 Oct 2012 23:41:45 +0200 Subject: [PATCH] Fixed #18796 -- Refactored conversion to bytes in HttpResponse Thanks mrmachine for the review. --- django/http/response.py | 57 ++++++++++----------- tests/regressiontests/httpwrappers/tests.py | 9 ++-- 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/django/http/response.py b/django/http/response.py index 6bbadd7e2b..56e3d00096 100644 --- a/django/http/response.py +++ b/django/http/response.py @@ -16,6 +16,7 @@ from django.http.cookie import SimpleCookie from django.utils import six, timezone from django.utils.encoding import force_bytes, iri_to_uri from django.utils.http import cookie_date +from django.utils.six.moves import map class BadHeaderError(ValueError): @@ -191,18 +192,33 @@ class HttpResponseBase(object): def make_bytes(self, value): """Turn a value into a bytestring encoded in the output charset.""" - # For backwards compatibility, this method supports values that are - # unlikely to occur in real applications. It has grown complex and - # should be refactored. It also overlaps __next__. See #18796. + # Per PEP 3333, this response body must be bytes. To avoid returning + # an instance of a subclass, this function returns `bytes(value)`. + # This doesn't make a copy when `value` already contains bytes. + + # If content is already encoded (eg. gzip), assume bytes. if self.has_header('Content-Encoding'): - if isinstance(value, int): - value = six.text_type(value) - if isinstance(value, six.text_type): - value = value.encode('ascii') - # force conversion to bytes in case chunk is a subclass return bytes(value) - else: - return force_bytes(value, self._charset) + + # Handle string types -- we can't rely on force_bytes here because: + # - under Python 3 it attemps str conversion first + # - when self._charset != 'utf-8' it re-encodes the content + if isinstance(value, bytes): + return bytes(value) + if isinstance(value, six.text_type): + return bytes(value.encode(self._charset)) + + # Handle non-string types (#16494) + return force_bytes(value, self._charset) + + def __iter__(self): + return self + + def __next__(self): + # Subclasses must define self._iterator for this function. + return self.make_bytes(next(self._iterator)) + + next = __next__ # Python 2 compatibility # These methods partially implement the file-like object interface. # See http://docs.python.org/lib/bltin-file-objects.html @@ -287,17 +303,6 @@ class HttpResponse(HttpResponseBase): self._iterator = iter(self._container) return self - def __next__(self): - chunk = next(self._iterator) - if isinstance(chunk, int): - chunk = six.text_type(chunk) - if isinstance(chunk, six.text_type): - chunk = chunk.encode(self._charset) - # force conversion to bytes in case chunk is a subclass - return bytes(chunk) - - next = __next__ # Python 2 compatibility - def write(self, content): self._consume_content() self._container.append(content) @@ -331,7 +336,7 @@ class StreamingHttpResponse(HttpResponseBase): @property def streaming_content(self): - return self._iterator + return map(self.make_bytes, self._iterator) @streaming_content.setter def streaming_content(self, value): @@ -340,14 +345,6 @@ class StreamingHttpResponse(HttpResponseBase): if hasattr(value, 'close'): self._closable_objects.append(value) - def __iter__(self): - return self - - def __next__(self): - return self.make_bytes(next(self._iterator)) - - next = __next__ # Python 2 compatibility - class CompatibleStreamingHttpResponse(StreamingHttpResponse): """ diff --git a/tests/regressiontests/httpwrappers/tests.py b/tests/regressiontests/httpwrappers/tests.py index 8e20c80de3..d908e33fb7 100644 --- a/tests/regressiontests/httpwrappers/tests.py +++ b/tests/regressiontests/httpwrappers/tests.py @@ -330,11 +330,12 @@ class HttpResponseTests(unittest.TestCase): self.assertEqual(r.content, b'123\xde\x9e') #with Content-Encoding header - r = HttpResponse([1,1,2,4,8]) + r = HttpResponse() r['Content-Encoding'] = 'winning' - self.assertEqual(r.content, b'11248') - r.content = ['\u079e',] - self.assertRaises(UnicodeEncodeError, + r.content = [b'abc', b'def'] + self.assertEqual(r.content, b'abcdef') + r.content = ['\u079e'] + self.assertRaises(TypeError if six.PY3 else UnicodeEncodeError, getattr, r, 'content') # .content can safely be accessed multiple times.