168043 2017-02-09 01:34:44 -0800 [Fetch API] fetch fails when undefined is passed as headers 2017-05-08 22:08:55 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 torjus.bjaen youennf cdumez commit-queue thomasmulhall410 torjus.bjaen youennf oldest_to_newest 1274789 0 torjus.bjaen 2017-02-09 01:34:44 -0800 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. 1275325 1 youennf 2017-02-10 08:01:41 -0800 Hi Torjus and thanks for the feedback. Your understanding seems indeed correct. 1275330 2 301160 youennf 2017-02-10 08:22:40 -0800 Created attachment 301160 Patch 1275336 3 301160 ggaren 2017-02-10 08:58:34 -0800 Comment on attachment 301160 Patch r=me What should happen if 'headers' is null? How about a string or a number? You should add tests for those cases. 1275340 4 301160 cdumez 2017-02-10 09:18:04 -0800 Comment on attachment 301160 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? 1275383 5 youennf 2017-02-10 10:32:22 -0800 We need IDL tests for all of them I guess. Checking with Chris, null should be fine (empty header), other values should not. 1275390 6 cdumez 2017-02-10 10:36:22 -0800 (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. 1275393 7 cdumez 2017-02-10 10:37:42 -0800 (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) 1275582 8 youennf 2017-02-10 15:43:48 -0800 (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. 1275592 9 youennf 2017-02-10 15:51:34 -0800 Landing this patch as is since it is a step in the right direction. We will probably complement it with additional patches. 1275596 10 301160 commit-queue 2017-02-10 15:55:06 -0800 Comment on attachment 301160 Patch Clearing flags on attachment: 301160 Committed r212162: <http://trac.webkit.org/changeset/212162> 1275597 11 commit-queue 2017-02-10 15:55:11 -0800 All reviewed patches have been landed. Closing bug. 1292229 12 ap 2017-03-28 17:32:23 -0700 *** Bug 169970 has been marked as a duplicate of this bug. *** 1306258 13 youennf 2017-05-08 22:08:55 -0700 (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. 301160 2017-02-10 08:22:40 -0800 2017-02-10 15:55:06 -0800 Patch bug-168043-20170210082014.patch text/plain 3849 youennf U3VidmVyc2lvbiBSZXZpc2lvbjogMjExOTUyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYTdhODhlNjAxYWZhYTRi YjU1Yjk1NGUxZWIzYjFkZGVjM2U0NmI5ZS4uZDNjNzU5M2E1MmQ0ZDFhYzg3MjhhMWM0Njk0Nzc0 YWE4MDJjYTQyMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE3LTAyLTEwICBZb3Vl bm4gRmFibGV0ICA8eW91ZW5uQGFwcGxlLmNvbT4KKworICAgICAgICBbRmV0Y2ggQVBJXSBmZXRj aCBmYWlscyB3aGVuIHVuZGVmaW5lZCBpcyBwYXNzZWQgYXMgaGVhZGVycworICAgICAgICBodHRw czovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTY4MDQzCisKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ292ZXJlZCBieSB1cGRhdGVkIHRl c3QuCisKKyAgICAgICAgKiBNb2R1bGVzL2ZldGNoL0ZldGNoSW50ZXJuYWxzLmpzOgorICAgICAg ICAoZmlsbEZldGNoSGVhZGVycyk6IEV4aXQgZWFybHkgaW4gY2FzZSBvZiB1bmRlZmluZWQgaGVh ZGVycywgc2luY2UgaXQgaXMgYW4gb3B0aW9uYWwgcGFyYW1ldGVyLgorCiAyMDE3LTAyLTA5ICBK ZXIgTm9ibGUgIDxqZXIubm9ibGVAYXBwbGUuY29tPgogCiAgICAgICAgIENvbmZpZ3VyZSBNb2Nr UmVhbHRpbWVBdWRpb1NvdXJjZU1hYyB0byBnZW5lcmF0ZSBzdGVyZW8gYXVkaW8KZGlmZiAtLWdp dCBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvZmV0Y2gvRmV0Y2hJbnRlcm5hbHMuanMgYi9Tb3Vy Y2UvV2ViQ29yZS9Nb2R1bGVzL2ZldGNoL0ZldGNoSW50ZXJuYWxzLmpzCmluZGV4IGY5NzlkYjBi ZGI0OTFjYzMwYTEyMDhjNGVhNjhiZDM1ZThjOTA0MGQuLjNiOTFjZDNiZGRlMjIwNTY0ZjFjM2Ni MDFhZWIyMzliM2IxYmNkZjggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvZmV0 Y2gvRmV0Y2hJbnRlcm5hbHMuanMKKysrIGIvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9mZXRjaC9G ZXRjaEludGVybmFscy5qcwpAQCAtMzAsNiArMzAsOSBAQCBmdW5jdGlvbiBmaWxsRmV0Y2hIZWFk ZXJzKGhlYWRlcnMsIGhlYWRlcnNJbml0KQogewogICAgICJ1c2Ugc3RyaWN0IjsKIAorICAgIGlm IChoZWFkZXJzSW5pdCA9PT0gQHVuZGVmaW5lZCkKKyAgICAgICAgcmV0dXJuOworCiAgICAgaWYg KGhlYWRlcnNJbml0IGluc3RhbmNlb2YgQEhlYWRlcnMpIHsKICAgICAgICAgQEhlYWRlcnMucHJv dG90eXBlLkBmaWxsRnJvbUpTLkBjYWxsKGhlYWRlcnMsIGhlYWRlcnNJbml0KTsKICAgICAgICAg cmV0dXJuOwpkaWZmIC0tZ2l0IGEvTGF5b3V0VGVzdHMvaW1wb3J0ZWQvdzNjL0NoYW5nZUxvZyBi L0xheW91dFRlc3RzL2ltcG9ydGVkL3czYy9DaGFuZ2VMb2cKaW5kZXggNWZkMTY4YmFkMTA2YTNi MmJiYmMzZDBlZTczNWJiZGY1ODc3MjhhYi4uMGRiMjcwNThjZjFkZWRjMGZmOGJkMjRhMWIyZGM5 NzZjN2YyOTRiZCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvaW1wb3J0ZWQvdzNjL0NoYW5nZUxv ZworKysgYi9MYXlvdXRUZXN0cy9pbXBvcnRlZC93M2MvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMg QEAKKzIwMTctMDItMTAgIFlvdWVubiBGYWJsZXQgIDx5b3Vlbm5AYXBwbGUuY29tPgorCisgICAg ICAgIFtGZXRjaCBBUEldIGZldGNoIGZhaWxzIHdoZW4gdW5kZWZpbmVkIGlzIHBhc3NlZCBhcyBo ZWFkZXJzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x NjgwNDMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAq IHdlYi1wbGF0Zm9ybS10ZXN0cy9mZXRjaC9hcGkvcmVxdWVzdC9yZXF1ZXN0LWhlYWRlcnMtZXhw ZWN0ZWQudHh0OgorICAgICAgICAqIHdlYi1wbGF0Zm9ybS10ZXN0cy9mZXRjaC9hcGkvcmVxdWVz dC9yZXF1ZXN0LWhlYWRlcnMuaHRtbDoKKwogMjAxNy0wMi0wOCAgQ2hyaXMgRHVtZXogIDxjZHVt ZXpAYXBwbGUuY29tPgogCiAgICAgICAgIFVwZGF0ZSB3ZWItcGxhdGZvcm0tdGVzdHMgcmVzb3Vy Y2VzCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9pbXBvcnRlZC93M2Mvd2ViLXBsYXRmb3JtLXRl c3RzL2ZldGNoL2FwaS9yZXF1ZXN0L3JlcXVlc3QtaGVhZGVycy1leHBlY3RlZC50eHQgYi9MYXlv dXRUZXN0cy9pbXBvcnRlZC93M2Mvd2ViLXBsYXRmb3JtLXRlc3RzL2ZldGNoL2FwaS9yZXF1ZXN0 L3JlcXVlc3QtaGVhZGVycy1leHBlY3RlZC50eHQKaW5kZXggNmMyN2ZkMTVkY2I0OGY3Zjk0OWFj YzljYTE5NmJhNzgwOWRiZTIxOC4uN2JiNTkwNzgyYmEwNDNiZDAxYTMzZTBmYjk5M2M3M2FiOTRj ZDVhOCAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvaW1wb3J0ZWQvdzNjL3dlYi1wbGF0Zm9ybS10 ZXN0cy9mZXRjaC9hcGkvcmVxdWVzdC9yZXF1ZXN0LWhlYWRlcnMtZXhwZWN0ZWQudHh0CisrKyBi L0xheW91dFRlc3RzL2ltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvZmV0Y2gvYXBpL3Jl cXVlc3QvcmVxdWVzdC1oZWFkZXJzLWV4cGVjdGVkLnR4dApAQCAtNTUsNCArNTUsNSBAQCBQQVNT IFJlcXVlc3Qgc2hvdWxkIGdldCBpdHMgY29udGVudC10eXBlIGZyb20gdGhlIGJvZHkgaWYgbm9u ZSBpcyBwcm92aWRlZAogUEFTUyBSZXF1ZXN0IHNob3VsZCBnZXQgaXRzIGNvbnRlbnQtdHlwZSBm cm9tIGluaXQgaGVhZGVycyBpZiBvbmUgaXMgcHJvdmlkZWQgCiBQQVNTIFRlc3RpbmcgcmVxdWVz dCBoZWFkZXIgY3JlYXRpb25zIHdpdGggdmFyaW91cyBvYmplY3RzIAogUEFTUyBUZXN0aW5nIGVt cHR5IFJlcXVlc3QgQ29udGVudC1UeXBlIGhlYWRlciAKK1BBU1MgUGFzc2luZyB1bmRlZmluZWQg aGVhZGVycyB0byBSZXF1ZXN0IGNvbnN0cnVjdG9yIHNob3VsZCBub3QgdGhyb3cgCiAKZGlmZiAt LWdpdCBhL0xheW91dFRlc3RzL2ltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvZmV0Y2gv YXBpL3JlcXVlc3QvcmVxdWVzdC1oZWFkZXJzLmh0bWwgYi9MYXlvdXRUZXN0cy9pbXBvcnRlZC93 M2Mvd2ViLXBsYXRmb3JtLXRlc3RzL2ZldGNoL2FwaS9yZXF1ZXN0L3JlcXVlc3QtaGVhZGVycy5o dG1sCmluZGV4IDFkNTQ3MjhiN2RhNjAxZjI0ZjNlOTEzYTk2NDhkYzg4MzllYmRkMWUuLjg4ZWQw MjBlZDg3NTMzZjAwOGZhOGQ4Y2QxODkyZWQxMmQxOGQ3YmIgMTAwNjQ0Ci0tLSBhL0xheW91dFRl c3RzL2ltcG9ydGVkL3czYy93ZWItcGxhdGZvcm0tdGVzdHMvZmV0Y2gvYXBpL3JlcXVlc3QvcmVx dWVzdC1oZWFkZXJzLmh0bWwKKysrIGIvTGF5b3V0VGVzdHMvaW1wb3J0ZWQvdzNjL3dlYi1wbGF0 Zm9ybS10ZXN0cy9mZXRjaC9hcGkvcmVxdWVzdC9yZXF1ZXN0LWhlYWRlcnMuaHRtbApAQCAtMTY4 LDYgKzE2OCwxMSBAQAogICAgICAgICB9KTsKICAgICAgIH0sICJUZXN0aW5nIGVtcHR5IFJlcXVl c3QgQ29udGVudC1UeXBlIGhlYWRlciIpOwogCisgICAgICB0ZXN0KGZ1bmN0aW9uKCkgeworICAg ICAgICB2YXIgcmVxdWVzdCA9IG5ldyBSZXF1ZXN0KCIiLCB7ImhlYWRlcnMiIDogdW5kZWZpbmVk LCAiYm9keSIgOiAidGhpcyBpcyBteSBwbGF0ZSIsICJtZXRob2QiIDogIlBPU1QifSk7CisgICAg ICAgIGFzc2VydF9lcXVhbHMocmVxdWVzdC5oZWFkZXJzLmdldCgiQ29udGVudC1UeXBlIiksICJ0 ZXh0L3BsYWluO2NoYXJzZXQ9VVRGLTgiKTsKKyAgICAgIH0sICJQYXNzaW5nIHVuZGVmaW5lZCBo ZWFkZXJzIHRvIFJlcXVlc3QgY29uc3RydWN0b3Igc2hvdWxkIG5vdCB0aHJvdyIpOworCiAgICAg PC9zY3JpcHQ+CiAgIDwvYm9keT4KIDwvaHRtbD4K