From b2f2788bd41febc62853bf38351844598d89f823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Sun, 8 Jan 2023 16:36:35 +0100 Subject: [PATCH] Add tests for binary-vs-text body --- scrapy/utils/response.py | 18 +------ tests/test_utils_response.py | 94 +++++++++++++++++++----------------- 2 files changed, 50 insertions(+), 62 deletions(-) diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index ea6b33f96..3878e4f45 100644 --- a/scrapy/utils/response.py +++ b/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: diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index c35ba4025..32e82172a 100644 --- a/tests/test_utils_response.py +++ b/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',