Changeset 58224 in webkit

Timestamp:

Apr 24, 2010 10:00:38 PM (14 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

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp (modified) (2 diffs)
• JavaScriptCore/runtime/UString.cpp (modified) (2 diffs)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/js/ToNumber-expected.txt (added)
• LayoutTests/fast/js/ToNumber.html (added)
• LayoutTests/fast/js/parseFloat-expected.txt (added)
• LayoutTests/fast/js/parseFloat.html (added)
• LayoutTests/fast/js/script-tests/ToNumber.js (added)
• LayoutTests/fast/js/script-tests/parseFloat.js (added)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2010-04-24  Darin Adler  <darin@apple.com>
+
+        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  Anton Muhin  <antonm@chromium.org>
 

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;

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-04-24  Darin Adler  <darin@apple.com>
+
+        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.
+
 2010-04-24  Dan Bernstein  <mitz@apple.com>