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.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 48/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Stale
- Tech stack
- python
- Domain
- networking
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.
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
}
- Dominant language
- Python
- Stars
- 54.7k
- Forks
- 10.8k
- Avg merge
- 18m
- Merged PRs (30d)
- 1
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing 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.
More from psf/requests
-
Difficulty 1/5 Under an hour Newbie friendliness 85/100
-
`iter_lines(decode_unicode=True, delimiter=<str>)` raises `TypeError` when `Response.encoding` is `None`May be free again A pull request for this issue was closed without being merged. Open
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
-
get_encoding_from_headers treats any content type containing 'text' as text/* (ISO-8859-1)Possibly taken @suyashgitt claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
Invalid `params` type (e.g. int) raises misleading "TypeError: Cannot mix str and non-str arguments"Open
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
Similar issues
-
plot_graph crashes when a mesh node has no m2m edgePossibly taken @tygvi claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
mllam/neural-lam#883 · 1 comment ·
Maintainers usually reply within 2 days
-
enhancement possible-duplicate
Difficulty 1/5 1-3 hours Newbie friendliness 78/100
AOSSIE-Org/PictoPy#1715 · 2 comments ·
Maintainers usually reply within 6 days
-
SQLite connector omits generated columns from the catalog, and drops relationships sourced on themOpen
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
NVIDIA/auto-ontology#511 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
ethereum/execution-specs#3996 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
cpp-best-practices/project_validator#267 ·
Maintainers usually reply within 1 day