Fixed #18796 -- Refactored conversion to bytes in HttpResponse

Thanks mrmachine for the review.

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.