From b6e5c58ae707a3d4bb491537b5519534050047e0 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 19 Jan 2026 16:44:03 +0100 Subject: [PATCH 01/15] Fix arbitrary callable execution from the Referrer-Policy response header --- docs/news.rst | 20 +++++ docs/topics/spider-middleware.rst | 17 ++++ scrapy/spidermiddlewares/referer.py | 112 ++++++++++++++----------- scrapy/utils/python.py | 20 +++++ tests/test_spidermiddleware_referer.py | 101 ++++++++++++++++++++++ 5 files changed, 220 insertions(+), 50 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index bab2e5ff9..dfa4a36f6 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -3,6 +3,26 @@ Release notes ============= +.. _release-2.14.2: + +Scrapy 2.14.2 (unreleased) +-------------------------- + +Security bug fixes +~~~~~~~~~~~~~~~~~~ + +- Values from the ``Referrer-Policy`` header of HTTP responses are no longer + executed as Python callables. See the `cwxj-rr6w-m6w7`_ security advisory + for details. + + .. _cwxj-rr6w-m6w7: https://github.com/scrapy/scrapy/security/advisories/GHSA-cwxj-rr6w-m6w7 + +New features +~~~~~~~~~~~~ + +- Added a new setting, :setting:`REFERER_POLICIES`, to allow customizing + supported referrer policies. + .. _release-2.14.1: Scrapy 2.14.1 (2026-01-12) diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 799d2f99c..64ae988b5 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -403,6 +403,23 @@ String value Class name (as a string) .. _"strict-origin-when-cross-origin": https://www.w3.org/TR/referrer-policy/#referrer-policy-strict-origin-when-cross-origin .. _"unsafe-url": https://www.w3.org/TR/referrer-policy/#referrer-policy-unsafe-url +.. setting:: REFERRER_POLICIES + +REFERRER_POLICIES +^^^^^^^^^^^^^^^^^ + +Default: ``{}`` + +A dictionary mapping policy names to import paths of +:class:`scrapy.spidermiddlewares.referer.ReferrerPolicy` subclasses, or +``None`` to disable support for a given policy name. + +This allows to override the policies triggered by the ``Referrer-Policy`` +response header. + +Use ``""`` to override the policy for responses with `no referrer policy +`__. + StartSpiderMiddleware --------------------- diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index e4d0f4014..fe9b76500 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -17,7 +17,7 @@ from scrapy.exceptions import NotConfigured from scrapy.http import Request, Response from scrapy.spidermiddlewares.base import BaseSpiderMiddleware from scrapy.utils.misc import load_object -from scrapy.utils.python import to_unicode +from scrapy.utils.python import _looks_like_import_path, to_unicode from scrapy.utils.url import strip_url if TYPE_CHECKING: @@ -287,56 +287,38 @@ class DefaultReferrerPolicy(NoReferrerWhenDowngradePolicy): name: str = POLICY_SCRAPY_DEFAULT -_policy_classes: dict[str, type[ReferrerPolicy]] = { - p.name: p - for p in ( - NoReferrerPolicy, - NoReferrerWhenDowngradePolicy, - SameOriginPolicy, - OriginPolicy, - StrictOriginPolicy, - OriginWhenCrossOriginPolicy, - StrictOriginWhenCrossOriginPolicy, - UnsafeUrlPolicy, - DefaultReferrerPolicy, - ) -} - -# Reference: https://www.w3.org/TR/referrer-policy/#referrer-policy-empty-string -_policy_classes[""] = NoReferrerWhenDowngradePolicy - - -def _load_policy_class( - policy: str, warning_only: bool = False -) -> type[ReferrerPolicy] | None: - """ - Expect a string for the path to the policy class, - otherwise try to interpret the string as a standard value - from https://www.w3.org/TR/referrer-policy/#referrer-policies - """ - try: - 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] - - msg = f"Could not load referrer policy {policy!r}" - if not warning_only: - raise RuntimeError(msg) - warnings.warn(msg, RuntimeWarning) - return None - - class RefererMiddleware(BaseSpiderMiddleware): def __init__(self, settings: BaseSettings | None = None): # pylint: disable=super-init-not-called self.default_policy: type[ReferrerPolicy] = DefaultReferrerPolicy - if settings is not None: - settings_policy = _load_policy_class(settings.get("REFERRER_POLICY")) - assert settings_policy - self.default_policy = settings_policy + self.policies: dict[str, type[ReferrerPolicy]] = { + p.name: p + for p in ( + NoReferrerPolicy, + NoReferrerWhenDowngradePolicy, + SameOriginPolicy, + OriginPolicy, + StrictOriginPolicy, + OriginWhenCrossOriginPolicy, + StrictOriginWhenCrossOriginPolicy, + UnsafeUrlPolicy, + DefaultReferrerPolicy, + ) + } + # Reference: https://www.w3.org/TR/referrer-policy/#referrer-policy-empty-string + self.policies[""] = NoReferrerWhenDowngradePolicy + if settings is None: + return + setting_policies = settings.getdict("REFERRER_POLICIES") + for policy_name, policy_class_import_path in setting_policies.items(): + if policy_class_import_path is None: + del self.policies[policy_name] + else: + self.policies[policy_name] = load_object(policy_class_import_path) + settings_policy = self._load_policy_class( + settings.get("REFERRER_POLICY"), allow_import_path=True + ) + assert settings_policy + self.default_policy = settings_policy @classmethod def from_crawler(cls, crawler: Crawler) -> Self: @@ -362,17 +344,47 @@ class RefererMiddleware(BaseSpiderMiddleware): it is used if valid - otherwise, the policy from settings is used. """ + allow_import_path = True policy_name = request.meta.get("referrer_policy") if policy_name is None and isinstance(resp_or_url, Response): policy_header = resp_or_url.headers.get("Referrer-Policy") if policy_header is not None: policy_name = to_unicode(policy_header.decode("latin1")) + allow_import_path = False if policy_name is None: return self.default_policy() - - cls = _load_policy_class(policy_name, warning_only=True) + cls = self._load_policy_class( + policy_name, warning_only=True, allow_import_path=allow_import_path + ) return cls() if cls else self.default_policy() + def _load_policy_class( + self, + policy: str, + warning_only: bool = False, + *, + allow_import_path: bool = False, + ) -> type[ReferrerPolicy] | None: + if allow_import_path: + try: + return cast("type[ReferrerPolicy]", load_object(policy)) + except ValueError: + pass + policy_names = [ + policy_name.strip() for policy_name in policy.lower().split(",") + ] + # https://www.w3.org/TR/referrer-policy/#parse-referrer-policy-from-header + for policy_name in policy_names[::-1]: + if policy_name in self.policies: + return self.policies[policy_name] + msg = f"Could not load referrer policy {policy!r}" + if not allow_import_path and _looks_like_import_path(policy): + msg += " (import paths from the response Referrer-Policy header are not allowed)" + if not warning_only: + raise RuntimeError(msg) + warnings.warn(msg, RuntimeWarning) + return None + def get_processed_request( self, request: Request, response: Response | None ) -> Request | None: diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py index 04da75a30..bf70fa7fc 100644 --- a/scrapy/utils/python.py +++ b/scrapy/utils/python.py @@ -335,3 +335,23 @@ class MutableAsyncChain(AsyncIterator[_T]): async def __anext__(self) -> _T: return await self.data.__anext__() + + +def _looks_like_import_path(value: str) -> bool: + """Return True if **value** looks like a valid Python import path or False + otherwise.""" + if not value: + return False + if any(c.isspace() for c in value): + return False + allowed_chars = set( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_." + ) + if any(c not in allowed_chars for c in value): + return False + if value[0] == "." or value[-1] == ".": + return False + parts = value.split(".") + if any(part == "" for part in parts): + return False + return all(part.isidentifier() for part in parts) diff --git a/tests/test_spidermiddleware_referer.py b/tests/test_spidermiddleware_referer.py index 68fcad98c..0c7e360c9 100644 --- a/tests/test_spidermiddleware_referer.py +++ b/tests/test_spidermiddleware_referer.py @@ -31,6 +31,8 @@ from scrapy.spidermiddlewares.referer import ( StrictOriginWhenCrossOriginPolicy, UnsafeUrlPolicy, ) +from scrapy.utils.defer import deferred_f_from_coro_f +from scrapy.utils.misc import build_from_crawler from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler @@ -1358,3 +1360,102 @@ class TestReferrerOnRedirectStrictOriginWhenCrossOrigin(TestReferrerOnRedirect): None, ), ] + + +@deferred_f_from_coro_f +async def test_response_policy_only_supports_policy_names(): + crawler = get_crawler(settings_dict={"REFERRER_POLICY": "no-referrer"}) + mw = build_from_crawler(RefererMiddleware, crawler) + + async def input_result(): + yield Request("https://example.com/") + + response = Response( + "https://example.com/", + headers={ + "Referrer-Policy": "scrapy.spidermiddlewares.referer.NoReferrerWhenDowngradePolicy" + }, + ) + with pytest.warns( + RuntimeWarning, + match=r"Could not load referrer policy 'scrapy\.spidermiddlewares\.referer\.NoReferrerWhenDowngradePolicy' \(import paths from the response Referrer-Policy header are not allowed\)", + ): + output = [ + request + async for request in mw.process_spider_output_async( + response, input_result() + ) + ] + assert len(output) == 1 + assert b"Referer" not in output[0].headers + + response = Response( + "https://example.com/", + headers={"Referrer-Policy": "no-referrer-when-downgrade"}, + ) + output = [ + request + async for request in mw.process_spider_output_async(response, input_result()) + ] + assert len(output) == 1 + assert output[0].headers == {b"Referer": [b"https://example.com/"]} + + +@deferred_f_from_coro_f +async def test_referer_policies_setting(): + crawler = get_crawler( + settings_dict={ + "REFERRER_POLICY": "no-referrer", + "REFERRER_POLICIES": { + "no-referrer-when-downgrade": None, + "custom-policy": CustomPythonOrgPolicy, + "": CustomPythonOrgPolicy, + }, + } + ) + mw = build_from_crawler(RefererMiddleware, crawler) + + async def input_result(): + yield Request("https://example.com/") + + # "no-referrer-when-downgrade": None, + response = Response( + "https://example.com/", + headers={"Referrer-Policy": "no-referrer-when-downgrade"}, + ) + with pytest.warns( + RuntimeWarning, + match=r"Could not load referrer policy 'no-referrer-when-downgrade'", + ): + output = [ + request + async for request in mw.process_spider_output_async( + response, input_result() + ) + ] + assert len(output) == 1 + assert b"Referer" not in output[0].headers + + # "custom-policy": CustomPythonOrgPolicy, + response = Response( + "https://example.com/", + headers={"Referrer-Policy": "custom-policy"}, + ) + output = [ + request + async for request in mw.process_spider_output_async(response, input_result()) + ] + assert len(output) == 1 + assert output[0].headers == {b"Referer": [b"https://python.org/"]} + + # "": CustomPythonOrgPolicy, + response = Response( + "https://example.com/", + headers={"Referrer-Policy": ""}, + ) + output = [ + request + async for request in mw.process_spider_output_async(response, input_result()) + ] + assert len(output) == 1 + assert output[0].headers == {b"Referer": [b"https://python.org/"]} From 6f03f3250b93ed9a36f36b7ef7a59dc89cf593f4 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 19 Jan 2026 16:46:53 +0100 Subject: [PATCH 02/15] Add versionadded --- docs/topics/spider-middleware.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index 64ae988b5..bb8030d38 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -408,6 +408,8 @@ String value Class name (as a string) REFERRER_POLICIES ^^^^^^^^^^^^^^^^^ +.. versionadded:: 2.14.2 + Default: ``{}`` A dictionary mapping policy names to import paths of From f7f18123eb37d5bca520032b34233b3b92ddb74b Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 19 Jan 2026 19:15:20 +0100 Subject: [PATCH 03/15] Remove body on 307/308 redirects and switch to GET on 301 --- scrapy/downloadermiddlewares/redirect.py | 10 ++++- tests/test_downloadermiddleware_redirect.py | 45 ++++++++++++++++++--- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 61f62ecfc..0ab2f58bc 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -43,7 +43,8 @@ def _build_redirect_request( 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: + has_request_body = bool(getattr(redirect_request, "body", None)) + if has_cookie_header or has_authorization_header or has_request_body: default_ports = {"http": 80, "https": 443} parsed_source_request = urlparse_cached(source_request) @@ -76,6 +77,11 @@ def _build_redirect_request( ): del redirect_request.headers["Authorization"] + if has_request_body and source_host != redirect_host: + redirect_request = redirect_request.replace(body=b"") + redirect_request.headers.pop("Content-Type", None) + redirect_request.headers.pop("Content-Length", None) + return redirect_request @@ -174,7 +180,7 @@ class RedirectMiddleware(BaseRedirectMiddleware): if urlparse_cached(redirected).scheme not in {"http", "https"}: return response - if response.status in (301, 307, 308) or request.method == "HEAD": + if response.status in (307, 308) or request.method == "HEAD": return self._redirect(redirected, request, response.status) redirected = self._redirect_request_using_get(request, redirected_url) diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index 281bc898a..b8cebba84 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -1004,7 +1004,7 @@ class TestRedirectMiddleware(Base.Test): return Response(request.url, status=status, headers=headers) def test_redirect_3xx_permanent(self): - def _test(method, status=301): + def _test(method, status: int): url = f"http://www.example.com/{status}" url2 = "http://www.example.com/redirected" req = Request(url, method=method) @@ -1019,10 +1019,6 @@ class TestRedirectMiddleware(Base.Test): del rsp.headers["Location"] assert self.mw.process_response(req, rsp) is rsp - _test("GET") - _test("POST") - _test("HEAD") - _test("GET", status=307) _test("POST", status=307) _test("HEAD", status=307) @@ -1031,6 +1027,45 @@ class TestRedirectMiddleware(Base.Test): _test("POST", status=308) _test("HEAD", status=308) + @pytest.mark.parametrize("status", [301, 302, 303]) + def test_post_method_converted_on_301_302_303(self, status): + source_url = f"http://www.example.com/{status}" + target_url = "http://www.example.com/redirected2" + request = Request( + source_url, + method="POST", + body="test", + headers={"Content-Type": "text/plain", "Content-length": "4"}, + ) + response = Response(source_url, headers={"Location": target_url}, status=status) + redirect_request = self.mw.process_response(request, response) + assert isinstance(redirect_request, Request) + assert redirect_request.url == target_url + assert redirect_request.method == "GET" + assert "Content-Type" not in redirect_request.headers + assert "Content-Length" not in redirect_request.headers + assert not redirect_request.body + + @pytest.mark.parametrize("status", [307, 308]) + def test_cross_origin_strip_body(self, status): + source_url = "https://example.com" + target_url = "https://attacker.example" + body = b"secret" + request = Request( + source_url, + method="POST", + body=body, + headers={"Content-Type": "application/json", "Content-Length": str(len(body))}, + ) + response1 = Response(source_url, headers={"Location": target_url}, status=status) + redirect_request = self.mw.process_response(request, response1) + assert isinstance(redirect_request, Request) + assert redirect_request.url == target_url + assert redirect_request.method == "POST" + assert not getattr(redirect_request, "body", None) + assert b"Content-Type" not in redirect_request.headers + assert b"Content-Length" not in redirect_request.headers + def test_redirect_302_head(self): url = "http://www.example.com/302" url2 = "http://www.example.com/redirected2" From 99d5d58e20f4b7a4e14b0502fcdfe609dd9b10ae Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 19 Jan 2026 19:19:02 +0100 Subject: [PATCH 04/15] Add a release notes entry --- docs/news.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index bab2e5ff9..b844389e4 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -3,6 +3,19 @@ Release notes ============= +.. _release-2.14.2: + +Scrapy 2.14.2 (unreleased) +-------------------------- + +Security bug fixes +~~~~~~~~~~~~~~~~~~ + +- Cross-origin 307/308 redirects now remove the request body. See the + 7j88-353p-36rg_ security advisory for details. + + .. _7j88-353p-36rg: https://github.com/scrapy/scrapy/security/advisories/GHSA-7j88-353p-36rg + .. _release-2.14.1: Scrapy 2.14.1 (2026-01-12) From 2651d48f20622fb4b2d2144c645f39eea4c5d791 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Mon, 19 Jan 2026 19:25:00 +0100 Subject: [PATCH 05/15] Cover the 301 behavior change in the release notes --- docs/news.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/news.rst b/docs/news.rst index b844389e4..23d64aa37 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -16,6 +16,10 @@ Security bug fixes .. _7j88-353p-36rg: https://github.com/scrapy/scrapy/security/advisories/GHSA-7j88-353p-36rg +- 301 redirects now force the request method to GET, in line with modern + browsers. The original request method is not only maintained on 307 and 308 + redirects. + .. _release-2.14.1: Scrapy 2.14.1 (2026-01-12) From 1b9c8b55dafc58bc4d05ffa6451c1e0a7160f633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 20 Jan 2026 12:13:44 +0100 Subject: [PATCH 06/15] =?UTF-8?q?not=20=E2=86=92=20now?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/news.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/news.rst b/docs/news.rst index 23d64aa37..d8efa9ba0 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -17,7 +17,7 @@ Security bug fixes .. _7j88-353p-36rg: https://github.com/scrapy/scrapy/security/advisories/GHSA-7j88-353p-36rg - 301 redirects now force the request method to GET, in line with modern - browsers. The original request method is not only maintained on 307 and 308 + browsers. The original request method is now only maintained on 307 and 308 redirects. .. _release-2.14.1: From 842d0becf0f36152a1090c62c0e5d9c950241975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 20 Jan 2026 12:15:25 +0100 Subject: [PATCH 07/15] Rename test function --- tests/test_downloadermiddleware_redirect.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index b8cebba84..421b654a7 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -1028,7 +1028,7 @@ class TestRedirectMiddleware(Base.Test): _test("HEAD", status=308) @pytest.mark.parametrize("status", [301, 302, 303]) - def test_post_method_converted_on_301_302_303(self, status): + def test_method_becomes_get(self, status): source_url = f"http://www.example.com/{status}" target_url = "http://www.example.com/redirected2" request = Request( From a39545195ea41f22d7bfdc3eab83ef564480e516 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 20 Jan 2026 12:20:37 +0100 Subject: [PATCH 08/15] =?UTF-8?q?allow=20to=20override=20=E2=86=92=20allow?= =?UTF-8?q?=20overriding?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/topics/spider-middleware.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/topics/spider-middleware.rst b/docs/topics/spider-middleware.rst index bb8030d38..8b000697a 100644 --- a/docs/topics/spider-middleware.rst +++ b/docs/topics/spider-middleware.rst @@ -416,7 +416,7 @@ A dictionary mapping policy names to import paths of :class:`scrapy.spidermiddlewares.referer.ReferrerPolicy` subclasses, or ``None`` to disable support for a given policy name. -This allows to override the policies triggered by the ``Referrer-Policy`` +This allows overriding the policies triggered by the ``Referrer-Policy`` response header. Use ``""`` to override the policy for responses with `no referrer policy From 04db6a542407666de586d277acb1a651c389354e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 20 Jan 2026 12:35:00 +0100 Subject: [PATCH 09/15] Add a docstring to _load_policy_class() --- scrapy/spidermiddlewares/referer.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index fe9b76500..d49c1814b 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -365,6 +365,29 @@ class RefererMiddleware(BaseSpiderMiddleware): *, allow_import_path: bool = False, ) -> type[ReferrerPolicy] | None: + """Load the :class:`ReferrerPolicy` class to use for *policy*. + + *policy* may be any of the following: + + - A standard policy name, e.g. ``"no-referrer"``, + ``"origin-when-cross-origin"``, etc. + + - The special ``"scrapy-default"`` policy. + + - The import path of a :class:`ReferrerPolicy` subclass, e.g. + ``"scrapy.spidermiddlewares.referer.NoReferrerPolicy"`` or + ``"myproject.policies.CustomReferrerPolicy"``. + + If *warning_only* is ``False`` (default) and *policy* cannot be turned + into a :class:`ReferrerPolicy` subclass, a :exc:`RuntimeError` is + raised. If *warning_only* is ``True``, a warning is logged and ``None`` + is returned instead. + + If *allow_import_path* is ``False`` (default), import paths are not + allowed, resulting in :exc:`RuntimeError` or ``None``. If ``True``, + they are allowed. Use ``True`` only if you trust the source of the + *policy* value. + """ if allow_import_path: try: return cast("type[ReferrerPolicy]", load_object(policy)) From ba3d7bc7a8329d26862fcae248ececa386c1548a Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Wed, 21 Jan 2026 18:44:52 +0100 Subject: [PATCH 10/15] Remove the non-standard 307/308 handling, and align other aspects with the standard --- docs/news.rst | 54 ++- scrapy/downloadermiddlewares/redirect.py | 198 +++++---- scrapy/spidermiddlewares/referer.py | 75 ++-- tests/test_downloadermiddleware_redirect.py | 312 +++++++++++++- tests/test_spidermiddleware_referer.py | 429 +++----------------- 5 files changed, 553 insertions(+), 515 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index d8efa9ba0..db6749ae7 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -8,17 +8,55 @@ Release notes Scrapy 2.14.2 (unreleased) -------------------------- -Security bug fixes -~~~~~~~~~~~~~~~~~~ +Deprecations +~~~~~~~~~~~~ -- Cross-origin 307/308 redirects now remove the request body. See the - 7j88-353p-36rg_ security advisory for details. +- Using a response URL string as the first parameter on calls to + :meth:`scrapy.spidermiddlewares.referer.RefererMiddleware.policy` is + deprecated. Pass a :class:`~scrapy.http.Response` instead. - .. _7j88-353p-36rg: https://github.com/scrapy/scrapy/security/advisories/GHSA-7j88-353p-36rg + The parameter has also been renamed to ``response`` to reflect this change. + The old parameter name (``resp_or_url``) is deprecated. -- 301 redirects now force the request method to GET, in line with modern - browsers. The original request method is now only maintained on 307 and 308 - redirects. +Bug fixes +~~~~~~~~~ + +- Aligned redirect method conversions to ``GET`` with the `standard + `__: + + - 301 redirects of ``POST`` requests turn into ``GET`` requests. + + - Only ``POST`` 302 redirects turn into ``GET`` requests, other methods + are preserved. + + - ``HEAD`` 303 redirects do not turn into ``GET`` requests. + + - ``GET`` 303 redirects do not get their body or standard ``Content-*`` + headers removed. + + .. note:: Turning into a ``GET`` request implies not only a method change, + but also omitting the body and ``Content-*`` headers in the redirect + request. + +- Redirects where the original request body is dropped now also get their + ``Content-Encoding``, ``Content-Language`` and ``Content-Location`` headers + removed, in addition to ``Content-Type`` and ``Content-Length`` that were + already being removed. + +- Redirects now maintain the source URL fragment if the redirect URL doesn't + have one. This may be useful when using browser-based download handlers, + like those of `scrapy-playwright`_ or `scrapy-zyte-api`_, and letting + Scrapy handle redirects. + + .. _scrapy-playwright: https://github.com/scrapy-plugins/scrapy-playwright + .. _scrapy-zyte-api: https://scrapy-zyte-api.readthedocs.io/en/latest/ + +- The ``Referer`` header is now removed on redirect if + :class:`~scrapy.spidermiddlewares.referer.RefererMiddleware` is disabled. + +- The handling of the ``Referer`` header on redirects now accounts for the + ``Referer-Policy`` header of the response that triggers the redirect + request. .. _release-2.14.1: diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 0ab2f58bc..9520a5cbb 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -2,14 +2,17 @@ from __future__ import annotations import logging from typing import TYPE_CHECKING, Any, cast -from urllib.parse import urljoin +from urllib.parse import urljoin, urlparse from w3lib.url import safe_url_string +from scrapy import signals from scrapy.exceptions import IgnoreRequest, NotConfigured from scrapy.http import HtmlResponse, Response +from scrapy.spidermiddlewares.referer import RefererMiddleware from scrapy.utils.decorators import _warn_spider_arg from scrapy.utils.httpobj import urlparse_cached +from scrapy.utils.python import global_object_name from scrapy.utils.response import get_meta_refresh if TYPE_CHECKING: @@ -24,67 +27,6 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -def _build_redirect_request( - source_request: Request, *, url: str, **kwargs: Any -) -> Request: - redirect_request = source_request.replace( - url=url, - **kwargs, - cls=None, - 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 - has_request_body = bool(getattr(redirect_request, "body", None)) - if has_cookie_header or has_authorization_header or has_request_body: - 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"] - - if has_request_body and source_host != redirect_host: - redirect_request = redirect_request.replace(body=b"") - redirect_request.headers.pop("Content-Type", None) - redirect_request.headers.pop("Content-Length", None) - - return redirect_request - - class BaseRedirectMiddleware: crawler: Crawler enabled_setting: str = "REDIRECT_ENABLED" @@ -95,13 +37,59 @@ class BaseRedirectMiddleware: self.max_redirect_times: int = settings.getint("REDIRECT_MAX_TIMES") self.priority_adjust: int = settings.getint("REDIRECT_PRIORITY_ADJUST") + self._referer_spider_middleware: RefererMiddleware | None = None @classmethod def from_crawler(cls, crawler: Crawler) -> Self: o = cls(crawler.settings) o.crawler = crawler + crawler.signals.connect(o._engine_started, signal=signals.engine_started) return o + def handle_referer(self, request: Request, response: Response) -> None: + """Remove, modify or keep the Referer header of *request* based on the + *response* that triggered *request*. + + By default, this method finds a run-time instance of + scrapy.spidermiddlewares.referer.RefererMiddleware (or of a subclass) + and uses it to set the right Referer header. + + Override this method if you use a different Scrapy component to handle + Referer headers, of if you want to use a custom logic to set the + Referer header on redirects. + """ + request.headers.pop("Referer", None) + if not self._referer_spider_middleware: + return + self._referer_spider_middleware.get_processed_request( + request, response + ) + + def _engine_started(self) -> None: + self._referer_spider_middleware = self.crawler.get_spider_middleware(RefererMiddleware) + if self._referer_spider_middleware: + return + redirect_cls = global_object_name(self.__class__) + referer_cls = global_object_name(RefererMiddleware) + if self.__class__ in (RedirectMiddleware, MetaRefreshMiddleware): + replacement = ( + f"replace {redirect_cls} with a subclass that overrides the " + f"handle_referer() method" + ) + else: + replacement = ( + f"or edit {redirect_cls} (if defined in your code base) to " + f"override the handle_referer() method, or replace " + f"{redirect_cls} with a subclass that overrides the " + f"handle_referer() method." + ) + logger.warning( + f"{redirect_cls} found no {referer_cls} instance to handle " + f"Referer header handling, so the Referer header will be removed " + f"on redirects. To set a Referer header on redirects, enable " + f"{referer_cls} (or a subclass), or {replacement}.", + ) + def _redirect(self, redirected: Request, request: Request, reason: Any) -> Request: ttl = request.meta.setdefault("redirect_ttl", self.max_redirect_times) redirects = request.meta.get("redirect_times", 0) + 1 @@ -132,17 +120,79 @@ class BaseRedirectMiddleware: ) raise IgnoreRequest("max redirections reached") - def _redirect_request_using_get( - self, request: Request, redirect_url: str + + def _build_redirect_request( + self, source_request: Request, response: Response, *, url: str, **kwargs: Any ) -> Request: - redirect_request = _build_redirect_request( + redirect_request = source_request.replace( + url=url, + **kwargs, + cls=None, + 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: + 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"] + + self.handle_referer(redirect_request, response) + + return redirect_request + + def _redirect_request_using_get( + self, request: Request, response: Response, redirect_url: str + ) -> Request: + redirect_request = self._build_redirect_request( request, + response, url=redirect_url, method="GET", body="", ) redirect_request.headers.pop("Content-Type", None) redirect_request.headers.pop("Content-Length", None) + redirect_request.headers.pop("Content-Encoding", None) + redirect_request.headers.pop("Content-Language", None) + redirect_request.headers.pop("Content-Location", None) return redirect_request @@ -176,14 +226,22 @@ class RedirectMiddleware(BaseRedirectMiddleware): location = request_scheme + "://" + location.lstrip("/") redirected_url = urljoin(request.url, location) - redirected = _build_redirect_request(request, url=redirected_url) + + if not urlparse(redirected_url).fragment: + fragment = urlparse_cached(request).fragment + if fragment: + redirected_url = urljoin(redirected_url, f"#{fragment}") + + redirected = self._build_redirect_request(request, response, url=redirected_url) if urlparse_cached(redirected).scheme not in {"http", "https"}: return response - if response.status in (307, 308) or request.method == "HEAD": - return self._redirect(redirected, request, response.status) + if ( + (response.status in (301, 302) and request.method == "POST") + or (response.status == 303 and request.method not in ("GET", "HEAD")) + ): + redirected = self._redirect_request_using_get(request, response, redirected_url) - redirected = self._redirect_request_using_get(request, redirected_url) return self._redirect(redirected, request, response.status) @@ -210,7 +268,7 @@ class MetaRefreshMiddleware(BaseRedirectMiddleware): interval, url = get_meta_refresh(response, ignore_tags=self._ignore_tags) if not url: return response - redirected = self._redirect_request_using_get(request, url) + redirected = self._redirect_request_using_get(request, response, url) if urlparse_cached(redirected).scheme not in {"http", "https"}: return response if cast("float", interval) < self._maxdelay: diff --git a/scrapy/spidermiddlewares/referer.py b/scrapy/spidermiddlewares/referer.py index e4d0f4014..5825f6467 100644 --- a/scrapy/spidermiddlewares/referer.py +++ b/scrapy/spidermiddlewares/referer.py @@ -9,6 +9,7 @@ import warnings from abc import ABC, abstractmethod from typing import TYPE_CHECKING, cast from urllib.parse import urlparse +from warnings import warn from w3lib.url import safe_url_string @@ -342,29 +343,47 @@ class RefererMiddleware(BaseSpiderMiddleware): def from_crawler(cls, crawler: Crawler) -> Self: if not crawler.settings.getbool("REFERER_ENABLED"): raise NotConfigured - mw = cls(crawler.settings) + return cls(crawler.settings) - # Note: this hook is a bit of a hack to intercept redirections - crawler.signals.connect(mw.request_scheduled, signal=signals.request_scheduled) - - return mw - - def policy(self, resp_or_url: Response | str, request: Request) -> ReferrerPolicy: - """ - Determine Referrer-Policy to use from a parent Response (or URL), - and a Request to be sent. + def policy( + self, + response: Response | str | None = None, + request: Request | None = None, + **kwargs, + ) -> ReferrerPolicy: + """Return the referrer policy to use for *request* based on *request* + meta, *response* and settings. - if a valid policy is set in Request meta, it is used. - - if the policy is set in meta but is wrong (e.g. a typo error), - the policy from settings is used - - if the policy is not set in Request meta, - but there is a Referrer-policy header in the parent response, - it is used if valid + - if the policy is set in meta but is wrong (e.g. a typo error), the + policy from settings is used + - if the policy is not set in Request meta, but there is a + Referrer-Policy header in the parent response, it is used if valid - otherwise, the policy from settings is used. """ + if "resp_or_url" in kwargs: + if response is not None: + raise TypeError("Cannot pass both 'response' and 'resp_or_url'") + response = kwargs.pop("resp_or_url") + warn( + "Passing 'resp_or_url' is deprecated, use 'response' instead.", + DeprecationWarning, + stacklevel=2, + ) + if response is None: + raise TypeError("Missing required argument: 'response'") + if request is None: + raise TypeError("Missing required argument: 'request'") + if isinstance(response, str): + warn( + "Passing a response URL to RefererMiddleware.policy() instead " + "of a Response object is deprecated.", + DeprecationWarning, + stacklevel=2, + ) policy_name = request.meta.get("referrer_policy") - if policy_name is None and isinstance(resp_or_url, Response): - policy_header = resp_or_url.headers.get("Referrer-Policy") + if policy_name is None and isinstance(response, Response): + policy_header = response.headers.get("Referrer-Policy") if policy_header is not None: policy_name = to_unicode(policy_header.decode("latin1")) if policy_name is None: @@ -383,25 +402,3 @@ class RefererMiddleware(BaseSpiderMiddleware): if referrer is not None: request.headers.setdefault("Referer", referrer) return request - - def request_scheduled(self, request: Request, spider: Spider) -> None: - # check redirected request to patch "Referer" header if necessary - redirected_urls = request.meta.get("redirect_urls", []) - if redirected_urls: - request_referrer = request.headers.get("Referer") - # we don't patch the referrer value if there is none - if request_referrer is not None: - # the request's referrer header value acts as a surrogate - # for the parent response URL - # - # Note: if the 3xx response contained a Referrer-Policy header, - # the information is not available using this hook - parent_url = safe_url_string(request_referrer) - policy_referrer = self.policy(parent_url, request).referrer( - parent_url, request.url - ) - if policy_referrer != request_referrer.decode("latin1"): - if policy_referrer is None: - request.headers.pop("Referer") - else: - request.headers["Referer"] = policy_referrer diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index 421b654a7..4197c7c86 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -1,4 +1,6 @@ +import logging from itertools import chain, product +from unittest.mock import MagicMock import pytest @@ -9,8 +11,14 @@ from scrapy.downloadermiddlewares.redirect import ( ) from scrapy.exceptions import IgnoreRequest from scrapy.http import HtmlResponse, Request, Response +from scrapy.spidermiddlewares.referer import ( + POLICY_NO_REFERRER, + POLICY_ORIGIN, + POLICY_UNSAFE_URL, + RefererMiddleware, +) from scrapy.spiders import Spider -from scrapy.utils.misc import set_environ +from scrapy.utils.misc import build_from_crawler, set_environ from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler @@ -18,8 +26,8 @@ from scrapy.utils.test import get_crawler class Base: class Test: def test_priority_adjust(self): - req = Request("http://a.com") - rsp = self.get_response(req, "http://a.com/redirected") + req = Request("http://a.example") + rsp = self.get_response(req, "http://a.example/redirected") req2 = self.mw.process_response(req, rsp) assert req2.priority > req.priority @@ -65,7 +73,7 @@ class Base: def test_max_redirect_times(self): self.mw.max_redirect_times = 1 - req = Request("http://scrapytest.org/302") + req = Request("http://a.example/302") rsp = self.get_response(req, "/redirected") req = self.mw.process_response(req, rsp) @@ -77,7 +85,7 @@ class Base: def test_ttl(self): self.mw.max_redirect_times = 100 - req = Request("http://scrapytest.org/302", meta={"redirect_ttl": 1}) + req = Request("http://a.example/302", meta={"redirect_ttl": 1}) rsp = self.get_response(req, "/a") req = self.mw.process_response(req, rsp) @@ -86,22 +94,22 @@ class Base: self.mw.process_response(req, rsp) def test_redirect_urls(self): - req1 = Request("http://scrapytest.org/first") + req1 = Request("http://a.example/first") rsp1 = self.get_response(req1, "/redirected") req2 = self.mw.process_response(req1, rsp1) rsp2 = self.get_response(req1, "/redirected2") req3 = self.mw.process_response(req2, rsp2) - assert req2.url == "http://scrapytest.org/redirected" - assert req2.meta["redirect_urls"] == ["http://scrapytest.org/first"] - assert req3.url == "http://scrapytest.org/redirected2" + assert req2.url == "http://a.example/redirected" + assert req2.meta["redirect_urls"] == ["http://a.example/first"] + assert req3.url == "http://a.example/redirected2" assert req3.meta["redirect_urls"] == [ - "http://scrapytest.org/first", - "http://scrapytest.org/redirected", + "http://a.example/first", + "http://a.example/redirected", ] def test_redirect_reasons(self): - req1 = Request("http://scrapytest.org/first") + req1 = Request("http://a.example/first") rsp1 = self.get_response(req1, "/redirected1") req2 = self.mw.process_response(req1, rsp1) rsp2 = self.get_response(req2, "/redirected2") @@ -1046,8 +1054,158 @@ class TestRedirectMiddleware(Base.Test): assert "Content-Length" not in redirect_request.headers assert not redirect_request.body + @pytest.mark.parametrize("status", [301, 302]) + @pytest.mark.parametrize("method", ["PUT", "DELETE"]) + def test_method_not_converted_on_301_302(self, status, method): + url = f"http://www.example.com/{status}" + url2 = "http://www.example.com/redirected" + body = b"test-body" + req = Request( + url, + method=method, + body=body, + headers={"Content-Type": "text/plain", "Content-Length": str(len(body))}, + ) + rsp = Response(url, headers={"Location": url2}, status=status) + + req2 = self.mw.process_response(req, rsp) + assert isinstance(req2, Request) + assert req2.url == url2 + assert req2.method == method + assert req2.body == body + assert req2.headers[b"Content-Type"] == b"text/plain" + assert req2.headers[b"Content-Length"] == str(len(body)).encode() + + @pytest.mark.parametrize("method", ["PUT", "DELETE"]) + def test_method_converted_on_303(self, method): + status = 303 + url = f"http://www.example.com/{status}" + url2 = "http://www.example.com/redirected" + req = Request(url, method=method) + rsp = Response(url, headers={"Location": url2}, status=status) + + req2 = self.mw.process_response(req, rsp) + assert isinstance(req2, Request) + assert req2.url == url2 + assert req2.method == "GET" + + def test_get_method_body_preserved_on_303(self): + status = 303 + url = f"http://www.example.com/{status}" + url2 = "http://www.example.com/redirected" + body = b"test-body" + req = Request( + url, + method="GET", + body=body, + headers={"Content-Type": "text/plain", "Content-Length": str(len(body))}, + ) + rsp = Response(url, headers={"Location": url2}, status=status) + + req2 = self.mw.process_response(req, rsp) + assert isinstance(req2, Request) + assert req2.url == url2 + assert req2.method == "GET" + assert req2.body == body + assert req2.headers[b"Content-Type"] == b"text/plain" + assert req2.headers[b"Content-Length"] == str(len(body)).encode() + + def test_redirect_strips_content_headers(self): + url = "http://www.example.com/303" + url2 = "http://www.example.com/redirected" + headers = { + "Content-Type": "application/json", + "Content-Length": "100", + "Content-Encoding": "gzip", + "Content-Language": "en", + "Content-Location": "http://www.example.com/original", + "X-Custom": "foo", + } + req = Request(url, method="POST", headers=headers, body=b"foo") + rsp = Response(url, headers={"Location": url2}, status=303) + + req2 = self.mw.process_response(req, rsp) + assert isinstance(req2, Request) + assert req2.url == url2 + assert req2.method == "GET" + assert req2.body == b"" + assert "Content-Type" not in req2.headers + assert "Content-Length" not in req2.headers + assert "Content-Encoding" not in req2.headers + assert "Content-Language" not in req2.headers + assert "Content-Location" not in req2.headers + assert req2.headers["X-Custom"] == b"foo" + + @pytest.mark.parametrize( + ( + "referer", + "source_url", + "target_url", + "policy_header", + "expected_referer", + ), + [ + ( + None, + "http://www.example.com/302", + "http://www.example.com/redirected", + None, + b"http://www.example.com/302", + ), + ( + "http://example.com/old", + "http://www.example.com/302", + "http://www.example.com/redirected", + None, + b"http://www.example.com/302", + ), + ( + "https://example.com/old", + "https://www.example.com/302", + "http://www.example.com/redirected", + None, + None, + ), + ( + "http://example.com/old", + "http://www.example.com/foo/bar", + "http://www.example.com/redirected", + "origin", + b"http://www.example.com/", + ), + ], + ) + def test_redirect_referer( + self, referer, source_url, target_url, policy_header, expected_referer + ): + headers = {"Referer": referer} if referer else {} + source_request = Request(source_url, headers=headers) + resp_headers = {"Location": target_url} + if policy_header: + resp_headers["Referrer-Policy"] = policy_header + response = Response(source_url, headers=resp_headers, status=302) + crawler = get_crawler() + referer_mw = build_from_crawler(RefererMiddleware, crawler) + redirect_mw = self.mwcls.from_crawler(crawler) + redirect_mw._referer_spider_middleware = referer_mw + redirect_request = redirect_mw.process_response(source_request, response) + if expected_referer: + assert redirect_request.headers.get("Referer") == expected_referer + else: + assert "Referer" not in redirect_request.headers + + def test_redirect_strips_referer_no_middleware(self): + source_url = "http://www.example.com/302" + redirect_url = "http://www.example.com/redirected" + source_request = Request(source_url, headers={"Referer": "http://example.com/old"}) + response = Response(source_url, headers={"Location": redirect_url}, status=302) + redirect_mw = self.mwcls.from_crawler(get_crawler()) + redirect_mw._referer_spider_middleware = None + redirect_request = redirect_mw.process_response(source_request, response) + assert "Referer" not in redirect_request.headers + @pytest.mark.parametrize("status", [307, 308]) - def test_cross_origin_strip_body(self, status): + def test_cross_origin_maintain_body(self, status): source_url = "https://example.com" target_url = "https://attacker.example" body = b"secret" @@ -1062,9 +1220,19 @@ class TestRedirectMiddleware(Base.Test): assert isinstance(redirect_request, Request) assert redirect_request.url == target_url assert redirect_request.method == "POST" - assert not getattr(redirect_request, "body", None) - assert b"Content-Type" not in redirect_request.headers - assert b"Content-Length" not in redirect_request.headers + assert redirect_request.body == body + assert redirect_request.headers[b"Content-Type"] == b"application/json" + assert redirect_request.headers[b"Content-Length"] == str(len(body)).encode() + + def test_redirect_keeps_fragment(self): + url = "http://www.example.com/301#frag" + url2 = "http://www.example.com/redirected" + req = Request(url) + rsp = Response(url, headers={"Location": url2}, status=302) + + req2 = self.mw.process_response(req, rsp) + assert isinstance(req2, Request) + assert req2.url == "http://www.example.com/redirected#frag" def test_redirect_302_head(self): url = "http://www.example.com/302" @@ -1311,4 +1479,114 @@ def test_meta_refresh_schemes(url, location, target): assert redirect == response else: assert isinstance(redirect, Request) - assert redirect.url == target + +@pytest.mark.parametrize( + ("policy", "source_url", "target_url", "expected_referrer"), + [ + # The policy header affects the outcome. + # (without it, the https → http switch would drop the referer) + ( + POLICY_UNSAFE_URL, + "https://a.example/1", + "http://a.example/2", + b"https://a.example/1", + ), + # The policy header can get the Referer header removed. + ( + POLICY_NO_REFERRER, + "http://a.example/1", + "http://a.example/2", + None, + ), + # The policy header can get the Referer header edited (path stripped). + ( + POLICY_ORIGIN, + "http://a.example/1", + "http://a.example/2", + b"http://a.example/", + ), + ], +) +def test_response_referrer_policy( + policy, source_url, target_url, expected_referrer +): + crawler = get_crawler() + referrer_mw = build_from_crawler(RefererMiddleware, crawler) + redirect_mw = build_from_crawler(RedirectMiddleware, crawler) + redirect_mw._referer_spider_middleware = referrer_mw + source_request = Request(source_url) + extra_headers = {} + if policy: + extra_headers["Referrer-Policy"] = policy + response_redirect = Response( + source_request.url, status=301, headers={"Location": target_url, **extra_headers} + ) + source_request = redirect_mw.process_response( + source_request, response_redirect + ) + assert isinstance(source_request, Request) + + assert source_request.headers.get("Referer") == expected_referrer + + +def test_no_warning_when_referer_middleware_present(caplog): + crawler = get_crawler() + crawler.get_spider_middleware = MagicMock(return_value=MagicMock()) + mw = build_from_crawler(RedirectMiddleware, crawler) + mw._engine_started() + assert not caplog.records + + +def test_warning_redirect_middleware(caplog): + crawler = get_crawler() + crawler.get_spider_middleware = MagicMock(return_value=None) + mw = build_from_crawler(RedirectMiddleware, crawler) + with caplog.at_level(logging.WARNING): + mw._engine_started() + assert ( + "scrapy.downloadermiddlewares.redirect.RedirectMiddleware found no " + "scrapy.spidermiddlewares.referer.RefererMiddleware" + ) in caplog.text + assert "enable scrapy.spidermiddlewares.referer.RefererMiddleware (or a subclass)" in caplog.text + assert ( + "replace scrapy.downloadermiddlewares.redirect.RedirectMiddleware " + "with a subclass that overrides the handle_referer() method" + ) in caplog.text + + +def test_warning_meta_refresh_middleware(caplog): + crawler = get_crawler() + crawler.get_spider_middleware = MagicMock(return_value=None) + mw = build_from_crawler(MetaRefreshMiddleware, crawler) + with caplog.at_level(logging.WARNING): + mw._engine_started() + assert ( + "scrapy.downloadermiddlewares.redirect.MetaRefreshMiddleware found no " + "scrapy.spidermiddlewares.referer.RefererMiddleware" + ) in caplog.text + assert "enable scrapy.spidermiddlewares.referer.RefererMiddleware (or a subclass)" in caplog.text + assert ( + "replace scrapy.downloadermiddlewares.redirect.MetaRefreshMiddleware " + "with a subclass that overrides the handle_referer() method" + ) in caplog.text + + +def test_warning_subclass(caplog): + class MyRedirectMiddleware(RedirectMiddleware): + pass + + crawler = get_crawler() + crawler.get_spider_middleware = MagicMock(return_value=None) + mw = build_from_crawler(MyRedirectMiddleware, crawler) + with caplog.at_level(logging.WARNING): + mw._engine_started() + assert ( + "test_warning_subclass..MyRedirectMiddleware found no " + "scrapy.spidermiddlewares.referer.RefererMiddleware" + ) in caplog.text + assert "enable scrapy.spidermiddlewares.referer.RefererMiddleware (or a subclass)" in caplog.text + assert "edit " in caplog.text + assert "test_warning_subclass..MyRedirectMiddleware" in caplog.text + assert ( + "(if defined in your code base) to override the handle_referer() method" + ) in caplog.text diff --git a/tests/test_spidermiddleware_referer.py b/tests/test_spidermiddleware_referer.py index 68fcad98c..b4090b78c 100644 --- a/tests/test_spidermiddleware_referer.py +++ b/tests/test_spidermiddleware_referer.py @@ -6,7 +6,6 @@ from urllib.parse import urlparse import pytest -from scrapy.downloadermiddlewares.redirect import RedirectMiddleware from scrapy.http import Request, Response from scrapy.settings import Settings from scrapy.spidermiddlewares.referer import ( @@ -31,6 +30,7 @@ from scrapy.spidermiddlewares.referer import ( StrictOriginWhenCrossOriginPolicy, UnsafeUrlPolicy, ) +from scrapy.utils.misc import build_from_crawler from scrapy.utils.spider import DefaultSpider from scrapy.utils.test import get_crawler @@ -970,391 +970,58 @@ class TestPolicyHeaderPrecedence004( resp_headers = {"Referrer-Policy": ""} -class TestReferrerOnRedirect(TestRefererMiddleware): - settings = {"REFERRER_POLICY": "scrapy.spidermiddlewares.referer.UnsafeUrlPolicy"} - scenarii: Sequence[ - tuple[str, str, tuple[tuple[int, str], ...], bytes | None, bytes | None] - ] = [ # type: ignore[assignment] - ( - "http://scrapytest.org/1", # parent - "http://scrapytest.org/2", # target - ( - # redirections: code, URL - (301, "http://scrapytest.org/3"), - (301, "http://scrapytest.org/4"), - ), - b"http://scrapytest.org/1", # expected initial referer - b"http://scrapytest.org/1", # expected referer for the redirection request - ), - ( - "https://scrapytest.org/1", - "https://scrapytest.org/2", - ( - # redirecting to non-secure URL - (301, "http://scrapytest.org/3"), - ), - b"https://scrapytest.org/1", - b"https://scrapytest.org/1", - ), - ( - "https://scrapytest.org/1", - "https://scrapytest.com/2", - ( - # redirecting to non-secure URL: different origin - (301, "http://scrapytest.com/3"), - ), - b"https://scrapytest.org/1", - b"https://scrapytest.org/1", - ), - ] +class TestPolicyMethodResponseParamRename: + def setup_method(self): + self.crawler = get_crawler() + self.mw = build_from_crawler(RefererMiddleware, self.crawler) + self.request = Request("http://www.example.com") + self.response = Response("http://www.example.com") - @pytest.fixture - def crawler(self) -> Crawler: - crawler = get_crawler(DefaultSpider, self.settings) - crawler.spider = crawler._create_spider() - return crawler + def test_pos_string(self): + with warnings.catch_warnings(record=True) as w: + self.mw.policy("http://old.com", self.request) + found = False + for warning in w: + if "Passing a response URL" in str(warning.message): + found = True + break + assert found - @pytest.fixture - def referrermw(self, crawler: Crawler) -> RefererMiddleware: - return RefererMiddleware.from_crawler(crawler) + def test_pos_response(self): + with warnings.catch_warnings(record=True) as w: + self.mw.policy(self.response, self.request) + for warning in w: + assert "resp_or_url" not in str(warning.message) - @pytest.fixture - def redirectmw(self, crawler: Crawler) -> RedirectMiddleware: - return RedirectMiddleware.from_crawler(crawler) + def test_key_resp_or_url(self): + with warnings.catch_warnings(record=True) as w: + self.mw.policy(resp_or_url=self.response, request=self.request) + found = False + for warning in w: + if "Passing 'resp_or_url' is deprecated, use 'response' instead" in str(warning.message): + found = True + break + assert found - def test( # type: ignore[override] - self, - crawler: Crawler, - referrermw: RefererMiddleware, - redirectmw: RedirectMiddleware, - ) -> None: - for ( - parent, - target, - redirections, - init_referrer, - final_referrer, - ) in self.scenarii: - response = self.get_response(parent) - request = self.get_request(target) + def test_key_response(self): + with warnings.catch_warnings(record=True) as w: + self.mw.policy(response=self.response, request=self.request) + for warning in w: + assert "resp_or_url" not in str(warning.message) - out = list(referrermw.process_spider_output(response, [request])) - assert out[0].headers.get("Referer") == init_referrer + def test_key_response_string(self): + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + self.mw.policy(response="http://old.com", request=self.request) + found = False + for warning in w: + if "Passing a response URL" in str(warning.message): + found = True + break + assert found - for status, url in redirections: - response = Response( - request.url, headers={"Location": url}, status=status - ) - request = cast( - "Request", redirectmw.process_response(request, response) - ) - assert crawler.spider - referrermw.request_scheduled(request, crawler.spider) - - assert isinstance(request, Request) - assert request.headers.get("Referer") == final_referrer + def test_both_resp_or_url_and_response(self): + with pytest.raises(TypeError, match="Cannot pass both 'response' and 'resp_or_url'"): + self.mw.policy(response=self.response, resp_or_url=self.response, request=self.request) -class TestReferrerOnRedirectNoReferrer(TestReferrerOnRedirect): - """ - No Referrer policy never sets the "Referer" header. - HTTP redirections should not change that. - """ - - settings = {"REFERRER_POLICY": "no-referrer"} - scenarii = [ - ( - "http://scrapytest.org/1", # parent - "http://scrapytest.org/2", # target - ( - # redirections: code, URL - (301, "http://scrapytest.org/3"), - (301, "http://scrapytest.org/4"), - ), - None, # expected initial "Referer" - None, # expected "Referer" for the redirection request - ), - ( - "https://scrapytest.org/1", - "https://scrapytest.org/2", - ((301, "http://scrapytest.org/3"),), - None, - None, - ), - ( - "https://scrapytest.org/1", - "https://example.com/2", # different origin - ((301, "http://scrapytest.com/3"),), - None, - None, - ), - ] - - -class TestReferrerOnRedirectSameOrigin(TestReferrerOnRedirect): - """ - Same Origin policy sends the full URL as "Referer" if the target origin - is the same as the parent response (same protocol, same domain, same port). - - HTTP redirections to a different domain or a lower secure level - should have the "Referer" removed. - """ - - settings = {"REFERRER_POLICY": "same-origin"} - scenarii = [ - ( - "http://scrapytest.org/101", # origin - "http://scrapytest.org/102", # target - ( - # redirections: code, URL - (301, "http://scrapytest.org/103"), - (301, "http://scrapytest.org/104"), - ), - b"http://scrapytest.org/101", # expected initial "Referer" - b"http://scrapytest.org/101", # expected referer for the redirection request - ), - ( - "https://scrapytest.org/201", - "https://scrapytest.org/202", - ( - # redirecting from secure to non-secure URL == different origin - (301, "http://scrapytest.org/203"), - ), - b"https://scrapytest.org/201", - None, - ), - ( - "https://scrapytest.org/301", - "https://scrapytest.org/302", - ( - # different domain == different origin - (301, "http://example.com/303"), - ), - b"https://scrapytest.org/301", - None, - ), - ] - - -class TestReferrerOnRedirectStrictOrigin(TestReferrerOnRedirect): - """ - Strict Origin policy will always send the "origin" as referrer - (think of it as the parent URL without the path part), - unless the security level is lower and no "Referer" is sent. - - Redirections from secure to non-secure URLs should have the - "Referrer" header removed if necessary. - """ - - settings = {"REFERRER_POLICY": POLICY_STRICT_ORIGIN} - scenarii = [ - ( - "http://scrapytest.org/101", - "http://scrapytest.org/102", - ( - (301, "http://scrapytest.org/103"), - (301, "http://scrapytest.org/104"), - ), - b"http://scrapytest.org/", # send origin - b"http://scrapytest.org/", # redirects to same origin: send origin - ), - ( - "https://scrapytest.org/201", - "https://scrapytest.org/202", - ( - # redirecting to non-secure URL: no referrer - (301, "http://scrapytest.org/203"), - ), - b"https://scrapytest.org/", - None, - ), - ( - "https://scrapytest.org/301", - "https://scrapytest.org/302", - ( - # redirecting to non-secure URL (different domain): no referrer - (301, "http://example.com/303"), - ), - b"https://scrapytest.org/", - None, - ), - ( - "http://scrapy.org/401", - "http://example.com/402", - ((301, "http://scrapytest.org/403"),), - b"http://scrapy.org/", - b"http://scrapy.org/", - ), - ( - "https://scrapy.org/501", - "https://example.com/502", - ( - # HTTPS all along, so origin referrer is kept as-is - (301, "https://google.com/503"), - (301, "https://facebook.com/504"), - ), - b"https://scrapy.org/", - b"https://scrapy.org/", - ), - ( - "https://scrapytest.org/601", - "http://scrapytest.org/602", # TLS to non-TLS: no referrer - ( - ( - 301, - "https://scrapytest.org/603", - ), # TLS URL again: (still) no referrer - ), - None, - None, - ), - ] - - -class TestReferrerOnRedirectOriginWhenCrossOrigin(TestReferrerOnRedirect): - """ - Origin When Cross-Origin policy sends the full URL as "Referer", - unless the target's origin is different (different domain, different protocol) - in which case only the origin is sent. - - Redirections to a different origin should strip the "Referer" - to the parent origin. - """ - - settings = {"REFERRER_POLICY": POLICY_ORIGIN_WHEN_CROSS_ORIGIN} - scenarii = [ - ( - "http://scrapytest.org/101", # origin - "http://scrapytest.org/102", # target + redirection - ( - # redirections: code, URL - (301, "http://scrapytest.org/103"), - (301, "http://scrapytest.org/104"), - ), - b"http://scrapytest.org/101", # expected initial referer - b"http://scrapytest.org/101", # expected referer for the redirection request - ), - ( - "https://scrapytest.org/201", - "https://scrapytest.org/202", - ( - # redirecting to non-secure URL: send origin - (301, "http://scrapytest.org/203"), - ), - b"https://scrapytest.org/201", - b"https://scrapytest.org/", - ), - ( - "https://scrapytest.org/301", - "https://scrapytest.org/302", - ( - # redirecting to non-secure URL (different domain): send origin - (301, "http://example.com/303"), - ), - b"https://scrapytest.org/301", - b"https://scrapytest.org/", - ), - ( - "http://scrapy.org/401", - "http://example.com/402", - ((301, "http://scrapytest.org/403"),), - b"http://scrapy.org/", - b"http://scrapy.org/", - ), - ( - "https://scrapy.org/501", - "https://example.com/502", - ( - # all different domains: send origin - (301, "https://google.com/503"), - (301, "https://facebook.com/504"), - ), - b"https://scrapy.org/", - b"https://scrapy.org/", - ), - ( - "https://scrapytest.org/301", - "http://scrapytest.org/302", # TLS to non-TLS: send origin - ((301, "https://scrapytest.org/303"),), # TLS URL again: send origin (also) - b"https://scrapytest.org/", - b"https://scrapytest.org/", - ), - ] - - -class TestReferrerOnRedirectStrictOriginWhenCrossOrigin(TestReferrerOnRedirect): - """ - Strict Origin When Cross-Origin policy sends the full URL as "Referer", - unless the target's origin is different (different domain, different protocol) - in which case only the origin is sent... - Unless there's also a downgrade in security and then the "Referer" header - is not sent. - - Redirections to a different origin should strip the "Referer" to the parent origin, - and from https:// to http:// will remove the "Referer" header. - """ - - settings = {"REFERRER_POLICY": POLICY_STRICT_ORIGIN_WHEN_CROSS_ORIGIN} - scenarii = [ - ( - "http://scrapytest.org/101", # origin - "http://scrapytest.org/102", # target + redirection - ( - # redirections: code, URL - (301, "http://scrapytest.org/103"), - (301, "http://scrapytest.org/104"), - ), - b"http://scrapytest.org/101", # expected initial referer - b"http://scrapytest.org/101", # expected referer for the redirection request - ), - ( - "https://scrapytest.org/201", - "https://scrapytest.org/202", - ( - # redirecting to non-secure URL: do not send the "Referer" header - (301, "http://scrapytest.org/203"), - ), - b"https://scrapytest.org/201", - None, - ), - ( - "https://scrapytest.org/301", - "https://scrapytest.org/302", - ( - # redirecting to non-secure URL (different domain): send origin - (301, "http://example.com/303"), - ), - b"https://scrapytest.org/301", - None, - ), - ( - "http://scrapy.org/401", - "http://example.com/402", - ((301, "http://scrapytest.org/403"),), - b"http://scrapy.org/", - b"http://scrapy.org/", - ), - ( - "https://scrapy.org/501", - "https://example.com/502", - ( - # all different domains: send origin - (301, "https://google.com/503"), - (301, "https://facebook.com/504"), - ), - b"https://scrapy.org/", - b"https://scrapy.org/", - ), - ( - "https://scrapytest.org/601", - "http://scrapytest.org/602", # TLS to non-TLS: do not send "Referer" - ( - ( - 301, - "https://scrapytest.org/603", - ), # TLS URL again: (still) send nothing - ), - None, - None, - ), - ] From 8974580e438d18a105b8a0475e90bce2f1eb4dca Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Tue, 3 Feb 2026 20:38:11 +0100 Subject: [PATCH 11/15] Reword the release note entry to consider the 301 redirect fix a security bug fix --- docs/news.rst | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index db6749ae7..a7d8e50f3 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -8,6 +8,18 @@ Release notes Scrapy 2.14.2 (unreleased) -------------------------- +Security bug fixes +~~~~~~~~~~~~~~~~~~ + +- In line with the `standard + `__, 301 redirects of + ``POST`` requests turn into ``GET`` requests. + + Turning into a ``GET`` request implies not only a method change, but also + omitting the body and ``Content-*`` headers in the redirect request. On + cross-origin redirects (e.g. cross-domain), this is effectively a security + bug fix for scenarios where the body contains secrets. + Deprecations ~~~~~~~~~~~~ @@ -21,10 +33,8 @@ Deprecations Bug fixes ~~~~~~~~~ -- Aligned redirect method conversions to ``GET`` with the `standard - `__: - - - 301 redirects of ``POST`` requests turn into ``GET`` requests. +- Made additional redirect scenarios convert to ``GET`` in line with the + `standard `__: - Only ``POST`` 302 redirects turn into ``GET`` requests, other methods are preserved. @@ -34,10 +44,6 @@ Bug fixes - ``GET`` 303 redirects do not get their body or standard ``Content-*`` headers removed. - .. note:: Turning into a ``GET`` request implies not only a method change, - but also omitting the body and ``Content-*`` headers in the redirect - request. - - Redirects where the original request body is dropped now also get their ``Content-Encoding``, ``Content-Language`` and ``Content-Location`` headers removed, in addition to ``Content-Type`` and ``Content-Length`` that were From 886131c7b2f2e792fc139e5660f908239836388c Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Thu, 12 Mar 2026 13:54:00 +0100 Subject: [PATCH 12/15] Run pre-commit --- scrapy/downloadermiddlewares/redirect.py | 18 ++++----- tests/test_downloadermiddleware_redirect.py | 41 ++++++++++++++------- 2 files changed, 37 insertions(+), 22 deletions(-) diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 9520a5cbb..69c58ca49 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -61,12 +61,12 @@ class BaseRedirectMiddleware: request.headers.pop("Referer", None) if not self._referer_spider_middleware: return - self._referer_spider_middleware.get_processed_request( - request, response - ) + self._referer_spider_middleware.get_processed_request(request, response) def _engine_started(self) -> None: - self._referer_spider_middleware = self.crawler.get_spider_middleware(RefererMiddleware) + self._referer_spider_middleware = self.crawler.get_spider_middleware( + RefererMiddleware + ) if self._referer_spider_middleware: return redirect_cls = global_object_name(self.__class__) @@ -120,7 +120,6 @@ class BaseRedirectMiddleware: ) raise IgnoreRequest("max redirections reached") - def _build_redirect_request( self, source_request: Request, response: Response, *, url: str, **kwargs: Any ) -> Request: @@ -236,11 +235,12 @@ class RedirectMiddleware(BaseRedirectMiddleware): if urlparse_cached(redirected).scheme not in {"http", "https"}: return response - if ( - (response.status in (301, 302) and request.method == "POST") - or (response.status == 303 and request.method not in ("GET", "HEAD")) + if (response.status in (301, 302) and request.method == "POST") or ( + response.status == 303 and request.method not in ("GET", "HEAD") ): - redirected = self._redirect_request_using_get(request, response, redirected_url) + redirected = self._redirect_request_using_get( + request, response, redirected_url + ) return self._redirect(redirected, request, response.status) diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index 4197c7c86..89185aafb 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -1197,7 +1197,9 @@ class TestRedirectMiddleware(Base.Test): def test_redirect_strips_referer_no_middleware(self): source_url = "http://www.example.com/302" redirect_url = "http://www.example.com/redirected" - source_request = Request(source_url, headers={"Referer": "http://example.com/old"}) + source_request = Request( + source_url, headers={"Referer": "http://example.com/old"} + ) response = Response(source_url, headers={"Location": redirect_url}, status=302) redirect_mw = self.mwcls.from_crawler(get_crawler()) redirect_mw._referer_spider_middleware = None @@ -1213,9 +1215,14 @@ class TestRedirectMiddleware(Base.Test): source_url, method="POST", body=body, - headers={"Content-Type": "application/json", "Content-Length": str(len(body))}, + headers={ + "Content-Type": "application/json", + "Content-Length": str(len(body)), + }, + ) + response1 = Response( + source_url, headers={"Location": target_url}, status=status ) - response1 = Response(source_url, headers={"Location": target_url}, status=status) redirect_request = self.mw.process_response(request, response1) assert isinstance(redirect_request, Request) assert redirect_request.url == target_url @@ -1480,6 +1487,7 @@ def test_meta_refresh_schemes(url, location, target): else: assert isinstance(redirect, Request) + @pytest.mark.parametrize( ("policy", "source_url", "target_url", "expected_referrer"), [ @@ -1507,9 +1515,7 @@ def test_meta_refresh_schemes(url, location, target): ), ], ) -def test_response_referrer_policy( - policy, source_url, target_url, expected_referrer -): +def test_response_referrer_policy(policy, source_url, target_url, expected_referrer): crawler = get_crawler() referrer_mw = build_from_crawler(RefererMiddleware, crawler) redirect_mw = build_from_crawler(RedirectMiddleware, crawler) @@ -1519,11 +1525,11 @@ def test_response_referrer_policy( if policy: extra_headers["Referrer-Policy"] = policy response_redirect = Response( - source_request.url, status=301, headers={"Location": target_url, **extra_headers} - ) - source_request = redirect_mw.process_response( - source_request, response_redirect + source_request.url, + status=301, + headers={"Location": target_url, **extra_headers}, ) + source_request = redirect_mw.process_response(source_request, response_redirect) assert isinstance(source_request, Request) assert source_request.headers.get("Referer") == expected_referrer @@ -1547,7 +1553,10 @@ def test_warning_redirect_middleware(caplog): "scrapy.downloadermiddlewares.redirect.RedirectMiddleware found no " "scrapy.spidermiddlewares.referer.RefererMiddleware" ) in caplog.text - assert "enable scrapy.spidermiddlewares.referer.RefererMiddleware (or a subclass)" in caplog.text + assert ( + "enable scrapy.spidermiddlewares.referer.RefererMiddleware (or a subclass)" + in caplog.text + ) assert ( "replace scrapy.downloadermiddlewares.redirect.RedirectMiddleware " "with a subclass that overrides the handle_referer() method" @@ -1564,7 +1573,10 @@ def test_warning_meta_refresh_middleware(caplog): "scrapy.downloadermiddlewares.redirect.MetaRefreshMiddleware found no " "scrapy.spidermiddlewares.referer.RefererMiddleware" ) in caplog.text - assert "enable scrapy.spidermiddlewares.referer.RefererMiddleware (or a subclass)" in caplog.text + assert ( + "enable scrapy.spidermiddlewares.referer.RefererMiddleware (or a subclass)" + in caplog.text + ) assert ( "replace scrapy.downloadermiddlewares.redirect.MetaRefreshMiddleware " "with a subclass that overrides the handle_referer() method" @@ -1584,7 +1596,10 @@ def test_warning_subclass(caplog): "test_warning_subclass..MyRedirectMiddleware found no " "scrapy.spidermiddlewares.referer.RefererMiddleware" ) in caplog.text - assert "enable scrapy.spidermiddlewares.referer.RefererMiddleware (or a subclass)" in caplog.text + assert ( + "enable scrapy.spidermiddlewares.referer.RefererMiddleware (or a subclass)" + in caplog.text + ) assert "edit " in caplog.text assert "test_warning_subclass..MyRedirectMiddleware" in caplog.text assert ( From 8e28f938d29a496c3bf9fbffb212e1808213d9c4 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Thu, 12 Mar 2026 14:29:51 +0100 Subject: [PATCH 13/15] Make test_no_warning_when_referer_middleware_present less brittle --- tests/test_downloadermiddleware_redirect.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index 89185aafb..ab63ad545 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -1539,8 +1539,14 @@ def test_no_warning_when_referer_middleware_present(caplog): crawler = get_crawler() crawler.get_spider_middleware = MagicMock(return_value=MagicMock()) mw = build_from_crawler(RedirectMiddleware, crawler) - mw._engine_started() - assert not caplog.records + caplog.clear() + with caplog.at_level(logging.WARNING): + mw._engine_started() + assert not [ + record + for record in caplog.records + if record.name == "scrapy.downloadermiddlewares.redirect" + ] def test_warning_redirect_middleware(caplog): From 378bb68039876c5e77b293cccd80eb5f306afd7e Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Thu, 12 Mar 2026 15:44:54 +0100 Subject: [PATCH 14/15] Proofread the release notes --- docs/news.rst | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index ee9d7c017..1407a321d 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -19,17 +19,18 @@ Security bug fixes - In line with the `standard `__, 301 redirects of - ``POST`` requests turn into ``GET`` requests. + ``POST`` requests are converted into ``GET`` requests. - Turning into a ``GET`` request implies not only a method change, but also + Converting to a ``GET`` request implies not only a method change, but also omitting the body and ``Content-*`` headers in the redirect request. On - cross-origin redirects (e.g. cross-domain), this is effectively a security - bug fix for scenarios where the body contains secrets. + cross-origin redirects (for example, cross-domain redirects), this is + effectively a security bug fix for scenarios where the body contains + secrets. Deprecations ~~~~~~~~~~~~ -- Using a response URL string as the first parameter on calls to +- Passing a response URL string as the first positional argument to :meth:`scrapy.spidermiddlewares.referer.RefererMiddleware.policy` is deprecated. Pass a :class:`~scrapy.http.Response` instead. @@ -48,23 +49,23 @@ Bug fixes - Made additional redirect scenarios convert to ``GET`` in line with the `standard `__: - - Only ``POST`` 302 redirects turn into ``GET`` requests, other methods - are preserved. + - Only ``POST`` 302 redirects are converted into ``GET`` requests; other + methods are preserved. - - ``HEAD`` 303 redirects do not turn into ``GET`` requests. + - ``HEAD`` 303 redirects are not converted into ``GET`` requests. - - ``GET`` 303 redirects do not get their body or standard ``Content-*`` + - ``GET`` 303 redirects do not have their body or standard ``Content-*`` headers removed. -- Redirects where the original request body is dropped now also get their +- Redirects where the original request body is dropped now also have their ``Content-Encoding``, ``Content-Language`` and ``Content-Location`` headers - removed, in addition to ``Content-Type`` and ``Content-Length`` that were - already being removed. + removed, in addition to the ``Content-Type`` and ``Content-Length`` headers + that were already being removed. -- Redirects now maintain the source URL fragment if the redirect URL doesn't - have one. This may be useful when using browser-based download handlers, - like those of `scrapy-playwright`_ or `scrapy-zyte-api`_, and letting - Scrapy handle redirects. +- Redirects now preserve the source URL fragment if the redirect URL does not + include one. This is useful when using browser-based download handlers, + such as `scrapy-playwright`_ or `scrapy-zyte-api`_, while letting Scrapy + handle redirects. .. _scrapy-playwright: https://github.com/scrapy-plugins/scrapy-playwright .. _scrapy-zyte-api: https://scrapy-zyte-api.readthedocs.io/en/latest/ @@ -72,9 +73,8 @@ Bug fixes - The ``Referer`` header is now removed on redirect if :class:`~scrapy.spidermiddlewares.referer.RefererMiddleware` is disabled. -- The handling of the ``Referer`` header on redirects now accounts for the - ``Referer-Policy`` header of the response that triggers the redirect - request. +- The handling of the ``Referer`` header on redirects now takes into account + the ``Referer-Policy`` header of the response that triggers the redirect. .. _release-2.14.1: From 498b4fc1a431c71ea699b2d7e0bd518c7ceca302 Mon Sep 17 00:00:00 2001 From: Adrian Chaves Date: Thu, 12 Mar 2026 15:45:16 +0100 Subject: [PATCH 15/15] =?UTF-8?q?Bump=20version:=202.14.1=20=E2=86=92=202.?= =?UTF-8?q?14.2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/news.rst | 2 +- pyproject.toml | 2 +- scrapy/VERSION | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 1407a321d..45c2f48be 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -5,7 +5,7 @@ Release notes .. _release-2.14.2: -Scrapy 2.14.2 (unreleased) +Scrapy 2.14.2 (2026-03-12) -------------------------- Security bug fixes diff --git a/pyproject.toml b/pyproject.toml index 7982b43b8..98dd0b42c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -111,7 +111,7 @@ module = "twisted" implicit_reexport = true [tool.bumpversion] -current_version = "2.14.1" +current_version = "2.14.2" commit = true tag = true tag_name = "{new_version}" diff --git a/scrapy/VERSION b/scrapy/VERSION index b70ae75a8..7243b12cf 100644 --- a/scrapy/VERSION +++ b/scrapy/VERSION @@ -1 +1 @@ -2.14.1 +2.14.2