Closed Bug 1143857 Opened 7 years ago Closed 7 years ago

Fetch does not serialize FormData body; breaks GitHub.

Categories

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

39 Branch
x86
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: callahad, Assigned: nsm)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1109751
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.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Attached file MozReview Request: bz://1143857/nsm (obsolete) —
/r/5537 - Bug 1143857 - Add FormData serialize support to Fetch API.

Pull down this commit:

hg pull review -r 34ac3e964055d87c5bd35a3442b23d3fbe5f393e
Attachment #8578712 - Flags: review?(ehsan)
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 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?
Attachment #8578712 - Flags: review?(ehsan)
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.
r=me with comment 6, for formality's sake.
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
Attachment #8578712 - Flags: review?(ehsan)
Comment on attachment 8578712 [details]
MozReview Request: bz://1143857/nsm

Dropping review since ehsan r+ed it and I've made the suggested changes.
Attachment #8578712 - Flags: review?(ehsan)
https://hg.mozilla.org/mozilla-central/rev/1ff28a6f0b77
Assignee: nobody → nsm.nikhil
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Depends on: 1150519
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.