Skip to content

Integrate with new draft cookie spec (draft-annevk-johannhof-httpbis-cookies/00+ε) #1807

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bvandersloot-mozilla
Copy link

@bvandersloot-mozilla bvandersloot-mozilla commented Jan 30, 2025

This adds algorithms to retrieve and store cookies via the new draft cookie spec, assuming we have some more partitioning arguments.

It is based on #1707, and in total it does the following:

  • lift samesite logic from 6265bis
  • append cookies to requests
  • pull cookies from responses
  • define partition keys for fetches
  • define when unpartitioned cookies cannot be used

This blocks on some HTML changes

This patch does the following on top of the work in #1707:

  • rebase to main

  • add logic for parsing and storing cookies

  • point to the IETF-hosted draft cookie spec

  • don't point to storage access API for has storage access, use a broken link instead

  • add a broken link to environment/ancestry

  • add a broken link for the request's initiator origin plumbed in from HTML. It'll be defined here, but we need to modify HTML so we can track it in the top.

  • add broken links to things that need to be added to HTML

  • fix some nits (e.g. "foo" -> "foo")

  • use [=secure context=] not scheme=https

  • use SameSite=None by default. Let's punt on that for now, given the current state of implementations and lack of clear path forward.

  • At least two implementers are interested (and none opposed):
    [x] Mozilla
    [x] Apple
    [x] Google

  • Tests are written and can be reviewed and commented upon at:

    • Tests already exist for much of this in cookies/samesite. Coverage has not been analyzed closely.
  • Implementation bugs are filed:

    • Chromium: n/a
    • Gecko: n/a
    • WebKit: n/a
  • MDN issue is filed: n/a

  • The top of this comment includes a clear commit message to use.


Preview | Diff

@bvandersloot-mozilla
Copy link
Author

@DCtheTall @annevk

@bvandersloot-mozilla bvandersloot-mozilla changed the title Integrate with new draft cookie spec (draft-annevk-johannhof-httpbis-cookies/00++) Integrate with new draft cookie spec (draft-annevk-johannhof-httpbis-cookies/00+ε) Jan 30, 2025
bvandersloot-mozilla added a commit to bvandersloot-mozilla/html that referenced this pull request Feb 4, 2025
This is a concept that originated in the Storage Access API, where it
has been stuck because of spec issues between Fetch and 6265bis. To
un-logjam this, I've started whatwg/fetch#1807.
That depends on this bit existing.

This patch adds the bit, which remains false, and does nothing.
bvandersloot-mozilla added a commit to bvandersloot-mozilla/html that referenced this pull request Feb 4, 2025
To un-logjam the cookie layering work, I've started whatwg/fetch#1807.
That depends on this info to be piped into Fetch so we can
actually specify in WHATWG what SameSite=Strict means.

This patch plumbs that through on top-level navigatable fetches.

This doesn't build because it relies upon the corresponding
patch in Fetch. Let me know to land these.
@bvandersloot-mozilla
Copy link
Author

This should be good, assuming whatwg/html#10991, whatwg/html#10990, and whatwg/html#8036 all land.

@johannhof
Copy link
Member

I think this PR series looks great to me overall, but I also think we should wait for HTTP WG adoption of the new cookies spec before seriously moving forward with this (should do the reviews now, though!)

@bvandersloot-mozilla
Copy link
Author

I've removed the partitioning bits from this, and will make them as a separate PR after we've merged this

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Thanks Ben for driving this forward. This definitely looks like the correct approach.

@bvandersloot-mozilla
Copy link
Author

Indentation is still a mess- I'll get to that next.

Copy link
Member

@johannhof johannhof left a comment

Choose a reason for hiding this comment

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

Having taken another look, this still looks great to me.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

You need to coordinate with @DCtheTall what additional calls into the IETF draft to make. This is not handling clearing cookies after storing, clearing cookies from UI, or clearing cookies periodically. We need to have some wording for all of that in the "Cookies" section.

@bvandersloot-mozilla
Copy link
Author

You need to coordinate with @DCtheTall what additional calls into the IETF draft to make. This is not handling clearing cookies after storing, clearing cookies from UI, or clearing cookies periodically. We need to have some wording for all of that in the "Cookies" section.

I think we need some cookie spec updates to be able to do the operations of clearing from UI and clearing periodically. I'll poke down the stack on that.

@annevk annevk mentioned this pull request May 19, 2025
3 tasks
annevk pushed a commit to bvandersloot-mozilla/html that referenced this pull request May 19, 2025
This is a concept that originated in the Storage Access API, where it
has been stuck because of spec issues between Fetch and 6265bis. To
un-logjam this, I've started whatwg/fetch#1807.
That depends on this bit existing.

This patch adds the bit, which remains false, and does nothing.
annevk pushed a commit to bvandersloot-mozilla/html that referenced this pull request May 20, 2025
This helps with the HTTP WG's layered cookies draft integration work. whatwg/fetch#1807 depends on this state being passed in so we can define SameSite=Strict properly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants