Add tests for binary-vs-text body

scrapy/utils/response.py

@@ -141,22 +141,6 @@ def _get_response_class_from_mime_type(mime_type):
     return Response
 
 
-def _remove_nul_byte_from_text(text):
-    """Return the text with removed null byte (b'\x00') if there are no other
-    binary bytes in the text, otherwise return the text as-is.
-
-    Based on https://github.com/scrapy/scrapy/issues/2481
-    """
-    for index in range(len(text)):
-        if (
-            text[index:index + 1] != b'\x00'
-            and is_binary_data(text[index:index + 1])
-        ):
-            return text
-
-    return text.replace(b'\x00', b'')
-
-
 def get_base_url(response: TextResponse) -> str:
     """Return the base url of the given response, joined with the response url"""
     warn(
@@ -214,7 +198,7 @@ def get_response_class(
                 mime_type = path_mime_type
     else:
         http_origin = True
-    body = _remove_nul_byte_from_text((body or b'')[:BODY_LIMIT])
+    body = (body or b'')[:BODY_LIMIT]
     if encoding:
         content_types = (_get_mime_type_from_encoding(encoding),)
     elif mime_type:

tests/test_utils_response.py

@@ -6,6 +6,7 @@ from pathlib import Path
 from urllib.parse import urlparse
 
 import pytest
+from xtractmime import BINARY_BYTES, RESOURCE_HEADER_BUFFER_LENGTH
 
 from scrapy.exceptions import ScrapyDeprecationWarning
 from scrapy.http import HtmlResponse, Response, TextResponse, XmlResponse
@@ -44,28 +45,33 @@ PRE_XTRACTMIME_SCENARIOS = (
             ("text/html", HtmlResponse),
             ("text/html; charset=utf-8", HtmlResponse),
             ("text/xml", XmlResponse),
+            *(
+                (mime_type, load_object(class_path))
+                for mime_type, class_path in ResponseTypes.CLASSES.items()
+            ),
         )
     ),
 
-    # Even if the body is binary, if the Content-Type says it is text, we
-    # interpret it as text, as long as the Content-Type is not one of the 4
-    # affected by the Apache bug.
-    #
-    # https://mimesniff.spec.whatwg.org/#interpreting-the-resource-metadata
+    # Content-Type triumphs body, except for the Apache bug special case.
     *(
         (
             {
-                'body': b'\x00\x01\xff',
+                'body': body,
                 'headers': Headers({'Content-Type': [content_type]}),
             },
-            TextResponse,
+            response_class,
         )
-        for content_type in (
-            'text/json',
-            # text/plain variants *not* affected by the Apache bug
-            'text/plain; charset=Iso-8859-1',
-            'text/plain; charset=utf-8',
-            'text/plain; charset=windows-1252',
+        for body, content_type, response_class in (
+            *(
+                (b'\x00\x01\xff', content_type, TextResponse)
+                for content_type in (
+                    'text/json',
+                    # text/plain variants *not* affected by the Apache bug
+                    'text/plain; charset=Iso-8859-1',
+                    'text/plain; charset=utf-8',
+                    'text/plain; charset=windows-1252',
+                )
+            ),
         )
     ),
 
@@ -129,20 +135,6 @@ PRE_XTRACTMIME_SCENARIOS = (
         )
     ),
 
-    # We continue to support the hard-coded MIME-to-Response-class mappings
-    # from scrapy.responsetypes.
-    *(
-        (
-            {
-                'headers': Headers(
-                    {'Content-Type': [mime_type]}
-                ),
-            },
-            load_object(class_path),
-        )
-        for mime_type, class_path in ResponseTypes.CLASSES.items()
-    ),
-
     # We take the file extension of URL paths into account, except for HTTP
     # responses, because “they are unreliable and easily spoofed”.
     #
@@ -207,26 +199,22 @@ PRE_XTRACTMIME_SCENARIOS = (
         )
     ),
 
-    # TODO: Make sure that we have a test that checks that Content-Type
-    # triumphs body.
-    (
-        {
-            'url': 'http://www.example.com/page/',
-            'headers': Headers(
-                {
-                    'Content-Disposition': [
-                        'attachment; filename="data.xml.gz"',
-                    ]
-                }
+    # A body is considered binary if its header (first 1445 bytes) contains any
+    # binary data byte.
+    *(
+        ({"body": body}, response_class)
+        for body, response_class in (
+            *((byte, Response) for byte in BINARY_BYTES[1:]),
+            # Binary characters at the end of the header still count.
+            *(
+                (b"a"*(RESOURCE_HEADER_BUFFER_LENGTH-1) + byte, Response)
+                for byte in BINARY_BYTES[1:]
             ),
-            'body': b'\x01\x02',
-        },
-        Response,
-    ),
-    (
-        {'body': b'Some plain\0 text data with\0 tabs and null bytes\0'},
-        TextResponse,
+            # Binary characters right after the header do not count.
+            (b"a"*RESOURCE_HEADER_BUFFER_LENGTH + BINARY_BYTES[0], TextResponse),
+        )
     ),
+
     (
         {
             'body': b'\x03\x02\xdf\xdd\x23',
@@ -401,6 +389,22 @@ POST_XTRACTMIME_SCENARIOS = (
         )
     ),
 
+    # A body is considered binary if its header (first 1445 bytes) contains any
+    # binary data byte.
+    *(
+        ({"body": body}, response_class)
+        for body, response_class in (
+            (BINARY_BYTES[0], Response),
+            # Binary characters at the end of the header still count.
+            (b"a"*(RESOURCE_HEADER_BUFFER_LENGTH-1) + BINARY_BYTES[0], Response),
+            # Binary characters right after the header do not count.
+            *(
+                (b"a"*RESOURCE_HEADER_BUFFER_LENGTH + byte, TextResponse)
+                for byte in BINARY_BYTES[1:]
+            ),
+        )
+    ),
+
     (
         {
             'body': b'Some plain text',