Add the charset right after the type, before all other params, if there is no existing charset in the type. Bug 413974, r=biesi, sr=sicking, a=beltzner
authorbzbarsky@mit.edu
Thu, 28 Feb 2008 20:24:32 -0800
changeset 12421 0eab09a63e8c583959a1f74af1315813b9bbc31b
parent 12420 66a61530eb92f90183c616b020cd9d31213e3965
child 12422 dcfff2d77fb07b1ca3d592d963316993f0e552c2
push idunknown
push userunknown
push dateunknown
reviewersbiesi, sicking, beltzner
bugs413974
milestone1.9b4pre
Add the charset right after the type, before all other params, if there is no existing charset in the type. Bug 413974, r=biesi, sr=sicking, a=beltzner
content/base/src/nsXMLHttpRequest.cpp
content/base/test/Makefile.in
content/base/test/test_bug413974.html
netwerk/base/public/nsINetUtil.idl
netwerk/base/src/nsIOService.cpp
netwerk/base/src/nsURLHelper.cpp
netwerk/test/unit/test_extract_charset_from_content_type.js
--- a/content/base/src/nsXMLHttpRequest.cpp
+++ b/content/base/src/nsXMLHttpRequest.cpp
@@ -2213,23 +2213,19 @@ nsXMLHttpRequest::Send(nsIVariant *aBody
         PRBool haveCharset;
         PRInt32 charsetStart, charsetEnd;
         rv = NS_ExtractCharsetFromContentType(contentType, specifiedCharset,
                                               &haveCharset, &charsetStart,
                                               &charsetEnd);
         if (NS_FAILED(rv)) {
           contentType.AssignLiteral("application/xml");
           specifiedCharset.Truncate();
-          haveCharset = PR_FALSE;
+          charsetStart = charsetEnd = contentType.Length();
         }
 
-        if (!haveCharset) {
-          charsetStart = charsetEnd = contentType.Length();
-        } 
-
         // If the content-type the page set already has a charset parameter,
         // 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,
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -161,16 +161,17 @@ include $(topsrcdir)/config/rules.mk
 		test_bug402150.html^headers^ \
 		test_bug401662.html \
 		test_bug403852.html \
 		test_bug403868.xml \
 		test_bug405182.html \
 		test_bug403841.html \
 		test_bug409380.html \
 		test_bug410229.html \
+		test_bug413974.html \
 		test_bug415860.html \
 		test_bug414190.html \
 		test_bug414796.html \
 		test_bug416383.html \
 		test_bug417255.html \
 		test_bug417384.html \
 		test_bug418214.html \
 		$(NULL)
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_bug413974.html
@@ -0,0 +1,37 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=413974
+-->
+<head>
+  <title>Test for Bug 413974</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=413974">Mozilla Bug 413974</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 413974 **/
+var req = new XMLHttpRequest();
+req.open("POST", window.location.href);
+req.setRequestHeader("Content-Type", "text/plain; boundary=01234567890");
+req.send("Some text");
+
+netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
+is(req.channel
+      .QueryInterface(Components.interfaces.nsIHttpChannel)
+      .getRequestHeader("Content-Type"),
+   "text/plain; charset=UTF-8; boundary=01234567890",
+   "Charset should come before boundary");
+</script>
+</pre>
+</body>
+</html>
+
--- a/netwerk/base/public/nsINetUtil.idl
+++ b/netwerk/base/public/nsINetUtil.idl
@@ -197,25 +197,26 @@ interface nsINetUtil : nsISupports
    * Extract the charset parameter location and value from a content-type
    * header.
    *
    * @param aTypeHeader the header string to parse
    * @param [out] aCharset the charset parameter specified in the
    *              header, if any.
    * @param [out] aCharsetStart index of the start of the charset parameter
    *              (the ';' separating it from what came before) in aTypeHeader.
-   *              This is only set if this function returns true.
+   *              If this function returns false, this argument will still be
+   *              set, to the index of the location where a new charset should
+   *              be inserted.
    * @param [out] aCharsetEnd index of the end of the charset parameter (the
-   *              ';' separating it from what comes after, or inded of the end
-   *              of the string) in aTypeHeader.  This is only set if this
-   *              function returns true.
+   *              ';' separating it from what comes after, or the end
+   *              of the string) in aTypeHeader.  If this function returns
+   *              false, this argument will still be set, to the index of the
+   *              location where a new charset should be inserted.
    *
    * @return whether a charset parameter was found.  This can be false even in
    * cases when parseContentType would claim to have a charset, if the type
