From fe5e41211483083644cbab166d6b9b97823a90d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 6 Feb 2020 18:48:03 +0100 Subject: [PATCH 1/7] Do not filter duplicate requests within a redirect chain --- scrapy/downloadermiddlewares/redirect.py | 4 ++++ scrapy/dupefilters.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 77cb5aa94..c910d3e0d 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -4,6 +4,7 @@ from urllib.parse import urljoin, urlparse from w3lib.url import safe_url_string from scrapy.http import HtmlResponse +from scrapy.utils.request import request_fingerprint from scrapy.utils.response import get_meta_refresh from scrapy.exceptions import IgnoreRequest, NotConfigured @@ -37,6 +38,9 @@ class BaseRedirectMiddleware(object): [request.url] redirected.meta['redirect_reasons'] = request.meta.get('redirect_reasons', []) + \ [reason] + fingerprints = request.meta.get('redirect_fingerprints', set()) + fingerprint = request_fingerprint(request) + redirected.meta['redirect_fingerprints'] = fingerprints | {fingerprint} redirected.dont_filter = request.dont_filter redirected.priority = request.priority + self.priority_adjust logger.debug("Redirecting (%(reason)s) to %(redirected)s from %(request)s", diff --git a/scrapy/dupefilters.py b/scrapy/dupefilters.py index ea6a4cfc3..0b2986caa 100644 --- a/scrapy/dupefilters.py +++ b/scrapy/dupefilters.py @@ -45,6 +45,10 @@ class RFPDupeFilter(BaseDupeFilter): def request_seen(self, request): fp = self.request_fingerprint(request) + redirect_fps = request.meta.get('redirect_fingerprints', set()) + if fp in redirect_fps: + assert fp in self.fingerprints + return False if fp in self.fingerprints: return True self.fingerprints.add(fp) From 03311f8857ec94399aff56b153be40b676b3d310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 6 Feb 2020 18:57:07 +0100 Subject: [PATCH 2/7] Improve how request_seen handles redirect fingerprints --- scrapy/dupefilters.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/scrapy/dupefilters.py b/scrapy/dupefilters.py index 0b2986caa..ae3eafc24 100644 --- a/scrapy/dupefilters.py +++ b/scrapy/dupefilters.py @@ -45,12 +45,9 @@ class RFPDupeFilter(BaseDupeFilter): def request_seen(self, request): fp = self.request_fingerprint(request) - redirect_fps = request.meta.get('redirect_fingerprints', set()) - if fp in redirect_fps: - assert fp in self.fingerprints - return False if fp in self.fingerprints: - return True + redirect_fps = request.meta.get('redirect_fingerprints', set()) + return fp not in redirect_fps self.fingerprints.add(fp) if self.file: self.file.write(fp + os.linesep) From cb077dc361c5a3b7155d92de2034153945fe81e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 28 Dec 2023 13:58:31 +0100 Subject: [PATCH 3/7] Use the global request fingerprinter --- scrapy/downloadermiddlewares/redirect.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 6c6edd0f2..762fa6cc5 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -12,7 +12,6 @@ from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import HtmlResponse, Response from scrapy.settings import BaseSettings from scrapy.utils.httpobj import urlparse_cached -from scrapy.utils.request import request_fingerprint from scrapy.utils.response import get_meta_refresh if TYPE_CHECKING: @@ -50,7 +49,9 @@ class BaseRedirectMiddleware: @classmethod def from_crawler(cls, crawler: Crawler) -> Self: - return cls(crawler.settings) + mw = cls(crawler.settings) + mw.crawler = crawler + return mw def _redirect( self, redirected: Request, request: Request, spider: Spider, reason: Any @@ -66,7 +67,7 @@ class BaseRedirectMiddleware: redirect_reasons = request.meta.get("redirect_reasons", []) + [reason] redirected.meta["redirect_reasons"] = redirect_reasons fingerprints = request.meta.get("redirect_fingerprints", set()) - fingerprint = request_fingerprint(request) + fingerprint = self.crawler.request_fingerprinter.fingerprint(request) redirected.meta["redirect_fingerprints"] = fingerprints | {fingerprint} redirected.dont_filter = request.dont_filter redirected.priority = request.priority + self.priority_adjust From 4c37f77c87f6dba247964de5e5d4fe01437c5015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 28 Dec 2023 15:20:55 +0100 Subject: [PATCH 4/7] Add tests And fix the implementation thanks to them. --- scrapy/dupefilters.py | 2 +- tests/test_downloadermiddleware_redirect.py | 92 +++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/scrapy/dupefilters.py b/scrapy/dupefilters.py index a0ca73978..e679f9626 100644 --- a/scrapy/dupefilters.py +++ b/scrapy/dupefilters.py @@ -87,7 +87,7 @@ class RFPDupeFilter(BaseDupeFilter): fp = self.request_fingerprint(request) if fp in self.fingerprints: redirect_fps = request.meta.get("redirect_fingerprints", set()) - return fp not in redirect_fps + return fp not in {_fp.hex() for _fp in redirect_fps} self.fingerprints.add(fp) if self.file: self.file.write(fp + "\n") diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index dc15b672c..582b56a7c 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -9,6 +9,8 @@ from scrapy.http import HtmlResponse, Request, Response from scrapy.spiders import Spider from scrapy.utils.test import get_crawler +from .test_dupefilters import _get_dupefilter + class RedirectMiddlewareTest(unittest.TestCase): def setUp(self): @@ -247,6 +249,96 @@ 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_self_redirect_direct(self): + dupefilter = _get_dupefilter(crawler=self.crawler) + request1 = Request("https://example.com/a") + self.assertFalse(dupefilter.request_seen(request1)) + + response1 = Response( + request1.url, + status=302, + headers={"Location": "/a"}, + ) + request2 = self.mw.process_response(request1, response1, self.spider) + self.assertIsInstance(request2, Request) + fingerprint1 = self.crawler.request_fingerprinter.fingerprint(request1) + fingerprint2 = self.crawler.request_fingerprinter.fingerprint(request2) + self.assertEqual(fingerprint1, fingerprint2) + + self.assertFalse(dupefilter.request_seen(request2)) + + def test_self_redirect_indirect(self): + dupefilter = _get_dupefilter(crawler=self.crawler) + request1 = Request("https://example.com/a") + self.assertFalse(dupefilter.request_seen(request1)) + + response1 = Response( + request1.url, + status=302, + headers={"Location": "/b"}, + ) + request2 = self.mw.process_response(request1, response1, self.spider) + self.assertIsInstance(request2, Request) + fingerprint1 = self.crawler.request_fingerprinter.fingerprint(request1) + fingerprint2 = self.crawler.request_fingerprinter.fingerprint(request2) + self.assertNotEqual(fingerprint1, fingerprint2) + + self.assertFalse(dupefilter.request_seen(request2)) + + response2 = Response( + request2.url, + status=302, + headers={"Location": "/a"}, + ) + request3 = self.mw.process_response(request2, response2, self.spider) + self.assertIsInstance(request3, Request) + fingerprint3 = self.crawler.request_fingerprinter.fingerprint(request3) + self.assertEqual(fingerprint1, fingerprint3) + + self.assertFalse(dupefilter.request_seen(request3)) + + def test_self_redirect_zigzag(self): + dupefilter = _get_dupefilter(crawler=self.crawler) + request1 = Request("https://example.com/a") + self.assertFalse(dupefilter.request_seen(request1)) + + response1 = Response( + request1.url, + status=302, + headers={"Location": "/b"}, + ) + request2 = self.mw.process_response(request1, response1, self.spider) + self.assertIsInstance(request2, Request) + fingerprint1 = self.crawler.request_fingerprinter.fingerprint(request1) + fingerprint2 = self.crawler.request_fingerprinter.fingerprint(request2) + self.assertNotEqual(fingerprint1, fingerprint2) + + self.assertFalse(dupefilter.request_seen(request2)) + + response2 = Response( + request2.url, + status=302, + headers={"Location": "/a"}, + ) + request3 = self.mw.process_response(request2, response2, self.spider) + self.assertIsInstance(request3, Request) + fingerprint3 = self.crawler.request_fingerprinter.fingerprint(request3) + self.assertEqual(fingerprint1, fingerprint3) + + self.assertFalse(dupefilter.request_seen(request3)) + + response3 = Response( + request3.url, + status=302, + headers={"Location": "/b"}, + ) + request4 = self.mw.process_response(request3, response3, self.spider) + self.assertIsInstance(request4, Request) + fingerprint4 = self.crawler.request_fingerprinter.fingerprint(request4) + self.assertEqual(fingerprint2, fingerprint4) + + self.assertFalse(dupefilter.request_seen(request4)) + class MetaRefreshMiddlewareTest(unittest.TestCase): def setUp(self): From d31ae7782fba565a7cf2f8df718f5293f51da1df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 28 Dec 2023 15:24:38 +0100 Subject: [PATCH 5/7] Fix typing issues --- scrapy/downloadermiddlewares/redirect.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 762fa6cc5..ac8028050 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -39,6 +39,7 @@ def _build_redirect_request( class BaseRedirectMiddleware: enabled_setting: str = "REDIRECT_ENABLED" + crawler: Crawler def __init__(self, settings: BaseSettings): if not settings.getbool(self.enabled_setting): @@ -67,6 +68,7 @@ class BaseRedirectMiddleware: redirect_reasons = request.meta.get("redirect_reasons", []) + [reason] redirected.meta["redirect_reasons"] = redirect_reasons fingerprints = request.meta.get("redirect_fingerprints", set()) + assert self.crawler.request_fingerprinter is not None fingerprint = self.crawler.request_fingerprinter.fingerprint(request) redirected.meta["redirect_fingerprints"] = fingerprints | {fingerprint} redirected.dont_filter = request.dont_filter From f6807f7d6f7ad66a36a90145b5342f91c2e91bc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Thu, 28 Dec 2023 15:47:49 +0100 Subject: [PATCH 6/7] Fix the initialization of RedirectMiddleware.from_crawler in some tests --- tests/test_downloadermiddleware_cookies.py | 3 +-- tests/test_spidermiddleware_referer.py | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_downloadermiddleware_cookies.py b/tests/test_downloadermiddleware_cookies.py index 4a81a638e..1b1a580fe 100644 --- a/tests/test_downloadermiddleware_cookies.py +++ b/tests/test_downloadermiddleware_cookies.py @@ -9,7 +9,6 @@ from scrapy.downloadermiddlewares.defaultheaders import DefaultHeadersMiddleware from scrapy.downloadermiddlewares.redirect import RedirectMiddleware from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response -from scrapy.settings import Settings from scrapy.spiders import Spider from scrapy.utils.python import to_bytes from scrapy.utils.test import get_crawler @@ -61,7 +60,7 @@ class CookiesMiddlewareTest(TestCase): def setUp(self): self.spider = Spider("foo") self.mw = CookiesMiddleware() - self.redirect_middleware = RedirectMiddleware(settings=Settings()) + self.redirect_middleware = RedirectMiddleware.from_crawler(get_crawler(Spider)) def tearDown(self): del self.mw diff --git a/tests/test_spidermiddleware_referer.py b/tests/test_spidermiddleware_referer.py index afffa87fb..66dcc0d3f 100644 --- a/tests/test_spidermiddleware_referer.py +++ b/tests/test_spidermiddleware_referer.py @@ -29,6 +29,7 @@ from scrapy.spidermiddlewares.referer import ( UnsafeUrlPolicy, ) from scrapy.spiders import Spider +from scrapy.utils.test import get_crawler class TestRefererMiddleware(TestCase): @@ -961,7 +962,7 @@ class TestReferrerOnRedirect(TestRefererMiddleware): self.spider = Spider("foo") settings = Settings(self.settings) self.referrermw = RefererMiddleware(settings) - self.redirectmw = RedirectMiddleware(settings) + self.redirectmw = RedirectMiddleware.from_crawler(get_crawler(Spider)) def test(self): for ( From 2b9e63e55d19a5fe56e17fba5d48ec8f7ddc04dd Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Fri, 31 Jul 2026 20:04:20 +0200 Subject: [PATCH 7/7] Give PyPy some leeway --- tests/test_utils_response.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_utils_response.py b/tests/test_utils_response.py index 608b2bbd9..72de08430 100644 --- a/tests/test_utils_response.py +++ b/tests/test_utils_response.py @@ -16,6 +16,11 @@ from scrapy.utils.response import ( response_status_message, ) +# Catastrophic backtracking makes the checks below take orders of magnitude +# longer than this, so the budget can be generous enough for slow interpreters +# like PyPy. +MAX_CPU_TIME = 0.2 + def _read_browser_output(burl: str) -> bytes: path = urlparse(burl).path @@ -185,8 +190,6 @@ def test_inject_base_url(body: bytes) -> None: def test_open_in_browser_redos_comment(): - MAX_CPU_TIME = 0.02 - # Exploit input from # https://makenowjust-labs.github.io/recheck/playground/ # for // (old pattern to remove comments). @@ -199,8 +202,6 @@ def test_open_in_browser_redos_comment(): def test_open_in_browser_redos_head(): - MAX_CPU_TIME = 0.02 - # Exploit input from # https://makenowjust-labs.github.io/recheck/playground/ # for /(|\s.*?>))/ (old pattern to find the head element).