psf / psf/requests

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

Open
#6,110 4 comments 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.