Fetch: data: URL HEAD request should result in empty response body

[Anne van Kesteren]

Test: http://w3c-test.org/XMLHttpRequest/data-uri.htm

Updated in: https://github.com/w3c/web-platform-tests/pull/6923

Even if Fetch initially ends up creating a response with a body, main fetch step 18 sets it to null again. This is my bad since it's likely you used to pass this.

[youenn fablet]

This is probably easy to fix and makes sense to me.

Looking at https://wpt.fyi/results/fetch/api/basic/scheme-data.any.html?label=master&label=experimental&aligned&q=fetch%2Fapi%2Fbasic%2Fscheme-data.any.html, all browsers consistently provide a body to HEAD data URL requests when using fetch API.

Looking at https://wpt.fyi/results/xhr/data-uri.htm?label=master&label=experimental&aligned&q=xhr%2Fdata-uri.htm, all browsers except Firefox do so when using XHR.

[Rob Buis]

Created attachment 380279 [details]
Patch

[Rob Buis]

Created attachment 380280 [details]
Patch

[Rob Buis]

Created attachment 380281 [details]
Patch

[youenn fablet]

Comment on attachment 380281 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380281&action=review

> Source/WebCore/ChangeLog:11
> +               imported/web-platform-tests/xhr/data-uri.html

Can we update ResourceLoader::loadDataURL()?

[Rob Buis]

Created attachment 380410 [details]
Patch

[Rob Buis]

(In reply to youenn fablet from comment #5)
> Comment on attachment 380281 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=380281&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +               imported/web-platform-tests/xhr/data-uri.html
> 
> Can we update ResourceLoader::loadDataURL()?

That does seem like the better spot to fix this for data url's, done.

[youenn fablet]

Comment on attachment 380410 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380410&action=review

> Source/WebCore/loader/ResourceLoader.cpp:288
> +            if (!this->reachedTerminalState() && dataSize && m_request.httpMethod() != "HEAD")

I believe we correctly handle these cases but do we have test coverage for gEt or HeAD?

As a side-note, we probably currently fail HEAD requests for blobs.
Are we expected to also do the same as for data URLs?

Since no HEAD request is expected to give a response with a body, I guess we could add an ASSERT in DocumentThreadableLoader to ensure that.

[Rob Buis]

(In reply to youenn fablet from comment #8)
> Comment on attachment 380410 [details]
> I believe we correctly handle these cases but do we have test coverage for
> gEt or HeAD?

I don't think there are tests, I quickly tried it locally on data-uri.htm and indeed we handle it correctly.

> As a side-note, we probably currently fail HEAD requests for blobs.
> Are we expected to also do the same as for data URLs?

IIUC according to the spec we should not even support anything other than GET:
https://fetch.spec.whatwg.org/#concept-scheme-fetch (blob 1.2)
However AFAIK chromium and WebKit do support HEAD. So this seems a grey area.

> Since no HEAD request is expected to give a response with a body, I guess we
> could add an ASSERT in DocumentThreadableLoader to ensure that.

We could, I will keep it in mind since there may be more bugs/tests that would require it.

[Rob Buis]

*** Bug 201136 has been marked as a duplicate of this bug. ***

[WebKit Commit Bot]

Comment on attachment 380410 [details]
Patch

Clearing flags on attachment: 380410

Committed r250822: <https://trac.webkit.org/changeset/250822>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56071479>