Re: [whatwg/fetch] HTTPS upgrades proposal (PR #1655)

@annevk commented on this pull request.



> @@ -2157,6 +2157,17 @@ Unless stated otherwise, it is false.
 
 <p class=note>This flag is for exclusive use by HTML's render-blocking mechanism. [[!HTML]]
 
+<p>A <a for=/>request</a> has an associated boolean <dfn export for=request>is HTTPS upgrade</dfn>.
+Unless stated otherwise, it is false.
+
+<p class=note>This is for exclusive use by HTTPS Upgrading algorithm.
+
+<p>A <a for=/>request</a> has an associated
+<dfn export for=request>HTTPS upgrade fallback URL</dfn>, which is a <a for=/>URL</a>. Unless

which is null or a URL*

> @@ -3265,6 +3276,110 @@ through TLS using ALPN. The protocol cannot be spoofed through HTTP requests in
 </div>
 
 
+<h3 id=https-upgrades>HTTPS upgrading</h3>
+
+<p>User agents may optionally upgrade requests with URLs that are not
+<a>potentially trustworthy URLs</a> to attempt to fetch them over
+<a>potentially trustworthy URLs</a>. If an upgraded request fails with a network error, it is
+retried over the original URL.
+
+<p>The HTTPS upgrading algorithm consists of <a>upgrade an HTTP request</a> and <a>HTTPS upgrade

No wrapping inside inline elements. See README for guidelines.

> @@ -3265,6 +3276,110 @@ through TLS using ALPN. The protocol cannot be spoofed through HTTP requests in
 </div>
 
 
+<h3 id=https-upgrades>HTTPS upgrading</h3>
+
+<p>User agents may optionally upgrade requests with URLs that are not
+<a>potentially trustworthy URLs</a> to attempt to fetch them over
+<a>potentially trustworthy URLs</a>. If an upgraded request fails with a network error, it is
+retried over the original URL.
+
+<p>The HTTPS upgrading algorithm consists of <a>upgrade an HTTP request</a> and <a>HTTPS upgrade
+fallback</a> algorithms.
+
+<div algorithm>

This should be below the `<h4>`. The heading isn't part of the algorithm. (This applies several times, not just here.)

> +   "<code>https</code>".
+
+   <li><p>Set <a for=request>is HTTPS upgrade</a> to true.
+  </ul>
+ </li>
+</ol>
+</div>
+
+<div algorithm>
+<h4 id=https-upgrades-fallback>Fallback algorithm</h4>
+
+<p>To run <dfn>HTTPS upgrade fallback</dfn> given a <a for=/>request</a> <var>request</var> and
+<a for=/>response</a> <var>response</var>:
+
+<ol>
+ <li><p>If <var>request</var>'s <a for=request>is HTTPS upgrade</a> is not set, return

then return*

> + </li>
+</ol>
+</div>
+
+<div algorithm>
+<h4 id=https-upgrades-fallback>Fallback algorithm</h4>
+
+<p>To run <dfn>HTTPS upgrade fallback</dfn> given a <a for=/>request</a> <var>request</var> and
+<a for=/>response</a> <var>response</var>:
+
+<ol>
+ <li><p>If <var>request</var>'s <a for=request>is HTTPS upgrade</a> is not set, return
+ <var>response</var>.
+
+ <li>
+  <p>If <var>response</var> is a network error, run the following steps:

```suggestion
  <p>If <var>response</var> is a <a>network error</a>:
```

> +   <li><p>Let <var>fallbackResponse</var> be a new <a for=/>Response</a> with its
+   <code>Location</code> header set to <var>request</var>'s <a for=request>HTTPS upgrade fallback
+   URL</a>, and its <a for="response">status</a> set to 307.

Response should be lowercase here. And you need to initialize the header list. See "scheme fetch" for some examples.

> + <li>
+  <p>Return <var>response</var>.
+  <p class=note>This means the upgrade was successful.
+
+</ol>

```suggestion
 <li>
  <p>Return <var>response</var>.

  <p class=note>This means the upgrade was successful.
</ol>
```

> +<p class=note>User agents can implement a fast-fallback path by canceling slow fetches on upgraded
+requests, in order to quickly initiate a fallback http load.

```suggestion

<p class=note>User agents can implement a fast-fallback path by canceling slow fetches on upgraded
requests, in order to quickly initiate a fallback http load.
```

> +
+</ol>
+<p class=note>User agents can implement a fast-fallback path by canceling slow fetches on upgraded
+requests, in order to quickly initiate a fallback http load.
+
+</div>
+
+<h4 id=http-upgrades-examples>Examples</h4>
+
+<div id=example-https-upgrade-good-https class=example>
+ <p><code>a.com</code> serves both <code>http://a.com</code> and <code>https://a.com</code>. An
+ eligible request to <code>http://a.com</code> will be upgraded to <code>https://a.com</code>.
+</div>
+
+<div id=example-https-upgrade-bad-https class=example>
+ <p>

This doesn't match the formatting guidelines.

Also for both of these examples is there a need for the wrapper div element? Why not put the class and id on the `<p>`s?

> + <li>If <var>request</var>'s <a for=request>is HTTPS upgrade</a> is set, set <var>response</var> and
+  <var>internalResponse</var> to the result of running <a>HTTPS upgrade fallback</a> given
+  <var>request</var> and <var>response</var>.

Wrapping is wrong here.

> +     <li><p>If <var>response</var>'s <a for=response>type</a> is "<code>error</code>", then
+     return the result of running <a>HTTPS upgrade fallback</a> given <var>request</var>
+     and a <a>network error</a>.

This doesn't seem like the right place to be doing this. This means that whenever we get an error back from a service worker we'd do this?

> @@ -3241,7 +3253,111 @@ through TLS using ALPN. The protocol cannot be spoofed through HTTP requests in
 </ol>
 </div>
 
+<h3 id=https-upgrades>HTTPS upgrading</h3>

The end of this inserted section still eats two newlines from the subsequent `<h2>`. Please do review the entire PR for this.

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

Message ID: <whatwg/fetch/pull/1655/review/1648502368@github.com>