1143857 - Fetch does not serialize FormData body; breaks GitHub.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Dan Callahan [:callahad]]

FormData bodies are incorrectly serialized when using fetch().
When I make this request:

    var data = new FormData();
    data.append('foo', 'bar');
    fetch('', { method: 'post', body: data });

I see a serialized body of "[object FormData]", which is not usable.
This is also incorrect per https://fetch.spec.whatwg.org/#body-mixin

This completely breaks all file uploads on GitHub.
(e.g., attaching images to issues or changing avatars)

Firefox 38 (Aurora) falls back to GitHub's polyfill and works great.

Firefox 39 (Nightly) defines fetch(), skips the polyfill, and is unusable.

Comment 2

[Dan Callahan [:callahad]]

Reopening per https://bugzilla.mozilla.org/show_bug.cgi?id=1109751#c11

This bug is for serializing and sending FormData, bug 1109751 is for the .formData() body consuming method.

Comment 3

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: MozReview Request: bz://1143857/nsm

/r/5537 - Bug 1143857 - Add FormData serialize support to Fetch API.

Pull down this commit:

hg pull review -r 34ac3e964055d87c5bd35a3442b23d3fbe5f393e

Comment 4

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

FYI, the test for the send is the same as the test for xhr https://hg.mozilla.org/integration/mozilla-inbound/file/24cf8a18a1a3/dom/html/test/formData_test.js#l108 except the indices are bumped since i added a string at the first position of the FormData

Comment 5

[(no longer active)]

Comment on attachment 8578712 [details]
MozReview Request: bz://1143857/nsm

https://reviewboard.mozilla.org/r/5535/#review4513

::: dom/fetch/moz.build
(Diff revision 1)
> +    '../base',

Can you please export the headers that you need instead?

::: dom/tests/mochitest/fetch/worker_test_fetch_basic_http.js
(Diff revision 1)
> +function testFormDataSend() {

Please add a comment indicating where this is copied from.

::: dom/tests/mochitest/fetch/worker_test_fetch_basic_http.js
(Diff revision 1)
> +function testFormDataRead() {

What's up with this empty function?

::: dom/workers/test/fetch/worker_test_request.js
(Diff revision 1)
> -function testBodyCreation() {

Why this change?

Comment 6

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

From IRC:

<nsm> ehsan: yes it makes sense. so i took the patch from 1109751 and split it up. and in interactive committing it was easier to leave the empty function in this patch and also make the greek string global. if you want i can put it in the right patch

<ehsan> nsm: please do, if possible, if it's too much of a hassle don't worry about it

I think i'll make the change and land.

Comment 7

[(no longer active)]

r=me with comment 6, for formality's sake.

Comment 8

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8578712 [details]
MozReview Request: bz://1143857/nsm

/r/5537 - Bug 1143857 - Add FormData serialize support to Fetch API.

Pull down this commit:

hg pull review -r fffc4114c45e775c6148730734f562922a151d7d

Comment 9

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8578712 [details]
MozReview Request: bz://1143857/nsm

Dropping review since ehsan r+ed it and I've made the suggested changes.

Comment 10

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a422e01a7ac5

Comment 11

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ff28a6f0b77

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1ff28a6f0b77

Comment 14

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: MozReview Request: Bug 1143857 - Add FormData serialize support to Fetch API.