Bug 1488192: Return input if no characters were modified in decode/encodeURI. r=jandem
authorAndré Bargull <andre.bargull@gmail.com>
Mon, 03 Sep 2018 05:33:41 -0700
changeset 482971 c3b29fcce16fed26eed17fc8e3910ae07bcca40d
parent 482970 a2a0dbf6d6e98a9146b5f809fb714d8007af2d54
child 482972 e32a8fc346e197e24180dd046329dab4cfc1ff73
push id232
push userfmarier@mozilla.com
push dateWed, 05 Sep 2018 20:45:54 +0000
reviewersjandem
bugs1488192
milestone63.0a1
Bug 1488192: Return input if no characters were modified in decode/encodeURI. r=jandem
js/src/builtin/String.cpp
js/src/util/Text.h
js/src/wasm/WasmDebug.cpp
--- a/js/src/builtin/String.cpp
+++ b/js/src/builtin/String.cpp
@@ -3746,21 +3746,23 @@ static const bool js_isUriUnescaped[] = 
 /* 10 */ true, true, true, true, true, true, true, true, true, true,
 /* 11 */ true, true, true, true, true, true, true, true, true, true,
 /* 12 */ true, true, true, ____, ____, ____, true, ____
 };
 
 #undef ____
 
 static inline bool
-TransferBufferToString(StringBuffer& sb, MutableHandleValue rval)
+TransferBufferToString(StringBuffer& sb, JSString* str, MutableHandleValue rval)
 {
-    JSString* str = sb.finishString();
-    if (!str)
-        return false;
+    if (!sb.empty()) {
+        str = sb.finishString();
+        if (!str)
+            return false;
+    }
     rval.setString(str);
     return true;
 }
 
 /*
  * ECMA 3, 15.1.3 URI Handling Function Properties
  *
  * The following are implementations of the algorithms
@@ -3771,34 +3773,49 @@ enum EncodeResult { Encode_Failure, Enco
 
 // Bug 1403318: GCC sometimes inlines this Encode function rather than the
 // caller Encode function. Annotate both functions with MOZ_NEVER_INLINE resp.
 // MOZ_ALWAYS_INLINE to ensure we get the desired inlining behavior.
 template <typename CharT>
 static MOZ_NEVER_INLINE EncodeResult
 Encode(StringBuffer& sb, const CharT* chars, size_t length, const bool* unescapedSet)
 {
-    Latin1Char hexBuf[4];
+    Latin1Char hexBuf[3];
     hexBuf[0] = '%';
-    hexBuf[3] = 0;
 
     auto appendEncoded = [&sb, &hexBuf](Latin1Char c) {
         static const char HexDigits[] = "0123456789ABCDEF"; /* NB: uppercase */
 
         hexBuf[1] = HexDigits[c >> 4];
         hexBuf[2] = HexDigits[c & 0xf];
         return sb.append(hexBuf, 3);
     };
 
