From 0007676e8d5210f8d0d05ffb12610ca780a3c5ea Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 09:21:12 +0200 Subject: [PATCH 01/28] Improve test coverage for http/ (#7672) * Improve test coverage for http/ * Use isinstance() instead of type() --- tests/test_http_cookies.py | 53 +++++++++++++++++++++++++++++++- tests/test_http_headers.py | 1 + tests/test_http_request.py | 10 ++++++ tests/test_http_request_form.py | 3 ++ tests/test_http_response.py | 5 +++ tests/test_http_response_text.py | 7 +++++ 6 files changed, 78 insertions(+), 1 deletion(-) diff --git a/tests/test_http_cookies.py b/tests/test_http_cookies.py index 660b76d08..ce9296764 100644 --- a/tests/test_http_cookies.py +++ b/tests/test_http_cookies.py @@ -1,8 +1,59 @@ +from http.cookiejar import DefaultCookiePolicy + from scrapy.http import Request, Response -from scrapy.http.cookies import WrappedRequest, WrappedResponse +from scrapy.http.cookies import CookieJar, WrappedRequest, WrappedResponse from scrapy.utils.httpobj import urlparse_cached +class TestCookieJar: + def setup_method(self): + self.jar = CookieJar() + self.request = Request("http://example.com/") + self.response = Response( + "http://example.com/", + headers={"Set-Cookie": "name=value; Domain=example.com; Path=/"}, + ) + + def test_extract_cookies(self): + assert len(self.jar) == 0 + self.jar.extract_cookies(self.response, self.request) + assert len(self.jar) == 1 + cookie = next(iter(self.jar)) + assert cookie.name == "name" + assert cookie.value == "value" + assert ".example.com" in self.jar._cookies + + def test_make_cookies_and_set_cookie(self): + cookies = self.jar.make_cookies(self.response, self.request) + assert len(cookies) == 1 + jar = CookieJar() + for cookie in cookies: + jar.set_cookie(cookie) + assert len(jar) == 1 + + def test_clear(self): + self.jar.extract_cookies(self.response, self.request) + assert len(self.jar) == 1 + self.jar.clear() + assert len(self.jar) == 0 + + def test_clear_session_cookies(self): + self.jar.extract_cookies(self.response, self.request) + assert len(self.jar) == 1 + self.jar.clear_session_cookies() + assert len(self.jar) == 0 + + def test_set_policy(self): + policy = DefaultCookiePolicy() + self.jar.set_policy(policy) + assert self.jar.jar._policy is policy + + def test_check_expired_frequency(self): + jar = CookieJar(check_expired_frequency=1) + jar.add_cookie_header(self.request) + assert jar.processed == 1 + + class TestWrappedRequest: def setup_method(self): self.request = Request( diff --git a/tests/test_http_headers.py b/tests/test_http_headers.py index 243aa6afe..aff3562e3 100644 --- a/tests/test_http_headers.py +++ b/tests/test_http_headers.py @@ -140,6 +140,7 @@ class TestHeaders: h1["foo"] = "bar" h1["foo"] = None h1.setdefault("foo", "bar") + assert h1["foo"] is None assert h1.get("foo") is None assert h1.getlist("foo") == [] diff --git a/tests/test_http_request.py b/tests/test_http_request.py index e22689d49..1941b826f 100644 --- a/tests/test_http_request.py +++ b/tests/test_http_request.py @@ -24,6 +24,10 @@ class TestRequest: with pytest.raises(TypeError): self.request_class(123) + # priority argument must be an integer + with pytest.raises(TypeError, match="Request priority not an integer"): + self.request_class("http://www.example.com", priority="1") + r = self.request_class("http://www.example.com") assert isinstance(r.url, str) assert r.url == "http://www.example.com" @@ -274,6 +278,12 @@ class TestRequest: assert r4.meta == {} assert r4.dont_filter is False + # the cls argument allows changing the resulting class + custom_request_cls = type("CustomRequest", (self.request_class,), {}) + r5 = r1.replace(cls=custom_request_cls) + assert isinstance(r5, custom_request_cls) + assert r5.url == r1.url + def test_method_always_str(self): r = self.request_class("http://www.example.com", method="POST") assert isinstance(r.method, str) diff --git a/tests/test_http_request_form.py b/tests/test_http_request_form.py index a4f87d50d..c667e48a1 100644 --- a/tests/test_http_request_form.py +++ b/tests/test_http_request_form.py @@ -294,6 +294,9 @@ class TestFormRequest(TestRequest): assert request.method == "GET" request = FormRequest.from_response(response, method="POST") assert request.method == "POST" + # an explicit method=None skips form-method normalization + request = FormRequest.from_response(response, method=None) + assert request.method == "NONE" def test_from_response_override_url(self): response = _buildresponse( diff --git a/tests/test_http_response.py b/tests/test_http_response.py index 079c547c7..a8ea4920e 100644 --- a/tests/test_http_response.py +++ b/tests/test_http_response.py @@ -254,6 +254,11 @@ class TestResponse: with pytest.raises(ValueError, match="url can't be None"): r.follow(None) + def test_follow_None_encoding(self): + r = self.response_class("http://example.com") + with pytest.raises(ValueError, match="encoding can't be None"): + r.follow("foo", encoding=None) + @pytest.mark.xfail( not W3LIB_STRIPS_URLS, reason="https://github.com/scrapy/w3lib/pull/207", diff --git a/tests/test_http_response_text.py b/tests/test_http_response_text.py index 4b3fa2302..507fd1864 100644 --- a/tests/test_http_response_text.py +++ b/tests/test_http_response_text.py @@ -14,6 +14,13 @@ from tests.test_http_response import TestResponse class TestTextResponse(TestResponse): response_class = TextResponse + def test_follow_None_encoding(self): + # unlike the base Response, TextResponse.follow() falls back to the + # response encoding when encoding is None instead of raising + r = self.response_class("http://example.com", body=b"hello", encoding="cp1252") + req = r.follow("foo", encoding=None) + assert req.encoding == "cp1252" + def test_replace(self): super().test_replace() r1 = self.response_class( From 0ccddb4f61b27dd493fd65329d039b88a3333e7b Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 11:47:30 +0200 Subject: [PATCH 02/28] Improve test coverage for contracts (#7677) --- tests/test_contracts.py | 180 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) diff --git a/tests/test_contracts.py b/tests/test_contracts.py index e80945b93..698044de7 100644 --- a/tests/test_contracts.py +++ b/tests/test_contracts.py @@ -133,6 +133,29 @@ class DemoSpider(Spider): """ return DemoItem(url=response.url) + def returns_request_range_fail(self, response): + """method which returns fewer requests than the expected range + @url http://scrapy.org + @returns requests 2 3 + """ + return Request("http://scrapy.org", callback=self.returns_item) + + def yields_item_and_request(self, response): + """yields one item and one request + @url http://scrapy.org + @returns items 1 1 + @scrapes name url + """ + yield DemoItem(name="test", url=response.url) + yield Request("http://scrapy.org", callback=self.returns_item) + + async def returns_async_gen(self, response): + """async generator callback + @url http://scrapy.org + @returns items 1 1 + """ + yield DemoItem(url=response.url) + def returns_dict_fail(self, response): """method which returns item @url http://scrapy.org @@ -437,6 +460,48 @@ class TestContractsManager: request.callback(response) self.should_error() + def test_returns_invalid_argument_count(self): + spider = DemoSpider() + with pytest.raises(ValueError, match="expected 1, 2 or 3, got 0"): + ReturnsContract(spider.returns_item) + with pytest.raises(ValueError, match="expected 1, 2 or 3, got 4"): + ReturnsContract(spider.returns_item, "items", "1", "2", "3") + + def test_returns_default_bounds(self): + spider = DemoSpider() + contract = ReturnsContract(spider.returns_item, "items") + assert contract.min_bound == 1 + assert contract.max_bound == float("inf") + + def test_returns_range_fail(self): + spider = DemoSpider() + response = ResponseMock() + + request = self.conman.from_method( + spider.returns_request_range_fail, self.results + ) + request.callback(response) + self.should_fail() + assert "expected 2..3" in self.results.failures[-1][-1] + + def test_returns_and_scrapes_ignore_other_types(self): + spider = DemoSpider() + response = ResponseMock() + + # @returns and @scrapes only count matching output objects and skip + # the request that is also yielded. + request = self.conman.from_method(spider.yields_item_and_request, self.results) + request.callback(response) + self.should_succeed() + + def test_testcase_str(self): + spider = DemoSpider() + contract = UrlContract(spider.returns_request, "http://scrapy.org") + assert ( + str(contract.testcase_pre) + == "[demo_spider] returns_request (@url pre-hook)" + ) + def test_scrapes(self): spider = DemoSpider() response = ResponseMock() @@ -570,6 +635,41 @@ class CustomFailContractPostProcess(Contract): raise KeyboardInterrupt("Post-process exception") +class PreProcessSuccessContract(Contract): + name = "pre_success" + + def pre_process(self, response): + return + + +class PreProcessAssertionFailContract(Contract): + name = "pre_assertion_fail" + + def pre_process(self, response): + raise AssertionError("pre-process assertion") + + +class PreProcessErrorContract(Contract): + name = "pre_error" + + def pre_process(self, response): + raise ValueError("pre-process error") + + +class PostProcessSuccessContract(Contract): + name = "post_success" + + def post_process(self, output): + return + + +class PostProcessErrorContract(Contract): + name = "post_error" + + def post_process(self, output): + raise ValueError("post-process error") + + class TestCustomContractPrePostProcess: def setup_method(self): self.results = TextTestResult(stream=None, descriptions=False, verbosity=0) @@ -601,3 +701,83 @@ class TestCustomContractPrePostProcess: assert not self.results.failures assert not self.results.errors + + def test_pre_hook_success(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessSuccessContract(spider.returns_request) + conman = ContractsManager([UrlContract, ReturnsContract, contract]) + + request = conman.from_method(spider.returns_request, self.results) + contract.add_pre_hook(request, self.results) + request.callback(response, **request.cb_kwargs) + + assert not self.results.failures + assert not self.results.errors + + def test_pre_hook_assertion_failure(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessAssertionFailContract(spider.returns_request) + conman = ContractsManager([UrlContract, ReturnsContract, contract]) + + request = conman.from_method(spider.returns_request, self.results) + contract.add_pre_hook(request, self.results) + request.callback(response, **request.cb_kwargs) + + assert self.results.failures + assert not self.results.errors + + def test_pre_hook_error(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessErrorContract(spider.returns_request) + conman = ContractsManager([UrlContract, ReturnsContract, contract]) + + request = conman.from_method(spider.returns_request, self.results) + contract.add_pre_hook(request, self.results) + request.callback(response, **request.cb_kwargs) + + assert self.results.errors + + def test_pre_hook_async_callback(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessSuccessContract(spider.returns_request_async) + request = Request("http://scrapy.org", callback=spider.returns_request_async) + contract.add_pre_hook(request, self.results) + + with pytest.raises(TypeError, match="async callbacks"): + request.callback(response) + + def test_pre_hook_async_generator(self): + spider = DemoSpider() + response = ResponseMock() + contract = PreProcessSuccessContract(spider.returns_async_gen) + request = Request("http://scrapy.org", callback=spider.returns_async_gen) + contract.add_pre_hook(request, self.results) + + with pytest.raises(TypeError, match="async callbacks"): + request.callback(response) + + def test_post_hook_async_generator(self): + spider = DemoSpider() + response = ResponseMock() + contract = PostProcessSuccessContract(spider.returns_async_gen) + request = Request("http://scrapy.org", callback=spider.returns_async_gen) + contract.add_post_hook(request, self.results) + + with pytest.raises(TypeError, match="async callbacks"): + request.callback(response) + + def test_post_hook_error(self): + spider = DemoSpider() + response = ResponseMock() + contract = PostProcessErrorContract(spider.returns_request) + conman = ContractsManager([UrlContract, ReturnsContract, contract]) + + request = conman.from_method(spider.returns_request, self.results) + contract.add_post_hook(request, self.results) + request.callback(response, **request.cb_kwargs) + + assert self.results.errors From 7ab404c72546a9f0ef5bf4a6ee54512d3c1d6851 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 11:58:37 +0200 Subject: [PATCH 03/28] Add a documentation page about security (#7678) --- docs/index.rst | 5 + docs/topics/download-handlers.rst | 4 + docs/topics/downloader-middleware.rst | 2 + docs/topics/security.rst | 207 ++++++++++++++++++++++++++ docs/topics/settings.rst | 16 ++ docs/topics/spider-middleware.rst | 2 + docs/topics/telnetconsole.rst | 4 + 7 files changed, 240 insertions(+) create mode 100644 docs/topics/security.rst diff --git a/docs/index.rst b/docs/index.rst index a46a2ad9f..8e8624a22 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -151,6 +151,7 @@ Solving specific problems topics/debug topics/contracts topics/practices + topics/security topics/broad-crawls topics/developer-tools topics/dynamic-content @@ -175,6 +176,10 @@ Solving specific problems :doc:`topics/practices` Get familiar with some Scrapy common practices. +:doc:`topics/security` + Understand the security implications of Scrapy defaults and how to harden + them. + :doc:`topics/broad-crawls` Tune Scrapy for crawling a lot domains in parallel. diff --git a/docs/topics/download-handlers.rst b/docs/topics/download-handlers.rst index 888bfaf08..c901e01e4 100644 --- a/docs/topics/download-handlers.rst +++ b/docs/topics/download-handlers.rst @@ -39,6 +39,10 @@ for additional schemes and to replace or disable default ones: "sftp": "my.download_handlers.SftpHandler", } +.. seealso:: :ref:`security-unencrypted-protocols` and + :ref:`security-local-resources`, for the security implications of the + default ``http``, ``ftp``, ``file`` and ``data`` handlers. + Replacing HTTP(S) download handlers ----------------------------------- diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index 0c1af5276..5649453b1 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -379,6 +379,8 @@ that this risks leaking credentials to unrelated domains. This setting must be explicitly configured whenever :setting:`HTTPAUTH_USER` or :setting:`HTTPAUTH_PASS` is set. +.. seealso:: :ref:`security-credential-leakage` + .. _Basic access authentication: https://en.wikipedia.org/wiki/Basic_access_authentication diff --git a/docs/topics/security.rst b/docs/topics/security.rst new file mode 100644 index 000000000..2ca270045 --- /dev/null +++ b/docs/topics/security.rst @@ -0,0 +1,207 @@ +.. _security: + +======== +Security +======== + +Scrapy defaults are optimized for web scraping, not for the security posture +that you might expect from software that handles untrusted input or runs in a +shared or exposed environment. Some common security practices are unnecessary +for many scraping use cases, and a few can even prevent valid ones (for +example, sites that you must scrape may use misconfigured TLS certificates or +serve content over unencrypted protocols). + +This page highlights the Scrapy defaults that have security implications, so +that you can make an informed decision about whether to keep them, and explains +how to harden them along with the trade-offs involved. + +.. note:: + + None of the options below are silver bullets. Which of them make sense + depends on your threat model: whether the URLs you crawl come from trusted + sources, whether the machine running Scrapy is exposed to a network you do + not control, whether the data you handle is sensitive, and so on. + +.. _security-untrusted-responses: + +Treat responses as untrusted input +================================== + +Regardless of any setting, remember that response data comes from servers you +do not control, even when you trust the site you are crawling, as responses may +be tampered with in transit or the server itself may be compromised. + +Never pass response data to functions that can execute code or otherwise act on +their input in an unsafe way, such as :func:`eval`, :func:`exec`, or +:func:`pickle.loads`, and be careful when writing response data to paths +derived from the response itself. + +TLS connections +=============== + +.. _security-certificate-verification: + +Certificate verification +------------------------ + +By default Scrapy does **not** verify the TLS certificate of HTTPS servers, as +controlled by the :setting:`DOWNLOAD_VERIFY_CERTIFICATES` setting (default: +``False``). + +This default favors reach over security: many sites that are otherwise fine to +scrape have expired, self-signed, or otherwise invalid certificates, and +verifying certificates would make requests to them fail. + +If the integrity of the connection matters to you (for example, to detect +man-in-the-middle attacks), set: + +.. code-block:: python + + DOWNLOAD_VERIFY_CERTIFICATES = True + +* **Pro:** requests to servers with invalid or untrusted certificates fail + instead of silently succeeding, protecting you from some man-in-the-middle + attacks. + +* **Con:** you can no longer scrape sites with misconfigured certificates + without re-disabling verification for them. + +.. _security-tls-protocols-ciphers: + +Protocol versions and ciphers +----------------------------- + +You can restrict the TLS protocol versions that Scrapy accepts through the +:setting:`DOWNLOAD_TLS_MIN_VERSION` and :setting:`DOWNLOAD_TLS_MAX_VERSION` +settings, e.g. to reject obsolete protocol versions. + +By default Scrapy uses the OpenSSL ``DEFAULT`` cipher list +(:setting:`DOWNLOADER_CLIENT_TLS_CIPHERS`), which favors compatibility and still +allows some older, weaker ciphers. Set it to ``None`` to instead use the curated +cipher list of the underlying TLS implementation (Twisted), which excludes weak +ciphers: + +.. code-block:: python + + DOWNLOADER_CLIENT_TLS_CIPHERS = None + +* **Pro:** connections that would negotiate a weak cipher fail instead of + succeeding. + +* **Con:** you can no longer connect to servers that only support the excluded + ciphers. + +.. _security-unencrypted-protocols: + +Unencrypted protocols +===================== + +By default Scrapy enables download handlers for unencrypted protocols, namely +``http://`` and ``ftp://`` (see :setting:`DOWNLOAD_HANDLERS_BASE`). Data sent +and received over these protocols, including any credentials, travels in plain +text and can be read or modified by anyone on the network path. + +If you only crawl over encrypted protocols, you can disable the unencrypted +ones so that no request can accidentally be sent unencrypted: + +.. code-block:: python + + DOWNLOAD_HANDLERS = { + "http": None, + "ftp": None, + } + +* **Pro:** a misconfigured or maliciously-redirected request cannot leak data + over an unencrypted connection, as such requests fail instead. + +* **Con:** you can no longer crawl resources that are only available over those + protocols. + +Note that disabling the ``http`` handler also prevents plain-HTTP requests that +result from following an ``http://`` redirect or link, which is often the point +of disabling it. + +.. _security-local-resources: + +Local and non-network resources +=============================== + +By default Scrapy enables download handlers for the ``file://`` and ``data:`` +schemes (see :setting:`DOWNLOAD_HANDLERS_BASE`). The ``file://`` handler reads +arbitrary files from the local filesystem, limited only by the permissions of +the process running Scrapy. + +This is convenient (for example, to parse a local HTML file), but it is a risk +if any of the URLs you schedule come from an untrusted source: a crafted +``file:///etc/passwd`` URL could read local files. + +If you do not need them, disable these handlers: + +.. code-block:: python + + DOWNLOAD_HANDLERS = { + "file": None, + "data": None, + } + +* **Pro:** crawled URLs cannot be used to read local files or inline data. + +* **Con:** you can no longer fetch ``file://`` or ``data:`` URLs. + +More generally, if you crawl URLs from untrusted sources, consider validating +their schemes (and, where applicable, their hosts) before scheduling requests, +to avoid server-side request forgery (SSRF) and similar issues. + +.. _security-telnet: + +Telnet console +============== + +Scrapy enables the :ref:`telnet console ` by default +(:setting:`TELNETCONSOLE_ENABLED`). The telnet console is a Python shell +running inside the Scrapy process, so anyone who can connect to it can run +arbitrary code in that process. + +By default the console binds to ``127.0.0.1`` (:setting:`TELNETCONSOLE_HOST`) +and is protected by a username (:setting:`TELNETCONSOLE_USERNAME`, default +``scrapy``) and an automatically generated password +(:setting:`TELNETCONSOLE_PASSWORD`), so it is only reachable from the local +machine. + +.. warning:: + + Telnet does not provide any transport-layer security, so the + username/password authentication does not protect the credentials or the + session from anyone able to observe the traffic. Never expose the telnet + console over an untrusted network by changing :setting:`TELNETCONSOLE_HOST` + to a non-local address. + +If you do not use the telnet console, disable it entirely: + +.. code-block:: python + + TELNETCONSOLE_ENABLED = False + +* **Pro:** removes a local code-execution surface and one less listening port. + +* **Con:** you can no longer :ref:`inspect and control a running crawler + ` through it. + +.. _security-credential-leakage: + +Credential leakage across domains +================================= + +Some Scrapy features attach credentials or other sensitive headers to requests, +and a crawl that spans multiple domains can leak them to unintended hosts: + +* HTTP authentication credentials set through + :class:`~scrapy.downloadermiddlewares.httpauth.HttpAuthMiddleware` are only + sent to the domain set in :setting:`HTTPAUTH_DOMAIN`. Leave this set to the + intended domain rather than ``None`` so that credentials are not sent to + every domain you crawl. + +* The ``Referer`` header may disclose the URLs you crawl to other sites. The + default :setting:`REFERRER_POLICY` already avoids sending the referrer from + HTTPS to HTTP, but you can tighten it further (for example, to + ``same-origin`` or ``no-referrer``) if needed. diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index b92733ede..11251a92c 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -774,6 +774,8 @@ implementation. handler `, so it's not guaranteed to be supported by all 3rd-party handlers. +.. seealso:: :ref:`security-tls-protocols-ciphers` + .. setting:: DOWNLOAD_TLS_MAX_VERSION DOWNLOAD_TLS_MAX_VERSION @@ -807,6 +809,8 @@ modern environments. by all 3rd-party handlers. Additionally, the set of supported TLS versions depends on the TLS implementation being used by the handler. +.. seealso:: :ref:`security-tls-protocols-ciphers` + .. setting:: DOWNLOAD_TLS_MIN_VERSION DOWNLOAD_TLS_MIN_VERSION @@ -819,6 +823,8 @@ be used by Scrapy. See :setting:`DOWNLOAD_TLS_MAX_VERSION` for the details and limitations. +.. seealso:: :ref:`security-tls-protocols-ciphers` + .. setting:: DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING DOWNLOADER_CLIENT_TLS_VERBOSE_LOGGING @@ -979,6 +985,9 @@ enabled in your project. See :setting:`DOWNLOAD_HANDLERS_BASE` for example format. +.. seealso:: :ref:`security-unencrypted-protocols` and + :ref:`security-local-resources` + .. setting:: DOWNLOAD_HANDLERS_BASE DOWNLOAD_HANDLERS_BASE @@ -1026,6 +1035,9 @@ handler (without replacement), place this in your ``settings.py``: "ftp": None, } +.. seealso:: :ref:`security-unencrypted-protocols` and + :ref:`security-local-resources` + .. setting:: DOWNLOAD_SLOTS @@ -1184,6 +1196,8 @@ when making a request and abort the request if the verification fails. certificate problems are logged when this setting is set to ``False``) depends on its implementation. +.. seealso:: :ref:`security-certificate-verification` + .. setting:: DUPEFILTER_CLASS DUPEFILTER_CLASS @@ -2074,6 +2088,8 @@ Default: ``True`` (``False`` when :setting:`TWISTED_REACTOR_ENABLED` is ``False` A boolean which specifies if the :ref:`telnet console ` will be enabled (provided its extension is also enabled). +.. seealso:: :ref:`security-telnet` + .. setting:: TEMPLATES_DIR TEMPLATES_DIR diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 99bbdf292..e2cd0f986 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -355,6 +355,8 @@ Default: ``'scrapy.spidermiddlewares.referer.DefaultReferrerPolicy'`` using the special ``"referrer_policy"`` :ref:`Request.meta ` key, with the same acceptable values as for the ``REFERRER_POLICY`` setting. +.. seealso:: :ref:`security-credential-leakage` + Acceptable values for REFERRER_POLICY ************************************* diff --git a/docs/topics/telnetconsole.rst b/docs/topics/telnetconsole.rst index e274edc9b..a30258112 100644 --- a/docs/topics/telnetconsole.rst +++ b/docs/topics/telnetconsole.rst @@ -29,6 +29,8 @@ disable it if you want. For more information about the extension itself see .. note:: This feature is not supported when :setting:`TWISTED_REACTOR_ENABLED` is ``False``. +.. seealso:: :ref:`security-telnet` + .. highlight:: none How to access the telnet console @@ -190,6 +192,8 @@ Default: ``'127.0.0.1'`` The interface the telnet console should listen on +.. seealso:: :ref:`security-telnet` + .. setting:: TELNETCONSOLE_USERNAME From 1b940a75ac20401b1e944a65c8901569c06e091b Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 13:18:22 +0200 Subject: [PATCH 04/28] Improve the item pipeline docs (#7676) --- docs/topics/item-pipeline.rst | 111 ++++++++++++++++++ .../project/module/pipelines.py.tmpl | 2 +- 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/docs/topics/item-pipeline.rst b/docs/topics/item-pipeline.rst index 9a53f88fb..f1fa463d8 100644 --- a/docs/topics/item-pipeline.rst +++ b/docs/topics/item-pipeline.rst @@ -57,6 +57,8 @@ Any of these methods may be defined as a coroutine function (``async def``). Item pipeline example ===================== +.. _price-pipeline-example: + Price validation and dropping items with no prices -------------------------------------------------- @@ -246,6 +248,8 @@ returns multiples items with the same id: return item +.. _activating-item-pipeline: + Activating an Item Pipeline component ===================================== @@ -262,3 +266,110 @@ To activate an Item Pipeline component you must add its class to the The integer values you assign to classes in this setting determine the order in which they run: items go through from lower valued to higher valued classes. It's customary to define these numbers in the 0-1000 range. + +A complete example +================== + +The examples above show item pipeline components on their own. In a project, a +pipeline is one of four pieces that work together: the :ref:`item +` your spider produces, the :ref:`spider ` that +yields it, the pipeline that processes it, and the :setting:`ITEM_PIPELINES` +setting that enables the pipeline. + +The following example wires those pieces together to validate the price of +books scraped from `books.toscrape.com`_, reusing the ``PricePipeline`` from +:ref:`price-pipeline-example` above. + +Define the item in ``myproject/items.py``: + +.. code-block:: python + + from dataclasses import dataclass + + + @dataclass + class BookItem: + title: str + price: float + +Yield instances of that item from your spider, e.g. in +``myproject/spiders/books.py``: + +.. skip: next +.. code-block:: python + + import scrapy + + from myproject.items import BookItem + + + class BooksSpider(scrapy.Spider): + name = "books" + start_urls = ["https://books.toscrape.com/"] + + def parse(self, response): + for book in response.css("article.product_pod"): + yield BookItem( + title=book.css("h3 a::attr(title)").get(), + price=float(book.css("p.price_color::text").re_first(r"[\d.]+")), + ) + +Put the ``PricePipeline`` shown earlier in ``myproject/pipelines.py``, and +enable it in ``myproject/settings.py``: + +.. code-block:: python + + ITEM_PIPELINES = { + "myproject.pipelines.PricePipeline": 300, + } + +With these pieces in place, every ``BookItem`` that ``BooksSpider`` yields +passes through ``PricePipeline`` before it reaches the :ref:`feed exports +` or any other output. + +.. _books.toscrape.com: https://books.toscrape.com/ + + +Common pitfalls +=============== + +The pipeline does not run +------------------------- + +A pipeline component only runs if its class is listed in the +:setting:`ITEM_PIPELINES` setting, normally in your project's +:file:`settings.py` file (see :ref:`activating-item-pipeline`). Adding it to +the spider or elsewhere has no effect. + +To confirm that Scrapy loaded your pipeline, look for a line like this near the +start of the crawl log:: + + [scrapy.middleware] INFO: Enabled item pipelines: + ['myproject.pipelines.PricePipeline'] + +If your pipeline is missing from that list, check that its import path matches +the :setting:`ITEM_PIPELINES` entry, and that the setting is not being +overridden, for example by :attr:`~scrapy.Spider.custom_settings` or by a +redefinition of :setting:`ITEM_PIPELINES` in :file:`settings.py`. + +The item is not returned +------------------------ + +:meth:`process_item` must return the item (or raise +:exc:`~scrapy.exceptions.DropItem`). A common mistake is to modify the item but +forget to return it: + +.. code-block:: python + + def process_item(self, item): + ItemAdapter(item)["price"] *= 1.15 + # Bug: returns None, so the next component gets None instead of the item. + +Return the item so that the next component, and the rest of Scrapy, can keep +processing it: + +.. code-block:: python + + def process_item(self, item): + ItemAdapter(item)["price"] *= 1.15 + return item diff --git a/scrapy/templates/project/module/pipelines.py.tmpl b/scrapy/templates/project/module/pipelines.py.tmpl index e845f43e9..a6494b391 100644 --- a/scrapy/templates/project/module/pipelines.py.tmpl +++ b/scrapy/templates/project/module/pipelines.py.tmpl @@ -9,5 +9,5 @@ from itemadapter import ItemAdapter class ${ProjectName}Pipeline: - def process_item(self, item, spider): + def process_item(self, item): return item From edc353c975d2c7fdef480bcaad27c4ff6602682d Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 16:40:36 +0200 Subject: [PATCH 05/28] Improve test coverage for shell/ (#7680) * Improve test coverage for shell/ * Address test issues * Make the shell config test more reliable on Windows --- tests/test_command_shell.py | 196 +++++++++++++++++++++++++++++++++++- 1 file changed, 195 insertions(+), 1 deletion(-) diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 1585835cc..1c109a333 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -1,19 +1,28 @@ from __future__ import annotations +import importlib.util import os +import signal import sys from io import BytesIO from pathlib import Path -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any +from unittest.mock import AsyncMock, MagicMock, patch import pytest from pexpect.popen_spawn import PopenSpawn +from scrapy import Spider +from scrapy.http import Request, Response +from scrapy.shell import Shell, inspect_response from scrapy.utils.reactor import _asyncio_reactor_path +from scrapy.utils.test import get_crawler from tests import NON_EXISTING_RESOLVABLE, tests_datadir from tests.utils.cmdline import proc +from tests.utils.decorators import coroutine_test if TYPE_CHECKING: + from scrapy.crawler import Crawler from tests.mockserver.http import MockServer @@ -139,6 +148,19 @@ class TestShellCommand: ) assert ret == 0, err + def test_shelp(self) -> None: + ret, out, _ = proc("shell", "-c", "shelp()") + assert ret == 0, out + assert "Available Scrapy objects" in out + + def test_fetch_request_with_callbacks(self, mockserver: MockServer) -> None: + url = mockserver.url("/text") + code = ( + f"fetch(scrapy.Request('{url}', callback=lambda r: r, errback=lambda f: f))" + ) + ret, out, _ = proc("shell", "-c", code) + assert ret == 0, out + class TestInteractiveShell: def test_fetch(self, mockserver: MockServer) -> None: @@ -161,3 +183,175 @@ class TestInteractiveShell: p.wait() # type: ignore[no-untyped-call] logfile.seek(0) assert "Traceback" not in logfile.read().decode() + + @staticmethod + def _isolate_config(env: dict[str, str], config_home: Path) -> None: + """Point every scrapy.cfg location (see + :func:`scrapy.utils.conf.get_sources`) at ``config_home``. + + ``XDG_CONFIG_HOME`` is read by Scrapy on all platforms, while + ``~/.scrapy.cfg`` goes through :func:`os.path.expanduser`, which uses + ``HOME`` on POSIX and ``USERPROFILE`` on Windows. The working directory + stays at the repository root (no scrapy.cfg) so subprocess coverage data + is still collected there. + """ + env.pop("SCRAPY_PYTHON_SHELL", None) + env["HOME"] = str(config_home) + env["USERPROFILE"] = str(config_home) + env["XDG_CONFIG_HOME"] = str(config_home) + + def _run_interactive_shell(self, env: dict[str, str]) -> str: + args = (sys.executable, "-m", "scrapy.cmdline", "shell") + logfile = BytesIO() + p = PopenSpawn(args, env=env, timeout=5) + p.logfile_read = logfile + p.expect_exact("Available Scrapy objects") + p.sendeof() + p.wait() # type: ignore[no-untyped-call] + logfile.seek(0) + return logfile.read().decode() + + @pytest.mark.skipif( + importlib.util.find_spec("IPython") is None, + reason="Without IPython installed, shell=python and the default both " + "select the standard Python shell, so the setting has no observable effect.", + ) + def test_shell_from_cfg(self, tmp_path: Path) -> None: + config_home = tmp_path / "config" + config_home.mkdir() + (config_home / "scrapy.cfg").write_text("[settings]\nshell = python\n") + env = os.environ.copy() + self._isolate_config(env, config_home) + args = (sys.executable, "-m", "scrapy.cmdline", "shell") + logfile = BytesIO() + p = PopenSpawn(args, env=env, timeout=10) + p.logfile_read = logfile + p.expect_exact("Available Scrapy objects") + # The standard Python shell never imports IPython, whereas the IPython + # shell (the default when installed) does; this confirms the configured + # shell=python was honored, regardless of platform-specific prompts. + p.sendline("import sys; print('IPYMODULE', 'IPython' in sys.modules)") + p.expect_exact("IPYMODULE False") + p.sendeof() + p.wait() # type: ignore[no-untyped-call] + logfile.seek(0) + assert "Traceback" not in logfile.read().decode() + + def test_shell_default_shells(self, tmp_path: Path) -> None: + config_home = tmp_path / "config" + config_home.mkdir() + env = os.environ.copy() + self._isolate_config(env, config_home) + assert "Traceback" not in self._run_interactive_shell(env) + + +@pytest.fixture +def restore_sigint(): + """Shell.start() installs SIG_IGN as the SIGINT handler; restore it.""" + handler = signal.getsignal(signal.SIGINT) + try: + yield + finally: + signal.signal(signal.SIGINT, handler) + + +def _no_reactor_crawler(monkeypatch: pytest.MonkeyPatch) -> Crawler: + """Return a crawler that reports ``TWISTED_REACTOR_ENABLED=False``. + + A genuine no-reactor crawler cannot be built while a Twisted reactor is + installed (as it is during the test run), so we build a normal crawler and + make its settings report the reactor as disabled, which is all the shell + code looks at. + """ + crawler = get_crawler() + real_getbool = crawler.settings.getbool + + def fake_getbool(name: str, *args: Any, **kwargs: Any) -> bool: + if name == "TWISTED_REACTOR_ENABLED": + return False + return real_getbool(name, *args, **kwargs) + + monkeypatch.setattr(crawler.settings, "getbool", fake_getbool) + return crawler + + +@pytest.mark.requires_reactor +class TestShell: + """Tests for :class:`~scrapy.shell.Shell` paths with no ``scrapy shell`` + command-line route: those reached through + :func:`scrapy.shell.inspect_response` (called from spider callbacks) or only + through direct API use, hence not covered by the subprocess tests above. + """ + + def test_populate_vars_fetch_not_available(self) -> None: + shell = Shell(get_crawler()) + shell._inthread = False + shell.populate_vars() + assert "fetch" not in shell.vars + + def test_get_help_fetch_not_available(self) -> None: + shell = Shell(get_crawler()) + shell._inthread = False + shell.populate_vars() + help_text = shell.get_help() + assert "fetch(url" not in help_text + assert "shelp()" in help_text + + def test_start_with_request(self, restore_sigint: None) -> None: + shell = Shell(get_crawler(), code="1") + shell.fetch = MagicMock() # type: ignore[method-assign] + request = Request("data:,") + shell.start(request=request) + shell.fetch.assert_called_once_with(request, None) + + def test_start_with_response( + self, restore_sigint: None, capsys: pytest.CaptureFixture[str] + ) -> None: + shell = Shell(get_crawler(), code="response.url") + request = Request("data:,") + response = Response("data:,", request=request) + shell.start(response=response) + assert "data:," in capsys.readouterr().out + assert shell.vars["response"] is response + assert shell.vars["request"] is request + + @patch("scrapy.shell.start_python_console") + def test_inspect_response( + self, mock_console: MagicMock, restore_sigint: None + ) -> None: + crawler = get_crawler() + spider = crawler._create_spider() + response = Response("data:,", request=Request("data:,")) + sigint_handler = signal.getsignal(signal.SIGINT) + inspect_response(response, spider) + mock_console.assert_called_once() + assert signal.getsignal(signal.SIGINT) is sigint_handler + + @coroutine_test + async def test_open_spider_explicit_spider(self) -> None: + crawler = get_crawler() + crawler.engine = MagicMock() + crawler.engine.open_spider_async = AsyncMock() + shell = Shell(crawler) + spider = Spider("test") + await shell._open_spider(spider) + assert shell.spider is spider + assert crawler.spider is spider + crawler.engine.open_spider_async.assert_called_once_with(close_if_idle=False) + + +@pytest.mark.only_asyncio +class TestShellNoReactor: + @coroutine_test + @patch("scrapy.shell.start_python_console") + async def test_inspect_response_no_reactor( + self, + mock_console: MagicMock, + restore_sigint: None, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + crawler = _no_reactor_crawler(monkeypatch) + spider = crawler._create_spider() + response = Response("data:,", request=Request("data:,")) + inspect_response(response, spider) + mock_console.assert_called_once() From cf5607f8bca45fa7c86b053b599a457542641ec1 Mon Sep 17 00:00:00 2001 From: Adrian Date: Fri, 26 Jun 2026 17:35:46 +0200 Subject: [PATCH 06/28] Undeprecated the basic FormRequest API (#7671) --- docs/topics/request-response.rst | 5 +++ scrapy/http/__init__.py | 17 +-------- scrapy/http/request/form.py | 63 ++++++++++++++++++++++++++++---- tests/test_http_request_form.py | 28 +++++++++++++- 4 files changed, 89 insertions(+), 24 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 700238fe9..5e70a4f98 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -911,6 +911,11 @@ Request subclasses Here is the list of built-in :class:`~scrapy.Request` subclasses. You can also subclass it to implement your own custom functionality. +FormRequest +----------- + +.. autoclass:: scrapy.FormRequest + JsonRequest ----------- diff --git a/scrapy/http/__init__.py b/scrapy/http/__init__.py index e20e894ae..0e5c2b53b 100644 --- a/scrapy/http/__init__.py +++ b/scrapy/http/__init__.py @@ -5,11 +5,9 @@ Use this module (instead of the more specific ones) when importing Headers, Request and Response outside this module. """ -from warnings import catch_warnings, filterwarnings - -from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http.headers import Headers from scrapy.http.request import Request +from scrapy.http.request.form import FormRequest from scrapy.http.request.json_request import JsonRequest from scrapy.http.request.rpc import XmlRpcRequest from scrapy.http.response import Response @@ -17,19 +15,6 @@ from scrapy.http.response.html import HtmlResponse from scrapy.http.response.json import JsonResponse from scrapy.http.response.text import TextResponse from scrapy.http.response.xml import XmlResponse -from scrapy.utils.deprecate import create_deprecated_class - -with catch_warnings(): - filterwarnings("ignore", category=ScrapyDeprecationWarning) - - from scrapy.http.request.form import FormRequest as _FormRequest - - FormRequest = create_deprecated_class( - name="FormRequest", - new_class=_FormRequest, - subclass_warn_message="{cls} inherits from deprecated class {old}, use the form2request library instead.", - instance_warn_message="{cls} is deprecated, use the form2request library instead.", - ) __all__ = [ "FormRequest", diff --git a/scrapy/http/request/form.py b/scrapy/http/request/form.py index 4da595b22..f1a8dbf3b 100644 --- a/scrapy/http/request/form.py +++ b/scrapy/http/request/form.py @@ -32,19 +32,61 @@ if TYPE_CHECKING: from scrapy.http.response.text import TextResponse -warn( - "The entire scrapy.http.request.form module is deprecated. Use the " - "form2request library instead.", - ScrapyDeprecationWarning, - stacklevel=2, -) - FormdataVType: TypeAlias = str | Iterable[str] FormdataKVType: TypeAlias = tuple[str, FormdataVType] FormdataType: TypeAlias = dict[str, FormdataVType] | list[FormdataKVType] | None class FormRequest(Request): + """A :class:`~scrapy.Request` subclass with a ``formdata`` parameter that + url-encodes the given data and assigns it to the request, which makes it + convenient to send arbitrary form data via HTTP POST or GET without an HTML + ``
`` element to parse. + + .. note:: To build a request from an HTML ```` element found in a + response, use :doc:`form2request ` instead. See + :ref:`form`. + + The remaining arguments are the same as for the :class:`~scrapy.Request` + class and are not documented here. + + :param formdata: a dictionary (or iterable of (key, value) tuples) + containing HTML form data which will be url-encoded. If + :attr:`~scrapy.Request.method` is not given and ``formdata`` is + provided, the method is set to ``"POST"`` and the data is assigned to + the request body; if the method is ``"GET"``, the data is added to the + URL query string instead. + :type formdata: dict or collections.abc.Iterable + + To send data via HTTP POST, simulating an HTML form submission, return a + :class:`~scrapy.FormRequest` object from your spider: + + .. skip: next + .. code-block:: python + + return [ + FormRequest( + url="http://www.example.com/post/action", + formdata={"name": "John Doe", "age": "27"}, + callback=self.after_post, + ) + ] + + To send the data in the URL query string instead, use the ``GET`` method: + + .. skip: next + .. code-block:: python + + return [ + FormRequest( + url="http://www.example.com/search", + method="GET", + formdata={"q": "keyword", "page": "1"}, + callback=self.parse_results, + ) + ] + """ + __slots__ = () valid_form_methods: ClassVar[list[str]] = ["GET", "POST"] @@ -84,6 +126,13 @@ class FormRequest(Request): formcss: str | None = None, **kwargs: Any, ) -> Self: + warn( + "FormRequest.from_response() is deprecated. Use the form2request " + "library instead.", + ScrapyDeprecationWarning, + stacklevel=2, + ) + kwargs.setdefault("encoding", response.encoding) if formcss is not None: diff --git a/tests/test_http_request_form.py b/tests/test_http_request_form.py index c667e48a1..af86b35c0 100644 --- a/tests/test_http_request_form.py +++ b/tests/test_http_request_form.py @@ -1,10 +1,12 @@ from __future__ import annotations import re +import warnings from urllib.parse import parse_qs, unquote_to_bytes import pytest +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import FormRequest, HtmlResponse from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.python import to_unicode @@ -26,15 +28,39 @@ def _qs(req, encoding="utf-8", to_unicode=False): return parse_qs(uqs, True) +# FormRequest.from_response() is deprecated in favor of form2request, so the +# many tests below that exercise it ignore the resulting deprecation warning. @pytest.mark.filterwarnings("ignore::scrapy.exceptions.ScrapyDeprecationWarning") class TestFormRequest(TestRequest): - request_class = FormRequest # type: ignore[assignment] + request_class = FormRequest def assertQueryEqual(self, first, second, msg=None): first = to_unicode(first).split("&") second = to_unicode(second).split("&") assert sorted(first) == sorted(second), msg + def test_init_not_deprecated(self): + # Building a request directly from form data is not deprecated. + with warnings.catch_warnings(): + warnings.simplefilter("error", ScrapyDeprecationWarning) + self.request_class( + "http://www.example.com", formdata={"a": "1"}, method="POST" + ) + self.request_class( + "http://www.example.com", method="GET", formdata={"a": "1"} + ) + + def test_from_response_deprecated(self): + response = _buildresponse( + """ + + """ + ) + with pytest.warns( + ScrapyDeprecationWarning, match=r"FormRequest\.from_response\(\)" + ): + self.request_class.from_response(response) + def test_empty_formdata(self): r1 = self.request_class("http://www.example.com", formdata={}) assert r1.body == b"" From 4b40d2d06a05174b693958145152559c94ebe487 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 26 Jun 2026 23:05:08 +0500 Subject: [PATCH 07/28] Close various garbage collectible resources (mostly in tests) (#7644) * Set TELNETCONSOLE_ENABLED=False in get_crawler(). * Make _get_console_and_portal() a context manager. * Allow PYTHONTRACEMALLOC in tox. * Close the event loop in test_custom_asyncio_loop_enabled_false(). * Close the handler in _uninstall_scrapy_root_handler(). * Close queues in test_squeues.py. * Close empty queues in init_prios(). * Close PopenSpawn stdin and stdout explicitly. * Silence the unclosed socket warning. * Implement more methods in CustomStatsCollector. * Link the Twisted issue. * Close the connection on download_maxsize. * Fix typing. * Add a comment about CustomStatsCollector. * Restore the coverage. * Properly close fixture queues. * More robust file closing in feed storages. * Close the file in TestMarshalItemExporter.test_nonstring_types_item(). * Cleanup temporary file handling in test_exporters.py. * Close the file in test_stats_file_failed(). * Use an explicit TextIOWrapper in XmlItemExporter. --- pyproject.toml | 2 + scrapy/core/downloader/handlers/http11.py | 3 +- scrapy/exporters.py | 11 ++- scrapy/extensions/feedexport.py | 30 +++---- scrapy/pqueues.py | 4 + scrapy/utils/ftp.py | 4 +- scrapy/utils/log.py | 9 ++- scrapy/utils/test.py | 1 + tests/test_command_runspider.py | 1 + tests/test_command_shell.py | 4 + tests/test_crawler_subprocess.py | 8 ++ tests/test_exporters.py | 16 +++- tests/test_extension_telnet.py | 95 ++++++++++++++--------- tests/test_feedexport.py | 8 +- tests/test_squeues.py | 9 +++ tests/test_squeues_request.py | 26 +++++-- tests/test_stats.py | 10 +++ tests/test_utils_asyncio.py | 9 +++ tox.ini | 1 + 19 files changed, 183 insertions(+), 68 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 6b0c561f8..b8d9067f9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -274,6 +274,8 @@ markers = [ ] filterwarnings = [ "ignore::DeprecationWarning:twisted.web.static", + # Twisted doesn't close failed sockets after CannotListenError: https://github.com/twisted/twisted/issues/6108 + "ignore:Exception ignored in. warnsize: diff --git a/scrapy/exporters.py b/scrapy/exporters.py index e18f1e6ed..c3540d724 100644 --- a/scrapy/exporters.py +++ b/scrapy/exporters.py @@ -171,7 +171,15 @@ class XmlItemExporter(BaseItemExporter): super().__init__(**kwargs) if not self.encoding: self.encoding = "utf-8" - self.xg = XMLGenerator(file, encoding=self.encoding) + # copied from xml.sax.saxutils._gettextwriter() + self.stream = TextIOWrapper( + file, + encoding=self.encoding, + errors="xmlcharrefreplace", + newline="\n", + write_through=True, + ) + self.xg = XMLGenerator(self.stream, encoding=self.encoding) def _beautify_newline(self, new_item: bool = False) -> None: if self.indent is not None and (self.indent > 0 or new_item): @@ -199,6 +207,7 @@ class XmlItemExporter(BaseItemExporter): def finish_exporting(self) -> None: self.xg.endElement(self.root_element) self.xg.endDocument() + self.stream.detach() # Avoid closing the wrapped file. def _export_xml_field(self, name: str, serialized_value: Any, depth: int) -> None: self._beautify_indent(depth=depth) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 8029f85c9..fa708a2e1 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -250,20 +250,22 @@ class S3FeedStorage(BlockingFeedStorage): def _store_in_thread(self, file: IO[bytes]) -> None: file.seek(0) - if self.acl: - self.s3_client.upload_fileobj( - Bucket=self.bucketname, - Key=self.keyname, - Fileobj=file, - ExtraArgs={"ACL": self.acl}, - ) - else: - self.s3_client.upload_fileobj( - Bucket=self.bucketname, - Key=self.keyname, - Fileobj=file, - ) - file.close() + try: + if self.acl: + self.s3_client.upload_fileobj( + Bucket=self.bucketname, + Key=self.keyname, + Fileobj=file, + ExtraArgs={"ACL": self.acl}, + ) + else: + self.s3_client.upload_fileobj( + Bucket=self.bucketname, + Key=self.keyname, + Fileobj=file, + ) + finally: + file.close() class GCSFeedStorage(BlockingFeedStorage): diff --git a/scrapy/pqueues.py b/scrapy/pqueues.py index 0ad0b5d78..2efc43d4b 100644 --- a/scrapy/pqueues.py +++ b/scrapy/pqueues.py @@ -141,10 +141,14 @@ class ScrapyPriorityQueue: q = self.qfactory(priority) if q: self.queues[priority] = q + else: + q.close() if self._start_queue_cls: q = self._sqfactory(priority) if q: self._start_queues[priority] = q + else: + q.close() self.curprio = min(startprios) diff --git a/scrapy/utils/ftp.py b/scrapy/utils/ftp.py index 152f3374e..a3e7a4306 100644 --- a/scrapy/utils/ftp.py +++ b/scrapy/utils/ftp.py @@ -1,4 +1,5 @@ import posixpath +from contextlib import closing from ftplib import FTP, error_perm from posixpath import dirname from typing import IO @@ -32,7 +33,7 @@ def ftp_store_file( """Opens a FTP connection with passed credentials,sets current directory to the directory extracted from given path, then uploads the file to server """ - with FTP() as ftp: + with FTP() as ftp, closing(file): ftp.connect(host, port) ftp.login(username, password) if use_active_mode: @@ -42,4 +43,3 @@ def ftp_store_file( ftp_makedirs_cwd(ftp, dirname) command = "STOR" if overwrite else "APPE" ftp.storbinary(f"{command} {filename}", file) - file.close() diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index 09b67805d..aa77e692a 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -149,11 +149,12 @@ def install_scrapy_root_handler(settings: Settings) -> None: def _uninstall_scrapy_root_handler() -> None: global _scrapy_root_handler # noqa: PLW0603 - if ( - _scrapy_root_handler is not None - and _scrapy_root_handler in logging.root.handlers - ): + if _scrapy_root_handler is None: + return + + if _scrapy_root_handler in logging.root.handlers: logging.root.removeHandler(_scrapy_root_handler) + _scrapy_root_handler.close() _scrapy_root_handler = None diff --git a/scrapy/utils/test.py b/scrapy/utils/test.py index b4e20c3c6..90ed70262 100644 --- a/scrapy/utils/test.py +++ b/scrapy/utils/test.py @@ -69,6 +69,7 @@ def get_crawler( # When needed, useful settings can be added here, e.g. ones that prevent # deprecation warnings. settings: dict[str, Any] = { + "TELNETCONSOLE_ENABLED": False, **get_reactor_settings(), **(settings_dict or {}), } diff --git a/tests/test_command_runspider.py b/tests/test_command_runspider.py index b1455611e..9dabbc942 100644 --- a/tests/test_command_runspider.py +++ b/tests/test_command_runspider.py @@ -212,6 +212,7 @@ class MySpider(scrapy.Spider): f"Using asyncio event loop: {loop.__module__}.{loop.__class__.__name__}" in log ) + loop.close() def test_no_reactor(self, tmp_path: Path) -> None: log = self.get_log( diff --git a/tests/test_command_shell.py b/tests/test_command_shell.py index 1c109a333..f24200f53 100644 --- a/tests/test_command_shell.py +++ b/tests/test_command_shell.py @@ -181,6 +181,10 @@ class TestInteractiveShell: p.expect_exact("HtmlResponse") p.sendeof() p.wait() # type: ignore[no-untyped-call] + if p.proc.stdin: + p.proc.stdin.close() + if p.proc.stdout: + p.proc.stdout.close() logfile.seek(0) assert "Traceback" not in logfile.read().decode() diff --git a/tests/test_crawler_subprocess.py b/tests/test_crawler_subprocess.py index 146d94ecd..e3f9ac161 100644 --- a/tests/test_crawler_subprocess.py +++ b/tests/test_crawler_subprocess.py @@ -215,6 +215,10 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): p.expect_exact("shutting down gracefully") p.expect_exact("Spider closed (shutdown)") p.wait() # type: ignore[no-untyped-call] + if p.proc.stdin: + p.proc.stdin.close() + if p.proc.stdout: + p.proc.stdout.close() def test_shutdown_graceful(self) -> None: self._test_shutdown_graceful() @@ -232,6 +236,10 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): p.kill(sig) p.expect_exact("forcing unclean shutdown") p.wait() # type: ignore[no-untyped-call] + if p.proc.stdin: + p.proc.stdin.close() + if p.proc.stdout: + p.proc.stdout.close() @coroutine_test async def test_shutdown_forced(self) -> None: diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 2fded613d..877e0708b 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -3,7 +3,6 @@ import json import marshal import pickle import re -import tempfile from abc import ABC, abstractmethod from datetime import datetime from io import BytesIO @@ -242,7 +241,6 @@ class TestPickleItemExporterDataclass(TestPickleItemExporter): class TestMarshalItemExporter(TestBaseItemExporter): def _get_exporter(self, **kwargs): - self.output = tempfile.TemporaryFile() return MarshalItemExporter(self.output, **kwargs) def _check_output(self): @@ -252,7 +250,7 @@ class TestMarshalItemExporter(TestBaseItemExporter): def test_nonstring_types_item(self): item = self._get_nonstring_types_item() item.pop("time") # datetime is not marshallable - fp = tempfile.TemporaryFile() + fp = BytesIO() ie = MarshalItemExporter(fp) ie.start_exporting() ie.export_item(item) @@ -260,6 +258,7 @@ class TestMarshalItemExporter(TestBaseItemExporter): del ie # See the first “del self.ie” in this file for context. fp.seek(0) assert marshal.load(fp) == item + fp.close() class TestMarshalItemExporterDataclass(TestMarshalItemExporter): @@ -269,7 +268,11 @@ class TestMarshalItemExporterDataclass(TestMarshalItemExporter): class TestCsvItemExporter(TestBaseItemExporter): def _get_exporter(self, **kwargs): - self.output = tempfile.TemporaryFile() + # We need a fresh instance for each exporter, because + # CsvItemExporter.stream.__del__() closes the underlying file + # (CsvItemExporter.finish_exporting() calls detach() but not all tests + # call it). + self.output = BytesIO() return CsvItemExporter(self.output, **kwargs) def assertCsvEqual(self, first, second, msg=None): @@ -389,6 +392,11 @@ class TestCsvItemExporterDataclass(TestCsvItemExporter): class TestXmlItemExporter(TestBaseItemExporter): def _get_exporter(self, **kwargs): + # We need a fresh instance for each exporter, because + # XmlItemExporter.stream.__del__() closes the underlying file + # (XmlItemExporter.finish_exporting() calls detach() but not all tests + # call it). + self.output = BytesIO() return XmlItemExporter(self.output, **kwargs) def assertXmlEquivalent(self, first, second, msg=None): diff --git a/tests/test_extension_telnet.py b/tests/test_extension_telnet.py index 3f9135867..20c801558 100644 --- a/tests/test_extension_telnet.py +++ b/tests/test_extension_telnet.py @@ -1,61 +1,86 @@ +from __future__ import annotations + +from contextlib import contextmanager +from typing import TYPE_CHECKING, Any + import pytest from twisted.conch.telnet import ITelnetProtocol from twisted.cred import credentials from scrapy.extensions.telnet import TelnetConsole +from scrapy.utils.defer import maybe_deferred_to_future from scrapy.utils.test import get_crawler -from tests.utils.decorators import inline_callbacks_test +from tests.utils.decorators import coroutine_test + +if TYPE_CHECKING: + from collections.abc import Generator + + from scrapy.crawler import Crawler pytestmark = pytest.mark.requires_reactor # TelnetConsole requires a reactor -class TestTelnetExtension: - def _get_console_and_portal(self, settings=None): - crawler = get_crawler(settings_dict=settings) - console = TelnetConsole(crawler) +def _get_crawler(settings_dict: dict[str, Any] | None = None) -> Crawler: + settings = { + "TELNETCONSOLE_ENABLED": True, + **(settings_dict or {}), + } + return get_crawler(settings_dict=settings) - # This function has some side effects we don't need for this test - console._get_telnet_vars = dict - console.start_listening() - protocol = console.protocol() - portal = protocol.protocolArgs[0] +@contextmanager +def _get_console_and_portal( + settings: dict[str, Any] | None = None, +) -> Generator[tuple[TelnetConsole, Any]]: + crawler = _get_crawler(settings_dict=settings) + console = TelnetConsole(crawler) - return console, portal + # This function has some side effects we don't need for this test + console._get_telnet_vars = dict # type: ignore[method-assign] - @inline_callbacks_test - def test_bad_credentials(self): - console, portal = self._get_console_and_portal() + console.start_listening() + protocol = console.protocol() + portal = protocol.protocolArgs[0] + + try: + yield console, portal + finally: + console.stop_listening() + + +@coroutine_test +async def test_bad_credentials() -> None: + with _get_console_and_portal() as (_, portal): creds = credentials.UsernamePassword(b"username", b"password") d = portal.login(creds, None, ITelnetProtocol) with pytest.raises(ValueError, match="Invalid credentials"): - yield d - console.stop_listening() + await maybe_deferred_to_future(d) - @inline_callbacks_test - def test_good_credentials(self): - console, portal = self._get_console_and_portal() + +@coroutine_test +async def test_good_credentials() -> None: + with _get_console_and_portal() as (console, portal): creds = credentials.UsernamePassword( console.username.encode("utf8"), console.password.encode("utf8") ) d = portal.login(creds, None, ITelnetProtocol) - yield d - console.stop_listening() + await maybe_deferred_to_future(d) - @inline_callbacks_test - def test_custom_credentials(self): - settings = { - "TELNETCONSOLE_USERNAME": "user", - "TELNETCONSOLE_PASSWORD": "pass", - } - console, portal = self._get_console_and_portal(settings=settings) + +@coroutine_test +async def test_custom_credentials() -> None: + settings = { + "TELNETCONSOLE_USERNAME": "user", + "TELNETCONSOLE_PASSWORD": "pass", + } + with _get_console_and_portal(settings=settings) as (_, portal): creds = credentials.UsernamePassword(b"user", b"pass") d = portal.login(creds, None, ITelnetProtocol) - yield d - console.stop_listening() + await maybe_deferred_to_future(d) - def test_invalid_reversed_portrange(self): - settings = {"TELNETCONSOLE_PORT": [2, 1]} - console = TelnetConsole(get_crawler(settings_dict=settings)) - with pytest.raises(ValueError, match=r"invalid portrange: \[2, 1\]"): - console.start_listening() + +def test_invalid_reversed_portrange() -> None: + settings = {"TELNETCONSOLE_PORT": [2, 1]} + console = TelnetConsole(_get_crawler(settings_dict=settings)) + with pytest.raises(ValueError, match=r"invalid portrange: \[2, 1\]"): + console.start_listening() diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index c1d6f04eb..d7ea60cc8 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -87,6 +87,7 @@ class DummyBlockingFeedStorage(BlockingFeedStorage): class FailingBlockingFeedStorage(DummyBlockingFeedStorage): def _store_in_thread(self, file): + file.close() raise OSError("Cannot store") @@ -477,9 +478,14 @@ class TestFeedExport(TestFeedExportBase): }, } crawler = get_crawler(ItemSpider, settings) + + def store(file: IO[bytes]) -> None: + file.close() + raise KeyError("foo") + with mock.patch( "scrapy.extensions.feedexport.FileFeedStorage.store", - side_effect=KeyError("foo"), + side_effect=store, ): yield crawler.crawl(mockserver=self.mockserver) assert "feedexport/failed_count/FileFeedStorage" in crawler.stats.get_stats() diff --git a/tests/test_squeues.py b/tests/test_squeues.py index ddc12766c..8544602af 100644 --- a/tests/test_squeues.py +++ b/tests/test_squeues.py @@ -42,6 +42,7 @@ def nonserializable_object_test(self): ValueError, match=r"unmarshallable object|can't pickle Selector objects" ): q.push(sel) + q.close() class FifoDiskQueueTestMixin: @@ -53,6 +54,7 @@ class FifoDiskQueueTestMixin: assert q.pop() == "a" assert q.pop() == 123 assert q.pop() == {"a": "dict"} + q.close() test_nonserializable_object = nonserializable_object_test @@ -93,6 +95,7 @@ class PickleFifoDiskQueueTest(t.FifoDiskQueueTest, FifoDiskQueueTestMixin): i2 = q.pop() assert isinstance(i2, MyItem) assert i == i2 + q.close() def test_serialize_loader(self): q = self.queue() @@ -102,6 +105,7 @@ class PickleFifoDiskQueueTest(t.FifoDiskQueueTest, FifoDiskQueueTestMixin): assert isinstance(loader2, MyLoader) assert loader2.default_item_class is MyItem assert loader2.name_out("x") == "xx" + q.close() def test_serialize_request_recursive(self): q = self.queue() @@ -112,6 +116,7 @@ class PickleFifoDiskQueueTest(t.FifoDiskQueueTest, FifoDiskQueueTestMixin): assert isinstance(r2, Request) assert r.url == r2.url assert r2.meta["request"] is r2 + q.close() def test_non_pickable_object(self): q = self.queue() @@ -158,6 +163,7 @@ class LifoDiskQueueTestMixin: assert q.pop() == {"a": "dict"} assert q.pop() == 123 assert q.pop() == "a" + q.close() test_nonserializable_object = nonserializable_object_test @@ -178,6 +184,7 @@ class PickleLifoDiskQueueTest(t.LifoDiskQueueTest, LifoDiskQueueTestMixin): i2 = q.pop() assert isinstance(i2, MyItem) assert i == i2 + q.close() def test_serialize_loader(self): q = self.queue() @@ -187,6 +194,7 @@ class PickleLifoDiskQueueTest(t.LifoDiskQueueTest, LifoDiskQueueTestMixin): assert isinstance(loader2, MyLoader) assert loader2.default_item_class is MyItem assert loader2.name_out("x") == "xx" + q.close() def test_serialize_request_recursive(self): q = self.queue() @@ -197,3 +205,4 @@ class PickleLifoDiskQueueTest(t.LifoDiskQueueTest, LifoDiskQueueTestMixin): assert isinstance(r2, Request) assert r.url == r2.url assert r2.meta["request"] is r2 + q.close() diff --git a/tests/test_squeues_request.py b/tests/test_squeues_request.py index 847a76ab6..c779b005f 100644 --- a/tests/test_squeues_request.py +++ b/tests/test_squeues_request.py @@ -70,7 +70,6 @@ class TestRequestQueueBase(ABC): if test_peek: assert q.peek() is None assert q.pop() is None - q.close() @pytest.mark.parametrize("test_peek", [True, False]) def test_order(self, q: queuelib.queue.BaseQueue, test_peek: bool): @@ -108,7 +107,6 @@ class TestRequestQueueBase(ABC): if test_peek: assert q.peek() is None assert q.pop() is None - q.close() class TestPickleFifoDiskQueueRequest(TestRequestQueueBase): @@ -116,9 +114,13 @@ class TestPickleFifoDiskQueueRequest(TestRequestQueueBase): @pytest.fixture def q(self, crawler, tmp_path): - return PickleFifoDiskQueue.from_crawler( + queue = PickleFifoDiskQueue.from_crawler( crawler=crawler, key=str(tmp_path / "pickle" / "fifo") ) + try: + yield queue + finally: + queue.close() class TestPickleLifoDiskQueueRequest(TestRequestQueueBase): @@ -126,9 +128,13 @@ class TestPickleLifoDiskQueueRequest(TestRequestQueueBase): @pytest.fixture def q(self, crawler, tmp_path): - return PickleLifoDiskQueue.from_crawler( + queue = PickleLifoDiskQueue.from_crawler( crawler=crawler, key=str(tmp_path / "pickle" / "lifo") ) + try: + yield queue + finally: + queue.close() class TestMarshalFifoDiskQueueRequest(TestRequestQueueBase): @@ -136,9 +142,13 @@ class TestMarshalFifoDiskQueueRequest(TestRequestQueueBase): @pytest.fixture def q(self, crawler, tmp_path): - return MarshalFifoDiskQueue.from_crawler( + queue = MarshalFifoDiskQueue.from_crawler( crawler=crawler, key=str(tmp_path / "marshal" / "fifo") ) + try: + yield queue + finally: + queue.close() class TestMarshalLifoDiskQueueRequest(TestRequestQueueBase): @@ -146,9 +156,13 @@ class TestMarshalLifoDiskQueueRequest(TestRequestQueueBase): @pytest.fixture def q(self, crawler, tmp_path): - return MarshalLifoDiskQueue.from_crawler( + queue = MarshalLifoDiskQueue.from_crawler( crawler=crawler, key=str(tmp_path / "marshal" / "lifo") ) + try: + yield queue + finally: + queue.close() class TestFifoMemoryQueueRequest(TestRequestQueueBase): diff --git a/tests/test_stats.py b/tests/test_stats.py index 6e6aa0cc4..05f609fda 100644 --- a/tests/test_stats.py +++ b/tests/test_stats.py @@ -131,6 +131,7 @@ class TestStatsCollector: @coroutine_test async def test_deprecated_spider_arg_custom_collector(self) -> None: + # the class reimplements many methods because those are called during the test crawl class CustomStatsCollector: def __init__(self, crawler): self._stats = {} @@ -141,10 +142,19 @@ class TestStatsCollector: def get_stats(self, spider=None): return self._stats + def get_value(self, key, default=None, spider=None): + return self._stats.get(key, default) + + def set_value(self, key, value, spider=None): + self._stats[key] = value + def inc_value(self, key, count=1, start=0, spider=None): d = self._stats d[key] = d.setdefault(key, start) + count + def max_value(self, key, value, spider=None) -> None: + self._stats[key] = max(self._stats.setdefault(key, value), value) + def close_spider(self, spider, reason): pass diff --git a/tests/test_utils_asyncio.py b/tests/test_utils_asyncio.py index 5532b4a31..a198d1c09 100644 --- a/tests/test_utils_asyncio.py +++ b/tests/test_utils_asyncio.py @@ -149,3 +149,12 @@ class TestAsyncioLoopingCall: with pytest.raises(TypeError): looping_call.start(0.1) assert not looping_call.running + + @coroutine_test + async def test_looping_function_raises( + self, caplog: pytest.LogCaptureFixture + ) -> None: + looping_call = AsyncioLoopingCall(lambda: 1 / 0) + looping_call.start(0.1) + assert not looping_call.running + assert "Error calling the AsyncioLoopingCall function" in caplog.text diff --git a/tox.ini b/tox.ini index 2fa7b455d..5017d7636 100644 --- a/tox.ini +++ b/tox.ini @@ -53,6 +53,7 @@ deps = {[test-requirements]deps} pytest >= 8.4.1 # https://github.com/pytest-dev/pytest/pull/13502 passenv = + PYTHONTRACEMALLOC PYTEST_ADDOPTS S3_TEST_FILE_URI AWS_ACCESS_KEY_ID From 185d6b9a20b7d0e77f4c60435d17e5072bd4d704 Mon Sep 17 00:00:00 2001 From: greymoth Date: Sat, 27 Jun 2026 04:53:32 +0900 Subject: [PATCH 08/28] Fix request_to_curl() corrupting dict cookies with bytes keys/values (#7675) * Fix request_to_curl() corrupting dict cookies with bytes keys/values PR #7603 made the list-cookie branch of request_to_curl() bytes-safe via _cookie_value_to_unicode(), but left the sibling dict-cookie branch using raw f-string interpolation. A dict cookie with bytes keys/values (a supported and common form, e.g. Request(url, cookies={b"k": b"v"})) was rendered as --cookie 'b'k'=b'v'' instead of --cookie 'k=v', producing a broken curl command. Route the dict branch through the same _cookie_value_to_unicode() helper, mirroring the list branch. Add a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) * fix: apply _cookie_value_to_unicode to list-branch cookie key/value --------- Co-authored-by: Claude Opus 4.8 (1M context) --- scrapy/utils/request.py | 7 +++++-- tests/test_utils_request.py | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/scrapy/utils/request.py b/scrapy/utils/request.py index 398403d90..4a85526c0 100644 --- a/scrapy/utils/request.py +++ b/scrapy/utils/request.py @@ -204,13 +204,16 @@ def request_to_curl(request: Request) -> str: cookies = "" if request.cookies: if isinstance(request.cookies, dict): - cookie = "; ".join(f"{k}={v}" for k, v in request.cookies.items()) + cookie = "; ".join( + f"{_cookie_value_to_unicode(k)}={_cookie_value_to_unicode(v)}" + for k, v in request.cookies.items() + ) cookies = f"--cookie '{cookie}'" elif isinstance(request.cookies, list): cookie = "; ".join( f"{_cookie_value_to_unicode(c['name'])}={_cookie_value_to_unicode(c['value'])}" if "name" in c and "value" in c - else f"{next(iter(c.keys()))}={next(iter(c.values()))}" + else f"{_cookie_value_to_unicode(next(iter(c.keys())))}={_cookie_value_to_unicode(next(iter(c.values())))}" for c in request.cookies ) cookies = f"--cookie '{cookie}'" diff --git a/tests/test_utils_request.py b/tests/test_utils_request.py index e4967d4e7..63926e9d2 100644 --- a/tests/test_utils_request.py +++ b/tests/test_utils_request.py @@ -401,6 +401,19 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) + def test_cookies_dict_bytes(self): + request_object = Request( + "https://www.httpbin.org/post", + method="POST", + cookies={b"foo": b"bar"}, + body=json.dumps({"foo": "bar"}), + ) + expected_curl_command = ( + "curl -X POST https://www.httpbin.org/post" + " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=bar'" + ) + self._test_request(request_object, expected_curl_command) + def test_cookies_list(self): request_object = Request( "https://www.httpbin.org/post", @@ -455,3 +468,16 @@ class TestRequestToCurl: " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=1'" ) self._test_request(request_object, expected_curl_command) + + def test_cookies_list_bytes_nonstandard_key(self): + request_object = Request( + "https://www.httpbin.org/post", + method="POST", + cookies=[{b"foo": b"bar"}], + body=json.dumps({"foo": "bar"}), + ) + expected_curl_command = ( + "curl -X POST https://www.httpbin.org/post" + " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=bar'" + ) + self._test_request(request_object, expected_curl_command) From 9559cbee1e7344d9746b2e2164d10a9d6575f9d1 Mon Sep 17 00:00:00 2001 From: Adrian Date: Mon, 29 Jun 2026 14:21:20 +0200 Subject: [PATCH 09/28] Solve timing issues with HTTP cache tests? (#7692) --- tests/test_downloadermiddleware_httpcache.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test_downloadermiddleware_httpcache.py b/tests/test_downloadermiddleware_httpcache.py index 6c86d7adf..9d5d6874e 100644 --- a/tests/test_downloadermiddleware_httpcache.py +++ b/tests/test_downloadermiddleware_httpcache.py @@ -6,6 +6,7 @@ import tempfile import time from contextlib import contextmanager from typing import TYPE_CHECKING, Any +from unittest import mock import pytest @@ -47,7 +48,6 @@ class TestBase: settings = { "HTTPCACHE_ENABLED": True, "HTTPCACHE_DIR": self.tmpdir, - "HTTPCACHE_EXPIRATION_SECS": 1, "HTTPCACHE_IGNORE_HTTP_CODES": [], "HTTPCACHE_POLICY": self.policy_class, "HTTPCACHE_STORAGE": self.storage_class, @@ -94,7 +94,7 @@ class StorageTestMixin: """Mixin containing storage-specific test methods.""" def test_storage(self): - with self._storage() as (storage, crawler): + with self._storage(HTTPCACHE_EXPIRATION_SECS=1) as (storage, crawler): request2 = self.request.copy() assert storage.retrieve_response(crawler.spider, request2) is None @@ -103,15 +103,17 @@ class StorageTestMixin: assert isinstance(response2, HtmlResponse) # content-type header self.assertEqualResponse(self.response, response2) - time.sleep(2) # wait for cache to expire - assert storage.retrieve_response(crawler.spider, request2) is None + expired = time.time() + storage.expiration_secs + 1 + with mock.patch("scrapy.extensions.httpcache.time", return_value=expired): + assert storage.retrieve_response(crawler.spider, request2) is None def test_storage_never_expire(self): with self._storage(HTTPCACHE_EXPIRATION_SECS=0) as (storage, crawler): assert storage.retrieve_response(crawler.spider, self.request) is None storage.store_response(crawler.spider, self.request, self.response) - time.sleep(0.5) # give the chance to expire - assert storage.retrieve_response(crawler.spider, self.request) + future = time.time() + 10**6 + with mock.patch("scrapy.extensions.httpcache.time", return_value=future): + assert storage.retrieve_response(crawler.spider, self.request) def test_storage_no_content_type_header(self): """Test that the response body is used to get the right response class From 4b2b56f3848111d55151ccfefddfc6b3d84d1302 Mon Sep 17 00:00:00 2001 From: Javier <114426455+javidiazz@users.noreply.github.com> Date: Mon, 29 Jun 2026 15:34:12 +0200 Subject: [PATCH 10/28] Update on Request objects (#7286) --- docs/topics/request-response.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index 5e70a4f98..f2fcfb0b5 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -63,7 +63,7 @@ Request objects .. invisible-code-block: python - from scrapy.http import Request + from scrapy import Request 1. Using a dict: From 6591cb756c16bdb0ce85425071fea84a7cddaf53 Mon Sep 17 00:00:00 2001 From: tanishqtayade Date: Mon, 29 Jun 2026 22:07:06 +0530 Subject: [PATCH 11/28] Fix to LocalCache with limit=0 still stores items rather than disabling the cache (#7663) * Fix cell-var-from-loop in _send_catch_log_deferred Replace lambda capturing receiver by reference with a default argument to capture it by value, fixing a potential bug where all deferred callbacks could reference the last receiver in the loop instead of their respective receivers. Removes the pylint disable comment and TODO that were suppressing this issue. * chore: trigger CI rerun for mypy network error * Fix mypy error: pass receiver via addBoth args instead of lambda default * style: apply pre-commit ruff formatting * fix: LocalCache with limit=0 incorrectly stores items When limit=0 is passed to LocalCache (e.g. when DNSCACHE_ENABLED=False), the condition 'if self.limit' evaluates to False due to Python's truthiness rules, causing items to be stored despite the cache being disabled. This leads to an unbounded memory leak during long crawls when DNS caching is explicitly disabled. Fix changes the condition to 'if self.limit is not None' and adds an early return when limit=0 to correctly handle the disabled cache case. * test: add resolver-level regression tests for DNSCACHE_ENABLED=False Add two regression tests that verify DNS results are not stored in dnscache when DNSCACHE_ENABLED=False (cache_size=0): - test_caching_hostname_resolver_dnscache_disabled_rejects_storage: verifies _CachingResolutionReceiver does not write to dnscache when CachingHostnameResolver is initialized with cache_size=0 - test_caching_threaded_resolver_dnscache_disabled_rejects_storage: verifies dnscache rejects storage at the LocalCache level when limit=0 * test: drop misleading threaded resolver test it was writing directly to dnscache, not actually going through CachingThreadedResolver at all. the threaded resolver already has its own if dnscache.limit: guard so the bug doesnt affect it anyway. keeping only the hostname resolver test which covers the actual bug path through _CachingResolutionReceiver * test: clean up comments in resolver test * test: remove unnecessary comment --- scrapy/utils/datatypes.py | 4 +++- tests/test_resolver.py | 18 ++++++++++++++++++ tests/test_utils_datatypes.py | 10 ++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/scrapy/utils/datatypes.py b/scrapy/utils/datatypes.py index 4e65c062e..dd0e062d0 100644 --- a/scrapy/utils/datatypes.py +++ b/scrapy/utils/datatypes.py @@ -152,7 +152,9 @@ class LocalCache(OrderedDict[_KT, _VT]): self.limit: int | None = limit def __setitem__(self, key: _KT, value: _VT) -> None: - if self.limit: + if self.limit is not None: + if self.limit == 0: + return while len(self) >= self.limit: self.popitem(last=False) super().__setitem__(key, value) diff --git a/tests/test_resolver.py b/tests/test_resolver.py index 83fc8693b..7cca45ed1 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -53,3 +53,21 @@ def test_caching_hostname_resolver_no_addresses_not_cached(): resolver.resolveHostName(Mock(), "example.com") assert "example.com" not in dnscache + + +def test_caching_hostname_resolver_dnscache_disabled_rejects_storage(): + + def fake_resolve(receiver, *_): + receiver.resolutionBegan(Mock()) + receiver.addressResolved(Mock()) + receiver.resolutionComplete() + return receiver + + reactor = Mock() + reactor.nameResolver.resolveHostName.side_effect = fake_resolve + + resolver = CachingHostnameResolver(reactor, cache_size=0) + resolver.resolveHostName(Mock(), "example.com") + + assert "example.com" not in dnscache + assert len(dnscache) == 0 diff --git a/tests/test_utils_datatypes.py b/tests/test_utils_datatypes.py index ba6b82503..fe1f60c7d 100644 --- a/tests/test_utils_datatypes.py +++ b/tests/test_utils_datatypes.py @@ -309,6 +309,16 @@ class TestLocalCache: assert str(x) in cache assert cache[str(x)] == x + def test_cache_with_zero_limit(self): + cache = LocalCache(limit=0) + cache["a"] = 1 + cache["b"] = 2 + cache["c"] = 3 + assert len(cache) == 0 + assert "a" not in cache + assert "b" not in cache + assert "c" not in cache + class TestLocalWeakReferencedCache: def test_cache_with_limit(self): From 52147017b43baff1f9b4668e46ab11341aa8f400 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Mon, 29 Jun 2026 21:49:09 +0500 Subject: [PATCH 12/28] Cleanup cookie handling in request_to_curl() (#7684) * Adjust CookiesT. * Drop list of plain cookie dicts support from request_to_curl(). * Extract _decode_cookie(). * Unify logging. * Extract _to_verbose_cookies(). * Sync and type hint _cookie_to_set_cookie_value() in tests. * Add tests for bytes in Request.cookies. * Type hint assertCookieValEqual(). --- scrapy/downloadermiddlewares/cookies.py | 32 ++------- scrapy/http/request/__init__.py | 2 +- scrapy/utils/request.py | 73 +++++++++++++------ tests/test_downloadermiddleware_cookies.py | 81 +++++++++++++--------- tests/test_utils_request.py | 44 +++--------- 5 files changed, 116 insertions(+), 116 deletions(-) diff --git a/scrapy/downloadermiddlewares/cookies.py b/scrapy/downloadermiddlewares/cookies.py index cd8c2abca..34d071fd5 100644 --- a/scrapy/downloadermiddlewares/cookies.py +++ b/scrapy/downloadermiddlewares/cookies.py @@ -12,6 +12,7 @@ from scrapy.http.cookies import CookieJar from scrapy.utils.decorators import _warn_spider_arg from scrapy.utils.httpobj import urlparse_cached from scrapy.utils.python import to_unicode +from scrapy.utils.request import _decode_cookie, _to_verbose_cookies if TYPE_CHECKING: from collections.abc import Iterable, Sequence @@ -134,29 +135,10 @@ class CookiesMiddleware: Given a dict consisting of cookie components, return its string representation. Decode from bytes if necessary. """ - decoded = {} + decoded = _decode_cookie(cookie, request) + if decoded is None: + return None flags = set() - for key in ("name", "value", "path", "domain"): - value = cookie.get(key) - if value is None: - if key in {"name", "value"}: - msg = f"Invalid cookie found in request {request}: {cookie} ('{key}' is missing)" - logger.warning(msg) - return None - continue - if isinstance(value, (bool, float, int, str)): - decoded[key] = str(value) - else: - assert isinstance(value, bytes) - try: - decoded[key] = value.decode("utf8") - except UnicodeDecodeError: - logger.warning( - "Non UTF-8 encoded cookie found in request %s: %s", - request, - cookie, - ) - decoded[key] = value.decode("latin1", errors="replace") for flag in ("secure",): value = cookie.get(flag, _UNSET) if value is _UNSET or not value: @@ -177,11 +159,7 @@ class CookiesMiddleware: """ if not request.cookies: return () - cookies: Iterable[VerboseCookie] - if isinstance(request.cookies, dict): - cookies = tuple({"name": k, "value": v} for k, v in request.cookies.items()) - else: - cookies = request.cookies + cookies: Iterable[VerboseCookie] = _to_verbose_cookies(request.cookies) for cookie in cookies: cookie.setdefault("secure", urlparse_cached(request).scheme == "https") formatted = filter(None, (self._format_cookie(c, request) for c in cookies)) diff --git a/scrapy/http/request/__init__.py b/scrapy/http/request/__init__.py index 7db648a45..73c2e7dd4 100644 --- a/scrapy/http/request/__init__.py +++ b/scrapy/http/request/__init__.py @@ -51,7 +51,7 @@ class VerboseCookie(TypedDict): secure: NotRequired[bool] -CookiesT: TypeAlias = dict[str, str] | list[VerboseCookie] +CookiesT: TypeAlias = dict[str | bytes, str | bytes] | list[VerboseCookie] RequestTypeVar = TypeVar("RequestTypeVar", bound="Request") diff --git a/scrapy/utils/request.py b/scrapy/utils/request.py index 4a85526c0..b2fd9a834 100644 --- a/scrapy/utils/request.py +++ b/scrapy/utils/request.py @@ -7,6 +7,7 @@ from __future__ import annotations import hashlib import json +import logging from typing import TYPE_CHECKING, Any, Protocol from urllib.parse import urlunparse from weakref import WeakKeyDictionary @@ -25,6 +26,9 @@ if TYPE_CHECKING: from typing_extensions import Self from scrapy.crawler import Crawler + from scrapy.http.request import CookiesT, VerboseCookie + +logger = logging.getLogger(__name__) _fingerprint_cache: WeakKeyDictionary[ @@ -179,17 +183,52 @@ def _get_method(obj: Any, name: Any) -> Any: raise ValueError(f"Method {name!r} not found in: {obj}") from None -def _cookie_value_to_unicode(value: str | bytes | float) -> str: - if isinstance(value, bytes): - return value.decode() - return str(value) +def _to_verbose_cookies(cookies: CookiesT) -> list[VerboseCookie]: + """Return a list of verbose cookies from ``request.cookies``. + + The list of dicts form is returned as is, the dict one is converted first. + """ + if isinstance(cookies, dict): + return [{"name": k, "value": v} for k, v in cookies.items()] + return cookies + + +def _decode_cookie(cookie: VerboseCookie, request: Request) -> dict[str, str] | None: + """Return a dict with non-flag verbose cookie values converted to strings. + + ``name``, ``value``, ``path``, ``domain`` are included, ``secure`` isn't. + """ + + decoded = {} + for key in ("name", "value", "path", "domain"): + value = cookie.get(key) + if value is None: + if key in {"name", "value"}: + logger.warning( + f"Invalid cookie found in request {request}:" + f" {cookie} ('{key}' is missing)" + ) + return None + continue + if isinstance(value, (bool, float, int, str)): + decoded[key] = str(value) + else: + assert isinstance(value, bytes) + try: + decoded[key] = value.decode("utf8") + except UnicodeDecodeError: + logger.warning( + f"Non UTF-8 encoded cookie found in request {request}: {cookie}", + ) + decoded[key] = value.decode("latin1", errors="replace") + return decoded def request_to_curl(request: Request) -> str: """ Converts a :class:`~scrapy.Request` object to a curl command. - :param :class:`~scrapy.Request`: Request object to be converted + :param request: Request object to be converted :return: string containing the curl command """ method = request.method @@ -201,22 +240,14 @@ def request_to_curl(request: Request) -> str: ) url = request.url - cookies = "" - if request.cookies: - if isinstance(request.cookies, dict): - cookie = "; ".join( - f"{_cookie_value_to_unicode(k)}={_cookie_value_to_unicode(v)}" - for k, v in request.cookies.items() - ) - cookies = f"--cookie '{cookie}'" - elif isinstance(request.cookies, list): - cookie = "; ".join( - f"{_cookie_value_to_unicode(c['name'])}={_cookie_value_to_unicode(c['value'])}" - if "name" in c and "value" in c - else f"{_cookie_value_to_unicode(next(iter(c.keys())))}={_cookie_value_to_unicode(next(iter(c.values())))}" - for c in request.cookies - ) - cookies = f"--cookie '{cookie}'" + + cookie_list: list[VerboseCookie] = _to_verbose_cookies(request.cookies) + pairs = [ + f"{decoded['name']}={decoded['value']}" + for c in cookie_list + if (decoded := _decode_cookie(c, request)) is not None + ] + cookies = f"--cookie '{'; '.join(pairs)}'" if pairs else "" curl_cmd = f"curl -X {method} {url} {data} {headers} {cookies}".strip() return " ".join(curl_cmd.split()) diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index f79591020..b4419dc67 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -1,4 +1,5 @@ import logging +from collections.abc import Iterable import pytest from testfixtures import LogCapture @@ -8,30 +9,34 @@ from scrapy.downloadermiddlewares.defaultheaders import DefaultHeadersMiddleware from scrapy.downloadermiddlewares.redirect import RedirectMiddleware from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response +from scrapy.http.request import CookiesT, VerboseCookie from scrapy.utils.python import to_bytes +from scrapy.utils.request import _to_verbose_cookies from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler UNSET = object() -def _cookie_to_set_cookie_value(cookie): +def _cookie_to_set_cookie_value(cookie: VerboseCookie) -> str | None: """Given a cookie defined as a dictionary with name and value keys, and optional path and domain keys, return the equivalent string that can be associated to a ``Set-Cookie`` header.""" decoded = {} for key in ("name", "value", "path", "domain"): - if cookie.get(key) is None: - if key in ("name", "value"): + value = cookie.get(key) + if value is None: + if key in {"name", "value"}: return None continue - if isinstance(cookie[key], (bool, float, int, str)): - decoded[key] = str(cookie[key]) + if isinstance(value, (bool, float, int, str)): + decoded[key] = str(value) else: + assert isinstance(value, bytes) try: - decoded[key] = cookie[key].decode("utf8") + decoded[key] = value.decode("utf8") except UnicodeDecodeError: - decoded[key] = cookie[key].decode("latin1", errors="replace") + decoded[key] = value.decode("latin1", errors="replace") cookie_str = f"{decoded.pop('name')}={decoded.pop('value')}" for key, value in decoded.items(): # path, domain @@ -39,24 +44,30 @@ def _cookie_to_set_cookie_value(cookie): return cookie_str -def _cookies_to_set_cookie_list(cookies): +def _cookies_to_set_cookie_list(cookies: CookiesT) -> Iterable[str]: """Given a group of cookie defined either as a dictionary or as a list of dictionaries (i.e. in a format supported by the cookies parameter of Request), return the equivalent list of strings that can be associated to a ``Set-Cookie`` header.""" if not cookies: return [] - if isinstance(cookies, dict): - cookies = ({"name": k, "value": v} for k, v in cookies.items()) - return filter(None, (_cookie_to_set_cookie_value(cookie) for cookie in cookies)) + return filter( + None, + ( + _cookie_to_set_cookie_value(cookie) + for cookie in _to_verbose_cookies(cookies) + ), + ) class TestCookiesMiddleware: - def assertCookieValEqual(self, first, second, msg=None): - def split_cookies(cookies): + @staticmethod + def assertCookieValEqual(first: bytes | str | None, second: bytes | str) -> None: + def split_cookies(cookies: bytes | str) -> list[bytes]: return sorted([s.strip() for s in to_bytes(cookies).split(b";")]) - assert split_cookies(first) == split_cookies(second), msg + assert first is not None + assert split_cookies(first) == split_cookies(second) def setup_method(self): crawler = get_crawler(DefaultSpider) @@ -372,21 +383,25 @@ class TestCookiesMiddleware: assert self.mw.process_request(req3) is None self.assertCookieValEqual(req3.headers["Cookie"], "a=new; c=d; e=f") - def test_request_cookies_encoding(self): - # 1) UTF8-encoded bytes - req1 = Request("http://example.org", cookies={"a": "á".encode()}) - assert self.mw.process_request(req1) is None - self.assertCookieValEqual(req1.headers["Cookie"], b"a=\xc3\xa1") - - # 2) Non UTF8-encoded bytes - req2 = Request("http://example.org", cookies={"a": "á".encode("latin1")}) - assert self.mw.process_request(req2) is None - self.assertCookieValEqual(req2.headers["Cookie"], b"a=\xc3\xa1") - - # 3) String - req3 = Request("http://example.org", cookies={"a": "á"}) - assert self.mw.process_request(req3) is None - self.assertCookieValEqual(req3.headers["Cookie"], b"a=\xc3\xa1") + @pytest.mark.parametrize( + "cookies", + [ + # UTF8-encoded bytes + {"a": "á".encode()}, + # non UTF8-encoded bytes + {"a": "á".encode("latin1")}, + # string + {"a": "á"}, + # key as bytes + {b"a": "á"}, + # key and value as bytes + {b"a": "á".encode()}, + ], + ) + def test_request_cookies_encoding(self, cookies: CookiesT) -> None: + req = Request("http://example.org", cookies=cookies) + assert self.mw.process_request(req) is None + self.assertCookieValEqual(req.headers["Cookie"], b"a=\xc3\xa1") @pytest.mark.xfail(reason="Cookie header is not currently being processed") def test_request_headers_cookie_encoding(self): @@ -410,7 +425,7 @@ class TestCookiesMiddleware: Invalid cookies are logged as warnings and discarded """ with LogCapture( - "scrapy.downloadermiddlewares.cookies", + "scrapy.utils.request", propagate=False, level=logging.INFO, ) as lc: @@ -425,19 +440,19 @@ class TestCookiesMiddleware: assert self.mw.process_request(req3) is None lc.check( ( - "scrapy.downloadermiddlewares.cookies", + "scrapy.utils.request", "WARNING", "Invalid cookie found in request :" " {'value': 'bar', 'secure': False} ('name' is missing)", ), ( - "scrapy.downloadermiddlewares.cookies", + "scrapy.utils.request", "WARNING", "Invalid cookie found in request :" " {'name': 'foo', 'secure': False} ('value' is missing)", ), ( - "scrapy.downloadermiddlewares.cookies", + "scrapy.utils.request", "WARNING", "Invalid cookie found in request :" " {'name': 'foo', 'value': None, 'secure': False} ('value' is missing)", diff --git a/tests/test_utils_request.py b/tests/test_utils_request.py index 63926e9d2..1642a932b 100644 --- a/tests/test_utils_request.py +++ b/tests/test_utils_request.py @@ -354,16 +354,18 @@ class TestCustomRequestFingerprinter: class TestRequestToCurl: - def _test_request(self, request_object, expected_curl_command): + def _test_request( + self, request_object: Request, expected_curl_command: str + ) -> None: curl_command = request_to_curl(request_object) assert curl_command == expected_curl_command - def test_get(self): + def test_get(self) -> None: request_object = Request("https://www.example.com") expected_curl_command = "curl -X GET https://www.example.com" self._test_request(request_object, expected_curl_command) - def test_post(self): + def test_post(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -374,7 +376,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_headers(self): + def test_headers(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -388,7 +390,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_cookies_dict(self): + def test_cookies_dict(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -401,7 +403,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_cookies_dict_bytes(self): + def test_cookies_dict_bytes(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -414,20 +416,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_cookies_list(self): - request_object = Request( - "https://www.httpbin.org/post", - method="POST", - cookies=[{"foo": "bar"}], - body=json.dumps({"foo": "bar"}), - ) - expected_curl_command = ( - "curl -X POST https://www.httpbin.org/post" - " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=bar'" - ) - self._test_request(request_object, expected_curl_command) - - def test_cookies_list_verbose(self): + def test_cookies_list_verbose(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -448,7 +437,7 @@ class TestRequestToCurl: ) self._test_request(request_object, expected_curl_command) - def test_cookies_list_verbose_non_string_value(self): + def test_cookies_list_verbose_non_string_value(self) -> None: request_object = Request( "https://www.httpbin.org/post", method="POST", @@ -468,16 +457,3 @@ class TestRequestToCurl: " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=1'" ) self._test_request(request_object, expected_curl_command) - - def test_cookies_list_bytes_nonstandard_key(self): - request_object = Request( - "https://www.httpbin.org/post", - method="POST", - cookies=[{b"foo": b"bar"}], - body=json.dumps({"foo": "bar"}), - ) - expected_curl_command = ( - "curl -X POST https://www.httpbin.org/post" - " --data-raw '{\"foo\": \"bar\"}' --cookie 'foo=bar'" - ) - self._test_request(request_object, expected_curl_command) From 6ad8a043cae3d4e0db906788cb60a4c8a3fd7d91 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 30 Jun 2026 12:05:42 +0200 Subject: [PATCH 13/28] Fix genspider --editor (#7683) --- scrapy/commands/edit.py | 23 +++++++++++++-- scrapy/commands/genspider.py | 12 +++++--- tests/test_command_genspider.py | 50 +++++++++++++++++++++++++++++++++ tests/test_commands.py | 28 ++++++++++++++++++ 4 files changed, 106 insertions(+), 7 deletions(-) diff --git a/scrapy/commands/edit.py b/scrapy/commands/edit.py index cd7c57f28..fa75cb09c 100644 --- a/scrapy/commands/edit.py +++ b/scrapy/commands/edit.py @@ -1,12 +1,29 @@ -import argparse +from __future__ import annotations + import os +import shlex +import subprocess import sys -from typing import Any, ClassVar +from pathlib import Path +from typing import TYPE_CHECKING, Any, ClassVar from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError from scrapy.spiderloader import get_spider_loader +if TYPE_CHECKING: + import argparse + + +def _edit_file(editor: str, file_path: str | os.PathLike[str]) -> int: + """Open ``file_path`` with ``editor`` and return the editor exit code. + + ``editor`` may include arguments (e.g. ``"code -w"``); it is split with + :func:`shlex.split` and the file is passed as a separate argument, so no + shell is involved. + """ + return subprocess.call([*shlex.split(editor), os.fspath(file_path)]) # noqa: S603 + class Command(ScrapyCommand): requires_project = True @@ -45,4 +62,4 @@ class Command(ScrapyCommand): sfile = sys.modules[spidercls.__module__].__file__ assert sfile sfile = sfile.replace(".pyc", ".py") - self.exitcode = os.system(f'{editor} "{sfile}"') # noqa: S605 + self.exitcode = _edit_file(editor, Path(sfile)) diff --git a/scrapy/commands/genspider.py b/scrapy/commands/genspider.py index cc8624fa1..4277232c3 100644 --- a/scrapy/commands/genspider.py +++ b/scrapy/commands/genspider.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os import shutil import string from importlib import import_module @@ -10,12 +9,14 @@ from urllib.parse import urlparse import scrapy from scrapy.commands import ScrapyCommand +from scrapy.commands.edit import _edit_file from scrapy.exceptions import UsageError from scrapy.spiderloader import get_spider_loader from scrapy.utils.template import render_templatefile, string_camelcase if TYPE_CHECKING: import argparse + import os def sanitize_module_name(module_name: str) -> str: @@ -118,9 +119,11 @@ class Command(ScrapyCommand): template_file = self._find_template(opts.template) if template_file: - self._genspider(module, name, url, opts.template, template_file) + spider_file = self._genspider( + module, name, url, opts.template, template_file + ) if opts.edit: - self.exitcode = os.system(f'scrapy edit "{name}"') # noqa: S605 + self.exitcode = _edit_file(self.settings["EDITOR"], spider_file) def _generate_template_variables( self, @@ -148,7 +151,7 @@ class Command(ScrapyCommand): url: str, template_name: str, template_file: str | os.PathLike[str], - ) -> None: + ) -> Path: """Generate the spider module, based on the given template""" assert self.settings is not None tvars = self._generate_template_variables(module, name, url, template_name) @@ -168,6 +171,7 @@ class Command(ScrapyCommand): ) if spiders_module: print(f"in module:\n {spiders_module.__name__}.{module}") + return Path(spider_file) def _find_template(self, template: str) -> Path | None: template_file = Path(self.templates_dir, f"{template}.tmpl") diff --git a/tests/test_command_genspider.py b/tests/test_command_genspider.py index 67e3eb50a..94db14c30 100644 --- a/tests/test_command_genspider.py +++ b/tests/test_command_genspider.py @@ -1,6 +1,7 @@ from __future__ import annotations import re +import sys from pathlib import Path import pytest @@ -9,6 +10,13 @@ from tests.test_commands import TestProjectBase from tests.utils.cmdline import call, proc +def write_recording_editor(editor: Path) -> None: + """Create an executable editor script that writes the path it is asked to + open (its last argument) into the file given as its first argument.""" + editor.write_text('#!/bin/sh\nprintf "%s" "$2" > "$1"\n', encoding="utf-8") + editor.chmod(0o755) + + def find_in_file(filename: Path, regex: str) -> re.Match[str] | None: """Find first pattern occurrence in file""" pattern = re.compile(regex) @@ -63,6 +71,28 @@ class TestGenspiderCommand(TestProjectBase): assert call("genspider", "--dump=basic", cwd=proj_path) == 0 assert call("genspider", "-d", "basic", cwd=proj_path) == 0 + @pytest.mark.skipif( + sys.platform == "win32", reason="requires a POSIX shell editor script" + ) + def test_edit(self, proj_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + spider = proj_path / self.project_name / "spiders" / "example2.py" + edited = proj_path / "edited.txt" + editor = proj_path / "fake-editor.sh" + write_recording_editor(editor) + # The extra argument exercises shlex-splitting of the EDITOR value. + monkeypatch.setenv("EDITOR", f"{editor} {edited}") + + returncode, _, err = proc( + "genspider", "--edit", "example2", "example2.com", cwd=proj_path + ) + + assert returncode == 0, err + assert "ModuleNotFoundError" not in err + assert spider.exists() + assert (proj_path / edited.read_text(encoding="utf-8")).resolve() == ( + spider.resolve() + ) + def test_same_name_as_project(self, proj_path: Path) -> None: assert call("genspider", self.project_name, cwd=proj_path) == 2 assert not ( @@ -168,6 +198,26 @@ class TestGenspiderStandaloneCommand: call("genspider", "example", "example.com", cwd=tmp_path) assert Path(tmp_path, "example.py").exists() + @pytest.mark.skipif( + sys.platform == "win32", reason="requires a POSIX shell editor script" + ) + def test_edit(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + spider = tmp_path / "example.py" + edited = tmp_path / "edited.txt" + editor = tmp_path / "fake-editor.sh" + write_recording_editor(editor) + monkeypatch.setenv("EDITOR", f"{editor} {edited}") + + returncode, _, err = proc( + "genspider", "--edit", "example", "example.com", cwd=tmp_path + ) + + assert returncode == 0, err + assert spider.exists() + assert (tmp_path / edited.read_text(encoding="utf-8")).resolve() == ( + spider.resolve() + ) + @pytest.mark.parametrize("force", [True, False]) def test_same_name_as_existing_file(self, force: bool, tmp_path: Path) -> None: file_name = "example" diff --git a/tests/test_commands.py b/tests/test_commands.py index d7b7a9ff2..9f81e8602 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2,6 +2,7 @@ from __future__ import annotations import argparse import json +import sys from io import StringIO from shutil import copytree from typing import TYPE_CHECKING @@ -420,6 +421,33 @@ class TestViewCommand: assert "URL using the Scrapy downloader and show its" in command.long_desc() +class TestEditCommand(TestProjectBase): + @pytest.mark.skipif( + sys.platform == "win32", reason="requires a POSIX shell editor script" + ) + def test_edit(self, proj_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + spider = proj_path / self.project_name / "spiders" / "example.py" + edited = proj_path / "edited.txt" + editor = proj_path / "fake-editor.sh" + # Records the file it is asked to open ($2) into the file given as $1. + editor.write_text('#!/bin/sh\nprintf "%s" "$2" > "$1"\n', encoding="utf-8") + editor.chmod(0o755) + monkeypatch.setenv("EDITOR", f"{editor} {edited}") + + assert call("genspider", "example", "example.com", cwd=proj_path) == 0 + returncode, _, err = proc("edit", "example", cwd=proj_path) + + assert returncode == 0, err + assert (proj_path / edited.read_text(encoding="utf-8")).resolve() == ( + spider.resolve() + ) + + def test_edit_spider_not_found(self, proj_path: Path) -> None: + returncode, _, err = proc("edit", "nonexistent", cwd=proj_path) + assert returncode == 1 + assert "Spider not found: nonexistent" in err + + class TestHelpMessage(TestProjectBase): @pytest.mark.parametrize( "command", From deb7e2861e616bbfefb96433bd72e0d50055cd0e Mon Sep 17 00:00:00 2001 From: Gaurav Yadav Date: Tue, 30 Jun 2026 18:19:47 +0530 Subject: [PATCH 14/28] Fix _get_tag_name() crash for non-string elem.tag (#7686) (#7687) * Fix _get_tag_name() crash for non-string elem.tag (#7686) * test: improve non-string tag test accuracy and add direct unit test * test: use minimal payload for non-string tag test * test: address Adrian+syncrain PR feedback - remove first docstring line (Adrian: unnecessary) - replace weak isinstance assert with no-op call - keep Cython function mention (Adrian: wording is great) * test: replace silent call with assert results == [] per Adrian * chore: drop accidental pyproject.toml and uv.lock changes --- scrapy/utils/sitemap.py | 9 +++++---- tests/test_utils_sitemap.py | 11 +++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/scrapy/utils/sitemap.py b/scrapy/utils/sitemap.py index 1520a4ff0..03b7bf3b1 100644 --- a/scrapy/utils/sitemap.py +++ b/scrapy/utils/sitemap.py @@ -97,10 +97,11 @@ class Sitemap: @staticmethod def _get_tag_name(elem: lxml.etree._Element) -> str: - if TYPE_CHECKING: - assert isinstance(elem.tag, str) - _, _, localname = elem.tag.partition("}") - return localname or elem.tag + tag = elem.tag + if not isinstance(tag, str): + return "" + _, _, localname = tag.partition("}") + return localname or tag def sitemap_urls_from_robots( diff --git a/tests/test_utils_sitemap.py b/tests/test_utils_sitemap.py index 3599c4824..ac57e1739 100644 --- a/tests/test_utils_sitemap.py +++ b/tests/test_utils_sitemap.py @@ -311,3 +311,14 @@ def test_xml_entity_expansion(): """ ) assert list(s) == [{"loc": "http://127.0.0.1:8000/"}] + + +def test_sitemap_non_string_tag(): + """With recover=True and resolve_entities=False, libxml2 >= 2.14.6 (used + by lxml >= 6.1.1) preserves undeclared entity reference nodes whose + .tag is a non-string ``Cython function`` object instead of a ``str``. + _get_tag_name must handle this gracefully instead of raising + AttributeError. + """ + results = list(Sitemap(b"&k;")) + assert results == [] From 00098cb596d0d3957236ba1ba193b09172696bc5 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 30 Jun 2026 18:27:32 +0500 Subject: [PATCH 15/28] Assorted docstring fixes (#7698) * Smaller fixes. * Add more code blocks in docstrings. * Queue stuff. * Response stuff. * Round 2. * Address feedback. --- scrapy/addons.py | 2 +- scrapy/commands/__init__.py | 2 +- scrapy/core/downloader/contextfactory.py | 9 +++- scrapy/core/downloader/handlers/ftp.py | 6 +-- scrapy/core/downloader/handlers/http11.py | 3 +- scrapy/core/http2/agent.py | 6 +-- scrapy/core/http2/protocol.py | 4 +- scrapy/core/scheduler.py | 12 ++--- scrapy/core/scraper.py | 4 +- scrapy/crawler.py | 12 ++--- .../downloadermiddlewares/httpcompression.py | 2 +- scrapy/downloadermiddlewares/redirect.py | 7 ++- scrapy/downloadermiddlewares/retry.py | 8 ++- scrapy/exceptions.py | 2 +- scrapy/extensions/logstats.py | 2 +- scrapy/extensions/throttle.py | 2 +- scrapy/http/cookies.py | 6 +-- scrapy/http/response/html.py | 5 +- scrapy/http/response/xml.py | 5 +- scrapy/item.py | 2 +- scrapy/link.py | 4 +- scrapy/logformatter.py | 24 +++++---- scrapy/mail.py | 2 - scrapy/pipelines/__init__.py | 2 +- scrapy/pipelines/files.py | 2 +- scrapy/pipelines/images.py | 2 +- scrapy/pqueues.py | 7 +-- scrapy/settings/__init__.py | 11 ++-- scrapy/shell.py | 5 +- scrapy/spidermiddlewares/base.py | 8 +-- scrapy/spidermiddlewares/httperror.py | 2 +- scrapy/spidermiddlewares/referer.py | 2 +- scrapy/spiders/feed.py | 29 ++++++----- scrapy/utils/datatypes.py | 2 - scrapy/utils/defer.py | 52 +++++++++++-------- scrapy/utils/deprecate.py | 15 +++--- scrapy/utils/log.py | 3 +- scrapy/utils/response.py | 2 +- scrapy/utils/signal.py | 7 ++- scrapy/utils/trackref.py | 4 +- scrapy/utils/url.py | 4 +- 41 files changed, 151 insertions(+), 139 deletions(-) diff --git a/scrapy/addons.py b/scrapy/addons.py index 2e12f8c8a..470be47c1 100644 --- a/scrapy/addons.py +++ b/scrapy/addons.py @@ -64,7 +64,7 @@ class AddonManager: :param settings: The :class:`~scrapy.settings.BaseSettings` object from \ which to read the early add-on configuration - :type settings: :class:`~scrapy.settings.Settings` + :type settings: :class:`~scrapy.settings.BaseSettings` """ for clspath in build_component_list(settings["ADDONS"]): addoncls = load_object(clspath) diff --git a/scrapy/commands/__init__.py b/scrapy/commands/__init__.py index 598e8060e..d9c919db9 100644 --- a/scrapy/commands/__init__.py +++ b/scrapy/commands/__init__.py @@ -73,7 +73,7 @@ class ScrapyCommand(ABC): def long_desc(self) -> str: """A long description of the command. Return short description when not available. It cannot contain newlines since contents will be formatted - by optparser which removes newlines and wraps text. + by argparse which removes newlines and wraps text. """ return self.short_desc() diff --git a/scrapy/core/downloader/contextfactory.py b/scrapy/core/downloader/contextfactory.py index c1796c723..a934cbbc7 100644 --- a/scrapy/core/downloader/contextfactory.py +++ b/scrapy/core/downloader/contextfactory.py @@ -47,8 +47,13 @@ class _ScrapyClientContextFactory(BrowserLikePolicyForHTTPS): instance. The purpose of this custom class is to provide a ``creatorForNetloc()`` - method that returns a ``_ScrapyClientTLSOptions`` instance configured based - on TLS settings provided to the factory. + method that returns: + + - a ``_ScrapyClientTLSOptions26`` or ``_ScrapyClientTLSOptions`` instance + configured based on TLS settings provided to the factory (when the + certificate verification is disabled); + - a result of ``optionsForClientTLS()`` called with those TLS settings + (when the certificate verification is enabled). """ def __init__( diff --git a/scrapy/core/downloader/handlers/ftp.py b/scrapy/core/downloader/handlers/ftp.py index 9064aa53a..07ff4a74e 100644 --- a/scrapy/core/downloader/handlers/ftp.py +++ b/scrapy/core/downloader/handlers/ftp.py @@ -2,9 +2,9 @@ An asynchronous FTP file download handler for scrapy which somehow emulates an http response. FTP connection parameters are passed using the request meta field: -- ftp_user (required) -- ftp_password (required) -- ftp_passive (by default, enabled) sets FTP connection passive mode +- ftp_user (optional, falls back to FTP_USER) +- ftp_password (optional, falls back to FTP_PASSWORD) +- ftp_passive (optional, falls back to FTP_PASSIVE_MODE) sets FTP connection passive mode - ftp_local_filename - If not given, file data will come in the response.body, as a normal scrapy Response, which will imply that the entire file will be on memory. diff --git a/scrapy/core/downloader/handlers/http11.py b/scrapy/core/downloader/handlers/http11.py index 1fc504c59..17dca5acb 100644 --- a/scrapy/core/downloader/handlers/http11.py +++ b/scrapy/core/downloader/handlers/http11.py @@ -104,7 +104,6 @@ class HTTP11DownloadHandler(BaseHttpDownloadHandler): self._disconnect_timeout: int = 1 async def download_request(self, request: Request) -> Response: - """Return a deferred for the HTTP download""" if hasattr(self._crawler.spider, "download_maxsize"): # pragma: no cover warn_on_deprecated_spider_attribute("download_maxsize", "DOWNLOAD_MAXSIZE") if hasattr(self._crawler.spider, "download_warnsize"): # pragma: no cover @@ -283,7 +282,7 @@ def _tunnel_request_data( class _TunnelingAgent(Agent): - """An agent that uses a L{TunnelingTCP4ClientEndpoint} to make HTTPS + """An agent that uses a ``_TunnelingTCP4ClientEndpoint`` to make HTTPS downloads. It may look strange that we have chosen to subclass Agent and not ProxyAgent but consider that after the tunnel is opened the proxy is transparent to the client; thus the agent should behave like there is no diff --git a/scrapy/core/http2/agent.py b/scrapy/core/http2/agent.py index 7137a0f2b..aa55e29a0 100644 --- a/scrapy/core/http2/agent.py +++ b/scrapy/core/http2/agent.py @@ -114,11 +114,7 @@ class H2ConnectionPool: d.errback(ResponseFailed(errors)) def close_connections(self) -> None: - """Close all the HTTP/2 connections and remove them from pool - - Returns: - Deferred that fires when all connections have been closed - """ + """Close all the HTTP/2 connections and remove them from pool.""" for conn in self._connections.values(): assert conn.transport is not None # typing conn.transport.abortConnection() diff --git a/scrapy/core/http2/protocol.py b/scrapy/core/http2/protocol.py index 39703f976..7136e829e 100644 --- a/scrapy/core/http2/protocol.py +++ b/scrapy/core/http2/protocol.py @@ -101,7 +101,7 @@ class H2ClientProtocol(Protocol, TimeoutMixin): uri is used to verify that incoming client requests have correct base URL. settings -- Scrapy project settings - conn_lost_deferred -- Deferred fires with the reason: Failure to notify + conn_lost_deferred -- Deferred that fires with the list of underlying exceptions to notify that connection was lost tls_verbose_logging -- Whether to log TLS details """ @@ -375,7 +375,7 @@ class H2ClientProtocol(Protocol, TimeoutMixin): def _handle_events(self, events: list[Event]) -> None: """Private method which acts as a bridge between the events - received from the HTTP/2 data and IH2EventsHandler + received from the HTTP/2 data and the handlers in this class. Arguments: events -- A list of events that the remote peer triggered by sending data diff --git a/scrapy/core/scheduler.py b/scrapy/core/scheduler.py index 7217da942..82e31b90b 100644 --- a/scrapy/core/scheduler.py +++ b/scrapy/core/scheduler.py @@ -131,10 +131,10 @@ class Scheduler(BaseScheduler): (:setting:`SCHEDULER_PRIORITY_QUEUE`) that sort requests by :attr:`~scrapy.http.Request.priority`. - By default, a single, memory-based priority queue is used for all requests. - When using :setting:`JOBDIR`, a disk-based priority queue is also created, + By default, memory-based priority queues are used for all requests. + When using :setting:`JOBDIR`, disk-based priority queues are also created, and only unserializable requests are stored in the memory-based priority - queue. For a given priority value, requests in memory take precedence over + queues. For a given priority value, requests in memory take precedence over requests in disk. Each priority queue stores requests in separate internal queues, one per @@ -209,8 +209,8 @@ class Scheduler(BaseScheduler): ------------------------- While pending requests are below the configured values of - :setting:`CONCURRENT_REQUESTS`, :setting:`CONCURRENT_REQUESTS_PER_DOMAIN` - or :setting:`CONCURRENT_REQUESTS_PER_IP`, those requests are sent + :setting:`CONCURRENT_REQUESTS` or + :setting:`CONCURRENT_REQUESTS_PER_DOMAIN`, those requests are sent concurrently. As a result, the first few requests of a crawl may not follow the desired @@ -342,7 +342,7 @@ class Scheduler(BaseScheduler): def open(self, spider: Spider) -> Deferred[None] | None: """ (1) initialize the memory queue - (2) initialize the disk queue if the ``jobdir`` attribute is a valid directory + (2) initialize the disk queue if the ``jobdir`` argument wasn't empty (3) return the result of the dupefilter's ``open`` method """ self.spider: Spider = spider diff --git a/scrapy/core/scraper.py b/scrapy/core/scraper.py index 466ce656d..58e37ce5e 100644 --- a/scrapy/core/scraper.py +++ b/scrapy/core/scraper.py @@ -441,7 +441,7 @@ class Scraper: self, output: Any, response: Response | Failure ) -> Deferred[None]: """Process each Request/Item (given in the output parameter) returned - from the given spider. + from the spider. Items are sent to the item pipelines, requests are scheduled. """ @@ -451,7 +451,7 @@ class Scraper: self, output: Any, response: Response | Failure ) -> None: """Process each Request/Item (given in the output parameter) returned - from the given spider. + from the spider. Items are sent to the item pipelines, requests are scheduled. """ diff --git a/scrapy/crawler.py b/scrapy/crawler.py index 9828fe6f5..c72752915 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -531,8 +531,8 @@ class AsyncCrawlerRunner(CrawlerRunnerBase): """ Run a crawler with the provided arguments. - It will call the given Crawler's :meth:`~Crawler.crawl` method, while - keeping track of it so it can be stopped later. + It will call the given Crawler's :meth:`~Crawler.crawl_async` method, + while keeping track of it so it can be stopped later. If ``crawler_or_spidercls`` isn't a :class:`~scrapy.crawler.Crawler` instance, this method will try to create one using this parameter as @@ -773,7 +773,7 @@ class CrawlerProcess(CrawlerProcessBase, CrawlerRunner): """ This method starts a :mod:`~twisted.internet.reactor`, adjusts its pool size to :setting:`REACTOR_THREADPOOL_MAXSIZE`, and installs a DNS - resolver based on :setting:`DNSCACHE_ENABLED`. + resolver based on :setting:`TWISTED_DNS_RESOLVER`. If ``stop_after_crawl`` is True, the reactor will be stopped after all crawlers have finished, using :meth:`join`. @@ -875,10 +875,10 @@ class AsyncCrawlerProcess(CrawlerProcessBase, AsyncCrawlerRunner): When using a reactor it adjusts its pool size to :setting:`REACTOR_THREADPOOL_MAXSIZE` and installs a DNS resolver based - on :setting:`DNSCACHE_ENABLED`. + on :setting:`TWISTED_DNS_RESOLVER`. - If ``stop_after_crawl`` is True, the reactor will be stopped after all - crawlers have finished, using :meth:`join`. + If ``stop_after_crawl`` is True, the reactor/event loop will be stopped + after all crawlers have finished, using :meth:`join`. :param bool stop_after_crawl: stop or not the reactor when all crawlers have finished diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 2b1721ced..6ca04a50e 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -60,7 +60,7 @@ else: class HttpCompressionMiddleware: - """This middleware allows compressed (gzip, deflate) traffic to be + """This middleware allows compressed (gzip, deflate etc.) traffic to be sent/received from websites""" def __init__( diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 821f41699..45af5c67a 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -196,10 +196,7 @@ class BaseRedirectMiddleware: class RedirectMiddleware(BaseRedirectMiddleware): - """ - Handle redirection of requests based on response status - and meta-refresh html tag. - """ + """Handle redirection of requests based on response status.""" @_warn_spider_arg def process_response( @@ -251,6 +248,8 @@ class RedirectMiddleware(BaseRedirectMiddleware): class MetaRefreshMiddleware(BaseRedirectMiddleware): + """Handle redirection of requests based on meta-refresh html tag.""" + enabled_setting = "METAREFRESH_ENABLED" def __init__(self, settings: BaseSettings): diff --git a/scrapy/downloadermiddlewares/retry.py b/scrapy/downloadermiddlewares/retry.py index 5f125cae4..a0a0b60a2 100644 --- a/scrapy/downloadermiddlewares/retry.py +++ b/scrapy/downloadermiddlewares/retry.py @@ -5,9 +5,6 @@ problems such as a connection timeout or HTTP 500 error. You can change the behaviour of this middleware by modifying the scraping settings: RETRY_TIMES - how many times to retry a failed page RETRY_HTTP_CODES - which HTTP response codes to retry - -Failed pages are collected on the scraping process and rescheduled at the end, -once the spider has finished crawling all regular (non-failed) pages. """ from __future__ import annotations @@ -70,8 +67,9 @@ def get_retry_request( and :ref:`stats `, and to provide extra logging context (see :func:`logging.debug`). - *reason* is a string or an :class:`Exception` object that indicates the - reason why the request needs to be retried. It is used to name retry stats. + *reason* is a string, an :class:`Exception` subclass or an + :class:`Exception` object that indicates the reason why the request needs + to be retried. It is used to name retry stats. *max_retry_times* is a number that determines the maximum number of times that *request* can be retried. If not specified or ``None``, the number is diff --git a/scrapy/exceptions.py b/scrapy/exceptions.py index 204132973..5330eab48 100644 --- a/scrapy/exceptions.py +++ b/scrapy/exceptions.py @@ -115,7 +115,7 @@ class UsageError(Exception): class ScrapyDeprecationWarning(Warning): """Warning category for deprecated features, since the default - DeprecationWarning is silenced on Python 2.7+ + :exc:`DeprecationWarning` is silenced. """ diff --git a/scrapy/extensions/logstats.py b/scrapy/extensions/logstats.py index 3d7674905..6c94d947e 100644 --- a/scrapy/extensions/logstats.py +++ b/scrapy/extensions/logstats.py @@ -22,7 +22,7 @@ logger = logging.getLogger(__name__) class LogStats: """Log basic scraping stats periodically like: - * RPM - Requests per Minute + * RPM - Responses per Minute * IPM - Items per Minute """ diff --git a/scrapy/extensions/throttle.py b/scrapy/extensions/throttle.py index cdb0671ae..542ff1cdc 100644 --- a/scrapy/extensions/throttle.py +++ b/scrapy/extensions/throttle.py @@ -116,7 +116,7 @@ class AutoThrottle: # It works better with problematic sites. new_delay = max(target_delay, new_delay) - # Make sure self.mindelay <= new_delay <= self.max_delay + # Make sure self.mindelay <= new_delay <= self.maxdelay new_delay = min(max(self.mindelay, new_delay), self.maxdelay) # Dont adjust delay if response status != 200 and new delay is smaller diff --git a/scrapy/http/cookies.py b/scrapy/http/cookies.py index 599f20947..8edeae01c 100644 --- a/scrapy/http/cookies.py +++ b/scrapy/http/cookies.py @@ -136,9 +136,9 @@ class _DummyLock: class WrappedRequest: - """Wraps a scrapy Request class with methods defined by urllib2.Request class to interact with CookieJar class - - see http://docs.python.org/library/urllib2.html#urllib2.Request + """Wraps a :class:`scrapy.Request` class with methods defined by + the :class:`urllib.request.Request` class to interact with + the :class:`http.cookiejar.CookieJar` class. """ def __init__(self, request: Request): diff --git a/scrapy/http/response/html.py b/scrapy/http/response/html.py index 70c08c11d..6d3a9ee12 100644 --- a/scrapy/http/response/html.py +++ b/scrapy/http/response/html.py @@ -1,6 +1,7 @@ """ -This module implements the HtmlResponse class which adds encoding -discovering through HTML encoding declarations to the TextResponse class. +This module implements the :class:`HtmlResponse` class which is used as a +content type marker by :class:`~scrapy.selector.Selector` and can be used in +``isinstance()`` checks. See documentation in docs/topics/request-response.rst """ diff --git a/scrapy/http/response/xml.py b/scrapy/http/response/xml.py index 6d9c4cb73..847fb2b3c 100644 --- a/scrapy/http/response/xml.py +++ b/scrapy/http/response/xml.py @@ -1,6 +1,7 @@ """ -This module implements the XmlResponse class which adds encoding -discovering through XML encoding declarations to the TextResponse class. +This module implements the :class:`XmlResponse` class which is used as a +content type marker by :class:`~scrapy.selector.Selector` and can be used in +``isinstance()`` checks. See documentation in docs/topics/request-response.rst """ diff --git a/scrapy/item.py b/scrapy/item.py index 1cc0ae584..d5adc1efb 100644 --- a/scrapy/item.py +++ b/scrapy/item.py @@ -1,7 +1,7 @@ """ Scrapy Item -See documentation in docs/topics/item.rst +See documentation in docs/topics/items.rst """ from __future__ import annotations diff --git a/scrapy/link.py b/scrapy/link.py index 046630403..8211faccd 100644 --- a/scrapy/link.py +++ b/scrapy/link.py @@ -9,7 +9,9 @@ its documentation in: docs/topics/link-extractors.rst class Link: """Link objects represent an extracted link by the LinkExtractor. - Using the anchor tag sample below to illustrate the parameters:: + Using the anchor tag sample below to illustrate the parameters: + + .. code-block:: html Dont follow this one diff --git a/scrapy/logformatter.py b/scrapy/logformatter.py index a50064e08..bfb2d5dff 100644 --- a/scrapy/logformatter.py +++ b/scrapy/logformatter.py @@ -58,18 +58,20 @@ class LogFormatter: logging an action the method must return ``None``. Here is an example on how to create a custom log formatter to lower the severity level of - the log message when an item is dropped from the pipeline:: + the log message when an item is dropped from the pipeline: - class PoliteLogFormatter(logformatter.LogFormatter): - def dropped(self, item, exception, response, spider): - return { - 'level': logging.INFO, # lowering the level from logging.WARNING - 'msg': "Dropped: %(exception)s" + os.linesep + "%(item)s", - 'args': { - 'exception': exception, - 'item': item, - } - } + .. code-block:: python + + class PoliteLogFormatter(logformatter.LogFormatter): + def dropped(self, item, exception, response, spider): + return { + "level": logging.INFO, # lowering the level from logging.WARNING + "msg": "Dropped: %(exception)s" + os.linesep + "%(item)s", + "args": { + "exception": exception, + "item": item, + }, + } """ def crawled( diff --git a/scrapy/mail.py b/scrapy/mail.py index fbd11ad1d..97123e63c 100644 --- a/scrapy/mail.py +++ b/scrapy/mail.py @@ -1,7 +1,5 @@ """ Mail sending helpers - -See documentation in docs/topics/email.rst """ from __future__ import annotations diff --git a/scrapy/pipelines/__init__.py b/scrapy/pipelines/__init__.py index 84fb5f85e..383c461e6 100644 --- a/scrapy/pipelines/__init__.py +++ b/scrapy/pipelines/__init__.py @@ -1,7 +1,7 @@ """ Item pipeline -See documentation in docs/item-pipeline.rst +See documentation in docs/topics/item-pipeline.rst """ from __future__ import annotations diff --git a/scrapy/pipelines/files.py b/scrapy/pipelines/files.py index 0066fd38f..44c422430 100644 --- a/scrapy/pipelines/files.py +++ b/scrapy/pipelines/files.py @@ -423,7 +423,7 @@ class FTPFilesStore: class FilesPipeline(MediaPipeline): - """Abstract pipeline that implement the file downloading + """Pipeline that implements file downloading. This pipeline tries to minimize network transfers and file processing, doing stat of the files and determining if file is new, up-to-date or diff --git a/scrapy/pipelines/images.py b/scrapy/pipelines/images.py index 762b0fdf1..79b6c4f27 100644 --- a/scrapy/pipelines/images.py +++ b/scrapy/pipelines/images.py @@ -47,7 +47,7 @@ class ImageException(FileException): class ImagesPipeline(FilesPipeline): - """Abstract pipeline that implement the image thumbnail generation logic""" + """Pipeline that implements the handling logic specific to images.""" MEDIA_NAME: str = "image" diff --git a/scrapy/pqueues.py b/scrapy/pqueues.py index 2efc43d4b..41411ceaf 100644 --- a/scrapy/pqueues.py +++ b/scrapy/pqueues.py @@ -92,9 +92,10 @@ class ScrapyPriorityQueue: - The :data:`~scrapy.Request.priority` of the request. For each combination of the above seen, this class creates an instance of - *downstream_queue_cls* with *key* set to a subdirectory of the persistence - directory, named as the request priority (e.g. ``1``), with an ``s`` suffix - in case of a start request (e.g. ``1s``). + *downstream_queue_cls* (or *start_queue_cls* for start requests if it was + passed) with *key* set to a subdirectory of the persistence directory, + named as the negated request priority (e.g. ``-1``), with an ``s`` suffix + in case of a start request (e.g. ``-1s``). """ @classmethod diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py index be298e9b1..932463ba9 100644 --- a/scrapy/settings/__init__.py +++ b/scrapy/settings/__init__.py @@ -454,9 +454,10 @@ class BaseSettings(MutableMapping[str, Any]): """ Store a key/value attribute with a given priority. - Settings should be populated *before* configuring the Crawler object - (through the :meth:`~scrapy.crawler.Crawler.configure` method), - otherwise they won't have any effect. + Settings should be populated *before* the Crawler object applies them + (in the :meth:`~scrapy.crawler.Crawler.crawl_async` or + :meth:`~scrapy.crawler.Crawler.crawl` method), otherwise they won't + have any effect. :param name: the setting name :type name: str @@ -613,7 +614,7 @@ class BaseSettings(MutableMapping[str, Any]): """ Make a deep copy of current settings. - This method returns a new instance of the :class:`Settings` class, + This method returns a new instance of this class, populated with the same values and their priorities. Modifications to the new object won't be reflected on the original @@ -658,7 +659,7 @@ class BaseSettings(MutableMapping[str, Any]): Make a copy of current settings and convert to a dict. This method returns a new dict populated with the same values - and their priorities as the current settings. + as the current settings. Modifications to the returned dict won't be reflected on the original settings. diff --git a/scrapy/shell.py b/scrapy/shell.py index 44dcd880e..dfea00c46 100644 --- a/scrapy/shell.py +++ b/scrapy/shell.py @@ -75,7 +75,7 @@ if TYPE_CHECKING: # running event loop. # # Side note: it should be possible to remove _request_deferred() by using -# engine.download_async() instead of engine.schedule(), losing the usual stuff +# engine.download_async() instead of engine.crawl(), losing the usual stuff # like spider middlewares (none of which should be important). # # Other architecture problems: @@ -188,7 +188,8 @@ class Shell: async def _schedule(self, request: Request, spider: Spider | None) -> Response: """Send the request to the engine, wait for the result. - Runs in the reactor thread. + Runs in the reactor thread when using the reactor, or in the asyncio + event loop thread otherwise. """ if not self.spider: await self._open_spider(spider) diff --git a/scrapy/spidermiddlewares/base.py b/scrapy/spidermiddlewares/base.py index e09f2d10e..6c62dccb8 100644 --- a/scrapy/spidermiddlewares/base.py +++ b/scrapy/spidermiddlewares/base.py @@ -75,7 +75,7 @@ class BaseSpiderMiddleware: ) -> Request | None: """Return a processed request from the spider output. - This method is called with a single request from the start seeds or the + This method is called with a single request from ``start()`` or the spider output. It should return the same or a different request, or ``None`` to ignore it. @@ -84,7 +84,7 @@ class BaseSpiderMiddleware: :param response: the response being processed :type response: :class:`~scrapy.http.Response` object or ``None`` for - start seeds + start requests :return: the processed request or ``None`` """ @@ -93,7 +93,7 @@ class BaseSpiderMiddleware: def get_processed_item(self, item: Any, response: Response | None) -> Any: """Return a processed item from the spider output. - This method is called with a single item from the start seeds or the + This method is called with a single item from ``start()`` or the spider output. It should return the same or a different item, or ``None`` to ignore it. @@ -102,7 +102,7 @@ class BaseSpiderMiddleware: :param response: the response being processed :type response: :class:`~scrapy.http.Response` object or ``None`` for - start seeds + start items :return: the processed item or ``None`` """ diff --git a/scrapy/spidermiddlewares/httperror.py b/scrapy/spidermiddlewares/httperror.py index 94b6dfbb5..156b73e7e 100644 --- a/scrapy/spidermiddlewares/httperror.py +++ b/scrapy/spidermiddlewares/httperror.py @@ -28,7 +28,7 @@ logger = logging.getLogger(__name__) class HttpError(IgnoreRequest): - """A non-200 response was filtered""" + """A non-2xx response was filtered""" def __init__(self, response: Response, *args: Any, **kwargs: Any): self.response = response diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index 264f685c1..1305874d5 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -92,7 +92,7 @@ class ReferrerPolicy(ABC): ) def origin(self, url: str) -> str | None: - """Return serialized origin (scheme, host, path) for a request or response URL.""" + """Return serialized origin (scheme, host, port) for a request or response URL.""" return self.strip_url(url, origin_only=True) def potentially_trustworthy(self, url: str) -> bool: diff --git a/scrapy/spiders/feed.py b/scrapy/spiders/feed.py index 395183613..925f31ede 100644 --- a/scrapy/spiders/feed.py +++ b/scrapy/spiders/feed.py @@ -54,17 +54,21 @@ class XMLFeedSpider(Spider): return response def parse_node(self, response: Response, selector: Selector) -> Any: - """This method must be overridden with your custom spider functionality""" + """This method is called for the nodes matching the provided tag name + (itertag). Receives the response and an Selector for each node. + + This method must return either an item, a request, or a list + containing any of them. + + This method must be overridden with your custom spider functionality. + """ if hasattr(self, "parse_item"): # backward compatibility return self.parse_item(response, selector) raise NotImplementedError def parse_nodes(self, response: Response, nodes: Iterable[Selector]) -> Any: """This method is called for the nodes matching the provided tag name - (itertag). Receives the response and an Selector for each node. - Overriding this method is mandatory. Otherwise, you spider won't work. - This method must return either an item, a request, or a list - containing any of them. + (itertag). Receives the response and an iterable of Selectors. """ for selector in nodes: @@ -113,6 +117,9 @@ class CSVFeedSpider(Spider): It receives a CSV file in a response; iterates through each of its rows, and calls parse_row with a dict containing each field's data. + This spider also gives the opportunity to override adapt_response and + process_results methods for pre and post-processing purposes. + You can set some options regarding the CSV file, such as the delimiter, quotechar and the file's headers. """ @@ -136,16 +143,14 @@ class CSVFeedSpider(Spider): return response def parse_row(self, response: Response, row: dict[str, str]) -> Any: - """This method must be overridden with your custom spider functionality""" + """Receives a response and a dict (representing each row) with a key for + each provided (or detected) header of the CSV file. + + This method must be overridden with your custom spider functionality. + """ raise NotImplementedError def parse_rows(self, response: Response) -> Any: - """Receives a response and a dict (representing each row) with a key for - each provided (or detected) header of the CSV file. This spider also - gives the opportunity to override adapt_response and - process_results methods for pre and post-processing purposes. - """ - for row in csviter( response, self.delimiter, self.headers, quotechar=self.quotechar ): diff --git a/scrapy/utils/datatypes.py b/scrapy/utils/datatypes.py index dd0e062d0..c020ff4b9 100644 --- a/scrapy/utils/datatypes.py +++ b/scrapy/utils/datatypes.py @@ -1,8 +1,6 @@ """ This module contains data types used by Scrapy which are not included in the Python Standard Library. - -This module must not depend on any module outside the Standard Library. """ from __future__ import annotations diff --git a/scrapy/utils/defer.py b/scrapy/utils/defer.py index 29a34d4ef..d0259b634 100644 --- a/scrapy/utils/defer.py +++ b/scrapy/utils/defer.py @@ -103,7 +103,7 @@ async def _defer_sleep_async() -> None: def defer_result(result: Any) -> Deferred[Any]: # pragma: no cover warnings.warn( "scrapy.utils.defer.defer_result() is deprecated, use" - " twisted.internet.defer.success() and twisted.internet.defer.fail()," + " twisted.internet.defer.succeed() and twisted.internet.defer.fail()," " plus an explicit sleep if needed, or explicit reactor.callLater().", category=ScrapyDeprecationWarning, stacklevel=2, @@ -469,22 +469,22 @@ def _maybeDeferred_coro( def deferred_to_future(d: Deferred[_T]) -> Future[_T]: """Return an :class:`asyncio.Future` object that wraps *d*. - This function requires - :class:`~twisted.internet.asyncioreactor.AsyncioSelectorReactor` to be - installed. + This function requires an installed asyncio reactor or a running asyncio + event loop, see :ref:`using-asyncio`. - When :ref:`using the asyncio reactor `, you cannot await - on :class:`~twisted.internet.defer.Deferred` objects from :ref:`Scrapy - callables defined as coroutines `, you can only await on - ``Future`` objects. Wrapping ``Deferred`` objects into ``Future`` objects - allows you to wait on them:: + In this state you cannot await on :class:`~twisted.internet.defer.Deferred` + objects from :ref:`Scrapy callables defined as coroutines + `, you can only await on ``Future`` objects. Wrapping + ``Deferred`` objects into ``Future`` objects allows you to wait on them: + + .. code-block:: python class MySpider(Spider): ... + async def parse(self, response): - additional_request = scrapy.Request('https://example.org/price') - deferred = self.crawler.engine.download(additional_request) - additional_response = await deferred_to_future(deferred) + deferred = some_dfd_helper() + result = await deferred_to_future(deferred) .. versionchanged:: 2.14 This function no longer installs an asyncio loop if called before the @@ -492,7 +492,10 @@ def deferred_to_future(d: Deferred[_T]) -> Future[_T]: in this case. """ if not is_asyncio_available(): - raise RuntimeError("deferred_to_future() requires AsyncioSelectorReactor.") + raise RuntimeError( + "deferred_to_future() requires an installed asyncio reactor" + " or a running asyncio event loop." + ) return d.asFuture(asyncio.get_event_loop()) @@ -501,23 +504,26 @@ def maybe_deferred_to_future(d: Deferred[_T]) -> Deferred[_T] | Future[_T]: defined as a coroutine `. What you can await in Scrapy callables defined as coroutines depends on the - value of :setting:`TWISTED_REACTOR`: + value of :setting:`TWISTED_REACTOR` and :setting:`TWISTED_REACTOR_ENABLED`: - - When :ref:`using the asyncio reactor `, you can only - await on :class:`asyncio.Future` objects. + - When :ref:`using the asyncio reactor `, or :ref:`not + using a reactor at all `, you can only await + on :class:`asyncio.Future` objects. - - When not using the asyncio reactor, you can only await on - :class:`~twisted.internet.defer.Deferred` objects. + - When :ref:`using a non-asyncio reactor `, you can only + await on :class:`~twisted.internet.defer.Deferred` objects. - If you want to write code that uses ``Deferred`` objects but works with any - reactor, use this function on all ``Deferred`` objects:: + If you want to write code that uses ``Deferred`` objects but works in both + of these states, use this function on all ``Deferred`` objects: + + .. code-block:: python class MySpider(Spider): ... + async def parse(self, response): - additional_request = scrapy.Request('https://example.org/price') - deferred = self.crawler.engine.download(additional_request) - additional_response = await maybe_deferred_to_future(deferred) + deferred = some_dfd_helper() + result = await maybe_deferred_to_future(deferred) """ if not is_asyncio_available(): return d diff --git a/scrapy/utils/deprecate.py b/scrapy/utils/deprecate.py index 359f819d7..4fd50fdad 100644 --- a/scrapy/utils/deprecate.py +++ b/scrapy/utils/deprecate.py @@ -43,15 +43,18 @@ def create_deprecated_class( It can be used to rename a base class in a library. For example, if we have - class OldName(SomeClass): - # ... + .. code-block:: python - and we want to rename it to NewName, we can do the following:: + class OldName(SomeClass): ... - class NewName(SomeClass): - # ... + and we want to rename it to NewName, we can do the following: - OldName = create_deprecated_class('OldName', NewName) + .. code-block:: python + + class NewName(SomeClass): ... + + + OldName = create_deprecated_class("OldName", NewName) Then, if user class inherits from OldName, warning is issued. Also, if some code uses ``issubclass(sub, OldName)`` or ``isinstance(sub(), OldName)`` diff --git a/scrapy/utils/log.py b/scrapy/utils/log.py index aa77e692a..7645b235e 100644 --- a/scrapy/utils/log.py +++ b/scrapy/utils/log.py @@ -248,8 +248,7 @@ def logformatter_adapter( ) -> tuple[int, str, dict[str, Any] | tuple[Any, ...]]: """ Helper that takes the dictionary output from the methods in LogFormatter - and adapts it into a tuple of positional arguments for logger.log calls, - handling backward compatibility as well. + and adapts it into a tuple of positional arguments for logger.log calls. """ level = logkws.get("level", logging.INFO) diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index c068d9b1e..7747a7b9b 100644 --- a/scrapy/utils/response.py +++ b/scrapy/utils/response.py @@ -88,7 +88,7 @@ def open_in_browser( def parse_details(self, response): - if "item name" not in response.body: + if "item name" not in response.text: open_in_browser(response) """ # circular imports diff --git a/scrapy/utils/signal.py b/scrapy/utils/signal.py index 919f67240..eca95e225 100644 --- a/scrapy/utils/signal.py +++ b/scrapy/utils/signal.py @@ -39,7 +39,7 @@ def send_catch_log( *arguments: TypingAny, **named: TypingAny, ) -> list[tuple[TypingAny, TypingAny]]: - """Like ``pydispatcher.robust.sendRobust()`` but it also logs errors and returns + """Like ``pydispatch.robust.sendRobust()`` but it also logs errors and returns Failures instead of exceptions. """ dont_log = named.pop("dont_log", ()) @@ -172,9 +172,8 @@ async def _send_catch_log_asyncio( Returns a coroutine that completes once all signal handlers have finished. - This function requires - :class:`~twisted.internet.asyncioreactor.AsyncioSelectorReactor` to be - installed. + This function requires an installed asyncio reactor or a running asyncio + event loop. .. versionadded:: 2.14 """ diff --git a/scrapy/utils/trackref.py b/scrapy/utils/trackref.py index 87df10a02..22f9eadd0 100644 --- a/scrapy/utils/trackref.py +++ b/scrapy/utils/trackref.py @@ -4,9 +4,7 @@ references to live object instances. If you want live objects for a particular class to be tracked, you only have to subclass from object_ref (instead of object). -About performance: This library has a minimal performance impact when enabled, -and no performance penalty at all when disabled (as object_ref becomes just an -alias to object in that case). +This library has a minimal performance impact. .. note:: PyPy uses a tracing garbage collector, so objects may remain in the ``live_refs`` longer than expected, even after they diff --git a/scrapy/utils/url.py b/scrapy/utils/url.py index 4d2bbdda2..f67853ece 100644 --- a/scrapy/utils/url.py +++ b/scrapy/utils/url.py @@ -117,8 +117,8 @@ def strip_url( - ``strip_credentials`` removes "user:password@" - ``strip_default_port`` removes ":80" (resp. ":443", ":21") from http:// (resp. https://, ftp://) URLs - - ``origin_only`` replaces path component with "/", also dropping - query and fragment components ; it also strips credentials + - ``origin_only`` replaces the path component with "/", also dropping + the query component; it also strips credentials - ``strip_fragment`` drops any #fragment component """ From a6d6a48aa600d1b4c3deb6409c17d0d7f55db312 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 30 Jun 2026 16:26:22 +0200 Subject: [PATCH 16/28] Keep Item fields in definition order (#7694) --- scrapy/item.py | 37 ++++++++++++++++++++++++++++++------- tests/test_feedexport.py | 2 +- tests/test_item.py | 24 ++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/scrapy/item.py b/scrapy/item.py index d5adc1efb..4d99ea79d 100644 --- a/scrapy/item.py +++ b/scrapy/item.py @@ -25,6 +25,25 @@ class Field(dict[str, Any]): """Container of field metadata""" +def _ordered_field_names(cls: type) -> list[str]: + """Return the names of the :class:`Field` attributes of *cls* in definition + order. + + Fields declared in base classes come first, ordered from the topmost base + to the most derived class. Within each class, fields keep their definition + order. A field redefined in a subclass keeps the position of its first + definition. + """ + names: list[str] = [] + seen: set[str] = set() + for base in reversed(cls.__mro__): + for name, value in vars(base).items(): + if isinstance(value, Field) and name not in seen: + seen.add(name) + names.append(name) + return names + + class ItemMeta(ABCMeta): """Metaclass_ of :class:`Item` that handles field definitions. @@ -39,13 +58,9 @@ class ItemMeta(ABCMeta): _class = super().__new__(mcs, "x_" + class_name, new_bases, attrs) fields = getattr(_class, "fields", {}) - new_attrs = {} - for n in dir(_class): - v = getattr(_class, n) - if isinstance(v, Field): - fields[n] = v - elif n in attrs: - new_attrs[n] = attrs[n] + for n in _ordered_field_names(_class): + fields[n] = getattr(_class, n) + new_attrs = {n: v for n, v in attrs.items() if not isinstance(v, Field)} new_attrs["fields"] = fields new_attrs["_class"] = _class @@ -80,6 +95,14 @@ class Item(MutableMapping[str, Any], object_ref, metaclass=ItemMeta): #: those populated. The keys are the field names and the values are the #: :class:`Field` objects used in the :ref:`Item declaration #: `. + #: + #: Fields are kept in definition order: fields declared in base classes + #: come first, followed by fields declared in subclasses, and a field + #: redefined in a subclass keeps the position of its first definition. + #: + #: .. versionchanged:: VERSION + #: Fields are now returned in definition order rather than alphabetical + #: order. fields: dict[str, Field] def __init__(self, *args: Any, **kwargs: Any): diff --git a/tests/test_feedexport.py b/tests/test_feedexport.py index d7ea60cc8..7d751f188 100644 --- a/tests/test_feedexport.py +++ b/tests/test_feedexport.py @@ -746,7 +746,7 @@ class TestFeedExport(TestFeedExportBase): ] formats = { - "csv": b"baz,egg,foo\r\n,spam1,bar1\r\n", + "csv": b"foo,egg,baz\r\nbar1,spam1,\r\n", "json": b'[\n{"hello": "world2", "foo": "bar2"}\n]', "jsonlines": ( b'{"foo": "bar1", "egg": "spam1"}\n{"hello": "world2", "foo": "bar2"}\n' diff --git a/tests/test_item.py b/tests/test_item.py index 34b054e12..7b4c2e918 100644 --- a/tests/test_item.py +++ b/tests/test_item.py @@ -142,6 +142,30 @@ class TestItem: self.assertSortedEqual(list(item.keys()), ["new"]) self.assertSortedEqual(list(item.values()), ["New"]) + def test_fields_order(self): + class TestItem(Item): + name = Field() + keys = Field() + values = Field() + + assert list(TestItem.fields) == ["name", "keys", "values"] + + def test_fields_order_inheritance(self): + class ParentItem(Item): + name = Field() + keys = Field() + values = Field() + + class TestItem(ParentItem): + extra = Field() + keys = Field(serializer=str) + + # Inherited fields come first, in their definition order, followed by + # the fields newly defined in the subclass. A redefined field keeps the + # position of its first definition while taking the new metadata. + assert list(TestItem.fields) == ["name", "keys", "values", "extra"] + assert TestItem.fields["keys"] == {"serializer": str} + def test_metaclass_inheritance(self): class ParentItem(Item): name = Field() From fc5216f15611e40d795f5ab566acff8f9ca0af1e Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Wed, 1 Jul 2026 11:50:32 +0500 Subject: [PATCH 17/28] Clarify/cleanup Selector.type (#7704) --- scrapy/selector/unified.py | 47 ++++++++++++++------------------------ 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/scrapy/selector/unified.py b/scrapy/selector/unified.py index 99b22aca9..f6334c32c 100644 --- a/scrapy/selector/unified.py +++ b/scrapy/selector/unified.py @@ -1,10 +1,6 @@ -""" -XPath selectors based on lxml -""" - from __future__ import annotations -from typing import Any +from typing import Any, Literal from parsel import Selector as _ParselSelector @@ -18,13 +14,10 @@ __all__ = ["Selector", "SelectorList"] _NOT_SET = object() -def _st(response: TextResponse | None, st: str | None) -> str: - if st is None: - return "xml" if isinstance(response, XmlResponse) else "html" - return st +SelectorType = Literal["html", "xml", "json", "text"] -def _response_from_text(text: str | bytes, st: str | None) -> TextResponse: +def _response_from_text(text: str | bytes, st: SelectorType | None) -> TextResponse: rt: type[TextResponse] = XmlResponse if st == "xml" else HtmlResponse return rt(url="about:blank", encoding="utf-8", body=to_bytes(text, "utf-8")) @@ -49,23 +42,16 @@ class Selector(_ParselSelector, object_ref): ``response`` isn't available. Using ``text`` and ``response`` together is undefined behavior. - ``type`` defines the selector type, it can be ``"html"``, ``"xml"``, ``"json"`` - or ``None`` (default). + ``type`` defines the selector type, it can be ``"html"``, ``"xml"``, + ``"json"``, ``"text"`` or ``None`` (default). It's passed to + :class:`parsel.Selector` and its meaning is defined there. However, when + ``type`` is ``None``, it is set to ``"xml"`` for an + :class:`~scrapy.http.XmlResponse` and to ``"html"`` otherwise before + passing it to :class:`parsel.Selector`. - If ``type`` is ``None``, the selector automatically chooses the best type - based on ``response`` type (see below), or defaults to ``"html"`` in case it - is used together with ``text``. - - If ``type`` is ``None`` and a ``response`` is passed, the selector type is - inferred from the response type as follows: - - * ``"html"`` for :class:`~scrapy.http.HtmlResponse` type - * ``"xml"`` for :class:`~scrapy.http.XmlResponse` type - * ``"json"`` for :class:`~scrapy.http.TextResponse` type - * ``"html"`` for anything else - - Otherwise, if ``type`` is set, the selector type will be forced and no - detection will occur. + .. note:: JSON selector support requires ``parsel`` 1.8.0 or higher. With + older versions setting ``type`` to ``"json"`` or ``"text"`` is not + supported. """ __slots__ = ["response"] @@ -75,7 +61,7 @@ class Selector(_ParselSelector, object_ref): self, response: TextResponse | None = None, text: str | None = None, - type: str | None = None, # noqa: A002 + type: SelectorType | None = None, # noqa: A002 root: Any | None = _NOT_SET, **kwargs: Any, ): @@ -84,10 +70,11 @@ class Selector(_ParselSelector, object_ref): f"{self.__class__.__name__}.__init__() received both response and text" ) - st = _st(response, type) + if type is None: + type = "xml" if isinstance(response, XmlResponse) else "html" # noqa: A001 if text is not None: - response = _response_from_text(text, st) + response = _response_from_text(text, type) if response is not None: text = response.text @@ -98,4 +85,4 @@ class Selector(_ParselSelector, object_ref): if root is not _NOT_SET: kwargs["root"] = root - super().__init__(text=text, type=st, **kwargs) + super().__init__(text=text, type=type, **kwargs) From 361f689df785959a59cf939b9efaefc72079f037 Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 1 Jul 2026 08:53:07 +0200 Subject: [PATCH 18/28] Improve test coverage for crawler.py (#7682) * Improve test coverage for crawler.py * Silence mypy warnings * Improve test coverage for crawler.py --- ...yncio_enabled_reactor_same_loop_default.py | 31 ++++ .../dns_resolver_deprecated.py | 31 ++++ .../reactorless_sleeping.py | 2 +- tests/AsyncCrawlerProcess/sleeping.py | 2 +- .../CrawlerProcess/dns_resolver_deprecated.py | 31 ++++ tests/CrawlerProcess/sleeping.py | 2 +- tests/test_crawler.py | 163 +++++++++++++++++- tests/test_crawler_subprocess.py | 30 +++- 8 files changed, 286 insertions(+), 6 deletions(-) create mode 100644 tests/AsyncCrawlerProcess/asyncio_enabled_reactor_same_loop_default.py create mode 100644 tests/AsyncCrawlerProcess/dns_resolver_deprecated.py create mode 100644 tests/CrawlerProcess/dns_resolver_deprecated.py diff --git a/tests/AsyncCrawlerProcess/asyncio_enabled_reactor_same_loop_default.py b/tests/AsyncCrawlerProcess/asyncio_enabled_reactor_same_loop_default.py new file mode 100644 index 000000000..c519e123b --- /dev/null +++ b/tests/AsyncCrawlerProcess/asyncio_enabled_reactor_same_loop_default.py @@ -0,0 +1,31 @@ +import asyncio +import sys + +from twisted.internet import asyncioreactor + +import scrapy +from scrapy.crawler import AsyncCrawlerProcess + +if sys.platform == "win32": + asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy()) +loop = asyncio.SelectorEventLoop() +asyncio.set_event_loop(loop) +asyncioreactor.install(loop) + + +class NoRequestsSpider(scrapy.Spider): + name = "no_request" + + async def start(self): + return + yield + + +process = AsyncCrawlerProcess( + settings={ + "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor", + "ASYNCIO_EVENT_LOOP": "asyncio.SelectorEventLoop", + } +) +process.crawl(NoRequestsSpider) +process.start() diff --git a/tests/AsyncCrawlerProcess/dns_resolver_deprecated.py b/tests/AsyncCrawlerProcess/dns_resolver_deprecated.py new file mode 100644 index 000000000..8c8df96bb --- /dev/null +++ b/tests/AsyncCrawlerProcess/dns_resolver_deprecated.py @@ -0,0 +1,31 @@ +import sys + +import scrapy +from scrapy.crawler import AsyncCrawlerProcess +from scrapy.settings import Settings + + +class NoRequestsSpider(scrapy.Spider): + name = "no_request" + + async def start(self): + return + yield + + +settings = Settings() +# The deprecated DNS_RESOLVER setting, set above its default priority so that +# AsyncCrawlerProcess._setup_reactor() emits the deprecation warning. +settings.set("DNS_RESOLVER", "scrapy.resolver.CachingThreadedResolver", priority=10) +if len(sys.argv) > 1 and sys.argv[1] == "twisted-wins": + # TWISTED_DNS_RESOLVER at a higher priority takes precedence over the + # deprecated DNS_RESOLVER setting. + settings.set( + "TWISTED_DNS_RESOLVER", + "scrapy.resolver.CachingThreadedResolver", + priority=20, + ) + +process = AsyncCrawlerProcess(settings) +process.crawl(NoRequestsSpider) +process.start() diff --git a/tests/AsyncCrawlerProcess/reactorless_sleeping.py b/tests/AsyncCrawlerProcess/reactorless_sleeping.py index 12101d221..0d11a0996 100644 --- a/tests/AsyncCrawlerProcess/reactorless_sleeping.py +++ b/tests/AsyncCrawlerProcess/reactorless_sleeping.py @@ -17,4 +17,4 @@ class SleepingSpider(scrapy.Spider): process = AsyncCrawlerProcess(settings={"TWISTED_REACTOR_ENABLED": False}) process.crawl(SleepingSpider) -process.start() +process.start(stop_after_crawl="--no-stop" not in sys.argv) diff --git a/tests/AsyncCrawlerProcess/sleeping.py b/tests/AsyncCrawlerProcess/sleeping.py index 88caf5032..dad6a3f20 100644 --- a/tests/AsyncCrawlerProcess/sleeping.py +++ b/tests/AsyncCrawlerProcess/sleeping.py @@ -17,4 +17,4 @@ class SleepingSpider(scrapy.Spider): process = AsyncCrawlerProcess(settings={}) process.crawl(SleepingSpider) -process.start() +process.start(stop_after_crawl="--no-stop" not in sys.argv) diff --git a/tests/CrawlerProcess/dns_resolver_deprecated.py b/tests/CrawlerProcess/dns_resolver_deprecated.py new file mode 100644 index 000000000..b8cd24325 --- /dev/null +++ b/tests/CrawlerProcess/dns_resolver_deprecated.py @@ -0,0 +1,31 @@ +import sys + +import scrapy +from scrapy.crawler import CrawlerProcess +from scrapy.settings import Settings + + +class NoRequestsSpider(scrapy.Spider): + name = "no_request" + + async def start(self): + return + yield + + +settings = Settings() +# The deprecated DNS_RESOLVER setting, set above its default priority so that +# CrawlerProcess._setup_reactor() emits the deprecation warning. +settings.set("DNS_RESOLVER", "scrapy.resolver.CachingThreadedResolver", priority=10) +if len(sys.argv) > 1 and sys.argv[1] == "twisted-wins": + # TWISTED_DNS_RESOLVER at a higher priority takes precedence over the + # deprecated DNS_RESOLVER setting. + settings.set( + "TWISTED_DNS_RESOLVER", + "scrapy.resolver.CachingThreadedResolver", + priority=20, + ) + +process = CrawlerProcess(settings) +process.crawl(NoRequestsSpider) +process.start() diff --git a/tests/CrawlerProcess/sleeping.py b/tests/CrawlerProcess/sleeping.py index cb8f869e1..a577b1909 100644 --- a/tests/CrawlerProcess/sleeping.py +++ b/tests/CrawlerProcess/sleeping.py @@ -23,4 +23,4 @@ class SleepingSpider(scrapy.Spider): process = CrawlerProcess(settings={}) process.crawl(SleepingSpider) -process.start() +process.start(stop_after_crawl="--no-stop" not in sys.argv) diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 0cddfd0ed..adac32df1 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -3,8 +3,11 @@ from __future__ import annotations import asyncio import logging import re +import signal +import threading from pathlib import Path -from typing import Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar +from unittest.mock import MagicMock import pytest from zope.interface.exceptions import MultipleInvalid @@ -32,6 +35,9 @@ from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler, get_reactor_settings from tests.utils.decorators import coroutine_test +if TYPE_CHECKING: + from collections.abc import Callable + BASE_SETTINGS: dict[str, Any] = {} @@ -651,6 +657,145 @@ class TestAsyncCrawlerProcess(TestBaseCrawler): self.assertOptionIsDefault(runner.settings, "RETRY_ENABLED") +class TestAsyncCrawlerProcessReactorlessHelpers: + """Unit tests for the reactorless shutdown helpers of AsyncCrawlerProcess. + + These cover defensive branches that guard against shutdown races and that + are not reachable through a full process run. + """ + + @staticmethod + def _bare_process( + monkeypatch: pytest.MonkeyPatch, + ) -> tuple[AsyncCrawlerProcess, list[Any]]: + # AsyncCrawlerProcess.__init__ has global side effects (it installs a + # reactor import hook and an asyncio event loop), so build a bare + # instance and set only the attributes these helpers read. The shutdown + # handlers installed by these helpers are recorded for assertions + # instead of touching the real process-wide signal handlers. + installed_handlers: list[Any] = [] + monkeypatch.setattr( + "scrapy.crawler.install_shutdown_handlers", + lambda handler, *args, **kwargs: installed_handlers.append(handler), + ) + return AsyncCrawlerProcess.__new__(AsyncCrawlerProcess), installed_handlers + + @staticmethod + def _run_in_thread(target: Callable[[], None]) -> None: + # Run target in a dedicated thread so its event loop is not nested + # inside the event loop that may already be running the test session. + thread = threading.Thread(target=target) + thread.start() + thread.join() + + def test_signal_shutdown_reactorless_without_loop( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + process, installed_handlers = self._bare_process(monkeypatch) + process._reactorless_loop = None + # No loop to schedule the shutdown task on, so it returns early, but it + # must still escalate the handler so a second signal forces a kill. + process._signal_shutdown_reactorless(signal.SIGINT, None) + assert installed_handlers == [process._signal_kill_reactorless] + + def test_signal_kill_reactorless_without_loop( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + process, installed_handlers = self._bare_process(monkeypatch) + process._reactorless_loop = None + process._reactorless_main_task = None + # No loop to cancel the main task on, so it returns early, but it must + # still ignore any further signals. + process._signal_kill_reactorless(signal.SIGINT, None) + assert installed_handlers == [signal.SIG_IGN] + + def test_signal_kill_reactorless_without_main_task( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + process, installed_handlers = self._bare_process(monkeypatch) + loop = MagicMock() + process._reactorless_loop = loop + process._reactorless_main_task = None + # No main task to cancel, so nothing is scheduled on the loop. + process._signal_kill_reactorless(signal.SIGINT, None) + assert installed_handlers == [signal.SIG_IGN] + loop.call_soon_threadsafe.assert_not_called() + + def test_shutdown_graceful_reactorless_main_task_already_done( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + process, _ = self._bare_process(monkeypatch) + process._stop_after_crawl = False + + async def noop() -> None: + return None + + monkeypatch.setattr(process, "stop", noop) + monkeypatch.setattr(process, "join", noop) + + def run() -> None: + loop = asyncio.new_event_loop() + try: + main_task: asyncio.Future[None] = loop.create_future() + main_task.set_result(None) + process._reactorless_main_task = main_task + # The main task is already done, so it is not cancelled. + loop.run_until_complete(process._shutdown_graceful_reactorless()) + assert not main_task.cancelled() + finally: + loop.close() + + self._run_in_thread(run) + + def test_create_shutdown_task_closed_loop( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + process, _ = self._bare_process(monkeypatch) + loop = asyncio.new_event_loop() + loop.close() + process._reactorless_loop = loop + process._stop_after_crawl = True + # create_task() raises RuntimeError on a closed loop; the coroutine + # must be closed instead of leaking. + process._create_shutdown_task() + + def test_cancel_all_tasks_logs_task_exception(self) -> None: + contexts: list[dict[str, Any]] = [] + task_was_cancelled: list[bool] = [] + + def run() -> None: + loop = asyncio.new_event_loop() + loop.set_exception_handler(lambda _loop, context: contexts.append(context)) + + async def fail_on_cancel() -> None: + try: + await asyncio.sleep(10) + except asyncio.CancelledError: + raise RuntimeError("boom") + + try: + task = loop.create_task(fail_on_cancel()) + # Let the task start and suspend on the sleep so the + # cancellation is raised inside its body and turned into a + # RuntimeError rather than cancelling the task cleanly. + loop.run_until_complete(asyncio.sleep(0)) + AsyncCrawlerProcess._cancel_all_tasks(loop) + task_was_cancelled.append(task.cancelled()) + finally: + loop.close() + + self._run_in_thread(run) + + # The task raised instead of being cancelled, so its exception is + # reported to the loop exception handler. + assert task_was_cancelled == [False] + assert any( + context.get("message") + == "unhandled exception during AsyncCrawlerProcess shutdown" + for context in contexts + ) + + @pytest.mark.parametrize("runner_cls", [AsyncCrawlerRunner, CrawlerRunner]) def test_runner_settings_applied_to_crawler_instance( runner_cls: type[CrawlerRunnerBase], @@ -687,6 +832,22 @@ def test_create_crawler_instance_consistent_with_spider_class() -> None: assert pre_built.settings["FOO"] == "runner" +@pytest.mark.parametrize("runner_cls", [AsyncCrawlerRunner, CrawlerRunner]) +def test_create_crawler_rejects_spider_object( + runner_cls: type[CrawlerRunnerBase], +) -> None: + runner = runner_cls() + with pytest.raises(ValueError, match="cannot be a spider object"): + runner.create_crawler(DefaultSpider()) # type: ignore[arg-type] + + +@pytest.mark.parametrize("runner_cls", [AsyncCrawlerRunner, CrawlerRunner]) +def test_crawl_rejects_spider_object(runner_cls: type[CrawlerRunnerBase]) -> None: + runner = runner_cls() + with pytest.raises(ValueError, match="cannot be a spider object"): + runner.crawl(DefaultSpider()) # type: ignore[arg-type] + + class ExceptionSpider(scrapy.Spider): name = "exception" diff --git a/tests/test_crawler_subprocess.py b/tests/test_crawler_subprocess.py index e3f9ac161..018a2b31b 100644 --- a/tests/test_crawler_subprocess.py +++ b/tests/test_crawler_subprocess.py @@ -126,6 +126,16 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): assert "TimeoutError" not in log assert "scrapy.exceptions.CannotResolveHostError" not in log + def test_dns_resolver_deprecated(self) -> None: + log = self.run_script("dns_resolver_deprecated.py") + assert "Spider closed (finished)" in log + assert "The DNS_RESOLVER setting is deprecated" in log + + def test_dns_resolver_deprecated_twisted_dns_resolver(self) -> None: + log = self.run_script("dns_resolver_deprecated.py", "twisted-wins") + assert "Spider closed (finished)" in log + assert "The DNS_RESOLVER setting is deprecated" in log + def test_twisted_reactor_asyncio(self) -> None: log = self.run_script("twisted_reactor_asyncio.py") assert "Spider closed (finished)" in log @@ -205,9 +215,11 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): assert "Spider closed (finished)" in log assert "The value of FOO is 42" in log - def _test_shutdown_graceful(self, script: str = "sleeping.py") -> None: + def _test_shutdown_graceful( + self, script: str = "sleeping.py", *extra_args: str + ) -> None: sig = signal.SIGINT if sys.platform != "win32" else signal.SIGBREAK # type: ignore[attr-defined] - args = self.get_script_args(script, "3") + args = self.get_script_args(script, "3", *extra_args) p = PopenSpawn(args, timeout=5, env=get_script_run_env()) p.expect_exact("Spider opened") p.expect_exact("Crawled (200)") @@ -245,6 +257,9 @@ class TestCrawlerProcessSubprocessBase(ScriptRunnerMixin): async def test_shutdown_forced(self) -> None: await self._test_shutdown_forced() + def test_shutdown_graceful_no_stop(self) -> None: + self._test_shutdown_graceful("sleeping.py", "--no-stop") + class TestCrawlerProcessSubprocess(TestCrawlerProcessSubprocessBase): @property @@ -429,6 +444,17 @@ class TestAsyncCrawlerProcessSubprocess(TestCrawlerProcessSubprocessBase): async def test_shutdown_forced(self) -> None: await self._test_shutdown_forced("reactorless_sleeping.py") + def test_shutdown_graceful_reactorless_no_stop(self) -> None: + self._test_shutdown_graceful("reactorless_sleeping.py", "--no-stop") + + def test_asyncio_enabled_reactor_same_loop_default(self) -> None: + log = self.run_script("asyncio_enabled_reactor_same_loop_default.py") + assert "Spider closed (finished)" in log + assert ( + "Using reactor: twisted.internet.asyncioreactor.AsyncioSelectorReactor" + in log + ) + class TestCrawlerRunnerSubprocessBase(ScriptRunnerMixin): """Common tests between CrawlerRunner and AsyncCrawlerRunner, From 870803b7fb1ed56c296eb6b51d0212f0460dfdda Mon Sep 17 00:00:00 2001 From: Fat-Coder-CN Date: Wed, 1 Jul 2026 15:16:01 +0800 Subject: [PATCH 19/28] fix-utf16-response-test-on-big-endian-systems (#7508) --- tests/test_http_response_text.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_http_response_text.py b/tests/test_http_response_text.py index 507fd1864..5ef89fe4a 100644 --- a/tests/test_http_response_text.py +++ b/tests/test_http_response_text.py @@ -163,12 +163,13 @@ class TestTextResponse(TestResponse): def test_utf16(self): """Test utf-16 because UnicodeDammit is known to have problems with""" + body = b"\xff\xfeh\x00i\x00" r = self.response_class( "http://www.example.com", - body=b"\xff\xfeh\x00i\x00", + body=body, encoding="utf-16", ) - self._assert_response_values(r, "utf-16", "hi") + self._assert_response_values(r, "utf-16", body) def test_invalid_utf8_encoded_body_with_valid_utf8_BOM(self): r6 = self.response_class( From dd10cb8e9a982fe3d311078d6e1207596e272717 Mon Sep 17 00:00:00 2001 From: Adrian Date: Sun, 5 Jul 2026 13:46:22 +0200 Subject: [PATCH 20/28] LxmlLinkExtractor: add deny_attrs and deny_tags (#7679) --- docs/topics/link-extractors.rst | 103 +--------------------- scrapy/linkextractors/lxmlhtml.py | 142 +++++++++++++++++++++++++++++- tests/test_linkextractors.py | 53 +++++++++++ 3 files changed, 194 insertions(+), 104 deletions(-) diff --git a/docs/topics/link-extractors.rst b/docs/topics/link-extractors.rst index 613e175da..3fc896507 100644 --- a/docs/topics/link-extractors.rst +++ b/docs/topics/link-extractors.rst @@ -47,108 +47,7 @@ LxmlLinkExtractor :synopsis: lxml's HTMLParser-based link extractors -.. class:: LxmlLinkExtractor(allow=(), deny=(), allow_domains=(), deny_domains=(), deny_extensions=None, restrict_xpaths=(), restrict_css=(), tags=('a', 'area'), attrs=('href',), canonicalize=False, unique=True, process_value=None, strip=True) - - LxmlLinkExtractor is the recommended link extractor with handy filtering - options. It is implemented using lxml's robust HTMLParser. - - :param allow: a single regular expression (or list of regular expressions) - that the (absolute) urls must match in order to be extracted. If not - given (or empty), it will match all links. - :type allow: str or list - - :param deny: a single regular expression (or list of regular expressions) - that the (absolute) urls must match in order to be excluded (i.e. not - extracted). It has precedence over the ``allow`` parameter. If not - given (or empty) it won't exclude any links. - :type deny: str or list - - :param allow_domains: a single value or a list of string containing - domains which will be considered for extracting the links - :type allow_domains: str or list - - :param deny_domains: a single value or a list of strings containing - domains which won't be considered for extracting the links - :type deny_domains: str or list - - :param deny_extensions: a single value or list of strings containing - extensions that should be ignored when extracting links. - If not given, it will default to - :data:`scrapy.linkextractors.IGNORED_EXTENSIONS`. - - :type deny_extensions: list - - :param restrict_xpaths: is an XPath (or list of XPath's) which defines - regions inside the response where links should be extracted from. - If given, only the text selected by those XPath will be scanned for - links. - :type restrict_xpaths: str or list - - :param restrict_css: a CSS selector (or list of selectors) which defines - regions inside the response where links should be extracted from. - Has the same behaviour as ``restrict_xpaths``. - :type restrict_css: str or list - - :param restrict_text: a single regular expression (or list of regular expressions) - that the link's text must match in order to be extracted. If not - given (or empty), it will match all links. If a list of regular expressions is - given, the link will be extracted if it matches at least one. - :type restrict_text: str or list - - :param tags: a tag or a list of tags to consider when extracting links. - Defaults to ``('a', 'area')``. - :type tags: str or list - - :param attrs: an attribute or list of attributes which should be considered when looking - for links to extract (only for those tags specified in the ``tags`` - parameter). Defaults to ``('href',)`` - :type attrs: list - - :param canonicalize: canonicalize each extracted url (using - w3lib.url.canonicalize_url). Defaults to ``False``. - Note that canonicalize_url is meant for duplicate checking; - it can change the URL visible at server side, so the response can be - different for requests with canonicalized and raw URLs. If you're - using LinkExtractor to follow links it is more robust to - keep the default ``canonicalize=False``. - :type canonicalize: bool - - :param unique: whether duplicate filtering should be applied to extracted - links. - :type unique: bool - - :param process_value: a function which receives each value extracted from - the tag and attributes scanned and can modify the value and return a - new one, or return ``None`` to ignore the link altogether. If not - given, ``process_value`` defaults to ``lambda x: x``. - - .. highlight:: html - - For example, to extract links from this code:: - - Link text - - .. highlight:: python - - You can use the following function in ``process_value``: - - .. code-block:: python - - def process_value(value): - m = re.search(r"javascript:goToPage\('(.*?)'", value) - if m: - return m.group(1) - - :type process_value: collections.abc.Callable - - :param strip: whether to strip whitespaces from extracted attributes. - According to HTML5 standard, leading and trailing whitespaces - must be stripped from ``href`` attributes of ````, ```` - and many other elements, ``src`` attribute of ````, ``