Bug 1338758 - Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI. r?m_kato draft
authorMasatoshi Kimura <VYV03354@nifty.ne.jp>
Sat, 11 Feb 2017 20:40:58 +0900
changeset 482189 5dac3faf3c5a2d3c2069846c570cc263d5f49998
parent 482164 855e6b2f6199189f37cea093cbdd1735e297e8aa
child 545382 f571d8dcca1c148b55d6f37e0e67d1742aba6a94
push id45025
push userVYV03354@nifty.ne.jp
push dateSat, 11 Feb 2017 12:59:57 +0000
reviewersm_kato
bugs1338758
milestone54.0a1
Bug 1338758 - Handle success codes from nsIUnicodeDecoder in nsTextToSubURI::UnEscapeNonAsciiURI. r?m_kato MozReview-Commit-ID: 43jAOFPYMeT
intl/uconv/nsTextToSubURI.cpp
intl/uconv/tests/unit/test_bug699673.js
intl/uconv/tests/unit/test_unEscapeNonAsciiURI.js
intl/uconv/tests/unit/xpcshell.ini
--- a/intl/uconv/nsTextToSubURI.cpp
+++ b/intl/uconv/nsTextToSubURI.cpp
@@ -257,32 +257,37 @@ NS_IMETHODIMP  nsTextToSubURI::UnEscapeU
   }
   const nsPromiseFlatString& unescapedResult = PromiseFlatString(_retval);
   nsString reescapedSpec;
   _retval = NS_EscapeURL(unescapedResult, mUnsafeChars, reescapedSpec);
 
   return NS_OK;
 }
 
-NS_IMETHODIMP  nsTextToSubURI::UnEscapeNonAsciiURI(const nsACString & aCharset, 
-                                                   const nsACString & aURIFragment, 
-                                                   nsAString &_retval)
+NS_IMETHODIMP
+nsTextToSubURI::UnEscapeNonAsciiURI(const nsACString& aCharset,
+                                    const nsACString& aURIFragment,
+                                    nsAString& _retval)
 {
   nsAutoCString unescapedSpec;
   NS_UnescapeURL(PromiseFlatCString(aURIFragment),
                  esc_AlwaysCopy | esc_OnlyNonASCII, unescapedSpec);
   // leave the URI as it is if it's not UTF-8 and aCharset is not a ASCII
-  // superset since converting "http:" with such an encoding is always a bad 
+  // superset since converting "http:" with such an encoding is always a bad
   // idea.
   if (!IsUTF8(unescapedSpec) && 
       (aCharset.LowerCaseEqualsLiteral("utf-16") ||
        aCharset.LowerCaseEqualsLiteral("utf-16be") ||
        aCharset.LowerCaseEqualsLiteral("utf-16le") ||
        aCharset.LowerCaseEqualsLiteral("utf-7") ||
        aCharset.LowerCaseEqualsLiteral("x-imap4-modified-utf7"))){
     CopyASCIItoUTF16(aURIFragment, _retval);
     return NS_OK;
   }
 
-  return convertURItoUnicode(PromiseFlatCString(aCharset), unescapedSpec, _retval);
+  nsresult rv = convertURItoUnicode(PromiseFlatCString(aCharset),
+                                    unescapedSpec, _retval);
+  // NS_OK_UDEC_MOREINPUT is a success code, so caller can't catch the error
+  // if the string ends with a valid (but incomplete) sequence.
+  return rv == NS_OK_UDEC_MOREINPUT ? NS_ERROR_UDEC_ILLEGALINPUT : rv;
 }
 
 //----------------------------------------------------------------------
