Bug 168043 – [Fetch API] fetch fails when undefined is passed as headers

• Component: New Bugs
• Status: RESOLVED FIXED

[Torjus]

When explicitly passing `undefined` to the `headers` option of `fetch()`, the promise is rejected with 'TypeError: undefined is not an object'.

To reproduce, run `fetch('/my-awesome-api', { headers: undefined })`.

Both Chrome and Firefox will accept explicitly passing `{ headers: undefined }`.
As the fetch API has been available in other browsers for quite some time now, I think passing `undefined` should be supported, as existing libraries or application may break when doesn't fall back to using a polyfill.

[youenn fablet]

Hi Torjus and thanks for the feedback.
Your understanding seems indeed correct.

[youenn fablet]

Created attachment 301160 [details]
Patch

[Geoffrey Garen]

Comment on attachment 301160 [details]
Patch

r=me

What should happen if 'headers' is null? How about a string or a number? You should add tests for those cases.

[Chris Dumez]

Comment on attachment 301160 [details]
Patch

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

> Source/WebCore/Modules/fetch/FetchInternals.js:33
> +    if (headersInit === @undefined)

What about null?

[youenn fablet]

We need IDL tests for all of them I guess.
Checking with Chris, null should be fine (empty header), other values should not.

[Chris Dumez]

(In reply to comment #5)
> We need IDL tests for all of them I guess.
> Checking with Chris, null should be fine (empty header), other values should
> not.

https://heycam.github.io/webidl/#es-union (Step 3.2.)
then
https://heycam.github.io/webidl/#es-record (Step 2)

So passing null or undefined should behave the same here.

[Chris Dumez]

(In reply to comment #6)
> (In reply to comment #5)
> > We need IDL tests for all of them I guess.
> > Checking with Chris, null should be fine (empty header), other values should
> > not.
> 
> https://heycam.github.io/webidl/#es-union (Step 3.2.)
> then
> https://heycam.github.io/webidl/#es-record (Step 2)
> 
> So passing null or undefined should behave the same here.

I would expect other non-object types to throw a TypeError based on https://heycam.github.io/webidl/#es-union (Step 17)

[youenn fablet]

(In reply to comment #6)
> (In reply to comment #5)
> > We need IDL tests for all of them I guess.
> > Checking with Chris, null should be fine (empty header), other values should
> > not.
> 
> https://heycam.github.io/webidl/#es-union (Step 3.2.)
> then
> https://heycam.github.io/webidl/#es-record (Step 2)
> 
> So passing null or undefined should behave the same here.

This feels a bit odd though in that case to have the same behavior.
Somehow {headers: undefined} means that there is no headers in the dictionary and we should reuse the RequestInfo headers if any.
{headers: null} might mean to create a request without any header.

Let's get more time for fetch+WebIDL spec to stabilise on that.

[youenn fablet]

Landing this patch as is since it is a step in the right direction.
We will probably complement it with additional patches.

[WebKit Commit Bot]

Comment on attachment 301160 [details]
Patch

Clearing flags on attachment: 301160

Committed r212162: <http://trac.webkit.org/changeset/212162>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

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

[youenn fablet]

(In reply to youenn fablet from comment #8)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > We need IDL tests for all of them I guess.
> > > Checking with Chris, null should be fine (empty header), other values should
> > > not.
> > 
> > https://heycam.github.io/webidl/#es-union (Step 3.2.)
> > then
> > https://heycam.github.io/webidl/#es-record (Step 2)
> > 
> > So passing null or undefined should behave the same here.

WebIDL spec has changed and we should now throw in case of null passed to a Record.