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

[11.x] Fix Cookie::queue to support domain-specific cookies #53202

Open
wants to merge 2 commits into
base: 11.x
Choose a base branch
from

Conversation

Sajid-al-islam
Copy link

Description

This PR addresses the issue where setting two cookies using Cookie::queue(...) with the same name and path but different domains results in only one cookie being sent to the client, as the latter overwrites the prior. The issue is detailed in #53159.

Problem Description

The issue arises from how CookieJar currently stores queued cookies, only considering the cookie's name and path while ignoring the domain. The Symfony ResponseHeaderBag::setCookie() method, on the other hand, correctly differentiates cookies by domain in addition to name and path.

Solution

This PR updates CookieJar to take domains into account when queuing cookies. Specifically:

  • The queue method now stores queued cookies using the cookie's name, path, and domain as keys, ensuring cookies with the same name and path but different domains do not overwrite each other.

Testing

  • Added multiple test cases to verify that cookies with the same name but different domains or paths are queued and sent independently:
    1. Cookies with the same name and path but different domains are queued without overwriting each other.
    2. Cookies with the same name but different paths are handled correctly.
    3. Cookies with the same name, path, and domain result in the latter cookie overwriting the former (as expected).
    4. Verified behavior for secure, httpOnly, and other attributes.

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

Successfully merging this pull request may close these issues.

2 participants