Bug 11049 - XMLHttpRequest always defaults Content-Type to application/xml, while it should depend on data type
Summary: XMLHttpRequest always defaults Content-Type to application/xml, while it shou...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Alexander Shalamov
URL: http://dev.w3.org/cvsweb/~checkout~/2...
Keywords:
: 34654 46149 99690 99973 (view as bug list)
Depends on: 99973 99983
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-26 12:24 PDT by Alexey Proskuryakov
Modified: 2014-10-21 09:49 PDT (History)
16 users (show)

See Also:


Attachments
Proposed patch (5.10 KB, patch)
2009-11-05 17:36 PST, Carol Szabo
eric: review-
Details | Formatted Diff | Diff
Proposed Patch (5.72 KB, patch)
2009-12-21 09:37 PST, Carol Szabo
ap: review-
ap: commit-queue-
Details | Formatted Diff | Diff
Patch 1 (7.25 KB, patch)
2012-10-22 01:27 PDT, Alexander Shalamov
no flags Details | Formatted Diff | Diff
Patch (24.14 KB, patch)
2014-09-02 07:04 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion (545.40 KB, application/zip)
2014-09-02 08:33 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (575.40 KB, application/zip)
2014-09-02 09:37 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (534.64 KB, application/zip)
2014-09-02 10:13 PDT, Build Bot
no flags Details
Adding new baselines for mac and efl (52.14 KB, patch)
2014-09-02 12:51 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2006-09-26 12:24:18 PDT
In bug 3565, we changed the default XMLHttpRequest Content-Type from "application/x-www-form-urlencoded" to "application/xml", to better match Firefox.

The current draft of XHR spec describes somewhat different behavior - most notably, Content-Type shouldn't be automatically set for non-Document data at all.

"If the argument to send() is a Document and no Content-Type header has been set user agents must set it to application/xml for XML documents and to the most appropriate media type for other documents (using intrinsic knowledge about the document)."
Comment 1 Anne van Kesteren 2007-03-21 05:42:35 PDT
Since it already needs to be serialized as XML I dropped the bit about other document types for the moment.
Comment 2 Carol Szabo 2009-11-04 14:43:24 PST
A more recent version of the specification http://www.w3.org/TR/XMLHttpRequest/#the-send-method specifies that the default Content-Type should be text/plain if the send method is called with a string. Which makes sense since there is no information about the content of the string.
Proposed Patch coming soon.
Comment 3 Alexey Proskuryakov 2009-11-04 14:49:13 PST
Sounds good.

I'd like to see results of comparison to other major browsers though - although the current spec test has been extensively discussed, I'm not sure how well it matches behavior of shipping browsers. Knowing that will help foresee possible compatibility problems.
Comment 4 Carol Szabo 2009-11-04 15:19:37 PST
Compatibility survey:
According to my tests:
Mozilla uses (like the current version of WebKit): application/xml
IE8: does not set the header at all.

To me neither is quite correct, but I'd pick IE over Mozilla since the content of the string may not necessarily be XML. On the other hand we could go with what the spec says.
To me if the client does not set a content-type, probably the server doesn't care either about the content-type, so there shouldn't be compatibility issues, but one never knows.
Maybe Anne is wiling to change his mind, and alter the standard to say that since the name of the object is XMLHttpRequest, it should probably assume that by default it sends XML unless otherwise specified.
This way I can make my testers happy too.
Comment 5 Alexey Proskuryakov 2009-11-04 15:29:37 PST
> Mozilla uses (like the current version of WebKit): application/xml
> IE8: does not set the header at all.
> 
> To me neither is quite correct, but I'd pick IE over Mozilla since the content
> of the string may not necessarily be XML.

That's actually a violation of RFC2616: "Any HTTP/1.1 message containing an entity-body SHOULD include a Content-Type header field defining the media type of that body."

Based on the above data, I'd say that we should probably preserve our behavior for now, and consult with Mozilla on when/if they are going to change Gecko behavior.
Comment 6 Carol Szabo 2009-11-05 07:40:08 PST
I acknowledged that IE is not correct but it is a violation of RFC2616 to put a content type that defines a media type and then provide a body of different media type. text/plain for a string is a safe choice in my opinion and I think that this is why Anne chose that for the spec.
The spec can be changed, though, to specify application/xml and to require the user to set the content type if the string contains something else then xml, as I said, the name of the object is XMLHttpRequest, not GenericHttpRequest.

