Re: [whatwg/fetch] Integrate with new draft cookie spec (draft-annevk-johannhof-httpbis-cookies/00+ε) (PR #1807)

@annevk commented on this pull request.

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

> @@ -2226,31 +2248,38 @@ or "<code>object</code>".
 <hr>
 
 <div algorithm>
-<p>A <a for=/>request</a> <var>request</var> has a
-<dfn for=request id=concept-request-tainted-origin>redirect-tainted origin</dfn> if these steps
-return true:
+<p>A <a for=/>request</a> has a <dfn for=request id=concept-request-redirect-taint>redirect-taint</dfn>,
+which is "<code>None</code>", "<code>Cross-Origin</code>", or "<code>Cross-Site</code>".
+<p>To get <a for=/>request</a> <var>request</var>'s <a>redirect-taint</a>:

Enum values, including internal enum values, are lowercase.

>     <li><p>If <var>url</var>'s <a for=url>origin</a> is not <a>same origin</a> with
    <var>lastURL</var>'s <a for=url>origin</a> and <var>request</var>'s <a for=request>origin</a> is
-   not <a>same origin</a> with <var>lastURL</var>'s <a for=url>origin</a>, then return true.
+   not <a>same origin</a> with <var>lastURL</var>'s <a for=url>origin</a>,
+   then let <var>crossOriginTaint</var> be "<code>Cross-Origin</code>"..

Should this be "same-site" or "same-site-cross-origin"?

Also, you overwrite a variable with "set" and "to". "Let ... be" is only for initializing.

Single dot at the end here too.

> @@ -2489,6 +2518,9 @@ this is also tracked internally using the request's <a for=request>timing allow
 <p>A <a for=/>response</a> has an associated <dfn for=response>has-cross-origin-redirects</dfn>
 (a boolean), which is initially false.
 
+<p>A <a for=/>response</a> has an associated <dfn for=response>has-cross-site-redirects</dfn>
+(a boolean), which is initially false.

Hmm, it strikes me that we should turn these two into a "redirect taint" enum for responses?

> @@ -3292,6 +3324,72 @@ through TLS using ALPN. The protocol cannot be spoofed through HTTP requests in
 
 <h2 id=http-extensions>HTTP extensions</h2>
 
+<h3 id=cookie-header>`<code>Cookie</code>` header</h3>
+
+<p>The `<dfn export http-header id=http-cookie><code>Cookie</code></dfn>`

I don't think it's appropriate to `<dfn>` it. We should do this more akin to `Content-Type`.

> @@ -3292,6 +3324,72 @@ through TLS using ALPN. The protocol cannot be spoofed through HTTP requests in
 
 <h2 id=http-extensions>HTTP extensions</h2>
 
+<h3 id=cookie-header>`<code>Cookie</code>` header</h3>
+
+<p>The `<dfn export http-header id=http-cookie><code>Cookie</code></dfn>`
+request <a for=/>header</a> allows the request to carry locally stored state, such as user credentials.
+
+<div algorithm>
+<p>To <dfn id=append-a-request-cookie-header>append a request `<code>Cookie</code>` header</dfn>,
+given a <a for=/>request</a> <var>request</var>, run these steps:

```suggestion
given a <a for=/>request</a> <var>request</var>:
```
Modern style is just "To DO X given Y _y_:"

> @@ -3292,6 +3324,72 @@ through TLS using ALPN. The protocol cannot be spoofed through HTTP requests in
 
 <h2 id=http-extensions>HTTP extensions</h2>
 
+<h3 id=cookie-header>`<code>Cookie</code>` header</h3>
+
+<p>The `<dfn export http-header id=http-cookie><code>Cookie</code></dfn>`
+request <a for=/>header</a> allows the request to carry locally stored state, such as user credentials.
+
+<div algorithm>
+<p>To <dfn id=append-a-request-cookie-header>append a request `<code>Cookie</code>` header</dfn>,
+given a <a for=/>request</a> <var>request</var>, run these steps:
+  <ol>
+     <li><p>Let |sameSite| be the result of [=determining the same-site mode=] for <var>request</var>.

Indentation is wrong here.

> @@ -3292,6 +3324,72 @@ through TLS using ALPN. The protocol cannot be spoofed through HTTP requests in
 
 <h2 id=http-extensions>HTTP extensions</h2>
 
+<h3 id=cookie-header>`<code>Cookie</code>` header</h3>
+
+<p>The `<dfn export http-header id=http-cookie><code>Cookie</code></dfn>`
+request <a for=/>header</a> allows the request to carry locally stored state, such as user credentials.
+
+<div algorithm>
+<p>To <dfn id=append-a-request-cookie-header>append a request `<code>Cookie</code>` header</dfn>,
+given a <a for=/>request</a> <var>request</var>, run these steps:
+  <ol>
+     <li><p>Let |sameSite| be the result of [=determining the same-site mode=] for <var>request</var>.
+     <li><p>Let |isSecure| be false.
+     <li><p>If <var>request</var>'s <a for=request>client</a> is a <a>secure context</a>, then set |isSecure| to true.

Isn't it strictly based on the URL scheme?

> +  <ol>
+     <li><p>Let |sameSite| be the result of [=determining the same-site mode=] for <var>request</var>.
+     <li><p>Let |isSecure| be false.
+     <li><p>If <var>request</var>'s <a for=request>client</a> is a <a>secure context</a>, then set |isSecure| to true.
+     <li><p>Let |httpOnlyAllowed| be true.
+     <p class=note>Fetch implies that the request is http-only, as opposed to document.cookie
+     <li><p>Let |cookies| be the result of running <a>retrieve cookies</a> given
+      |isSecure|,
+      <var>request</var>'s <a for=request>current URL</a>'s <a for=url>host</a>,
+      <var>request</var>'s <a for=request>current URL</a>'s <a for=url>path</a>,
+      |httpOnlyAllowed|, and
+      |sameSite|
+
+     <p class=note>It is expected that the cookie store returns an ordered list of cookies
+     <li>If |cookies| <a for="list">is empty</a>, then return.
+     <li>Let |value| be the result of running <a>serialize cookies</a> given |cookies|.

```suggestion
     <li>Let |value| be the result of <a>serializing cookies</a> given |cookies|.
```

> @@ -3292,6 +3324,72 @@ through TLS using ALPN. The protocol cannot be spoofed through HTTP requests in
 
 <h2 id=http-extensions>HTTP extensions</h2>
 
+<h3 id=cookie-header>`<code>Cookie</code>` header</h3>

I think we need a dedicated Cookies section and the the Cookie header can be one part of that.

The other part will need to tackle https://github.com/johannhof/draft-annevk-johannhof-httpbis-cookies/pull/13. In particular defining requirements for user agents around clearing cookies and how those changes are propagated across documents. At least I think we want all of the architecture to be handled by Fetch and then have HTML, Cookie Store API, and Service Workers (although maybe no Service Workers changes are required? I don't recall) call into that.

>  </pre>
 
 <pre class=biblio>
 {
+    "COOKIES": {
+      "authors": ["Johann Hofmann", "Anne Van Kesteren"],

```suggestion
      "authors": ["Johann Hofmann", "Anne van Kesteren"],
```

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/1807#pullrequestreview-2666905822
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/fetch/pull/1807/review/2666905822@github.com>