Cookies that include a port flag set to 443 aren't stored if they came from an https location without the 443 port in the URI
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 54.3k
- Forks
- 10.5k
- Avg merge
- 16h 43m
- Merged PRs (30d)
- 3
Description
When performing a request to an https location that doesn't include a port explicitly (for example: https://localhost) and that location returns a cookie that includes a port flag set to 443, that cookie isn't stored into the cookie jar. However, this kind of cookie is stored into the cookie jar, when the port is set explicitly in the https location (for example: https://localhost:443).
Expected Result
A cookie that includes a port flag set to 443 should be stored into the cookie jar when the request is performed to an https location that doesn't include a port.
Reproduction Steps
I hope you don't mind that I've moved the Actual Result below this section. I did it because I've written a test-suite to reproduce the issue and the result of running that test-suite has been pasted in the Actual Result section.
import http
import pytest
import requests
from io import BytesIO
from urllib3.response import HTTPHeaderDict, HTTPResponse
class OriginalResponseShim:
def __init__(self, headers):
self.msg = headers
def isclosed(self):
return True
def close(self):
return
def make_http_response(
method,
url,
status=requests.codes.ok,
headers=None
):
http_headers = HTTPHeaderDict()
if headers is not None:
http_headers.extend(headers)
def _make_http_response(adapter, request, *args, **kwargs):
http_response = HTTPResponse(
status=status,
reason=http.client.responses.get(status, None),
body=BytesIO(b''),
headers=http_headers,
original_response=OriginalResponseShim(http_headers),
preload_content=False,
)
return adapter.build_response(request, http_response)
return _make_http_response
class TestCookies:
@pytest.mark.parametrize('url', [
'http://localhost',
'http://localhost:80',
'http://localhost:8080',
'https://localhost',
'https://localhost:443',
'https://localhost:8080',
])
def test_cookie_without_port(self, mocker, url):
mocker.patch(
'requests.adapters.HTTPAdapter.send',
new=make_http_response(
'GET',
url,
headers={
'set-cookie': 'cookie=value',
},
)
)
session = requests.Session()
resp = session.request('GET', url)
assert resp.status_code == requests.codes.ok
assert 'set-cookie' in resp.headers
assert 'cookie' in session.cookies
cookie = next(iter(session.cookies))
assert cookie.name == 'cookie'
assert cookie.value == 'value'
assert cookie.port is None
@pytest.mark.parametrize('url, port', [
('http://localhost', 80),
('http://localhost:80', 80),
('http://localhost:443', 443),
('http://localhost:8080', 8080),
('https://localhost', 443),
('https://localhost:80', 80),
('https://localhost:443', 443),
('https://localhost:8080', 8080),
])
def test_cookie_with_port(self, mocker, url, port):
mocker.patch(
'requests.adapters.HTTPAdapter.send',
new=make_http_response(
'GET',
url,
headers={
'set-cookie': f'cookie=value; port={port}',
},
)
)
session = requests.Session()
resp = session.request('GET', url)
assert resp.status_code == requests.codes.ok
assert 'set-cookie' in resp.headers
assert 'cookie' in session.cookies
cookie = next(iter(session.cookies))
assert cookie.name == 'cookie'
assert cookie.value == 'value'
assert cookie.port == f'{port}'
Actual Result
platform darwin -- Python 3.10.2, pytest-6.2.5, py-1.11.0, pluggy-1.0.0
rootdir: /Users/fran/code/requests, configfile: pytest.ini
plugins: httpbin-1.0.0, mock-2.0.0, cov-3.0.0
collected 14 items
tests/test_cookies.py ..........F... [100%]
======================================== FAILURES ========================================
________________ TestCookies.test_cookie_with_port[https://localhost-443] ________________
self = <tests.test_cookies.TestCookies object at 0x10e309ff0>
mocker = <pytest_mock.plugin.MockFixture object at 0x10e30a290>, url = 'https://localhost'
port = 443
@pytest.mark.parametrize('url, port', [
('http://localhost', 80),
('http://localhost:80', 80),
('http://localhost:443', 443),
('http://localhost:8080', 8080),
('https://localhost', 443),
('https://localhost:80', 80),
('https://localhost:443', 443),
('https://localhost:8080', 8080),
])
def test_cookie_with_port(self, mocker, url, port):
mocker.patch(
'requests.adapters.HTTPAdapter.send',
new=make_http_response(
'GET',
url,
headers={
'set-cookie': f'cookie=value; port={port}',
},
)
)
session = requests.Session()
resp = session.request('GET', url)
assert resp.status_code == requests.codes.ok
assert 'set-cookie' in resp.headers
> assert 'cookie' in session.cookies
E AssertionError: assert 'cookie' in <RequestsCookieJar[]>
E + where <RequestsCookieJar[]> = <requests.sessions.Session object at 0x10e309b10>.cookies
tests/test_cookies.py:105: AssertionError
System Information
$ python -m requests.help
{
"chardet": {
"version": null
},
"charset_normalizer": {
"version": "2.0.12"
},
"cryptography": {
"version": ""
},
"idna": {
"version": "3.3"
},
"implementation": {
"name": "CPython",
"version": "3.10.2"
},
"platform": {
"release": "21.4.0",
"system": "Darwin"
},
"pyOpenSSL": {
"openssl_version": "",
"version": null
},
"requests": {
"version": "2.27.1"
},
"system_ssl": {
"version": "101010ef"
},
"urllib3": {
"version": "1.26.9"
},
"using_charset_normalizer": true,
"using_pyopenssl": false
}
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the reproducer in tests/test_cookies.py and run the parametrized cookie tests to confirm the failure for https://localhost with port=443. Trace how the Session transfers response cookies into its cookie jar, then make the failing case pass without regressing the other URL and port combinations.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100