fix: preserve GitHub CDN redirects
This commit is contained in:
@@ -231,16 +231,38 @@ class _HttpsOnlyRedirectHandler(urllib.request.HTTPRedirectHandler):
|
|||||||
self.origin_url = origin_url
|
self.origin_url = origin_url
|
||||||
self.same_origin = same_origin
|
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":
|
if urllib.parse.urlsplit(newurl).scheme != "https":
|
||||||
raise ValueError("generic HTTPS request redirected to a non-HTTPS URL")
|
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):
|
if self.same_origin and self.origin_url and not _same_origin(self.origin_url, newurl):
|
||||||
raise ValueError("generic HTTPS redirect changed origin")
|
raise ValueError("generic HTTPS redirect changed origin")
|
||||||
# Keep configured headers on each allowed redirect. The source is
|
# Follow urllib's redirect behavior: use ordinary request headers, not
|
||||||
# explicitly configured by the user, so this also supports CDN hops.
|
# 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(
|
redirected = urllib.request.Request(
|
||||||
newurl,
|
newurl,
|
||||||
headers=dict(req.header_items()),
|
headers=redirect_headers,
|
||||||
origin_req_host=req.origin_req_host,
|
origin_req_host=req.origin_req_host,
|
||||||
unverifiable=True,
|
unverifiable=True,
|
||||||
method=req.get_method(),
|
method=req.get_method(),
|
||||||
|
|||||||
@@ -3,6 +3,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import hashlib
|
import hashlib
|
||||||
|
from http.client import HTTPMessage
|
||||||
import io
|
import io
|
||||||
import json
|
import json
|
||||||
import subprocess
|
import subprocess
|
||||||
@@ -693,7 +694,7 @@ class TestGenericSource:
|
|||||||
)
|
)
|
||||||
with pytest.raises(ValueError, match="non-HTTPS"):
|
with pytest.raises(ValueError, match="non-HTTPS"):
|
||||||
sideload._HttpsOnlyRedirectHandler().redirect_request(
|
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):
|
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"):
|
with pytest.raises(ValueError, match="changed origin"):
|
||||||
handler.redirect_request(
|
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(
|
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"
|
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):
|
def test_atomic_download_rejects_downgrade_before_response_body(self):
|
||||||
request = urllib.request.Request("https://downloads.example/file.apk")
|
request = urllib.request.Request("https://downloads.example/file.apk")
|
||||||
handler = sideload._HttpsOnlyRedirectHandler(
|
handler = sideload._HttpsOnlyRedirectHandler(
|
||||||
@@ -720,7 +763,7 @@ class TestGenericSource:
|
|||||||
with pytest.raises(ValueError, match="non-HTTPS"):
|
with pytest.raises(ValueError, match="non-HTTPS"):
|
||||||
handler.redirect_request(
|
handler.redirect_request(
|
||||||
request, mock.Mock(read=mock.Mock(side_effect=AssertionError)),
|
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):
|
def test_redirected_page_url_is_used_for_relative_links(self):
|
||||||
|
|||||||
Reference in New Issue
Block a user