From 197781e3af51cd46ae7761938afa474c40c36b76 Mon Sep 17 00:00:00 2001 From: Yash nagarkar <116726926+yash08123@users.noreply.github.com> Date: Fri, 22 Sep 2023 13:42:20 +0530 Subject: [PATCH 01/60] Cover the removal of is_botocore on the release notes (#6061) --- docs/news.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index fc3cfd9e8..c5b75aae2 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -469,6 +469,10 @@ Deprecation removals has now been removed. (:issue:`5719`) +- The ``scrapy.utils.boto.is_botocore()`` function, deprecated in Scrapy 2.4, + has now been removed. + (:issue:`5719`) + Deprecations ~~~~~~~~~~~~ From 8dc72dfc4d5a651e034a956a03d8e0a5f4c8a94d Mon Sep 17 00:00:00 2001 From: kokobhara <146670393+kokobhara@users.noreply.github.com> Date: Mon, 2 Oct 2023 15:44:05 +0530 Subject: [PATCH 02/60] Cover PythonItemExporter backwaird-incompatible changes in 2.11 (#6081) --- docs/news.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index c5b75aae2..fd8fa3ea3 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -40,6 +40,9 @@ Backward-incompatible changes UTF-32). If you need to deal with JSON documents in an invalid encoding, use ``json.loads(response.text)`` instead. (:issue:`6016`) +- :class:`~scrapy.exporters.PythonItemExporter` used the binary output by + default but it no longer does. (:issue:`6006`, :issue:`6007`) + Deprecation removals ~~~~~~~~~~~~~~~~~~~~ From 5fccf370b87378fe2db6bdd52b98c1e2a951df3b Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 15 Nov 2023 15:38:13 +0100 Subject: [PATCH 03/60] Update the RTD URL for coverage --- docs/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index 38ca81932..9ca0f817a 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -276,7 +276,7 @@ coverage_ignore_pyobjects = [ intersphinx_mapping = { "attrs": ("https://www.attrs.org/en/stable/", None), - "coverage": ("https://coverage.readthedocs.io/en/stable", None), + "coverage": ("https://coverage.readthedocs.io/en/latest", None), "cryptography": ("https://cryptography.io/en/latest/", None), "cssselect": ("https://cssselect.readthedocs.io/en/latest", None), "itemloaders": ("https://itemloaders.readthedocs.io/en/latest/", None), From 080fecd8900b6b1f94e8e143e90338279ba8d6e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 15 Nov 2023 15:39:30 +0100 Subject: [PATCH 04/60] Drop the Authorization header on cross-domain redirect --- docs/news.rst | 27 ++++++++++++++++++ scrapy/downloadermiddlewares/redirect.py | 10 +++++-- tests/test_downloadermiddleware_redirect.py | 31 +++++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index fd8fa3ea3..b19ec2e99 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -3,6 +3,20 @@ Release notes ============= +.. _release-2.11.1: + +Scrapy 2.11.1 (unreleased) +-------------------------- + +**Security bug fix:** + +- The ``Authorization`` header is now dropped on redirects to a different + domain. Please, see the `cw9j-q3vf-hrrv security advisory`_ for more + information. + + .. _cw9j-q3vf-hrrv security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-cw9j-q3vf-hrrv + + .. _release-2.11.0: Scrapy 2.11.0 (2023-09-18) @@ -2869,6 +2883,19 @@ affect subclasses: (:issue:`3884`) +.. _release-1.8.4: + +Scrapy 1.8.4 (unreleased) +------------------------- + +**Security bug fix:** + +- The ``Authorization`` header is now dropped on redirects to a different + domain. Please, see the `cw9j-q3vf-hrrv security advisory`_ for more + information. + + .. _cw9j-q3vf-hrrv security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-cw9j-q3vf-hrrv + .. _release-1.8.3: diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 65f1d2224..3176ed930 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -17,11 +17,17 @@ def _build_redirect_request(source_request, *, url, **kwargs): **kwargs, cookies=None, ) - if "Cookie" in redirect_request.headers: + has_cookie_header = "Cookie" in redirect_request.headers + has_authorization_header = "Authorization" in redirect_request.headers + if has_cookie_header or has_authorization_header: source_request_netloc = urlparse_cached(source_request).netloc redirect_request_netloc = urlparse_cached(redirect_request).netloc if source_request_netloc != redirect_request_netloc: - del redirect_request.headers["Cookie"] + if has_cookie_header: + del redirect_request.headers["Cookie"] + # https://fetch.spec.whatwg.org/#ref-for-cors-non-wildcard-request-header-name + if has_authorization_header: + del redirect_request.headers["Authorization"] return redirect_request diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index dc15b672c..10b8ca9af 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -247,6 +247,37 @@ class RedirectMiddlewareTest(unittest.TestCase): perc_encoded_utf8_url = "http://scrapytest.org/a%C3%A7%C3%A3o" self.assertEqual(perc_encoded_utf8_url, req_result.url) + def test_cross_domain_header_dropping(self): + safe_headers = {"A": "B"} + original_request = Request( + "https://example.com", + headers={"Cookie": "a=b", "Authorization": "a", **safe_headers}, + ) + + internal_response = Response( + "https://example.com", + headers={"Location": "https://example.com/a"}, + status=301, + ) + internal_redirect_request = self.mw.process_response( + original_request, internal_response, self.spider + ) + self.assertIsInstance(internal_redirect_request, Request) + self.assertEqual(original_request.headers, internal_redirect_request.headers) + + external_response = Response( + "https://example.com", + headers={"Location": "https://example.org/a"}, + status=301, + ) + external_redirect_request = self.mw.process_response( + original_request, external_response, self.spider + ) + self.assertIsInstance(external_redirect_request, Request) + self.assertEqual( + safe_headers, external_redirect_request.headers.to_unicode_dict() + ) + class MetaRefreshMiddlewareTest(unittest.TestCase): def setUp(self): From 75e99c75b3c6219df50546877e02f7bbb37324c3 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 15 Nov 2023 19:51:04 +0400 Subject: [PATCH 05/60] Improve the docs about Crawler attributes and settings initialization. --- docs/news.rst | 6 ++++-- docs/topics/spiders.rst | 10 ++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index fd8fa3ea3..0c202639e 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -32,8 +32,10 @@ Backward-incompatible changes :meth:`scrapy.crawler.Crawler.__init__` and before the settings are finalized and frozen. This change was needed to allow changing the settings in :meth:`scrapy.Spider.from_crawler`. If you want to access the final - setting values in the spider code as early as possible you can do this in - :meth:`~scrapy.Spider.start_requests`. (:issue:`6038`) + setting values and the initialized :class:`~scrapy.crawler.Crawler` + attributes in the spider code as early as possible you can do this in + :meth:`~scrapy.Spider.start_requests` or in a handler of the + :signal:`engine_started` signal. (:issue:`6038`) - The :meth:`TextResponse.json ` method now requires the response to be in a valid JSON encoding (UTF-8, UTF-16, or diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 20452d558..30677fe74 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -142,8 +142,14 @@ scrapy.Spider method, which is handy if you want to modify them based on arguments. As a consequence, these settings aren't the final values as they can be modified later by e.g. :ref:`add-ons - `. The final settings are available in the - :meth:`start_requests` method and later. + `. For the same reason, most of the + :class:`~scrapy.crawler.Crawler` attributes aren't initialized at + this point. + + The final settings and the initialized + :class:`~scrapy.crawler.Crawler` attributes are available in the + :meth:`start_requests` method, handlers of the + :signal:`engine_started` signal and later. :param crawler: crawler to which the spider will be bound :type crawler: :class:`~scrapy.crawler.Crawler` instance From ffbf943e9d0fed636174fab34b2d957b95ee8800 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 2 Oct 2023 20:40:25 +0400 Subject: [PATCH 06/60] Merge pull request #6077 from 11-aryan/11-aryan --- docs/topics/request-response.rst | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 41df51589..adf3d0f4a 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -115,20 +115,9 @@ Request objects cookies for that domain and will be sent again in future requests. That's the typical behaviour of any regular web browser. - To create a request that does not send stored cookies and does not - store received cookies, set the ``dont_merge_cookies`` key to ``True`` - in :attr:`request.meta `. - - Example of a request that sends manually-defined cookies and ignores - cookie storage: - - .. code-block:: python - - Request( - url="http://www.example.com", - cookies={"currency": "USD", "country": "UY"}, - meta={"dont_merge_cookies": True}, - ) + Note that setting the :reqmeta:`dont_merge_cookies` key to ``True`` in + :attr:`request.meta ` causes custom cookies to be + ignored. For more info see :ref:`cookies-mw`. From 59cfdeaa5c83ca1e65be7220296366c135b7676c Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 3 Oct 2023 20:20:12 +0400 Subject: [PATCH 07/60] Merge pull request #6083 from wRAR/py3.12-release Adapt to the Python 3.12 final release --- .github/workflows/tests-macos.yml | 2 +- .github/workflows/tests-ubuntu.yml | 20 +++++++++----------- .github/workflows/tests-windows.yml | 8 ++++---- tests/requirements.txt | 6 ++---- tests/test_downloader_handlers.py | 2 +- tests/test_feedexport.py | 3 --- tests/test_pipeline_files.py | 5 ----- 7 files changed, 17 insertions(+), 29 deletions(-) diff --git a/.github/workflows/tests-macos.yml b/.github/workflows/tests-macos.yml index 3044a1af3..aa9b3851d 100644 --- a/.github/workflows/tests-macos.yml +++ b/.github/workflows/tests-macos.yml @@ -7,7 +7,7 @@ jobs: strategy: fail-fast: false matrix: - python-version: ["3.8", "3.9", "3.10", "3.11"] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] steps: - uses: actions/checkout@v3 diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index 5ff92a571..62b5f123a 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -17,7 +17,10 @@ jobs: - python-version: "3.11" env: TOXENV: py - - python-version: "3.11" + - python-version: "3.12" + env: + TOXENV: py + - python-version: "3.12" env: TOXENV: asyncio - python-version: pypy3.9 @@ -41,22 +44,17 @@ jobs: env: TOXENV: botocore-pinned - - python-version: "3.11" + - python-version: "3.12" env: TOXENV: extra-deps - - python-version: "3.11" + - python-version: "3.12" env: TOXENV: botocore - - python-version: "3.12.0-rc.2" - env: - TOXENV: py - - python-version: "3.12.0-rc.2" + # keep until uvloop supports 3.12 + - python-version: "3.11" env: TOXENV: asyncio - - python-version: "3.12.0-rc.2" - env: - TOXENV: extra-deps steps: - uses: actions/checkout@v3 @@ -67,7 +65,7 @@ jobs: python-version: ${{ matrix.python-version }} - name: Install system libraries - if: matrix.python-version == 'pypy3.9' || contains(matrix.env.TOXENV, 'pinned') || contains(matrix.python-version, '3.12.0') + if: matrix.python-version == 'pypy3.9' || contains(matrix.env.TOXENV, 'pinned') run: | sudo apt-get update sudo apt-get install libxml2-dev libxslt-dev diff --git a/.github/workflows/tests-windows.yml b/.github/workflows/tests-windows.yml index c8d1928d7..48e0bea76 100644 --- a/.github/workflows/tests-windows.yml +++ b/.github/workflows/tests-windows.yml @@ -17,13 +17,13 @@ jobs: - python-version: "3.10" env: TOXENV: py - - python-version: "3.10" - env: - TOXENV: asyncio - python-version: "3.11" env: TOXENV: py - - python-version: "3.11" + - python-version: "3.12" + env: + TOXENV: py + - python-version: "3.12" env: TOXENV: asyncio diff --git a/tests/requirements.txt b/tests/requirements.txt index 3ea7f3333..c07fda2d6 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,7 +1,6 @@ # Tests requirements attrs -# https://github.com/giampaolo/pyftpdlib/issues/560 -pyftpdlib; python_version < "3.12" +pyftpdlib >= 1.5.8 pytest pytest-cov==4.0.0 pytest-xdist @@ -10,8 +9,7 @@ testfixtures # uvloop currently doesn't build on 3.12 uvloop; platform_system != "Windows" and python_version < "3.12" -# bpython requires greenlet which currently doesn't build on 3.12 -bpython; python_version < "3.12" # optional for shell wrapper tests +bpython # optional for shell wrapper tests brotli; implementation_name != 'pypy' # optional for HTTP compress downloader middleware tests # 1.1.0 is broken on PyPy: https://github.com/google/brotli/issues/1072 brotli==1.0.9; implementation_name == 'pypy' # optional for HTTP compress downloader middleware tests diff --git a/tests/test_downloader_handlers.py b/tests/test_downloader_handlers.py index 57211d97a..f12243e1d 100644 --- a/tests/test_downloader_handlers.py +++ b/tests/test_downloader_handlers.py @@ -127,7 +127,7 @@ class FileTestCase(unittest.TestCase): return self.download_request(request, Spider("foo")).addCallback(_test) def test_non_existent(self): - request = Request(f"file://{self.mktemp()}") + request = Request(path_to_file_uri(self.mktemp())) d = self.download_request(request, Spider("foo")) return self.assertFailure(d, OSError) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 6b82974fa..56967c0d5 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -125,9 +125,6 @@ class FileFeedStorageTest(unittest.TestCase): path.unlink() -@pytest.mark.skipif( - sys.version_info >= (3, 12), reason="pyftpdlib doesn't support Python 3.12 yet" -) class FTPFeedStorageTest(unittest.TestCase): def get_test_spider(self, settings=None): class TestSpider(scrapy.Spider): diff --git a/tests/test_pipeline_files.py b/tests/test_pipeline_files.py index bf96f17b6..468751446 100644 --- a/tests/test_pipeline_files.py +++ b/tests/test_pipeline_files.py @@ -1,7 +1,6 @@ import dataclasses import os import random -import sys import time from datetime import datetime from io import BytesIO @@ -12,7 +11,6 @@ from unittest import mock from urllib.parse import urlparse import attr -import pytest from itemadapter import ItemAdapter from twisted.internet import defer from twisted.trial import unittest @@ -648,9 +646,6 @@ class TestGCSFilesStore(unittest.TestCase): store.bucket.get_blob.assert_called_with(expected_blob_path) -@pytest.mark.skipif( - sys.version_info >= (3, 12), reason="pyftpdlib doesn't support Python 3.12 yet" -) class TestFTPFileStore(unittest.TestCase): @defer.inlineCallbacks def test_persist(self): From 538192916f496eb21846d797a6feff5c05f501cf Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 17 Oct 2023 17:08:23 +0400 Subject: [PATCH 08/60] Merge pull request #6064 from wRAR/signals-proper Refactor installing signals. --- scrapy/crawler.py | 12 +++++++----- scrapy/utils/ossignal.py | 9 +++------ scrapy/utils/testproc.py | 7 ++++--- setup.py | 3 +-- tests/CrawlerProcess/sleeping.py | 24 +++++++++++++++++++++++ tests/requirements.txt | 1 + tests/test_command_shell.py | 26 +++++++++++++++++++++++++ tests/test_crawler.py | 33 ++++++++++++++++++++++++++++++-- 8 files changed, 97 insertions(+), 18 deletions(-) create mode 100644 tests/CrawlerProcess/sleeping.py diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 22fd65be7..6f54e62e9 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -404,8 +404,8 @@ class CrawlerProcess(CrawlerRunner): :param bool stop_after_crawl: stop or not the reactor when all crawlers have finished - :param bool install_signal_handlers: whether to install the shutdown - handlers (default: True) + :param bool install_signal_handlers: whether to install the OS signal + handlers from Twisted and Scrapy (default: True) """ from twisted.internet import reactor @@ -416,15 +416,17 @@ class CrawlerProcess(CrawlerRunner): return d.addBoth(self._stop_reactor) - if install_signal_handlers: - install_shutdown_handlers(self._signal_shutdown) resolver_class = load_object(self.settings["DNS_RESOLVER"]) resolver = create_instance(resolver_class, self.settings, self, reactor=reactor) resolver.install_on_reactor() tp = reactor.getThreadPool() tp.adjustPoolsize(maxthreads=self.settings.getint("REACTOR_THREADPOOL_MAXSIZE")) reactor.addSystemEventTrigger("before", "shutdown", self.stop) - reactor.run(installSignalHandlers=False) # blocking call + if install_signal_handlers: + reactor.addSystemEventTrigger( + "after", "startup", install_shutdown_handlers, self._signal_shutdown + ) + reactor.run(installSignalHandlers=install_signal_handlers) # blocking call def _graceful_stop_reactor(self) -> Deferred: d = self.stop() diff --git a/scrapy/utils/ossignal.py b/scrapy/utils/ossignal.py index 2334ea792..db9a71273 100644 --- a/scrapy/utils/ossignal.py +++ b/scrapy/utils/ossignal.py @@ -19,13 +19,10 @@ def install_shutdown_handlers( function: SignalHandlerT, override_sigint: bool = True ) -> None: """Install the given function as a signal handler for all common shutdown - signals (such as SIGINT, SIGTERM, etc). If override_sigint is ``False`` the - SIGINT handler won't be install if there is already a handler in place - (e.g. Pdb) + signals (such as SIGINT, SIGTERM, etc). If ``override_sigint`` is ``False`` the + SIGINT handler won't be installed if there is already a handler in place + (e.g. Pdb) """ - from twisted.internet import reactor - - reactor._handleSignals() signal.signal(signal.SIGTERM, function) if signal.getsignal(signal.SIGINT) == signal.default_int_handler or override_sigint: signal.signal(signal.SIGINT, function) diff --git a/scrapy/utils/testproc.py b/scrapy/utils/testproc.py index 5f7a7db14..0688e014b 100644 --- a/scrapy/utils/testproc.py +++ b/scrapy/utils/testproc.py @@ -2,7 +2,7 @@ from __future__ import annotations import os import sys -from typing import Iterable, Optional, Tuple, cast +from typing import Iterable, List, Optional, Tuple, cast from twisted.internet.defer import Deferred from twisted.internet.error import ProcessTerminated @@ -26,14 +26,15 @@ class ProcessTest: env = os.environ.copy() if settings is not None: env["SCRAPY_SETTINGS_MODULE"] = settings + assert self.command cmd = self.prefix + [self.command] + list(args) pp = TestProcessProtocol() - pp.deferred.addBoth(self._process_finished, cmd, check_code) + pp.deferred.addCallback(self._process_finished, cmd, check_code) reactor.spawnProcess(pp, cmd[0], cmd, env=env, path=self.cwd) return pp.deferred def _process_finished( - self, pp: TestProcessProtocol, cmd: str, check_code: bool + self, pp: TestProcessProtocol, cmd: List[str], check_code: bool ) -> Tuple[int, bytes, bytes]: if pp.exitcode and check_code: msg = f"process {cmd} exit with code {pp.exitcode}" diff --git a/setup.py b/setup.py index 47c0af0b0..405633f55 100644 --- a/setup.py +++ b/setup.py @@ -6,8 +6,7 @@ version = (Path(__file__).parent / "scrapy/VERSION").read_text("ascii").strip() install_requires = [ - # 23.8.0 incompatibility: https://github.com/scrapy/scrapy/issues/6024 - "Twisted>=18.9.0,<23.8.0", + "Twisted>=18.9.0", "cryptography>=36.0.0", "cssselect>=0.9.1", "itemloaders>=1.0.1", diff --git a/tests/CrawlerProcess/sleeping.py b/tests/CrawlerProcess/sleeping.py new file mode 100644 index 000000000..420d9d328 --- /dev/null +++ b/tests/CrawlerProcess/sleeping.py @@ -0,0 +1,24 @@ +from twisted.internet.defer import Deferred + +import scrapy +from scrapy.crawler import CrawlerProcess +from scrapy.utils.defer import maybe_deferred_to_future + + +class SleepingSpider(scrapy.Spider): + name = "sleeping" + + start_urls = ["data:,;"] + + async def parse(self, response): + from twisted.internet import reactor + + d = Deferred() + reactor.callLater(3, d.callback, None) + await maybe_deferred_to_future(d) + + +process = CrawlerProcess(settings={}) + +process.crawl(SleepingSpider) +process.start() diff --git a/tests/requirements.txt b/tests/requirements.txt index c07fda2d6..d4bfead40 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -1,5 +1,6 @@ # Tests requirements attrs +pexpect >= 4.8.0 pyftpdlib >= 1.5.8 pytest pytest-cov==4.0.0 diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 6589381f3..7d87eb62c 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -1,11 +1,15 @@ +import sys +from io import BytesIO from pathlib import Path +from pexpect.popen_spawn import PopenSpawn from twisted.internet import defer from twisted.trial import unittest from scrapy.utils.testproc import ProcessTest from scrapy.utils.testsite import SiteTest from tests import NON_EXISTING_RESOLVABLE, tests_datadir +from tests.mockserver import MockServer class ShellTest(ProcessTest, SiteTest, unittest.TestCase): @@ -133,3 +137,25 @@ class ShellTest(ProcessTest, SiteTest, unittest.TestCase): args = ["-c", code, "--set", f"TWISTED_REACTOR={reactor_path}"] _, _, err = yield self.execute(args, check_code=True) self.assertNotIn(b"RuntimeError: There is no current event loop in thread", err) + + +class InteractiveShellTest(unittest.TestCase): + def test_fetch(self): + args = ( + sys.executable, + "-m", + "scrapy.cmdline", + "shell", + ) + logfile = BytesIO() + p = PopenSpawn(args, timeout=5) + p.logfile_read = logfile + p.expect_exact("Available Scrapy objects") + with MockServer() as mockserver: + p.sendline(f"fetch('{mockserver.url('/')}')") + p.sendline("type(response)") + p.expect_exact("HtmlResponse") + p.sendeof() + p.wait() + logfile.seek(0) + self.assertNotIn("Traceback", logfile.read().decode()) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 2b141e894..60b92377d 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -1,13 +1,16 @@ import logging import os import platform +import signal import subprocess import sys import warnings from pathlib import Path +from typing import List import pytest from packaging.version import parse as parse_version +from pexpect.popen_spawn import PopenSpawn from pytest import mark, raises from twisted.internet import defer from twisted.trial import unittest @@ -289,9 +292,12 @@ class ScriptRunnerMixin: script_dir: Path cwd = os.getcwd() - def run_script(self, script_name: str, *script_args): + def get_script_args(self, script_name: str, *script_args: str) -> List[str]: script_path = self.script_dir / script_name - args = [sys.executable, str(script_path)] + list(script_args) + return [sys.executable, str(script_path)] + list(script_args) + + def run_script(self, script_name: str, *script_args: str) -> str: + args = self.get_script_args(script_name, *script_args) p = subprocess.Popen( args, env=get_mockserver_env(), @@ -517,6 +523,29 @@ class CrawlerProcessSubprocess(ScriptRunnerMixin, unittest.TestCase): self.assertIn("Spider closed (finished)", log) self.assertIn("The value of FOO is 42", log) + def test_shutdown_graceful(self): + sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK + args = self.get_script_args("sleeping.py") + p = PopenSpawn(args, timeout=5) + p.expect_exact("Spider opened") + p.expect_exact("Crawled (200)") + p.kill(sig) + p.expect_exact("shutting down gracefully") + p.expect_exact("Spider closed (shutdown)") + p.wait() + + def test_shutdown_forced(self): + sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK + args = self.get_script_args("sleeping.py") + p = PopenSpawn(args, timeout=5) + p.expect_exact("Spider opened") + p.expect_exact("Crawled (200)") + p.kill(sig) + p.expect_exact("shutting down gracefully") + p.kill(sig) + p.expect_exact("forcing unclean shutdown") + p.wait() + class CrawlerRunnerSubprocess(ScriptRunnerMixin, unittest.TestCase): script_dir = Path(__file__).parent.resolve() / "CrawlerRunner" From 5e4fb0bc5fc066136b171ce488599b1ddd64c83a Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 17 Oct 2023 21:24:44 +0400 Subject: [PATCH 09/60] Re-enable uvloop tests on 3.12 (#6098) --- .github/workflows/checks.yml | 4 ++-- .github/workflows/publish.yml | 2 +- .github/workflows/tests-ubuntu.yml | 5 ----- scrapy/contracts/__init__.py | 6 ++++-- tests/requirements.txt | 3 +-- tox.ini | 2 +- 6 files changed, 9 insertions(+), 13 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index ee0cb4b1e..f91055ba5 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -8,7 +8,7 @@ jobs: fail-fast: false matrix: include: - - python-version: "3.11" + - python-version: "3.12" env: TOXENV: pylint - python-version: 3.8 @@ -17,7 +17,7 @@ jobs: - python-version: "3.11" # Keep in sync with .readthedocs.yml env: TOXENV: docs - - python-version: "3.11" + - python-version: "3.12" env: TOXENV: twinecheck diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 22b8996b6..095793299 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -11,7 +11,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: 3.11 + python-version: 3.12 - run: | pip install --upgrade build twine python -m build diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index 62b5f123a..c883f958c 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -51,11 +51,6 @@ jobs: env: TOXENV: botocore - # keep until uvloop supports 3.12 - - python-version: "3.11" - env: - TOXENV: asyncio - steps: - uses: actions/checkout@v3 diff --git a/scrapy/contracts/__init__.py b/scrapy/contracts/__init__.py index 1ec2a0234..2d9ddd89a 100644 --- a/scrapy/contracts/__init__.py +++ b/scrapy/contracts/__init__.py @@ -41,7 +41,9 @@ class Contract: cb_result = cb(response, **cb_kwargs) if isinstance(cb_result, (AsyncGenerator, CoroutineType)): raise TypeError("Contracts don't support async callbacks") - return list(iterate_spider_output(cb_result)) + return list( # pylint: disable=return-in-finally + iterate_spider_output(cb_result) + ) request.callback = wrapper @@ -68,7 +70,7 @@ class Contract: else: results.addSuccess(self.testcase_post) finally: - return output + return output # pylint: disable=return-in-finally request.callback = wrapper diff --git a/tests/requirements.txt b/tests/requirements.txt index d4bfead40..5b75674f5 100644 --- a/tests/requirements.txt +++ b/tests/requirements.txt @@ -7,8 +7,7 @@ pytest-cov==4.0.0 pytest-xdist sybil >= 1.3.0 # https://github.com/cjw296/sybil/issues/20#issuecomment-605433422 testfixtures -# uvloop currently doesn't build on 3.12 -uvloop; platform_system != "Windows" and python_version < "3.12" +uvloop; platform_system != "Windows" bpython # optional for shell wrapper tests brotli; implementation_name != 'pypy' # optional for HTTP compress downloader middleware tests diff --git a/tox.ini b/tox.ini index 9c2522a43..381da9773 100644 --- a/tox.ini +++ b/tox.ini @@ -57,7 +57,7 @@ commands = basepython = python3 deps = {[testenv:extra-deps]deps} - pylint==2.17.5 + pylint==3.0.1 commands = pylint conftest.py docs extras scrapy setup.py tests From 1045856a50d379d145e514ec9c7aeeed231aefd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 30 Oct 2023 09:35:29 +0100 Subject: [PATCH 10/60] Merge pull request #6112 from wRAR/test-shutdown-forced Make shutdown tests more robust. --- tests/CrawlerProcess/sleeping.py | 2 +- tests/test_command_shell.py | 5 ++++- tests/test_crawler.py | 11 +++++++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/CrawlerProcess/sleeping.py b/tests/CrawlerProcess/sleeping.py index 420d9d328..45479ea4f 100644 --- a/tests/CrawlerProcess/sleeping.py +++ b/tests/CrawlerProcess/sleeping.py @@ -14,7 +14,7 @@ class SleepingSpider(scrapy.Spider): from twisted.internet import reactor d = Deferred() - reactor.callLater(3, d.callback, None) + reactor.callLater(int(self.sleep), d.callback, None) await maybe_deferred_to_future(d) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 7d87eb62c..7918d94b2 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -1,3 +1,4 @@ +import os import sys from io import BytesIO from pathlib import Path @@ -147,8 +148,10 @@ class InteractiveShellTest(unittest.TestCase): "scrapy.cmdline", "shell", ) + env = os.environ.copy() + env["SCRAPY_PYTHON_SHELL"] = "python" logfile = BytesIO() - p = PopenSpawn(args, timeout=5) + p = PopenSpawn(args, env=env, timeout=5) p.logfile_read = logfile p.expect_exact("Available Scrapy objects") with MockServer() as mockserver: diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 60b92377d..0a7f9bac8 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -525,7 +525,7 @@ class CrawlerProcessSubprocess(ScriptRunnerMixin, unittest.TestCase): def test_shutdown_graceful(self): sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK - args = self.get_script_args("sleeping.py") + args = self.get_script_args("sleeping.py", "-a", "sleep=3") p = PopenSpawn(args, timeout=5) p.expect_exact("Spider opened") p.expect_exact("Crawled (200)") @@ -534,14 +534,21 @@ class CrawlerProcessSubprocess(ScriptRunnerMixin, unittest.TestCase): p.expect_exact("Spider closed (shutdown)") p.wait() + @defer.inlineCallbacks def test_shutdown_forced(self): + from twisted.internet import reactor + sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK - args = self.get_script_args("sleeping.py") + args = self.get_script_args("sleeping.py", "-a", "sleep=10") p = PopenSpawn(args, timeout=5) p.expect_exact("Spider opened") p.expect_exact("Crawled (200)") p.kill(sig) p.expect_exact("shutting down gracefully") + # sending the second signal too fast often causes problems + d = defer.Deferred() + reactor.callLater(0.1, d.callback, None) + yield d p.kill(sig) p.expect_exact("forcing unclean shutdown") p.wait() From 150f9d6d888970d5f164387761989aba59e830c0 Mon Sep 17 00:00:00 2001 From: Jessica Allman-LaPorte Date: Fri, 3 Nov 2023 05:02:18 -0400 Subject: [PATCH 11/60] Make shell switching more clear in the tutorial (#6128) --- docs/intro/tutorial.rst | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/intro/tutorial.rst b/docs/intro/tutorial.rst index 19a76fc16..8ea98f29b 100644 --- a/docs/intro/tutorial.rst +++ b/docs/intro/tutorial.rst @@ -493,7 +493,15 @@ in the callback, as you can see below: "tags": quote.css("div.tags a.tag::text").getall(), } -If you run this spider, it will output the extracted data with the log:: +To run this spider, exit the scrapy shell by entering:: + + quit() + +Then, run:: + + scrapy crawl quotes + +Now, it should output the extracted data with the log:: 2016-09-19 18:57:19 [scrapy.core.scraper] DEBUG: Scraped from <200 https://quotes.toscrape.com/page/1/> {'tags': ['life', 'love'], 'author': 'André Gide', 'text': '“It is better to be hated for what you are than to be loved for what you are not.”'} From 49b284ab8508d3400582781343ea8171980b1e70 Mon Sep 17 00:00:00 2001 From: Kiran <75929997+Kiran1689@users.noreply.github.com> Date: Tue, 14 Nov 2023 00:43:10 +0530 Subject: [PATCH 12/60] Updated README.rst (#6144) --- README.rst | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/README.rst b/README.rst index 1918850d6..14adff648 100644 --- a/README.rst +++ b/README.rst @@ -17,9 +17,10 @@ Scrapy :target: https://github.com/scrapy/scrapy/actions?query=workflow%3AUbuntu :alt: Ubuntu -.. image:: https://github.com/scrapy/scrapy/workflows/macOS/badge.svg - :target: https://github.com/scrapy/scrapy/actions?query=workflow%3AmacOS - :alt: macOS +.. .. image:: https://github.com/scrapy/scrapy/workflows/macOS/badge.svg + .. :target: https://github.com/scrapy/scrapy/actions?query=workflow%3AmacOS + .. :alt: macOS + .. image:: https://github.com/scrapy/scrapy/workflows/Windows/badge.svg :target: https://github.com/scrapy/scrapy/actions?query=workflow%3AWindows @@ -41,7 +42,7 @@ Scrapy Overview ======== -Scrapy is a fast high-level web crawling and web scraping framework, used to +Scrapy is a BSD-licensed fast high-level web crawling and web scraping framework, used to crawl websites and extract structured data from their pages. It can be used for a wide range of purposes, from data mining to monitoring and automated testing. @@ -110,4 +111,4 @@ See https://scrapy.org/companies/ for a list. Commercial Support ================== -See https://scrapy.org/support/ for details. +See https://scrapy.org/support/ for details. \ No newline at end of file From 6969041c5f6891a0298d7e68ece762adee1bb222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 15:52:00 +0100 Subject: [PATCH 13/60] Protect against gzip bombs --- .../downloadermiddlewares/httpcompression.py | 26 +++++++++++----- scrapy/utils/_compression.py | 2 ++ scrapy/utils/gz.py | 14 +++++++-- tests/sample_data/compressed/bomb-gzip.bin | Bin 0 -> 27988 bytes ...st_downloadermiddleware_httpcompression.py | 29 ++++++++++++++++-- 5 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 scrapy/utils/_compression.py create mode 100644 tests/sample_data/compressed/bomb-gzip.bin diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index ead426951..5dd67ea87 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -2,9 +2,10 @@ import io import warnings import zlib -from scrapy.exceptions import NotConfigured +from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes +from scrapy.utils._compression import _DecompressionMaxSizeExceeded from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip @@ -29,24 +30,26 @@ class HttpCompressionMiddleware: """This middleware allows compressed (gzip, deflate) traffic to be sent/received from web sites""" - def __init__(self, stats=None): - self.stats = stats + def __init__(self, crawler=None): + self.stats = crawler.stats + self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") @classmethod def from_crawler(cls, crawler): if not crawler.settings.getbool("COMPRESSION_ENABLED"): raise NotConfigured try: - return cls(stats=crawler.stats) + return cls(crawler=crawler) except TypeError: warnings.warn( "HttpCompressionMiddleware subclasses must either modify " - "their '__init__' method to support a 'stats' parameter or " - "reimplement the 'from_crawler' method.", + "their '__init__' method to support a 'crawler' parameter or " + "reimplement their 'from_crawler' method.", ScrapyDeprecationWarning, ) result = cls() result.stats = crawler.stats + result._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") return result def process_request(self, request, spider): @@ -59,7 +62,14 @@ class HttpCompressionMiddleware: content_encoding = response.headers.getlist("Content-Encoding") if content_encoding: encoding = content_encoding.pop() - decoded_body = self._decode(response.body, encoding.lower()) + try: + decoded_body = self._decode(response.body, encoding.lower()) + except _DecompressionMaxSizeExceeded: + raise IgnoreRequest( + f"Ignored response {response} because its body " + f"({len(response.body)}B) exceeded DOWNLOAD_MAXSIZE " + f"({self._max_size}B) during decompression." + ) if self.stats: self.stats.inc_value( "httpcompression/response_bytes", @@ -85,7 +95,7 @@ class HttpCompressionMiddleware: def _decode(self, body, encoding): if encoding == b"gzip" or encoding == b"x-gzip": - body = gunzip(body) + body = gunzip(body, max_size=self._max_size) if encoding == b"deflate": try: diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py new file mode 100644 index 000000000..e726a70f5 --- /dev/null +++ b/scrapy/utils/_compression.py @@ -0,0 +1,2 @@ +class _DecompressionMaxSizeExceeded(ValueError): + pass diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index c7f74030e..cd5059a5c 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -5,8 +5,10 @@ from typing import List from scrapy.http import Response +from ._compression import _DecompressionMaxSizeExceeded -def gunzip(data: bytes) -> bytes: + +def gunzip(data: bytes, max_size: int = 0) -> bytes: """Gunzip the given data and return as much data as possible. This is resilient to CRC checksum errors. @@ -14,10 +16,10 @@ def gunzip(data: bytes) -> bytes: f = GzipFile(fileobj=BytesIO(data)) output_list: List[bytes] = [] chunk = b"." + decompressed_size = 0 while chunk: try: chunk = f.read1(8196) - output_list.append(chunk) except (OSError, EOFError, struct.error): # complete only if there is some data, otherwise re-raise # see issue 87 about catching struct.error @@ -25,6 +27,14 @@ def gunzip(data: bytes) -> bytes: if output_list: break raise + decompressed_size += len(chunk) + if max_size and decompressed_size > max_size: + raise _DecompressionMaxSizeExceeded( + f"The number of bytes decompressed so far " + f"({decompressed_size}B) exceed the specified maximum " + f"({max_size}B)." + ) + output_list.append(chunk) return b"".join(output_list) diff --git a/tests/sample_data/compressed/bomb-gzip.bin b/tests/sample_data/compressed/bomb-gzip.bin new file mode 100644 index 0000000000000000000000000000000000000000..64aa0c3696cc1c6d86d218c70635d88f2035ec9e GIT binary patch literal 27988 zcmeIyElY!86b9f&oiK|G(Y#;~27Xl0-yq1UE0YN_<|{r6TM$G+ga1HfWkC=@i}}T- zAQQ0;tA;J=f*|@M2A1oDJDzYj_mw}*X4m_rN*LQrYP)-t7`Kz1`EpV#FVq|L(0ja} zI9SSs+qBrtti6t3Pxsob#`n?W)Wc%`Y$l!UY&@@AZN0t3&;4p=`TZgaH}D5)fC3Vd zkc1>8Aqh!HLK2dYgd`*(2}wvo5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8Aqh!H zLK2dYgd`*(2}wvo5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8Aqh!HLK2d2h!PI& z=1wx8 S;eSfl{TO{ZZ^qTjoA3)j<4WiN literal 0 HcmV?d00001 diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 9dad056de..f834e78f5 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -10,7 +10,7 @@ from scrapy.downloadermiddlewares.httpcompression import ( ACCEPTED_ENCODINGS, HttpCompressionMiddleware, ) -from scrapy.exceptions import NotConfigured, ScrapyDeprecationWarning +from scrapy.exceptions import IgnoreRequest, NotConfigured, ScrapyDeprecationWarning from scrapy.http import HtmlResponse, Request, Response from scrapy.responsetypes import responsetypes from scrapy.spiders import Spider @@ -35,12 +35,24 @@ FORMAT = { "html-zstd-streaming-no-content-size.bin", "zstd", ), + **{ + f"bomb-{format_id}": (f"bomb-{format_id}.bin", format_id) + for format_id in ( + # "br", + "gzip", # 27 988 → 11 511 612 + # "deflate", + # "zstd", + ) + }, } class HttpCompressionTest(TestCase): def setUp(self): - self.crawler = get_crawler(Spider) + settings = { + "DOWNLOAD_MAXSIZE": 10_000_000, # For compression bomb tests. + } + self.crawler = get_crawler(Spider, settings_dict=settings) self.spider = self.crawler._create_spider("scrapytest.org") self.mw = HttpCompressionMiddleware.from_crawler(self.crawler) self.crawler.stats.open_spider(self.spider) @@ -373,6 +385,19 @@ class HttpCompressionTest(TestCase): self.assertStatsEqual("httpcompression/response_count", None) self.assertStatsEqual("httpcompression/response_bytes", None) + def _test_compression_bomb(self, compression_id): + response = self._getresponse(f"bomb-{compression_id}") + self.assertRaises( + IgnoreRequest, + self.mw.process_response, + response.request, + response, + self.spider, + ) + + def test_compression_bomb_gzip(self): + self._test_compression_bomb("gzip") + class HttpCompressionSubclassTest(TestCase): def test_init_missing_stats(self): From 0bf29a7b1b9b6a641c486780b7b0fa455577bf39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 16:10:50 +0100 Subject: [PATCH 14/60] Update test expectations --- scrapy/downloadermiddlewares/httpcompression.py | 4 +++- .../test_downloadermiddleware_httpcompression.py | 16 ++-------------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 5dd67ea87..0fec05a14 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -30,7 +30,9 @@ class HttpCompressionMiddleware: """This middleware allows compressed (gzip, deflate) traffic to be sent/received from web sites""" - def __init__(self, crawler=None): + def __init__(self, *, crawler=None): + if not crawler: + return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index f834e78f5..f5dedd28d 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -127,18 +127,6 @@ class HttpCompressionTest(TestCase): self.assertStatsEqual("httpcompression/response_count", 1) self.assertStatsEqual("httpcompression/response_bytes", 74837) - def test_process_response_gzip_no_stats(self): - mw = HttpCompressionMiddleware() - response = self._getresponse("gzip") - request = response.request - - self.assertEqual(response.headers["Content-Encoding"], b"gzip") - newresponse = mw.process_response(request, response, self.spider) - self.assertEqual(mw.stats, None) - assert newresponse is not response - assert newresponse.body.startswith(b" Date: Wed, 22 Nov 2023 17:12:43 +0100 Subject: [PATCH 15/60] Protect against deflate bombs --- .../downloadermiddlewares/httpcompression.py | 20 +++------ scrapy/utils/_compression.py | 39 ++++++++++++++++++ scrapy/utils/gz.py | 2 +- tests/sample_data/compressed/bomb-deflate.bin | Bin 0 -> 27968 bytes ...st_downloadermiddleware_httpcompression.py | 5 ++- 5 files changed, 49 insertions(+), 17 deletions(-) create mode 100644 tests/sample_data/compressed/bomb-deflate.bin diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 0fec05a14..8cec87c47 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -1,11 +1,10 @@ import io import warnings -import zlib from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes -from scrapy.utils._compression import _DecompressionMaxSizeExceeded +from scrapy.utils._compression import _DecompressionMaxSizeExceeded, _inflate from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip @@ -97,23 +96,14 @@ class HttpCompressionMiddleware: def _decode(self, body, encoding): if encoding == b"gzip" or encoding == b"x-gzip": - body = gunzip(body, max_size=self._max_size) - + return gunzip(body, max_size=self._max_size) if encoding == b"deflate": - try: - body = zlib.decompress(body) - except zlib.error: - # ugly hack to work with raw deflate content that may - # be sent by microsoft servers. For more information, see: - # http://carsten.codimi.de/gzip.yaws/ - # http://www.port80software.com/200ok/archive/2005/10/31/868.aspx - # http://www.gzip.org/zlib/zlib_faq.html#faq38 - body = zlib.decompress(body, -15) + return _inflate(body, max_size=self._max_size) if encoding == b"br" and b"br" in ACCEPTED_ENCODINGS: - body = brotli.decompress(body) + return brotli.decompress(body) if encoding == b"zstd" and b"zstd" in ACCEPTED_ENCODINGS: # Using its streaming API since its simple API could handle only cases # where there is content size data embedded in the frame reader = zstandard.ZstdDecompressor().stream_reader(io.BytesIO(body)) - body = reader.read() + return reader.read() return body diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index e726a70f5..34bf2e4f7 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -1,2 +1,41 @@ +import zlib +from io import BytesIO +from typing import List + + class _DecompressionMaxSizeExceeded(ValueError): pass + + +def _inflate(data: bytes, *, max_size: int = 0) -> bytes: + decompressor = zlib.decompressobj() + raw_decompressor = zlib.decompressobj(wbits=-15) + input_stream = BytesIO(data) + output_list: List[bytes] = [] + output_chunk = b"." + decompressed_size = 0 + CHUNK_SIZE = 8196 + while output_chunk: + input_chunk = input_stream.read(CHUNK_SIZE) + try: + output_chunk = decompressor.decompress(input_chunk) + except zlib.error: + if decompressor != raw_decompressor: + # ugly hack to work with raw deflate content that may + # be sent by microsoft servers. For more information, see: + # http://carsten.codimi.de/gzip.yaws/ + # http://www.port80software.com/200ok/archive/2005/10/31/868.aspx + # http://www.gzip.org/zlib/zlib_faq.html#faq38 + decompressor = raw_decompressor + output_chunk = decompressor.decompress(input_chunk) + else: + raise + decompressed_size += len(output_chunk) + if max_size and decompressed_size > max_size: + raise _DecompressionMaxSizeExceeded( + f"The number of bytes decompressed so far " + f"({decompressed_size}B) exceed the specified maximum " + f"({max_size}B)." + ) + output_list.append(output_chunk) + return b"".join(output_list) diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index cd5059a5c..548134721 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -8,7 +8,7 @@ from scrapy.http import Response from ._compression import _DecompressionMaxSizeExceeded -def gunzip(data: bytes, max_size: int = 0) -> bytes: +def gunzip(data: bytes, *, max_size: int = 0) -> bytes: """Gunzip the given data and return as much data as possible. This is resilient to CRC checksum errors. diff --git a/tests/sample_data/compressed/bomb-deflate.bin b/tests/sample_data/compressed/bomb-deflate.bin new file mode 100644 index 0000000000000000000000000000000000000000..3598aca0777ec2511a721e9655cabb8c21c658bd GIT binary patch literal 27968 zcmeI&u}VS#6b9f+=-?6}`2-Gb=^8GEdrPzhZ7q%$M35jzaB^}JadC@=dVsh@OD>AI zMF>rSC{CdeWcdhg3f~#dd^nu*O@FmB>%Sy!^U2^bI{%2BjpGkTvtGCQb9b0}%gx*A zC^NVm7VfVnqwxEtJUIF4gqj_=18;x=5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8 zAqh!HLK2dYgd`*(2}wvo5|VI-C0ssb8?oTOioa2%K7C$JY75N{+<`Yh0SQS+LK2dY zgd`*(2}wvo5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8Aqh#iPZGYjN(Y-T;OY9R z_8O#jIJamt;d0?};d0?}5|WUFBqSjTNk~Exl8}TXBq0e&NJ0{lkc1>8Aqh#im4waX I@bhBz2V-bE-2eap literal 0 HcmV?d00001 diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index f5dedd28d..3af8202cc 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -39,8 +39,8 @@ FORMAT = { f"bomb-{format_id}": (f"bomb-{format_id}.bin", format_id) for format_id in ( # "br", + "deflate", # 27 968 → 11 511 612 "gzip", # 27 988 → 11 511 612 - # "deflate", # "zstd", ) }, @@ -383,6 +383,9 @@ class HttpCompressionTest(TestCase): self.spider, ) + def test_compression_bomb_deflate(self): + self._test_compression_bomb("deflate") + def test_compression_bomb_gzip(self): self._test_compression_bomb("gzip") From fba167c5e1f356bcc452e95e92199f1a15135c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 17:32:09 +0100 Subject: [PATCH 16/60] Protect against brotli bombs --- .../downloadermiddlewares/httpcompression.py | 14 +++++----- scrapy/utils/_compression.py | 26 +++++++++++++++++++ tests/sample_data/compressed/bomb-br.bin | 2 ++ ...st_downloadermiddleware_httpcompression.py | 9 ++++++- 4 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 tests/sample_data/compressed/bomb-br.bin diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 8cec87c47..91748e57e 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -4,25 +4,25 @@ import warnings from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes -from scrapy.utils._compression import _DecompressionMaxSizeExceeded, _inflate +from scrapy.utils._compression import _DecompressionMaxSizeExceeded, _inflate, _unbrotli from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip ACCEPTED_ENCODINGS = [b"gzip", b"deflate"] try: - import brotli - - ACCEPTED_ENCODINGS.append(b"br") + import brotli # noqa: F401 except ImportError: pass +else: + ACCEPTED_ENCODINGS.append(b"br") try: import zstandard - - ACCEPTED_ENCODINGS.append(b"zstd") except ImportError: pass +else: + ACCEPTED_ENCODINGS.append(b"zstd") class HttpCompressionMiddleware: @@ -100,7 +100,7 @@ class HttpCompressionMiddleware: if encoding == b"deflate": return _inflate(body, max_size=self._max_size) if encoding == b"br" and b"br" in ACCEPTED_ENCODINGS: - return brotli.decompress(body) + return _unbrotli(body, max_size=self._max_size) if encoding == b"zstd" and b"zstd" in ACCEPTED_ENCODINGS: # Using its streaming API since its simple API could handle only cases # where there is content size data embedded in the frame diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 34bf2e4f7..9a32ce4f0 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -2,6 +2,11 @@ import zlib from io import BytesIO from typing import List +try: + import brotli +except ImportError: + pass + class _DecompressionMaxSizeExceeded(ValueError): pass @@ -39,3 +44,24 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: ) output_list.append(output_chunk) return b"".join(output_list) + + +def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: + decompressor = brotli.Decompressor() + input_stream = BytesIO(data) + output_list: List[bytes] = [] + output_chunk = b"." + decompressed_size = 0 + CHUNK_SIZE = 8196 + while output_chunk: + input_chunk = input_stream.read(CHUNK_SIZE) + output_chunk = decompressor.process(input_chunk) + decompressed_size += len(output_chunk) + if max_size and decompressed_size > max_size: + raise _DecompressionMaxSizeExceeded( + f"The number of bytes decompressed so far " + f"({decompressed_size}B) exceed the specified maximum " + f"({max_size}B)." + ) + output_list.append(output_chunk) + return b"".join(output_list) diff --git a/tests/sample_data/compressed/bomb-br.bin b/tests/sample_data/compressed/bomb-br.bin new file mode 100644 index 000000000..50059866f --- /dev/null +++ b/tests/sample_data/compressed/bomb-br.bin @@ -0,0 +1,2 @@ +;nުVp SmoY2 +()-д=_o \ No newline at end of file diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 3af8202cc..8858916bc 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -38,7 +38,7 @@ FORMAT = { **{ f"bomb-{format_id}": (f"bomb-{format_id}.bin", format_id) for format_id in ( - # "br", + "br", # 34 → 11 511 612 "deflate", # 27 968 → 11 511 612 "gzip", # 27 988 → 11 511 612 # "zstd", @@ -383,6 +383,13 @@ class HttpCompressionTest(TestCase): self.spider, ) + def test_compression_bomb_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_compression_bomb("br") + def test_compression_bomb_deflate(self): self._test_compression_bomb("deflate") From 9cc870387745f45f744ceef6ed226eefcce0e066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 17:53:00 +0100 Subject: [PATCH 17/60] Protect against zstandard bombs --- .../downloadermiddlewares/httpcompression.py | 15 ++++++----- scrapy/utils/_compression.py | 25 ++++++++++++++++++ tests/sample_data/compressed/bomb-zstd.bin | Bin 0 -> 1096 bytes ...st_downloadermiddleware_httpcompression.py | 5 +++- 4 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 tests/sample_data/compressed/bomb-zstd.bin diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 91748e57e..8ee1d95a6 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -1,10 +1,14 @@ -import io import warnings from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes -from scrapy.utils._compression import _DecompressionMaxSizeExceeded, _inflate, _unbrotli +from scrapy.utils._compression import ( + _DecompressionMaxSizeExceeded, + _inflate, + _unbrotli, + _unzstd, +) from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip @@ -18,7 +22,7 @@ else: ACCEPTED_ENCODINGS.append(b"br") try: - import zstandard + import zstandard # noqa: F401 except ImportError: pass else: @@ -102,8 +106,5 @@ class HttpCompressionMiddleware: if encoding == b"br" and b"br" in ACCEPTED_ENCODINGS: return _unbrotli(body, max_size=self._max_size) if encoding == b"zstd" and b"zstd" in ACCEPTED_ENCODINGS: - # Using its streaming API since its simple API could handle only cases - # where there is content size data embedded in the frame - reader = zstandard.ZstdDecompressor().stream_reader(io.BytesIO(body)) - return reader.read() + return _unzstd(body, max_size=self._max_size) return body diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 9a32ce4f0..93aa254b2 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -7,6 +7,11 @@ try: except ImportError: pass +try: + import zstandard +except ImportError: + pass + class _DecompressionMaxSizeExceeded(ValueError): pass @@ -65,3 +70,23 @@ def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: ) output_list.append(output_chunk) return b"".join(output_list) + + +def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: + decompressor = zstandard.ZstdDecompressor() + stream_reader = decompressor.stream_reader(BytesIO(data)) + output_list: List[bytes] = [] + output_chunk = b"." + decompressed_size = 0 + CHUNK_SIZE = 8196 + while output_chunk: + output_chunk = stream_reader.read(CHUNK_SIZE) + decompressed_size += len(output_chunk) + if max_size and decompressed_size > max_size: + raise _DecompressionMaxSizeExceeded( + f"The number of bytes decompressed so far " + f"({decompressed_size}B) exceed the specified maximum " + f"({max_size}B)." + ) + output_list.append(output_chunk) + return b"".join(output_list) diff --git a/tests/sample_data/compressed/bomb-zstd.bin b/tests/sample_data/compressed/bomb-zstd.bin new file mode 100644 index 0000000000000000000000000000000000000000..4b0efa8a41c88a38dffe7cea9e4a6726bdb137c4 GIT binary patch literal 1096 zcmdPcs{gko!e;q;1{Fqz2O$}m#R@=_sF0kWTTql*T%4Jor;wDNo219Z$k6h?-fpfB z0|SQwBg3EnmI6#5b|Mlx7l~br#Lh!vBdZBP5+5~lG&~1uT5@4vU|?kU`+vT_!f29* VWPM*?)(2)~i{-##pxebr9RRD(6-595 literal 0 HcmV?d00001 diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 8858916bc..7babd1318 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -41,7 +41,7 @@ FORMAT = { "br", # 34 → 11 511 612 "deflate", # 27 968 → 11 511 612 "gzip", # 27 988 → 11 511 612 - # "zstd", + "zstd", # 1 096 → 11 511 612 ) }, } @@ -396,6 +396,9 @@ class HttpCompressionTest(TestCase): def test_compression_bomb_gzip(self): self._test_compression_bomb("gzip") + def test_compression_bomb_zstd(self): + self._test_compression_bomb("zstd") + class HttpCompressionSubclassTest(TestCase): def test_init_missing_stats(self): From 3fda2fe103dafa8d4d48b21c63b2321ccec9c378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 18:34:37 +0100 Subject: [PATCH 18/60] Protect against gzip bomb sitemaps --- scrapy/spiders/sitemap.py | 8 +++++++- tests/test_spider.py | 15 +++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index aaf75a519..cc8b13cc3 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -3,6 +3,7 @@ import re from scrapy.http import Request, XmlResponse from scrapy.spiders import Spider +from scrapy.utils._compression import _DecompressionMaxSizeExceeded from scrapy.utils.gz import gunzip, gzip_magic_number from scrapy.utils.sitemap import Sitemap, sitemap_urls_from_robots @@ -71,7 +72,12 @@ class SitemapSpider(Spider): if isinstance(response, XmlResponse): return response.body if gzip_magic_number(response): - return gunzip(response.body) + try: + return gunzip( + response.body, max_size=self.settings.getint("DOWNLOAD_MAXSIZE") + ) + except _DecompressionMaxSizeExceeded: + return None # actual gzipped sitemap files are decompressed above ; # if we are here (response body is not gzipped) # and have a response for .xml.gz, diff --git a/tests/test_spider.py b/tests/test_spider.py index 00da3d485..875ff5454 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -2,6 +2,7 @@ import gzip import inspect import warnings from io import BytesIO +from pathlib import Path from typing import Any from unittest import mock @@ -25,7 +26,7 @@ from scrapy.spiders import ( ) from scrapy.spiders.init import InitSpider from scrapy.utils.test import get_crawler -from tests import get_testdata +from tests import get_testdata, tests_datadir class SpiderTest(unittest.TestCase): @@ -489,7 +490,8 @@ class SitemapSpiderTest(SpiderTest): GZBODY = f.getvalue() def assertSitemapBody(self, response, body): - spider = self.spider_class("example.com") + crawler = get_crawler() + spider = self.spider_class.from_crawler(crawler, "example.com") self.assertEqual(spider._get_sitemap_body(response), body) def test_get_sitemap_body(self): @@ -692,6 +694,15 @@ Sitemap: /sitemap-relative-url.xml ["http://www.example.com/sitemap2.xml"], ) + def test_compression_bomb(self): + settings = {"DOWNLOAD_MAXSIZE": 10_000_000} + crawler = get_crawler(settings_dict=settings) + spider = self.spider_class.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + response = Response(url="https://example.com", body=body) + self.assertIsNone(spider._get_sitemap_body(response)) + class DeprecationTest(unittest.TestCase): def test_crawl_spider(self): From e0b66c021ae20cdcc24e4bb02ffae56005d3a073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 22 Nov 2023 19:03:24 +0100 Subject: [PATCH 19/60] Mind Spider.download_maxsize and Request.meta['download_maxsize'] --- .../downloadermiddlewares/httpcompression.py | 30 ++++-- scrapy/spiders/sitemap.py | 10 +- ...st_downloadermiddleware_httpcompression.py | 99 ++++++++++++++++--- tests/test_spider.py | 35 ++++++- 4 files changed, 143 insertions(+), 31 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 8ee1d95a6..e6463307e 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -1,5 +1,6 @@ import warnings +from scrapy import signals from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import Response, TextResponse from scrapy.responsetypes import responsetypes @@ -38,6 +39,7 @@ class HttpCompressionMiddleware: return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + crawler.signals.connect(self.open_spider, signals.spider_opened) @classmethod def from_crawler(cls, crawler): @@ -52,10 +54,15 @@ class HttpCompressionMiddleware: "reimplement their 'from_crawler' method.", ScrapyDeprecationWarning, ) - result = cls() - result.stats = crawler.stats - result._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") - return result + spider = cls() + spider.stats = crawler.stats + spider._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + crawler.signals.connect(spider.open_spider, signals.spider_opened) + return spider + + def open_spider(self, spider): + if hasattr(spider, "download_maxsize"): + self._max_size = spider.download_maxsize def process_request(self, request, spider): request.headers.setdefault("Accept-Encoding", b", ".join(ACCEPTED_ENCODINGS)) @@ -67,8 +74,11 @@ class HttpCompressionMiddleware: content_encoding = response.headers.getlist("Content-Encoding") if content_encoding: encoding = content_encoding.pop() + max_size = request.meta.get("download_maxsize", self._max_size) try: - decoded_body = self._decode(response.body, encoding.lower()) + decoded_body = self._decode( + response.body, encoding.lower(), max_size + ) except _DecompressionMaxSizeExceeded: raise IgnoreRequest( f"Ignored response {response} because its body " @@ -98,13 +108,13 @@ class HttpCompressionMiddleware: return response - def _decode(self, body, encoding): + def _decode(self, body, encoding, max_size): if encoding == b"gzip" or encoding == b"x-gzip": - return gunzip(body, max_size=self._max_size) + return gunzip(body, max_size=max_size) if encoding == b"deflate": - return _inflate(body, max_size=self._max_size) + return _inflate(body, max_size=max_size) if encoding == b"br" and b"br" in ACCEPTED_ENCODINGS: - return _unbrotli(body, max_size=self._max_size) + return _unbrotli(body, max_size=max_size) if encoding == b"zstd" and b"zstd" in ACCEPTED_ENCODINGS: - return _unzstd(body, max_size=self._max_size) + return _unzstd(body, max_size=max_size) return body diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index cc8b13cc3..3bca3f5c2 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -72,10 +72,14 @@ class SitemapSpider(Spider): if isinstance(response, XmlResponse): return response.body if gzip_magic_number(response): + max_size = response.meta.get( + "download_maxsize", + getattr( + self, "download_maxsize", self.settings.getint("DOWNLOAD_MAXSIZE") + ), + ) try: - return gunzip( - response.body, max_size=self.settings.getint("DOWNLOAD_MAXSIZE") - ) + return gunzip(response.body, max_size=max_size) except _DecompressionMaxSizeExceeded: return None # actual gzipped sitemap files are decompressed above ; diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 7babd1318..6d71ba71e 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -49,10 +49,7 @@ FORMAT = { class HttpCompressionTest(TestCase): def setUp(self): - settings = { - "DOWNLOAD_MAXSIZE": 10_000_000, # For compression bomb tests. - } - self.crawler = get_crawler(Spider, settings_dict=settings) + self.crawler = get_crawler(Spider) self.spider = self.crawler._create_spider("scrapytest.org") self.mw = HttpCompressionMiddleware.from_crawler(self.crawler) self.crawler.stats.open_spider(self.spider) @@ -373,31 +370,103 @@ class HttpCompressionTest(TestCase): self.assertStatsEqual("httpcompression/response_count", None) self.assertStatsEqual("httpcompression/response_bytes", None) - def _test_compression_bomb(self, compression_id): + def _test_compression_bomb_setting(self, compression_id): + settings = {"DOWNLOAD_MAXSIZE": 10_000_000} + crawler = get_crawler(Spider, settings_dict=settings) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + response = self._getresponse(f"bomb-{compression_id}") self.assertRaises( IgnoreRequest, - self.mw.process_response, + mw.process_response, response.request, response, - self.spider, + spider, ) - def test_compression_bomb_br(self): + def test_compression_bomb_setting_br(self): try: import brotli # noqa: F401 except ImportError: raise SkipTest("no brotli") - self._test_compression_bomb("br") + self._test_compression_bomb_setting("br") - def test_compression_bomb_deflate(self): - self._test_compression_bomb("deflate") + def test_compression_bomb_setting_deflate(self): + self._test_compression_bomb_setting("deflate") - def test_compression_bomb_gzip(self): - self._test_compression_bomb("gzip") + def test_compression_bomb_setting_gzip(self): + self._test_compression_bomb_setting("gzip") - def test_compression_bomb_zstd(self): - self._test_compression_bomb("zstd") + def test_compression_bomb_setting_zstd(self): + self._test_compression_bomb_setting("zstd") + + def _test_compression_bomb_spider_attr(self, compression_id): + class DownloadMaxSizeSpider(Spider): + download_maxsize = 10_000_000 + + crawler = get_crawler(DownloadMaxSizeSpider) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + + response = self._getresponse(f"bomb-{compression_id}") + self.assertRaises( + IgnoreRequest, + mw.process_response, + response.request, + response, + spider, + ) + + def test_compression_bomb_spider_attr_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_compression_bomb_spider_attr("br") + + def test_compression_bomb_spider_attr_deflate(self): + self._test_compression_bomb_spider_attr("deflate") + + def test_compression_bomb_spider_attr_gzip(self): + self._test_compression_bomb_spider_attr("gzip") + + def test_compression_bomb_spider_attr_zstd(self): + self._test_compression_bomb_spider_attr("zstd") + + def _test_compression_bomb_request_meta(self, compression_id): + crawler = get_crawler(Spider) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + + response = self._getresponse(f"bomb-{compression_id}") + response.meta["download_maxsize"] = 10_000_000 + self.assertRaises( + IgnoreRequest, + mw.process_response, + response.request, + response, + spider, + ) + + def test_compression_bomb_request_meta_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_compression_bomb_request_meta("br") + + def test_compression_bomb_request_meta_deflate(self): + self._test_compression_bomb_request_meta("deflate") + + def test_compression_bomb_request_meta_gzip(self): + self._test_compression_bomb_request_meta("gzip") + + def test_compression_bomb_request_meta_zstd(self): + self._test_compression_bomb_request_meta("zstd") class HttpCompressionSubclassTest(TestCase): diff --git a/tests/test_spider.py b/tests/test_spider.py index 875ff5454..e8480ceb4 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -509,6 +509,7 @@ class SitemapSpiderTest(SpiderTest): url="http://www.example.com/sitemap", body=self.GZBODY, headers={"content-type": "application/gzip"}, + request=Request("http://www.example.com/sitemap"), ) self.assertSitemapBody(r, self.BODY) @@ -517,7 +518,11 @@ class SitemapSpiderTest(SpiderTest): self.assertSitemapBody(r, self.BODY) def test_get_sitemap_body_xml_url_compressed(self): - r = Response(url="http://www.example.com/sitemap.xml.gz", body=self.GZBODY) + r = Response( + url="http://www.example.com/sitemap.xml.gz", + body=self.GZBODY, + request=Request("http://www.example.com/sitemap"), + ) self.assertSitemapBody(r, self.BODY) # .xml.gz but body decoded by HttpCompression middleware already @@ -694,13 +699,37 @@ Sitemap: /sitemap-relative-url.xml ["http://www.example.com/sitemap2.xml"], ) - def test_compression_bomb(self): + def test_compression_bomb_setting(self): settings = {"DOWNLOAD_MAXSIZE": 10_000_000} crawler = get_crawler(settings_dict=settings) spider = self.spider_class.from_crawler(crawler, "example.com") body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") body = body_path.read_bytes() - response = Response(url="https://example.com", body=body) + request = Request(url="https://example.com") + response = Response(url="https://example.com", body=body, request=request) + self.assertIsNone(spider._get_sitemap_body(response)) + + def test_compression_bomb_spider_attr(self): + class DownloadMaxSizeSpider(self.spider_class): + download_maxsize = 10_000_000 + + crawler = get_crawler() + spider = DownloadMaxSizeSpider.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request(url="https://example.com") + response = Response(url="https://example.com", body=body, request=request) + self.assertIsNone(spider._get_sitemap_body(response)) + + def test_compression_bomb_request_meta(self): + crawler = get_crawler() + spider = self.spider_class.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request( + url="https://example.com", meta={"download_maxsize": 10_000_000} + ) + response = Response(url="https://example.com", body=body, request=request) self.assertIsNone(spider._get_sitemap_body(response)) From 1087bb7b2eab28543bf9ba13149adb7acd4a3675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 09:11:14 +0100 Subject: [PATCH 20/60] Update the docs --- docs/topics/request-response.rst | 1 + docs/topics/settings.rst | 36 +++++++++++++++++--------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index adf3d0f4a..2d1227cf8 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -702,6 +702,7 @@ Those are: * :reqmeta:`download_fail_on_dataloss` * :reqmeta:`download_latency` * :reqmeta:`download_maxsize` +* :reqmeta:`download_warnsize` * :reqmeta:`download_timeout` * ``ftp_password`` (See :setting:`FTP_PASSWORD` for more info) * ``ftp_user`` (See :setting:`FTP_USER` for more info) diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 7cdfb8768..eb24b834a 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -873,40 +873,42 @@ The amount of time (in secs) that the downloader will wait before timing out. Request.meta key. .. setting:: DOWNLOAD_MAXSIZE +.. reqmeta:: download_maxsize DOWNLOAD_MAXSIZE ---------------- -Default: ``1073741824`` (1024MB) +Default: ``1073741824`` (1 GiB) -The maximum response size (in bytes) that downloader will download. +The maximum response body size (in bytes) allowed. Bigger responses are +aborted and ignored. -If you want to disable it set to 0. +This applies both before and after compression. If decompressing a response +body would exceed this limit, decompression is aborted and the response is +ignored. -.. reqmeta:: download_maxsize +Use ``0`` to disable this limit. -.. note:: - - This size can be set per spider using :attr:`download_maxsize` - spider attribute and per-request using :reqmeta:`download_maxsize` - Request.meta key. +This limit can be set per spider using the :attr:`download_maxsize` spider +attribute and per request using the :reqmeta:`download_maxsize` Request.meta +key. .. setting:: DOWNLOAD_WARNSIZE +.. reqmeta:: download_warnsize DOWNLOAD_WARNSIZE ----------------- -Default: ``33554432`` (32MB) +Default: ``33554432`` (32 MiB) -The response size (in bytes) that downloader will start to warn. +If the size of a response exceeds this value, before or after compression, a +warning will be logged about it. -If you want to disable it set to 0. +Use ``0`` to disable this limit. -.. note:: - - This size can be set per spider using :attr:`download_warnsize` - spider attribute and per-request using :reqmeta:`download_warnsize` - Request.meta key. +This limit can be set per spider using the :attr:`download_warnsize` spider +attribute and per request using the :reqmeta:`download_warnsize` Request.meta +key. .. setting:: DOWNLOAD_FAIL_ON_DATALOSS From 03d9866518ab43844ba0309394529240f4cf115e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 10:26:47 +0100 Subject: [PATCH 21/60] Also use DOWNLOAD_WARNSIZE for decompressions --- .../downloadermiddlewares/httpcompression.py | 18 ++- scrapy/spiders/sitemap.py | 35 ++++- scrapy/utils/_compression.py | 12 +- scrapy/utils/gz.py | 4 +- ...st_downloadermiddleware_httpcompression.py | 130 ++++++++++++++++++ tests/test_spider.py | 78 +++++++++++ 6 files changed, 260 insertions(+), 17 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index e6463307e..95bc1849d 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -1,4 +1,5 @@ import warnings +from logging import getLogger from scrapy import signals from scrapy.exceptions import IgnoreRequest, NotConfigured @@ -13,6 +14,8 @@ from scrapy.utils._compression import ( from scrapy.utils.deprecate import ScrapyDeprecationWarning from scrapy.utils.gz import gunzip +logger = getLogger(__name__) + ACCEPTED_ENCODINGS = [b"gzip", b"deflate"] try: @@ -39,6 +42,7 @@ class HttpCompressionMiddleware: return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + self._warn_size = crawler.settings.getint("DOWNLOAD_WARNSIZE") crawler.signals.connect(self.open_spider, signals.spider_opened) @classmethod @@ -57,12 +61,15 @@ class HttpCompressionMiddleware: spider = cls() spider.stats = crawler.stats spider._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + spider._warn_size = crawler.settings.getint("DOWNLOAD_WARNSIZE") crawler.signals.connect(spider.open_spider, signals.spider_opened) return spider def open_spider(self, spider): if hasattr(spider, "download_maxsize"): self._max_size = spider.download_maxsize + if hasattr(spider, "download_warnsize"): + self._warn_size = spider.download_warnsize def process_request(self, request, spider): request.headers.setdefault("Accept-Encoding", b", ".join(ACCEPTED_ENCODINGS)) @@ -75,6 +82,7 @@ class HttpCompressionMiddleware: if content_encoding: encoding = content_encoding.pop() max_size = request.meta.get("download_maxsize", self._max_size) + warn_size = request.meta.get("download_warnsize", self._warn_size) try: decoded_body = self._decode( response.body, encoding.lower(), max_size @@ -82,8 +90,14 @@ class HttpCompressionMiddleware: except _DecompressionMaxSizeExceeded: raise IgnoreRequest( f"Ignored response {response} because its body " - f"({len(response.body)}B) exceeded DOWNLOAD_MAXSIZE " - f"({self._max_size}B) during decompression." + f"({len(response.body)} B) exceeded DOWNLOAD_MAXSIZE " + f"({self._max_size} B) during decompression." + ) + if len(response.body) < warn_size and len(decoded_body) >= warn_size: + logger.warning( + f"{response} body size after decompression " + f"({len(decoded_body)} B) is larger than the " + f"download warning size ({warn_size} B)." ) if self.stats: self.stats.inc_value( diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index 3bca3f5c2..0574f0ccb 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -1,5 +1,6 @@ import logging import re +from typing import TYPE_CHECKING, Any from scrapy.http import Request, XmlResponse from scrapy.spiders import Spider @@ -7,6 +8,12 @@ from scrapy.utils._compression import _DecompressionMaxSizeExceeded from scrapy.utils.gz import gunzip, gzip_magic_number from scrapy.utils.sitemap import Sitemap, sitemap_urls_from_robots +if TYPE_CHECKING: + # typing.Self requires Python 3.11 + from typing_extensions import Self + + from scrapy.crawler import Crawler + logger = logging.getLogger(__name__) @@ -16,6 +23,17 @@ class SitemapSpider(Spider): sitemap_follow = [""] sitemap_alternate_links = False + @classmethod + def from_crawler(cls, crawler: "Crawler", *args: Any, **kwargs: Any) -> "Self": + spider = super().from_crawler(crawler, *args, **kwargs) + spider._max_size = getattr( + spider, "download_maxsize", spider.settings.getint("DOWNLOAD_MAXSIZE") + ) + spider._warn_size = getattr( + spider, "download_warnsize", spider.settings.getint("DOWNLOAD_WARNSIZE") + ) + return spider + def __init__(self, *a, **kw): super().__init__(*a, **kw) self._cbs = [] @@ -72,16 +90,19 @@ class SitemapSpider(Spider): if isinstance(response, XmlResponse): return response.body if gzip_magic_number(response): - max_size = response.meta.get( - "download_maxsize", - getattr( - self, "download_maxsize", self.settings.getint("DOWNLOAD_MAXSIZE") - ), - ) + uncompressed_size = len(response.body) + max_size = response.meta.get("download_maxsize", self._max_size) + warn_size = response.meta.get("download_warnsize", self._warn_size) try: - return gunzip(response.body, max_size=max_size) + body = gunzip(response.body, max_size=max_size) except _DecompressionMaxSizeExceeded: return None + if uncompressed_size < warn_size and len(body) >= warn_size: + logger.warning( + f"{response} body size after decompression ({len(body)} B) " + f"is larger than the download warning size ({warn_size} B)." + ) + return body # actual gzipped sitemap files are decompressed above ; # if we are here (response body is not gzipped) # and have a response for .xml.gz, diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 93aa254b2..a70f6c275 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -44,8 +44,8 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( f"The number of bytes decompressed so far " - f"({decompressed_size}B) exceed the specified maximum " - f"({max_size}B)." + f"({decompressed_size} B) exceed the specified maximum " + f"({max_size} B)." ) output_list.append(output_chunk) return b"".join(output_list) @@ -65,8 +65,8 @@ def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( f"The number of bytes decompressed so far " - f"({decompressed_size}B) exceed the specified maximum " - f"({max_size}B)." + f"({decompressed_size} B) exceed the specified maximum " + f"({max_size} B)." ) output_list.append(output_chunk) return b"".join(output_list) @@ -85,8 +85,8 @@ def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( f"The number of bytes decompressed so far " - f"({decompressed_size}B) exceed the specified maximum " - f"({max_size}B)." + f"({decompressed_size} B) exceed the specified maximum " + f"({max_size} B)." ) output_list.append(output_chunk) return b"".join(output_list) diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index 548134721..e5cf68d62 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -31,8 +31,8 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( f"The number of bytes decompressed so far " - f"({decompressed_size}B) exceed the specified maximum " - f"({max_size}B)." + f"({decompressed_size} B) exceed the specified maximum " + f"({max_size} B)." ) output_list.append(chunk) return b"".join(output_list) diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index 6d71ba71e..f74fff218 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -1,9 +1,11 @@ from gzip import GzipFile from io import BytesIO +from logging import WARNING from pathlib import Path from unittest import SkipTest, TestCase from warnings import catch_warnings +from testfixtures import LogCapture from w3lib.encoding import resolve_encoding from scrapy.downloadermiddlewares.httpcompression import ( @@ -468,6 +470,134 @@ class HttpCompressionTest(TestCase): def test_compression_bomb_request_meta_zstd(self): self._test_compression_bomb_request_meta("zstd") + def _test_download_warnsize_setting(self, compression_id): + settings = {"DOWNLOAD_WARNSIZE": 10_000_000} + crawler = get_crawler(Spider, settings_dict=settings) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + response = self._getresponse(f"bomb-{compression_id}") + + with LogCapture( + "scrapy.downloadermiddlewares.httpcompression", + propagate=False, + level=WARNING, + ) as log: + mw.process_response(response.request, response, spider) + log.check( + ( + "scrapy.downloadermiddlewares.httpcompression", + "WARNING", + ( + "<200 http://scrapytest.org/> body size after " + "decompression (11511612 B) is larger than the download " + "warning size (10000000 B)." + ), + ), + ) + + def test_download_warnsize_setting_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_download_warnsize_setting("br") + + def test_download_warnsize_setting_deflate(self): + self._test_download_warnsize_setting("deflate") + + def test_download_warnsize_setting_gzip(self): + self._test_download_warnsize_setting("gzip") + + def test_download_warnsize_setting_zstd(self): + self._test_download_warnsize_setting("zstd") + + def _test_download_warnsize_spider_attr(self, compression_id): + class DownloadWarnSizeSpider(Spider): + download_warnsize = 10_000_000 + + crawler = get_crawler(DownloadWarnSizeSpider) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + response = self._getresponse(f"bomb-{compression_id}") + + with LogCapture( + "scrapy.downloadermiddlewares.httpcompression", + propagate=False, + level=WARNING, + ) as log: + mw.process_response(response.request, response, spider) + log.check( + ( + "scrapy.downloadermiddlewares.httpcompression", + "WARNING", + ( + "<200 http://scrapytest.org/> body size after " + "decompression (11511612 B) is larger than the download " + "warning size (10000000 B)." + ), + ), + ) + + def test_download_warnsize_spider_attr_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_download_warnsize_spider_attr("br") + + def test_download_warnsize_spider_attr_deflate(self): + self._test_download_warnsize_spider_attr("deflate") + + def test_download_warnsize_spider_attr_gzip(self): + self._test_download_warnsize_spider_attr("gzip") + + def test_download_warnsize_spider_attr_zstd(self): + self._test_download_warnsize_spider_attr("zstd") + + def _test_download_warnsize_request_meta(self, compression_id): + crawler = get_crawler(Spider) + spider = crawler._create_spider("scrapytest.org") + mw = HttpCompressionMiddleware.from_crawler(crawler) + mw.open_spider(spider) + response = self._getresponse(f"bomb-{compression_id}") + response.meta["download_warnsize"] = 10_000_000 + + with LogCapture( + "scrapy.downloadermiddlewares.httpcompression", + propagate=False, + level=WARNING, + ) as log: + mw.process_response(response.request, response, spider) + log.check( + ( + "scrapy.downloadermiddlewares.httpcompression", + "WARNING", + ( + "<200 http://scrapytest.org/> body size after " + "decompression (11511612 B) is larger than the download " + "warning size (10000000 B)." + ), + ), + ) + + def test_download_warnsize_request_meta_br(self): + try: + import brotli # noqa: F401 + except ImportError: + raise SkipTest("no brotli") + self._test_download_warnsize_request_meta("br") + + def test_download_warnsize_request_meta_deflate(self): + self._test_download_warnsize_request_meta("deflate") + + def test_download_warnsize_request_meta_gzip(self): + self._test_download_warnsize_request_meta("gzip") + + def test_download_warnsize_request_meta_zstd(self): + self._test_download_warnsize_request_meta("zstd") + class HttpCompressionSubclassTest(TestCase): def test_init_missing_stats(self): diff --git a/tests/test_spider.py b/tests/test_spider.py index e8480ceb4..3f595cc93 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -2,6 +2,7 @@ import gzip import inspect import warnings from io import BytesIO +from logging import WARNING from pathlib import Path from typing import Any from unittest import mock @@ -732,6 +733,83 @@ Sitemap: /sitemap-relative-url.xml response = Response(url="https://example.com", body=body, request=request) self.assertIsNone(spider._get_sitemap_body(response)) + def test_download_warnsize_setting(self): + settings = {"DOWNLOAD_WARNSIZE": 10_000_000} + crawler = get_crawler(settings_dict=settings) + spider = self.spider_class.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request(url="https://example.com") + response = Response(url="https://example.com", body=body, request=request) + with LogCapture( + "scrapy.spiders.sitemap", propagate=False, level=WARNING + ) as log: + spider._get_sitemap_body(response) + log.check( + ( + "scrapy.spiders.sitemap", + "WARNING", + ( + "<200 https://example.com> body size after decompression " + "(11511612 B) is larger than the download warning size " + "(10000000 B)." + ), + ), + ) + + def test_download_warnsize_spider_attr(self): + class DownloadWarnSizeSpider(self.spider_class): + download_warnsize = 10_000_000 + + crawler = get_crawler() + spider = DownloadWarnSizeSpider.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request( + url="https://example.com", meta={"download_warnsize": 10_000_000} + ) + response = Response(url="https://example.com", body=body, request=request) + with LogCapture( + "scrapy.spiders.sitemap", propagate=False, level=WARNING + ) as log: + spider._get_sitemap_body(response) + log.check( + ( + "scrapy.spiders.sitemap", + "WARNING", + ( + "<200 https://example.com> body size after decompression " + "(11511612 B) is larger than the download warning size " + "(10000000 B)." + ), + ), + ) + + def test_download_warnsize_request_meta(self): + crawler = get_crawler() + spider = self.spider_class.from_crawler(crawler, "example.com") + body_path = Path(tests_datadir, "compressed", "bomb-gzip.bin") + body = body_path.read_bytes() + request = Request( + url="https://example.com", meta={"download_warnsize": 10_000_000} + ) + response = Response(url="https://example.com", body=body, request=request) + with LogCapture( + "scrapy.spiders.sitemap", propagate=False, level=WARNING + ) as log: + spider._get_sitemap_body(response) + log.check( + ( + "scrapy.spiders.sitemap", + "WARNING", + ( + "<200 https://example.com> body size after decompression " + "(11511612 B) is larger than the download warning size " + "(10000000 B)." + ), + ), + ) + class DeprecationTest(unittest.TestCase): def test_crawl_spider(self): From b53ed52a22470adbe269f5a7dcc67a0da369eaf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 11:36:45 +0100 Subject: [PATCH 22/60] Update the release notes --- docs/news.rst | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index 0c202639e..c4081b99b 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -3,6 +3,20 @@ Release notes ============= +.. _release-2.11.1: + +Scrapy 2.11.1 (unreleased) +-------------------------- + +**Security bug fix:** + +- :setting:`DOWNLOAD_MAXSIZE` and :setting:`DOWNLOAD_WARNSIZE` now also apply + to the decompressed response body. Please, see the `7j7m-v7m3-jqm7 security + advisory`_ for more information. + + .. _7j7m-v7m3-jqm7 security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-7j7m-v7m3-jqm7 + + .. _release-2.11.0: Scrapy 2.11.0 (2023-09-18) @@ -2871,6 +2885,17 @@ affect subclasses: (:issue:`3884`) +.. _release-1.8.4: + +Scrapy 1.8.4 (unreleased) +------------------------- + +**Security bug fix:** + +- :setting:`DOWNLOAD_MAXSIZE` and :setting:`DOWNLOAD_WARNSIZE` now also apply + to the decompressed response body. Please, see the `7j7m-v7m3-jqm7 security + advisory`_ for more information. + .. _release-1.8.3: From cf80e5670e8317858b9f60008a5d2d97b4988da0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 12:07:15 +0100 Subject: [PATCH 23/60] Solve linting and typing issues --- scrapy/downloadermiddlewares/httpcompression.py | 2 +- scrapy/spiders/sitemap.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 95bc1849d..6c8b659bd 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -93,7 +93,7 @@ class HttpCompressionMiddleware: f"({len(response.body)} B) exceeded DOWNLOAD_MAXSIZE " f"({self._max_size} B) during decompression." ) - if len(response.body) < warn_size and len(decoded_body) >= warn_size: + if len(response.body) < warn_size <= len(decoded_body): logger.warning( f"{response} body size after decompression " f"({len(decoded_body)} B) is larger than the " diff --git a/scrapy/spiders/sitemap.py b/scrapy/spiders/sitemap.py index 0574f0ccb..386aa6a6e 100644 --- a/scrapy/spiders/sitemap.py +++ b/scrapy/spiders/sitemap.py @@ -22,6 +22,8 @@ class SitemapSpider(Spider): sitemap_rules = [("", "parse")] sitemap_follow = [""] sitemap_alternate_links = False + _max_size: int + _warn_size: int @classmethod def from_crawler(cls, crawler: "Crawler", *args: Any, **kwargs: Any) -> "Self": @@ -97,7 +99,7 @@ class SitemapSpider(Spider): body = gunzip(response.body, max_size=max_size) except _DecompressionMaxSizeExceeded: return None - if uncompressed_size < warn_size and len(body) >= warn_size: + if uncompressed_size < warn_size <= len(body): logger.warning( f"{response} body size after decompression ({len(body)} B) " f"is larger than the download warning size ({warn_size} B)." From 8e25f8c157e53c9f5df51e950fed18becfe7797d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 23 Nov 2023 14:12:59 +0100 Subject: [PATCH 24/60] Fix bad message --- scrapy/downloadermiddlewares/httpcompression.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 6c8b659bd..f03294d65 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -91,7 +91,7 @@ class HttpCompressionMiddleware: raise IgnoreRequest( f"Ignored response {response} because its body " f"({len(response.body)} B) exceeded DOWNLOAD_MAXSIZE " - f"({self._max_size} B) during decompression." + f"({max_size} B) during decompression." ) if len(response.body) < warn_size <= len(decoded_body): logger.warning( From 5f2827efe7b069e514a87bd0f7a9589f49f0a97a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 10:02:56 +0100 Subject: [PATCH 25/60] Make HttpCompressionMiddleware changes backward-comaptible --- scrapy/downloadermiddlewares/httpcompression.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index f03294d65..1a3f6962a 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -37,8 +37,10 @@ class HttpCompressionMiddleware: """This middleware allows compressed (gzip, deflate) traffic to be sent/received from web sites""" - def __init__(self, *, crawler=None): + def __init__(self, stats=None, *, crawler=None): if not crawler: + if stats: + self.stats = stats return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") From 6c278e1862c5453ec45a8f6a5c2472710895cad9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 10:15:21 +0100 Subject: [PATCH 26/60] =?UTF-8?q?List[bytes]=20=E2=86=92=20BytesIO?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/utils/_compression.py | 22 ++++++++++++---------- scrapy/utils/gz.py | 12 ++++++------ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index a70f6c275..b17fd7881 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -1,6 +1,5 @@ import zlib from io import BytesIO -from typing import List try: import brotli @@ -21,7 +20,7 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: decompressor = zlib.decompressobj() raw_decompressor = zlib.decompressobj(wbits=-15) input_stream = BytesIO(data) - output_list: List[bytes] = [] + output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 CHUNK_SIZE = 8196 @@ -47,14 +46,15 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: f"({decompressed_size} B) exceed the specified maximum " f"({max_size} B)." ) - output_list.append(output_chunk) - return b"".join(output_list) + output_stream.write(output_chunk) + output_stream.seek(0) + return output_stream.read() def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: decompressor = brotli.Decompressor() input_stream = BytesIO(data) - output_list: List[bytes] = [] + output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 CHUNK_SIZE = 8196 @@ -68,14 +68,15 @@ def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: f"({decompressed_size} B) exceed the specified maximum " f"({max_size} B)." ) - output_list.append(output_chunk) - return b"".join(output_list) + output_stream.write(output_chunk) + output_stream.seek(0) + return output_stream.read() def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: decompressor = zstandard.ZstdDecompressor() stream_reader = decompressor.stream_reader(BytesIO(data)) - output_list: List[bytes] = [] + output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 CHUNK_SIZE = 8196 @@ -88,5 +89,6 @@ def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: f"({decompressed_size} B) exceed the specified maximum " f"({max_size} B)." ) - output_list.append(output_chunk) - return b"".join(output_list) + output_stream.write(output_chunk) + output_stream.seek(0) + return output_stream.read() diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index e5cf68d62..5d23e8f05 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -1,7 +1,6 @@ import struct from gzip import GzipFile from io import BytesIO -from typing import List from scrapy.http import Response @@ -14,7 +13,7 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: This is resilient to CRC checksum errors. """ f = GzipFile(fileobj=BytesIO(data)) - output_list: List[bytes] = [] + output_stream = BytesIO() chunk = b"." decompressed_size = 0 while chunk: @@ -23,8 +22,8 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: except (OSError, EOFError, struct.error): # complete only if there is some data, otherwise re-raise # see issue 87 about catching struct.error - # some pages are quite small so output_list is empty - if output_list: + # some pages are quite small so output_stream is empty + if output_stream: break raise decompressed_size += len(chunk) @@ -34,8 +33,9 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: f"({decompressed_size} B) exceed the specified maximum " f"({max_size} B)." ) - output_list.append(chunk) - return b"".join(output_list) + output_stream.write(chunk) + output_stream.seek(0) + return output_stream.read() def gzip_magic_number(response: Response) -> bool: From 62398e424c0a3d66d0bdd3908e47284cbe797ef9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 10:20:14 +0100 Subject: [PATCH 27/60] =?UTF-8?q?CHUNK=5FSIZE:=208=20KiB=20=E2=86=92=2032?= =?UTF-8?q?=20KiB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/utils/_compression.py | 12 ++++++------ scrapy/utils/gz.py | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index b17fd7881..5610595d3 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -12,6 +12,9 @@ except ImportError: pass +_CHUNK_SIZE = 65536 # 64 KiB + + class _DecompressionMaxSizeExceeded(ValueError): pass @@ -23,9 +26,8 @@ def _inflate(data: bytes, *, max_size: int = 0) -> bytes: output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 - CHUNK_SIZE = 8196 while output_chunk: - input_chunk = input_stream.read(CHUNK_SIZE) + input_chunk = input_stream.read(_CHUNK_SIZE) try: output_chunk = decompressor.decompress(input_chunk) except zlib.error: @@ -57,9 +59,8 @@ def _unbrotli(data: bytes, *, max_size: int = 0) -> bytes: output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 - CHUNK_SIZE = 8196 while output_chunk: - input_chunk = input_stream.read(CHUNK_SIZE) + input_chunk = input_stream.read(_CHUNK_SIZE) output_chunk = decompressor.process(input_chunk) decompressed_size += len(output_chunk) if max_size and decompressed_size > max_size: @@ -79,9 +80,8 @@ def _unzstd(data: bytes, *, max_size: int = 0) -> bytes: output_stream = BytesIO() output_chunk = b"." decompressed_size = 0 - CHUNK_SIZE = 8196 while output_chunk: - output_chunk = stream_reader.read(CHUNK_SIZE) + output_chunk = stream_reader.read(_CHUNK_SIZE) decompressed_size += len(output_chunk) if max_size and decompressed_size > max_size: raise _DecompressionMaxSizeExceeded( diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index 5d23e8f05..cf7316e82 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -4,7 +4,7 @@ from io import BytesIO from scrapy.http import Response -from ._compression import _DecompressionMaxSizeExceeded +from ._compression import _CHUNK_SIZE, _DecompressionMaxSizeExceeded def gunzip(data: bytes, *, max_size: int = 0) -> bytes: @@ -18,7 +18,7 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: decompressed_size = 0 while chunk: try: - chunk = f.read1(8196) + chunk = f.read1(_CHUNK_SIZE) except (OSError, EOFError, struct.error): # complete only if there is some data, otherwise re-raise # see issue 87 about catching struct.error From 8a73c6c90c5984292d39a0a9d4be86e792d81cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 10:25:01 +0100 Subject: [PATCH 28/60] Fix HttpCompressionMiddleware backward compatibility --- scrapy/downloadermiddlewares/httpcompression.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 1a3f6962a..58ca1017f 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -39,8 +39,9 @@ class HttpCompressionMiddleware: def __init__(self, stats=None, *, crawler=None): if not crawler: - if stats: - self.stats = stats + self.stats = stats + self._max_size = 1073741824 + self._warn_size = 33554432 return self.stats = crawler.stats self._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") From a113208a0643263fdd7198238bf9213f9148bc1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 24 Nov 2023 11:35:15 +0100 Subject: [PATCH 29/60] Fix BytesIO non-emptiness check --- scrapy/utils/gz.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scrapy/utils/gz.py b/scrapy/utils/gz.py index cf7316e82..2e487d88b 100644 --- a/scrapy/utils/gz.py +++ b/scrapy/utils/gz.py @@ -23,7 +23,7 @@ def gunzip(data: bytes, *, max_size: int = 0) -> bytes: # complete only if there is some data, otherwise re-raise # see issue 87 about catching struct.error # some pages are quite small so output_stream is empty - if output_stream: + if output_stream.getbuffer().nbytes > 0: break raise decompressed_size += len(chunk) From c947f51077e1ab246f30764cc4cc7a1cc5835d40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 29 Nov 2023 11:54:08 +0100 Subject: [PATCH 30/60] Set an arbitrary upper limit on ReDoS-vulnerable regexps --- docs/news.rst | 28 ++++++++++++++++++++++++++++ scrapy/utils/iterators.py | 12 +++++++----- scrapy/utils/response.py | 4 ++-- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index fd8fa3ea3..121cc0322 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -3,6 +3,22 @@ Release notes ============= +.. _release-2.11.1: + +Scrapy 2.11.1 (unreleased) +-------------------------- + +**Security bug fix:** + +- The regular expressions of the ``iternodes`` node iterator of + :class:`~scrapy.spiders.XMLFeedSpider` are no longer susceptible to a + `ReDoS attack`_. Please, see the `cc65-xxvf-f7r9 security + advisory`_ for more information. + + .. _ReDoS attack: https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS + .. _cc65-xxvf-f7r9 security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-cc65-xxvf-f7r9 + + .. _release-2.11.0: Scrapy 2.11.0 (2023-09-18) @@ -2869,6 +2885,18 @@ affect subclasses: (:issue:`3884`) +.. _release-1.8.4: + +Scrapy 1.8.4 (unreleased) +------------------------- + +**Security bug fix:** + +- The regular expressions of the ``iternodes`` node iterator of + :class:`~scrapy.spiders.XMLFeedSpider` are no longer susceptible to a + `ReDoS attack`_. Please, see the `cc65-xxvf-f7r9 security + advisory`_ for more information. + .. _release-1.8.3: diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index 03d779afb..6b89334e0 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -40,10 +40,10 @@ def xmliter( """ nodename_patt = re.escape(nodename) - DOCUMENT_HEADER_RE = re.compile(r"<\?xml[^>]+>\s*", re.S) + DOCUMENT_HEADER_RE = re.compile(r"<\?xml[^>]{1,1024}>\s*", re.S) HEADER_END_RE = re.compile(rf"<\s*/{nodename_patt}\s*>", re.S) - END_TAG_RE = re.compile(r"<\s*/([^\s>]+)\s*>", re.S) - NAMESPACE_RE = re.compile(r"((xmlns[:A-Za-z]*)=[^>\s]+)", re.S) + END_TAG_RE = re.compile(r"<\s*/([^\s>]{1,1024})\s*>", re.S) + NAMESPACE_RE = re.compile(r"((xmlns[:A-Za-z]{,1024})=[^>\s]+)", re.S) text = _body_or_str(obj) document_header_match = re.search(DOCUMENT_HEADER_RE, text) @@ -57,13 +57,15 @@ def xmliter( for tagname in reversed(re.findall(END_TAG_RE, header_end)): assert header_end_idx tag = re.search( - rf"<\s*{tagname}.*?xmlns[:=][^>]*>", text[: header_end_idx[1]], re.S + rf"<\s*{tagname}.{{,1024}}?xmlns[:=][^>]{{,1024}}>", + text[: header_end_idx[1]], + re.S, ) if tag: for x in re.findall(NAMESPACE_RE, tag.group()): namespaces[x[1]] = x[0] - r = re.compile(rf"<{nodename_patt}[\s>].*?", re.DOTALL) + r = re.compile(rf"<{nodename_patt}[\s>].{{,1024}}?", re.DOTALL) for match in r.finditer(text): nodetext = ( document_header diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index c540d6278..b0e106c5e 100644 --- a/scrapy/utils/response.py +++ b/scrapy/utils/response.py @@ -91,8 +91,8 @@ def open_in_browser( if isinstance(response, HtmlResponse): if b"' - body = re.sub(b"", b"", body, flags=re.DOTALL) - body = re.sub(rb"(|\s.*?>))", to_bytes(repl), body) + body = re.sub(b"", b"", body, flags=re.DOTALL) + body = re.sub(rb"(|\s.{,1024}?>))", to_bytes(repl), body) ext = ".html" elif isinstance(response, TextResponse): ext = ".txt" From eb8b2c5197f3dd8570bad1945b23886ee57d045f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 29 Nov 2023 12:13:04 +0100 Subject: [PATCH 31/60] Mention open_in_browser in the release notes --- docs/news.rst | 16 ++++++++-------- docs/topics/debug.rst | 18 +++--------------- scrapy/utils/response.py | 17 +++++++++++++++-- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 121cc0322..2bbe833a5 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -10,10 +10,10 @@ Scrapy 2.11.1 (unreleased) **Security bug fix:** -- The regular expressions of the ``iternodes`` node iterator of - :class:`~scrapy.spiders.XMLFeedSpider` are no longer susceptible to a - `ReDoS attack`_. Please, see the `cc65-xxvf-f7r9 security - advisory`_ for more information. +- Fixed regular expressions susceptible to a `ReDoS attack`_ affecting the + ``iternodes`` node iterator of :class:`~scrapy.spiders.XMLFeedSpider` and + the :func:`~scrapy.utils.response.open_in_browser` function. Please, see + the `cc65-xxvf-f7r9 security advisory`_ for more information. .. _ReDoS attack: https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS .. _cc65-xxvf-f7r9 security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-cc65-xxvf-f7r9 @@ -2892,10 +2892,10 @@ Scrapy 1.8.4 (unreleased) **Security bug fix:** -- The regular expressions of the ``iternodes`` node iterator of - :class:`~scrapy.spiders.XMLFeedSpider` are no longer susceptible to a - `ReDoS attack`_. Please, see the `cc65-xxvf-f7r9 security - advisory`_ for more information. +- Fixed regular expressions susceptible to a `ReDoS attack`_ affecting the + ``iternodes`` node iterator of :class:`~scrapy.spiders.XMLFeedSpider` and + the :func:`~scrapy.utils.response.open_in_browser` function. Please, see + the `cc65-xxvf-f7r9 security advisory`_ for more information. .. _release-1.8.3: diff --git a/docs/topics/debug.rst b/docs/topics/debug.rst index 49c5b0410..988e37bbd 100644 --- a/docs/topics/debug.rst +++ b/docs/topics/debug.rst @@ -125,25 +125,15 @@ Fortunately, the :command:`shell` is your bread and butter in this case (see See also: :ref:`topics-shell-inspect-response`. + Open in browser =============== Sometimes you just want to see how a certain response looks in a browser, you -can use the ``open_in_browser`` function for that. Here is an example of how -you would use it: +can use the :func:`~scrapy.utils.response.open_in_browser` function for that: -.. code-block:: python +.. autofunction:: scrapy.utils.response.open_in_browser - from scrapy.utils.response import open_in_browser - - - def parse_details(self, response): - if "item name" not in response.body: - open_in_browser(response) - -``open_in_browser`` will open a browser with the response received by Scrapy at -that point, adjusting the `base tag`_ so that images and styles are displayed -properly. Logging ======= @@ -163,8 +153,6 @@ available in all future runs should they be necessary again: For more information, check the :ref:`topics-logging` section. -.. _base tag: https://www.w3schools.com/tags/tag_base.asp - .. _debug-vscode: Visual Studio Code diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index b0e106c5e..8401d4ed1 100644 --- a/scrapy/utils/response.py +++ b/scrapy/utils/response.py @@ -81,8 +81,21 @@ def open_in_browser( ], _openfunc: Callable[[str], Any] = webbrowser.open, ) -> Any: - """Open the given response in a local web browser, populating the - tag for external links to work + """Open *response* in a local web browser, adjusting the `base tag`_ for + external links to work, e.g. so that images and styles are displayed. + + .. _base tag: https://www.w3schools.com/tags/tag_base.asp + + For example: + + .. code-block:: python + + from scrapy.utils.response import open_in_browser + + + def parse_details(self, response): + if "item name" not in response.body: + open_in_browser(response) """ from scrapy.http import HtmlResponse, TextResponse From 40b3efbbee3919e8f6900daeaed5e14183cabfd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 29 Nov 2023 12:47:04 +0100 Subject: [PATCH 32/60] Remove open_in_browser from the 1.8.4 release notes --- docs/news.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 2bbe833a5..c14815d06 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -2893,9 +2893,8 @@ Scrapy 1.8.4 (unreleased) **Security bug fix:** - Fixed regular expressions susceptible to a `ReDoS attack`_ affecting the - ``iternodes`` node iterator of :class:`~scrapy.spiders.XMLFeedSpider` and - the :func:`~scrapy.utils.response.open_in_browser` function. Please, see - the `cc65-xxvf-f7r9 security advisory`_ for more information. + ``iternodes`` node iterator of :class:`~scrapy.spiders.XMLFeedSpider`. + Please, see the `cc65-xxvf-f7r9 security advisory`_ for more information. .. _release-1.8.3: From bb74badd1bd66c59a63268c52342507c76d290b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Mon, 11 Dec 2023 17:39:55 +0100 Subject: [PATCH 33/60] =?UTF-8?q?spider=20=E2=86=92=20mw?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scrapy/downloadermiddlewares/httpcompression.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 58ca1017f..816be25a1 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -61,12 +61,12 @@ class HttpCompressionMiddleware: "reimplement their 'from_crawler' method.", ScrapyDeprecationWarning, ) - spider = cls() - spider.stats = crawler.stats - spider._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") - spider._warn_size = crawler.settings.getint("DOWNLOAD_WARNSIZE") - crawler.signals.connect(spider.open_spider, signals.spider_opened) - return spider + mw = cls() + mw.stats = crawler.stats + mw._max_size = crawler.settings.getint("DOWNLOAD_MAXSIZE") + mw._warn_size = crawler.settings.getint("DOWNLOAD_WARNSIZE") + crawler.signals.connect(mw.open_spider, signals.spider_opened) + return mw def open_spider(self, spider): if hasattr(spider, "download_maxsize"): From 1533b69032e2fb5e495e88a3fed57c0d98502612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 13 Dec 2023 12:01:35 +0100 Subject: [PATCH 34/60] Test and address ReDoS attack vectors for open_in_browser --- scrapy/utils/response.py | 6 +++--- tests/test_utils_response.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index 8401d4ed1..4369e6439 100644 --- a/scrapy/utils/response.py +++ b/scrapy/utils/response.py @@ -103,9 +103,9 @@ def open_in_browser( body = response.body if isinstance(response, HtmlResponse): if b"' - body = re.sub(b"", b"", body, flags=re.DOTALL) - body = re.sub(rb"(|\s.{,1024}?>))", to_bytes(repl), body) + repl = rf'\0' + body = re.sub(b"(?s)|$)", b"", body) + body = re.sub(rb"]*?>)", to_bytes(repl), body, count=1) ext = ".html" elif isinstance(response, TextResponse): ext = ".txt" diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index 80e15a60f..942584d92 100644 --- a/tests/test_utils_response.py +++ b/tests/test_utils_response.py @@ -1,10 +1,12 @@ import unittest import warnings from pathlib import Path +from time import process_time from urllib.parse import urlparse from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import HtmlResponse, Response, TextResponse +from scrapy.settings.default_settings import DOWNLOAD_MAXSIZE from scrapy.utils.python import to_bytes from scrapy.utils.response import ( get_base_url, @@ -198,3 +200,37 @@ class ResponseUtilsTest(unittest.TestCase): assert open_in_browser( r5, _openfunc=check_base_url ), "Inject unique base url with conditional comment" + + def test_open_in_browser_redos_comment(self): + MAX_CPU_TIME = 30 + + # Exploit input from + # https://makenowjust-labs.github.io/recheck/playground/ + # for // (old pattern to remove comments). + body = b"->" + + response = HtmlResponse("https://example.com", body=body) + + start_time = process_time() + + open_in_browser(response, lambda url: True) + + end_time = process_time() + self.assertLess(end_time - start_time, MAX_CPU_TIME) + + def test_open_in_browser_redos_head(self): + MAX_CPU_TIME = 15 + + # Exploit input from + # https://makenowjust-labs.github.io/recheck/playground/ + # for /(|\s.*?>))/ (old pattern to find the head element). + body = b" Date: Tue, 17 Oct 2023 17:49:22 -0300 Subject: [PATCH 35/60] Remove deprecated scrapy.downloadermiddlewares.decompression --- scrapy/downloadermiddlewares/decompression.py | 94 ------------------- ...test_downloadermiddleware_decompression.py | 53 ----------- 2 files changed, 147 deletions(-) delete mode 100644 scrapy/downloadermiddlewares/decompression.py delete mode 100644 tests/test_downloadermiddleware_decompression.py diff --git a/scrapy/downloadermiddlewares/decompression.py b/scrapy/downloadermiddlewares/decompression.py deleted file mode 100644 index 3b8702419..000000000 --- a/scrapy/downloadermiddlewares/decompression.py +++ /dev/null @@ -1,94 +0,0 @@ -""" This module implements the DecompressionMiddleware which tries to recognise -and extract the potentially compressed responses that may arrive. -""" - -import bz2 -import gzip -import logging -import tarfile -import zipfile -from io import BytesIO -from tempfile import mktemp -from warnings import warn - -from scrapy.exceptions import ScrapyDeprecationWarning -from scrapy.responsetypes import responsetypes - -warn( - "scrapy.downloadermiddlewares.decompression is deprecated", - ScrapyDeprecationWarning, - stacklevel=2, -) - - -logger = logging.getLogger(__name__) - - -class DecompressionMiddleware: - """This middleware tries to recognise and extract the possibly compressed - responses that may arrive.""" - - def __init__(self): - self._formats = { - "tar": self._is_tar, - "zip": self._is_zip, - "gz": self._is_gzip, - "bz2": self._is_bzip2, - } - - def _is_tar(self, response): - archive = BytesIO(response.body) - try: - tar_file = tarfile.open(name=mktemp(), fileobj=archive) - except tarfile.ReadError: - return - - body = tar_file.extractfile(tar_file.members[0]).read() - respcls = responsetypes.from_args(filename=tar_file.members[0].name, body=body) - return response.replace(body=body, cls=respcls) - - def _is_zip(self, response): - archive = BytesIO(response.body) - try: - zip_file = zipfile.ZipFile(archive) - except zipfile.BadZipFile: - return - - namelist = zip_file.namelist() - body = zip_file.read(namelist[0]) - respcls = responsetypes.from_args(filename=namelist[0], body=body) - return response.replace(body=body, cls=respcls) - - def _is_gzip(self, response): - archive = BytesIO(response.body) - try: - body = gzip.GzipFile(fileobj=archive).read() - except OSError: - return - - respcls = responsetypes.from_args(body=body) - return response.replace(body=body, cls=respcls) - - def _is_bzip2(self, response): - try: - body = bz2.decompress(response.body) - except OSError: - return - - respcls = responsetypes.from_args(body=body) - return response.replace(body=body, cls=respcls) - - def process_response(self, request, response, spider): - if not response.body: - return response - - for fmt, func in self._formats.items(): - new_response = func(response) - if new_response: - logger.debug( - "Decompressed response with format: %(responsefmt)s", - {"responsefmt": fmt}, - extra={"spider": spider}, - ) - return new_response - return response diff --git a/tests/test_downloadermiddleware_decompression.py b/tests/test_downloadermiddleware_decompression.py deleted file mode 100644 index 95739414e..000000000 --- a/tests/test_downloadermiddleware_decompression.py +++ /dev/null @@ -1,53 +0,0 @@ -from unittest import TestCase, main - -from scrapy.downloadermiddlewares.decompression import DecompressionMiddleware -from scrapy.http import Response, XmlResponse -from scrapy.spiders import Spider -from scrapy.utils.test import assert_samelines -from tests import get_testdata - - -def _test_data(formats): - uncompressed_body = get_testdata("compressed", "feed-sample1.xml") - test_responses = {} - for format in formats: - body = get_testdata("compressed", "feed-sample1." + format) - test_responses[format] = Response("http://foo.com/bar", body=body) - return uncompressed_body, test_responses - - -class DecompressionMiddlewareTest(TestCase): - test_formats = ["tar", "xml.bz2", "xml.gz", "zip"] - uncompressed_body, test_responses = _test_data(test_formats) - - def setUp(self): - self.mw = DecompressionMiddleware() - self.spider = Spider("foo") - - def test_known_compression_formats(self): - for fmt in self.test_formats: - rsp = self.test_responses[fmt] - new = self.mw.process_response(None, rsp, self.spider) - error_msg = f"Failed {fmt}, response type {type(new).__name__}" - assert isinstance(new, XmlResponse), error_msg - assert_samelines(self, new.body, self.uncompressed_body, fmt) - - def test_plain_response(self): - rsp = Response(url="http://test.com", body=self.uncompressed_body) - new = self.mw.process_response(None, rsp, self.spider) - assert new is rsp - assert_samelines(self, new.body, rsp.body) - - def test_empty_response(self): - rsp = Response(url="http://test.com", body=b"") - new = self.mw.process_response(None, rsp, self.spider) - assert new is rsp - assert not rsp.body - assert not new.body - - def tearDown(self): - del self.mw - - -if __name__ == "__main__": - main() From 12b10a7a6427c43968cc18d98a3ed3c6366eeabd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 13 Dec 2023 13:35:05 +0100 Subject: [PATCH 36/60] Cover scrapy.downloadermiddlewares.decompression in the release notes --- docs/news.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index c4081b99b..a12bda53f 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -16,6 +16,10 @@ Scrapy 2.11.1 (unreleased) .. _7j7m-v7m3-jqm7 security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-7j7m-v7m3-jqm7 +- Also in relation with the `7j7m-v7m3-jqm7 security advisory`_, the + deprecated ``scrapy.downloadermiddlewares.decompression`` module has been + removed. + .. _release-2.11.0: @@ -2896,6 +2900,10 @@ Scrapy 1.8.4 (unreleased) to the decompressed response body. Please, see the `7j7m-v7m3-jqm7 security advisory`_ for more information. +- Also in relation with the `7j7m-v7m3-jqm7 security advisory`_, use of the + ``scrapy.downloadermiddlewares.decompression`` module is discouraged and + will trigger a warning. + .. _release-1.8.3: From 4f72b49f975a406784779dc19ede31364f92235f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 Dec 2023 10:06:13 +0100 Subject: [PATCH 37/60] Fix namespaces nodename support for xmliter_lxml --- scrapy/utils/iterators.py | 23 +++++++++++++++++++++-- tests/test_utils_iterators.py | 5 ----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index 6b89334e0..9c53ab524 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -12,11 +12,14 @@ from typing import ( List, Literal, Optional, + Tuple, Union, cast, overload, ) +from lxml import etree + from scrapy.http import Response, TextResponse from scrapy.selector import Selector from scrapy.utils.python import re_rsearch, to_unicode @@ -77,15 +80,31 @@ def xmliter( yield Selector(text=nodetext, type="xml") +def _resolve_xml_namespace(element_name: str, data: bytes) -> Tuple[str, str]: + if ":" not in element_name: + return element_name, None, None + reader: "SupportsReadClose[bytes]" = _StreamReader(data) + node_prefix, element_name = element_name.split(":", maxsplit=1) + ns_iterator = etree.iterparse( + reader, encoding=reader.encoding, events=("start-ns",) + ) + for event, (_prefix, _namespace) in ns_iterator: + if _prefix != node_prefix: + continue + return element_name, _prefix, _namespace + return f"{node_prefix}:{element_name}", None, None + + def xmliter_lxml( obj: Union[Response, str, bytes], nodename: str, namespace: Optional[str] = None, prefix: str = "x", ) -> Generator[Selector, Any, None]: - from lxml import etree + if not namespace: + nodename, prefix, namespace = _resolve_xml_namespace(nodename, obj) - reader = _StreamReader(obj) + reader: "SupportsReadClose[bytes]" = _StreamReader(obj) tag = f"{{{namespace}}}{nodename}" if namespace else nodename iterable = etree.iterparse( cast("SupportsReadClose[bytes]", reader), tag=tag, encoding=reader.encoding diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py index 3598fa0bb..24f03155b 100644 --- a/tests/test_utils_iterators.py +++ b/tests/test_utils_iterators.py @@ -1,4 +1,3 @@ -from pytest import mark from twisted.trial import unittest from scrapy.http import Response, TextResponse, XmlResponse @@ -247,10 +246,6 @@ class XmliterTestCase(unittest.TestCase): class LxmlXmliterTestCase(XmliterTestCase): xmliter = staticmethod(xmliter_lxml) - @mark.xfail(reason="known bug of the current implementation") - def test_xmliter_namespaced_nodename(self): - super().test_xmliter_namespaced_nodename() - def test_xmliter_iterate_namespace(self): body = b""" From d50f436a73ef13fca8d3d9c302ae1a48e984a4a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 Dec 2023 10:08:45 +0100 Subject: [PATCH 38/60] Enable huge_tree for xmliter_lxml --- scrapy/utils/iterators.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index 9c53ab524..1c51c0c6a 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -86,7 +86,10 @@ def _resolve_xml_namespace(element_name: str, data: bytes) -> Tuple[str, str]: reader: "SupportsReadClose[bytes]" = _StreamReader(data) node_prefix, element_name = element_name.split(":", maxsplit=1) ns_iterator = etree.iterparse( - reader, encoding=reader.encoding, events=("start-ns",) + reader, + encoding=reader.encoding, + events=("start-ns",), + huge_tree=True, ) for event, (_prefix, _namespace) in ns_iterator: if _prefix != node_prefix: @@ -107,7 +110,10 @@ def xmliter_lxml( reader: "SupportsReadClose[bytes]" = _StreamReader(obj) tag = f"{{{namespace}}}{nodename}" if namespace else nodename iterable = etree.iterparse( - cast("SupportsReadClose[bytes]", reader), tag=tag, encoding=reader.encoding + reader, + tag=tag, + encoding=reader.encoding, + huge_tree=True, ) selxpath = "//" + (f"{prefix}:{nodename}" if namespace else nodename) for _, node in iterable: From 9655b0b8eb4bbc66b0fe540a19265b6342cf371b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 Dec 2023 10:19:47 +0100 Subject: [PATCH 39/60] Mark slow tests, with their own tox env and CI job --- .github/workflows/tests-ubuntu.yml | 6 ++++ pytest.ini | 2 ++ tests/test_utils_response.py | 50 +++++++++++++++++------------- tox.ini | 6 ++++ 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index 5ff92a571..7562cf22b 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -47,6 +47,9 @@ jobs: - python-version: "3.11" env: TOXENV: botocore + - python-version: "3.11" + env: + TOXENV: slow - python-version: "3.12.0-rc.2" env: @@ -57,6 +60,9 @@ jobs: - python-version: "3.12.0-rc.2" env: TOXENV: extra-deps + - python-version: "3.12.0-rc.2" + env: + TOXENV: slow steps: - uses: actions/checkout@v3 diff --git a/pytest.ini b/pytest.ini index 16983be5e..877fbcd1d 100644 --- a/pytest.ini +++ b/pytest.ini @@ -17,10 +17,12 @@ addopts = --ignore=docs/topics/stats.rst --ignore=docs/topics/telnetconsole.rst --ignore=docs/utils + -m 'not slow' markers = only_asyncio: marks tests as only enabled when --reactor=asyncio is passed only_not_asyncio: marks tests as only enabled when --reactor=asyncio is not passed requires_uvloop: marks tests as only enabled when uvloop is known to be working + slow: marks tests as slow, not executed by default filterwarnings = ignore:scrapy.downloadermiddlewares.decompression is deprecated ignore:Module scrapy.utils.reqser is deprecated diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index 942584d92..93b9bacaf 100644 --- a/tests/test_utils_response.py +++ b/tests/test_utils_response.py @@ -4,6 +4,8 @@ from pathlib import Path from time import process_time from urllib.parse import urlparse +import pytest + from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import HtmlResponse, Response, TextResponse from scrapy.settings.default_settings import DOWNLOAD_MAXSIZE @@ -201,36 +203,40 @@ class ResponseUtilsTest(unittest.TestCase): r5, _openfunc=check_base_url ), "Inject unique base url with conditional comment" - def test_open_in_browser_redos_comment(self): - MAX_CPU_TIME = 30 - # Exploit input from - # https://makenowjust-labs.github.io/recheck/playground/ - # for // (old pattern to remove comments). - body = b"->" +@pytest.mark.slow +def test_open_in_browser_redos_comment(): + MAX_CPU_TIME = 30 - response = HtmlResponse("https://example.com", body=body) + # Exploit input from + # https://makenowjust-labs.github.io/recheck/playground/ + # for // (old pattern to remove comments). + body = b"->" - start_time = process_time() + response = HtmlResponse("https://example.com", body=body) - open_in_browser(response, lambda url: True) + start_time = process_time() - end_time = process_time() - self.assertLess(end_time - start_time, MAX_CPU_TIME) + open_in_browser(response, lambda url: True) - def test_open_in_browser_redos_head(self): - MAX_CPU_TIME = 15 + end_time = process_time() + assert (end_time - start_time) < MAX_CPU_TIME - # Exploit input from - # https://makenowjust-labs.github.io/recheck/playground/ - # for /(|\s.*?>))/ (old pattern to find the head element). - body = b"|\s.*?>))/ (old pattern to find the head element). + body = b" Date: Fri, 15 Dec 2023 10:23:24 +0100 Subject: [PATCH 40/60] Restore the implementation of xmliter --- scrapy/utils/iterators.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index 1c51c0c6a..b67029433 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -43,10 +43,10 @@ def xmliter( """ nodename_patt = re.escape(nodename) - DOCUMENT_HEADER_RE = re.compile(r"<\?xml[^>]{1,1024}>\s*", re.S) + DOCUMENT_HEADER_RE = re.compile(r"<\?xml[^>]+>\s*", re.S) HEADER_END_RE = re.compile(rf"<\s*/{nodename_patt}\s*>", re.S) - END_TAG_RE = re.compile(r"<\s*/([^\s>]{1,1024})\s*>", re.S) - NAMESPACE_RE = re.compile(r"((xmlns[:A-Za-z]{,1024})=[^>\s]+)", re.S) + END_TAG_RE = re.compile(r"<\s*/([^\s>]+)\s*>", re.S) + NAMESPACE_RE = re.compile(r"((xmlns[:A-Za-z]*)=[^>\s]+)", re.S) text = _body_or_str(obj) document_header_match = re.search(DOCUMENT_HEADER_RE, text) @@ -60,15 +60,13 @@ def xmliter( for tagname in reversed(re.findall(END_TAG_RE, header_end)): assert header_end_idx tag = re.search( - rf"<\s*{tagname}.{{,1024}}?xmlns[:=][^>]{{,1024}}>", - text[: header_end_idx[1]], - re.S, + rf"<\s*{tagname}.*?xmlns[:=][^>]*>", text[: header_end_idx[1]], re.S ) if tag: for x in re.findall(NAMESPACE_RE, tag.group()): namespaces[x[1]] = x[0] - r = re.compile(rf"<{nodename_patt}[\s>].{{,1024}}?", re.DOTALL) + r = re.compile(rf"<{nodename_patt}[\s>].*?", re.DOTALL) for match in r.finditer(text): nodetext = ( document_header From 150d96764b5a455c75315596ca8ba5ded0f416dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 Dec 2023 11:42:55 +0100 Subject: [PATCH 41/60] Deprecate xmliter in favor of xmliter_lxml --- docs/faq.rst | 10 ++++--- docs/news.rst | 32 +++++++++++++++++----- scrapy/spiders/feed.py | 4 +-- scrapy/utils/iterators.py | 23 ++++++++++++++-- tests/test_utils_iterators.py | 35 +++++++++++++++++++++--- tests/test_utils_response.py | 50 +++++++++++++++++------------------ 6 files changed, 110 insertions(+), 44 deletions(-) diff --git a/docs/faq.rst b/docs/faq.rst index 20dd814df..657802fd3 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -297,9 +297,13 @@ build the DOM of the entire feed in memory, and this can be quite slow and consume a lot of memory. In order to avoid parsing all the entire feed at once in memory, you can use -the functions ``xmliter`` and ``csviter`` from ``scrapy.utils.iterators`` -module. In fact, this is what the feed spiders (see :ref:`topics-spiders`) use -under the cover. +the :func:`~scrapy.utils.iterators.xmliter_lxml` and +:func:`~scrapy.utils.iterators.csviter` functions. In fact, this is what +:class:`~scrapy.spiders.XMLFeedSpider` uses. + +.. autofunction:: scrapy.utils.iterators.xmliter_lxml + +.. autofunction:: scrapy.utils.iterators.csviter Does Scrapy manage cookies automatically? ----------------------------------------- diff --git a/docs/news.rst b/docs/news.rst index c14815d06..57b99e94f 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -10,12 +10,23 @@ Scrapy 2.11.1 (unreleased) **Security bug fix:** -- Fixed regular expressions susceptible to a `ReDoS attack`_ affecting the - ``iternodes`` node iterator of :class:`~scrapy.spiders.XMLFeedSpider` and - the :func:`~scrapy.utils.response.open_in_browser` function. Please, see - the `cc65-xxvf-f7r9 security advisory`_ for more information. +- Addressed `ReDoS vulnerabilities`_: - .. _ReDoS attack: https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS + - ``scrapy.utils.iterators.xmliter`` is now deprecated in favor of + :func:`~scrapy.utils.iterators.xmliter_lxml`, which + :class:`~scrapy.spiders.XMLFeedSpider` now uses. + + To minimize the impact of this change on existing code, + :func:`~scrapy.utils.iterators.xmliter_lxml` now supports indicating + the node namespace with a prefix in the node name, and big files with + highly nested trees. + + - Fixed regular expressions in the implementation of the + :func:`~scrapy.utils.response.open_in_browser` function. + + Please, see the `cc65-xxvf-f7r9 security advisory`_ for more information. + + .. _ReDoS vulnerabilities: https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS .. _cc65-xxvf-f7r9 security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-cc65-xxvf-f7r9 @@ -2892,8 +2903,15 @@ Scrapy 1.8.4 (unreleased) **Security bug fix:** -- Fixed regular expressions susceptible to a `ReDoS attack`_ affecting the - ``iternodes`` node iterator of :class:`~scrapy.spiders.XMLFeedSpider`. +- Due to its `ReDoS vulnerabilities`_, ``scrapy.utils.iterators.xmliter`` is + now deprecated in favor of :func:`~scrapy.utils.iterators.xmliter_lxml`, + which :class:`~scrapy.spiders.XMLFeedSpider` now uses. + + To minimize the impact of this change on existing code, + :func:`~scrapy.utils.iterators.xmliter_lxml` now supports indicating + the node namespace as a prefix in the node name, and big files with highly + nested trees when using lxml 4.2 or later. + Please, see the `cc65-xxvf-f7r9 security advisory`_ for more information. diff --git a/scrapy/spiders/feed.py b/scrapy/spiders/feed.py index 6afadc577..42675c76a 100644 --- a/scrapy/spiders/feed.py +++ b/scrapy/spiders/feed.py @@ -7,7 +7,7 @@ See documentation in docs/topics/spiders.rst from scrapy.exceptions import NotConfigured, NotSupported from scrapy.selector import Selector from scrapy.spiders import Spider -from scrapy.utils.iterators import csviter, xmliter +from scrapy.utils.iterators import csviter, xmliter_lxml from scrapy.utils.spider import iterate_spider_output @@ -84,7 +84,7 @@ class XMLFeedSpider(Spider): return self.parse_nodes(response, nodes) def _iternodes(self, response): - for node in xmliter(response, self.itertag): + for node in xmliter_lxml(response, self.itertag): self._register_namespaces(node) yield node diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index b67029433..7574e377a 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -17,9 +17,12 @@ from typing import ( cast, overload, ) +from warnings import warn from lxml import etree +from packaging.version import Version +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Response, TextResponse from scrapy.selector import Selector from scrapy.utils.python import re_rsearch, to_unicode @@ -29,6 +32,12 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) +_LXML_VERSION = Version(etree.__version__) +_LXML_HUGE_TREE_VERSION = Version("4.2") +_ITERPARSE_KWARGS = {} +if _LXML_VERSION >= _LXML_HUGE_TREE_VERSION: + _ITERPARSE_KWARGS["huge_tree"] = True + def xmliter( obj: Union[Response, str, bytes], nodename: str @@ -41,6 +50,16 @@ def xmliter( - a unicode string - a string encoded as utf-8 """ + warn( + ( + "xmliter is deprecated and its use strongly discouraged because " + "it is vulnerable to ReDoS attacks. Use xmliter_lxml instead. See " + "https://github.com/scrapy/scrapy/security/advisories/GHSA-cc65-xxvf-f7r9" + ), + ScrapyDeprecationWarning, + stacklevel=2, + ) + nodename_patt = re.escape(nodename) DOCUMENT_HEADER_RE = re.compile(r"<\?xml[^>]+>\s*", re.S) @@ -87,7 +106,7 @@ def _resolve_xml_namespace(element_name: str, data: bytes) -> Tuple[str, str]: reader, encoding=reader.encoding, events=("start-ns",), - huge_tree=True, + **_ITERPARSE_KWARGS, ) for event, (_prefix, _namespace) in ns_iterator: if _prefix != node_prefix: @@ -111,7 +130,7 @@ def xmliter_lxml( reader, tag=tag, encoding=reader.encoding, - huge_tree=True, + **_ITERPARSE_KWARGS, ) selxpath = "//" + (f"{prefix}:{nodename}" if namespace else nodename) for _, node in iterable: diff --git a/tests/test_utils_iterators.py b/tests/test_utils_iterators.py index 24f03155b..505cc276c 100644 --- a/tests/test_utils_iterators.py +++ b/tests/test_utils_iterators.py @@ -1,13 +1,14 @@ +import pytest from twisted.trial import unittest +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Response, TextResponse, XmlResponse from scrapy.utils.iterators import _body_or_str, csviter, xmliter, xmliter_lxml from tests import get_testdata -class XmliterTestCase(unittest.TestCase): - xmliter = staticmethod(xmliter) - +class XmliterBaseTestCase: + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter(self): body = b""" @@ -39,6 +40,7 @@ class XmliterTestCase(unittest.TestCase): attrs, [("001", ["Name 1"], ["Type 1"]), ("002", ["Name 2"], ["Type 2"])] ) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_unusual_node(self): body = b""" @@ -52,6 +54,7 @@ class XmliterTestCase(unittest.TestCase): ] self.assertEqual(nodenames, [["matchme..."]]) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_unicode(self): # example taken from https://github.com/scrapy/scrapy/issues/1665 body = """ @@ -111,6 +114,7 @@ class XmliterTestCase(unittest.TestCase): [("26", ["-"], ["80"]), ("21", ["Ab"], ["76"]), ("27", ["A"], ["27"])], ) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_text(self): body = ( '' @@ -122,6 +126,7 @@ class XmliterTestCase(unittest.TestCase): [["one"], ["two"]], ) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_namespaces(self): body = b""" @@ -161,6 +166,7 @@ class XmliterTestCase(unittest.TestCase): self.assertEqual(node.xpath("id/text()").getall(), []) self.assertEqual(node.xpath("price/text()").getall(), []) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_namespaced_nodename(self): body = b""" @@ -189,6 +195,7 @@ class XmliterTestCase(unittest.TestCase): ["http://www.mydummycompany.com/images/item1.jpg"], ) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_namespaced_nodename_missing(self): body = b""" @@ -213,6 +220,7 @@ class XmliterTestCase(unittest.TestCase): with self.assertRaises(StopIteration): next(my_iter) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_exception(self): body = ( '' @@ -225,10 +233,12 @@ class XmliterTestCase(unittest.TestCase): self.assertRaises(StopIteration, next, iter) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_objtype_exception(self): i = self.xmliter(42, "product") self.assertRaises(TypeError, next, i) + @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") def test_xmliter_encoding(self): body = ( b'\n' @@ -243,7 +253,24 @@ class XmliterTestCase(unittest.TestCase): ) -class LxmlXmliterTestCase(XmliterTestCase): +class XmliterTestCase(XmliterBaseTestCase, unittest.TestCase): + xmliter = staticmethod(xmliter) + + def test_deprecation(self): + body = b""" + + + + + """ + with pytest.warns( + ScrapyDeprecationWarning, + match="xmliter", + ): + next(self.xmliter(body, "product")) + + +class LxmlXmliterTestCase(XmliterBaseTestCase, unittest.TestCase): xmliter = staticmethod(xmliter_lxml) def test_xmliter_iterate_namespace(self): diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index 93b9bacaf..1dbe187bf 100644 --- a/tests/test_utils_response.py +++ b/tests/test_utils_response.py @@ -203,40 +203,38 @@ class ResponseUtilsTest(unittest.TestCase): r5, _openfunc=check_base_url ), "Inject unique base url with conditional comment" + @pytest.mark.slow + def test_open_in_browser_redos_comment(self): + MAX_CPU_TIME = 30 -@pytest.mark.slow -def test_open_in_browser_redos_comment(): - MAX_CPU_TIME = 30 + # Exploit input from + # https://makenowjust-labs.github.io/recheck/playground/ + # for // (old pattern to remove comments). + body = b"->" - # Exploit input from - # https://makenowjust-labs.github.io/recheck/playground/ - # for // (old pattern to remove comments). - body = b"->" + response = HtmlResponse("https://example.com", body=body) - response = HtmlResponse("https://example.com", body=body) + start_time = process_time() - start_time = process_time() + open_in_browser(response, lambda url: True) - open_in_browser(response, lambda url: True) + end_time = process_time() + self.assertLess(end_time - start_time, MAX_CPU_TIME) - end_time = process_time() - assert (end_time - start_time) < MAX_CPU_TIME + @pytest.mark.slow + def test_open_in_browser_redos_head(self): + MAX_CPU_TIME = 15 + # Exploit input from + # https://makenowjust-labs.github.io/recheck/playground/ + # for /(|\s.*?>))/ (old pattern to find the head element). + body = b"|\s.*?>))/ (old pattern to find the head element). - body = b" Date: Fri, 15 Dec 2023 11:49:22 +0100 Subject: [PATCH 42/60] Minor naming changes --- scrapy/utils/iterators.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index 7574e377a..f239630f4 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -101,18 +101,18 @@ def _resolve_xml_namespace(element_name: str, data: bytes) -> Tuple[str, str]: if ":" not in element_name: return element_name, None, None reader: "SupportsReadClose[bytes]" = _StreamReader(data) - node_prefix, element_name = element_name.split(":", maxsplit=1) + input_prefix, element_name = element_name.split(":", maxsplit=1) ns_iterator = etree.iterparse( reader, encoding=reader.encoding, events=("start-ns",), **_ITERPARSE_KWARGS, ) - for event, (_prefix, _namespace) in ns_iterator: - if _prefix != node_prefix: + for event, (prefix, namespace) in ns_iterator: + if prefix != input_prefix: continue - return element_name, _prefix, _namespace - return f"{node_prefix}:{element_name}", None, None + return element_name, prefix, namespace + return f"{input_prefix}:{element_name}", None, None def xmliter_lxml( From a49c8762dd163b60cc73c4486a662471cfa7ac7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 Dec 2023 12:14:53 +0100 Subject: [PATCH 43/60] Avoid calling iterparse twice --- scrapy/utils/iterators.py | 42 +++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index f239630f4..8610e9b77 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -12,7 +12,6 @@ from typing import ( List, Literal, Optional, - Tuple, Union, cast, overload, @@ -97,43 +96,38 @@ def xmliter( yield Selector(text=nodetext, type="xml") -def _resolve_xml_namespace(element_name: str, data: bytes) -> Tuple[str, str]: - if ":" not in element_name: - return element_name, None, None - reader: "SupportsReadClose[bytes]" = _StreamReader(data) - input_prefix, element_name = element_name.split(":", maxsplit=1) - ns_iterator = etree.iterparse( - reader, - encoding=reader.encoding, - events=("start-ns",), - **_ITERPARSE_KWARGS, - ) - for event, (prefix, namespace) in ns_iterator: - if prefix != input_prefix: - continue - return element_name, prefix, namespace - return f"{input_prefix}:{element_name}", None, None - - def xmliter_lxml( obj: Union[Response, str, bytes], nodename: str, namespace: Optional[str] = None, prefix: str = "x", ) -> Generator[Selector, Any, None]: - if not namespace: - nodename, prefix, namespace = _resolve_xml_namespace(nodename, obj) - reader: "SupportsReadClose[bytes]" = _StreamReader(obj) tag = f"{{{namespace}}}{nodename}" if namespace else nodename iterable = etree.iterparse( reader, - tag=tag, encoding=reader.encoding, + events=("end", "start-ns"), **_ITERPARSE_KWARGS, ) selxpath = "//" + (f"{prefix}:{nodename}" if namespace else nodename) - for _, node in iterable: + needs_namespace_resolution = not namespace and ":" in nodename + if needs_namespace_resolution: + prefix, nodename = nodename.split(":", maxsplit=1) + for event, data in iterable: + if event == "start-ns": + if needs_namespace_resolution: + _prefix, _namespace = data + if _prefix != prefix: + continue + namespace = _namespace + needs_namespace_resolution = False + selxpath = f"//{prefix}:{nodename}" + tag = f"{{{namespace}}}{nodename}" + continue + node = data + if node.tag != tag: + continue nodetext = etree.tostring(node, encoding="unicode") node.clear() xs = Selector(text=nodetext, type="xml") From ce9d290eff8b5992023ffa5e833881b83a0669c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 Dec 2023 12:23:26 +0100 Subject: [PATCH 44/60] Remove the lxml version check for huge_tree on xmliter_lxml iterparse supports the option since lxml 2.2.1, it was the HTML parser that only got it in 4.2 --- docs/news.rst | 2 +- scrapy/utils/iterators.py | 9 +-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 525ddbf40..fab8b6f20 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -19,7 +19,7 @@ Scrapy 2.11.1 (unreleased) To minimize the impact of this change on existing code, :func:`~scrapy.utils.iterators.xmliter_lxml` now supports indicating the node namespace with a prefix in the node name, and big files with - highly nested trees. + highly nested trees when using libxml2 2.7+. - Fixed regular expressions in the implementation of the :func:`~scrapy.utils.response.open_in_browser` function. diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index 8610e9b77..b6abe2e0c 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -19,7 +19,6 @@ from typing import ( from warnings import warn from lxml import etree -from packaging.version import Version from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Response, TextResponse @@ -31,12 +30,6 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -_LXML_VERSION = Version(etree.__version__) -_LXML_HUGE_TREE_VERSION = Version("4.2") -_ITERPARSE_KWARGS = {} -if _LXML_VERSION >= _LXML_HUGE_TREE_VERSION: - _ITERPARSE_KWARGS["huge_tree"] = True - def xmliter( obj: Union[Response, str, bytes], nodename: str @@ -108,7 +101,7 @@ def xmliter_lxml( reader, encoding=reader.encoding, events=("end", "start-ns"), - **_ITERPARSE_KWARGS, + huge_tree=True, ) selxpath = "//" + (f"{prefix}:{nodename}" if namespace else nodename) needs_namespace_resolution = not namespace and ":" in nodename From bc138ef8e958f4bac5a4413d40566efc2b59acfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 Dec 2023 12:24:04 +0100 Subject: [PATCH 45/60] Minor release notes fix --- docs/news.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/news.rst b/docs/news.rst index fab8b6f20..f346d1239 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -2912,7 +2912,7 @@ Scrapy 1.8.4 (unreleased) To minimize the impact of this change on existing code, :func:`~scrapy.utils.iterators.xmliter_lxml` now supports indicating the node namespace as a prefix in the node name, and big files with highly - nested trees when using lxml 4.2 or later. + nested trees when using libxml2 2.7+. Please, see the `cc65-xxvf-f7r9 security advisory`_ for more information. From c7c7a488b950806888691f58dda0b06478b98c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 Dec 2023 13:18:23 +0100 Subject: [PATCH 46/60] Fix typing issues --- scrapy/utils/iterators.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scrapy/utils/iterators.py b/scrapy/utils/iterators.py index b6abe2e0c..ab48e525f 100644 --- a/scrapy/utils/iterators.py +++ b/scrapy/utils/iterators.py @@ -95,10 +95,10 @@ def xmliter_lxml( namespace: Optional[str] = None, prefix: str = "x", ) -> Generator[Selector, Any, None]: - reader: "SupportsReadClose[bytes]" = _StreamReader(obj) + reader = _StreamReader(obj) tag = f"{{{namespace}}}{nodename}" if namespace else nodename iterable = etree.iterparse( - reader, + cast("SupportsReadClose[bytes]", reader), encoding=reader.encoding, events=("end", "start-ns"), huge_tree=True, @@ -109,6 +109,7 @@ def xmliter_lxml( prefix, nodename = nodename.split(":", maxsplit=1) for event, data in iterable: if event == "start-ns": + assert isinstance(data, tuple) if needs_namespace_resolution: _prefix, _namespace = data if _prefix != prefix: @@ -118,6 +119,7 @@ def xmliter_lxml( selxpath = f"//{prefix}:{nodename}" tag = f"{{{namespace}}}{nodename}" continue + assert isinstance(data, etree._Element) node = data if node.tag != tag: continue From 27781a85e738052e0441c81d773b3ec124194594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 15 Dec 2023 13:52:12 +0100 Subject: [PATCH 47/60] Fix bad closing tags in XMLFeedSpider tests --- tests/test_spider.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_spider.py b/tests/test_spider.py index 00da3d485..5c4007d87 100644 --- a/tests/test_spider.py +++ b/tests/test_spider.py @@ -149,10 +149,10 @@ class XMLFeedSpiderTest(SpiderTest): body = b""" - http://www.example.com/Special-Offers.html2009-08-16 + http://www.example.com/Special-Offers.html2009-08-16 - http://www.example.com/2009-08-16 + http://www.example.com/2009-08-16 """ response = XmlResponse(url="http://example.com/sitemap.xml", body=body) From b095dd218fe64f2541079d691e3c2c68d2e03ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 30 Nov 2023 10:54:09 +0100 Subject: [PATCH 48/60] Extend Request.meta documentation (#5565) --- docs/topics/request-response.rst | 47 ++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index adf3d0f4a..8edf710bc 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -193,18 +193,47 @@ Request objects :meth:`replace`. .. attribute:: Request.meta + :value: {} - A dict that contains arbitrary metadata for this request. This dict is - empty for new Requests, and is usually populated by different Scrapy - components (extensions, middlewares, etc). So the data contained in this - dict depends on the extensions you have enabled. + A dictionary of arbitrary metadata for the request. - See :ref:`topics-request-meta` for a list of special meta keys - recognized by Scrapy. + You may extend request metadata as you see fit. - This dict is :doc:`shallow copied ` when the request is - cloned using the ``copy()`` or ``replace()`` methods, and can also be - accessed, in your spider, from the ``response.meta`` attribute. + Request metadata can also be accessed through the + :attr:`~scrapy.http.Response.meta` attribute of a response. + + To pass data from one spider callback to another, consider using + :attr:`cb_kwargs` instead. However, request metadata may be the right + choice in certain scenarios, such as to maintain some debugging data + across all follow-up requests (e.g. the source URL). + + A common use of request metadata is to define request-specific + parameters for Scrapy components (extensions, middlewares, etc.). For + example, if you set ``dont_retry`` to ``True``, + :class:`~scrapy.downloadermiddlewares.retry.RetryMiddleware` will never + retry that request, even if it fails. See :ref:`topics-request-meta`. + + You may also use request metadata in your custom Scrapy components, for + example, to keep request state information relevant to your component. + For example, + :class:`~scrapy.downloadermiddlewares.retry.RetryMiddleware` uses the + ``retry_times`` metadata key to keep track of how many times a request + has been retried so far. + + Copying all the metadata of a previous request into a new, follow-up + request in a spider callback is a bad practice, because request + metadata may include metadata set by Scrapy components that is not + meant to be copied into other requests. For example, copying the + ``retry_times`` metadata key into follow-up requests can lower the + amount of retries allowed for those follow-up requests. + + You should only copy all request metadata from one request to another + if the new request is meant to replace the old request, as is often the + case when returning a request from a :ref:`downloader middleware + ` method. + + Also mind that the :meth:`copy` and :meth:`replace` request methods + :doc:`shallow-copy ` request metadata. .. attribute:: Request.cb_kwargs From 369712ee50f7438c2863359f053cb1b221a42169 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 30 Nov 2023 11:01:22 +0100 Subject: [PATCH 49/60] =?UTF-8?q?SPM=20=E2=86=92=20Zyte=20API=20(#6163)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/topics/practices.rst | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index f64da22d8..b1b8c9e9c 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -288,9 +288,8 @@ Here are some tips to keep in mind when dealing with these kinds of sites: * use a pool of rotating IPs. For example, the free `Tor project`_ or paid services like `ProxyMesh`_. An open source alternative is `scrapoxy`_, a super proxy that you can attach your own proxies to. -* use a highly distributed downloader that circumvents bans internally, so you - can just focus on parsing clean pages. One example of such downloaders is - `Zyte Smart Proxy Manager`_ +* use a ban avoidance service, such as `Zyte API`_, which provides a `Scrapy + plugin `__ If you are still unable to prevent your bot getting banned, consider contacting `commercial support`_. @@ -301,4 +300,4 @@ If you are still unable to prevent your bot getting banned, consider contacting .. _Common Crawl: https://commoncrawl.org/ .. _testspiders: https://github.com/scrapinghub/testspiders .. _scrapoxy: https://scrapoxy.io/ -.. _Zyte Smart Proxy Manager: https://www.zyte.com/smart-proxy-manager/ +.. _Zyte API: https://docs.zyte.com/zyte-api/get-started.html From 48a9a58ff27d24910b55a0ad5e6b014589c71115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 20 Dec 2023 12:47:18 +0100 Subject: [PATCH 50/60] =?UTF-8?q?Link=20to=20Zyte=E2=80=99s=20export=20gui?= =?UTF-8?q?des?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/topics/feed-exports.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/topics/feed-exports.rst b/docs/topics/feed-exports.rst index 700775e4b..f64bbac06 100644 --- a/docs/topics/feed-exports.rst +++ b/docs/topics/feed-exports.rst @@ -13,6 +13,11 @@ Scrapy provides this functionality out of the box with the Feed Exports, which allows you to generate feeds with the scraped items, using multiple serialization formats and storage backends. +This page provides detailed documentation for all feed export features. If you +are looking for a step-by-step guide, check out `Zyte’s export guides`_. + +.. _Zyte’s export guides: https://docs.zyte.com/web-scraping/guides/export/index.html#exporting-scraped-data + .. _topics-feed-format: Serialization formats From badc7c5be9dcfaf7c8acbb87fa530f0477ec3c35 Mon Sep 17 00:00:00 2001 From: Chan Sau Yee <15137352+y26805@users.noreply.github.com> Date: Fri, 29 Dec 2023 20:32:51 +0900 Subject: [PATCH 51/60] Update black reference in docs (#6192) --- docs/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 2b3249601..d728338da 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -178,7 +178,7 @@ Scrapy: * We use `black `_ for code formatting. There is a hook in the pre-commit config that will automatically format your code before every commit. You can also - run black manually with ``tox -e black``. + run black manually with ``tox -e pre-commit``. * Don't put your name in the code you contribute; git provides enough metadata to identify author of the code. From 6127f7d27824de1f9847f7bb07f9755c955d9c3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 28 Dec 2023 12:25:01 +0100 Subject: [PATCH 52/60] Update quotes.toscrape.com page copies (#6190) --- docs/_tests/quotes.html | 2 +- docs/_tests/quotes1.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/_tests/quotes.html b/docs/_tests/quotes.html index 71aff8847..f4002ecd1 100644 --- a/docs/_tests/quotes.html +++ b/docs/_tests/quotes.html @@ -273,7 +273,7 @@ Quotes by: GoodReads.com

- Made with ❤ by Scrapinghub + Made with ❤ by Zyte

diff --git a/docs/_tests/quotes1.html b/docs/_tests/quotes1.html index 71aff8847..f4002ecd1 100644 --- a/docs/_tests/quotes1.html +++ b/docs/_tests/quotes1.html @@ -273,7 +273,7 @@ Quotes by: GoodReads.com

- Made with ❤ by Scrapinghub + Made with ❤ by Zyte

From 09a7efef7c75558c9ea198a00fc11ab26fb16ce5 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 12 Jan 2024 18:30:41 +0400 Subject: [PATCH 53/60] Remove a defer.returnValue call. --- tests/test_feedexport.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index 56967c0d5..ae5810fb8 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -2300,7 +2300,7 @@ class BatchDeliveriesTest(FeedExportTestBase): content[feed["format"]].append(file.read_bytes()) finally: self.tearDown() - defer.returnValue(content) + return content @defer.inlineCallbacks def assertExportedJsonLines(self, items, rows, settings=None): From c5dad41190551578c2973c34520952f26f75dc7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 2 Feb 2024 14:03:16 +0100 Subject: [PATCH 54/60] Speed up tests, remove comments without regexps --- .github/workflows/tests-ubuntu.yml | 3 -- scrapy/utils/response.py | 14 +++++++- tests/test_utils_response.py | 51 ++++++++++++++++++++++++++---- tox.ini | 6 ---- 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index 388ba9572..338c99584 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -50,9 +50,6 @@ jobs: - python-version: "3.12" env: TOXENV: botocore - - python-version: "3.12" - env: - TOXENV: slow steps: - uses: actions/checkout@v3 diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index 4369e6439..fabfb1167 100644 --- a/scrapy/utils/response.py +++ b/scrapy/utils/response.py @@ -74,6 +74,18 @@ def response_httprepr(response: Response) -> bytes: return b"".join(values) +def _remove_html_comments(body): + start = body.find(b"", start + 1) + if end == -1: + return body[:start] + else: + body = body[:start] + body[end + 3 :] + start = body.find(b"|$)", b"", body) body = re.sub(rb"]*?>)", to_bytes(repl), body, count=1) ext = ".html" elif isinstance(response, TextResponse): diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index 1dbe187bf..db3c31b89 100644 --- a/tests/test_utils_response.py +++ b/tests/test_utils_response.py @@ -8,9 +8,9 @@ import pytest from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import HtmlResponse, Response, TextResponse -from scrapy.settings.default_settings import DOWNLOAD_MAXSIZE from scrapy.utils.python import to_bytes from scrapy.utils.response import ( + _remove_html_comments, get_base_url, get_meta_refresh, open_in_browser, @@ -203,14 +203,13 @@ class ResponseUtilsTest(unittest.TestCase): r5, _openfunc=check_base_url ), "Inject unique base url with conditional comment" - @pytest.mark.slow def test_open_in_browser_redos_comment(self): - MAX_CPU_TIME = 30 + MAX_CPU_TIME = 0.001 # Exploit input from # https://makenowjust-labs.github.io/recheck/playground/ # for // (old pattern to remove comments). - body = b"->" + body = b"->" response = HtmlResponse("https://example.com", body=body) @@ -221,14 +220,13 @@ class ResponseUtilsTest(unittest.TestCase): end_time = process_time() self.assertLess(end_time - start_time, MAX_CPU_TIME) - @pytest.mark.slow def test_open_in_browser_redos_head(self): - MAX_CPU_TIME = 15 + MAX_CPU_TIME = 0.001 # Exploit input from # https://makenowjust-labs.github.io/recheck/playground/ # for /(|\s.*?>))/ (old pattern to find the head element). - body = b"b", + b"ab", + ), + ( + b"ac", + b"ac", + ), + ( + b"acccd", + b"acd", + ), + ( + b"ad", + b"ad", + ), + ), +) +def test_remove_html_comments(input_body, output_body): + assert ( + _remove_html_comments(input_body) == output_body + ), f"{_remove_html_comments(input_body)=} == {output_body=}" diff --git a/tox.ini b/tox.ini index e87d6a175..381da9773 100644 --- a/tox.ini +++ b/tox.ini @@ -221,9 +221,3 @@ setenv = {[pinned]setenv} commands = pytest --cov=scrapy --cov-report=xml --cov-report= {posargs:tests -k s3} - - -[testenv:slow] -basepython = python3 -commands = - {[testenv]commands} -m 'slow' From 810aaa637da12a1f393291eb2b13aa0c8a163efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 2 Feb 2024 14:04:28 +0100 Subject: [PATCH 55/60] Undo an unintended change --- .github/workflows/tests-ubuntu.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests-ubuntu.yml b/.github/workflows/tests-ubuntu.yml index 338c99584..c883f958c 100644 --- a/.github/workflows/tests-ubuntu.yml +++ b/.github/workflows/tests-ubuntu.yml @@ -50,6 +50,7 @@ jobs: - python-version: "3.12" env: TOXENV: botocore + steps: - uses: actions/checkout@v3 From 5e5a92026e43023b80f7733844a2703c3f966009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Fri, 2 Feb 2024 14:06:45 +0100 Subject: [PATCH 56/60] Remove slow leftovers --- pytest.ini | 2 -- 1 file changed, 2 deletions(-) diff --git a/pytest.ini b/pytest.ini index 877fbcd1d..16983be5e 100644 --- a/pytest.ini +++ b/pytest.ini @@ -17,12 +17,10 @@ addopts = --ignore=docs/topics/stats.rst --ignore=docs/topics/telnetconsole.rst --ignore=docs/utils - -m 'not slow' markers = only_asyncio: marks tests as only enabled when --reactor=asyncio is passed only_not_asyncio: marks tests as only enabled when --reactor=asyncio is not passed requires_uvloop: marks tests as only enabled when uvloop is known to be working - slow: marks tests as slow, not executed by default filterwarnings = ignore:scrapy.downloadermiddlewares.decompression is deprecated ignore:Module scrapy.utils.reqser is deprecated From a55e933c11899997757bd4107738f9472d1d3c2e Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 14 Feb 2024 20:08:40 +0400 Subject: [PATCH 57/60] Release notes for 2.11.1 (#6150) --- docs/news.rst | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index 0c202639e..c26cef22c 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -3,6 +3,64 @@ Release notes ============= +.. _release-2.11.1: + +Scrapy 2.11.1 (YYYY-MM-DD) +-------------------------- + +Highlights: + +- Support for Twisted >= 23.8.0. + +- Documentation improvements. + +Modified requirements +~~~~~~~~~~~~~~~~~~~~~ + +- The Twisted dependency is no longer restricted to < 23.8.0. (:issue:`6024`, + :issue:`6064`, :issue:`6142`) + +Bug fixes +~~~~~~~~~ + +- The OS signal handling code was refactored to no longer use private Twisted + functions. (:issue:`6024`, :issue:`6064`, :issue:`6112`) + +Documentation +~~~~~~~~~~~~~ + +- Improved documentation for :class:`~scrapy.crawler.Crawler` initialization + changes made in the 2.11.0 release. (:issue:`6057`, :issue:`6147`) + +- Extended documentation for :attr:`Request.meta `. + (:issue:`5565`) + +- Fixed the :reqmeta:`dont_merge_cookies` documentation. (:issue:`5936`, + :issue:`6077`) + +- Added a link to Zyte's export guides to the :ref:`feed exports + ` documentation. (:issue:`6183`) + +- Added a missing note about backward-incompatible changes in + :class:`~scrapy.exporters.PythonItemExporter` to the 2.11.0 release notes. + (:issue:`6060`, :issue:`6081`) + +- Added a missing note about removing the deprecated + ``scrapy.utils.boto.is_botocore()`` function to the 2.8.0 release notes. + (:issue:`6056`, :issue:`6061`) + +- Other documentation improvements. (:issue:`6128`, :issue:`6144`, + :issue:`6163`, :issue:`6190`, :issue:`6192`) + +Quality assurance +~~~~~~~~~~~~~~~~~ + +- Added Python 3.12 to the CI configuration, re-enabled tests that were + disabled when the pre-release support was added. (:issue:`5985`, + :issue:`6083`, :issue:`6098`) + +- Fixed a test issue on PyPy 7.3.14. (:issue:`6204`, :issue:`6205`) + .. _release-2.11.0: Scrapy 2.11.0 (2023-09-18) From 6b88b3346c393f07c4e4481405c3fd1ab4cc58a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 14 Feb 2024 18:16:40 +0100 Subject: [PATCH 58/60] Set the release date of versions 2.11.1 and 1.8.4 --- docs/news.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 16e7e79a7..518632a5b 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -5,7 +5,7 @@ Release notes .. _release-2.11.1: -Scrapy 2.11.1 (unreleased) +Scrapy 2.11.1 (2024-02-14) -------------------------- Highlights: @@ -2972,7 +2972,7 @@ affect subclasses: .. _release-1.8.4: -Scrapy 1.8.4 (unreleased) +Scrapy 1.8.4 (2024-02-14) ------------------------- **Security bug fixes:** From 502addc717b6b971425a9385359a382b8d0187a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 14 Feb 2024 18:17:48 +0100 Subject: [PATCH 59/60] =?UTF-8?q?Bump=20version:=202.11.0=20=E2=86=92=202.?= =?UTF-8?q?11.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .bumpversion.cfg | 2 +- scrapy/VERSION | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index f76bf783d..6ce6e2a59 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 2.11.0 +current_version = 2.11.1 commit = True tag = True tag_name = {new_version} diff --git a/scrapy/VERSION b/scrapy/VERSION index 46b81d815..6ceb272ee 100644 --- a/scrapy/VERSION +++ b/scrapy/VERSION @@ -1 +1 @@ -2.11.0 +2.11.1 From 2f1d345e74d19e33016f9e69fcda0bda9afb568d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 14 Feb 2024 18:59:01 +0100 Subject: [PATCH 60/60] Solve test issues --- ...st_downloadermiddleware_httpcompression.py | 24 +++++++++++++++++++ tests/test_utils_response.py | 4 ++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/tests/test_downloadermiddleware_httpcompression.py b/tests/test_downloadermiddleware_httpcompression.py index f74fff218..9deb81c37 100644 --- a/tests/test_downloadermiddleware_httpcompression.py +++ b/tests/test_downloadermiddleware_httpcompression.py @@ -402,6 +402,10 @@ class HttpCompressionTest(TestCase): self._test_compression_bomb_setting("gzip") def test_compression_bomb_setting_zstd(self): + try: + import zstandard # noqa: F401 + except ImportError: + raise SkipTest("no zstd support (zstandard)") self._test_compression_bomb_setting("zstd") def _test_compression_bomb_spider_attr(self, compression_id): @@ -436,6 +440,10 @@ class HttpCompressionTest(TestCase): self._test_compression_bomb_spider_attr("gzip") def test_compression_bomb_spider_attr_zstd(self): + try: + import zstandard # noqa: F401 + except ImportError: + raise SkipTest("no zstd support (zstandard)") self._test_compression_bomb_spider_attr("zstd") def _test_compression_bomb_request_meta(self, compression_id): @@ -468,6 +476,10 @@ class HttpCompressionTest(TestCase): self._test_compression_bomb_request_meta("gzip") def test_compression_bomb_request_meta_zstd(self): + try: + import zstandard # noqa: F401 + except ImportError: + raise SkipTest("no zstd support (zstandard)") self._test_compression_bomb_request_meta("zstd") def _test_download_warnsize_setting(self, compression_id): @@ -510,6 +522,10 @@ class HttpCompressionTest(TestCase): self._test_download_warnsize_setting("gzip") def test_download_warnsize_setting_zstd(self): + try: + import zstandard # noqa: F401 + except ImportError: + raise SkipTest("no zstd support (zstandard)") self._test_download_warnsize_setting("zstd") def _test_download_warnsize_spider_attr(self, compression_id): @@ -554,6 +570,10 @@ class HttpCompressionTest(TestCase): self._test_download_warnsize_spider_attr("gzip") def test_download_warnsize_spider_attr_zstd(self): + try: + import zstandard # noqa: F401 + except ImportError: + raise SkipTest("no zstd support (zstandard)") self._test_download_warnsize_spider_attr("zstd") def _test_download_warnsize_request_meta(self, compression_id): @@ -596,6 +616,10 @@ class HttpCompressionTest(TestCase): self._test_download_warnsize_request_meta("gzip") def test_download_warnsize_request_meta_zstd(self): + try: + import zstandard # noqa: F401 + except ImportError: + raise SkipTest("no zstd support (zstandard)") self._test_download_warnsize_request_meta("zstd") diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index db3c31b89..37ef89e76 100644 --- a/tests/test_utils_response.py +++ b/tests/test_utils_response.py @@ -204,7 +204,7 @@ class ResponseUtilsTest(unittest.TestCase): ), "Inject unique base url with conditional comment" def test_open_in_browser_redos_comment(self): - MAX_CPU_TIME = 0.001 + MAX_CPU_TIME = 0.02 # Exploit input from # https://makenowjust-labs.github.io/recheck/playground/ @@ -221,7 +221,7 @@ class ResponseUtilsTest(unittest.TestCase): self.assertLess(end_time - start_time, MAX_CPU_TIME) def test_open_in_browser_redos_head(self): - MAX_CPU_TIME = 0.001 + MAX_CPU_TIME = 0.02 # Exploit input from # https://makenowjust-labs.github.io/recheck/playground/