+    auto appendRange = [&sb, chars, length](size_t start, size_t end) {
+        MOZ_ASSERT(start <= end);
+
+        if (start < end) {
+            if (start == 0) {
+                if (!sb.reserve(length))
+                    return false;
+            }
+            return sb.append(chars + start, chars + end);
+        }
+        return true;
+    };
+
+    size_t startAppend = 0;
     for (size_t k = 0; k < length; k++) {
         CharT c = chars[k];
         if (c < 128 && (js_isUriUnescaped[c] || (unescapedSet && unescapedSet[c]))) {
-            if (!sb.append(Latin1Char(c)))
+            continue;
+        } else {
+            if (!appendRange(startAppend, k))
                 return Encode_Failure;
-        } else {
+
             if (mozilla::IsSame<CharT, Latin1Char>::value) {
                 if (c < 0x80) {
                     if (!appendEncoded(c))
                         return Encode_Failure;
                 } else {
                     if (!appendEncoded(0xC0 | (c >> 6)) || !appendEncoded(0x80 | (c & 0x3F)))
                         return Encode_Failure;
                 }
@@ -3823,34 +3840,39 @@ Encode(StringBuffer& sb, const CharT* ch
 
                 uint8_t utf8buf[4];
                 size_t L = OneUcs4ToUtf8Char(utf8buf, v);
                 for (size_t j = 0; j < L; j++) {
                     if (!appendEncoded(utf8buf[j]))
                         return Encode_Failure;
                 }
             }
+
+            startAppend = k + 1;
         }
     }
 
+    if (startAppend > 0) {
+        if (!appendRange(startAppend, length))
+            return Encode_Failure;
+    }
+
     return Encode_Success;
 }
 
 static MOZ_ALWAYS_INLINE bool
 Encode(JSContext* cx, HandleLinearString str, const bool* unescapedSet, MutableHandleValue rval)
 {
     size_t length = str->length();
     if (length == 0) {
         rval.setString(cx->runtime()->emptyString);
         return true;
     }
 
     StringBuffer sb(cx);
-    if (!sb.reserve(length))
-        return false;
 
     EncodeResult res;
     if (str->hasLatin1Chars()) {
         AutoCheckCannotGC nogc;
         res = Encode(sb, str->latin1Chars(nogc), str->length(), unescapedSet);
     } else {
         AutoCheckCannotGC nogc;
         res = Encode(sb, str->twoByteChars(nogc), str->length(), unescapedSet);
@@ -3860,46 +3882,55 @@ Encode(JSContext* cx, HandleLinearString
         return false;
 
     if (res == Encode_BadUri) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_BAD_URI);
         return false;
     }
 
     MOZ_ASSERT(res == Encode_Success);
-    return TransferBufferToString(sb, rval);
+    return TransferBufferToString(sb, str, rval);
 }
 
 enum DecodeResult { Decode_Failure, Decode_BadUri, Decode_Success };
 
 template <typename CharT>
 static DecodeResult
 Decode(StringBuffer& sb, const CharT* chars, size_t length, const bool* reservedSet)
 {
+    auto appendRange = [&sb, chars](size_t start, size_t end) {
+        MOZ_ASSERT(start <= end);
+
+        if (start < end)
+            return sb.append(chars + start, chars + end);
+        return true;
+    };
+
+    size_t startAppend = 0;
     for (size_t k = 0; k < length; k++) {
         CharT c = chars[k];
         if (c == '%') {
             size_t start = k;
             if ((k + 2) >= length)
                 return Decode_BadUri;
 
             if (!JS7_ISHEX(chars[k+1]) || !JS7_ISHEX(chars[k+2]))
                 return Decode_BadUri;
 
             uint32_t B = JS7_UNHEX(chars[k+1]) * 16 + JS7_UNHEX(chars[k+2]);
             k += 2;
             if (B < 128) {
-                c = CharT(B);
-                if (reservedSet && reservedSet[c]) {
-                    if (!sb.append(chars + start, k - start + 1))
-                        return Decode_Failure;
-                } else {
-                    if (!sb.append(c))
-                        return Decode_Failure;
-                }
+                Latin1Char ch = Latin1Char(B);
+                if (reservedSet && reservedSet[ch])
+                    continue;
+
+                if (!appendRange(startAppend, start))
+                    return Decode_Failure;
+                if (!sb.append(ch))
+                    return Decode_Failure;
             } else {
                 int n = 1;
                 while (B & (0x80 >> n))
                     n++;
 
                 if (n == 1 || n > 4)
                     return Decode_BadUri;
 
@@ -3919,37 +3950,44 @@ Decode(StringBuffer& sb, const CharT* ch
                     B = JS7_UNHEX(chars[k+1]) * 16 + JS7_UNHEX(chars[k+2]);
                     if ((B & 0xC0) != 0x80)
                         return Decode_BadUri;
 
                     k += 2;
                     octets[j] = char(B);
                 }
 
+                if (!appendRange(startAppend, start))
+                    return Decode_Failure;
+
                 uint32_t v = JS::Utf8ToOneUcs4Char(octets, n);
                 MOZ_ASSERT(v >= 128);
                 if (v >= unicode::NonBMPMin) {
                     if (v > unicode::NonBMPMax)
                         return Decode_BadUri;
 
                     if (!sb.append(unicode::LeadSurrogate(v)))
                         return Decode_Failure;
                     if (!sb.append(unicode::TrailSurrogate(v)))
                         return Decode_Failure;
                 } else {
                     if (!sb.append(char16_t(v)))
                         return Decode_Failure;
                 }
             }
-        } else {
-            if (!sb.append(c))
-                return Decode_Failure;
+
+            startAppend = k + 1;
         }
     }
 
+    if (startAppend > 0) {
+        if (!appendRange(startAppend, length))
+            return Decode_Failure;
+    }
+
     return Decode_Success;
 }
 
 static bool
 Decode(JSContext* cx, HandleLinearString str, const bool* reservedSet, MutableHandleValue rval)
 {
     size_t length = str->length();
     if (length == 0) {
@@ -3972,17 +4010,17 @@ Decode(JSContext* cx, HandleLinearString
         return false;
 
     if (res == Decode_BadUri) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_BAD_URI);
         return false;
     }
 
     MOZ_ASSERT(res == Decode_Success);
-    return TransferBufferToString(sb, rval);
+    return TransferBufferToString(sb, str, rval);
 }
 
 static bool
 str_decodeURI(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     RootedLinearString str(cx, ArgToLinearString(cx, args, 0));
     if (!str)
@@ -4019,28 +4057,31 @@ str_encodeURI_Component(JSContext* cx, u
     CallArgs args = CallArgsFromVp(argc, vp);
     RootedLinearString str(cx, ArgToLinearString(cx, args, 0));
     if (!str)
         return false;
 
     return Encode(cx, str, nullptr, args.rval());
 }
 
-bool
-js::EncodeURI(JSContext* cx, StringBuffer& sb, const char* chars, size_t length)
+JSString*
+js::EncodeURI(JSContext* cx, const char* chars, size_t length)
 {
+    StringBuffer sb(cx);
     EncodeResult result = Encode(sb, reinterpret_cast<const Latin1Char*>(chars), length,
                                  js_isUriReservedPlusPound);
     if (result == EncodeResult::Encode_Failure)
-        return false;
+        return nullptr;
     if (result == EncodeResult::Encode_BadUri) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_BAD_URI);
-        return false;
+        return nullptr;
     }
