From 6499214a4f6817e1845073bd167deb33ed5261af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Wed, 6 Mar 2024 15:40:13 +0100 Subject: [PATCH] Keep the Cookie header on scheme upgrades --- scrapy/downloadermiddlewares/redirect.py | 2 +- tests/test_downloadermiddleware_redirect.py | 66 +++++++++++++++++++-- 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 494c98271..a26715e12 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -38,7 +38,7 @@ def _build_redirect_request(source_request, *, url, **kwargs): or default_ports.get(parsed_redirect_request.scheme), ) - if source_scheme != redirect_scheme or source_host != redirect_host: + if redirect_scheme != "https" or source_host != redirect_host: if has_cookie_header: del redirect_request.headers["Cookie"] diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index 5292a02ba..c14f87d91 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -257,6 +257,8 @@ class RedirectMiddlewareTest(unittest.TestCase): headers={**safe_headers, **cookie_header, **authorization_header}, ) + # Redirects to the same origin (same scheme, same domain, same port) + # keep all headers. internal_response = Response( "https://example.com", headers={"Location": "https://example.com/a"}, @@ -268,19 +270,39 @@ class RedirectMiddlewareTest(unittest.TestCase): self.assertIsInstance(internal_redirect_request, Request) self.assertEqual(original_request.headers, internal_redirect_request.headers) - default_port_response = Response( + # For default ports, whether the port is explicit or implicit does not + # affect the outcome, it is still the same origin. + to_explicit_port_response = Response( "https://example.com", headers={"Location": "https://example.com:443/a"}, status=301, ) - default_port_redirect_request = self.mw.process_response( - original_request, default_port_response, self.spider + to_explicit_port_redirect_request = self.mw.process_response( + original_request, to_explicit_port_response, self.spider ) - self.assertIsInstance(default_port_redirect_request, Request) + self.assertIsInstance(to_explicit_port_redirect_request, Request) self.assertEqual( - original_request.headers, default_port_redirect_request.headers + original_request.headers, to_explicit_port_redirect_request.headers ) + # For default ports, whether the port is explicit or implicit does not + # affect the outcome, it is still the same origin. + to_implicit_port_response = Response( + "https://example.com:433", + headers={"Location": "https://example.com/a"}, + status=301, + ) + to_implicit_port_redirect_request = self.mw.process_response( + original_request, to_implicit_port_response, self.spider + ) + self.assertIsInstance(to_implicit_port_redirect_request, Request) + self.assertEqual( + original_request.headers, to_implicit_port_redirect_request.headers + ) + + # A port change drops the Authorization header because the origin + # changes, but keeps the Cookie header because the domain remains the + # same. different_port_response = Response( "https://example.com", headers={"Location": "https://example.com:8080/a"}, @@ -295,6 +317,7 @@ class RedirectMiddlewareTest(unittest.TestCase): different_port_redirect_request.headers.to_unicode_dict(), ) + # A domain change drops both the Authorization and the Cookie header. external_response = Response( "https://example.com", headers={"Location": "https://example.org/a"}, @@ -308,6 +331,36 @@ class RedirectMiddlewareTest(unittest.TestCase): safe_headers, external_redirect_request.headers.to_unicode_dict() ) + # A scheme upgrade (http → https) drops the Authorization header + # because the origin changes, but keeps the Cookie header because the + # domain remains the same. + http_request = Request( + "http://example.com", + headers={**safe_headers, **cookie_header, **authorization_header}, + ) + upgrade_response = Response( + "http://example.com", + headers={"Location": "https://example.com/a"}, + status=301, + ) + upgrade_redirect_request = self.mw.process_response( + http_request, upgrade_response, self.spider + ) + self.assertIsInstance(upgrade_redirect_request, Request) + self.assertEqual( + {**safe_headers, **cookie_header}, + upgrade_redirect_request.headers.to_unicode_dict(), + ) + + # A scheme downgrade (https → http) drops the Authorization header + # because the origin changes, and the Cookie header because its value + # cannot indicate whether the cookies were secure (HTTPS-only) or not. + # + # Note: If the Cookie header is set by the cookie management + # middleware, as recommended in the docs, the dropping of Cookie on + # scheme downgrade is not an issue, because the cookie management + # middleware will add again the Cookie header to the new request if + # appropriate. downgrade_response = Response( "https://example.com", headers={"Location": "http://example.com/a"}, @@ -318,7 +371,8 @@ class RedirectMiddlewareTest(unittest.TestCase): ) self.assertIsInstance(downgrade_redirect_request, Request) self.assertEqual( - safe_headers, downgrade_redirect_request.headers.to_unicode_dict() + safe_headers, + downgrade_redirect_request.headers.to_unicode_dict(), )