Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make ToNumber(String) conversion more spec-compliant #383

Merged
merged 1 commit into from
Jan 31, 2018
Merged

Make ToNumber(String) conversion more spec-compliant #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/org/mozilla/javascript/NativeGlobal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ static Object js_parseInt(Object[] args) {
}
}

double d = ScriptRuntime.stringToNumber(s, start, radix);
double d = ScriptRuntime.stringPrefixToNumber(s, start, radix);
return ScriptRuntime.wrapNumber(negative ? -d : d);
}

Expand Down
110 changes: 76 additions & 34 deletions src/org/mozilla/javascript/ScriptRuntime.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,14 +462,21 @@ public static double toNumber(Object[] args, int index) {

public static final Double NaNobj = new Double(NaN);

static double stringPrefixToNumber(String s, int start, int radix) {
return stringToNumber(s, start, s.length() - 1, radix, true);
}

static double stringToNumber(String s, int start, int end, int radix) {
return stringToNumber(s, start, end, radix, false);
}

/*
* Helper function for toNumber, parseInt, and TokenStream.getToken.
*/
static double stringToNumber(String s, int start, int radix) {
private static double stringToNumber(String source, int sourceStart, int sourceEnd, int radix, boolean isPrefix) {
char digitMax = '9';
char lowerCaseBound = 'a';
char upperCaseBound = 'A';
int len = s.length();
if (radix < 10) {
digitMax = (char) ('0' + radix - 1);
}
Expand All @@ -479,20 +486,22 @@ static double stringToNumber(String s, int start, int radix) {
}
int end;
double sum = 0.0;
for (end=start; end < len; end++) {
char c = s.charAt(end);
for (end = sourceStart; end <= sourceEnd; end++) {
char c = source.charAt(end);
int newDigit;
if ('0' <= c && c <= digitMax)
newDigit = c - '0';
else if ('a' <= c && c < lowerCaseBound)
newDigit = c - 'a' + 10;
else if ('A' <= c && c < upperCaseBound)
newDigit = c - 'A' + 10;
else if (!isPrefix)
return NaN; // isn't a prefix but found unexpected char
else
break;
break; // unexpected char
sum = sum*radix + newDigit;
}
if (start == end) {
if (sourceStart == end) { // stopped right at the beginning
return NaN;
}
if (sum >= 9007199254740992.0) {
Expand All @@ -503,7 +512,7 @@ else if ('A' <= c && c < upperCaseBound)
* answer.
*/
try {
return Double.parseDouble(s.substring(start, end));
return Double.parseDouble(source.substring(sourceStart, end));
} catch (NumberFormatException nfe) {
return NaN;
}
Expand Down Expand Up @@ -535,12 +544,13 @@ else if ('A' <= c && c < upperCaseBound)
boolean bit53 = false;
// bit54 is the 54th bit (the first dropped from the mantissa)
boolean bit54 = false;
int pos = sourceStart;

for (;;) {
if (bitShiftInChar == 1) {
if (start == end)
if (pos == end)
break;
digit = s.charAt(start++);
digit = source.charAt(pos++);
if ('0' <= digit && digit <= '9')
digit -= '0';
else if ('a' <= digit && digit <= 'z')
Expand Down Expand Up @@ -614,61 +624,93 @@ else if ('a' <= digit && digit <= 'z')
return sum;
}


/**
* ToNumber applied to the String type
*
* See ECMA 9.3.1
* See the #sec-tonumber-applied-to-the-string-type section of ECMA
*/
public static double toNumber(String s) {
int len = s.length();
final int len = s.length();

// Skip whitespace at the start
int start = 0;
char startChar;
for (;;) {
if (start == len) {
// Empty or contains only whitespace
// empty or contains only whitespace
return +0.0;
}
startChar = s.charAt(start);
if (!ScriptRuntime.isStrWhiteSpaceChar(startChar))
if (!ScriptRuntime.isStrWhiteSpaceChar(startChar)) {
// found first non-whitespace character
break;
}
start++;
}

// Skip whitespace at the end
int end = len - 1;
char endChar;
while (ScriptRuntime.isStrWhiteSpaceChar(endChar = s.charAt(end))) {
end--;
}

// Do not break scripts relying on old non-compliant conversion
// (see bug #368)
// 1. makes ToNumber parse only a valid prefix in hex literals (similar to 'parseInt()')
// ToNumber('0x10 something') => 16
// 2. allows plus and minus signs for hexadecimal numbers
// ToNumber('-0x10') => -16
// 3. disables support for binary ('0b10') and octal ('0o13') literals
// ToNumber('0b1') => NaN
// ToNumber('0o5') => NaN
final Context cx = Context.getCurrentContext();
final boolean oldParsingMode =
cx == null || cx.getLanguageVersion() < Context.VERSION_ES6;

// Handle non-base10 numbers
if (startChar == '0') {
if (start + 2 < len) {
int c1 = s.charAt(start + 1);
if (c1 == 'x' || c1 == 'X') {
// A hexadecimal number
return stringToNumber(s, start + 2, 16);
if (start + 2 <= end) {
final char radixC = s.charAt(start + 1);
int radix = -1;
if (radixC == 'x' || radixC == 'X') {
radix = 16;
} else if (!oldParsingMode && (radixC == 'o' || radixC == 'O')) {
radix = 8;
} else if (!oldParsingMode && (radixC == 'b' || radixC == 'B')) {
radix = 2;
}
if (radix != -1) {
if (oldParsingMode) {
return stringPrefixToNumber(s, start + 2, radix);
}
return stringToNumber(s, start + 2, end, radix);
}
}
} else if (startChar == '+' || startChar == '-') {
if (start + 3 < len && s.charAt(start + 1) == '0') {
int c2 = s.charAt(start + 2);
if (c2 == 'x' || c2 == 'X') {
// A hexadecimal number with sign
double val = stringToNumber(s, start + 3, 16);
} else if (oldParsingMode && (startChar == '+' || startChar == '-')) {
// If in old parsing mode, check for a signed hexadecimal number
if (start + 3 <= end && s.charAt(start + 1) == '0') {
final char radixC = s.charAt(start + 2);
if (radixC == 'x' || radixC == 'X') {
double val = stringPrefixToNumber(s, start + 3, 16);
return startChar == '-' ? -val : val;
}
}
}

int end = len - 1;
char endChar;
while (ScriptRuntime.isStrWhiteSpaceChar(endChar = s.charAt(end)))
end--;
if (endChar == 'y') {
// check for "Infinity"
if (startChar == '+' || startChar == '-')
if (startChar == '+' || startChar == '-') {
start++;
if (start + 7 == end && s.regionMatches(start, "Infinity", 0, 8))
}
if (start + 7 == end && s.regionMatches(start, "Infinity", 0, 8)) {
return startChar == '-'
? Double.NEGATIVE_INFINITY
: Double.POSITIVE_INFINITY;
? Double.NEGATIVE_INFINITY
: Double.POSITIVE_INFINITY;
}
return NaN;
}
// A non-hexadecimal, non-infinity number:
// A base10, non-infinity number:
// just try a normal floating point conversion
String sub = s.substring(start, end+1);
// Quick test to check string contains only valid characters because
Expand Down
2 changes: 1 addition & 1 deletion src/org/mozilla/javascript/TokenStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ final int getToken() throws IOException
return Token.ERROR;
}
} else {
dval = ScriptRuntime.stringToNumber(numString, 0, base);
dval = ScriptRuntime.stringPrefixToNumber(numString, 0, base);
}

this.number = dval;
Expand Down
159 changes: 159 additions & 0 deletions testsrc/org/mozilla/javascript/tests/ToNumberConversionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
package org.mozilla.javascript.tests;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.mozilla.javascript.Context;
import org.mozilla.javascript.Scriptable;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;

import static org.junit.Assert.assertTrue;

/**
* Test cases for ToNumber conversion applied to a String type.
*
* See the #sec-tonumber-applied-to-the-string-type section of ECMA262.
*
* This is only enabled with a language version >= ES6 (200).
*/
@RunWith(Parameterized.class)
public class ToNumberConversionsTest {
private static final int[] OPT_LEVELS = {-1, 0, 9};
private static final Object[][] TESTS = {
// order: expected result, source string
// (0) special
{"-Infinity", " -Infinity "},
{"+Infinity", " +Infinity "},

// (1) bin
{"4", " 0b100 "},
{"5", "0B0000101 "},
// bin, invalid
{"NaN", "-0b100"},
{"NaN", "+0b100"},
{"NaN", "0b100.1"},
{"NaN", "0b100e1"},
{"NaN", "0b100e-1"},
{"NaN", "0b100e+1"},
{"NaN", "0b2"},
{"NaN", "0b"},
{"NaN", "1b1"},

// (2) oct
{"8", " 0o10 "},
{"10", "0O000012 "},
// oct, invalid
{"NaN", "-0o10"},
{"NaN", "+0o10"},
{"NaN", "0o10.1"},
{"NaN", "0o10e1"},
{"NaN", "0o10e-1"},
{"NaN", "0o10e+1"},
{"NaN", "0o9"},
{"NaN", "1o9"},
{"NaN", "0o"},

// (3) dec
{"210", " 210 "},
{"210", "21e1 "},
{"-210", " -0021.00e+1 "},
{"210", " +02100.00e-1"},
// dec, invalid
{"NaN", " 210d2"},
{"NaN", " 210 d2"},

// (4) hex
{"210", " 0x00d2 "},
{"210", " 0x00D2 "},
{"210", " 0X00D2 "},
{"53985", "0xd2e1"}, // 'an exponent without sign' variant is a valid hex literal
// hex, invalid
{"NaN", "-0xd2"},
{"NaN", "+0xd2"},
{"NaN", "0xd2.00"},
{"NaN", "0xd2e+1"},
{"NaN", "0xd2e-1"},
{"NaN", "0xd2g"},
{"NaN", "0xd2 g"},
{"NaN", "0x7f.0x0.0x0.0x1"},
{"NaN", "0x"},
{"NaN", "1xd2"},
{"NaN", "+1xd2"},
{"NaN", "-0x"}
};
private static final String PRELUDE =
"function eq(a,b) {" +
"if (a != a) return b != b;" +
"return a == b;" +
"}\n";

@Parameterized.Parameters(name = "ToNumber(\"{1}\") == {0} (opt={2})")
public static Collection<Object[]> data() {
List<Object[]> cases = new ArrayList<>();

for (int optLevel : OPT_LEVELS) {
for (Object[] test : TESTS) {
cases.add(new Object[]{test[0], test[1], optLevel});
}
}

return cases;
}

@Parameterized.Parameter(0)
public String expected;
@Parameterized.Parameter(1)
public String source;
@Parameterized.Parameter(2)
public int optLevel;

@SuppressWarnings("ConstantConditions")
private boolean execute(Context cx, Scriptable scope, String script) {
return (Boolean) cx.evaluateString(scope, script, "inline", 1, null);
}

public Context cx;
public Scriptable scope;

@Before
public void setup() {
cx = Context.enter();
cx.setOptimizationLevel(optLevel);
cx.setLanguageVersion(Context.VERSION_ES6);
scope = cx.initSafeStandardObjects();
}

@After
public void tearDown() {
Context.exit();
}

@Test
public void test_NumberConstructor() {
String script = String.format("%seq(Number(\"%s\"), %s)", PRELUDE, source, expected);
assertTrue("Number('" + source + "') doesn't produce " + expected, execute(cx, scope, script));
}

@Test
public void test_coercion() {
String script = String.format("%seq(+(\"%s\"), %s)", PRELUDE, source, expected);
assertTrue("+('" + source + "') doesn't produce " + expected, execute(cx, scope, script));
}

@Test
public void test_isNaN() {
String script = String.format("%seq(isNaN(\"%s\"), isNaN(%s))", PRELUDE, source, expected);
assertTrue("isNaN('" + source + "') !== isNaN(" + expected + ")", execute(cx, scope, script));
}

@Test
public void test_isFinite() {
String script = String.format("%seq(isFinite(\"%s\"), isFinite(%s))", PRELUDE, source, expected);
assertTrue("isFinite('" + source + "') !== isFinite(" + expected + ")", execute(cx, scope, script));
}
}
Loading