Skip to content

Commit 8b10afd

Browse files
Fix credential leak on same-host redirects with different ports (aio-libs#12275)
1 parent b26c9ae commit 8b10afd

4 files changed

Lines changed: 20 additions & 51 deletions

File tree

CHANGES/5783.feature

Lines changed: 0 additions & 1 deletion
This file was deleted.

aiohttp/client.py

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -869,12 +869,6 @@ async def _connect_and_send_request(
869869
elif not scheme:
870870
parsed_redirect_url = url.join(parsed_redirect_url)
871871

872-
is_same_host_https_redirect = (
873-
url.host == parsed_redirect_url.host
874-
and parsed_redirect_url.scheme == "https"
875-
and url.scheme == "http"
876-
)
877-
878872
try:
879873
redirect_origin = parsed_redirect_url.origin()
880874
except ValueError as origin_val_err:
@@ -886,10 +880,7 @@ async def _connect_and_send_request(
886880
"Invalid redirect URL origin",
887881
) from origin_val_err
888882

889-
if (
890-
not is_same_host_https_redirect
891-
and url.origin() != redirect_origin
892-
):
883+
if url.origin() != redirect_origin:
893884
auth = None
894885
headers.pop(hdrs.AUTHORIZATION, None)
895886
headers.pop(hdrs.COOKIE, None)

docs/client_advanced.rst

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -114,17 +114,6 @@ In cases where the authentication header value expires periodically, an
114114
:mod:`asyncio` task may be used to update the session's default headers in the
115115
background.
116116

117-
.. note::
118-
119-
The ``Authorization`` header will be removed if you get redirected
120-
to a different host or protocol, except the case when HTTP → HTTPS
121-
redirect is performed on the same host.
122-
123-
.. versionchanged:: 4.0
124-
125-
Started keeping the ``Authorization`` header during HTTP → HTTPS
126-
redirects when the host remains the same.
127-
128117
.. _aiohttp-client-middleware:
129118

130119
Client Middleware

tests/test_client_functional.py

Lines changed: 19 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3390,61 +3390,51 @@ def create(url: URL, srv: Handler) -> Awaitable[TestServer]:
33903390

33913391

33923392
@pytest.mark.parametrize(
3393-
["url_from_s", "url_to_s", "is_drop_header_expected"],
3394-
[
3395-
[
3396-
"http://host1.com/path1",
3397-
"http://host2.com/path2",
3398-
True,
3399-
],
3400-
["http://host1.com/path1", "https://host1.com/path1", False],
3401-
["https://host1.com/path1", "http://host1.com/path2", True],
3402-
],
3393+
("url_from_s", "url_to_s"),
3394+
(
3395+
("http://host1.com/path1", "http://host2.com/path2"),
3396+
("http://host1.com/path1", "https://host1.com/path1"),
3397+
("https://host1.com/path1", "http://host1.com/path2"),
3398+
("http://host1.com/path1", "https://host1.com:9443/path1"),
3399+
),
34033400
ids=(
34043401
"entirely different hosts",
34053402
"http -> https",
34063403
"https -> http",
3404+
"http -> https different port",
34073405
),
34083406
)
34093407
async def test_drop_auth_on_redirect_to_other_host(
34103408
create_server_for_url_and_handler: Callable[[URL, Handler], Awaitable[TestServer]],
34113409
url_from_s: str,
34123410
url_to_s: str,
3413-
is_drop_header_expected: bool,
34143411
) -> None:
34153412
url_from, url_to = URL(url_from_s), URL(url_to_s)
34163413

34173414
async def srv_from(request: web.Request) -> NoReturn:
3418-
assert request.host == url_from.host
3415+
assert request.host.split(":")[0] == url_from.host
34193416
assert request.headers["Authorization"] == "Basic dXNlcjpwYXNz"
34203417
raise web.HTTPFound(url_to)
34213418

34223419
async def srv_to(request: web.Request) -> web.Response:
3423-
assert request.host == url_to.host
3424-
if is_drop_header_expected:
3425-
assert "Authorization" not in request.headers, "Header wasn't dropped"
3426-
assert "Proxy-Authorization" not in request.headers
3427-
assert "Cookie" not in request.headers
3428-
else:
3429-
assert "Authorization" in request.headers, "Header was dropped"
3430-
assert "Proxy-Authorization" in request.headers
3431-
assert "Cookie" in request.headers
3420+
assert request.host.split(":")[0] == url_to.host
3421+
assert "Authorization" not in request.headers, "Header wasn't dropped"
3422+
assert "Proxy-Authorization" not in request.headers
3423+
assert "Cookie" not in request.headers
34323424
return web.Response()
34333425

34343426
server_from = await create_server_for_url_and_handler(url_from, srv_from)
34353427
server_to = await create_server_for_url_and_handler(url_to, srv_to)
34363428

34373429
assert (
3438-
url_from.host != url_to.host or server_from.scheme != server_to.scheme
3439-
), "Invalid test case, host or scheme must differ"
3430+
url_from.host != url_to.host
3431+
or server_from.scheme != server_to.scheme
3432+
or url_from.port != url_to.port
3433+
), "Invalid test case, host, scheme, or port must differ"
34403434

3441-
protocol_port_map = {
3442-
"http": 80,
3443-
"https": 443,
3444-
}
34453435
etc_hosts = {
3446-
(url_from.host, protocol_port_map[server_from.scheme]): server_from,
3447-
(url_to.host, protocol_port_map[server_to.scheme]): server_to,
3436+
(url_from.host, url_from.port): server_from,
3437+
(url_to.host, url_to.port): server_to,
34483438
}
34493439

34503440
class FakeResolver(AbstractResolver):

0 commit comments

Comments
 (0)