Specify identity encoding for range requests. Fixes #747. #751

jakearchibald · 2018-06-01T09:54:38Z

jakearchibald · 2018-06-01T09:56:27Z

jakearchibald · 2018-06-01T10:00:16Z

This is different to what browser appear to implement. Chrome and Safari seem to special-case media, and they add the Accept-Encoding header before the service worker (although Safari then filters it before the service worker).

This PR adds Accept-Encoding: identity for all requests with a Range header, regardless of source, and adds the header after the service worker (where Accept-Encoding is usually added).

I'm confident these differences are justified. If this is a weirdness for range requests, we should fix all range requests, and since the developer doesn't receive the encoded bytes, Accept-Encoding should be after the service worker.

youennf · 2018-06-01T16:06:55Z

The PR looks good to me.

I wonder what is the fetch spec policy regarding external links like https://jakearchibald.github.io/accept-encoding-range-test/.
It does not seem to work in Safari for instance and my locally installed Chrome bails out because of TransformStream not being present.
Maybe a snapshot of the results could be hosted somewhere (whatwg owned maybe) and would link to that particular URL for fresher results.

This PR adds Accept-Encoding: identity for all requests with a Range header, regardless of source, and adds the header after the service worker (where Accept-Encoding is usually added).

That makes sense to me. This is simple to implement, will simplify range requests authoring, including when intercepted by service workers. There seems to be no justification for making Accept-Encoding a special privileged header at the moment.

jakearchibald · 2018-06-01T17:47:17Z

@youennf

It does not seem to work in Safari for instance and my locally installed Chrome bails out because of TransformStream not being present.

I reckon I can make it work in the latest versions of Safari, Chrome, Firefox, & Edge. The TransformStream stuff is just a performance enhancement, and mostly just me taking the opportunity to play with some new stuff.

could be hosted somewhere (whatwg owned maybe)

I've got no problem with moving it. @annevk, any preferences?

domfarolino

