From d2f1e00a6afa46c26a334a24217d5bf605bab9fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 14 May 2024 18:54:11 +0200 Subject: [PATCH 01/21] Merge 2.11.2 changes (#6363) --- .bumpversion.cfg | 2 +- docs/faq.rst | 40 +- docs/news.rst | 116 +- docs/topics/benchmarking.rst | 4 +- docs/topics/downloader-middleware.rst | 44 +- docs/topics/settings.rst | 2 +- docs/topics/signals.rst | 11 +- docs/topics/spider-middleware.rst | 40 +- docs/topics/spiders.rst | 3 +- scrapy/VERSION | 2 +- scrapy/core/engine.py | 17 +- scrapy/downloadermiddlewares/httpproxy.py | 19 +- scrapy/downloadermiddlewares/offsite.py | 77 ++ scrapy/downloadermiddlewares/redirect.py | 62 +- scrapy/settings/default_settings.py | 2 +- scrapy/spidermiddlewares/offsite.py | 8 +- scrapy/utils/python.py | 2 +- tests/test_downloadermiddleware.py | 8 +- tests/test_downloadermiddleware_offsite.py | 184 +++ tests/test_downloadermiddleware_redirect.py | 1359 +++++++++++++++---- tests/test_engine.py | 40 +- tests/test_utils_project.py | 21 +- tox.ini | 17 +- 23 files changed, 1726 insertions(+), 354 deletions(-) create mode 100644 scrapy/downloadermiddlewares/offsite.py create mode 100644 tests/test_downloadermiddleware_offsite.py diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 968a34d96..599cd0cff 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 2.11.1 +current_version = 2.11.2 commit = True tag = True tag_name = {new_version} diff --git a/docs/faq.rst b/docs/faq.rst index 7090f0bcd..d394406e8 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -138,39 +138,37 @@ See previous question. How can I prevent memory errors due to many allowed domains? ------------------------------------------------------------ -If you have a spider with a long list of -:attr:`~scrapy.Spider.allowed_domains` (e.g. 50,000+), consider -replacing the default -:class:`~scrapy.spidermiddlewares.offsite.OffsiteMiddleware` spider middleware -with a :ref:`custom spider middleware ` that requires -less memory. For example: +If you have a spider with a long list of :attr:`~scrapy.Spider.allowed_domains` +(e.g. 50,000+), consider replacing the default +:class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware` downloader +middleware with a :ref:`custom downloader middleware +` that requires less memory. For example: - If your domain names are similar enough, use your own regular expression - instead joining the strings in - :attr:`~scrapy.Spider.allowed_domains` into a complex regular - expression. + instead joining the strings in :attr:`~scrapy.Spider.allowed_domains` into + a complex regular expression. - If you can `meet the installation requirements`_, use pyre2_ instead of Python’s re_ to compile your URL-filtering regular expression. See :issue:`1908`. -See also other suggestions at `StackOverflow`_. +See also `other suggestions at StackOverflow +`__. .. note:: Remember to disable - :class:`scrapy.spidermiddlewares.offsite.OffsiteMiddleware` when you enable - your custom implementation: + :class:`scrapy.downloadermiddlewares.offsite.OffsiteMiddleware` when you + enable your custom implementation: .. code-block:: python - SPIDER_MIDDLEWARES = { - "scrapy.spidermiddlewares.offsite.OffsiteMiddleware": None, - "myproject.middlewares.CustomOffsiteMiddleware": 500, + DOWNLOADER_MIDDLEWARES = { + "scrapy.downloadermiddlewares.offsite.OffsiteMiddleware": None, + "myproject.middlewares.CustomOffsiteMiddleware": 50, } .. _meet the installation requirements: https://github.com/andreasvc/pyre2#installation .. _pyre2: https://github.com/andreasvc/pyre2 .. _re: https://docs.python.org/library/re.html -.. _StackOverflow: https://stackoverflow.com/q/36440681/939364 Can I use Basic HTTP Authentication in my spiders? -------------------------------------------------- @@ -206,12 +204,10 @@ I get "Filtered offsite request" messages. How can I fix them? Those messages (logged with ``DEBUG`` level) don't necessarily mean there is a problem, so you may not need to fix them. -Those messages are thrown by the Offsite Spider Middleware, which is a spider -middleware (enabled by default) whose purpose is to filter out requests to -domains outside the ones covered by the spider. - -For more info see: -:class:`~scrapy.spidermiddlewares.offsite.OffsiteMiddleware`. +Those messages are thrown by +:class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware`, which is a +downloader middleware (enabled by default) whose purpose is to filter out +requests to domains outside the ones covered by the spider. What is the recommended way to deploy a Scrapy crawler in production? --------------------------------------------------------------------- diff --git a/docs/news.rst b/docs/news.rst index 7db4e59a1..758b22d80 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -3,7 +3,6 @@ Release notes ============= - .. _release-VERSION: Scrapy VERSION (YYYY-MM-DD) @@ -12,11 +11,122 @@ Scrapy VERSION (YYYY-MM-DD) Deprecations ~~~~~~~~~~~~ -- :func:`scrapy.core.downloader.Downloader._get_slot_key` is now deprecated. - Consider using its corresponding public method get_slot_key() instead. +- :meth:`scrapy.core.downloader.Downloader._get_slot_key` is deprecated, use + :meth:`scrapy.core.downloader.Downloader.get_slot_key` instead. (:issue:`6340`) +.. _release-2.11.2: + +Scrapy 2.11.2 (2024-05-14) +-------------------------- + +Security bug fixes +~~~~~~~~~~~~~~~~~~ + +- Redirects to non-HTTP protocols are no longer followed. Please, see the + `23j4-mw76-5v7h security advisory`_ for more information. (:issue:`457`) + + .. _23j4-mw76-5v7h security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-23j4-mw76-5v7h + +- The ``Authorization`` header is now dropped on redirects to a different + scheme (``http://`` or ``https://``) or port, even if the domain is the + same. Please, see the `4qqq-9vqf-3h3f security advisory`_ for more + information. + + .. _4qqq-9vqf-3h3f security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-4qqq-9vqf-3h3f + +- When using system proxy settings that are different for ``http://`` and + ``https://``, redirects to a different URL scheme will now also trigger the + corresponding change in proxy settings for the redirected request. Please, + see the `jm3v-qxmh-hxwv security advisory`_ for more information. + (:issue:`767`) + + .. _jm3v-qxmh-hxwv security advisory: https://github.com/scrapy/scrapy/security/advisories/GHSA-jm3v-qxmh-hxwv + +- :attr:`Spider.allowed_domains ` is now + enforced for all requests, and not only requests from spider callbacks. + (:issue:`1042`, :issue:`2241`, :issue:`6358`) + +- :func:`~scrapy.utils.iterators.xmliter_lxml` no longer resolves XML + entities. (:issue:`6265`) + +- defusedxml_ is now used to make + :class:`scrapy.http.request.rpc.XmlRpcRequest` more secure. + (:issue:`6250`, :issue:`6251`) + + .. _defusedxml: https://github.com/tiran/defusedxml + +Bug fixes +~~~~~~~~~ + +- Restored support for brotlipy_, which had been dropped in Scrapy 2.11.1 in + favor of brotli_. (:issue:`6261`) + + .. _brotli: https://github.com/google/brotli + + .. note:: brotlipy is deprecated, both in Scrapy and upstream. Use brotli + instead if you can. + +- Make :setting:`METAREFRESH_IGNORE_TAGS` ``["noscript"]`` by default. This + prevents + :class:`~scrapy.downloadermiddlewares.redirect.MetaRefreshMiddleware` from + following redirects that would not be followed by web browsers with + JavaScript enabled. (:issue:`6342`, :issue:`6347`) + +- During :ref:`feed export `, do not close the + underlying file from :ref:`built-in post-processing plugins + `. + (:issue:`5932`, :issue:`6178`, :issue:`6239`) + +- :class:`LinkExtractor ` + now properly applies the ``unique`` and ``canonicalize`` parameters. + (:issue:`3273`, :issue:`6221`) + +- Do not initialize the scheduler disk queue if :setting:`JOBDIR` is an empty + string. (:issue:`6121`, :issue:`6124`) + +- Fix :attr:`Spider.logger ` not logging custom extra + information. (:issue:`6323`, :issue:`6324`) + +- ``robots.txt`` files with a non-UTF-8 encoding no longer prevent parsing + the UTF-8-compatible (e.g. ASCII) parts of the document. + (:issue:`6292`, :issue:`6298`) + +- :meth:`scrapy.http.cookies.WrappedRequest.get_header` no longer raises an + exception if ``default`` is ``None``. + (:issue:`6308`, :issue:`6310`) + +- :class:`~scrapy.selector.Selector` now uses + :func:`scrapy.utils.response.get_base_url` to determine the base URL of a + given :class:`~scrapy.http.Response`. (:issue:`6265`) + +- The :meth:`media_to_download` method of :ref:`media pipelines + ` now logs exceptions before stripping them. + (:issue:`5067`, :issue:`5068`) + +- When passing a callback to the :command:`parse` command, build the callback + callable with the right signature. + (:issue:`6182`) + +Documentation +~~~~~~~~~~~~~ + +- Add a FAQ entry about :ref:`creating blank requests `. + (:issue:`6203`, :issue:`6208`) + +- Document that :attr:`scrapy.selector.Selector.type` can be ``"json"``. + (:issue:`6328`, :issue:`6334`) + +Quality assurance +~~~~~~~~~~~~~~~~~ + +- Make builds reproducible. (:issue:`5019`, :issue:`6322`) + +- Packaging and test fixes. + (:issue:`6286`, :issue:`6290`, :issue:`6312`, :issue:`6316`, :issue:`6344`) + + .. _release-2.11.1: Scrapy 2.11.1 (2024-02-14) diff --git a/docs/topics/benchmarking.rst b/docs/topics/benchmarking.rst index 0643df6a6..b704e54ed 100644 --- a/docs/topics/benchmarking.rst +++ b/docs/topics/benchmarking.rst @@ -24,7 +24,8 @@ You should see an output like this:: 'scrapy.extensions.telnet.TelnetConsole', 'scrapy.extensions.corestats.CoreStats'] 2016-12-16 21:18:49 [scrapy.middleware] INFO: Enabled downloader middlewares: - ['scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware', + ['scrapy.downloadermiddlewares.offsite.OffsiteMiddleware', + 'scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware', 'scrapy.downloadermiddlewares.httpauth.HttpAuthMiddleware', 'scrapy.downloadermiddlewares.downloadtimeout.DownloadTimeoutMiddleware', 'scrapy.downloadermiddlewares.defaultheaders.DefaultHeadersMiddleware', @@ -37,7 +38,6 @@ You should see an output like this:: 'scrapy.downloadermiddlewares.stats.DownloaderStats'] 2016-12-16 21:18:49 [scrapy.middleware] INFO: Enabled spider middlewares: ['scrapy.spidermiddlewares.httperror.HttpErrorMiddleware', - 'scrapy.spidermiddlewares.offsite.OffsiteMiddleware', 'scrapy.spidermiddlewares.referer.RefererMiddleware', 'scrapy.spidermiddlewares.urllength.UrlLengthMiddleware', 'scrapy.spidermiddlewares.depth.DepthMiddleware'] diff --git a/docs/topics/downloader-middleware.rst b/docs/topics/downloader-middleware.rst index d4cd062fe..c31f7fe43 100644 --- a/docs/topics/downloader-middleware.rst +++ b/docs/topics/downloader-middleware.rst @@ -763,6 +763,44 @@ HttpProxyMiddleware Keep in mind this value will take precedence over ``http_proxy``/``https_proxy`` environment variables, and it will also ignore ``no_proxy`` environment variable. +OffsiteMiddleware +----------------- + +.. module:: scrapy.downloadermiddlewares.offsite + :synopsis: Offsite Middleware + +.. class:: OffsiteMiddleware + + .. versionadded:: 2.11.2 + + Filters out Requests for URLs outside the domains covered by the spider. + + This middleware filters out every request whose host names aren't in the + spider's :attr:`~scrapy.Spider.allowed_domains` attribute. + All subdomains of any domain in the list are also allowed. + E.g. the rule ``www.example.org`` will also allow ``bob.www.example.org`` + but not ``www2.example.com`` nor ``example.com``. + + When your spider returns a request for a domain not belonging to those + covered by the spider, this middleware will log a debug message similar to + this one:: + + DEBUG: Filtered offsite request to 'offsite.example': + + To avoid filling the log with too much noise, it will only print one of + these messages for each new domain filtered. So, for example, if another + request for ``offsite.example`` is filtered, no log message will be + printed. But if a request for ``other.example`` is filtered, a message + will be printed (but only for the first request filtered). + + If the spider doesn't define an + :attr:`~scrapy.Spider.allowed_domains` attribute, or the + attribute is empty, the offsite middleware will allow all requests. + + If the request has the :attr:`~scrapy.Request.dont_filter` attribute + set, the offsite middleware will allow the request even if its domain is not + listed in allowed domains. + RedirectMiddleware ------------------ @@ -882,7 +920,11 @@ Meta tags within these tags are ignored. .. versionchanged:: 2.0 The default value of :setting:`METAREFRESH_IGNORE_TAGS` changed from - ``['script', 'noscript']`` to ``[]``. + ``["script", "noscript"]`` to ``[]``. + +.. versionchanged:: 2.11.2 + The default value of :setting:`METAREFRESH_IGNORE_TAGS` changed from + ``[]`` to ``["noscript"]``. .. versionchanged:: VERSION The default value of :setting:`METAREFRESH_IGNORE_TAGS` changed from diff --git a/docs/topics/settings.rst b/docs/topics/settings.rst index 2bd9cf1ed..904bd7ecc 100644 --- a/docs/topics/settings.rst +++ b/docs/topics/settings.rst @@ -674,6 +674,7 @@ Default: .. code-block:: python { + "scrapy.downloadermiddlewares.offsite.OffsiteMiddleware": 50, "scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware": 100, "scrapy.downloadermiddlewares.httpauth.HttpAuthMiddleware": 300, "scrapy.downloadermiddlewares.downloadtimeout.DownloadTimeoutMiddleware": 350, @@ -1613,7 +1614,6 @@ Default: { "scrapy.spidermiddlewares.httperror.HttpErrorMiddleware": 50, - "scrapy.spidermiddlewares.offsite.OffsiteMiddleware": 500, "scrapy.spidermiddlewares.referer.RefererMiddleware": 700, "scrapy.spidermiddlewares.urllength.UrlLengthMiddleware": 800, "scrapy.spidermiddlewares.depth.DepthMiddleware": 900, diff --git a/docs/topics/signals.rst b/docs/topics/signals.rst index 9bfd1761c..13e636055 100644 --- a/docs/topics/signals.rst +++ b/docs/topics/signals.rst @@ -343,11 +343,18 @@ request_scheduled .. signal:: request_scheduled .. function:: request_scheduled(request, spider) - Sent when the engine schedules a :class:`~scrapy.Request`, to be - downloaded later. + Sent when the engine is asked to schedule a :class:`~scrapy.Request`, to be + downloaded later, before the request reaches the :ref:`scheduler + `. + + Raise :exc:`~scrapy.exceptions.IgnoreRequest` to drop a request before it + reaches the scheduler. This signal does not support returning deferreds from its handlers. + .. versionadded:: 2.11.2 + Allow dropping requests with :exc:`~scrapy.exceptions.IgnoreRequest`. + :param request: the request that reached the scheduler :type request: :class:`~scrapy.Request` object diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 3f16efea5..8ddf17a14 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -51,8 +51,8 @@ value. For example, if you want to disable the off-site middleware: .. code-block:: python SPIDER_MIDDLEWARES = { - "myproject.middlewares.CustomSpiderMiddleware": 543, - "scrapy.spidermiddlewares.offsite.OffsiteMiddleware": None, + "scrapy.spidermiddlewares.referer.RefererMiddleware": None, + "myproject.middlewares.CustomRefererSpiderMiddleware": 700, } Finally, keep in mind that some middlewares may need to be enabled through a @@ -313,42 +313,6 @@ Default: ``False`` Pass all responses, regardless of its status code. -OffsiteMiddleware ------------------ - -.. module:: scrapy.spidermiddlewares.offsite - :synopsis: Offsite Spider Middleware - -.. class:: OffsiteMiddleware - - Filters out Requests for URLs outside the domains covered by the spider. - - This middleware filters out every request whose host names aren't in the - spider's :attr:`~scrapy.Spider.allowed_domains` attribute. - All subdomains of any domain in the list are also allowed. - E.g. the rule ``www.example.org`` will also allow ``bob.www.example.org`` - but not ``www2.example.com`` nor ``example.com``. - - When your spider returns a request for a domain not belonging to those - covered by the spider, this middleware will log a debug message similar to - this one:: - - DEBUG: Filtered offsite request to 'www.othersite.com': - - To avoid filling the log with too much noise, it will only print one of - these messages for each new domain filtered. So, for example, if another - request for ``www.othersite.com`` is filtered, no log message will be - printed. But if a request for ``someothersite.com`` is filtered, a message - will be printed (but only for the first request filtered). - - If the spider doesn't define an - :attr:`~scrapy.Spider.allowed_domains` attribute, or the - attribute is empty, the offsite middleware will allow all requests. - - If the request has the :attr:`~scrapy.Request.dont_filter` attribute - set, the offsite middleware will allow the request even if its domain is not - listed in allowed domains. - RefererMiddleware ----------------- diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 30677fe74..8a0102a51 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -75,7 +75,8 @@ scrapy.Spider An optional list of strings containing domains that this spider is allowed to crawl. Requests for URLs not belonging to the domain names specified in this list (or their subdomains) won't be followed if - :class:`~scrapy.spidermiddlewares.offsite.OffsiteMiddleware` is enabled. + :class:`~scrapy.downloadermiddlewares.offsite.OffsiteMiddleware` is + enabled. Let's say your target url is ``https://www.example.com/1.html``, then add ``'example.com'`` to the list. diff --git a/scrapy/VERSION b/scrapy/VERSION index 6ceb272ee..9e5bb77a3 100644 --- a/scrapy/VERSION +++ b/scrapy/VERSION @@ -1 +1 @@ -2.11.1 +2.11.2 diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 6bf3f3e26..4eca03800 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -28,7 +28,7 @@ from twisted.python.failure import Failure from scrapy import signals from scrapy.core.downloader import Downloader from scrapy.core.scraper import Scraper -from scrapy.exceptions import CloseSpider, DontCloseSpider +from scrapy.exceptions import CloseSpider, DontCloseSpider, IgnoreRequest from scrapy.http import Request, Response from scrapy.logformatter import LogFormatter from scrapy.settings import BaseSettings, Settings @@ -36,6 +36,7 @@ from scrapy.signalmanager import SignalManager from scrapy.spiders import Spider from scrapy.utils.log import failure_to_exc_info, logformatter_adapter from scrapy.utils.misc import build_from_crawler, load_object +from scrapy.utils.python import global_object_name from scrapy.utils.reactor import CallLaterOnce if TYPE_CHECKING: @@ -292,9 +293,19 @@ class ExecutionEngine: self.slot.nextcall.schedule() # type: ignore[union-attr] def _schedule_request(self, request: Request, spider: Spider) -> None: - self.signals.send_catch_log( - signals.request_scheduled, request=request, spider=spider + request_scheduled_result = self.signals.send_catch_log( + signals.request_scheduled, + request=request, + spider=spider, + dont_log=IgnoreRequest, ) + for handler, result in request_scheduled_result: + if isinstance(result, Failure) and isinstance(result.value, IgnoreRequest): + logger.debug( + f"Signal handler {global_object_name(handler)} dropped " + f"request {request} before it reached the scheduler." + ) + return if not self.slot.scheduler.enqueue_request(request): # type: ignore[union-attr] self.signals.send_catch_log( signals.request_dropped, request=request, spider=spider diff --git a/scrapy/downloadermiddlewares/httpproxy.py b/scrapy/downloadermiddlewares/httpproxy.py index 335896ac1..5b56ad449 100644 --- a/scrapy/downloadermiddlewares/httpproxy.py +++ b/scrapy/downloadermiddlewares/httpproxy.py @@ -60,26 +60,33 @@ class HttpProxyMiddleware: def process_request( self, request: Request, spider: Spider ) -> Union[Request, Response, None]: - creds, proxy_url = None, None + creds, proxy_url, scheme = None, None, None if "proxy" in request.meta: if request.meta["proxy"] is not None: creds, proxy_url = self._get_proxy(request.meta["proxy"], "") elif self.proxies: parsed = urlparse_cached(request) - scheme = parsed.scheme + _scheme = parsed.scheme if ( # 'no_proxy' is only supported by http schemes - scheme not in ("http", "https") + _scheme not in ("http", "https") or (parsed.hostname and not proxy_bypass(parsed.hostname)) - ) and scheme in self.proxies: + ) and _scheme in self.proxies: + scheme = _scheme creds, proxy_url = self.proxies[scheme] - self._set_proxy_and_creds(request, proxy_url, creds) + self._set_proxy_and_creds(request, proxy_url, creds, scheme) return None def _set_proxy_and_creds( - self, request: Request, proxy_url: Optional[str], creds: Optional[bytes] + self, + request: Request, + proxy_url: Optional[str], + creds: Optional[bytes], + scheme: Optional[str], ) -> None: + if scheme: + request.meta["_scheme_proxy"] = True if proxy_url: request.meta["proxy"] = proxy_url elif request.meta.get("proxy") is not None: diff --git a/scrapy/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py new file mode 100644 index 000000000..1e5026925 --- /dev/null +++ b/scrapy/downloadermiddlewares/offsite.py @@ -0,0 +1,77 @@ +import logging +import re +import warnings + +from scrapy import signals +from scrapy.exceptions import IgnoreRequest +from scrapy.utils.httpobj import urlparse_cached + +logger = logging.getLogger(__name__) + + +class OffsiteMiddleware: + @classmethod + def from_crawler(cls, crawler): + o = cls(crawler.stats) + crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) + crawler.signals.connect(o.request_scheduled, signal=signals.request_scheduled) + return o + + def __init__(self, stats): + self.stats = stats + self.domains_seen = set() + + def spider_opened(self, spider): + self.host_regex = self.get_host_regex(spider) + + def request_scheduled(self, request, spider): + self.process_request(request, spider) + + def process_request(self, request, spider): + if request.dont_filter or self.should_follow(request, spider): + return None + domain = urlparse_cached(request).hostname + if domain and domain not in self.domains_seen: + self.domains_seen.add(domain) + logger.debug( + "Filtered offsite request to %(domain)r: %(request)s", + {"domain": domain, "request": request}, + extra={"spider": spider}, + ) + self.stats.inc_value("offsite/domains", spider=spider) + self.stats.inc_value("offsite/filtered", spider=spider) + raise IgnoreRequest + + def should_follow(self, request, spider): + regex = self.host_regex + # hostname can be None for wrong urls (like javascript links) + host = urlparse_cached(request).hostname or "" + return bool(regex.search(host)) + + def get_host_regex(self, spider): + """Override this method to implement a different offsite policy""" + allowed_domains = getattr(spider, "allowed_domains", None) + if not allowed_domains: + return re.compile("") # allow all by default + url_pattern = re.compile(r"^https?://.*$") + port_pattern = re.compile(r":\d+$") + domains = [] + for domain in allowed_domains: + if domain is None: + continue + if url_pattern.match(domain): + message = ( + "allowed_domains accepts only domains, not URLs. " + f"Ignoring URL entry {domain} in allowed_domains." + ) + warnings.warn(message) + elif port_pattern.search(domain): + message = ( + "allowed_domains accepts only domains without ports. " + f"Ignoring entry {domain} in allowed_domains." + ) + warnings.warn(message) + else: + domains.append(re.escape(domain)) + regex = rf'^(.*\.)?({"|".join(domains)})$' + return re.compile(regex) diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 24089afea..aa08827c4 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -29,17 +29,49 @@ def _build_redirect_request( **kwargs, cookies=None, ) + if "_scheme_proxy" in redirect_request.meta: + source_request_scheme = urlparse_cached(source_request).scheme + redirect_request_scheme = urlparse_cached(redirect_request).scheme + if source_request_scheme != redirect_request_scheme: + redirect_request.meta.pop("_scheme_proxy") + redirect_request.meta.pop("proxy", None) + redirect_request.meta.pop("_auth_proxy", None) + redirect_request.headers.pop(b"Proxy-Authorization", None) has_cookie_header = "Cookie" in redirect_request.headers has_authorization_header = "Authorization" in redirect_request.headers if has_cookie_header or has_authorization_header: - source_request_netloc = urlparse_cached(source_request).netloc - redirect_request_netloc = urlparse_cached(redirect_request).netloc - if source_request_netloc != redirect_request_netloc: - if has_cookie_header: - del redirect_request.headers["Cookie"] - # https://fetch.spec.whatwg.org/#ref-for-cors-non-wildcard-request-header-name - if has_authorization_header: - del redirect_request.headers["Authorization"] + default_ports = {"http": 80, "https": 443} + + parsed_source_request = urlparse_cached(source_request) + source_scheme, source_host, source_port = ( + parsed_source_request.scheme, + parsed_source_request.hostname, + parsed_source_request.port + or default_ports.get(parsed_source_request.scheme), + ) + + parsed_redirect_request = urlparse_cached(redirect_request) + redirect_scheme, redirect_host, redirect_port = ( + parsed_redirect_request.scheme, + parsed_redirect_request.hostname, + parsed_redirect_request.port + or default_ports.get(parsed_redirect_request.scheme), + ) + + if has_cookie_header and ( + redirect_scheme not in {source_scheme, "https"} + or source_host != redirect_host + ): + del redirect_request.headers["Cookie"] + + # https://fetch.spec.whatwg.org/#ref-for-cors-non-wildcard-request-header-name + if has_authorization_header and ( + source_scheme != redirect_scheme + or source_host != redirect_host + or source_port != redirect_port + ): + del redirect_request.headers["Authorization"] + return redirect_request @@ -129,9 +161,11 @@ class RedirectMiddleware(BaseRedirectMiddleware): location = request_scheme + "://" + location.lstrip("/") redirected_url = urljoin(request.url, location) + redirected = _build_redirect_request(request, url=redirected_url) + if urlparse_cached(redirected).scheme not in {"http", "https"}: + return response if response.status in (301, 307, 308) or request.method == "HEAD": - redirected = _build_redirect_request(request, url=redirected_url) return self._redirect(redirected, request, spider, response.status) redirected = self._redirect_request_using_get(request, redirected_url) @@ -153,12 +187,16 @@ class MetaRefreshMiddleware(BaseRedirectMiddleware): request.meta.get("dont_redirect", False) or request.method == "HEAD" or not isinstance(response, HtmlResponse) + or urlparse_cached(request).scheme not in {"http", "https"} ): return response interval, url = get_meta_refresh(response, ignore_tags=self._ignore_tags) - if url and cast(float, interval) < self._maxdelay: - redirected = self._redirect_request_using_get(request, url) + if not url: + return response + redirected = self._redirect_request_using_get(request, url) + if urlparse_cached(redirected).scheme not in {"http", "https"}: + return response + if cast(float, interval) < self._maxdelay: return self._redirect(redirected, request, spider, "meta refresh") - return response diff --git a/scrapy/settings/default_settings.py b/scrapy/settings/default_settings.py index d7ac7ec35..932475fb5 100644 --- a/scrapy/settings/default_settings.py +++ b/scrapy/settings/default_settings.py @@ -101,6 +101,7 @@ DOWNLOADER_MIDDLEWARES = {} DOWNLOADER_MIDDLEWARES_BASE = { # Engine side + "scrapy.downloadermiddlewares.offsite.OffsiteMiddleware": 50, "scrapy.downloadermiddlewares.robotstxt.RobotsTxtMiddleware": 100, "scrapy.downloadermiddlewares.httpauth.HttpAuthMiddleware": 300, "scrapy.downloadermiddlewares.downloadtimeout.DownloadTimeoutMiddleware": 350, @@ -301,7 +302,6 @@ SPIDER_MIDDLEWARES = {} SPIDER_MIDDLEWARES_BASE = { # Engine side "scrapy.spidermiddlewares.httperror.HttpErrorMiddleware": 50, - "scrapy.spidermiddlewares.offsite.OffsiteMiddleware": 500, "scrapy.spidermiddlewares.referer.RefererMiddleware": 700, "scrapy.spidermiddlewares.urllength.UrlLengthMiddleware": 800, "scrapy.spidermiddlewares.depth.DepthMiddleware": 900, diff --git a/scrapy/spidermiddlewares/offsite.py b/scrapy/spidermiddlewares/offsite.py index dd2fccfcb..50c93ac9f 100644 --- a/scrapy/spidermiddlewares/offsite.py +++ b/scrapy/spidermiddlewares/offsite.py @@ -13,15 +13,21 @@ from typing import TYPE_CHECKING, Any, AsyncIterable, Iterable, Set from scrapy import Spider, signals from scrapy.crawler import Crawler +from scrapy.exceptions import ScrapyDeprecationWarning from scrapy.http import Request, Response from scrapy.statscollectors import StatsCollector from scrapy.utils.httpobj import urlparse_cached +warnings.warn( + "The scrapy.spidermiddlewares.offsite module is deprecated, use " + "scrapy.downloadermiddlewares.offsite instead.", + ScrapyDeprecationWarning, +) + if TYPE_CHECKING: # typing.Self requires Python 3.11 from typing_extensions import Self - logger = logging.getLogger(__name__) diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 5d2d490b2..578cde2ac 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -331,7 +331,7 @@ def global_object_name(obj: Any) -> str: >>> global_object_name(Request) 'scrapy.http.request.Request' """ - return f"{obj.__module__}.{obj.__name__}" + return f"{obj.__module__}.{obj.__qualname__}" if hasattr(sys, "pypy_version_info"): diff --git a/tests/test_downloadermiddleware.py b/tests/test_downloadermiddleware.py index 062e8a8b4..0155c62eb 100644 --- a/tests/test_downloadermiddleware.py +++ b/tests/test_downloadermiddleware.py @@ -22,13 +22,11 @@ class ManagerTestCase(TestCase): self.crawler = get_crawler(Spider, self.settings_dict) self.spider = self.crawler._create_spider("foo") self.mwman = DownloaderMiddlewareManager.from_crawler(self.crawler) - # some mw depends on stats collector - self.crawler.stats.open_spider(self.spider) - return self.mwman.open_spider(self.spider) + self.crawler.engine = self.crawler._create_engine() + return self.crawler.engine.open_spider(self.spider, start_requests=()) def tearDown(self): - self.crawler.stats.close_spider(self.spider, "") - return self.mwman.close_spider(self.spider) + return self.crawler.engine.close_spider(self.spider) def _download(self, request, response=None): """Executes downloader mw manager's download method and returns diff --git a/tests/test_downloadermiddleware_offsite.py b/tests/test_downloadermiddleware_offsite.py new file mode 100644 index 000000000..d4669f450 --- /dev/null +++ b/tests/test_downloadermiddleware_offsite.py @@ -0,0 +1,184 @@ +import pytest + +from scrapy import Request, Spider +from scrapy.downloadermiddlewares.offsite import OffsiteMiddleware +from scrapy.exceptions import IgnoreRequest +from scrapy.utils.test import get_crawler + +UNSET = object() + + +@pytest.mark.parametrize( + ("allowed_domain", "url", "allowed"), + ( + ("example.com", "http://example.com/1", True), + ("example.com", "http://example.org/1", False), + ("example.com", "http://sub.example.com/1", True), + ("sub.example.com", "http://sub.example.com/1", True), + ("sub.example.com", "http://example.com/1", False), + ("example.com", "http://example.com:8000/1", True), + ("example.com", "http://example.org/example.com", False), + ("example.com", "http://example.org/foo.example.com", False), + ("example.com", "http://example.com.example", False), + ("a.example", "http://nota.example", False), + ("b.a.example", "http://notb.a.example", False), + ), +) +def test_process_request_domain_filtering(allowed_domain, url, allowed): + crawler = get_crawler(Spider) + spider = crawler._create_spider(name="a", allowed_domains=[allowed_domain]) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request(url) + if allowed: + assert mw.process_request(request, spider) is None + else: + with pytest.raises(IgnoreRequest): + mw.process_request(request, spider) + + +@pytest.mark.parametrize( + ("value", "filtered"), + ( + (UNSET, True), + (None, True), + (False, True), + (True, False), + ), +) +def test_process_request_dont_filter(value, filtered): + crawler = get_crawler(Spider) + spider = crawler._create_spider(name="a", allowed_domains=["a.example"]) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + kwargs = {} + if value is not UNSET: + kwargs["dont_filter"] = value + request = Request("https://b.example", **kwargs) + if filtered: + with pytest.raises(IgnoreRequest): + mw.process_request(request, spider) + else: + assert mw.process_request(request, spider) is None + + +@pytest.mark.parametrize( + "value", + ( + UNSET, + None, + [], + ), +) +def test_process_request_no_allowed_domains(value): + crawler = get_crawler(Spider) + kwargs = {} + if value is not UNSET: + kwargs["allowed_domains"] = value + spider = crawler._create_spider(name="a", **kwargs) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request("https://example.com") + assert mw.process_request(request, spider) is None + + +def test_process_request_invalid_domains(): + crawler = get_crawler(Spider) + allowed_domains = ["a.example", None, "http:////b.example", "//c.example"] + spider = crawler._create_spider(name="a", allowed_domains=allowed_domains) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request("https://a.example") + assert mw.process_request(request, spider) is None + for letter in ("b", "c"): + request = Request(f"https://{letter}.example") + with pytest.raises(IgnoreRequest): + mw.process_request(request, spider) + + +@pytest.mark.parametrize( + ("allowed_domain", "url", "allowed"), + ( + ("example.com", "http://example.com/1", True), + ("example.com", "http://example.org/1", False), + ("example.com", "http://sub.example.com/1", True), + ("sub.example.com", "http://sub.example.com/1", True), + ("sub.example.com", "http://example.com/1", False), + ("example.com", "http://example.com:8000/1", True), + ("example.com", "http://example.org/example.com", False), + ("example.com", "http://example.org/foo.example.com", False), + ("example.com", "http://example.com.example", False), + ("a.example", "http://nota.example", False), + ("b.a.example", "http://notb.a.example", False), + ), +) +def test_request_scheduled_domain_filtering(allowed_domain, url, allowed): + crawler = get_crawler(Spider) + spider = crawler._create_spider(name="a", allowed_domains=[allowed_domain]) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request(url) + if allowed: + assert mw.request_scheduled(request, spider) is None + else: + with pytest.raises(IgnoreRequest): + mw.request_scheduled(request, spider) + + +@pytest.mark.parametrize( + ("value", "filtered"), + ( + (UNSET, True), + (None, True), + (False, True), + (True, False), + ), +) +def test_request_scheduled_dont_filter(value, filtered): + crawler = get_crawler(Spider) + spider = crawler._create_spider(name="a", allowed_domains=["a.example"]) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + kwargs = {} + if value is not UNSET: + kwargs["dont_filter"] = value + request = Request("https://b.example", **kwargs) + if filtered: + with pytest.raises(IgnoreRequest): + mw.request_scheduled(request, spider) + else: + assert mw.request_scheduled(request, spider) is None + + +@pytest.mark.parametrize( + "value", + ( + UNSET, + None, + [], + ), +) +def test_request_scheduled_no_allowed_domains(value): + crawler = get_crawler(Spider) + kwargs = {} + if value is not UNSET: + kwargs["allowed_domains"] = value + spider = crawler._create_spider(name="a", **kwargs) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request("https://example.com") + assert mw.request_scheduled(request, spider) is None + + +def test_request_scheduled_invalid_domains(): + crawler = get_crawler(Spider) + allowed_domains = ["a.example", None, "http:////b.example", "//c.example"] + spider = crawler._create_spider(name="a", allowed_domains=allowed_domains) + mw = OffsiteMiddleware.from_crawler(crawler) + mw.spider_opened(spider) + request = Request("https://a.example") + assert mw.request_scheduled(request, spider) is None + for letter in ("b", "c"): + request = Request(f"https://{letter}.example") + with pytest.raises(IgnoreRequest): + mw.request_scheduled(request, spider) diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index 83ff25982..4bfd34fe2 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -1,5 +1,9 @@ import unittest +from itertools import chain, product +import pytest + +from scrapy.downloadermiddlewares.httpproxy import HttpProxyMiddleware from scrapy.downloadermiddlewares.redirect import ( MetaRefreshMiddleware, RedirectMiddleware, @@ -7,22 +11,1030 @@ from scrapy.downloadermiddlewares.redirect import ( from scrapy.exceptions import IgnoreRequest from scrapy.http import HtmlResponse, Request, Response from scrapy.spiders import Spider +from scrapy.utils.misc import set_environ from scrapy.utils.test import get_crawler -class RedirectMiddlewareTest(unittest.TestCase): +class Base: + class Test(unittest.TestCase): + def test_priority_adjust(self): + req = Request("http://a.com") + rsp = self.get_response(req, "http://a.com/redirected") + req2 = self.mw.process_response(req, rsp, self.spider) + self.assertGreater(req2.priority, req.priority) + + def test_dont_redirect(self): + url = "http://www.example.com/301" + url2 = "http://www.example.com/redirected" + req = Request(url, meta={"dont_redirect": True}) + rsp = self.get_response(req, url2) + + r = self.mw.process_response(req, rsp, self.spider) + assert isinstance(r, Response) + assert r is rsp + + # Test that it redirects when dont_redirect is False + req = Request(url, meta={"dont_redirect": False}) + rsp = self.get_response(req, url2) + + r = self.mw.process_response(req, rsp, self.spider) + assert isinstance(r, Request) + + def test_post(self): + url = "http://www.example.com/302" + url2 = "http://www.example.com/redirected2" + req = Request( + url, + method="POST", + body="test", + headers={"Content-Type": "text/plain", "Content-length": "4"}, + ) + rsp = self.get_response(req, url2) + + req2 = self.mw.process_response(req, rsp, self.spider) + assert isinstance(req2, Request) + self.assertEqual(req2.url, url2) + self.assertEqual(req2.method, "GET") + assert ( + "Content-Type" not in req2.headers + ), "Content-Type header must not be present in redirected request" + assert ( + "Content-Length" not in req2.headers + ), "Content-Length header must not be present in redirected request" + assert not req2.body, f"Redirected body must be empty, not '{req2.body}'" + + def test_max_redirect_times(self): + self.mw.max_redirect_times = 1 + req = Request("http://scrapytest.org/302") + rsp = self.get_response(req, "/redirected") + + req = self.mw.process_response(req, rsp, self.spider) + assert isinstance(req, Request) + assert "redirect_times" in req.meta + self.assertEqual(req.meta["redirect_times"], 1) + self.assertRaises( + IgnoreRequest, self.mw.process_response, req, rsp, self.spider + ) + + def test_ttl(self): + self.mw.max_redirect_times = 100 + req = Request("http://scrapytest.org/302", meta={"redirect_ttl": 1}) + rsp = self.get_response(req, "/a") + + req = self.mw.process_response(req, rsp, self.spider) + assert isinstance(req, Request) + self.assertRaises( + IgnoreRequest, self.mw.process_response, req, rsp, self.spider + ) + + def test_redirect_urls(self): + req1 = Request("http://scrapytest.org/first") + rsp1 = self.get_response(req1, "/redirected") + req2 = self.mw.process_response(req1, rsp1, self.spider) + rsp2 = self.get_response(req1, "/redirected2") + req3 = self.mw.process_response(req2, rsp2, self.spider) + + self.assertEqual(req2.url, "http://scrapytest.org/redirected") + self.assertEqual( + req2.meta["redirect_urls"], ["http://scrapytest.org/first"] + ) + self.assertEqual(req3.url, "http://scrapytest.org/redirected2") + self.assertEqual( + req3.meta["redirect_urls"], + ["http://scrapytest.org/first", "http://scrapytest.org/redirected"], + ) + + def test_redirect_reasons(self): + req1 = Request("http://scrapytest.org/first") + rsp1 = self.get_response(req1, "/redirected1") + req2 = self.mw.process_response(req1, rsp1, self.spider) + rsp2 = self.get_response(req2, "/redirected2") + req3 = self.mw.process_response(req2, rsp2, self.spider) + self.assertEqual(req2.meta["redirect_reasons"], [self.reason]) + self.assertEqual(req3.meta["redirect_reasons"], [self.reason, self.reason]) + + def test_cross_origin_header_dropping(self): + safe_headers = {"A": "B"} + cookie_header = {"Cookie": "a=b"} + authorization_header = {"Authorization": "Bearer 123456"} + + original_request = Request( + "https://example.com", + headers={**safe_headers, **cookie_header, **authorization_header}, + ) + + # Redirects to the same origin (same scheme, same domain, same port) + # keep all headers. + internal_response = self.get_response( + original_request, "https://example.com/a" + ) + internal_redirect_request = self.mw.process_response( + original_request, internal_response, self.spider + ) + self.assertIsInstance(internal_redirect_request, Request) + self.assertEqual( + original_request.headers, internal_redirect_request.headers + ) + + # Redirects to the same origin (same scheme, same domain, same port) + # keep all headers also when the scheme is http. + http_request = Request( + "http://example.com", + headers={**safe_headers, **cookie_header, **authorization_header}, + ) + http_response = self.get_response(http_request, "http://example.com/a") + http_redirect_request = self.mw.process_response( + http_request, http_response, self.spider + ) + self.assertIsInstance(http_redirect_request, Request) + self.assertEqual(http_request.headers, http_redirect_request.headers) + + # For default ports, whether the port is explicit or implicit does not + # affect the outcome, it is still the same origin. + to_explicit_port_response = self.get_response( + original_request, "https://example.com:443/a" + ) + to_explicit_port_redirect_request = self.mw.process_response( + original_request, to_explicit_port_response, self.spider + ) + self.assertIsInstance(to_explicit_port_redirect_request, Request) + self.assertEqual( + original_request.headers, to_explicit_port_redirect_request.headers + ) + + # For default ports, whether the port is explicit or implicit does not + # affect the outcome, it is still the same origin. + to_implicit_port_response = self.get_response( + original_request, "https://example.com/a" + ) + to_implicit_port_redirect_request = self.mw.process_response( + original_request, to_implicit_port_response, self.spider + ) + self.assertIsInstance(to_implicit_port_redirect_request, Request) + self.assertEqual( + original_request.headers, to_implicit_port_redirect_request.headers + ) + + # A port change drops the Authorization header because the origin + # changes, but keeps the Cookie header because the domain remains the + # same. + different_port_response = self.get_response( + original_request, "https://example.com:8080/a" + ) + different_port_redirect_request = self.mw.process_response( + original_request, different_port_response, self.spider + ) + self.assertIsInstance(different_port_redirect_request, Request) + self.assertEqual( + {**safe_headers, **cookie_header}, + different_port_redirect_request.headers.to_unicode_dict(), + ) + + # A domain change drops both the Authorization and the Cookie header. + external_response = self.get_response( + original_request, "https://example.org/a" + ) + external_redirect_request = self.mw.process_response( + original_request, external_response, self.spider + ) + self.assertIsInstance(external_redirect_request, Request) + self.assertEqual( + safe_headers, external_redirect_request.headers.to_unicode_dict() + ) + + # A scheme upgrade (http → https) drops the Authorization header + # because the origin changes, but keeps the Cookie header because the + # domain remains the same. + upgrade_response = self.get_response(http_request, "https://example.com/a") + upgrade_redirect_request = self.mw.process_response( + http_request, upgrade_response, self.spider + ) + self.assertIsInstance(upgrade_redirect_request, Request) + self.assertEqual( + {**safe_headers, **cookie_header}, + upgrade_redirect_request.headers.to_unicode_dict(), + ) + + # A scheme downgrade (https → http) drops the Authorization header + # because the origin changes, and the Cookie header because its value + # cannot indicate whether the cookies were secure (HTTPS-only) or not. + # + # Note: If the Cookie header is set by the cookie management + # middleware, as recommended in the docs, the dropping of Cookie on + # scheme downgrade is not an issue, because the cookie management + # middleware will add again the Cookie header to the new request if + # appropriate. + downgrade_response = self.get_response( + original_request, "http://example.com/a" + ) + downgrade_redirect_request = self.mw.process_response( + original_request, downgrade_response, self.spider + ) + self.assertIsInstance(downgrade_redirect_request, Request) + self.assertEqual( + safe_headers, + downgrade_redirect_request.headers.to_unicode_dict(), + ) + + def test_meta_proxy_http_absolute(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + meta = {"proxy": "https://a:@a.example"} + request1 = Request("http://example.com", meta=meta) + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "http://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "http://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_meta_proxy_http_relative(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + meta = {"proxy": "https://a:@a.example"} + request1 = Request("http://example.com", meta=meta) + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "/a") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "/a") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_meta_proxy_https_absolute(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + meta = {"proxy": "https://a:@a.example"} + request1 = Request("https://example.com", meta=meta) + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "https://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "https://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_meta_proxy_https_relative(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + meta = {"proxy": "https://a:@a.example"} + request1 = Request("https://example.com", meta=meta) + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "/a") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "/a") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_meta_proxy_http_to_https(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + meta = {"proxy": "https://a:@a.example"} + request1 = Request("http://example.com", meta=meta) + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "https://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "http://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_meta_proxy_https_to_http(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + meta = {"proxy": "https://a:@a.example"} + request1 = Request("https://example.com", meta=meta) + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "http://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "https://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_system_proxy_http_absolute(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "http_proxy": "https://a:@a.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("http://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "http://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "http://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_system_proxy_http_relative(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "http_proxy": "https://a:@a.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("http://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "/a") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "/a") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_system_proxy_https_absolute(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "https_proxy": "https://a:@a.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("https://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "https://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "https://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_system_proxy_https_relative(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "https_proxy": "https://a:@a.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("https://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "/a") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "/a") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_system_proxy_proxied_http_to_proxied_https(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "http_proxy": "https://a:@a.example", + "https_proxy": "https://b:@b.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("http://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "https://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic Yjo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://b.example") + self.assertEqual(request2.meta["proxy"], "https://b.example") + + response2 = self.get_response(request2, "http://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_system_proxy_proxied_http_to_unproxied_https(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "http_proxy": "https://a:@a.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("http://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request1.meta["proxy"], "https://a.example") + + response1 = self.get_response(request1, "https://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + proxy_mw.process_request(request2, spider) + + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + response2 = self.get_response(request2, "http://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request3.meta["proxy"], "https://a.example") + + def test_system_proxy_unproxied_http_to_proxied_https(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "https_proxy": "https://b:@b.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("http://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertNotIn("Proxy-Authorization", request1.headers) + self.assertNotIn("_auth_proxy", request1.meta) + self.assertNotIn("proxy", request1.meta) + + response1 = self.get_response(request1, "https://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic Yjo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://b.example") + self.assertEqual(request2.meta["proxy"], "https://b.example") + + response2 = self.get_response(request2, "http://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + proxy_mw.process_request(request3, spider) + + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + def test_system_proxy_unproxied_http_to_unproxied_https(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("http://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertNotIn("Proxy-Authorization", request1.headers) + self.assertNotIn("_auth_proxy", request1.meta) + self.assertNotIn("proxy", request1.meta) + + response1 = self.get_response(request1, "https://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + proxy_mw.process_request(request2, spider) + + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + response2 = self.get_response(request2, "http://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + proxy_mw.process_request(request3, spider) + + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + def test_system_proxy_proxied_https_to_proxied_http(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "http_proxy": "https://a:@a.example", + "https_proxy": "https://b:@b.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("https://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic Yjo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://b.example") + self.assertEqual(request1.meta["proxy"], "https://b.example") + + response1 = self.get_response(request1, "http://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "https://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic Yjo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://b.example") + self.assertEqual(request3.meta["proxy"], "https://b.example") + + def test_system_proxy_proxied_https_to_unproxied_http(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "https_proxy": "https://b:@b.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("https://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertEqual(request1.headers["Proxy-Authorization"], b"Basic Yjo=") + self.assertEqual(request1.meta["_auth_proxy"], "https://b.example") + self.assertEqual(request1.meta["proxy"], "https://b.example") + + response1 = self.get_response(request1, "http://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + proxy_mw.process_request(request2, spider) + + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + response2 = self.get_response(request2, "https://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + proxy_mw.process_request(request3, spider) + + self.assertEqual(request3.headers["Proxy-Authorization"], b"Basic Yjo=") + self.assertEqual(request3.meta["_auth_proxy"], "https://b.example") + self.assertEqual(request3.meta["proxy"], "https://b.example") + + def test_system_proxy_unproxied_https_to_proxied_http(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + env = { + "http_proxy": "https://a:@a.example", + } + with set_environ(**env): + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("https://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertNotIn("Proxy-Authorization", request1.headers) + self.assertNotIn("_auth_proxy", request1.meta) + self.assertNotIn("proxy", request1.meta) + + response1 = self.get_response(request1, "http://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + proxy_mw.process_request(request2, spider) + + self.assertEqual(request2.headers["Proxy-Authorization"], b"Basic YTo=") + self.assertEqual(request2.meta["_auth_proxy"], "https://a.example") + self.assertEqual(request2.meta["proxy"], "https://a.example") + + response2 = self.get_response(request2, "https://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + proxy_mw.process_request(request3, spider) + + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + def test_system_proxy_unproxied_https_to_unproxied_http(self): + crawler = get_crawler() + redirect_mw = self.mwcls.from_crawler(crawler) + proxy_mw = HttpProxyMiddleware.from_crawler(crawler) + + request1 = Request("https://example.com") + spider = None + proxy_mw.process_request(request1, spider) + + self.assertNotIn("Proxy-Authorization", request1.headers) + self.assertNotIn("_auth_proxy", request1.meta) + self.assertNotIn("proxy", request1.meta) + + response1 = self.get_response(request1, "http://example.com") + request2 = redirect_mw.process_response(request1, response1, spider) + + self.assertIsInstance(request2, Request) + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + proxy_mw.process_request(request2, spider) + + self.assertNotIn("Proxy-Authorization", request2.headers) + self.assertNotIn("_auth_proxy", request2.meta) + self.assertNotIn("proxy", request2.meta) + + response2 = self.get_response(request2, "https://example.com") + request3 = redirect_mw.process_response(request2, response2, spider) + + self.assertIsInstance(request3, Request) + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + proxy_mw.process_request(request3, spider) + + self.assertNotIn("Proxy-Authorization", request3.headers) + self.assertNotIn("_auth_proxy", request3.meta) + self.assertNotIn("proxy", request3.meta) + + +class RedirectMiddlewareTest(Base.Test): + mwcls = RedirectMiddleware + reason = 302 + def setUp(self): self.crawler = get_crawler(Spider) self.spider = self.crawler._create_spider("foo") - self.mw = RedirectMiddleware.from_crawler(self.crawler) + self.mw = self.mwcls.from_crawler(self.crawler) - def test_priority_adjust(self): - req = Request("http://a.com") - rsp = Response( - "http://a.com", headers={"Location": "http://a.com/redirected"}, status=301 - ) - req2 = self.mw.process_response(req, rsp, self.spider) - assert req2.priority > req.priority + def get_response(self, request, location, status=302): + headers = {"Location": location} + return Response(request.url, status=status, headers=headers) def test_redirect_3xx_permanent(self): def _test(method, status=301): @@ -52,51 +1064,6 @@ class RedirectMiddlewareTest(unittest.TestCase): _test("POST", status=308) _test("HEAD", status=308) - def test_dont_redirect(self): - url = "http://www.example.com/301" - url2 = "http://www.example.com/redirected" - req = Request(url, meta={"dont_redirect": True}) - rsp = Response(url, headers={"Location": url2}, status=301) - - r = self.mw.process_response(req, rsp, self.spider) - assert isinstance(r, Response) - assert r is rsp - - # Test that it redirects when dont_redirect is False - req = Request(url, meta={"dont_redirect": False}) - rsp = Response(url2, status=200) - - r = self.mw.process_response(req, rsp, self.spider) - assert isinstance(r, Response) - assert r is rsp - - def test_redirect_302(self): - url = "http://www.example.com/302" - url2 = "http://www.example.com/redirected2" - req = Request( - url, - method="POST", - body="test", - headers={"Content-Type": "text/plain", "Content-length": "4"}, - ) - rsp = Response(url, headers={"Location": url2}, status=302) - - req2 = self.mw.process_response(req, rsp, self.spider) - assert isinstance(req2, Request) - self.assertEqual(req2.url, url2) - self.assertEqual(req2.method, "GET") - assert ( - "Content-Type" not in req2.headers - ), "Content-Type header must not be present in redirected request" - assert ( - "Content-Length" not in req2.headers - ), "Content-Length header must not be present in redirected request" - assert not req2.body, f"Redirected body must be empty, not '{req2.body}'" - - # response without Location header but with status code is 3XX should be ignored - del rsp.headers["Location"] - assert self.mw.process_response(req, rsp, self.spider) is rsp - def test_redirect_302_head(self): url = "http://www.example.com/302" url2 = "http://www.example.com/redirected2" @@ -108,10 +1075,6 @@ class RedirectMiddlewareTest(unittest.TestCase): self.assertEqual(req2.url, url2) self.assertEqual(req2.method, "HEAD") - # response without Location header but with status code is 3XX should be ignored - del rsp.headers["Location"] - assert self.mw.process_response(req, rsp, self.spider) is rsp - def test_redirect_302_relative(self): url = "http://www.example.com/302" url2 = "///i8n.example2.com/302" @@ -124,81 +1087,6 @@ class RedirectMiddlewareTest(unittest.TestCase): self.assertEqual(req2.url, url3) self.assertEqual(req2.method, "HEAD") - # response without Location header but with status code is 3XX should be ignored - del rsp.headers["Location"] - assert self.mw.process_response(req, rsp, self.spider) is rsp - - def test_max_redirect_times(self): - self.mw.max_redirect_times = 1 - req = Request("http://scrapytest.org/302") - rsp = Response( - "http://scrapytest.org/302", headers={"Location": "/redirected"}, status=302 - ) - - req = self.mw.process_response(req, rsp, self.spider) - assert isinstance(req, Request) - assert "redirect_times" in req.meta - self.assertEqual(req.meta["redirect_times"], 1) - self.assertRaises( - IgnoreRequest, self.mw.process_response, req, rsp, self.spider - ) - - def test_ttl(self): - self.mw.max_redirect_times = 100 - req = Request("http://scrapytest.org/302", meta={"redirect_ttl": 1}) - rsp = Response( - "http://www.scrapytest.org/302", - headers={"Location": "/redirected"}, - status=302, - ) - - req = self.mw.process_response(req, rsp, self.spider) - assert isinstance(req, Request) - self.assertRaises( - IgnoreRequest, self.mw.process_response, req, rsp, self.spider - ) - - def test_redirect_urls(self): - req1 = Request("http://scrapytest.org/first") - rsp1 = Response( - "http://scrapytest.org/first", - headers={"Location": "/redirected"}, - status=302, - ) - req2 = self.mw.process_response(req1, rsp1, self.spider) - rsp2 = Response( - "http://scrapytest.org/redirected", - headers={"Location": "/redirected2"}, - status=302, - ) - req3 = self.mw.process_response(req2, rsp2, self.spider) - - self.assertEqual(req2.url, "http://scrapytest.org/redirected") - self.assertEqual(req2.meta["redirect_urls"], ["http://scrapytest.org/first"]) - self.assertEqual(req3.url, "http://scrapytest.org/redirected2") - self.assertEqual( - req3.meta["redirect_urls"], - ["http://scrapytest.org/first", "http://scrapytest.org/redirected"], - ) - - def test_redirect_reasons(self): - req1 = Request("http://scrapytest.org/first") - rsp1 = Response( - "http://scrapytest.org/first", - headers={"Location": "/redirected1"}, - status=301, - ) - req2 = self.mw.process_response(req1, rsp1, self.spider) - rsp2 = Response( - "http://scrapytest.org/redirected1", - headers={"Location": "/redirected2"}, - status=301, - ) - req3 = self.mw.process_response(req2, rsp2, self.spider) - - self.assertEqual(req2.meta["redirect_reasons"], [301]) - self.assertEqual(req3.meta["redirect_reasons"], [301, 301]) - def test_spider_handling(self): smartspider = self.crawler._create_spider("smarty") smartspider.handle_httpstatus_list = [404, 301, 302] @@ -247,53 +1135,84 @@ class RedirectMiddlewareTest(unittest.TestCase): perc_encoded_utf8_url = "http://scrapytest.org/a%C3%A7%C3%A3o" self.assertEqual(perc_encoded_utf8_url, req_result.url) - def test_cross_domain_header_dropping(self): - safe_headers = {"A": "B"} - original_request = Request( - "https://example.com", - headers={"Cookie": "a=b", "Authorization": "a", **safe_headers}, - ) - - internal_response = Response( - "https://example.com", - headers={"Location": "https://example.com/a"}, - status=301, - ) - internal_redirect_request = self.mw.process_response( - original_request, internal_response, self.spider - ) - self.assertIsInstance(internal_redirect_request, Request) - self.assertEqual(original_request.headers, internal_redirect_request.headers) - - external_response = Response( - "https://example.com", - headers={"Location": "https://example.org/a"}, - status=301, - ) - external_redirect_request = self.mw.process_response( - original_request, external_response, self.spider - ) - self.assertIsInstance(external_redirect_request, Request) - self.assertEqual( - safe_headers, external_redirect_request.headers.to_unicode_dict() - ) + def test_no_location(self): + request = Request("https://example.com") + response = Response(request.url, status=302) + assert self.mw.process_response(request, response, self.spider) is response -class MetaRefreshMiddlewareTest(unittest.TestCase): +SCHEME_PARAMS = ("url", "location", "target") +HTTP_SCHEMES = ("http", "https") +NON_HTTP_SCHEMES = ("data", "file", "ftp", "s3", "foo") +REDIRECT_SCHEME_CASES = ( + # http/https → http/https redirects + *( + ( + f"{input_scheme}://example.com/a", + f"{output_scheme}://example.com/b", + f"{output_scheme}://example.com/b", + ) + for input_scheme, output_scheme in product(HTTP_SCHEMES, repeat=2) + ), + # http/https → data/file/ftp/s3/foo does not redirect + *( + ( + f"{input_scheme}://example.com/a", + f"{output_scheme}://example.com/b", + None, + ) + for input_scheme in HTTP_SCHEMES + for output_scheme in NON_HTTP_SCHEMES + ), + # http/https → relative redirects + *( + ( + f"{scheme}://example.com/a", + location, + f"{scheme}://example.com/b", + ) + for scheme in HTTP_SCHEMES + for location in ("//example.com/b", "/b") + ), + # Note: We do not test data/file/ftp/s3 schemes for the initial URL + # because their download handlers cannot return a status code of 3xx. +) + + +@pytest.mark.parametrize(SCHEME_PARAMS, REDIRECT_SCHEME_CASES) +def test_redirect_schemes(url, location, target): + crawler = get_crawler(Spider) + spider = crawler._create_spider("foo") + mw = RedirectMiddleware.from_crawler(crawler) + request = Request(url) + response = Response(url, headers={"Location": location}, status=301) + redirect = mw.process_response(request, response, spider) + if target is None: + assert redirect == response + else: + assert isinstance(redirect, Request) + assert redirect.url == target + + +def meta_refresh_body(url, interval=5): + html = f"""""" + return html.encode("utf-8") + + +class MetaRefreshMiddlewareTest(Base.Test): + mwcls = MetaRefreshMiddleware + reason = "meta refresh" + def setUp(self): crawler = get_crawler(Spider) self.spider = crawler._create_spider("foo") - self.mw = MetaRefreshMiddleware.from_crawler(crawler) + self.mw = self.mwcls.from_crawler(crawler) def _body(self, interval=5, url="http://example.org/newpage"): - html = f"""""" - return html.encode("utf-8") + return meta_refresh_body(url, interval) - def test_priority_adjust(self): - req = Request("http://a.com") - rsp = HtmlResponse(req.url, body=self._body()) - req2 = self.mw.process_response(req, rsp, self.spider) - assert req2.priority > req.priority + def get_response(self, request, location): + return HtmlResponse(request.url, body=self._body(url=location)) def test_meta_refresh(self): req = Request(url="http://example.org") @@ -332,62 +1251,6 @@ class MetaRefreshMiddlewareTest(unittest.TestCase): ), "Content-Length header must not be present in redirected request" assert not req2.body, f"Redirected body must be empty, not '{req2.body}'" - def test_max_redirect_times(self): - self.mw.max_redirect_times = 1 - req = Request("http://scrapytest.org/max") - rsp = HtmlResponse(req.url, body=self._body()) - - req = self.mw.process_response(req, rsp, self.spider) - assert isinstance(req, Request) - assert "redirect_times" in req.meta - self.assertEqual(req.meta["redirect_times"], 1) - self.assertRaises( - IgnoreRequest, self.mw.process_response, req, rsp, self.spider - ) - - def test_ttl(self): - self.mw.max_redirect_times = 100 - req = Request("http://scrapytest.org/302", meta={"redirect_ttl": 1}) - rsp = HtmlResponse(req.url, body=self._body()) - - req = self.mw.process_response(req, rsp, self.spider) - assert isinstance(req, Request) - self.assertRaises( - IgnoreRequest, self.mw.process_response, req, rsp, self.spider - ) - - def test_redirect_urls(self): - req1 = Request("http://scrapytest.org/first") - rsp1 = HtmlResponse(req1.url, body=self._body(url="/redirected")) - req2 = self.mw.process_response(req1, rsp1, self.spider) - assert isinstance(req2, Request), req2 - rsp2 = HtmlResponse(req2.url, body=self._body(url="/redirected2")) - req3 = self.mw.process_response(req2, rsp2, self.spider) - assert isinstance(req3, Request), req3 - self.assertEqual(req2.url, "http://scrapytest.org/redirected") - self.assertEqual(req2.meta["redirect_urls"], ["http://scrapytest.org/first"]) - self.assertEqual(req3.url, "http://scrapytest.org/redirected2") - self.assertEqual( - req3.meta["redirect_urls"], - ["http://scrapytest.org/first", "http://scrapytest.org/redirected"], - ) - - def test_redirect_reasons(self): - req1 = Request("http://scrapytest.org/first") - rsp1 = HtmlResponse( - "http://scrapytest.org/first", body=self._body(url="/redirected") - ) - req2 = self.mw.process_response(req1, rsp1, self.spider) - rsp2 = HtmlResponse( - "http://scrapytest.org/redirected", body=self._body(url="/redirected1") - ) - req3 = self.mw.process_response(req2, rsp2, self.spider) - - self.assertEqual(req2.meta["redirect_reasons"], ["meta refresh"]) - self.assertEqual( - req3.meta["redirect_reasons"], ["meta refresh", "meta refresh"] - ) - def test_ignore_tags_default(self): req = Request(url="http://example.org") body = ( @@ -413,5 +1276,45 @@ class MetaRefreshMiddlewareTest(unittest.TestCase): assert isinstance(response, Response) +@pytest.mark.parametrize( + SCHEME_PARAMS, + ( + *REDIRECT_SCHEME_CASES, + # data/file/ftp/s3/foo → * does not redirect + *( + ( + f"{input_scheme}://example.com/a", + f"{output_scheme}://example.com/b", + None, + ) + for input_scheme in NON_HTTP_SCHEMES + for output_scheme in chain(HTTP_SCHEMES, NON_HTTP_SCHEMES) + ), + # data/file/ftp/s3/foo → relative does not redirect + *( + ( + f"{scheme}://example.com/a", + location, + None, + ) + for scheme in NON_HTTP_SCHEMES + for location in ("//example.com/b", "/b") + ), + ), +) +def test_meta_refresh_schemes(url, location, target): + crawler = get_crawler(Spider) + spider = crawler._create_spider("foo") + mw = MetaRefreshMiddleware.from_crawler(crawler) + request = Request(url) + response = HtmlResponse(url, body=meta_refresh_body(location)) + redirect = mw.process_response(request, response, spider) + if target is None: + assert redirect == response + else: + assert isinstance(redirect, Request) + assert redirect.url == target + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_engine.py b/tests/test_engine.py index 8d7afb6a1..33544e8db 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -15,8 +15,10 @@ import subprocess import sys from collections import defaultdict from dataclasses import dataclass +from logging import DEBUG from pathlib import Path from threading import Timer +from unittest.mock import Mock from urllib.parse import urlparse import attr @@ -27,11 +29,13 @@ from twisted.trial import unittest from twisted.web import server, static, util from scrapy import signals -from scrapy.core.engine import ExecutionEngine -from scrapy.exceptions import CloseSpider +from scrapy.core.engine import ExecutionEngine, Slot +from scrapy.core.scheduler import BaseScheduler +from scrapy.exceptions import CloseSpider, IgnoreRequest from scrapy.http import Request from scrapy.item import Field, Item from scrapy.linkextractors import LinkExtractor +from scrapy.signals import request_scheduled from scrapy.spiders import Spider from scrapy.utils.signal import disconnect_all from scrapy.utils.test import get_crawler @@ -467,6 +471,38 @@ class EngineTest(unittest.TestCase): self.assertNotIn(b"Traceback", stderr) +def test_request_scheduled_signal(caplog): + class TestScheduler(BaseScheduler): + def __init__(self): + self.enqueued = [] + + def enqueue_request(self, request: Request) -> bool: + self.enqueued.append(request) + return True + + def signal_handler(request: Request, spider: Spider) -> None: + if "drop" in request.url: + raise IgnoreRequest + + spider = TestSpider() + crawler = get_crawler(spider.__class__) + engine = ExecutionEngine(crawler, lambda _: None) + engine.downloader._slot_gc_loop.stop() + scheduler = TestScheduler() + engine.slot = Slot((), None, Mock(), scheduler) + crawler.signals.connect(signal_handler, request_scheduled) + keep_request = Request("https://keep.example") + engine._schedule_request(keep_request, spider) + drop_request = Request("https://drop.example") + caplog.set_level(DEBUG) + engine._schedule_request(drop_request, spider) + assert scheduler.enqueued == [ + keep_request + ], f"{scheduler.enqueued!r} != [{keep_request!r}]" + assert "dropped request " in caplog.text + crawler.signals.disconnect(signal_handler, request_scheduled) + + if __name__ == "__main__": if len(sys.argv) > 1 and sys.argv[1] == "runserver": start_test_site(debug=True) diff --git a/tests/test_utils_project.py b/tests/test_utils_project.py index 90bd350a5..3831f4c21 100644 --- a/tests/test_utils_project.py +++ b/tests/test_utils_project.py @@ -6,6 +6,7 @@ import unittest import warnings from pathlib import Path +from scrapy.utils.misc import set_environ from scrapy.utils.project import data_path, get_project_settings @@ -38,20 +39,6 @@ class ProjectUtilsTest(unittest.TestCase): self.assertEqual(abspath, data_path(abspath)) -@contextlib.contextmanager -def set_env(**update): - modified = set(update.keys()) & set(os.environ.keys()) - update_after = {k: os.environ[k] for k in modified} - remove_after = frozenset(k for k in update if k not in os.environ) - try: - os.environ.update(update) - yield - finally: - os.environ.update(update_after) - for k in remove_after: - os.environ.pop(k) - - class GetProjectSettingsTestCase(unittest.TestCase): def test_valid_envvar(self): value = "tests.test_cmdline.settings" @@ -60,7 +47,7 @@ class GetProjectSettingsTestCase(unittest.TestCase): } with warnings.catch_warnings(): warnings.simplefilter("error") - with set_env(**envvars): + with set_environ(**envvars): settings = get_project_settings() assert settings.get("SETTINGS_MODULE") == value @@ -69,7 +56,7 @@ class GetProjectSettingsTestCase(unittest.TestCase): envvars = { "SCRAPY_FOO": "bar", } - with set_env(**envvars): + with set_environ(**envvars): settings = get_project_settings() assert settings.get("SCRAPY_FOO") is None @@ -80,7 +67,7 @@ class GetProjectSettingsTestCase(unittest.TestCase): "SCRAPY_FOO": "bar", "SCRAPY_SETTINGS_MODULE": value, } - with set_env(**envvars): + with set_environ(**envvars): settings = get_project_settings() assert settings.get("SETTINGS_MODULE") == value assert settings.get("SCRAPY_FOO") is None diff --git a/tox.ini b/tox.ini index ede139756..cde4243f3 100644 --- a/tox.ini +++ b/tox.ini @@ -26,6 +26,9 @@ deps = # mitmproxy does not support PyPy mitmproxy; implementation_name != 'pypy' + # https://github.com/pallets/werkzeug/pull/2768 breaks flask, required by + # mitmproxy. + werkzeug < 3; python_version < '3.9' and implementation_name != 'pypy' passenv = S3_TEST_FILE_URI AWS_ACCESS_KEY_ID @@ -90,6 +93,7 @@ commands = twine check dist/* [pinned] +basepython = python3.8 deps = cryptography==36.0.0 cssselect==0.9.1 @@ -116,7 +120,7 @@ commands = pytest --cov=scrapy --cov-report=xml --cov-report= {posargs:--durations=10 scrapy tests} [testenv:pinned] -basepython = python3.8 +basepython = {[pinned]basepython} deps = {[pinned]deps} PyDispatcher==2.0.5 @@ -126,7 +130,7 @@ setenv = commands = {[pinned]commands} [testenv:windows-pinned] -basepython = python3 +basepython = {[pinned]basepython} deps = {[pinned]deps} PyDispatcher==2.0.5 @@ -155,7 +159,7 @@ deps = ipython [testenv:extra-deps-pinned] -basepython = python3.8 +basepython = {[pinned]basepython} deps = {[pinned]deps} boto3==1.20.0 @@ -179,6 +183,7 @@ commands = {[testenv]commands} --reactor=asyncio [testenv:asyncio-pinned] +basepython = {[pinned]basepython} deps = {[testenv:pinned]deps} commands = {[pinned]commands} --reactor=asyncio install_command = {[pinned]install_command} @@ -191,12 +196,12 @@ commands = pytest {posargs:--durations=10 docs scrapy tests} [testenv:pypy3-pinned] -basepython = {[testenv:pypy3]basepython} +basepython = pypy3.8 deps = {[pinned]deps} PyPyDispatcher==2.1.0 commands = - pytest --durations=10 scrapy tests + pytest {posargs:--durations=10 scrapy tests} install_command = {[pinned]install_command} setenv = {[pinned]setenv} @@ -244,7 +249,7 @@ commands = pytest --cov=scrapy --cov-report=xml --cov-report= {posargs:tests -k s3} [testenv:botocore-pinned] -basepython = python3.8 +basepython = {[pinned]basepython} deps = {[pinned]deps} botocore==1.4.87 From 812fd2368f705d033f5f39c152130b12a0fe9b1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 15 May 2024 11:48:43 +0200 Subject: [PATCH 02/21] Allow user-defined secure cookies (#6357) --- docs/topics/request-response.rst | 3 +- scrapy/downloadermiddlewares/cookies.py | 14 ++- scrapy/utils/_compression.py | 9 +- tests/test_downloadermiddleware_cookies.py | 111 ++++++++++++++++++++- 4 files changed, 126 insertions(+), 11 deletions(-) diff --git a/docs/topics/request-response.rst b/docs/topics/request-response.rst index eb70ebce8..3c2843bc1 100644 --- a/docs/topics/request-response.rst +++ b/docs/topics/request-response.rst @@ -94,13 +94,14 @@ Request objects .. code-block:: python request_with_cookies = Request( - url="http://www.example.com", + url="https://www.example.com", cookies=[ { "name": "currency", "value": "USD", "domain": "example.com", "path": "/currency", + "secure": True, }, ], ) diff --git a/scrapy/downloadermiddlewares/cookies.py b/scrapy/downloadermiddlewares/cookies.py index 85781efd6..6ada3b474 100644 --- a/scrapy/downloadermiddlewares/cookies.py +++ b/scrapy/downloadermiddlewares/cookies.py @@ -33,6 +33,7 @@ logger = logging.getLogger(__name__) _split_domain = TLDExtract(include_psl_private_domains=True) +_UNSET = object() def _is_public_domain(domain: str) -> bool: @@ -133,6 +134,7 @@ class CookiesMiddleware: Decode from bytes if necessary. """ decoded = {} + flags = set() for key in ("name", "value", "path", "domain"): if cookie.get(key) is None: if key in ("name", "value"): @@ -152,10 +154,16 @@ class CookiesMiddleware: cookie, ) decoded[key] = cookie[key].decode("latin1", errors="replace") - + for flag in ("secure",): + value = cookie.get(flag, _UNSET) + if value is _UNSET or not value: + continue + flags.add(flag) cookie_str = f"{decoded.pop('name')}={decoded.pop('value')}" for key, value in decoded.items(): # path, domain cookie_str += f"; {key.capitalize()}={value}" + for flag in flags: # secure + cookie_str += f"; {flag.capitalize()}" return cookie_str def _get_request_cookies( @@ -168,9 +176,11 @@ class CookiesMiddleware: return [] cookies: Iterable[Dict[str, Any]] if isinstance(request.cookies, dict): - cookies = ({"name": k, "value": v} for k, v in request.cookies.items()) + cookies = tuple({"name": k, "value": v} for k, v in request.cookies.items()) else: 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)) response = Response(request.url, headers={"Set-Cookie": formatted}) return jar.make_cookies(response, request) diff --git a/scrapy/utils/_compression.py b/scrapy/utils/_compression.py index 84c255c28..591737b8e 100644 --- a/scrapy/utils/_compression.py +++ b/scrapy/utils/_compression.py @@ -20,11 +20,10 @@ else: "You have brotlipy installed, and Scrapy will use it, but " "Scrapy support for brotlipy is deprecated and will stop " "working in a future version of Scrapy. brotlipy itself is " - "deprecated, it has been superseded by brotlicffi. " - "Please, uninstall brotlipy " - "and install brotli or brotlicffi instead. brotlipy has the same import " - "name as brotli, so keeping both installed is strongly " - "discouraged." + "deprecated, it has been superseded by brotlicffi. Please, " + "uninstall brotlipy and install brotli or brotlicffi instead. " + "brotlipy has the same import name as brotli, so keeping both " + "installed is strongly discouraged." ), ScrapyDeprecationWarning, ) diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 425fabcc7..1f7e6615c 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -14,6 +14,8 @@ from scrapy.spiders import Spider from scrapy.utils.python import to_bytes from scrapy.utils.test import get_crawler +UNSET = object() + def _cookie_to_set_cookie_value(cookie): """Given a cookie defined as a dictionary with name and value keys, and @@ -414,19 +416,19 @@ class CookiesMiddlewareTest(TestCase): "scrapy.downloadermiddlewares.cookies", "WARNING", "Invalid cookie found in request :" - " {'value': 'bar'} ('name' is missing)", + " {'value': 'bar', 'secure': False} ('name' is missing)", ), ( "scrapy.downloadermiddlewares.cookies", "WARNING", "Invalid cookie found in request :" - " {'name': 'foo'} ('value' is missing)", + " {'name': 'foo', 'secure': False} ('value' is missing)", ), ( "scrapy.downloadermiddlewares.cookies", "WARNING", "Invalid cookie found in request :" - " {'name': 'foo', 'value': None} ('value' is missing)", + " {'name': 'foo', 'value': None, 'secure': False} ('value' is missing)", ), ) self.assertCookieValEqual(req1.headers["Cookie"], "key=value1") @@ -732,3 +734,106 @@ class CookiesMiddlewareTest(TestCase): "co.uk", cookies=True, ) + + def _test_cookie_redirect_scheme_change( + self, secure, from_scheme, to_scheme, cookies1, cookies2, cookies3 + ): + """When a redirect causes the URL scheme to change from *from_scheme* + to *to_scheme*, while domain and port remain the same, and given a + cookie on the initial request with its secure attribute set to + *secure*, check if the cookie should be set on the Cookie header of the + initial request (*cookies1*), if it should be kept by the redirect + middleware (*cookies2*), and if it should be present on the Cookie + header in the redirected request (*cookie3*).""" + cookie_kwargs = {} + if secure is not UNSET: + cookie_kwargs["secure"] = secure + input_cookies = [{"name": "a", "value": "b", **cookie_kwargs}] + + request1 = Request(f"{from_scheme}://a.example", cookies=input_cookies) + self.mw.process_request(request1, self.spider) + cookies = request1.headers.get("Cookie") + self.assertEqual(cookies, b"a=b" if cookies1 else None) + + response = Response( + f"{from_scheme}://a.example", + headers={"Location": f"{to_scheme}://a.example"}, + status=301, + ) + self.assertEqual( + self.mw.process_response(request1, response, self.spider), + response, + ) + + request2 = self.redirect_middleware.process_response( + request1, + response, + self.spider, + ) + self.assertIsInstance(request2, Request) + cookies = request2.headers.get("Cookie") + self.assertEqual(cookies, b"a=b" if cookies2 else None) + + self.mw.process_request(request2, self.spider) + cookies = request2.headers.get("Cookie") + self.assertEqual(cookies, b"a=b" if cookies3 else None) + + def test_cookie_redirect_secure_undefined_downgrade(self): + self._test_cookie_redirect_scheme_change( + secure=UNSET, + from_scheme="https", + to_scheme="http", + cookies1=True, + cookies2=True, # xfail, due to a bug in the redirect middleware fixed elsewhere + cookies3=False, + ) + + def test_cookie_redirect_secure_undefined_upgrade(self): + self._test_cookie_redirect_scheme_change( + secure=UNSET, + from_scheme="http", + to_scheme="https", + cookies1=True, + cookies2=True, + cookies3=True, + ) + + def test_cookie_redirect_secure_false_downgrade(self): + self._test_cookie_redirect_scheme_change( + secure=False, + from_scheme="https", + to_scheme="http", + cookies1=True, + cookies2=True, # xfail, due to a bug in the redirect middleware fixed elsewhere + cookies3=True, + ) + + def test_cookie_redirect_secure_false_upgrade(self): + self._test_cookie_redirect_scheme_change( + secure=False, + from_scheme="http", + to_scheme="https", + cookies1=True, + cookies2=True, + cookies3=True, + ) + + def test_cookie_redirect_secure_true_downgrade(self): + self._test_cookie_redirect_scheme_change( + secure=True, + from_scheme="https", + to_scheme="http", + cookies1=True, + cookies2=True, # xfail, due to a bug in the redirect middleware fixed elsewhere + cookies3=False, + ) + + def test_cookie_redirect_secure_true_upgrade(self): + self._test_cookie_redirect_scheme_change( + secure=True, + from_scheme="http", + to_scheme="https", + cookies1=False, + cookies2=False, + cookies3=True, + ) From 631fc65fadb874629787ae5f7fdd876b9ec96a29 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Thu, 16 May 2024 18:42:09 +0400 Subject: [PATCH 03/21] Update expectations of cookies after redirects. (#6367) --- tests/test_downloadermiddleware_cookies.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 1f7e6615c..5eccd396a 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -784,7 +784,7 @@ class CookiesMiddlewareTest(TestCase): from_scheme="https", to_scheme="http", cookies1=True, - cookies2=True, # xfail, due to a bug in the redirect middleware fixed elsewhere + cookies2=False, cookies3=False, ) @@ -804,7 +804,7 @@ class CookiesMiddlewareTest(TestCase): from_scheme="https", to_scheme="http", cookies1=True, - cookies2=True, # xfail, due to a bug in the redirect middleware fixed elsewhere + cookies2=False, cookies3=True, ) @@ -824,7 +824,7 @@ class CookiesMiddlewareTest(TestCase): from_scheme="https", to_scheme="http", cookies1=True, - cookies2=True, # xfail, due to a bug in the redirect middleware fixed elsewhere + cookies2=False, cookies3=False, ) From b99526b740890e63f1d05074b3e358a9ae59b77f Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Sun, 19 May 2024 15:45:51 +0500 Subject: [PATCH 04/21] Full typing for scrapy/contracts. --- scrapy/contracts/__init__.py | 75 ++++++++++++++++++++++++------------ scrapy/contracts/default.py | 19 ++++----- 2 files changed, 60 insertions(+), 34 deletions(-) diff --git a/scrapy/contracts/__init__.py b/scrapy/contracts/__init__.py index d46eb7c51..b300b8457 100644 --- a/scrapy/contracts/__init__.py +++ b/scrapy/contracts/__init__.py @@ -3,10 +3,23 @@ import sys from functools import wraps from inspect import getmembers from types import CoroutineType -from typing import AsyncGenerator, Dict, Optional, Type -from unittest import TestCase +from typing import ( + Any, + AsyncGenerator, + Callable, + Dict, + Iterable, + List, + Optional, + Tuple, + Type, +) +from unittest import TestCase, TestResult -from scrapy.http import Request +from twisted.python.failure import Failure + +from scrapy import Spider +from scrapy.http import Request, Response from scrapy.utils.python import get_spec from scrapy.utils.spider import iterate_spider_output @@ -15,18 +28,20 @@ class Contract: """Abstract class for contracts""" request_cls: Optional[Type[Request]] = None + name: str - def __init__(self, method, *args): + def __init__(self, method: Callable, *args: Any): self.testcase_pre = _create_testcase(method, f"@{self.name} pre-hook") self.testcase_post = _create_testcase(method, f"@{self.name} post-hook") - self.args = args + self.args: Tuple[Any, ...] = args - def add_pre_hook(self, request, results): + def add_pre_hook(self, request: Request, results: TestResult) -> Request: if hasattr(self, "pre_process"): cb = request.callback + assert cb is not None @wraps(cb) - def wrapper(response, **cb_kwargs): + def wrapper(response: Response, **cb_kwargs: Any) -> List[Any]: try: results.startTest(self.testcase_pre) self.pre_process(response) @@ -49,12 +64,13 @@ class Contract: return request - def add_post_hook(self, request, results): + def add_post_hook(self, request: Request, results: TestResult) -> Request: if hasattr(self, "post_process"): cb = request.callback + assert cb is not None @wraps(cb) - def wrapper(response, **cb_kwargs): + def wrapper(response: Response, **cb_kwargs: Any) -> List[Any]: cb_result = cb(response, **cb_kwargs) if isinstance(cb_result, (AsyncGenerator, CoroutineType)): raise TypeError("Contracts don't support async callbacks") @@ -76,18 +92,18 @@ class Contract: return request - def adjust_request_args(self, args): + def adjust_request_args(self, args: Dict[str, Any]) -> Dict[str, Any]: return args class ContractsManager: - contracts: Dict[str, Contract] = {} + contracts: Dict[str, Type[Contract]] = {} - def __init__(self, contracts): + def __init__(self, contracts: Iterable[Type[Contract]]): for contract in contracts: self.contracts[contract.name] = contract - def tested_methods_from_spidercls(self, spidercls): + def tested_methods_from_spidercls(self, spidercls: Type[Spider]) -> List[str]: is_method = re.compile(r"^\s*@", re.MULTILINE).search methods = [] for key, value in getmembers(spidercls): @@ -96,21 +112,26 @@ class ContractsManager: return methods - def extract_contracts(self, method): - contracts = [] + def extract_contracts(self, method: Callable) -> List[Contract]: + contracts: List[Contract] = [] + assert method.__doc__ is not None for line in method.__doc__.split("\n"): line = line.strip() if line.startswith("@"): - name, args = re.match(r"@(\w+)\s*(.*)", line).groups() + m = re.match(r"@(\w+)\s*(.*)", line) + assert m is not None + name, args = m.groups() args = re.split(r"\s+", args) contracts.append(self.contracts[name](method, *args)) return contracts - def from_spider(self, spider, results): - requests = [] + def from_spider( + self, spider: Spider, results: TestResult + ) -> List[Optional[Request]]: + requests: List[Optional[Request]] = [] for method in self.tested_methods_from_spidercls(type(spider)): bound_method = spider.__getattribute__(method) try: @@ -121,7 +142,7 @@ class ContractsManager: return requests - def from_method(self, method, results): + def from_method(self, method: Callable, results: TestResult) -> Optional[Request]: contracts = self.extract_contracts(method) if contracts: request_cls = Request @@ -154,14 +175,18 @@ class ContractsManager: self._clean_req(request, method, results) return request + return None - def _clean_req(self, request, method, results): + def _clean_req( + self, request: Request, method: Callable, results: TestResult + ) -> None: """stop the request from returning objects and records any errors""" cb = request.callback + assert cb is not None @wraps(cb) - def cb_wrapper(response, **cb_kwargs): + def cb_wrapper(response: Response, **cb_kwargs: Any) -> None: try: output = cb(response, **cb_kwargs) output = list(iterate_spider_output(output)) @@ -169,7 +194,7 @@ class ContractsManager: case = _create_testcase(method, "callback") results.addError(case, sys.exc_info()) - def eb_wrapper(failure): + def eb_wrapper(failure: Failure) -> None: case = _create_testcase(method, "errback") exc_info = failure.type, failure.value, failure.getTracebackObject() results.addError(case, exc_info) @@ -178,11 +203,11 @@ class ContractsManager: request.errback = eb_wrapper -def _create_testcase(method, desc): - spider = method.__self__.name +def _create_testcase(method: Callable, desc: str) -> TestCase: + spider = method.__self__.name # type: ignore[attr-defined] class ContractTestCase(TestCase): - def __str__(_self): + def __str__(_self) -> str: return f"[{spider}] {method.__name__} ({desc})" name = f"{spider}_{method.__name__}" diff --git a/scrapy/contracts/default.py b/scrapy/contracts/default.py index eac702cef..71ca4168a 100644 --- a/scrapy/contracts/default.py +++ b/scrapy/contracts/default.py @@ -1,4 +1,5 @@ import json +from typing import Any, Callable, Dict, List, Optional from itemadapter import ItemAdapter, is_item @@ -15,7 +16,7 @@ class UrlContract(Contract): name = "url" - def adjust_request_args(self, args): + def adjust_request_args(self, args: Dict[str, Any]) -> Dict[str, Any]: args["url"] = self.args[0] return args @@ -29,7 +30,7 @@ class CallbackKeywordArgumentsContract(Contract): name = "cb_kwargs" - def adjust_request_args(self, args): + def adjust_request_args(self, args: Dict[str, Any]) -> Dict[str, Any]: args["cb_kwargs"] = json.loads(" ".join(self.args)) return args @@ -48,14 +49,14 @@ class ReturnsContract(Contract): """ name = "returns" - object_type_verifiers = { + object_type_verifiers: Dict[Optional[str], Callable[[Any], bool]] = { "request": lambda x: isinstance(x, Request), "requests": lambda x: isinstance(x, Request), "item": is_item, "items": is_item, } - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any): super().__init__(*args, **kwargs) if len(self.args) not in [1, 2, 3]: @@ -66,16 +67,16 @@ class ReturnsContract(Contract): self.obj_type_verifier = self.object_type_verifiers[self.obj_name] try: - self.min_bound = int(self.args[1]) + self.min_bound: float = int(self.args[1]) except IndexError: self.min_bound = 1 try: - self.max_bound = int(self.args[2]) + self.max_bound: float = int(self.args[2]) except IndexError: self.max_bound = float("inf") - def post_process(self, output): + def post_process(self, output: List[Any]) -> None: occurrences = 0 for x in output: if self.obj_type_verifier(x): @@ -85,7 +86,7 @@ class ReturnsContract(Contract): if not assertion: if self.min_bound == self.max_bound: - expected = self.min_bound + expected = str(self.min_bound) else: expected = f"{self.min_bound}..{self.max_bound}" @@ -101,7 +102,7 @@ class ScrapesContract(Contract): name = "scrapes" - def post_process(self, output): + def post_process(self, output: List[Any]) -> None: for x in output: if is_item(x): missing = [arg for arg in self.args if arg not in ItemAdapter(x)] From e676cd3ce0d488f56498b766912725066bcee4d9 Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Wed, 22 May 2024 07:55:53 -0300 Subject: [PATCH 05/21] docs: Remove top-level reactor imports from CrawlerProces/CrawlerRunner examples --- docs/topics/practices.rst | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index cd359b147..7731180fe 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -92,7 +92,6 @@ reactor after ``MySpider`` has finished running. .. code-block:: python - from twisted.internet import reactor import scrapy from scrapy.crawler import CrawlerRunner from scrapy.utils.log import configure_logging @@ -107,6 +106,33 @@ reactor after ``MySpider`` has finished running. runner = CrawlerRunner() d = runner.crawl(MySpider) + from twisted.internet import reactor + + d.addBoth(lambda _: reactor.stop()) + reactor.run() # the script will block here until the crawling is finished + +Same example but using a non-default reactor, is only necessary call ``install_reactor`` if you are using ``CrawlerRunner`` since ``CrawlerProcess`` already does this automatically. + +.. code-block:: python + + import scrapy + from scrapy.crawler import CrawlerRunner + from scrapy.utils.log import configure_logging + + + class MySpider(scrapy.Spider): + # Your spider definition + ... + + + configure_logging({"LOG_FORMAT": "%(levelname)s: %(message)s"}) + from scrapy.utils.reactor import install_reactor + + install_reactor("twisted.internet.asyncioreactor.AsyncioSelectorReactor") + runner = CrawlerRunner() + d = runner.crawl(MySpider) + from twisted.internet import reactor + d.addBoth(lambda _: reactor.stop()) reactor.run() # the script will block here until the crawling is finished @@ -151,7 +177,6 @@ Same example using :class:`~scrapy.crawler.CrawlerRunner`: .. code-block:: python import scrapy - from twisted.internet import reactor from scrapy.crawler import CrawlerRunner from scrapy.utils.log import configure_logging from scrapy.utils.project import get_project_settings @@ -173,6 +198,8 @@ Same example using :class:`~scrapy.crawler.CrawlerRunner`: runner.crawl(MySpider1) runner.crawl(MySpider2) d = runner.join() + from twisted.internet import reactor + d.addBoth(lambda _: reactor.stop()) reactor.run() # the script will block here until all crawling jobs are finished @@ -181,7 +208,7 @@ Same example but running the spiders sequentially by chaining the deferreds: .. code-block:: python - from twisted.internet import reactor, defer + from twisted.internet import defer from scrapy.crawler import CrawlerRunner from scrapy.utils.log import configure_logging from scrapy.utils.project import get_project_settings @@ -209,6 +236,8 @@ Same example but running the spiders sequentially by chaining the deferreds: reactor.stop() + from twisted.internet import reactor + crawl() reactor.run() # the script will block here until the last crawl call is finished From 8210fae25a9d812447df617155001b9861e0d834 Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Wed, 22 May 2024 18:50:50 -0300 Subject: [PATCH 06/21] Update docs/topics/practices.rst Co-authored-by: Andrey Rakhmatullin --- docs/topics/practices.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index 7731180fe..710be7aa2 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -111,7 +111,9 @@ reactor after ``MySpider`` has finished running. d.addBoth(lambda _: reactor.stop()) reactor.run() # the script will block here until the crawling is finished -Same example but using a non-default reactor, is only necessary call ``install_reactor`` if you are using ``CrawlerRunner`` since ``CrawlerProcess`` already does this automatically. +Same example but using a non-default reactor, it's only necessary call +``install_reactor`` if you are using ``CrawlerRunner`` since ``CrawlerProcess`` + already does this automatically. .. code-block:: python From dc6a495fee41949d50178b9e46d6f41e83425ca2 Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Wed, 22 May 2024 18:51:02 -0300 Subject: [PATCH 07/21] Update docs/topics/practices.rst Co-authored-by: Andrey Rakhmatullin --- docs/topics/practices.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index 710be7aa2..cec098012 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -106,6 +106,7 @@ reactor after ``MySpider`` has finished running. runner = CrawlerRunner() d = runner.crawl(MySpider) + from twisted.internet import reactor d.addBoth(lambda _: reactor.stop()) From 3f66b66e3f645393dbb263a1ec7ab04bdabd74b4 Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Wed, 22 May 2024 22:01:55 -0300 Subject: [PATCH 08/21] fix: checks --- docs/topics/practices.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index cec098012..aa81ceea5 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -113,8 +113,7 @@ reactor after ``MySpider`` has finished running. reactor.run() # the script will block here until the crawling is finished Same example but using a non-default reactor, it's only necessary call -``install_reactor`` if you are using ``CrawlerRunner`` since ``CrawlerProcess`` - already does this automatically. +``install_reactor`` if you are using ``CrawlerRunner`` since ``CrawlerProcess`` already does this automatically. .. code-block:: python From e143dc795228424fa98cb40e17b9993617ae61ae Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Wed, 22 May 2024 22:26:31 -0300 Subject: [PATCH 09/21] Update tests-macos.yml --- .github/workflows/tests-macos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests-macos.yml b/.github/workflows/tests-macos.yml index 252176464..95016146e 100644 --- a/.github/workflows/tests-macos.yml +++ b/.github/workflows/tests-macos.yml @@ -1,4 +1,4 @@ -name: macOS +name: macOS. on: [push, pull_request] concurrency: From 9d5a0d287b69a69fe34cbe3130438fb36f1f3441 Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Wed, 22 May 2024 22:27:07 -0300 Subject: [PATCH 10/21] Retrigger CI --- .github/workflows/tests-macos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests-macos.yml b/.github/workflows/tests-macos.yml index 95016146e..252176464 100644 --- a/.github/workflows/tests-macos.yml +++ b/.github/workflows/tests-macos.yml @@ -1,4 +1,4 @@ -name: macOS. +name: macOS on: [push, pull_request] concurrency: From 17e623cf0cfb5c695c43ceb069026d44cb28ca21 Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Thu, 23 May 2024 07:00:24 -0300 Subject: [PATCH 11/21] Update docs/topics/practices.rst Co-authored-by: Andrey Rakhmatullin --- docs/topics/practices.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index aa81ceea5..64b3b6e81 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -128,6 +128,7 @@ Same example but using a non-default reactor, it's only necessary call configure_logging({"LOG_FORMAT": "%(levelname)s: %(message)s"}) + from scrapy.utils.reactor import install_reactor install_reactor("twisted.internet.asyncioreactor.AsyncioSelectorReactor") From 8ec67ca230a69effb2e0442fb2a6c06cd6c92adf Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Thu, 23 May 2024 07:00:35 -0300 Subject: [PATCH 12/21] Update docs/topics/practices.rst Co-authored-by: Andrey Rakhmatullin --- docs/topics/practices.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index 64b3b6e81..ee484e63f 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -134,6 +134,7 @@ Same example but using a non-default reactor, it's only necessary call install_reactor("twisted.internet.asyncioreactor.AsyncioSelectorReactor") runner = CrawlerRunner() d = runner.crawl(MySpider) + from twisted.internet import reactor d.addBoth(lambda _: reactor.stop()) From 62c89aaf056687091235bb846ac565f7c801c359 Mon Sep 17 00:00:00 2001 From: Laerte Pereira <5853172+Laerte@users.noreply.github.com> Date: Thu, 23 May 2024 07:00:45 -0300 Subject: [PATCH 13/21] Update docs/topics/practices.rst Co-authored-by: Andrey Rakhmatullin --- docs/topics/practices.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/topics/practices.rst b/docs/topics/practices.rst index ee484e63f..1500011e7 100644 --- a/docs/topics/practices.rst +++ b/docs/topics/practices.rst @@ -202,6 +202,7 @@ Same example using :class:`~scrapy.crawler.CrawlerRunner`: runner.crawl(MySpider1) runner.crawl(MySpider2) d = runner.join() + from twisted.internet import reactor d.addBoth(lambda _: reactor.stop()) From 2facdd4fb08ec3edaf1752047dd86d5b565621a1 Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Sun, 26 May 2024 19:55:54 -0300 Subject: [PATCH 14/21] Add change reactor test to CrawlerRunner --- .flake8 | 1 + scrapy/crawler.py | 2 ++ tests/CrawlerRunner/change_reactor.py | 31 +++++++++++++++++++++++++++ tests/test_crawler.py | 8 +++++++ 4 files changed, 42 insertions(+) create mode 100644 tests/CrawlerRunner/change_reactor.py diff --git a/.flake8 b/.flake8 index 62ccad9cf..0e43b9b56 100644 --- a/.flake8 +++ b/.flake8 @@ -9,6 +9,7 @@ exclude = per-file-ignores = # Exclude files that are meant to provide top-level imports # E402: Module level import not at top of file + tests/CrawlerRunner/change_reactor.py:E402 # F401: Module imported but unused scrapy/__init__.py:E402 scrapy/core/downloader/handlers/http.py:F401 diff --git a/scrapy/crawler.py b/scrapy/crawler.py index ccfe78891..4fe5987a7 100644 --- a/scrapy/crawler.py +++ b/scrapy/crawler.py @@ -129,6 +129,8 @@ class Crawler: if is_asyncio_reactor_installed() and event_loop: verify_installed_asyncio_event_loop(event_loop) + log_reactor_info() + self.extensions = ExtensionManager.from_crawler(self) self.settings.freeze() diff --git a/tests/CrawlerRunner/change_reactor.py b/tests/CrawlerRunner/change_reactor.py new file mode 100644 index 000000000..b20aa0c7c --- /dev/null +++ b/tests/CrawlerRunner/change_reactor.py @@ -0,0 +1,31 @@ +from scrapy import Spider +from scrapy.crawler import CrawlerRunner +from scrapy.utils.log import configure_logging + + +class NoRequestsSpider(Spider): + name = "no_request" + + custom_settings = { + "TWISTED_REACTOR": "twisted.internet.asyncioreactor.AsyncioSelectorReactor", + } + + def start_requests(self): + return [] + + +configure_logging({"LOG_FORMAT": "%(levelname)s: %(message)s", "LOG_LEVEL": "DEBUG"}) + + +from scrapy.utils.reactor import install_reactor + +install_reactor("twisted.internet.asyncioreactor.AsyncioSelectorReactor") + +runner = CrawlerRunner() + +d = runner.crawl(NoRequestsSpider) + +from twisted.internet import reactor + +d.addBoth(callback=lambda _: reactor.stop()) +reactor.run() diff --git a/tests/test_crawler.py b/tests/test_crawler.py index 989208694..791ea1faa 100644 --- a/tests/test_crawler.py +++ b/tests/test_crawler.py @@ -926,3 +926,11 @@ class CrawlerRunnerSubprocess(ScriptRunnerMixin, unittest.TestCase): self.assertIn("INFO: Host: not.a.real.domain", log) self.assertIn("INFO: Type: ", log) self.assertIn("INFO: IP address: 127.0.0.1", log) + + def test_change_default_reactor(self): + log = self.run_script("change_reactor.py") + self.assertIn( + "DEBUG: Using reactor: twisted.internet.asyncioreactor.AsyncioSelectorReactor", + log, + ) + self.assertIn("DEBUG: Using asyncio event loop", log) From 6cd085785028d97393f26e6fee22e6c03e5c90a8 Mon Sep 17 00:00:00 2001 From: Laerte Pereira Date: Sun, 26 May 2024 19:57:16 -0300 Subject: [PATCH 15/21] Move path --- .flake8 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.flake8 b/.flake8 index 0e43b9b56..cf1a96476 100644 --- a/.flake8 +++ b/.flake8 @@ -9,7 +9,6 @@ exclude = per-file-ignores = # Exclude files that are meant to provide top-level imports # E402: Module level import not at top of file - tests/CrawlerRunner/change_reactor.py:E402 # F401: Module imported but unused scrapy/__init__.py:E402 scrapy/core/downloader/handlers/http.py:F401 @@ -17,6 +16,7 @@ per-file-ignores = scrapy/linkextractors/__init__.py:E402,F401 scrapy/selector/__init__.py:F401 scrapy/spiders/__init__.py:E402,F401 + tests/CrawlerRunner/change_reactor.py:E402 # Issues pending a review: scrapy/utils/url.py:F403,F405 From 9ba4dd311dd9d2a5341ee9c0c6d1b50eb44ac406 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 28 May 2024 12:27:49 +0400 Subject: [PATCH 16/21] Install typing stubs for boto3 and botocore. (#6370) --- scrapy/core/downloader/handlers/s3.py | 3 ++- scrapy/extensions/feedexport.py | 11 +++++++---- tox.ini | 13 ++++++++----- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/scrapy/core/downloader/handlers/s3.py b/scrapy/core/downloader/handlers/s3.py index 9a0811a50..1a3d36f45 100644 --- a/scrapy/core/downloader/handlers/s3.py +++ b/scrapy/core/downloader/handlers/s3.py @@ -59,7 +59,8 @@ class S3DownloadHandler: assert aws_access_key_id is not None assert aws_secret_access_key is not None SignerCls = botocore.auth.AUTH_TYPE_MAPS["s3"] - self._signer = SignerCls( + # botocore.auth.BaseSigner doesn't have an __init__() with args, only subclasses do + self._signer = SignerCls( # type: ignore[call-arg] botocore.credentials.Credentials( aws_access_key_id, aws_secret_access_key, aws_session_token ) diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py index 97f39afe7..3c2bb5593 100644 --- a/scrapy/extensions/feedexport.py +++ b/scrapy/extensions/feedexport.py @@ -238,13 +238,16 @@ class S3FeedStorage(BlockingFeedStorage): self.acl: Optional[str] = acl self.endpoint_url: Optional[str] = endpoint_url self.region_name: Optional[str] = region_name + # It can be either botocore.client.BaseClient or mypy_boto3_s3.S3Client, + # there seems to be no good way to infer it statically. + self.s3_client: Any if IS_BOTO3_AVAILABLE: import boto3.session - session = boto3.session.Session() + boto3_session = boto3.session.Session() - self.s3_client = session.client( + self.s3_client = boto3_session.client( "s3", aws_access_key_id=self.access_key, aws_secret_access_key=self.secret_key, @@ -261,9 +264,9 @@ class S3FeedStorage(BlockingFeedStorage): import botocore.session - session = botocore.session.get_session() + botocore_session = botocore.session.get_session() - self.s3_client = session.create_client( + self.s3_client = botocore_session.create_client( "s3", aws_access_key_id=self.access_key, aws_secret_access_key=self.secret_key, diff --git a/tox.ini b/tox.ini index cde4243f3..5a5e80496 100644 --- a/tox.ini +++ b/tox.ini @@ -48,12 +48,15 @@ basepython = python3 deps = mypy==1.10.0 typing-extensions==4.11.0 - types-attrs==19.1.0 types-lxml==2024.4.14 - types-Pillow==10.2.0.20240423 - types-Pygments==2.17.0.20240310 - types-pyOpenSSL==24.0.0.20240417 - types-setuptools==69.5.0.20240423 + types-Pygments==2.18.0.20240506 + types-pyOpenSSL==24.1.0.20240425 + types-setuptools==69.5.0.20240518 + botocore-stubs==1.34.94 + boto3-stubs[s3]==1.34.108 + attrs >= 18.2.0 + Pillow >= 10.3.0 + pytest >= 8.2.0 # 2.1.2 fixes a typing bug: https://github.com/scrapy/w3lib/pull/211 w3lib >= 2.1.2 commands = From 986d1ee1dd5b2efba0f787af8ee510450b4af3b4 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Tue, 28 May 2024 12:37:19 +0400 Subject: [PATCH 17/21] Move CI from the decommissioned macos-11 to macos-latest. (#6372) --- .github/workflows/tests-macos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests-macos.yml b/.github/workflows/tests-macos.yml index 252176464..a297f494c 100644 --- a/.github/workflows/tests-macos.yml +++ b/.github/workflows/tests-macos.yml @@ -7,7 +7,7 @@ concurrency: jobs: tests: - runs-on: macos-11 + runs-on: macos-latest strategy: fail-fast: false matrix: From cadb0dd707fc54670cb0eab06f0af65dcaa99354 Mon Sep 17 00:00:00 2001 From: Sanchay Kumar <51812506+kumar-sanchay@users.noreply.github.com> Date: Tue, 28 May 2024 14:12:58 +0530 Subject: [PATCH 18/21] Fix overridable methods in MediaPipeline (#6368) --- scrapy/pipelines/media.py | 26 +++--- tests/test_pipeline_media.py | 173 ++++++++++++----------------------- 2 files changed, 73 insertions(+), 126 deletions(-) diff --git a/scrapy/pipelines/media.py b/scrapy/pipelines/media.py index 5f6c5cb07..25e00b0ea 100644 --- a/scrapy/pipelines/media.py +++ b/scrapy/pipelines/media.py @@ -2,6 +2,7 @@ from __future__ import annotations import functools import logging +from abc import ABC, abstractmethod from collections import defaultdict from typing import TYPE_CHECKING @@ -27,7 +28,7 @@ def _DUMMY_CALLBACK(response): return response -class MediaPipeline: +class MediaPipeline(ABC): LOG_FAILED_RESULTS = True class SpiderInfo: @@ -55,14 +56,6 @@ class MediaPipeline: self.handle_httpstatus_list = SequenceExclude(range(300, 400)) def _key_for_pipe(self, key, base_class_name=None, settings=None): - """ - >>> MediaPipeline()._key_for_pipe("IMAGES") - 'IMAGES' - >>> class MyPipe(MediaPipeline): - ... pass - >>> MyPipe()._key_for_pipe("IMAGES", base_class_name="MediaPipeline") - 'MYPIPE_IMAGES' - """ class_name = self.__class__.__name__ formatted_key = f"{class_name.upper()}_{key}" if ( @@ -192,21 +185,25 @@ class MediaPipeline: defer_result(result).chainDeferred(wad) # Overridable Interface + @abstractmethod def media_to_download(self, request, info, *, item=None): """Check request before starting download""" - pass + raise NotImplementedError() + @abstractmethod def get_media_requests(self, item, info): """Returns the media requests to download""" - pass + raise NotImplementedError() + @abstractmethod def media_downloaded(self, response, request, info, *, item=None): """Handler for success downloads""" - return response + raise NotImplementedError() + @abstractmethod def media_failed(self, failure, request, info): """Handler for failed downloads""" - return failure + raise NotImplementedError() def item_completed(self, results, item, info): """Called per item when all media requests has been processed""" @@ -221,6 +218,7 @@ class MediaPipeline: ) return item + @abstractmethod def file_path(self, request, response=None, info=None, *, item=None): """Returns the path where downloaded media should be stored""" - pass + raise NotImplementedError() diff --git a/tests/test_pipeline_media.py b/tests/test_pipeline_media.py index d4dde4a40..763453551 100644 --- a/tests/test_pipeline_media.py +++ b/tests/test_pipeline_media.py @@ -1,4 +1,3 @@ -import io from typing import Optional from testfixtures import LogCapture @@ -11,7 +10,6 @@ from scrapy import signals from scrapy.http import Request, Response from scrapy.http.request import NO_CALLBACK from scrapy.pipelines.files import FileException -from scrapy.pipelines.images import ImagesPipeline from scrapy.pipelines.media import MediaPipeline from scrapy.settings import Settings from scrapy.spiders import Spider @@ -35,8 +33,26 @@ def _mocked_download_func(request, info): return response() if callable(response) else response +class UserDefinedPipeline(MediaPipeline): + + def media_to_download(self, request, info, *, item=None): + pass + + def get_media_requests(self, item, info): + pass + + def media_downloaded(self, response, request, info, *, item=None): + return {} + + def media_failed(self, failure, request, info): + return failure + + def file_path(self, request, response=None, info=None, *, item=None): + return "" + + class BaseMediaPipelineTestCase(unittest.TestCase): - pipeline_class = MediaPipeline + pipeline_class = UserDefinedPipeline settings = None def setUp(self): @@ -54,54 +70,6 @@ class BaseMediaPipelineTestCase(unittest.TestCase): if not name.startswith("_"): disconnect_all(signal) - def test_default_media_to_download(self): - request = Request("http://url") - assert self.pipe.media_to_download(request, self.info) is None - - def test_default_get_media_requests(self): - item = {"name": "name"} - assert self.pipe.get_media_requests(item, self.info) is None - - def test_default_media_downloaded(self): - request = Request("http://url") - response = Response("http://url", body=b"") - assert self.pipe.media_downloaded(response, request, self.info) is response - - def test_default_media_failed(self): - request = Request("http://url") - fail = Failure(Exception()) - assert self.pipe.media_failed(fail, request, self.info) is fail - - def test_default_item_completed(self): - item = {"name": "name"} - assert self.pipe.item_completed([], item, self.info) is item - - # Check that failures are logged by default - fail = Failure(Exception()) - results = [(True, 1), (False, fail)] - - with LogCapture() as log: - new_item = self.pipe.item_completed(results, item, self.info) - - assert new_item is item - assert len(log.records) == 1 - record = log.records[0] - assert record.levelname == "ERROR" - self.assertTupleEqual(record.exc_info, failure_to_exc_info(fail)) - - # disable failure logging and check again - self.pipe.LOG_FAILED_RESULTS = False - with LogCapture() as log: - new_item = self.pipe.item_completed(results, item, self.info) - assert new_item is item - assert len(log.records) == 0 - - @inlineCallbacks - def test_default_process_item(self): - item = {"name": "name"} - new_item = yield self.pipe.process_item(item, self.spider) - assert new_item is item - def test_modify_media_request(self): request = Request("http://url") self.pipe._modify_media_request(request) @@ -175,8 +143,38 @@ class BaseMediaPipelineTestCase(unittest.TestCase): context = getattr(info.downloaded[fp].value, "__context__", None) self.assertIsNone(context) + def test_default_item_completed(self): + item = {"name": "name"} + assert self.pipe.item_completed([], item, self.info) is item -class MockedMediaPipeline(MediaPipeline): + # Check that failures are logged by default + fail = Failure(Exception()) + results = [(True, 1), (False, fail)] + + with LogCapture() as log: + new_item = self.pipe.item_completed(results, item, self.info) + + assert new_item is item + assert len(log.records) == 1 + record = log.records[0] + assert record.levelname == "ERROR" + self.assertTupleEqual(record.exc_info, failure_to_exc_info(fail)) + + # disable failure logging and check again + self.pipe.LOG_FAILED_RESULTS = False + with LogCapture() as log: + new_item = self.pipe.item_completed(results, item, self.info) + assert new_item is item + assert len(log.records) == 0 + + @inlineCallbacks + def test_default_process_item(self): + item = {"name": "name"} + new_item = yield self.pipe.process_item(item, self.spider) + assert new_item is item + + +class MockedMediaPipeline(UserDefinedPipeline): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._mockcalled = [] @@ -232,7 +230,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): ) item = {"requests": req} new_item = yield self.pipe.process_item(item, self.spider) - self.assertEqual(new_item["results"], [(True, rsp)]) + self.assertEqual(new_item["results"], [(True, {})]) self.assertEqual( self.pipe._mockcalled, [ @@ -277,7 +275,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): req2 = Request("http://url2", meta={"response": fail}) item = {"requests": [req1, req2]} new_item = yield self.pipe.process_item(item, self.spider) - self.assertEqual(new_item["results"], [(True, rsp1), (False, fail)]) + self.assertEqual(new_item["results"], [(True, {}), (False, fail)]) m = self.pipe._mockcalled # only once self.assertEqual(m[0], "get_media_requests") # first hook called @@ -315,7 +313,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): item = {"requests": req1} new_item = yield self.pipe.process_item(item, self.spider) self.assertTrue(new_item is item) - self.assertEqual(new_item["results"], [(True, rsp1)]) + self.assertEqual(new_item["results"], [(True, {})]) # rsp2 is ignored, rsp1 must be in results because request fingerprints are the same req2 = Request( @@ -325,7 +323,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): new_item = yield self.pipe.process_item(item, self.spider) self.assertTrue(new_item is item) self.assertEqual(self.fingerprint(req1), self.fingerprint(req2)) - self.assertEqual(new_item["results"], [(True, rsp1)]) + self.assertEqual(new_item["results"], [(True, {})]) @inlineCallbacks def test_results_are_cached_for_requests_of_single_item(self): @@ -337,7 +335,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): item = {"requests": [req1, req2]} new_item = yield self.pipe.process_item(item, self.spider) self.assertTrue(new_item is item) - self.assertEqual(new_item["results"], [(True, rsp1), (True, rsp1)]) + self.assertEqual(new_item["results"], [(True, {}), (True, {})]) @inlineCallbacks def test_wait_if_request_is_downloading(self): @@ -363,7 +361,7 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): req2 = Request(req1.url, meta={"response": rsp2_func}) item = {"requests": [req1, req2]} new_item = yield self.pipe.process_item(item, self.spider) - self.assertEqual(new_item["results"], [(True, rsp1), (True, rsp1)]) + self.assertEqual(new_item["results"], [(True, {}), (True, {})]) @inlineCallbacks def test_use_media_to_download_result(self): @@ -376,57 +374,15 @@ class MediaPipelineTestCase(BaseMediaPipelineTestCase): ["get_media_requests", "media_to_download", "item_completed"], ) - -class MockedMediaPipelineDeprecatedMethods(ImagesPipeline): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - self._mockcalled = [] - - def get_media_requests(self, item, info): - item_url = item["image_urls"][0] - output_img = io.BytesIO() - img = Image.new("RGB", (60, 30), color="red") - img.save(output_img, format="JPEG") - return Request( - item_url, - meta={ - "response": Response(item_url, status=200, body=output_img.getvalue()) - }, + def test_key_for_pipe(self): + self.assertEqual( + self.pipe._key_for_pipe("IMAGES", base_class_name="MediaPipeline"), + "MOCKEDMEDIAPIPELINE_IMAGES", ) - def inc_stats(self, *args, **kwargs): - return True - - def media_to_download(self, request, info): - self._mockcalled.append("media_to_download") - return super().media_to_download(request, info) - - def media_downloaded(self, response, request, info): - self._mockcalled.append("media_downloaded") - return super().media_downloaded(response, request, info) - - def file_downloaded(self, response, request, info): - self._mockcalled.append("file_downloaded") - return super().file_downloaded(response, request, info) - - def file_path(self, request, response=None, info=None): - self._mockcalled.append("file_path") - return super().file_path(request, response, info) - - def thumb_path(self, request, thumb_id, response=None, info=None): - self._mockcalled.append("thumb_path") - return super().thumb_path(request, thumb_id, response, info) - - def get_images(self, response, request, info): - self._mockcalled.append("get_images") - return super().get_images(response, request, info) - - def image_downloaded(self, response, request, info): - self._mockcalled.append("image_downloaded") - return super().image_downloaded(response, request, info) - class MediaPipelineAllowRedirectSettingsTestCase(unittest.TestCase): + def _assert_request_no3xx(self, pipeline_class, settings): pipe = pipeline_class(settings=Settings(settings)) request = Request("http://url") @@ -452,18 +408,11 @@ class MediaPipelineAllowRedirectSettingsTestCase(unittest.TestCase): else: self.assertNotIn(status, request.meta["handle_httpstatus_list"]) - def test_standard_setting(self): - self._assert_request_no3xx(MediaPipeline, {"MEDIA_ALLOW_REDIRECTS": True}) - def test_subclass_standard_setting(self): - class UserDefinedPipeline(MediaPipeline): - pass self._assert_request_no3xx(UserDefinedPipeline, {"MEDIA_ALLOW_REDIRECTS": True}) def test_subclass_specific_setting(self): - class UserDefinedPipeline(MediaPipeline): - pass self._assert_request_no3xx( UserDefinedPipeline, {"USERDEFINEDPIPELINE_MEDIA_ALLOW_REDIRECTS": True} From 0d58af86971ba54bfc1feffa88fc564ffba650f4 Mon Sep 17 00:00:00 2001 From: Fabian Schneebauer Date: Wed, 29 May 2024 10:59:32 +0200 Subject: [PATCH 19/21] Add support for multiple referer policy tokens. --- scrapy/spidermiddlewares/referer.py | 20 ++++++----- tests/test_spidermiddleware_referer.py | 47 ++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index a0b6851e5..7706c8c15 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -323,15 +323,17 @@ def _load_policy_class( try: return cast(Type[ReferrerPolicy], load_object(policy)) except ValueError: - try: - return _policy_classes[policy.lower()] - except KeyError: - msg = f"Could not load referrer policy {policy!r}" - if not warning_only: - raise RuntimeError(msg) - else: - warnings.warn(msg, RuntimeWarning) - return None + tokens = [token.strip() for token in policy.lower().split(",")] + for token in tokens[::-1]: + if token in _policy_classes: + return _policy_classes[token] + + msg = f"Could not load referrer policy {policy!r}" + if not warning_only: + raise RuntimeError(msg) + else: + warnings.warn(msg, RuntimeWarning) + return None class RefererMiddleware: diff --git a/tests/test_spidermiddleware_referer.py b/tests/test_spidermiddleware_referer.py index afffa87fb..5797edfbd 100644 --- a/tests/test_spidermiddleware_referer.py +++ b/tests/test_spidermiddleware_referer.py @@ -884,6 +884,53 @@ class TestSettingsPolicyByName(TestCase): with self.assertRaises(RuntimeError): RefererMiddleware(settings) + def test_multiple_policy_tokens(self): + # test parsing without space(s) after the comma + settings1 = Settings( + { + "REFERRER_POLICY": ",".join( + [ + "some-custom-unknown-policy", + POLICY_SAME_ORIGIN, + POLICY_STRICT_ORIGIN_WHEN_CROSS_ORIGIN, + "another-custom-unknown-policy", + ] + ) + } + ) + mw1 = RefererMiddleware(settings1) + self.assertEqual(mw1.default_policy, StrictOriginWhenCrossOriginPolicy) + + # test parsing with space(s) after the comma + settings2 = Settings( + { + "REFERRER_POLICY": ", ".join( + [ + POLICY_STRICT_ORIGIN_WHEN_CROSS_ORIGIN, + "another-custom-unknown-policy", + POLICY_UNSAFE_URL, + ] + ) + } + ) + mw2 = RefererMiddleware(settings2) + self.assertEqual(mw2.default_policy, UnsafeUrlPolicy) + + def test_multiple_policy_tokens_all_invalid(self): + settings = Settings( + { + "REFERRER_POLICY": ",".join( + [ + "some-custom-unknown-policy", + "another-custom-unknown-policy", + "yet-another-custom-unknown-policy", + ] + ) + } + ) + with self.assertRaises(RuntimeError): + RefererMiddleware(settings) + class TestPolicyHeaderPrecedence001(MixinUnsafeUrl, TestRefererMiddleware): settings = {"REFERRER_POLICY": "scrapy.spidermiddlewares.referer.SameOriginPolicy"} From 62a028b99dc73b8ddddd37f986780a5a070f2938 Mon Sep 17 00:00:00 2001 From: Fabian Schneebauer <67049088+0xdeb@users.noreply.github.com> Date: Wed, 29 May 2024 13:19:27 +0200 Subject: [PATCH 20/21] Add spec link to scrapy/spidermiddlewares/referer.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Adrián Chaves --- scrapy/spidermiddlewares/referer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index 7706c8c15..8af0bdf5b 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -324,6 +324,7 @@ def _load_policy_class( return cast(Type[ReferrerPolicy], load_object(policy)) except ValueError: tokens = [token.strip() for token in policy.lower().split(",")] + # https://www.w3.org/TR/referrer-policy/#parse-referrer-policy-from-header for token in tokens[::-1]: if token in _policy_classes: return _policy_classes[token] From b4293e8f9efac5046f92e4ebfd744be443b858b0 Mon Sep 17 00:00:00 2001 From: Andrey Rakhmatullin Date: Fri, 31 May 2024 10:50:36 +0400 Subject: [PATCH 21/21] Misc typing improvements. (#6384) --- scrapy/core/http2/agent.py | 4 +-- scrapy/core/http2/protocol.py | 8 +++-- scrapy/core/http2/stream.py | 4 +-- .../downloadermiddlewares/httpcompression.py | 14 +++++---- scrapy/downloadermiddlewares/offsite.py | 30 ++++++++++++------- scrapy/loader/__init__.py | 12 +++++++- scrapy/utils/benchserver.py | 12 ++++---- scrapy/utils/curl.py | 23 +++++++++----- scrapy/utils/datatypes.py | 2 +- scrapy/utils/request.py | 2 +- scrapy/utils/response.py | 2 +- scrapy/utils/testsite.py | 4 +-- 12 files changed, 78 insertions(+), 39 deletions(-) diff --git a/scrapy/core/http2/agent.py b/scrapy/core/http2/agent.py index 215ea9716..935af2214 100644 --- a/scrapy/core/http2/agent.py +++ b/scrapy/core/http2/agent.py @@ -119,7 +119,7 @@ class H2Agent: self._reactor, self._context_factory, connect_timeout, bind_address ) - def get_endpoint(self, uri: URI): + def get_endpoint(self, uri: URI) -> HostnameEndpoint: return self.endpoint_factory.endpointForURI(uri) def get_key(self, uri: URI) -> Tuple: @@ -161,7 +161,7 @@ class ScrapyProxyH2Agent(H2Agent): ) self._proxy_uri = proxy_uri - def get_endpoint(self, uri: URI): + def get_endpoint(self, uri: URI) -> HostnameEndpoint: return self.endpoint_factory.endpointForURI(self._proxy_uri) def get_key(self, uri: URI) -> Tuple: diff --git a/scrapy/core/http2/protocol.py b/scrapy/core/http2/protocol.py index bc8da50d7..8898b8118 100644 --- a/scrapy/core/http2/protocol.py +++ b/scrapy/core/http2/protocol.py @@ -22,7 +22,11 @@ from h2.events import ( from h2.exceptions import FrameTooLargeError, H2Error from twisted.internet.defer import Deferred from twisted.internet.error import TimeoutError -from twisted.internet.interfaces import IHandshakeListener, IProtocolNegotiationFactory +from twisted.internet.interfaces import ( + IAddress, + IHandshakeListener, + IProtocolNegotiationFactory, +) from twisted.internet.protocol import Factory, Protocol, connectionDone from twisted.internet.ssl import Certificate from twisted.protocols.policies import TimeoutMixin @@ -431,7 +435,7 @@ class H2ClientFactory(Factory): self.settings = settings self.conn_lost_deferred = conn_lost_deferred - def buildProtocol(self, addr) -> H2ClientProtocol: + def buildProtocol(self, addr: IAddress) -> H2ClientProtocol: return H2ClientProtocol(self.uri, self.settings, self.conn_lost_deferred) def acceptableProtocols(self) -> List[bytes]: diff --git a/scrapy/core/http2/stream.py b/scrapy/core/http2/stream.py index 4132fc385..224691078 100644 --- a/scrapy/core/http2/stream.py +++ b/scrapy/core/http2/stream.py @@ -1,7 +1,7 @@ import logging from enum import Enum from io import BytesIO -from typing import TYPE_CHECKING, Dict, List, Optional, Tuple +from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple from h2.errors import ErrorCodes from h2.exceptions import H2Error, ProtocolError, StreamClosedError @@ -142,7 +142,7 @@ class Stream: "headers": Headers({}), } - def _cancel(_) -> None: + def _cancel(_: Any) -> None: # Close this stream as gracefully as possible # If the associated request is initiated we reset this stream # else we directly call close() method diff --git a/scrapy/downloadermiddlewares/httpcompression.py b/scrapy/downloadermiddlewares/httpcompression.py index 0e5e215ac..8e170a1c7 100644 --- a/scrapy/downloadermiddlewares/httpcompression.py +++ b/scrapy/downloadermiddlewares/httpcompression.py @@ -3,7 +3,7 @@ from __future__ import annotations import warnings from itertools import chain from logging import getLogger -from typing import TYPE_CHECKING, List, Optional, Union +from typing import TYPE_CHECKING, List, Optional, Tuple, Union from scrapy import Request, Spider, signals from scrapy.crawler import Crawler @@ -149,20 +149,24 @@ class HttpCompressionMiddleware: return response - def _handle_encoding(self, body, content_encoding, max_size): + def _handle_encoding( + self, body: bytes, content_encoding: List[bytes], max_size: int + ) -> Tuple[bytes, List[bytes]]: to_decode, to_keep = self._split_encodings(content_encoding) for encoding in to_decode: body = self._decode(body, encoding, max_size) return body, to_keep - def _split_encodings(self, content_encoding): - to_keep = [ + def _split_encodings( + self, content_encoding: List[bytes] + ) -> Tuple[List[bytes], List[bytes]]: + to_keep: List[bytes] = [ encoding.strip().lower() for encoding in chain.from_iterable( encodings.split(b",") for encodings in content_encoding ) ] - to_decode = [] + to_decode: List[bytes] = [] while to_keep: encoding = to_keep.pop() if encoding not in ACCEPTED_ENCODINGS: diff --git a/scrapy/downloadermiddlewares/offsite.py b/scrapy/downloadermiddlewares/offsite.py index 1e5026925..bd8dbe329 100644 --- a/scrapy/downloadermiddlewares/offsite.py +++ b/scrapy/downloadermiddlewares/offsite.py @@ -1,33 +1,43 @@ +from __future__ import annotations + import logging import re import warnings +from typing import TYPE_CHECKING, Set -from scrapy import signals +from scrapy import Request, Spider, signals +from scrapy.crawler import Crawler from scrapy.exceptions import IgnoreRequest +from scrapy.statscollectors import StatsCollector from scrapy.utils.httpobj import urlparse_cached +if TYPE_CHECKING: + # typing.Self requires Python 3.11 + from typing_extensions import Self + logger = logging.getLogger(__name__) class OffsiteMiddleware: @classmethod - def from_crawler(cls, crawler): + def from_crawler(cls, crawler: Crawler) -> Self: + assert crawler.stats o = cls(crawler.stats) crawler.signals.connect(o.spider_opened, signal=signals.spider_opened) crawler.signals.connect(o.request_scheduled, signal=signals.request_scheduled) return o - def __init__(self, stats): + def __init__(self, stats: StatsCollector): self.stats = stats - self.domains_seen = set() + self.domains_seen: Set[str] = set() - def spider_opened(self, spider): - self.host_regex = self.get_host_regex(spider) + def spider_opened(self, spider: Spider) -> None: + self.host_regex: re.Pattern[str] = self.get_host_regex(spider) - def request_scheduled(self, request, spider): + def request_scheduled(self, request: Request, spider: Spider) -> None: self.process_request(request, spider) - def process_request(self, request, spider): + def process_request(self, request: Request, spider: Spider) -> None: if request.dont_filter or self.should_follow(request, spider): return None domain = urlparse_cached(request).hostname @@ -42,13 +52,13 @@ class OffsiteMiddleware: self.stats.inc_value("offsite/filtered", spider=spider) raise IgnoreRequest - def should_follow(self, request, spider): + def should_follow(self, request: Request, spider: Spider) -> bool: regex = self.host_regex # hostname can be None for wrong urls (like javascript links) host = urlparse_cached(request).hostname or "" return bool(regex.search(host)) - def get_host_regex(self, spider): + def get_host_regex(self, spider: Spider) -> re.Pattern[str]: """Override this method to implement a different offsite policy""" allowed_domains = getattr(spider, "allowed_domains", None) if not allowed_domains: diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py index 529fa279e..db0b4820f 100644 --- a/scrapy/loader/__init__.py +++ b/scrapy/loader/__init__.py @@ -4,8 +4,11 @@ Item Loader See documentation in docs/topics/loaders.rst """ +from typing import Any, Optional + import itemloaders +from scrapy.http import TextResponse from scrapy.item import Item from scrapy.selector import Selector @@ -82,7 +85,14 @@ class ItemLoader(itemloaders.ItemLoader): default_item_class: type = Item default_selector_class = Selector - def __init__(self, item=None, selector=None, response=None, parent=None, **context): + def __init__( + self, + item: Any = None, + selector: Optional[Selector] = None, + response: Optional[TextResponse] = None, + parent: Optional[itemloaders.ItemLoader] = None, + **context: Any + ): if selector is None and response is not None: try: selector = self.default_selector_class(response) diff --git a/scrapy/utils/benchserver.py b/scrapy/utils/benchserver.py index f6f704d4b..e9ea51aa1 100644 --- a/scrapy/utils/benchserver.py +++ b/scrapy/utils/benchserver.py @@ -1,21 +1,23 @@ import random +from typing import Any from urllib.parse import urlencode from twisted.web.resource import Resource -from twisted.web.server import Site +from twisted.web.server import Request, Site class Root(Resource): isLeaf = True - def getChild(self, name, request): + def getChild(self, name: str, request: Request) -> Resource: return self - def render(self, request): + def render(self, request: Request) -> bytes: total = _getarg(request, b"total", 100, int) show = _getarg(request, b"show", 10, int) nlist = [random.randint(1, total) for _ in range(show)] # nosec request.write(b"") + assert request.args is not None args = request.args.copy() for nl in nlist: args["n"] = nl @@ -27,7 +29,7 @@ class Root(Resource): return b"" -def _getarg(request, name, default=None, type=str): +def _getarg(request, name: bytes, default: Any = None, type=str): return type(request.args[name][0]) if name in request.args else default @@ -38,7 +40,7 @@ if __name__ == "__main__": factory = Site(root) httpPort = reactor.listenTCP(8998, Site(root)) - def _print_listening(): + def _print_listening() -> None: httpHost = httpPort.getHost() print(f"Bench server at http://{httpHost.host}:{httpHost.port}") diff --git a/scrapy/utils/curl.py b/scrapy/utils/curl.py index f5dbbd64e..c10e48511 100644 --- a/scrapy/utils/curl.py +++ b/scrapy/utils/curl.py @@ -2,13 +2,20 @@ import argparse import warnings from http.cookies import SimpleCookie from shlex import split +from typing import Any, Dict, List, NoReturn, Optional, Sequence, Tuple, Union from urllib.parse import urlparse from w3lib.http import basic_auth_header class DataAction(argparse.Action): - def __call__(self, parser, namespace, values, option_string=None): + def __call__( + self, + parser: argparse.ArgumentParser, + namespace: argparse.Namespace, + values: Union[str, Sequence[Any], None], + option_string: Optional[str] = None, + ) -> None: value = str(values) if value.startswith("$"): value = value[1:] @@ -16,7 +23,7 @@ class DataAction(argparse.Action): class CurlParser(argparse.ArgumentParser): - def error(self, message): + def error(self, message: str) -> NoReturn: error_msg = f"There was an error parsing the curl command: {message}" raise ValueError(error_msg) @@ -42,9 +49,11 @@ for argument in safe_to_ignore_arguments: curl_parser.add_argument(*argument, action="store_true") -def _parse_headers_and_cookies(parsed_args): - headers = [] - cookies = {} +def _parse_headers_and_cookies( + parsed_args: argparse.Namespace, +) -> Tuple[List[Tuple[str, bytes]], Dict[str, str]]: + headers: List[Tuple[str, bytes]] = [] + cookies: Dict[str, str] = {} for header in parsed_args.headers or (): name, val = header.split(":", 1) name = name.strip() @@ -64,7 +73,7 @@ def _parse_headers_and_cookies(parsed_args): def curl_to_request_kwargs( curl_command: str, ignore_unknown_options: bool = True -) -> dict: +) -> Dict[str, Any]: """Convert a cURL command syntax to Request kwargs. :param str curl_command: string containing the curl command @@ -98,7 +107,7 @@ def curl_to_request_kwargs( method = parsed_args.method or "GET" - result = {"method": method.upper(), "url": url} + result: Dict[str, Any] = {"method": method.upper(), "url": url} headers, cookies = _parse_headers_and_cookies(parsed_args) diff --git a/scrapy/utils/datatypes.py b/scrapy/utils/datatypes.py index 0ba2fe4e2..b2118495f 100644 --- a/scrapy/utils/datatypes.py +++ b/scrapy/utils/datatypes.py @@ -110,7 +110,7 @@ class CaseInsensitiveDict(collections.UserDict): as keys and allows case-insensitive lookups. """ - def __init__(self, *args, **kwargs) -> None: + def __init__(self, *args: Any, **kwargs: Any) -> None: self._keys: dict = {} super().__init__(*args, **kwargs) diff --git a/scrapy/utils/request.py b/scrapy/utils/request.py index c86f9fe39..5be80ec0f 100644 --- a/scrapy/utils/request.py +++ b/scrapy/utils/request.py @@ -138,7 +138,7 @@ class RequestFingerprinter: """ @classmethod - def from_crawler(cls, crawler) -> Self: + def from_crawler(cls, crawler: Crawler) -> Self: return cls(crawler) def __init__(self, crawler: Optional[Crawler] = None): diff --git a/scrapy/utils/response.py b/scrapy/utils/response.py index a0b06f75c..320059b3a 100644 --- a/scrapy/utils/response.py +++ b/scrapy/utils/response.py @@ -58,7 +58,7 @@ def response_status_message(status: Union[bytes, float, int, str]) -> str: return f"{status_int} {to_unicode(message)}" -def _remove_html_comments(body): +def _remove_html_comments(body: bytes) -> bytes: start = body.find(b"", start + 1) diff --git a/scrapy/utils/testsite.py b/scrapy/utils/testsite.py index de9ce992a..ca1f68116 100644 --- a/scrapy/utils/testsite.py +++ b/scrapy/utils/testsite.py @@ -15,12 +15,12 @@ class SiteTest: super().tearDown() self.site.stopListening() - def url(self, path): + def url(self, path: str) -> str: return urljoin(self.baseurl, path) class NoMetaRefreshRedirect(util.Redirect): - def render(self, request): + def render(self, request: server.Request) -> bytes: content = util.Redirect.render(self, request) return content.replace( b'http-equiv="refresh"', b'http-no-equiv="do-not-refresh-me"'