(In reply to comment #5)
> > Mozilla uses (like the current version of WebKit): application/xml
> > IE8: does not set the header at all.
> > 
> > To me neither is quite correct, but I'd pick IE over Mozilla since the content
> > of the string may not necessarily be XML.
> 
> That's actually a violation of RFC2616: "Any HTTP/1.1 message containing an
> entity-body SHOULD include a Content-Type header field defining the media type
> of that body."
> 
> Based on the above data, I'd say that we should probably preserve our behavior
> for now, and consult with Mozilla on when/if they are going to change Gecko
> behavior.
Comment 7 Anne van Kesteren 2009-11-05 15:15:40 PST
I rather not change the spec. We will add a FormData object soon which will make it default to multipart/form-data. Jonas (Mozilla) told me he would fix Mozilla XMLHttpRequest bugs so maybe you could file a bug on them?
Comment 8 Carol Szabo 2009-11-05 17:36:30 PST
Created attachment 42613 [details]
Proposed patch

According to Anne's comment above, the standard isn't likely to change in the regards that affect this bug, also, apparently, there is commitment from Firefox to change their behavior to match the current version of the standard. IE8 will probably converge too as their current behavior is in clear violation of HTTP standard.
Since the risk of breaking anything through this change is low I suggest that we go forwaard and match the standard now.
Comment 9 Alexey Proskuryakov 2009-11-06 09:24:00 PST
I'm not sufficiently opposed to this to mark the patch r-, but changing long-standing behavior without a good reason is not great. We don't even know if Mozilla has a bug filed about this, let alone what their timeframe for fixing would be.
Comment 10 Darin Adler 2009-11-06 10:18:10 PST
(In reply to comment #8)
> Since the risk of breaking anything through this change is low

How did you determine that?
Comment 11 Carol Szabo 2009-11-06 11:50:59 PST
(In reply to comment #10)
> (In reply to comment #8)
> > Since the risk of breaking anything through this change is low
> 
> How did you determine that?

Since IE does not send back any Content-Type and most websites work with IE, I assume that all those websites do not care about the Content-Type, they just assume the content is formatted as they expect it.
Furthermore, if somebody was careless enough to not set a content type on their post it sounds improbable to me that they would be careful to check if the content-arrives with the right type at the other end.
Since cross-site XHR is experimental, I assumed that the same team if not person designed both the JS sending the XHR and the code receiving it.
Pretty much this is my reasoning and I am aware that it is not bullet proof, but I weigh heavily conformance to standards, versus maintaining legacy dubious behavior after the standard is relatively solid.
If both IE and Mozilla had the same behavior, I would have agreed that there is a de facto standard whose legacy we should be mindful of, but since handling has not been consistent in the past, and the standard seems to be sensible I would suggest that we move on and support it.
Comment 12 Anne van Kesteren 2009-11-06 14:41:35 PST
Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=527123
Comment 13 Eric Seidel (no email) 2009-11-25 22:11:21 PST
Comment on attachment 42613 [details]
Proposed patch

The test could be made much better using shouldBe.  Or at least something which checks the expected value against the expected instead of requiring visual inspection.

Is this covered by any spec?  if so, that should be listed in the Changelog.  Likewise the ChangeLog should mention if this brings us closer or further from FF/IE's behavior.
Comment 14 Eric Seidel (no email) 2009-11-25 22:13:56 PST
Sorry, I clearly didn't read enough of the bug above before my review.  My comments still stand though: for easy review, the ChangeLog should mention the resolution of this bug, including possible specs and other browser behaviors.  Basically, ask yourself what you'd want to see when reviewing this, or when trying to figure out why the hell we'd made this change 5 years from now.  That information should all be in the ChangeLog for easy review and understanding 5 years from now. :)
Comment 15 Carol Szabo 2009-12-21 09:37:21 PST
Created attachment 45336 [details]
Proposed Patch

Addressed Eric's concerns about the test case and the ChangeLog.
There is no consensus in browser behavior. Mozilla was said to intend to align to the standard. IE has the unacceptable behavior of not sending a content type at all, which is non-conforming with HTTP.
Here is a link to the latest version of the XMR standard: http://dev.w3.org/2006/webapi/XMLHttpRequest/#the-send-method
Comment 16 WebKit Review Bot 2009-12-21 09:38:26 PST
style-queue ran check-webkit-style on attachment 45336 [details] without any errors.
Comment 17 Alexey Proskuryakov 2009-12-21 09:51:37 PST
Comment on attachment 45336 [details]
Proposed Patch

I'm against changing our behavior to something that matches no other browser. We can revisit this once Mozilla takes some action with the bug Anne filed.

The fact that IE and Firefox behave differently gives some hope for servers not depending on either behavior, but it is also quite probable that client-side JS has different code paths for different browsers, adjusting Content-Type as necessary.
Comment 18 Carol Szabo 2009-12-21 10:17:24 PST
(In reply to comment #17)
> (From update of attachment 45336 [details])
> I'm against changing our behavior to something that matches no other browser.
> We can revisit this once Mozilla takes some action with the bug Anne filed.
> 
> The fact that IE and Firefox behave differently gives some hope for servers not
> depending on either behavior, but it is also quite probable that client-side JS
> has different code paths for different browsers, adjusting Content-Type as
> necessary.

This bug only applies to cases when JS writers do not actually care to set the content type, hence they do not have different paths for Mozilla and IE in this regard.
Yes, you are right that probably servers do not care, or otherwise IE or Mozilla would not work, but wouldn't it be nice for WebKit to set a good example and be the first to do the reasonable thing.

If somebody in power (i.e. reviewer) is staunchly opposed to doing anything on this issue, which is indeed of minute importance, maybe it would be productive to close the bug as won't fix, so that test developers know what to expect, and others stop wasting time trying to address it, maybe Anne can be convinced to change the standard by vigurous opposition to actually implementing it, which I personally would not like, but it seems better than the status quo, where we have a standard to which the major players do not adhere and there is no movement toward it either.
Comment 19 Darin Adler 2009-12-21 10:52:57 PST
(In reply to comment #18)
> wouldn't it be nice for WebKit to set a good
> example and be the first to do the reasonable thing.

The tradeoff is against breaking something that currently works. If we had evidence that making this change would not have an adverse effect on existing websites or other uses of WebKit, then it would be an easy decision. We could just switch to "the reasonable thing" and set a good example. Although even then we could fall prey to the problem where we implement something in a standard, and then the standard later changes. That's happened to use on the WebKit project before.

Since there is a real chance this change could affect websites, we may need to be a bit more cautious.
Comment 20 Alexey Proskuryakov 2010-02-05 21:45:36 PST
*** Bug 34654 has been marked as a duplicate of this bug. ***
Comment 21 Alexey Proskuryakov 2010-09-20 18:31:12 PDT
*** Bug 46149 has been marked as a duplicate of this bug. ***
Comment 23 Alexey Proskuryakov 2012-10-18 23:00:28 PDT
*** Bug 99690 has been marked as a duplicate of this bug. ***
Comment 24 Alexander Shalamov 2012-10-22 01:27:56 PDT
Created attachment 169849 [details]
Patch 1

Moving proposed patch from https://bugs.webkit.org/show_bug.cgi?id=99690
- Removed ::send(String) and ::send(Blob) related fixes from original patch, should be fixed separately.
- Rebased to master.
Comment 25 Alexey Proskuryakov 2012-10-22 10:43:26 PDT
Comment on attachment 169849 [details]
Patch 1

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

I think that it would be better to have a separate bug for this, too, and either keep this one as a meta, or just close it.

This needs a study of other browsers' behavior, and some discussion about why servers and proxies are likely to accept content type strings like these.

Charset in particular seems fairly dangerous and useless, because UTF-8 is the default anyway.

> LayoutTests/http/tests/xmlhttprequest/post-document-content-type-expected.txt:8
> +PASS Content-type: application/xml;charset=encoding
> +PASS Content-type: text/html;charset=encoding
> +PASS Content-type: application/xml;charset=encoding

Ideally, a script test like this makes it clear what happens in a subtest. Here, there is no indication why the results are different, yet all are a PASS. Also, it's unclear why an unknown encoding name "encoding" is a PASS.
Comment 26 Alexander Shalamov 2012-10-26 04:08:22 PDT
(In reply to comment #25)
> I think that it would be better to have a separate bug for this, too, and either keep this one as a meta, or just close it.

Ok, I will create separate bug.

> This needs a study of other browsers' behavior, and some discussion about why servers and proxies are likely to accept content type strings like these.

I executed same tests in Opera and Mozilla and they have the same output.

> Ideally, a script test like this makes it clear what happens in a subtest. Here, there is no indication why the results are different, yet all are a PASS. Also, it's unclear why an unknown encoding name "encoding" is a PASS.

I was thinking that we could have different encodings on bots (for document).
If not, I could output actual encoding that is returned from the document.
Comment 27 Darin Adler 2013-05-13 18:15:58 PDT
Comment on attachment 169849 [details]
Patch 1

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

Thanks for tackling this. Looks good. I have a few suggestions for improvement.

> Source/WebCore/xml/XMLHttpRequest.cpp:562
> +        String documentMIMEType = document->isHTMLDocument() ? "text/html" : "application/xml";

There is no need to use a String just to hold one or two different ASCII literals. Should just use const char* instead.

Also seems unnecessary to do this out here since it’s only used if contentType is empty. Should move this code closer to where it’s used.

> Source/WebCore/xml/XMLHttpRequest.cpp:592
> +        if (!encoding.isValid() || encoding == UTF16BigEndianEncoding() || encoding == UTF16LittleEndianEncoding())

The function TextEncoding::closestByteBasedEquivalent() takes care of mapping UTF-16 to UTF-8, and that is what we should use here instead of specifically special casing UTF-16.
Comment 28 youenn fablet 2014-02-18 00:24:18 PST
The current XHR spec has a simpler charset handling: UTF-8 should be used in all cases.
It still mandates using text/html for HTMLDocument and application/xml otherwise, which would be good to have.
Comment 29 Darin Adler 2014-08-19 08:41:43 PDT
Comment on attachment 169849 [details]
Patch 1

Clearing the review flag on this because it seems we will want to do something different given the change in the XHR spec.
Comment 30 youenn fablet 2014-09-02 07:04:12 PDT
Created attachment 237486 [details]
Patch
Comment 31 youenn fablet 2014-09-02 08:19:00 PDT
(In reply to comment #30)
> Created an attachment (id=237486) [details]
> Patch

This patch updates default Content-Type for string to text/plain;charset=UTF-8
This aligns with http://w3c-test.org/XMLHttpRequest/send-content-type-charset.htm, Firefox and Blink.

This patch also changes HTML document default content-Type to text/html;charset=UTF-8.
This aligns with the spec. From a quick check, Blink and Firefox are still using application/xml as mime type for HTML documents.
Comment 32 Build Bot 2014-09-02 08:33:47 PDT
Comment on attachment 237486 [details]
Patch

Attachment 237486 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5581839196487680

New failing tests:
http/tests/xmlhttprequest/workers/shared-worker-methods-async.html
http/tests/xmlhttprequest/methods.html
http/tests/xmlhttprequest/workers/shared-worker-methods.html
http/tests/xmlhttprequest/workers/methods-async.html
http/tests/xmlhttprequest/methods-async.html
http/tests/xmlhttprequest/workers/methods.html
Comment 33 Build Bot 2014-09-02 08:33:53 PDT
Created attachment 237488 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 34 Build Bot 2014-09-02 09:37:41 PDT
Comment on attachment 237486 [details]
Patch

Attachment 237486 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5118068627865600

New failing tests:
http/tests/xmlhttprequest/workers/shared-worker-methods-async.html
http/tests/xmlhttprequest/methods.html
http/tests/xmlhttprequest/workers/shared-worker-methods.html
http/tests/xmlhttprequest/workers/methods-async.html
http/tests/xmlhttprequest/methods-async.html
http/tests/xmlhttprequest/workers/methods.html
Comment 35 Build Bot 2014-09-02 09:37:46 PDT
Created attachment 237497 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 36 Build Bot 2014-09-02 10:13:04 PDT
Comment on attachment 237486 [details]
Patch

Attachment 237486 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5697537562378240

New failing tests:
http/tests/xmlhttprequest/workers/shared-worker-methods-async.html
http/tests/xmlhttprequest/methods.html
http/tests/xmlhttprequest/workers/shared-worker-methods.html
http/tests/xmlhttprequest/workers/methods-async.html
http/tests/xmlhttprequest/methods-async.html
http/tests/xmlhttprequest/workers/methods.html
Comment 37 Build Bot 2014-09-02 10:13:10 PDT
Created attachment 237502 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 38 youenn fablet 2014-09-02 12:51:29 PDT
Created attachment 237509 [details]
Adding new baselines for mac and efl
Comment 39 Darin Adler 2014-09-03 23:28:18 PDT
Comment on attachment 237509 [details]
Adding new baselines for mac and efl

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

> Source/WebCore/xml/XMLHttpRequest.cpp:596
> +                setRequestHeaderInternal("Content-Type", document->isHTMLDocument() ? "text/html;charset=UTF-8":"application/xml;charset=UTF-8");

Wrong formatting here. There should be spaces around the colon.

All the string literals that are being passed as String should have ASCIILiteral around them, to eliminate unnecessary memory allocation.
Comment 40 WebKit Commit Bot 2014-09-04 00:04:06 PDT
Comment on attachment 237509 [details]
Adding new baselines for mac and efl

Clearing flags on attachment: 237509

Committed r173254: <http://trac.webkit.org/changeset/173254>
Comment 41 WebKit Commit Bot 2014-09-04 00:04:17 PDT
All reviewed patches have been landed.  Closing bug.
Comment 42 Alexey Proskuryakov 2014-10-21 09:49:27 PDT
*** Bug 99973 has been marked as a duplicate of this bug. ***