Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle cookies on redirect. #3086

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions httpx/_client.py
Expand Up @@ -346,7 +346,8 @@ def build_request(
"""
url = self._merge_url(url)
headers = self._merge_headers(headers)
cookies = self._merge_cookies(cookies)
user_cookies = Cookies.for_url(url, cookies)
cookies = self._merge_cookies(user_cookies)
params = self._merge_queryparams(params)
extensions = {} if extensions is None else extensions
if "timeout" not in extensions:
Expand All @@ -366,6 +367,7 @@ def build_request(
params=params,
headers=headers,
cookies=cookies,
user_cookies=user_cookies,
extensions=extensions,
)

Expand Down Expand Up @@ -462,12 +464,13 @@ def _build_redirect_request(self, request: Request, response: Response) -> Reque
url = self._redirect_url(request, response)
headers = self._redirect_headers(request, url, method)
stream = self._redirect_stream(request, method)
cookies = Cookies(self.cookies)
cookies = self._merge_cookies(request.user_cookies)
return Request(
method=method,
url=url,
headers=headers,
cookies=cookies,
user_cookies=request.user_cookies,
stream=stream,
extensions=request.extensions,
)
Expand Down
45 changes: 38 additions & 7 deletions httpx/_models.py
Expand Up @@ -316,6 +316,7 @@ def __init__(
params: QueryParamTypes | None = None,
headers: HeaderTypes | None = None,
cookies: CookieTypes | None = None,
user_cookies: Cookies | None = None,
content: RequestContent | None = None,
data: RequestData | None = None,
files: RequestFiles | None = None,
Expand All @@ -334,6 +335,12 @@ def __init__(
self.headers = Headers(headers)
self.extensions = {} if extensions is None else extensions

# Original cookies passed by the client code, extended with domain.
# Used by follow-up requests, when follow_redirects == True
self.user_cookies = (
Cookies.for_url(self.url, cookies) if user_cookies is None else user_cookies
)

if cookies:
Cookies(cookies).set_cookie_header(self)

Expand Down Expand Up @@ -436,7 +443,7 @@ def __getstate__(self) -> dict[str, typing.Any]:
return {
name: value
for name, value in self.__dict__.items()
if name not in ["extensions", "stream"]
if name not in ["extensions", "stream", "user_cookies"]
}

def __setstate__(self, state: dict[str, typing.Any]) -> None:
Expand Down Expand Up @@ -1032,6 +1039,21 @@ def __init__(self, cookies: CookieTypes | None = None) -> None:
else:
self.jar = cookies

@classmethod
def for_url(cls, url: URL, cookies: CookieTypes | None = None) -> "Cookies":
if cookies is None or isinstance(cookies, (Cookies, CookieJar)):
return cls(cookies)
if isinstance(cookies, Mapping):
cookies = cookies.items() # type: ignore

domain = url.host
secure = url.scheme == "https"
cookies_obj = Cookies()

for name, value in cookies: # type: ignore
cookies_obj.set(name, value, domain, secure=secure)
return cookies_obj

def extract_cookies(self, response: Response) -> None:
"""
Loads any cookies based on the response `Set-Cookie` headers.
Expand All @@ -1048,7 +1070,14 @@ def set_cookie_header(self, request: Request) -> None:
urllib_request = self._CookieCompatRequest(request)
self.jar.add_cookie_header(urllib_request)

def set(self, name: str, value: str, domain: str = "", path: str = "/") -> None:
def set(
self,
name: str,
value: str,
domain: str = "",
path: str = "/",
secure: bool = False,
) -> None:
"""
Set a cookie value by name. May optionally include domain and path.
"""
Expand All @@ -1063,7 +1092,7 @@ def set(self, name: str, value: str, domain: str = "", path: str = "/") -> None:
"domain_initial_dot": domain.startswith("."),
"path": path,
"path_specified": bool(path),
"secure": False,
"secure": secure,
"expires": None,
"discard": True,
"comment": None,
Expand All @@ -1080,6 +1109,7 @@ def get( # type: ignore
default: str | None = None,
domain: str | None = None,
path: str | None = None,
secure: bool | None = None,
) -> str | None:
"""
Get a cookie by name. May optionally include domain and path
Expand All @@ -1090,10 +1120,11 @@ def get( # type: ignore
if cookie.name == name:
if domain is None or cookie.domain == domain:
if path is None or cookie.path == path:
if value is not None:
message = f"Multiple cookies exist with name={name}"
raise CookieConflict(message)
value = cookie.value
if secure is None or cookie.secure == secure:
if value is not None:
message = f"Multiple cookies exist with name={name}"
raise CookieConflict(message)
value = cookie.value

if value is None:
return default
Expand Down
129 changes: 129 additions & 0 deletions tests/client/test_redirects.py
Expand Up @@ -445,3 +445,132 @@ async def test_async_invalid_redirect():
await client.get(
"http://example.org/invalid_redirect", follow_redirects=True
)


def cookies_redirects(request: httpx.Request) -> httpx.Response:
if request.url.scheme not in ("http", "https"):
raise httpx.UnsupportedProtocol(f"Scheme {request.url.scheme!r} not supported.")

if request.url.path == "/redir_echo":
status_code = httpx.codes.MOVED_PERMANENTLY
headers = {"location": "https://example.com/echo"}
return httpx.Response(status_code, headers=headers)

if request.url.path == "/redir_double":
status_code = httpx.codes.MOVED_PERMANENTLY
headers = {"location": "https://not-example.com/redir_echo"}
return httpx.Response(status_code, headers=headers)

elif request.url.path == "/redir_other":
status_code = httpx.codes.MOVED_PERMANENTLY
headers = {"location": "https://not-example.com/echo"}
return httpx.Response(status_code, headers=headers)

elif request.url.path == "/redir_http":
status_code = httpx.codes.MOVED_PERMANENTLY
headers = {"location": "http://example.com/echo"}
return httpx.Response(status_code, headers=headers)

elif request.url.path == "/echo":
data = {"cookies": request.headers.get("cookie")}
return httpx.Response(200, json=data)

return httpx.Response(404, html="<html><body>Not found!</body></html>")


def test_cookies_dont_cross_domain_on_redirect():
cookies = httpx.Cookies()
cookies.set("with_domain", "example-value", domain="example.com")

client = httpx.Client(
transport=httpx.MockTransport(cookies_redirects),
follow_redirects=True,
cookies=cookies,
)

response = client.get("http://example.com/redir_other")
assert response.status_code == 200
assert response.json() == {"cookies": None}


def test_dict_cookies_dont_cross_domain_on_redirect():
cookies = {
"with_domain": "example-value",
}

client = httpx.Client(
transport=httpx.MockTransport(cookies_redirects),
follow_redirects=True,
)

with pytest.warns(DeprecationWarning):
response = client.get("http://example.com/redir_other", cookies=cookies)
assert response.status_code == 200
assert response.json() == {"cookies": None}


def test_dict_cookies_follow_redirect():
cookies = {
"with_domain": "example-value",
}

client = httpx.Client(
transport=httpx.MockTransport(cookies_redirects),
follow_redirects=True,
)

with pytest.warns(DeprecationWarning):
response = client.get("http://example.com/redir_echo", cookies=cookies)
assert response.status_code == 200
assert response.json() == {"cookies": "with_domain=example-value"}


def test_request_cookies_dont_cross_domain_on_redirect():
cookies = httpx.Cookies()
cookies.set("with_domain", "example-value", domain="example.com")

client = httpx.Client(
transport=httpx.MockTransport(cookies_redirects),
follow_redirects=True,
)

with pytest.warns(DeprecationWarning):
response = client.get(
"http://example.com/redir_other",
cookies=cookies,
)
assert response.status_code == 200
assert response.json() == {"cookies": None}


def test_request_cookies_follow_double_redirect_across_hosts():
cookies = {
"with_domain": "example-value",
}

with httpx.Client(
transport=httpx.MockTransport(cookies_redirects), follow_redirects=True
) as client:
with pytest.warns(DeprecationWarning):
response = client.get("http://example.com/redir_double", cookies=cookies)

assert response.status_code == 200
assert response.json() == {"cookies": "with_domain=example-value"}

intermediate_response = response.history[1]
assert "Cookie" not in intermediate_response.request.headers


def test_request_cookies_dont_follow_on_http_downgrade():
cookies = {
"with_domain": "example-value",
}

with httpx.Client(
transport=httpx.MockTransport(cookies_redirects), follow_redirects=True
) as client:
with pytest.warns(DeprecationWarning):
response = client.get("https://example.com/redir_http", cookies=cookies)

assert response.status_code == 200
assert response.json() == {"cookies": None}
40 changes: 40 additions & 0 deletions tests/models/test_cookies.py
Expand Up @@ -52,6 +52,46 @@ def test_cookies_with_domain_and_path():
assert len(cookies) == 0


def test_cookies_for_url_cookies():
cookies = httpx.Cookies()
cookies.set("name", "value")
assert httpx.Cookies.for_url(httpx.URL("http://example.com/"), cookies) == cookies


def test_cookies_for_url_cookiejar():
cookies = httpx.Cookies()
cookies.set("name", "value")
assert (
httpx.Cookies.for_url(httpx.URL("http://example.com/"), cookies.jar) == cookies
)


def test_cookies_for_url_http_dict():
cookies = httpx.Cookies.for_url(httpx.URL("http://example.com/"), {"name": "value"})
assert cookies.get("name", domain="example.com") == "value"


def test_cookies_for_url_http_list():
cookies = httpx.Cookies.for_url(
httpx.URL("http://example.com/"), [("name", "value")]
)
assert cookies.get("name", domain="example.com") == "value"


def test_cookies_for_url_https_dict():
cookies = httpx.Cookies.for_url(
httpx.URL("https://example.com/"), {"name": "value"}
)
assert cookies.get("name", domain="example.com", secure=True) == "value"


def test_cookies_for_url_https_list():
cookies = httpx.Cookies.for_url(
httpx.URL("https://example.com/"), [("name", "value")]
)
assert cookies.get("name", domain="example.com", secure=True) == "value"


def test_multiple_set_cookie():
jar = http.cookiejar.CookieJar()
headers = [
Expand Down