Changeset 58224 in webkit for trunk/JavaScriptCore/runtime

Timestamp:

Apr 24, 2010, 10:00:38 PM (15 years ago)

Author:

Darin Adler

Message:

2010-04-24 Darin Adler <Darin Adler>

Reviewed by Dan Bernstein.

REGRESSION (r56560): Crash in parseFloat if passed invalid UTF-16 data
​https://bugs.webkit.org/show_bug.cgi?id=38083
rdar://problem/7901044

Tests: fast/js/ToNumber.html

fast/js/parseFloat.html

• runtime/JSGlobalObjectFunctions.cpp: (JSC::parseInt): Added a FIXME comment about a problem I noticed. (JSC::parseFloat): Added a FIXME comment about a problem I noticed; covered by test cases in the test I added.
• runtime/UString.cpp: (JSC::UString::toDouble): Added FIXME comments about two problem I noticed; covered by test cases in the tests I added. Added a return statement so we don't crash when illegal UTF-16 sequences are present.

2010-04-24 Darin Adler <Darin Adler>

Reviewed by Dan Bernstein.

REGRESSION (r56560): Crash in parseFloat if passed invalid UTF-16 data
​https://bugs.webkit.org/show_bug.cgi?id=38083
rdar://problem/7901044

• fast/js/parseFloat-expected.txt: Added.
• fast/js/parseFloat.html: Added.
• fast/js/script-tests/parseFloat.js: Added.

• fast/js/ToNumber-expected.txt: Added.
• fast/js/ToNumber.html: Added.
• fast/js/script-tests/ToNumber.js: Added.

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);
@@ -270 +271 @@
         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

@@ -255 +255 @@
     }
 
+    // FIXME: If tolerateTrailingJunk is true, then we want to tolerate junk 
+    // after the number, even if it contains invalid UTF-16 sequences. So we
+    // shouldn't use the UTF8String function, which returns null when it
+    // encounters invalid UTF-16. Further, we have no need to convert the
+    // non-ASCII characters to UTF-8, so the UTF8String does quite a bit of
+    // unnecessary work.
     CString s = UTF8String();
+    if (s.isNull())
+        return NaN;
     const char* c = s.data();
 
@@ -319 +327 @@
         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;