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

Extend roadmap: cookies #423

Merged
merged 5 commits into from
Jul 31, 2023
Merged

Conversation

sadym-chromium
Copy link
Contributor

No description provided.

@jgraham
Copy link
Member

jgraham commented Jun 14, 2023

The big practical question for cookies is how to handle double keyed cookies (maybe more generally multi-keyed cookies) i.e. situations where the cookies sent depend both on the origin of the request and the origin of the request source. Current WebDriver entirely punts on this issue and just says "return the entire content of the cookie store", without any way to tell which cookies will actually apply to a given request, or set the key for cookies.

@sadym-chromium
Copy link
Contributor Author

TODO: allow cross-origin cookies access

@mathiasbynens
Copy link
Contributor

The big practical question for cookies is how to handle double keyed cookies (maybe more generally multi-keyed cookies) i.e. situations where the cookies sent depend both on the origin of the request and the origin of the request source.

For my own understanding: is this only about SameSite cookies, or does double-keying apply to more scenarios?

@OrKoN How does CDP handle this, if at all? CDP’s Network.getCookies seems to only accept a list of URLs, returning all applicable cookies for those URLs — allowing to specify only one out of two “keys”. https://chromedevtools.github.io/devtools-protocol/tot/Network/#method-getCookies

@OrKoN
Copy link
Contributor

OrKoN commented Jun 28, 2023

@mathiasbynens we should probably be using https://chromedevtools.github.io/devtools-protocol/tot/Storage/#method-getCookies which has partition keys (I assume that it is used for double-keying?)

@jgraham
Copy link
Member

jgraham commented Jun 28, 2023

See https://hacks.mozilla.org/2021/02/introducing-state-partitioning/ for example.

Basically it means that cookies are keyed not just on the destination origin (e.g. example.org only gets example.org cookies), but also on the source origin i.e. a request from example.net for an example.org resource doesn't see cookies for example.org that were set as part of a request from example.com (or any other origin).

Also, at least in Gecko, we have the concept of containers, which are used as an additional key for storage. It's unclear to me how we want this to work for WebDriver. For example if we want a feature to partition storage between different tests, using containers for that partitioning makes sense. But then should getting cookies return cookies for all containers, or require specifying which container to use? This might also apply to WebKit/Safari's profiles; I'm not sure (it doesn't apply to Firefox profiles because there's a 1:1 relationship between a browser process and a profile, but 1:many between a browser process and containers).

@lolaodelola
Copy link
Contributor

Hi @sadym-chromium, just wanted to know if you're waiting for anything in particular to get this merged? Since work on cookies is being discussed & definitely going to happen, it'd be good to have this in the main roadmap.

@thiagowfx
Copy link
Member

@lolaodelola FYI Maksim is on leave. You may want to reach out to @mathiasbynens privately in the meantime to discuss the next steps.

roadmap.md Outdated Show resolved Hide resolved
@mathiasbynens mathiasbynens marked this pull request as ready for review July 13, 2023 09:12
@mathiasbynens
Copy link
Contributor

I’ve marked the PR as ready for review now. I agree it would make sense to merge this in some form, since we already agreed that this is important and happening.

roadmap.md Outdated Show resolved Hide resolved
Copy link
Contributor

@OrKoN OrKoN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % suggestion to make scenario a bit more specific.

Co-authored-by: Alex Rudenko <OrKoN@users.noreply.github.com>
roadmap.md Outdated
@@ -93,7 +93,7 @@ This scenario loads a web page and prints it as a PDF. In spec terms, this invol

### Cookies

This scenario loads a web page and gets and sets its cookies. In spec terms, this involves:
This scenario loads a web page, reads the cookies set by the server and the page, updates and deletes some of the cookies, and triggers an action to verify that modified cookies are processed correctly. In spec terms, this involves:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't cover cookies for pages that aren't loaded. My understanding is that the need to load a page before interacting with its cookies is considered a significant limitation for WebDriver. It's also the source of much of the complexity here (since having a page loaded in a specific context answers a lot of the questions around keying, given that a particular browsing context will have a particular storage key).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any suggested edit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this addressed by the edit that happened after this comment was posted?

roadmap.md Outdated Show resolved Hide resolved
Co-authored-by: jgraham <james@hoppipolla.co.uk>
@OrKoN OrKoN requested a review from jgraham July 14, 2023 08:43
roadmap.md Outdated Show resolved Hide resolved
@mathiasbynens mathiasbynens merged commit c10c304 into w3c:main Jul 31, 2023
github-actions bot added a commit that referenced this pull request Jul 31, 2023
SHA: c10c304
Reason: push, by mathiasbynens

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

6 participants