Opened 9 years ago

Closed 7 years ago

#12750 closed defect (fixed)

[patch][ccla] Forced default content type prevents FormData object in dojo/request

Reported by: ghiloni Owned by: Bryan Forbes
Priority: blocker Milestone: 1.8
Component: IO Version: 1.6.0
Keywords: Cc: cjolif
Blocked By: Blocking:

Description

with the advent of the HTML5 Files API, the FormData? object can be used to post files to an endpoint using XMLHTTPRequest, and more specifically, dojo.xhr. The problem lies within the setup of the native XHR. In dojo.xhr(), the content type is always set. The FormData? object will set the content type on the XHR for you, including the appropriate boundary, which it creates when it serializes the object. The ioArgs.query object should be checked if it is a FormData? instance before setting the default content type (while still respecting the explicit contentType argument, if one is passed).

A suggested patch has been attached. This occurs on all fully HTML5-capable browsers (FF4, Chrome 9, Safari 5) that I've tested.

Attachments (3)

xhr.patch (609 bytes) - added by ghiloni 9 years ago.
12750-1_7.patch (739 bytes) - added by cjolif 7 years ago.
fixing the issue for 1.7 (working on IE + formatting fix)
12750-1_8.patch (458 bytes) - added by cjolif 7 years ago.
fixes for 1.8 where the problem with FormData? is slightly different. It happens Content-Type is not set to a default value in 1.8 however ioArgs.data is pass to objectToQuery which we don't want for FormData? that needs to stay as-is.

Download all attachments as: .zip

Change History (15)

Changed 9 years ago by ghiloni

Attachment: xhr.patch added

comment:1 Changed 9 years ago by rmurphey

See also #12141

comment:2 Changed 9 years ago by Douglas Hays

Summary: allow null content type on dojo.xhr when ioArgs.query is a FormData object[patch][ccla]allow null content type on dojo.xhr when ioArgs.query is a FormData object

comment:3 Changed 8 years ago by Colin Snover

Priority: highblocker

Bulk update of open ticket priorities.

comment:4 Changed 8 years ago by bill

Owner: changed from James Burke to Bryan Forbes

Bulk change to reassign IO tickets to Bryan, since he is working on new dojo/request module. Some of these tickets should probably be closed as already fixed, invalid, or wontfix.

comment:5 Changed 8 years ago by cjolif

Cc: cjolif added

comment:6 Changed 8 years ago by Adam Peller

Priority: blockerhigh
Type: defectenhancement

Some issues with the patch: FormData? would likely trigger a ReferenceError? on older browsers. Also, there's no else clause, so all codepaths are not necessarily covered, are they?

comment:7 in reply to:  6 Changed 8 years ago by cjolif

Replying to peller:

Some issues with the patch: FormData? would likely trigger a ReferenceError? on older browsers. Also, there's no else clause, so all codepaths are not necessarily covered, are they?

I think the else clause is the case they want to cover, i.e. if there is no explicit contentType and that there is a FormData in the query then they don't want a default content type to be set because it will be automatically done.

Changed 7 years ago by cjolif

Attachment: 12750-1_7.patch added

fixing the issue for 1.7 (working on IE + formatting fix)

Changed 7 years ago by cjolif

Attachment: 12750-1_8.patch added

fixes for 1.8 where the problem with FormData? is slightly different. It happens Content-Type is not set to a default value in 1.8 however ioArgs.data is pass to objectToQuery which we don't want for FormData? that needs to stay as-is.

comment:8 Changed 7 years ago by Adam Peller

Milestone: tbd1.8

comment:9 Changed 7 years ago by Colin Snover

Milestone: 1.82.0

1.8 is frozen. Move all enhancements to next release. If you need an exemption from the freeze for this ticket, contact me immediately.

comment:10 Changed 7 years ago by Colin Snover

Milestone: 2.01.8
Priority: highblocker
Status: newassigned
Summary: [patch][ccla]allow null content type on dojo.xhr when ioArgs.query is a FormData object[patch][ccla] Forced default content type prevents FormData object in dojo/request
Type: enhancementdefect

comment:11 Changed 7 years ago by Bryan Forbes

In [28927]:

Make sure FormData? objects are skipped for data processing. Added unit test. refs #12750 !strict

comment:12 Changed 7 years ago by Bryan Forbes

Resolution: fixed
Status: assignedclosed

The provided patch for 1.7 removes the way to remove the "Content-Type" header that has been in Dojo since 1.0 and would break the API. In 1.7, the following should work without the provided patch:

dojo.xhrPost({
    url: "/some/url",
    headers: { "Content-Type": false },
    postData: FormData(dojo.byId("someNode"))
}).then(function(){
});

The patch for 1.8 is somewhat correct, however there should be a has test for native FormData support and the instanceof check should be passed into util.parseArgs() as the third parameter. I checked in a fix and unit test for this. Please include unit tests in the future.

Note: See TracTickets for help on using tickets.