[6265bis] Refactor cookie retrieval algorithm to support non-HTTP APIs #1428

englehardt · 2021-03-09T19:07:05Z

This PR refactors the cookie retrieval algorithm that formerly lived in The Cookie Header section to a new Retrieval Model section that mirrors the more generic Storage Model section for cookie setting.

This didn't require too much legwork in the end. Of note, I've moved us over from working with a request-target to target URI. A request target may not be a full URI (i.e., the "origin form" is a path + query), but we want a full URI so we can call the retrieval algorithm with a Document's URI for non-HTTP APIs. This was mostly a drop-in replacement in the rest of the doc, but I did remove one section that elaborated on extracting a path from the different forms of a request target.

I'm wondering whether we want to keep the non-HTTP API section at all. This only specifies one type of non-HTTP API (i.e., those with an associated Document); I'm not sure if there are any other non-HTTP APIs that user agents implement? And that functionality is already specified in more detail in HTML here. I wonder whether it's better to just update HTML with a reference to the new Retrieval Algorithm once we release a new version of the draft. cc/ @annevk for input.

Addresses #1233, #769, and #1163.

miketaylr · 2021-03-09T19:33:28Z

draft-ietf-httpbis-rfc6265bis.md

+If a user agent does return cookies for a given call to a "non-HTTP" API with an
+associated Document, then the user agent MUST compute the cookie-string following
+the algorithm defined in {{retrieval-algorithm}, whereby the retrieval-uri is the
+associated Document's URL. The retrieval is same-site if the Document's "site for


Should we reference https://dom.spec.whatwg.org/#dom-document-url somewhere (for Document's URL)?

I don't think that's accurate. See the open issues against HTML. There's a document cookie URL that browsers use.

@annevk you mean whatwg/html#2641 and whatwg/html#332? Are there others?

Yeah, those seem the most relevant here. https://github.com/whatwg/html/labels/topic%3A%20cookie is the full list.

@annevk it seems like the best thing to do then is to get a cookie URL defined in HTML that captures these nuances. I've updated the reference here to assume that already exists, and can help get those changes into HTML.

draft-ietf-httpbis-rfc6265bis.md

annevk

It seems reasonable to move this lookup to HTML. (I hope we have someone willing to do that work though.) We probably also want to move the cookie store into Fetch eventually so partitioning and such can be defined better.

(There's also a proposal for a cookie API that would offer lower level access than a string and also from workers. That would also need to poke at the store directly.)

annevk · 2021-03-10T10:05:21Z

draft-ietf-httpbis-rfc6265bis.md

+If a user agent does return cookies for a given call to a "non-HTTP" API with an
+associated Document, then the user agent MUST compute the cookie-string following
+the algorithm defined in {{retrieval-algorithm}, whereby the retrieval-uri is the
+associated Document's URL. The retrieval is same-site if the Document's "site for


I don't think that's accurate. See the open issues against HTML. There's a document cookie URL that browsers use.

annevk · 2021-03-10T10:07:54Z

draft-ietf-httpbis-rfc6265bis.md

@@ -1629,6 +1649,8 @@ the user agent might wish to try using the UTF-8 character encoding {{RFC3629}}
 to decode the octet sequence. This decoding might fail, however, because not
 every sequence of octets is valid UTF-8.


This also does not match what actually happens. Only Chrome can have such bytes at the moment and Chrome drops the cookies it cannot serialize, but does serialize the others.

englehardt · 2021-03-10T16:19:46Z

It seems reasonable to move this lookup to HTML.

To be clear, I only meant the Non-HTTP API section. I think the actual retrieval algorithm should continue to live here. Let me know if you meant something different.

(I hope we have someone willing to do that work though.)

I can.

We probably also want to move the cookie store into Fetch eventually so partitioning and such can be defined better.

We'd like to keep this revision scoped to interop. Would you mind to file an issue with a little more detail on what parts you think should move to fetch?

annevk · 2021-03-10T16:43:26Z

Thanks for clarifying, I misunderstood. I don't really have strong opinions on how it's organized as long as all possible scenarios have clear steps to follow.

I guess there's a couple potential things with the lookup algorithm:

It needs some kind of argument to indicate whether the caller is a "non-HTTP" API (assuming you would remove that definition).
For document.cookie the serialization needs might be a bit more involved than what the current text implies.
Something like https://github.com/WICG/cookie-store probably has even more advanced needs (but also doesn't have broad support yet).

I'll file an issue on Fetch integration later.

chlily1

Thanks! This looks pretty good overall.

I think having analogous HTTP/non-HTTP subsections would also be good for the Storage Model section to explicitly spell out what events constitute "receiving a cookie", but that can be a separate change.

draft-ietf-httpbis-rfc6265bis.md

englehardt · 2021-05-04T21:14:46Z

I guess there's a couple potential things with the lookup algorithm:
1. It needs some kind of argument to indicate whether the caller is a "non-HTTP" API (assuming you would remove that definition).

Done. I decided to keep that section, but even still it makes sense to include reference to an argument in the algorithm.

2. For `document.cookie` the serialization needs might be a bit more involved than what the current text implies.

I filed #1502 for this. If you have more context to share, please do!

3. Something like https://github.com/WICG/cookie-store probably has even more advanced needs (but also doesn't have broad support yet).

This is out of scope.

chlily1

Thanks for the changes! Looking really good.

I just had one suggestion for refactoring the algorithm parameters slightly: if you make the URI, same-site status, and HTTP/non-HTTP bit all properties of the retrieval, you can just pass the cookie store and the retrieval into the algorithm, and refer to "the retrieval's URI", "the retrieval's same-site status" etc. I think it makes sense semantically to associate them with the retrieval since every retrieval must have those properties.

draft-ietf-httpbis-rfc6265bis.md

chlily1

Lgtm!

(also, adds the changelog for httpwg#1428)

Addresses issue #1301 - Consistently use "header field" (also, adds the changelog for #1428)

These tests were temporarily disabled due to an incompatibility between Firefox and Chromium. Now that this has been resolved by a spec PR (httpwg/http-extensions#1428), the tests can be re-enabled. Additionally, one of the tests was adjusted to reflect the behavior for redirects specified in httpwg/http-extensions#1348, which is enabled as part of --enable-experimental-web-platform-features. Bug: 1074441 Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d

These tests were temporarily disabled due to an incompatibility between Firefox and Chromium. Now that this has been resolved by a spec PR (httpwg/http-extensions#1428), the tests can be re-enabled. Additionally, one of the tests was adjusted to reflect the behavior for redirects specified in httpwg/http-extensions#1348, which is enabled as part of --enable-experimental-web-platform-features. Bug: 1074441 Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294 Reviewed-by: Steven Bingler <bingler@chromium.org> Commit-Queue: Lily Chen <chlily@chromium.org> Cr-Commit-Position: refs/heads/master@{#905421}

…=testonly Automatic update from web-platform-tests Re-enable SameSite cookie iframe WPTs These tests were temporarily disabled due to an incompatibility between Firefox and Chromium. Now that this has been resolved by a spec PR (httpwg/http-extensions#1428), the tests can be re-enabled. Additionally, one of the tests was adjusted to reflect the behavior for redirects specified in httpwg/http-extensions#1348, which is enabled as part of --enable-experimental-web-platform-features. Bug: 1074441 Change-Id: I7e1deadf0c080927cc328b54d300d5d418fa5c6d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3054294 Reviewed-by: Steven Bingler <bingler@chromium.org> Commit-Queue: Lily Chen <chlily@chromium.org> Cr-Commit-Position: refs/heads/master@{#905421} -- wpt-commits: d4deee9e5303f3c09f65bc0d40783cc18a30606e wpt-pr: 29792

englehardt added 3 commits March 9, 2021 10:16

Initial refactor of Cookie header section

e17614e

Refactor sections a bit

4910624

Remove whitespace

4be47be

englehardt marked this pull request as draft March 9, 2021 19:07

miketaylr reviewed Mar 9, 2021

View reviewed changes

annevk reviewed Mar 10, 2021

View reviewed changes

may --> MAY

c96d724

Address review comments

0f65519

annevk mentioned this pull request Mar 10, 2021

Cookies and Fetch #1430

Closed

chlily1 reviewed Mar 10, 2021

View reviewed changes

Revert whitespace change

1c3063e

englehardt added the 6265bis label Mar 15, 2021

englehardt added 3 commits April 30, 2021 17:31

Fix nits from review.

e22f47c

Address review comments

82fcd53

Fix mismatched brackets

88d44a4

englehardt mentioned this pull request May 4, 2021

Consider better specifying document.cookie serialization #1502

Closed

englehardt marked this pull request as ready for review May 4, 2021 21:21

englehardt requested a review from chlily1 May 4, 2021 21:22

chlily1 reviewed May 5, 2021

View reviewed changes

draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved

draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved

englehardt added 3 commits May 18, 2021 12:03

Address review comments

a6a6c66

Merge remote-tracking branch 'origin/main' into 6265-non-http-apis

0189876

Merge remote-tracking branch 'origin/main' into 6265-non-http-apis

973124f

chlily1 approved these changes May 18, 2021

View reviewed changes

englehardt merged commit bc06705 into main May 20, 2021

englehardt deleted the 6265-non-http-apis branch May 20, 2021 17:52

englehardt restored the 6265-non-http-apis branch May 20, 2021 18:56

miketaylr added a commit to miketaylr/http-extensions that referenced this pull request Jun 2, 2021

Issue httpwg#1301 - Add changelog entry

3badc2d

(also, adds the changelog for httpwg#1428)

chlily1 pushed a commit that referenced this pull request Jun 2, 2021

Fixes #1301 - Consistently use "header field" (#1527)

0988cf0

Addresses issue #1301 - Consistently use "header field" (also, adds the changelog for #1428)

This was referenced Jun 4, 2021

Specify how non-HTTP cookies should be exposed to sites #1233

Closed

Clarify SameSite behavior for non-HTTP API #769

Closed

document.cookie should not be able to access SameSite cookies in cross-site iframes #1163

Closed

mnot deleted the 6265-non-http-apis branch June 9, 2021 04:55

chromium-wpt-export-bot mentioned this pull request Jul 26, 2021

Re-enable SameSite cookie iframe WPTs web-platform-tests/wpt#29792

Merged

		@@ -1629,6 +1649,8 @@ the user agent might wish to try using the UTF-8 character encoding {{RFC3629}}
		to decode the octet sequence. This decoding might fail, however, because not
		every sequence of octets is valid UTF-8.

[6265bis] Refactor cookie retrieval algorithm to support non-HTTP APIs #1428

[6265bis] Refactor cookie retrieval algorithm to support non-HTTP APIs #1428

Uh oh!

Conversation

englehardt commented Mar 9, 2021

Uh oh!

miketaylr Mar 9, 2021

Choose a reason for hiding this comment

Uh oh!

annevk Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

englehardt Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

annevk Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

englehardt May 4, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

annevk Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

annevk Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

englehardt commented Mar 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

annevk commented Mar 10, 2021

Uh oh!

chlily1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

englehardt commented May 4, 2021

Uh oh!

chlily1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chlily1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

englehardt commented Mar 10, 2021 •

edited

Loading