diff --git a/scrapy/downloadermiddlewares/redirect.py b/scrapy/downloadermiddlewares/redirect.py index 3176ed930..36bd9a849 100644 --- a/scrapy/downloadermiddlewares/redirect.py +++ b/scrapy/downloadermiddlewares/redirect.py @@ -20,14 +20,38 @@ def _build_redirect_request(source_request, *, url, **kwargs): has_cookie_header = "Cookie" in redirect_request.headers has_authorization_header = "Authorization" in redirect_request.headers if has_cookie_header or has_authorization_header: - source_request_netloc = urlparse_cached(source_request).netloc - redirect_request_netloc = urlparse_cached(redirect_request).netloc - if source_request_netloc != redirect_request_netloc: - if has_cookie_header: - del redirect_request.headers["Cookie"] - # https://fetch.spec.whatwg.org/#ref-for-cors-non-wildcard-request-header-name - if has_authorization_header: - del redirect_request.headers["Authorization"] + default_ports = {"http": 80, "https": 443} + + parsed_source_request = urlparse_cached(source_request) + source_scheme, source_host, source_port = ( + parsed_source_request.scheme, + parsed_source_request.hostname, + parsed_source_request.port + or default_ports.get(parsed_source_request.scheme), + ) + + parsed_redirect_request = urlparse_cached(redirect_request) + redirect_scheme, redirect_host, redirect_port = ( + parsed_redirect_request.scheme, + parsed_redirect_request.hostname, + parsed_redirect_request.port + or default_ports.get(parsed_redirect_request.scheme), + ) + + if has_cookie_header and ( + (source_scheme != redirect_scheme and redirect_scheme != "https") + or source_host != redirect_host + ): + del redirect_request.headers["Cookie"] + + # https://fetch.spec.whatwg.org/#ref-for-cors-non-wildcard-request-header-name + if has_authorization_header and ( + source_scheme != redirect_scheme + or source_host != redirect_host + or source_port != redirect_port + ): + del redirect_request.headers["Authorization"] + return redirect_request diff --git a/tests/test_downloadermiddleware_redirect.py b/tests/test_downloadermiddleware_redirect.py index 83ff25982..203678302 100644 --- a/tests/test_downloadermiddleware_redirect.py +++ b/tests/test_downloadermiddleware_redirect.py @@ -247,13 +247,18 @@ class RedirectMiddlewareTest(unittest.TestCase): perc_encoded_utf8_url = "http://scrapytest.org/a%C3%A7%C3%A3o" self.assertEqual(perc_encoded_utf8_url, req_result.url) - def test_cross_domain_header_dropping(self): + def test_cross_origin_header_dropping(self): safe_headers = {"A": "B"} + cookie_header = {"Cookie": "a=b"} + authorization_header = {"Authorization": "Bearer 123456"} + original_request = Request( "https://example.com", - headers={"Cookie": "a=b", "Authorization": "a", **safe_headers}, + 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"}, @@ -265,6 +270,71 @@ class RedirectMiddlewareTest(unittest.TestCase): self.assertIsInstance(internal_redirect_request, Request) self.assertEqual(original_request.headers, internal_redirect_request.headers) + # Redirects to the same origin (same scheme, same domain, same port) + # keep all headers also when the scheme is http. + http_request = Request( + "http://example.com", + headers={**safe_headers, **cookie_header, **authorization_header}, + ) + http_response = Response( + "http://example.com", + headers={"Location": "http://example.com/a"}, + status=301, + ) + http_redirect_request = self.mw.process_response( + http_request, http_response, self.spider + ) + self.assertIsInstance(http_redirect_request, Request) + self.assertEqual(http_request.headers, http_redirect_request.headers) + + # For default ports, whether the port is explicit or implicit does not + # affect the outcome, it is still the same origin. + to_explicit_port_response = Response( + "https://example.com", + headers={"Location": "https://example.com:443/a"}, + status=301, + ) + to_explicit_port_redirect_request = self.mw.process_response( + original_request, to_explicit_port_response, self.spider + ) + self.assertIsInstance(to_explicit_port_redirect_request, Request) + self.assertEqual( + original_request.headers, to_explicit_port_redirect_request.headers + ) + + # For default ports, whether the port is explicit or implicit does not + # affect the outcome, it is still the same origin. + to_implicit_port_response = 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"}, + status=301, + ) + different_port_redirect_request = self.mw.process_response( + original_request, different_port_response, self.spider + ) + self.assertIsInstance(different_port_redirect_request, Request) + self.assertEqual( + {**safe_headers, **cookie_header}, + different_port_redirect_request.headers.to_unicode_dict(), + ) + + # A domain change drops both the Authorization and the Cookie header. external_response = Response( "https://example.com", headers={"Location": "https://example.org/a"}, @@ -278,6 +348,46 @@ 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. + 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"}, + status=301, + ) + downgrade_redirect_request = self.mw.process_response( + original_request, downgrade_response, self.spider + ) + self.assertIsInstance(downgrade_redirect_request, Request) + self.assertEqual( + safe_headers, + downgrade_redirect_request.headers.to_unicode_dict(), + ) + class MetaRefreshMiddlewareTest(unittest.TestCase): def setUp(self):