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

[fjcuadrado]

trafficstars

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
}

[fjcuadrado]

Also I wrote the following workaround to fix the issue:

diff --git a/requests/cookies.py b/requests/cookies.py
index 56fccd9c..afe6f597 100644
--- a/requests/cookies.py
+++ b/requests/cookies.py
@@ -43,7 +43,13 @@ class MockRequest(object):
         return self.type

     def get_host(self):
-        return urlparse(self._r.url).netloc
+        url_parts = urlparse(self._r.url)
+        host = url_parts.netloc
+
+        if url_parts.scheme == 'https' and not url_parts.port:
+            host = f'{host}:443'
+
+        return host

     def get_origin_req_host(self):
         return self.get_host()

[nateprewitt]

Hi @fjcuadrado, this seems like a reasonable behavior change. We'll need to do a bit of digging to make sure there isn't anything in the cookie spec explicitly preventing this. I would assume that's not the case, but we'd need to do some validating before approving anything.

As far as approach goes, we already do something very similar for authorization headers here. We may be able to rely on a similar pattern for making these decisions.

[sethmlarson]

Adding some context to this issue, the current RFC for cookies removes mentions of the port parameter and instead has:

... cookies for a given host are shared across all the ports on that host ...

[fjcuadrado]

The problem is that the set_ok_port function of DefaultCookiePolicy from the http.cookiejar standard library module doesn’t follow what @sethmlarson said.

Instead, as you can see, it checks the port of the cookie is the same than the provided in the request host, using the 80 port as the default value when there is no port in the host.