Define preconnect and dns-prefetch with CSP by noamr · Pull Request #1620 · whatwg/fetch

noamr · 2023-03-20T17:10:31Z

Preconnect & dns-prefetch test a fake prefetch request for CSP, and then obtain a connection/resolve an origin.

At least two implementers are interested (and none opposed):
- …
- …
Tests are written and can be reviewed and commented upon at:
- …
Implementation bugs are filed:
- Chromium: …
- Gecko: …
- WebKit: …
- Deno (not for CORS changes): …
MDN issue is filed: …

(See WHATWG Working Mode: Changes for more details.)

fetch.bs

annevk

It's unfortunate these have to be separate algorithms. I was hoping we could do CSP checks as part of the normal "obtain a connection" routine.

fetch.bs

noamr · 2023-04-24T10:12:34Z

It's unfortunate these have to be separate algorithms. I was hoping we could do CSP checks as part of the normal "obtain a connection" routine.

We can do that but then there would be double CSP checks, one for the request and one for the "fake" request, or we can add a flag to obtain/resolve of whether to perform the CSP check.

annevk · 2023-05-04T08:13:27Z

fetch.bs

-  and <a href="https://en.wikipedia.org/wiki/Proxy_auto-config">proxy auto-config (PAC)</a> come
-  into play. The "<code>DIRECT</code>" value means to not use a proxy for this particular
-  <var>url</var>.
+ <li><p>Let <var>proxies</var> of calling <a>resolve proxies</a>.


This is missing words.

What words are missing? The note was moved to resolve proxies.

annevk · 2023-05-04T08:14:01Z

fetch.bs

+<p>To <dfn>resolve proxies</dfn>, return the result of finding proxies for <var>url</var> in an
+<a>implementation-defined</a> manner. If there are no proxies, return « "<code>DIRECT</code>" ».


Where does url come from here? If it needs to be an argument, let's make it explicit.

annevk · 2023-05-04T08:14:45Z

fetch.bs


+<h3 id=preemptive-connections>Preemptive connections</h3>
+
+<div algorithm="preemmptively-obtain-a-connection">


Does algorithm need this value? Normally it works without.

annevk · 2023-05-04T08:15:10Z

fetch.bs

+  <p><a>Check CSP for preemptive operation</a> with <var>environment</var> and <var>url</var>. If
+  that returns <b>allowed</b>, <a>obtain a connection</a> with <var>key</var>, <var>url</var> and
+  <var>credentials</var>.


Oxford comma.

annevk · 2023-05-04T08:16:24Z

fetch.bs

+<div algorithm="check-preemptive-operation-csp">
+<p>To <dfn>check CSP for preemptive operation</dfn>, given an
+<a>environment settings object</a> <var>environment</var>, and a <a for=/>URL</a> <var>url</var>,
+run these steps:


Perhaps this can have a more descriptive name such as "does CSP allow a connection" and we normalize the return value from CSP to true/false while we're here.

Seems this went unaddressed?

fetch.bs

annevk · 2023-05-08T10:50:19Z

fetch.bs

+</ol>
+</div>
+
+<div algorithm="check-preemptive-operation-csp">


This attribute value isn't needed.

annevk · 2023-05-08T10:50:44Z

fetch.bs

+</ol>
+</div>
+
+<div>


This needs an algorithm attribute.

annevk · 2023-05-08T10:51:48Z

fetch.bs

+</div>
+
+<div algorithm="check-preemptive-operation-csp">
+<p>To <dfn>Should connection be allowed by Content Security Policy?</dfn>, given an


This algorithm needs a better name. E.g., "determine if Content Security allows connection". The name shouldn't have a question mark and should read naturally after "To" if you're using to to lead it in.

annevk · 2023-05-08T10:52:00Z

fetch.bs

+<div algorithm="check-preemptive-operation-csp">
+<p>To <dfn>Should connection be allowed by Content Security Policy?</dfn>, given an
+<a>environment settings object</a> <var>environment</var>, and a <a for=/>URL</a> <var>url</var>,
+run these steps:


No need for "run these steps" when you lead it in with "To".

annevk · 2023-05-08T10:52:41Z

fetch.bs

+ <li><p>Return the result of running <a>should request be blocked by Content Security Policy?</a>
+ given <var>request</var>.


We should put the result in a variable first and then convert that to a boolean.

See whatwg/html#9035

Co-authored-by: Valentin Gosu <[email protected]>

Co-authored-by: Anne van Kesteren <[email protected]>

annevk

Apologies for the delay here. Found a couple more issues to address.

annevk · 2023-09-28T11:48:23Z

fetch.bs

+  <p class=note>The user agent can attempt to initiate a preconnect and perform the full
+  connection handshake (DNS+TCP for HTTP, and DNS+TCP+TLS for HTTPS origins) whenever possible, but
+  is allowed to elect to perform a partial handshake (DNS only for HTTP, and DNS or DNS+TCP for
+  HTTPS origins), or skip it entirely, due to resource constraints or other reasons.</p>
+
+  <p class=note>The optimal number of connections per origin is dependent on the negotiated
+  protocol, users current connectivity profile, available device resources, global connection
+  limits, and other context specific variables. As a result, the decision for how many connections
+  should be opened is deferred to the user agent.</p>


These notes feel like normative material. One even contains "should"! In the step above where we obtain the connection perhaps we should have a "with these caveats" ending that then leads into some bullet points?

fetch.bs

Co-authored-by: Anne van Kesteren <[email protected]>

valenting reviewed Mar 20, 2023

View reviewed changes

fetch.bs Outdated Show resolved Hide resolved

noamr mentioned this pull request Mar 21, 2023

Govern dns-prefetch and preconnect with CSP whatwg/html#9035

Open

annevk reviewed Apr 24, 2023

View reviewed changes

noamr force-pushed the preempt branch from e3ae980 to 6cb2721 Compare April 24, 2023 08:15

annevk reviewed May 4, 2023

View reviewed changes

annevk reviewed May 8, 2023

View reviewed changes

noamr and others added 8 commits May 8, 2023 14:06

Define preconnect and dns-prefetch with CSP

ddaaef9

See whatwg/html#9035

Update fetch.bs

daab9c1

Co-authored-by: Valentin Gosu <[email protected]>

Update fetch.bs

2e4bae6

Co-authored-by: Anne van Kesteren <[email protected]>

Update fetch.bs

7e6e995

Co-authored-by: Anne van Kesteren <[email protected]>

PR nits

ef41a25

should

a85fdaa

A few editorial nits

c267d4e

Use boolean for algo

f64322a

noamr force-pushed the preempt branch from c005718 to f64322a Compare May 8, 2023 11:13

annevk reviewed Sep 28, 2023

View reviewed changes

noamr and others added 2 commits September 28, 2023 14:57

Update fetch.bs

b5bc78b

Co-authored-by: Anne van Kesteren <[email protected]>

Update fetch.bs

fa560eb

Co-authored-by: Anne van Kesteren <[email protected]>

		<p>To <dfn>resolve proxies</dfn>, return the result of finding proxies for <var>url</var> in an
		<a>implementation-defined</a> manner. If there are no proxies, return « "<code>DIRECT</code>" ».


		<h3 id=preemptive-connections>Preemptive connections</h3>

		<div algorithm="preemmptively-obtain-a-connection">

+              </ol>
+              </div>
+              <div>

		<li><p>Return the result of running <a>should request be blocked by Content Security Policy?</a>
		given <var>request</var>.

Conversation

noamr commented Mar 20, 2023 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

noamr commented Apr 24, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annevk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

noamr commented Mar 20, 2023 •

edited by pr-preview bot

Loading