rename from intl/uconv/tests/unit/test_bug699673.js
rename to intl/uconv/tests/unit/test_unEscapeNonAsciiURI.js
--- a/intl/uconv/tests/unit/test_bug699673.js
+++ b/intl/uconv/tests/unit/test_unEscapeNonAsciiURI.js
@@ -1,7 +1,46 @@
-// Tests whether nsTextToSubURI does UTF-16 unescaping (it shouldn't)
- function run_test()
-{
-  var testURI = "data:text/html,%FE%FF";
-  var textToSubURI = Components.classes["@mozilla.org/intl/texttosuburi;1"].getService(Components.interfaces.nsITextToSubURI);
+// Tests for nsITextToSubURI.unEscapeNonAsciiURI
+function run_test() {
+  const textToSubURI = Components.classes["@mozilla.org/intl/texttosuburi;1"].getService(Components.interfaces.nsITextToSubURI);
+
+  // Tests whether nsTextToSubURI does UTF-16 unescaping (it shouldn't)
+  const testURI = "data:text/html,%FE%FF";
   do_check_eq(textToSubURI.unEscapeNonAsciiURI("UTF-16", testURI), testURI);
+
+  // Tests whether incomplete multibyte sequences throw.
+  const tests = [{
+    input: "http://example.com/?p=%E9",
+    throws: Components.results.NS_ERROR_ILLEGAL_INPUT,
+  }, {
+    input: "http://example.com/?p=%E9%80",
+    throws: Components.results.NS_ERROR_ILLEGAL_INPUT,
+  }, {
+    input: "http://example.com/?p=%E9%80%80",
+    expected: "http://example.com/?p=\u9000",
+  }, {
+    input: "http://example.com/?p=%E9e",
+    throws: Components.results.NS_ERROR_ILLEGAL_INPUT,
+  }, {
+    input: "http://example.com/?p=%E9%E9",
+    throws: Components.results.NS_ERROR_ILLEGAL_INPUT,
+  }, {
+    input: "http://example.com/?name=M%FCller/",
+    throws: Components.results.NS_ERROR_ILLEGAL_INPUT,
+  }, {
+    input: "http://example.com/?name=M%C3%BCller/",
+    expected: "http://example.com/?name=Müller/",
+  }];
+
+  for (const t of tests) {
+    if (t.throws !== undefined) {
+      let thrown = undefined;
+      try {
+        textToSubURI.unEscapeNonAsciiURI("UTF-8", t.input);
+      } catch (e) {
+        thrown = e.result;
+      }
+      do_check_eq(thrown, t.throws);
+    } else {
+      do_check_eq(textToSubURI.unEscapeNonAsciiURI("UTF-8", t.input), t.expected);
+    }
+  }
 }
--- a/intl/uconv/tests/unit/xpcshell.ini
+++ b/intl/uconv/tests/unit/xpcshell.ini
@@ -20,20 +20,20 @@ support-files =
 [test_bug381412.js]
 [test_bug396637.js]
 [test_bug399257.js]
 [test_bug457886.js]
 [test_bug522931.js]
 [test_bug563283.js]
 [test_bug563618.js]
 [test_bug601429.js]
-[test_bug699673.js]
 [test_bug715319.euc_jp.js]
 [test_bug715319.gb2312.js]
 [test_bug715319.dbcs.js]
+[test_bug1008832.js]
 [test_charset_conversion.js]
 [test_decode_8859-1.js]
 [test_decode_8859-10.js]
 [test_decode_8859-11.js]
 [test_decode_8859-13.js]
 [test_decode_8859-14.js]
 [test_decode_8859-15.js]
 [test_decode_8859-2.js]
@@ -110,12 +110,12 @@ support-files =
 [test_encode_x_mac_greek.js]
 [test_encode_x_mac_gujarati.js]
 [test_encode_x_mac_gurmukhi.js]
 [test_encode_x_mac_hebrew.js]
 [test_encode_x_mac_icelandic.js]
 [test_encode_macintosh.js]
 [test_encode_x_mac_romanian.js]
 [test_encode_x_mac_turkish.js]
+[test_input_stream.js]
+[test_unEscapeNonAsciiURI.js]
+[test_unmapped.js]
 [test_utf8_illegals.js]
-[test_input_stream.js]
-[test_bug1008832.js]
-[test_unmapped.js]