From 8d7d06bdbf1c9c14ca8f268600643568b204da10 Mon Sep 17 00:00:00 2001 From: Antoine Van Elstraete Date: Sat, 8 Aug 2026 16:37:07 +0200 Subject: [PATCH] fix: preserve GitHub CDN redirects --- src/sideload.py | 30 +++++++++++++++++++++---- tests/test_sideload.py | 51 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/src/sideload.py b/src/sideload.py index a0459d3..04b77ab 100644 --- a/src/sideload.py +++ b/src/sideload.py @@ -231,16 +231,38 @@ class _HttpsOnlyRedirectHandler(urllib.request.HTTPRedirectHandler): self.origin_url = origin_url self.same_origin = same_origin - def redirect_request(self, req, fp, code, msg, newurl, headers): + def redirect_request(self, req, fp, code, msg, headers, newurl): if urllib.parse.urlsplit(newurl).scheme != "https": raise ValueError("generic HTTPS request redirected to a non-HTTPS URL") if self.same_origin and self.origin_url and not _same_origin(self.origin_url, newurl): raise ValueError("generic HTTPS redirect changed origin") - # Keep configured headers on each allowed redirect. The source is - # explicitly configured by the user, so this also supports CDN hops. + # Follow urllib's redirect behavior: use ordinary request headers, not + # unredirected headers populated by the HTTP transport (notably Host). + # Do not carry entity or hop-by-hop headers to the new request, while + # retaining application headers such as Authorization for configured + # CDN redirects. + excluded_headers = { + "connection", + "content-length", + "content-type", + "keep-alive", + "proxy-authenticate", + "proxy-authorization", + "proxy-connection", + "te", + "trailer", + "transfer-encoding", + "upgrade", + "host", + } + redirect_headers = { + name: value + for name, value in req.headers.items() + if name.lower() not in excluded_headers + } redirected = urllib.request.Request( newurl, - headers=dict(req.header_items()), + headers=redirect_headers, origin_req_host=req.origin_req_host, unverifiable=True, method=req.get_method(), diff --git a/tests/test_sideload.py b/tests/test_sideload.py index 51c91de..5e8ff84 100644 --- a/tests/test_sideload.py +++ b/tests/test_sideload.py @@ -3,6 +3,7 @@ from __future__ import annotations import hashlib +from http.client import HTTPMessage import io import json import subprocess @@ -693,7 +694,7 @@ class TestGenericSource: ) with pytest.raises(ValueError, match="non-HTTPS"): sideload._HttpsOnlyRedirectHandler().redirect_request( - request, mock.Mock(), 302, "Found", "http://evil.example/", {} + request, mock.Mock(), 302, "Found", {}, "http://evil.example/" ) def test_cross_host_redirect_is_blocked_and_same_origin_keeps_headers(self): @@ -705,13 +706,55 @@ class TestGenericSource: ) with pytest.raises(ValueError, match="changed origin"): handler.redirect_request( - request, mock.Mock(), 302, "Found", "https://cdn.example/file", {} + request, mock.Mock(), 302, "Found", {}, "https://cdn.example/file" ) redirected = handler.redirect_request( - request, mock.Mock(), 302, "Found", "https://downloads.example/file", {} + request, mock.Mock(), 302, "Found", {}, "https://downloads.example/file" ) assert redirected.get_header("Authorization") == "secret-token" + def test_github_cdn_redirect_drops_unredirected_host_and_keeps_authorization(self): + request = urllib.request.Request( + "https://github.com/example/app/releases/download/v1/app.apk", + headers={"Authorization": "secret-token"}, + ) + request.unredirected_hdrs["Host"] = "github.com" + handler = sideload._HttpsOnlyRedirectHandler(same_origin=False) + + redirected = handler.redirect_request( + request, + mock.Mock(), + 302, + "Found", + {}, + "https://objects.githubusercontent.com/app.apk", + ) + + assert redirected.get_header("Host") is None + assert redirected.get_header("Authorization") == "secret-token" + + def test_https_redirect_accepts_http_message_headers(self): + request = urllib.request.Request( + "https://downloads.example/start", headers={"Authorization": "secret-token"} + ) + headers = HTTPMessage() + headers.add_header("Location", "https://downloads.example/file.apk") + handler = sideload._HttpsOnlyRedirectHandler( + "https://downloads.example/start", same_origin=True + ) + + redirected = handler.redirect_request( + request, + mock.Mock(), + 302, + "Found", + headers, + "https://downloads.example/file.apk", + ) + + assert redirected.full_url == "https://downloads.example/file.apk" + assert redirected.get_header("Authorization") == "secret-token" + def test_atomic_download_rejects_downgrade_before_response_body(self): request = urllib.request.Request("https://downloads.example/file.apk") handler = sideload._HttpsOnlyRedirectHandler( @@ -720,7 +763,7 @@ class TestGenericSource: with pytest.raises(ValueError, match="non-HTTPS"): handler.redirect_request( request, mock.Mock(read=mock.Mock(side_effect=AssertionError)), - 302, "Found", "http://downloads.example/file.apk", {} + 302, "Found", {}, "http://downloads.example/file.apk" ) def test_redirected_page_url_is_used_for_relative_links(self):