-    return true;
+    if (sb.empty())
+        return NewStringCopyN<CanGC>(cx, chars, length);
+    return sb.finishString();
 }
 
 static bool
 FlatStringMatchHelper(JSContext* cx, HandleString str, HandleString pattern, bool* isFlat, int32_t* match)
 {
     RootedLinearString linearPattern(cx, pattern->ensureLinear(cx));
     if (!linearPattern)
         return false;
--- a/js/src/util/Text.h
+++ b/js/src/util/Text.h
@@ -214,14 +214,14 @@ inline bool
 FileEscapedString(FILE* fp, const char* chars, size_t length, uint32_t quote)
 {
     Fprinter out(fp);
     bool res = EscapedStringPrinter(out, chars, length, quote);
     out.finish();
     return res;
 }
 
-bool
-EncodeURI(JSContext* cx, StringBuffer& sb, const char* chars, size_t length);
+JSString*
+EncodeURI(JSContext* cx, const char* chars, size_t length);
 
 } // namespace js
 
 #endif // util_Text_h
--- a/js/src/wasm/WasmDebug.cpp
+++ b/js/src/wasm/WasmDebug.cpp
@@ -450,24 +450,26 @@ DebugState::debugDisplayURL(JSContext* c
     // - URI encoded filename from metadata (if can be encoded), plus ":";
     // - 64-bit hash of the module bytes (as hex dump).
 
     js::StringBuffer result(cx);
     if (!result.append("wasm:"))
         return nullptr;
 
     if (const char* filename = metadata().filename.get()) {
-        js::StringBuffer filenamePrefix(cx);
         // EncodeURI returns false due to invalid chars or OOM -- fail only
         // during OOM.
-        if (!EncodeURI(cx, filenamePrefix, filename, strlen(filename))) {
-            if (!cx->isExceptionPending())
+        JSString* filenamePrefix = EncodeURI(cx, filename, strlen(filename));
+        if (!filenamePrefix) {
+            if (cx->isThrowingOutOfMemory())
                 return nullptr;
+
+            MOZ_ASSERT(!cx->isThrowingOverRecursed());
             cx->clearPendingException(); // ignore invalid URI
-        } else if (!result.append(filenamePrefix.finishString())) {
+        } else if (!result.append(filenamePrefix)) {
             return nullptr;
         }
     }
 
     if (metadata().debugEnabled) {
         if (!result.append(":"))
             return nullptr;