Looking good to me. Regarding the external URL, FWIW I know we make official references to triple-underscore translations (like https://triple-underscore.github.io/Streams-ja.html and https://triple-underscore.github.io/console-ja.html) which are of course hosted on non-whatwg owned properties. But yeah I'll defer to Anne for final call.

jakearchibald · 2018-06-02T13:23:28Z

FWIW https://jakearchibald.github.io/accept-encoding-range-test/ now works across Safari, Chrome, Firefox & Edge.

annevk

Historically WHATWG standards have included non-normative external links when they add a lot of background information, so this seems fine.

I think for certain canvas algorithms we might still normatively refer to books, even, which feels much worse and not a precedent I'd be happy with following.

annevk · 2018-06-04T07:47:31Z

fetch.bs

+    <a for=request>header list</a>.
+
+    <p class="note no-backref">
+    <a href="https://jakearchibald.github.io/accept-encoding-range-test/">Many servers</a>


You cannot have a newline before <a here. We only allow whitespace at the end of paragraphs.

annevk · 2018-06-04T07:49:12Z

fetch.bs

@@ -3661,7 +3671,7 @@ Range Requests</cite>. [[HTTP-RANGE]] However, this is not widely supported by b
    `<code>Connection</code>`,
    `<code>DNT</code>`, and
    `<code>Host</code>`,
-    are to be <a for="header list">appended</a> if necessary.
+    are to be <a for="header list">appended</a> if necessary, unless already specified.


This should be part of the normative requirement, no?

Modify httpRequest's header list per HTTP, except for headers already appended prior to this step.

jakearchibald · 2018-06-04T12:03:44Z

@annevk I've tried to make the text normative, although it's a bit clunky.

annevk · 2018-06-04T12:22:35Z

fetch.bs

+    <a for=request>header list</a>.
+
+    <p class="note no-backref"><a href="https://jakearchibald.github.io/accept-encoding-range-test/">Many
+    servers</a> mistakenly ignore `<code>Range</code>` headers if a non-identity encoding is


No newline here. Only HTML allows newlines in inlines. I can fix this up for you before landing though.

Ah, I thought the fetch rule was only for references. Fixed.

annevk · 2018-06-04T12:25:52Z

With browser bugs this can land I think. I'll leave things another day for @youennf to comment.

jakearchibald · 2018-06-06T03:28:31Z

Chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=849952

WebKit: https://bugs.webkit.org/show_bug.cgi?id=186335 (@youennf I couldn't find a general 'fetch' component for this to go in, so it may need triaged).

Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1467010 (@annevk can you triage? The 'component' drop down seems to only contain irrelevant stuff these days).

Edge: https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/17784386/

annevk · 2018-06-06T07:23:47Z

Thanks! I think WebKit doesn't really use components and for Firefox you want to use the Core product. The Firefox product is the frontend, mostly (similar to Blink/Chromium).

For whatwg/fetch#751.

…s, a=testonly Automatic update from web-platform-testsFetch: test identity encoding for range requests For whatwg/fetch#751. -- wpt-commits: 1421a4a976d4d8263bde4864a1ce412eba106d39 wpt-pr: 11291

…s, a=testonly Automatic update from web-platform-testsFetch: test identity encoding for range requests For whatwg/fetch#751. -- wpt-commits: 1421a4a976d4d8263bde4864a1ce412eba106d39 wpt-pr: 11291 UltraBlame original commit: febbc42c7d8f857afb981fa2d2958f5976a47729

…s, a=testonly Automatic update from web-platform-testsFetch: test identity encoding for range requests For whatwg/fetch#751. -- wpt-commits: 1421a4a976d4d8263bde4864a1ce412eba106d39 wpt-pr: 11291 UltraBlame original commit: 954f7290ff43966a30c2ab2806e6841ab3618059

…s, a=testonly Automatic update from web-platform-testsFetch: test identity encoding for range requests For whatwg/fetch#751. -- wpt-commits: 1421a4a976d4d8263bde4864a1ce412eba106d39 wpt-pr: 11291 UltraBlame original commit: febbc42c7d8f857afb981fa2d2958f5976a47729

…s, a=testonly Automatic update from web-platform-testsFetch: test identity encoding for range requests For whatwg/fetch#751. -- wpt-commits: 1421a4a976d4d8263bde4864a1ce412eba106d39 wpt-pr: 11291 UltraBlame original commit: 954f7290ff43966a30c2ab2806e6841ab3618059

…s, a=testonly Automatic update from web-platform-testsFetch: test identity encoding for range requests For whatwg/fetch#751. -- wpt-commits: 1421a4a976d4d8263bde4864a1ce412eba106d39 wpt-pr: 11291 UltraBlame original commit: febbc42c7d8f857afb981fa2d2958f5976a47729

…s, a=testonly Automatic update from web-platform-testsFetch: test identity encoding for range requests For whatwg/fetch#751. -- wpt-commits: 1421a4a976d4d8263bde4864a1ce412eba106d39 wpt-pr: 11291 UltraBlame original commit: 954f7290ff43966a30c2ab2806e6841ab3618059

Specify identity encoding for range requests. Fixes #747.

ed4821e

jakearchibald requested review from annevk and youennf June 1, 2018 09:54

jakearchibald mentioned this pull request Jun 1, 2018

Test identity encoding for range requests web-platform-tests/wpt#11291

Merged

domfarolino approved these changes Jun 2, 2018

View reviewed changes

annevk reviewed Jun 4, 2018

View reviewed changes

Fixing formatting and normative text.

955f5cc

annevk reviewed Jun 4, 2018

View reviewed changes

FIxing wrapping

4698273

jyasskin mentioned this pull request Jun 5, 2018

Sending a Purpose: prefetch header for prefetched resources w3c/resource-hints#74

Closed

annevk approved these changes Jun 6, 2018

View reviewed changes

annevk merged commit 2f3d04d into master Jun 6, 2018

annevk deleted the accept-encoding-range branch June 6, 2018 07:26

annevk pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 6, 2018

Fetch: test identity encoding for range requests

1421a4a

For whatwg/fetch#751.

dhdavvie pushed a commit to dhdavvie/wpt that referenced this pull request Jun 7, 2018

Fetch: test identity encoding for range requests

36e2f96

For whatwg/fetch#751.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Specify identity encoding for range requests. Fixes #747. #751

Specify identity encoding for range requests. Fixes #747. #751

jakearchibald commented Jun 1, 2018 •

edited by pr-preview bot

Loading

jakearchibald commented Jun 1, 2018

jakearchibald commented Jun 1, 2018 •

edited

Loading

youennf commented Jun 1, 2018

jakearchibald commented Jun 1, 2018 •

edited

Loading

domfarolino left a comment

jakearchibald commented Jun 2, 2018

annevk left a comment

annevk Jun 4, 2018

annevk Jun 4, 2018

jakearchibald commented Jun 4, 2018 •

edited

Loading

annevk Jun 4, 2018

jakearchibald Jun 4, 2018

annevk commented Jun 4, 2018

jakearchibald commented Jun 6, 2018 •

edited

Loading

annevk commented Jun 6, 2018

Specify identity encoding for range requests. Fixes #747. #751

Specify identity encoding for range requests. Fixes #747. #751

Conversation

jakearchibald commented Jun 1, 2018 • edited by pr-preview bot Loading

jakearchibald commented Jun 1, 2018

jakearchibald commented Jun 1, 2018 • edited Loading

youennf commented Jun 1, 2018

jakearchibald commented Jun 1, 2018 • edited Loading

domfarolino left a comment

Choose a reason for hiding this comment

jakearchibald commented Jun 2, 2018

annevk left a comment

Choose a reason for hiding this comment

annevk Jun 4, 2018

Choose a reason for hiding this comment

annevk Jun 4, 2018

Choose a reason for hiding this comment

jakearchibald commented Jun 4, 2018 • edited Loading

annevk Jun 4, 2018

Choose a reason for hiding this comment

jakearchibald Jun 4, 2018

Choose a reason for hiding this comment

annevk commented Jun 4, 2018

jakearchibald commented Jun 6, 2018 • edited Loading

annevk commented Jun 6, 2018

jakearchibald commented Jun 1, 2018 •

edited by pr-preview bot

Loading

jakearchibald commented Jun 1, 2018 •

edited

Loading

jakearchibald commented Jun 1, 2018 •

edited

Loading

jakearchibald commented Jun 4, 2018 •

edited

Loading

jakearchibald commented Jun 6, 2018 •

edited

Loading