-   * that won out does not have a charset parameter specified, because in this
-   * case setting the charset does in fact correspond to appending to the
-   * string.
+   * that won out does not have a charset parameter specified.
    */
   boolean extractCharsetFromContentType(in AUTF8String aTypeHeader,
                                         out AUTF8String aCharset,
                                         out long aCharsetStart,
                                         out long aCharsetEnd);
 };
--- a/netwerk/base/src/nsIOService.cpp
+++ b/netwerk/base/src/nsIOService.cpp
@@ -972,14 +972,14 @@ nsIOService::ExtractCharsetFromContentTy
                                            nsACString &aCharset,
                                            PRInt32 *aCharsetStart,
                                            PRInt32 *aCharsetEnd,
                                            PRBool *aHadCharset)
 {
     nsCAutoString ignored;
     net_ParseContentType(aTypeHeader, ignored, aCharset, aHadCharset,
                          aCharsetStart, aCharsetEnd);
-    if (*aHadCharset && *aCharsetStart == -1) {
+    if (*aHadCharset && *aCharsetStart == *aCharsetEnd) {
         *aHadCharset = PR_FALSE;
     }
     return NS_OK;
 }
 
--- a/netwerk/base/src/nsURLHelper.cpp
+++ b/netwerk/base/src/nsURLHelper.cpp
@@ -825,27 +825,36 @@ net_ParseMediaType(const nsACString &aMe
         // Common case here is that aContentType is empty
         PRBool eq = !aContentType.IsEmpty() &&
             aContentType.Equals(Substring(type, typeEnd),
                                 nsCaseInsensitiveCStringComparator());
         if (!eq) {
             aContentType.Assign(type, typeEnd - type);
             ToLowerCase(aContentType);
         }
+
         if ((!eq && *aHadCharset) || typeHasCharset) {
             *aHadCharset = PR_TRUE;
             aContentCharset.Assign(charset, charsetEnd - charset);
             if (typeHasCharset) {
                 *aCharsetStart = charsetParamStart + aOffset;
                 *aCharsetEnd = charsetParamEnd + aOffset;
-            } else {
-                *aCharsetStart = -1;
-                *aCharsetEnd = -1;
             }
         }
+        // Only set a new charset position if this is a different type
+        // from the last one we had and it doesn't already have a
+        // charset param.  If this is the same type, we probably want
+        // to leave the charset position on its first occurrence.
+        if (!eq && !typeHasCharset) {
+            PRInt32 charsetStart = PRInt32(paramStart);
+            if (charsetStart == kNotFound)
+                charsetStart =  flatStr.Length();
+
+            *aCharsetEnd = *aCharsetStart = charsetStart + aOffset;
+        }
     }
 }
 
 #undef HTTP_LWS
 
 void
 net_ParseContentType(const nsACString &aHeaderStr,
                      nsACString       &aContentType,
--- a/netwerk/test/unit/test_extract_charset_from_content_type.js
+++ b/netwerk/test/unit/test_extract_charset_from_content_type.js
@@ -46,58 +46,57 @@ function reset() {
   delete charsetStart.value;
   delete charsetEnd.value;
   hadCharset = undefined;
 }
 
 function check(aHadCharset, aCharset, aCharsetStart, aCharsetEnd) {
   do_check_eq(aHadCharset, hadCharset);
   do_check_eq(aCharset, charset.value);
-  if (hadCharset) {
-    do_check_eq(aCharsetStart, charsetStart.value);
-    do_check_eq(aCharsetEnd, charsetEnd.value);
-  }
+  do_check_eq(aCharsetStart, charsetStart.value);
+  do_check_eq(aCharsetEnd, charsetEnd.value);
 }
 
 function run_test() {
   var netutil = Components.classes["@mozilla.org/network/util;1"]
                           .getService(Components.interfaces.nsINetUtil);
   hadCharset =
     netutil.extractCharsetFromContentType("text/html", charset, charsetStart,
 					  charsetEnd);
-  check(false, "");
+  check(false, "", 9, 9);
 
   hadCharset =
     netutil.extractCharsetFromContentType("TEXT/HTML", charset, charsetStart,
 					  charsetEnd);
-  check(false, "");
+  check(false, "", 9, 9);
 
   hadCharset =
     netutil.extractCharsetFromContentType("text/html, text/html", charset,
 					  charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 9, 9);
 
   hadCharset =
     netutil.extractCharsetFromContentType("text/html, text/plain",
 					  charset, charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 21, 21);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/html, ', charset, charsetStart,
 					  charsetEnd);
-  check(false, "");
+  check(false, "", 9, 9);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/html, */*', charset,
 					  charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 9, 9);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/html, foo', charset,
 					  charsetStart, charsetEnd);
+  check(false, "", 9, 9);
 
   hadCharset =
     netutil.extractCharsetFromContentType("text/html; charset=ISO-8859-1",
 					  charset, charsetStart, charsetEnd);
   check(true, "ISO-8859-1", 9, 29);
 
   hadCharset =
     netutil.extractCharsetFromContentType("text/html  ;    charset=ISO-8859-1",
@@ -137,17 +136,17 @@ function run_test() {
   hadCharset =
     netutil.extractCharsetFromContentType("text/html; charset=ISO-8859-1, TEXT/HTML",
 					  charset, charsetStart, charsetEnd);
   check(true, "ISO-8859-1", 9, 29);
 
   hadCharset =
     netutil.extractCharsetFromContentType("text/html; charset=ISO-8859-1, TEXT/plain",
 					  charset, charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 41, 41);
 
   hadCharset =
     netutil.extractCharsetFromContentType("text/plain, TEXT/HTML; charset='ISO-8859-1', text/html, TEXT/HTML",
 					  charset, charsetStart, charsetEnd);
   check(true, "ISO-8859-1", 21, 43);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/plain, TEXT/HTML; param="charset=UTF8"; charset=\'ISO-8859-1\'; param2="charset=UTF16", text/html, TEXT/HTML',
@@ -157,40 +156,40 @@ function run_test() {
   hadCharset =
     netutil.extractCharsetFromContentType('text/plain, TEXT/HTML; param=charset=UTF8; charset=\'ISO-8859-1\'; param2=charset=UTF16, text/html, TEXT/HTML',
 					  charset, charsetStart, charsetEnd);
   check(true, "ISO-8859-1", 41, 63);
 
   hadCharset =
     netutil.extractCharsetFromContentType("text/plain; param= , text/html",
 					  charset, charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 30, 30);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/plain; param=", text/html"',
 					  charset, charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 10, 10);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/plain; param=", \\" , text/html"',
 					  charset, charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 10, 10);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/plain; param=", \\" , text/html , "',
 					  charset, charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 10, 10);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/plain param=", \\" , text/html , "',
 					  charset, charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 38, 38);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/plain charset=UTF8',
 					  charset, charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 23, 23);
 
   hadCharset =
     netutil.extractCharsetFromContentType('text/plain, TEXT/HTML; param="charset=UTF8"; ; param2="charset=UTF16", text/html, TEXT/HTML',
 					  charset, charsetStart, charsetEnd);
-  check(false, "");
+  check(false, "", 21, 21);
 }