Changeset 60328 in webkit for trunk/JavaScriptCore/runtime

Timestamp:

May 27, 2010, 4:09:48 PM (15 years ago)

Author:

Darin Adler

Message:

2010-05-26 Darin Adler <Darin Adler>

Reviewed by Kent Tamura.

Null characters handled incorrectly in ToNumber conversion
​https://bugs.webkit.org/show_bug.cgi?id=38088

• runtime/JSGlobalObjectFunctions.cpp: (JSC::parseInt): Changed code to use UTF8String().data() instead of ascii() to fix the thread safety issue. Code path is covered by existing tests in run-javascriptcore-tests. (JSC::parseFloat): Moved comment to UString::toDouble since the issue affects all clients, not just parseFloat. Specifically, this also affects standard JavaScript numeric conversion, ToNumber.

• runtime/UString.cpp: (JSC::UString::toDouble): Added a comment about incorrect space skipping. Changed trailing junk check to use the length of the CString instead of checking for a null character. Also got rid of a little unneeded logic in the case where we tolerate trailing junk.

2010-05-26 Darin Adler <Darin Adler>

Reviewed by Kent Tamura.

Null characters handled incorrectly in ToNumber conversion
​https://bugs.webkit.org/show_bug.cgi?id=38088

• fast/js/ToNumber-expected.txt: Updated for new tests and to expect PASS for two null character tests.
• fast/js/ToNumber.js: Added more test cases.
• fast/js/parseFloat-expected.txt: Updated for new test case.
• fast/js/script-tests/parseFloat.js: Added a test case.

Location:

trunk/JavaScriptCore/runtime

Files:

• JSGlobalObjectFunctions.cpp (modified) (2 diffs)
• UString.cpp (modified) (2 diffs)

trunk/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp

@@ -242 +242 @@
 
     if (number >= mantissaOverflowLowerBound) {
-        // FIXME: It is incorrect to use UString::ascii() here because it's not thread-safe.
         if (radix == 10)
-            number = WTF::strtod(s.substr(firstDigitPosition, p - firstDigitPosition).ascii(), 0);
+            number = WTF::strtod(s.substr(firstDigitPosition, p - firstDigitPosition).UTF8String().data(), 0);
         else if (radix == 2 || radix == 4 || radix == 8 || radix == 16 || radix == 32)
-            number = parseIntOverflow(s.substr(firstDigitPosition, p - firstDigitPosition).ascii(), p - firstDigitPosition, radix);
+            number = parseIntOverflow(s.substr(firstDigitPosition, p - firstDigitPosition).UTF8String().data(), p - firstDigitPosition, radix);
     }
 
@@ -271 +270 @@
         return 0;
 
-    // FIXME: UString::toDouble will ignore leading ASCII spaces, but we need to ignore
-    // other StrWhiteSpaceChar values as well.
     return s.toDouble(true /*tolerant*/, false /* NaN for empty string */);
 }

trunk/JavaScriptCore/runtime/UString.cpp

@@ -263 +263 @@
     // non-ASCII characters to UTF-8, so the UTF8String does quite a bit of
     // unnecessary work.
+
+    // FIXME: The space skipping code below skips only ASCII spaces, but callers
+    // need to skip all StrWhiteSpace. The isStrWhiteSpace function does the
+    // right thing but requires UChar, not char, for its argument.
+
     CString s = UTF8String();
     if (s.isNull())
@@ -325 +330 @@
     }
 
-    // allow trailing white space
-    while (isASCIISpace(*c))
-        c++;
-    // don't allow anything after - unless tolerant=true
-    // FIXME: If string contains a U+0000 character, then this check is incorrect.
-    if (!tolerateTrailingJunk && *c != '\0')
-        d = NaN;
+    if (!tolerateTrailingJunk) {
+        // allow trailing white space
+        while (isASCIISpace(*c))
+            c++;
+        if (c != s.data() + s.length())
+            d = NaN;
+    }
 
     return d;