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

Cookie having a Domain cookie-attribute with empty string fails to be added to the cookie jar #6245

Open
ecclejau opened this issue Sep 22, 2022 · 1 comment

Comments

@ecclejau
Copy link

Using requests to access an API hosted on an application server we noticed that cookies were not added to the cookie jar in the session.
It seems that after an upgrade to the application server framework it adds a domain=; cookie-attribute to the cookies in the response.

Expected Result

The cookie to be added to the jar

Actual Result

The cookie is dropped.

Reproduction Steps

import unittest
from typing import Tuple
from unittest.mock import MagicMock

import requests
from hamcrest import assert_that, has_length


class TestMissingSessionCookieIssue(unittest.TestCase):
    @staticmethod
    def __make_mocks(cookie_header_value: str) -> Tuple[MagicMock, MagicMock]:
        message = MagicMock()
        message.get_all.side_effect = [[], [cookie_header_value]]

        response = MagicMock()
        response.info.return_value = message

        request = MagicMock()
        request.get_full_url.return_value = "http://localhost:1234/page"
        request.origin_req_host = "localhost"

        return request, response

    def test_cookie_with_no_domain(self) -> None:
        request, response = self.__make_mocks("SESSION_ID=12345678; HttpOnly; Path=/page; SameSite=Strict")
        session = requests.session()

        session.cookies.extract_cookies(response, request)

        assert_that(session.cookies, has_length(1))

    def test_cookie_with_empty_domain(self) -> None:
        request, response = self.__make_mocks("SESSION_ID=12345678; domain=; path=/page; samesite=strict; httponly")
        session = requests.session()

        session.cookies.extract_cookies(response, request)

        assert_that(session.cookies, has_length(1))

Requirements:

  • PyHamcrest==2.0.2
  • requests==2.28.1
  • Python 3.9

System Information

$ python -m requests.help
{
  "chardet": {
    "version": null
  },
  "charset_normalizer": {
    "version": "2.0.12"
  },
  "cryptography": {
    "version": ""
  },
  "idna": {
    "version": "3.4"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.9.5"
  },
  "platform": {
    "release": "5.14.0-1052-oem",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "",
    "version": null
  },
  "requests": {
    "version": "2.28.1"
  },
  "system_ssl": {
    "version": "1010106f"
  },
  "urllib3": {
    "version": "1.26.12"
  },
  "using_charset_normalizer": true,
  "using_pyopenssl": false
}
@eth7
Copy link

eth7 commented Apr 15, 2023

The issue is that the set_ok_domain function from the DefaultCookiePolicy from the cookiejar library does not allow a mismatch between request host and cookie domain.
In this code snippet below, the set_ok method has been overridden to always return True, effectively allowing any cookie to be set regardless of the request host and cookie domain match.
It's important to keep in mind that this should not be used in production.
You can however verify that it is indeed the DefaultCookiePolicy, which prevents such use-cases, as the empty string in the domain.

import requests
from http import cookiejar


class DangerouslyAllowEverything(cookiejar.CookiePolicy):
    def set_ok(self, *args, **kwargs):
        return True

    netscape = True
    rfc2965 = False


session = requests.session()
session.cookies.set_policy(DangerouslyAllowEverything())
response = session.get("http://localhost")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants