Skip to content

Commit

Permalink
Merge pull request from GHSA-g4mx-q9vg-27p4
Browse files Browse the repository at this point in the history
  • Loading branch information
illia-v committed Oct 17, 2023
1 parent 80808b0 commit 4e50fbc
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 3 deletions.
6 changes: 6 additions & 0 deletions dummyserver/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,12 @@ def encodingrequest(self, request: httputil.HTTPServerRequest) -> Response:
def headers(self, request: httputil.HTTPServerRequest) -> Response:
return Response(json.dumps(dict(request.headers)))

def headers_and_params(self, request: httputil.HTTPServerRequest) -> Response:
params = request_params(request)
return Response(
json.dumps({"headers": dict(request.headers), "params": params})
)

def multi_headers(self, request: httputil.HTTPServerRequest) -> Response:
return Response(json.dumps({"headers": list(request.headers.get_all())}))

Expand Down
20 changes: 19 additions & 1 deletion src/urllib3/_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
if typing.TYPE_CHECKING:
# We can only import Protocol if TYPE_CHECKING because it's a development
# dependency, and is not available at runtime.
from typing_extensions import Protocol
from typing_extensions import Protocol, Self

class HasGettableStringKeys(Protocol):
def keys(self) -> typing.Iterator[str]:
Expand Down Expand Up @@ -391,6 +391,24 @@ def getlist(
# meets our external interface requirement of `Union[List[str], _DT]`.
return vals[1:]

def _prepare_for_method_change(self) -> Self:
"""
Remove content-specific header fields before changing the request
method to GET or HEAD according to RFC 9110, Section 15.4.
"""
content_specific_headers = [
"Content-Encoding",
"Content-Language",
"Content-Location",
"Content-Type",
"Content-Length",
"Digest",
"Last-Modified",
]
for header in content_specific_headers:
self.discard(header)
return self

# Backwards compatibility for httplib
getheaders = getlist
getallmatchingheaders = getlist
Expand Down
5 changes: 5 additions & 0 deletions src/urllib3/connectionpool.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from types import TracebackType

from ._base_connection import _TYPE_BODY
from ._collections import HTTPHeaderDict
from ._request_methods import RequestMethods
from .connection import (
BaseSSLError,
Expand Down Expand Up @@ -893,7 +894,11 @@ def urlopen( # type: ignore[override]
redirect_location = redirect and response.get_redirect_location()
if redirect_location:
if response.status == 303:
# Change the method according to RFC 9110, Section 15.4.4.
method = "GET"
# And lose the body not to transfer anything sensitive.
body = None
headers = HTTPHeaderDict(headers)._prepare_for_method_change()

try:
retries = retries.increment(method, url, response=response, _pool=self)
Expand Down
7 changes: 5 additions & 2 deletions src/urllib3/poolmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from types import TracebackType
from urllib.parse import urljoin

from ._collections import RecentlyUsedContainer
from ._collections import HTTPHeaderDict, RecentlyUsedContainer
from ._request_methods import RequestMethods
from .connection import ProxyConfig
from .connectionpool import HTTPConnectionPool, HTTPSConnectionPool, port_by_scheme
Expand Down Expand Up @@ -449,9 +449,12 @@ def urlopen( # type: ignore[override]
# Support relative URLs for redirecting.
redirect_location = urljoin(url, redirect_location)

# RFC 7231, Section 6.4.4
if response.status == 303:
# Change the method according to RFC 9110, Section 15.4.4.
method = "GET"
# And lose the body not to transfer anything sensitive.
kw["body"] = None
kw["headers"] = HTTPHeaderDict(kw["headers"])._prepare_for_method_change()

retries = kw.get("retries")
if not isinstance(retries, Retry):
Expand Down
11 changes: 11 additions & 0 deletions test/with_dummyserver/test_connectionpool.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,17 @@ def test_redirect(self) -> None:
assert r.status == 200
assert r.data == b"Dummy server!"

def test_303_redirect_makes_request_lose_body(self) -> None:
with HTTPConnectionPool(self.host, self.port) as pool:
response = pool.request(
"POST",
"/redirect",
fields={"target": "/headers_and_params", "status": "303 See Other"},
)
data = response.json()
assert data["params"] == {}
assert "Content-Type" not in HTTPHeaderDict(data["headers"])

def test_bad_connect(self) -> None:
with HTTPConnectionPool("badhost.invalid", self.port) as pool:
with pytest.raises(MaxRetryError) as e:
Expand Down
14 changes: 14 additions & 0 deletions test/with_dummyserver/test_poolmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,20 @@ def test_redirect_without_preload_releases_connection(self) -> None:
assert r._pool.num_connections == 1
assert len(http.pools) == 1

def test_303_redirect_makes_request_lose_body(self) -> None:
with PoolManager() as http:
response = http.request(
"POST",
f"{self.base_url}/redirect",
fields={
"target": f"{self.base_url}/headers_and_params",
"status": "303 See Other",
},
)
data = response.json()
assert data["params"] == {}
assert "Content-Type" not in HTTPHeaderDict(data["headers"])

def test_unknown_scheme(self) -> None:
with PoolManager() as http:
unknown_scheme = "unknown"
Expand Down

0 comments on commit 4e50fbc

Please sign in to comment.