Commit 019c8ffa authored by darin@apple.com's avatar darin@apple.com

2010-05-26 Darin Adler <darin@apple.com>

        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@apple.com>

        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.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@60328 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 272ffd8f
2010-05-26 Darin Adler <darin@apple.com>
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-27 Nathan Lawrence <nlawrence@apple.com>
Reviewed by Geoffrey Garen.
......
......@@ -241,11 +241,10 @@ static double parseInt(const UString& s, int radix)
}
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);
}
if (!sawDigit)
......@@ -270,8 +269,6 @@ static double parseFloat(const UString& s)
if (length - p >= 2 && data[p] == '0' && (data[p + 1] == 'x' || data[p + 1] == 'X'))
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 */);
}
......
......@@ -262,6 +262,11 @@ double UString::toDouble(bool tolerateTrailingJunk, bool tolerateEmptyString) co
// 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.
// 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())
return NaN;
......@@ -324,13 +329,13 @@ double UString::toDouble(bool tolerateTrailingJunk, bool tolerateEmptyString) co
}
}
// 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;
}
......
2010-05-26 Darin Adler <darin@apple.com>
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.
2010-05-27 Julie Parent <jparent@chromium.org>
Unreviewed.
......
......@@ -9,6 +9,7 @@ PASS +false is 0
PASS +true is 1
PASS +2 is 2
PASS +'' is 0
PASS +' ' is 0
PASS +' 1' is 1
PASS +'1 ' is 1
PASS +'x1' is NaN
......@@ -16,8 +17,8 @@ PASS +'1x' is NaN
PASS +'0x1' is 1
PASS +'1x0' is NaN
FAIL +(nullCharacter + '1') should be NaN. Was 0.
FAIL +('1' + nullCharacter) should be NaN. Was 1.
FAIL +('1' + nullCharacter + '1') should be NaN. Was 1.
PASS +('1' + nullCharacter) is NaN
PASS +('1' + nullCharacter + '1') is NaN
PASS +(nonASCIICharacter + '1') is NaN
PASS +('1' + nonASCIICharacter) is NaN
PASS +('1' + nonASCIICharacter + '1') is NaN
......@@ -44,6 +45,30 @@ PASS +'AB' is NaN
PASS +'0xAB' is 171
PASS +'1e1' is 10
PASS +'1E1' is 10
PASS +tab is 0
FAIL +nbsp should be 0. Was NaN.
PASS +ff is 0
PASS +vt is 0
PASS +cr is 0
PASS +lf is 0
FAIL +ls should be 0. Was NaN.
FAIL +ps should be 0. Was NaN.
FAIL +oghamSpaceMark should be 0. Was NaN.
FAIL +mongolianVowelSeparator should be 0. Was NaN.
FAIL +enQuad should be 0. Was NaN.
FAIL +emQuad should be 0. Was NaN.
FAIL +enSpace should be 0. Was NaN.
FAIL +emSpace should be 0. Was NaN.
FAIL +threePerEmSpace should be 0. Was NaN.
FAIL +fourPerEmSpace should be 0. Was NaN.
FAIL +sixPerEmSpace should be 0. Was NaN.
FAIL +figureSpace should be 0. Was NaN.
FAIL +punctuationSpace should be 0. Was NaN.
FAIL +thinSpace should be 0. Was NaN.
FAIL +hairSpace should be 0. Was NaN.
FAIL +narrowNoBreakSpace should be 0. Was NaN.
FAIL +mediumMathematicalSpace should be 0. Was NaN.
FAIL +ideographicSpace should be 0. Was NaN.
PASS +(tab + '1') is 1
FAIL +(nbsp + '1') should be 1. Was NaN.
PASS +(ff + '1') is 1
......
......@@ -5,6 +5,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
PASS parseFloat() is NaN
PASS parseFloat('') is NaN
PASS parseFloat(' ') is NaN
PASS parseFloat(' 0') is 0
PASS parseFloat('0 ') is 0
PASS parseFloat('x0') is NaN
......
......@@ -37,6 +37,7 @@ shouldBe("+false", "0");
shouldBe("+true", "1");
shouldBe("+2", "2");
shouldBe("+''", "0");
shouldBe("+' '", "0");
shouldBe("+' 1'", "1");
shouldBe("+'1 '", "1");
shouldBe("+'x1'", "NaN");
......@@ -72,6 +73,30 @@ shouldBe("+'AB'", "NaN");
shouldBe("+'0xAB'", "171");
shouldBe("+'1e1'", "10");
shouldBe("+'1E1'", "10");
shouldBe("+tab", "0");
shouldBe("+nbsp", "0");
shouldBe("+ff", "0");
shouldBe("+vt", "0");
shouldBe("+cr", "0");
shouldBe("+lf", "0");
shouldBe("+ls", "0");
shouldBe("+ps", "0");
shouldBe("+oghamSpaceMark", "0");
shouldBe("+mongolianVowelSeparator", "0");
shouldBe("+enQuad", "0");
shouldBe("+emQuad", "0");
shouldBe("+enSpace", "0");
shouldBe("+emSpace", "0");
shouldBe("+threePerEmSpace", "0");
shouldBe("+fourPerEmSpace", "0");
shouldBe("+sixPerEmSpace", "0");
shouldBe("+figureSpace", "0");
shouldBe("+punctuationSpace", "0");
shouldBe("+thinSpace", "0");
shouldBe("+hairSpace", "0");
shouldBe("+narrowNoBreakSpace", "0");
shouldBe("+mediumMathematicalSpace", "0");
shouldBe("+ideographicSpace", "0");
shouldBe("+(tab + '1')", "1");
shouldBe("+(nbsp + '1')", "1");
shouldBe("+(ff + '1')", "1");
......
......@@ -31,6 +31,7 @@ var ideographicSpace = String.fromCharCode(0x3000);
shouldBe("parseFloat()", "NaN");
shouldBe("parseFloat('')", "NaN");
shouldBe("parseFloat(' ')", "NaN");
shouldBe("parseFloat(' 0')", "0");
shouldBe("parseFloat('0 ')", "0");
shouldBe("parseFloat('x0')", "NaN");
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment