Bug 918742 - Correct handling of author-supplied charsets in XMLHttpRequest, r=bz
authorMichael Layzell <michael@thelayzells.com>
Fri, 28 Aug 2015 17:13:46 -0400
changeset 263755 d0814284126324de850f1b92dc230ec46a443c74
parent 263754 4d45c4323570afd1a19695dea9f3c148a88891a4
child 263756 0d687549721e4f0d8902ec4e9704488b0b052cc2
push id65418
push usermichael@thelayzells.com
push dateTue, 22 Sep 2015 16:26:20 +0000
treeherdermozilla-inbound@d08142841263 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs918742
milestone44.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 918742 - Correct handling of author-supplied charsets in XMLHttpRequest, r=bz
devtools/client/netmonitor/har/test/browser_net_har_post_data.js
dom/base/nsXMLHttpRequest.cpp
dom/base/test/test_XHRSendData.html
dom/base/test/test_bug413974.html
dom/base/test/test_bug431701.html
testing/web-platform/meta/XMLHttpRequest/send-content-type-charset.htm.ini
testing/web-platform/meta/XMLHttpRequest/send-content-type-string.htm.ini
testing/web-platform/meta/XMLHttpRequest/setrequestheader-content-type.htm.ini
toolkit/modules/tests/xpcshell/test_Http.js
--- a/devtools/client/netmonitor/har/test/browser_net_har_post_data.js
+++ b/devtools/client/netmonitor/har/test/browser_net_har_post_data.js
@@ -24,16 +24,16 @@ add_task(function*() {
   let har = JSON.parse(jsonString);
 
   // Check out the HAR log.
   isnot(har.log, null, "The HAR log must exist");
   is(har.log.pages.length, 1, "There must be one page");
   is(har.log.entries.length, 1, "There must be one request");
 
   let entry = har.log.entries[0];
-  is(entry.request.postData.mimeType, "application/json; charset=UTF-8",
+  is(entry.request.postData.mimeType, "application/json",
     "Check post data content type");
   is(entry.request.postData.text, "{'first': 'John', 'last': 'Doe'}",
     "Check post data payload");
 
   // Clean up
   teardown(aMonitor).then(finish);
 });
--- a/dom/base/nsXMLHttpRequest.cpp
+++ b/dom/base/nsXMLHttpRequest.cpp
@@ -2698,27 +2698,34 @@ nsXMLHttpRequest::Send(nsIVariant* aVari
       // If no content type header was set by the client, we set it to
       // application/xml.
       nsAutoCString contentType;
       if (NS_FAILED(httpChannel->
                       GetRequestHeader(NS_LITERAL_CSTRING("Content-Type"),
                                        contentType)) ||
           contentType.IsEmpty()) {
         contentType = defaultContentType;
+
+        if (!charset.IsEmpty()) {
+          // If we are providing the default content type, then we also need to
+          // provide a charset declaration.
+          contentType.Append(NS_LITERAL_CSTRING(";charset="));
+          contentType.Append(charset);
+        }
       }
 
       // We don't want to set a charset for streams.
       if (!charset.IsEmpty()) {
         nsAutoCString specifiedCharset;
         bool haveCharset;
         int32_t charsetStart, charsetEnd;
         rv = NS_ExtractCharsetFromContentType(contentType, specifiedCharset,
                                               &haveCharset, &charsetStart,
                                               &charsetEnd);
-        if (NS_SUCCEEDED(rv)) {
+        while (NS_SUCCEEDED(rv) && haveCharset) {
           // special case: the extracted charset is quoted with single quotes
           // -- for the purpose of preserving what was set we want to handle
           // them as delimiters (although they aren't really)
           if (specifiedCharset.Length() >= 2 &&
               specifiedCharset.First() == '\'' &&
               specifiedCharset.Last() == '\'') {
             specifiedCharset = Substring(specifiedCharset, 1,
                                          specifiedCharset.Length() - 2);
@@ -2728,21 +2735,36 @@ nsXMLHttpRequest::Send(nsIVariant* aVari
           // and it's the same charset, up to case, as |charset|, just send the
           // page-set content-type header.  Apparently at least
           // google-web-toolkit is broken and relies on the exact case of its
           // charset parameter, which makes things break if we use |charset|
           // (which is always a fully resolved charset per our charset alias
           // table, hence might be differently cased).
           if (!specifiedCharset.Equals(charset,
                                        nsCaseInsensitiveCStringComparator())) {
-            nsAutoCString newCharset("; charset=");
-            newCharset.Append(charset);
-            contentType.Replace(charsetStart, charsetEnd - charsetStart,
-                                newCharset);
+            // Find the start of the actual charset declaration, skipping the
+            // "; charset=" to avoid modifying whitespace.
+            int32_t charIdx =
+              Substring(contentType, charsetStart,
+                        charsetEnd - charsetStart).FindChar('=') + 1;
+            MOZ_ASSERT(charIdx != -1);
+
+            contentType.Replace(charsetStart + charIdx,
+                                charsetEnd - charsetStart - charIdx,
+                                charset);
           }
+
+          // Look for another charset declaration in the string, limiting the
+          // search to only look for charsets before the current charset, to
+          // prevent finding the same charset twice.
+          nsDependentCSubstring interestingSection =
+            Substring(contentType, 0, charsetStart);
+          rv = NS_ExtractCharsetFromContentType(interestingSection,
+                                                specifiedCharset, &haveCharset,
+                                                &charsetStart, &charsetEnd);
         }
       }
 
       // If necessary, wrap the stream in a buffered stream so as to guarantee
       // support for our upload when calling ExplicitSetUploadStream.
       if (!NS_InputStreamIsBuffered(postDataStream)) {
         nsCOMPtr<nsIInputStream> bufferedStream;
         rv = NS_NewBufferedInputStream(getter_AddRefs(bufferedStream),
--- a/dom/base/test/test_XHRSendData.html
+++ b/dom/base/test/test_XHRSendData.html
@@ -89,74 +89,74 @@ function createFileWithDataExt(fileData,
 tests = [{ body: null,
            resBody: "",
          },
          { body: undefined,
            resBody: "",
          },
          { body: "hi",
            resBody: "hi",
-           resContentType: "text/plain; charset=UTF-8",
+           resContentType: "text/plain;charset=UTF-8",
          },
          { body: "r\xe4ksm\xf6rg\xe5s",
            resBody: "r\xc3\xa4ksm\xc3\xb6rg\xc3\xa5s",
-           resContentType: "text/plain; charset=UTF-8",
+           resContentType: "text/plain;charset=UTF-8",
          },
          { body: "hi",
            contentType: "",
            resBody: "hi",
-           resContentType: "text/plain; charset=UTF-8",
+           resContentType: "text/plain;charset=UTF-8",
          },
          { body: "hi",
            contentType: "foo/bar",
            resBody: "hi",
-           resContentType: "foo/bar; charset=UTF-8",
+           resContentType: "foo/bar",
          },
          { body: "hi",
            contentType: "foo/bar; baz=bin",
            resBody: "hi",
-           resContentType: "foo/bar; charset=UTF-8; baz=bin",
+           resContentType: "foo/bar; baz=bin",
          },
          { body: "hi",
            contentType: "foo/bar; charset=ascii; baz=bin",
            resBody: "hi",
            resContentType: "foo/bar; charset=UTF-8; baz=bin",
          },
          { body: "hi",
            contentType: "foo/bar; charset=uTf-8",
            resBody: "hi",
            resContentType: "foo/bar; charset=uTf-8",
          },
          { body: testDoc1,
            resBody: "<!-- comment -->\n<out>hi</out>",
-           resContentType: "application/xml; charset=UTF-8",
+           resContentType: "application/xml;charset=UTF-8",
          },
          { body: testDoc1,
            contentType: "foo/bar",
            resBody: "<!-- comment -->\n<out>hi</out>",
-           resContentType: "foo/bar; charset=UTF-8",
+           resContentType: "foo/bar",
          },
          { body: testDoc1,
            contentType: "foo/bar; charset=ascii; baz=bin",
            resBody: "<!-- comment -->\n<out>hi</out>",
            resContentType: "foo/bar; charset=UTF-8; baz=bin",
          },
          { body: testDoc1,
            contentType: "foo/bar; charset=wIndows-1252",
            resBody: "<!-- comment -->\n<out>hi</out>",
            resContentType: "foo/bar; charset=UTF-8",
          },
          { body: testDoc2,
            resBody: "<!-- doc 2 -->\n<res>text</res>",
-           resContentType: "application/xml; charset=UTF-8",
+           resContentType: "application/xml;charset=UTF-8",
          },
          { body: testDoc2,
            contentType: "foo/bar",
            resBody: "<!-- doc 2 -->\n<res>text</res>",
-           resContentType: "foo/bar; charset=UTF-8",
+           resContentType: "foo/bar",
          },
          { body: testDoc2,
            contentType: "foo/bar; charset=ascii; baz=bin",
            resBody: "<!-- doc 2 -->\n<res>text</res>",
            resContentType: "foo/bar; charset=UTF-8; baz=bin",
          },
          { body: testDoc2,
            contentType: "foo/bar; charset=uTf-8",
--- a/dom/base/test/test_bug413974.html
+++ b/dom/base/test/test_bug413974.html
@@ -21,15 +21,15 @@ https://bugzilla.mozilla.org/show_bug.cg
 var req = new XMLHttpRequest();
 req.open("POST", window.location.href);
 req.setRequestHeader("Content-Type", "text/plain; boundary=01234567890");
 req.send("Some text");
 
 is(SpecialPowers.wrap(req).channel
       .QueryInterface(SpecialPowers.Ci.nsIHttpChannel)
       .getRequestHeader("Content-Type"),
-   "text/plain; charset=UTF-8; boundary=01234567890",
+   "text/plain; boundary=01234567890",
    "Charset should come before boundary");
 </script>
 </pre>
 </body>
 </html>
 
--- a/dom/base/test/test_bug431701.html
+++ b/dom/base/test/test_bug431701.html
@@ -100,17 +100,17 @@ function startTest() {
   }
 
   // Now check what xhr does
   var xhr = new XMLHttpRequest();
   xhr.open("POST", document.location.href);
   xhr.send(createDoc());
   is(SpecialPowers.wrap(xhr).channel.QueryInterface(SpecialPowers.Ci.nsIHttpChannel)
                 .getRequestHeader("Content-Type"),
-     "application/xml; charset=UTF-8", "Testing correct type on the wire");
+     "application/xml;charset=UTF-8", "Testing correct type on the wire");
   xhr.abort();
                      
   SimpleTest.finish();
 };
 
 
 
 
--- a/testing/web-platform/meta/XMLHttpRequest/send-content-type-charset.htm.ini
+++ b/testing/web-platform/meta/XMLHttpRequest/send-content-type-charset.htm.ini
@@ -1,53 +1,5 @@
 [send-content-type-charset.htm]
   type: testharness
-  [XMLHttpRequest: send() - charset parameter of Content-Type]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - charset parameter of Content-Type 1]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - charset parameter of Content-Type 2]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - charset parameter of Content-Type 3]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - charset parameter of Content-Type 4]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - charset parameter of Content-Type 6]
-    expected: FAIL
-
-  [XMLHttpRequest: send() - charset parameter of Content-Type 7]
-    expected: FAIL
-
-  [header with invalid MIME type is not changed]
-    expected: FAIL
-
-  [known charset but bogus header - missing MIME type]
-    expected: FAIL
-
-  [bogus charset and bogus header - missing MIME type]
-    expected: FAIL
-
-  [If no charset= param is given, implementation should not add one - unknown MIME]
-    expected: FAIL
-
-  [If no charset= param is given, implementation should not add one - known MIME]
-    expected: FAIL
-
-  [charset given but wrong, fix it (unknown MIME, bogus charset)]
-    expected: FAIL
-
   [charset given but wrong, fix it (known MIME, bogus charset)]
     expected: FAIL
 
-  [charset given but wrong, fix it (known MIME, actual charset)]
-    expected: FAIL
-
-  [If multiple charset parameters are given, all should be rewritten]
-    expected: FAIL
-
-  [No content type set, give MIME and charset]
-    expected: FAIL
-
--- a/testing/web-platform/meta/XMLHttpRequest/send-content-type-string.htm.ini
+++ b/testing/web-platform/meta/XMLHttpRequest/send-content-type-string.htm.ini
@@ -1,11 +1,8 @@
 [send-content-type-string.htm]
   type: testharness
-  [XMLHttpRequest: send() - Content-Type]
-    expected: FAIL
-
   [XMLHttpRequest: send() - Content-Type 1]
     expected: FAIL
 
   [XMLHttpRequest: send() - Content-Type 2]
     expected: FAIL
 
--- a/testing/web-platform/meta/XMLHttpRequest/setrequestheader-content-type.htm.ini
+++ b/testing/web-platform/meta/XMLHttpRequest/setrequestheader-content-type.htm.ini
@@ -1,14 +1,5 @@
 [setrequestheader-content-type.htm]
   type: testharness
   [XMLHttpRequest: setRequestHeader() - Content-Type header ()]
     expected: FAIL
 
-  [XMLHttpRequest: setRequestHeader() - Content-Type header ( )]
-    expected: FAIL
-
-  [XMLHttpRequest: setRequestHeader() - Content-Type header (null)]
-    expected: FAIL
-
-  [XMLHttpRequest: setRequestHeader() - Content-Type header (undefined)]
-    expected: FAIL
-
--- a/toolkit/modules/tests/xpcshell/test_Http.js
+++ b/toolkit/modules/tests/xpcshell/test_Http.js
@@ -18,17 +18,17 @@ const kPostPath = "/post";
 const kPostUrl = kBaseUrl + kPostPath;
 const kPostDataSent = [["foo", "bar"], ["complex", "!*()@"]];
 const kPostDataReceived = "foo=bar&complex=%21%2A%28%29%40";
 const kPostMimeTypeReceived = "application/x-www-form-urlencoded; charset=utf-8";
 
 const kJsonPostPath = "/json_post";
 const kJsonPostUrl = kBaseUrl + kJsonPostPath;
 const kJsonPostData = JSON.stringify(kPostDataSent);
-const kJsonPostMimeType = "application/json; charset=UTF-8";
+const kJsonPostMimeType = "application/json";
 
 const kPutPath = "/put";
 const kPutUrl = kBaseUrl + kPutPath;
 const kPutDataSent = [["P", "NP"]];
 const kPutDataReceived = "P=NP";
 
 const kGetPath = "/get";
 const kGetUrl = kBaseUrl + kGetPath;