From 9e265a2c1f6bccb551e8292785e09462594b8402 Mon Sep 17 00:00:00 2001
From: Kromitvs <74136201+Kromitvs@users.noreply.github.com>
Date: Thu, 16 Jun 2022 19:52:19 +0100
Subject: [PATCH] Mind body to choose response class in cache, FTP and HTTP/1.0
(#4873)
---
scrapy/core/downloader/handlers/ftp.py | 6 +--
scrapy/core/downloader/webclient.py | 2 +-
scrapy/extensions/httpcache.py | 4 +-
scrapy/responsetypes.py | 10 ++--
tests/test_downloader_handlers.py | 54 ++++++++++++++++++--
tests/test_downloadermiddleware_httpcache.py | 15 ++++++
tests/test_responsetypes.py | 2 +
7 files changed, 78 insertions(+), 15 deletions(-)
diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py
index 3ef129587..a495874bd 100644
--- a/scrapy/core/downloader/handlers/ftp.py
+++ b/scrapy/core/downloader/handlers/ftp.py
@@ -102,11 +102,11 @@ class FTPDownloadHandler:
def _build_response(self, result, request, protocol):
self.result = result
- respcls = responsetypes.from_args(url=request.url)
protocol.close()
- body = protocol.filename or protocol.body.read()
headers = {"local filename": protocol.filename or '', "size": protocol.size}
- return respcls(url=request.url, status=200, body=to_bytes(body), headers=headers)
+ body = to_bytes(protocol.filename or protocol.body.read())
+ respcls = responsetypes.from_args(url=request.url, body=body)
+ return respcls(url=request.url, status=200, body=body, headers=headers)
def _failed(self, result, request):
message = result.getErrorMessage()
diff --git a/scrapy/core/downloader/webclient.py b/scrapy/core/downloader/webclient.py
index 06cb96489..7d048c1e4 100644
--- a/scrapy/core/downloader/webclient.py
+++ b/scrapy/core/downloader/webclient.py
@@ -112,7 +112,7 @@ class ScrapyHTTPClientFactory(ClientFactory):
request.meta['download_latency'] = self.headers_time - self.start_time
status = int(self.status)
headers = Headers(self.response_headers)
- respcls = responsetypes.from_args(headers=headers, url=self._url)
+ respcls = responsetypes.from_args(headers=headers, url=self._url, body=body)
return respcls(url=self._url, status=status, headers=headers, body=body, protocol=to_unicode(self.version))
def _set_connection_attributes(self, request):
diff --git a/scrapy/extensions/httpcache.py b/scrapy/extensions/httpcache.py
index c71484cfa..843e14812 100644
--- a/scrapy/extensions/httpcache.py
+++ b/scrapy/extensions/httpcache.py
@@ -240,7 +240,7 @@ class DbmCacheStorage:
status = data['status']
headers = Headers(data['headers'])
body = data['body']
- respcls = responsetypes.from_args(headers=headers, url=url)
+ respcls = responsetypes.from_args(headers=headers, url=url, body=body)
response = respcls(url=url, headers=headers, status=status, body=body)
return response
@@ -299,7 +299,7 @@ class FilesystemCacheStorage:
url = metadata.get('response_url')
status = metadata['status']
headers = Headers(headers_raw_to_dict(rawheaders))
- respcls = responsetypes.from_args(headers=headers, url=url)
+ respcls = responsetypes.from_args(headers=headers, url=url, body=body)
response = respcls(url=url, headers=headers, status=status, body=body)
return response
diff --git a/scrapy/responsetypes.py b/scrapy/responsetypes.py
index 6ed9f8b8f..3efd4d2fd 100644
--- a/scrapy/responsetypes.py
+++ b/scrapy/responsetypes.py
@@ -95,12 +95,14 @@ class ResponseTypes:
chunk = to_bytes(chunk)
if not binary_is_text(chunk):
return self.from_mimetype('application/octet-stream')
- elif b"" in chunk.lower():
+ lowercase_chunk = chunk.lower()
+ if b"" in lowercase_chunk:
return self.from_mimetype('text/html')
- elif b"' in lowercase_chunk:
+ return self.from_mimetype('text/html')
+ return self.from_mimetype('text')
def from_args(self, headers=None, url=None, filename=None, body=None):
"""Guess the most appropriate Response class based on
diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py
index 2bb53950d..72f52121e 100644
--- a/tests/test_downloader_handlers.py
+++ b/tests/test_downloader_handlers.py
@@ -25,7 +25,7 @@ from scrapy.core.downloader.handlers.http10 import HTTP10DownloadHandler
from scrapy.core.downloader.handlers.http11 import HTTP11DownloadHandler
from scrapy.core.downloader.handlers.s3 import S3DownloadHandler
from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning
-from scrapy.http import Headers, Request
+from scrapy.http import Headers, HtmlResponse, Request
from scrapy.http.response.text import TextResponse
from scrapy.responsetypes import responsetypes
from scrapy.spiders import Spider
@@ -389,6 +389,23 @@ class HttpTestCase(unittest.TestCase):
d.addCallback(self.assertEqual, b'159')
return d
+ def _test_response_class(self, filename, body, response_class):
+ def _test(response):
+ self.assertEqual(type(response), response_class)
+
+ request = Request(self.getURL(filename), body=body)
+ return self.download_request(request, Spider('foo')).addCallback(_test)
+
+ def test_response_class_from_url(self):
+ return self._test_response_class('foo.html', b'', HtmlResponse)
+
+ def test_response_class_from_body(self):
+ return self._test_response_class(
+ 'foo',
+ b"\n.",
+ HtmlResponse,
+ )
+
class Http10TestCase(HttpTestCase):
"""HTTP 1.0 test case"""
@@ -971,6 +988,12 @@ class BaseFTPTestCase(unittest.TestCase):
password = "passwd"
req_meta = {"ftp_user": username, "ftp_password": password}
+ test_files = (
+ ('file.txt', b"I have the power!"),
+ ('file with spaces.txt', b"Moooooooooo power!"),
+ ('html-file-without-extension', b"\n."),
+ )
+
def setUp(self):
from twisted.protocols.ftp import FTPRealm, FTPFactory
from scrapy.core.downloader.handlers.ftp import FTPDownloadHandler
@@ -981,8 +1004,8 @@ class BaseFTPTestCase(unittest.TestCase):
userdir = os.path.join(self.directory, self.username)
os.mkdir(userdir)
fp = FilePath(userdir)
- fp.child('file.txt').setContent(b"I have the power!")
- fp.child('file with spaces.txt').setContent(b"Moooooooooo power!")
+ for filename, content in self.test_files:
+ fp.child(filename).setContent(content)
# setup server
realm = FTPRealm(anonymousRoot=self.directory, userHome=self.directory)
@@ -1069,6 +1092,27 @@ class BaseFTPTestCase(unittest.TestCase):
return self._add_test_callbacks(d, _test)
+ def _test_response_class(self, filename, response_class):
+ f, local_fname = tempfile.mkstemp()
+ local_fname = to_bytes(local_fname)
+ os.close(f)
+ meta = {}
+ meta.update(self.req_meta)
+ request = Request(url=f"ftp://127.0.0.1:{self.portNum}/{filename}",
+ meta=meta)
+ d = self.download_handler.download_request(request, None)
+
+ def _test(r):
+ self.assertEqual(type(r), response_class)
+ os.remove(local_fname)
+ return self._add_test_callbacks(d, _test)
+
+ def test_response_class_from_url(self):
+ return self._test_response_class('file.txt', TextResponse)
+
+ def test_response_class_from_body(self):
+ return self._test_response_class('html-file-without-extension', HtmlResponse)
+
class FTPTestCase(BaseFTPTestCase):
@@ -1104,8 +1148,8 @@ class AnonymousFTPTestCase(BaseFTPTestCase):
os.mkdir(self.directory)
fp = FilePath(self.directory)
- fp.child('file.txt').setContent(b"I have the power!")
- fp.child('file with spaces.txt').setContent(b"Moooooooooo power!")
+ for filename, content in self.test_files:
+ fp.child(filename).setContent(content)
# setup server for anonymous access
realm = FTPRealm(anonymousRoot=self.directory)
diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py
index 0c6dcf2aa..928c007f5 100644
--- a/tests/test_downloadermiddleware_httpcache.py
+++ b/tests/test_downloadermiddleware_httpcache.py
@@ -122,6 +122,21 @@ class DefaultStorageTest(_BaseTest):
time.sleep(0.5) # give the chance to expire
assert storage.retrieve_response(self.spider, self.request)
+ def test_storage_no_content_type_header(self):
+ """Test that the response body is used to get the right response class
+ even if there is no Content-Type header"""
+ with self._storage() as storage:
+ assert storage.retrieve_response(self.spider, self.request) is None
+ response = Response(
+ 'http://www.example.com',
+ body=b'\n.',
+ status=202,
+ )
+ storage.store_response(self.spider, self.request, response)
+ cached_response = storage.retrieve_response(self.spider, self.request)
+ self.assertIsInstance(cached_response, HtmlResponse)
+ self.assertEqualResponse(response, cached_response)
+
class DbmStorageTest(DefaultStorageTest):
diff --git a/tests/test_responsetypes.py b/tests/test_responsetypes.py
index c07d3a99c..4b4095fb0 100644
--- a/tests/test_responsetypes.py
+++ b/tests/test_responsetypes.py
@@ -54,6 +54,8 @@ class ResponseTypesTest(unittest.TestCase):
(b'\x03\x02\xdf\xdd\x23', Response),
(b'Some plain text\ndata with tabs\t and null bytes\0', TextResponse),
(b'Hello', HtmlResponse),
+ # https://codersblock.com/blog/the-smallest-valid-html5-page/
+ (b'\n.', HtmlResponse),
(b'