diff --git a/src/org/mozilla/javascript/ScriptRuntime.java b/src/org/mozilla/javascript/ScriptRuntime.java index 825eb2537c..2a02469156 100644 --- a/src/org/mozilla/javascript/ScriptRuntime.java +++ b/src/org/mozilla/javascript/ScriptRuntime.java @@ -14,6 +14,7 @@ import java.text.MessageFormat; import java.util.Arrays; import java.util.Locale; +import java.util.Optional; import java.util.ResourceBundle; import java.util.function.BiConsumer; import org.mozilla.javascript.ast.FunctionNode; @@ -1382,6 +1383,27 @@ public static char toUint16(Object val) { return (char) DoubleConversion.doubleToInt32(d); } + /** + * If "arg" is a "canonical numeric index," which means any number constructed from a string + * that doesn't have extra whitespace or non-standard formatting, return it -- otherwise return + * an empty option. Defined in ECMA 7.1.21. + */ + public static Optional canonicalNumericIndexString(String arg) { + if ("-0".equals(arg)) { + return Optional.of(Double.NEGATIVE_INFINITY); + } + double num = toNumber(arg); + // According to tests, "NaN" is not a number ;-) + if (Double.isNaN(num)) { + return Optional.empty(); + } + String numStr = toString(num); + if (numStr.equals(arg)) { + return Optional.of(num); + } + return Optional.empty(); + } + // XXX: this is until setDefaultNamespace will learn how to store NS // properly and separates namespace form Scriptable.get etc. private static final String DEFAULT_NS_TAG = "__default_namespace__"; @@ -1454,6 +1476,10 @@ static Function getExistingCtor(Context cx, Scriptable scope, String constructor * Return -1L if str is not an index, or the index value as lower 32 bits of the result. Note * that the result needs to be cast to an int in order to produce the actual index, which may be * negative. + * + *

Note that this method on its own does not actually produce an index that is useful for an + * actual Object or Array, because it may be larger than Integer.MAX_VALUE. Most callers should + * instead call toStringOrIndex, which calls this under the covers. */ public static long indexFromString(String str) { // The length of the decimal string representation of @@ -1540,7 +1566,7 @@ public static long testUint32String(String str) { /** If s represents index, then return index value wrapped as Integer and othewise return s. */ static Object getIndexObject(String s) { long indexTest = indexFromString(s); - if (indexTest >= 0) { + if (indexTest >= 0 && indexTest <= Integer.MAX_VALUE) { return Integer.valueOf((int) indexTest); } return s; @@ -1564,7 +1590,7 @@ static Object getIndexObject(double d) { * * @see ScriptRuntime#toStringIdOrIndex(Context, Object) */ - static final class StringIdOrIndex { + public static final class StringIdOrIndex { final String stringId; final int index; @@ -1577,15 +1603,26 @@ static final class StringIdOrIndex { this.stringId = null; this.index = index; } + + public String getStringId() { + return stringId; + } + + public int getIndex() { + return index; + } } /** * If id is a number or a string presentation of an int32 value, then id the returning * StringIdOrIndex has the index set, otherwise the stringId is set. */ - static StringIdOrIndex toStringIdOrIndex(Object id) { + public static StringIdOrIndex toStringIdOrIndex(Object id) { if (id instanceof Number) { double d = ((Number) id).doubleValue(); + if (d < 0.0) { + return new StringIdOrIndex(toString(id)); + } int index = (int) d; if (index == d) { return new StringIdOrIndex(index); @@ -1599,7 +1636,7 @@ static StringIdOrIndex toStringIdOrIndex(Object id) { s = toString(id); } long indexTest = indexFromString(s); - if (indexTest >= 0) { + if (indexTest >= 0 && indexTest <= Integer.MAX_VALUE) { return new StringIdOrIndex((int) indexTest); } return new StringIdOrIndex(s); @@ -1723,7 +1760,7 @@ public static Object getObjectIndex(Object obj, double dblIndex, Context cx, Scr } int index = (int) dblIndex; - if (index == dblIndex) { + if (index == dblIndex && index >= 0) { return getObjectIndex(sobj, index, cx); } String s = toString(dblIndex); @@ -1827,7 +1864,7 @@ public static Object setObjectIndex( } int index = (int) dblIndex; - if (index == dblIndex) { + if (index == dblIndex && index >= 0) { return setObjectIndex(sobj, index, value, cx); } String s = toString(dblIndex); diff --git a/src/org/mozilla/javascript/json/JsonParser.java b/src/org/mozilla/javascript/json/JsonParser.java index b07f2ee0f7..e00b0ac7ed 100644 --- a/src/org/mozilla/javascript/json/JsonParser.java +++ b/src/org/mozilla/javascript/json/JsonParser.java @@ -10,6 +10,7 @@ import java.util.List; import org.mozilla.javascript.Context; import org.mozilla.javascript.ScriptRuntime; +import org.mozilla.javascript.ScriptRuntime.StringIdOrIndex; import org.mozilla.javascript.Scriptable; /** @@ -118,13 +119,12 @@ private Object readObject() throws ParseException { consume(':'); value = readValue(); - long index = ScriptRuntime.indexFromString(id); - if (index < 0) { - object.put(id, object, value); + StringIdOrIndex indexObj = ScriptRuntime.toStringIdOrIndex(id); + if (indexObj.getStringId() == null) { + object.put(indexObj.getIndex(), object, value); } else { - object.put((int) index, object, value); + object.put(indexObj.getStringId(), object, value); } - needsComma = true; break; default: diff --git a/src/org/mozilla/javascript/typedarrays/NativeTypedArrayView.java b/src/org/mozilla/javascript/typedarrays/NativeTypedArrayView.java index 0864fbfc8b..f97116055e 100644 --- a/src/org/mozilla/javascript/typedarrays/NativeTypedArrayView.java +++ b/src/org/mozilla/javascript/typedarrays/NativeTypedArrayView.java @@ -11,6 +11,7 @@ import java.util.Iterator; import java.util.List; import java.util.ListIterator; +import java.util.Optional; import java.util.RandomAccess; import org.mozilla.javascript.Context; import org.mozilla.javascript.ExternalArrayData; @@ -46,26 +47,77 @@ protected NativeTypedArrayView(NativeArrayBuffer ab, int off, int len, int byteL length = len; } - // Array properties implementation + // Array properties implementation. + // Typed array objects are "Integer-indexed exotic objects" in the ECMAScript spec. + // Integer properties, and string properties that can be converted to integer indices, + // behave differently than in other types of JavaScript objects, in that they are + // silently ignored (and always valued as "undefined") when they are out of bounds. @Override public Object get(int index, Scriptable start) { return js_get(index); } + @Override + public Object get(String name, Scriptable start) { + Optional num = ScriptRuntime.canonicalNumericIndexString(name); + if (num.isPresent()) { + // Now we had a valid number, so no matter what we try to return an array element + int ix = toIndex(num.get()); + if (ix >= 0) { + return js_get(ix); + } + } + return super.get(name, start); + } + @Override public boolean has(int index, Scriptable start) { return !checkIndex(index); } + @Override + public boolean has(String name, Scriptable start) { + Optional num = ScriptRuntime.canonicalNumericIndexString(name); + if (num.isPresent()) { + int ix = toIndex(num.get()); + if (ix >= 0) { + return !checkIndex(ix); + } + } + return super.has(name, start); + } + @Override public void put(int index, Scriptable start, Object val) { js_set(index, val); } + @Override + public void put(String name, Scriptable start, Object val) { + Optional num = ScriptRuntime.canonicalNumericIndexString(name); + if (num.isPresent()) { + int ix = toIndex(num.get()); + if (ix >= 0) { + js_set(ix, val); + } + } else { + super.put(name, start, val); + } + } + @Override public void delete(int index) {} + @Override + public void delete(String name) { + Optional num = ScriptRuntime.canonicalNumericIndexString(name); + if (!num.isPresent()) { + // No delete for indexed elements, so only delete if "name" is not a number + super.delete(name); + } + } + @Override public Object[] getIds() { Object[] ret = new Object[length]; @@ -75,6 +127,18 @@ public Object[] getIds() { return ret; } + /** + * To aid in parsing: Return a positive (or zero) integer if the double is a valid array index, + * and -1 if not. + */ + private static int toIndex(double num) { + int ix = (int) num; + if (ix == num && ix >= 0) { + return ix; + } + return -1; + } + // Actual functions protected boolean checkIndex(int index) { diff --git a/testsrc/jstests/harmony/typedarrays.js b/testsrc/jstests/harmony/typedarrays.js index 8bf648abba..89f37f5e0b 100644 --- a/testsrc/jstests/harmony/typedarrays.js +++ b/testsrc/jstests/harmony/typedarrays.js @@ -531,26 +531,22 @@ function TestTypedArraysWithIllegalIndices() { assertEquals(10, a["-1e2"]); assertEquals(undefined, a[-1e2]); - /* Rhino: Seems to think that "-0" is a real index. Not going to change that here. a["-0"] = 256; var s2 = " -0"; a[s2] = 255; assertEquals(undefined, a["-0"]); assertEquals(255, a[s2]); assertEquals(0, a[-0]); - */ /* Chromium bug: 424619 * a[-Infinity] = 50; * assertEquals(undefined, a[-Infinity]); */ - /* Rhino: Seems to think that 1.5 is a real index. Not going to change that here. a[1.5] = 10; assertEquals(undefined, a[1.5]); var nan = Math.sqrt(-1); a[nan] = 5; assertEquals(5, a[nan]); - */ var x = 0; var y = -0; @@ -584,26 +580,22 @@ function TestTypedArraysWithIllegalIndicesStrict() { assertEquals(10, a["-1e2"]); assertEquals(undefined, a[-1e2]); - /* Rhino: Seems to think that "-0" is a real index. Not going to change that here. - a["-0"] = 256; + a["-0"] = 256; var s2 = " -0"; a[s2] = 255; assertEquals(undefined, a["-0"]); assertEquals(255, a[s2]); assertEquals(0, a[-0]); - */ /* Chromium bug: 424619 * a[-Infinity] = 50; * assertEquals(undefined, a[-Infinity]); */ - /* Rhino: Seems to think that 1.5 is a real index. Not going to change that here. a[1.5] = 10; assertEquals(undefined, a[1.5]); var nan = Math.sqrt(-1); a[nan] = 5; assertEquals(5, a[nan]); - */ var x = 0; var y = -0; diff --git a/testsrc/org/mozilla/javascript/tests/IndexTest.java b/testsrc/org/mozilla/javascript/tests/IndexTest.java new file mode 100644 index 0000000000..1c7164dd47 --- /dev/null +++ b/testsrc/org/mozilla/javascript/tests/IndexTest.java @@ -0,0 +1,59 @@ +package org.mozilla.javascript.tests; + +import static org.junit.Assert.*; + +import org.junit.Test; +import org.mozilla.javascript.ScriptRuntime; + +public class IndexTest { + private void expectInteger(ScriptRuntime.StringIdOrIndex id, int v) { + assertEquals(v, id.getIndex()); + assertNull(id.getStringId()); + } + + private void expectString(ScriptRuntime.StringIdOrIndex id, String s) { + assertEquals(s, id.getStringId()); + } + + @Test + public void testNumericIndices() { + // Normal integers + expectInteger(ScriptRuntime.toStringIdOrIndex(0), 0); + expectInteger(ScriptRuntime.toStringIdOrIndex(1), 1); + expectInteger(ScriptRuntime.toStringIdOrIndex(Integer.MAX_VALUE), Integer.MAX_VALUE); + // Negative integers + expectString(ScriptRuntime.toStringIdOrIndex(-1), "-1"); + expectString(ScriptRuntime.toStringIdOrIndex(Integer.MIN_VALUE), "-2147483648"); + // Floating-point -- but rounding is weird so just check nullness + ScriptRuntime.StringIdOrIndex id; + id = ScriptRuntime.toStringIdOrIndex(1.1f); + assertNotNull(id.getStringId()); + } + + @Test + public void testStringIndices() { + // Normal integers + expectInteger(ScriptRuntime.toStringIdOrIndex("0"), 0); + expectInteger(ScriptRuntime.toStringIdOrIndex("1"), 1); + expectInteger( + ScriptRuntime.toStringIdOrIndex(String.valueOf(Integer.MAX_VALUE)), + Integer.MAX_VALUE); + // Negative integers + expectString(ScriptRuntime.toStringIdOrIndex("-1"), "-1"); + expectString( + ScriptRuntime.toStringIdOrIndex(String.valueOf(Integer.MIN_VALUE)), "-2147483648"); + // Floating-point + expectString(ScriptRuntime.toStringIdOrIndex("3.14"), "3.14"); + expectString(ScriptRuntime.toStringIdOrIndex("1.1"), "1.1"); + // Out of range + expectString( + ScriptRuntime.toStringIdOrIndex(String.valueOf(Long.MAX_VALUE)), + "9223372036854775807"); + // Others + expectString(ScriptRuntime.toStringIdOrIndex(Double.NaN), "NaN"); + // Junk + expectString( + ScriptRuntime.toStringIdOrIndex("This is not an integer"), + "This is not an integer"); + } +} diff --git a/testsrc/org/mozilla/javascript/tests/json/JsonParserTest.java b/testsrc/org/mozilla/javascript/tests/json/JsonParserTest.java index 5ce6a09000..65498905dc 100644 --- a/testsrc/org/mozilla/javascript/tests/json/JsonParserTest.java +++ b/testsrc/org/mozilla/javascript/tests/json/JsonParserTest.java @@ -4,6 +4,7 @@ package org.mozilla.javascript.tests.json; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import org.junit.After; @@ -119,7 +120,6 @@ public void shouldParseEmptyJsonArray() throws Exception { } @Test - @SuppressWarnings("unchecked") public void shouldParseHeterogeneousJsonArray() throws Exception { NativeArray actual = (NativeArray) parser.parseValue("[ \"hello\" , 3, null, [false] ]"); assertEquals("hello", actual.get(0, actual)); @@ -138,18 +138,37 @@ public void shouldFailToParseArrayWithInvalidElements() throws Exception { } @Test - @SuppressWarnings({"serial", "unchecked"}) public void shouldParseJsonObject() throws Exception { String json = "{" + "\"bool\" : false, " + "\"str\" : \"xyz\", " + "\"obj\" : {\"a\":1} " + "}"; NativeObject actual = (NativeObject) parser.parseValue(json); assertEquals(false, actual.get("bool", actual)); assertEquals("xyz", actual.get("str", actual)); + assertArrayEquals( + "Property ordering should match", + new Object[] {"bool", "str", "obj"}, + actual.getIds()); NativeObject innerObj = (NativeObject) actual.get("obj", actual); assertEquals(1, innerObj.get("a", innerObj)); } + @Test + public void testECMAKeyOrdering() throws Exception { + try (Context cx = Context.enter()) { + cx.setLanguageVersion(Context.VERSION_ES6); + String json = + "{\"foo\": \"a\", \"bar\": \"b\", \"1\": \"c\", \"-1\": \"d\", \"x\": \"e\"}"; + NativeObject actual = (NativeObject) parser.parseValue(json); + // Ensure that modern ECMAScript property ordering works, which depends on + // valid index values being treated as numbers and not as strings. + assertArrayEquals( + "Property ordering should match", + new Object[] {1, "foo", "bar", "-1", "x"}, + actual.getIds()); + } + } + @Test(expected = ParseException.class) public void shouldFailToParseJsonObjectsWithInvalidFormat() throws Exception { parser.parseValue("{\"only\", \"keys\"}"); diff --git a/testsrc/test262.properties b/testsrc/test262.properties index 57d2fba45b..a08d684d80 100644 --- a/testsrc/test262.properties +++ b/testsrc/test262.properties @@ -696,7 +696,7 @@ built-ins/isNaN 8/16 (50.0%) ~built-ins/IteratorPrototype -built-ins/JSON 35/140 (25.0%) +built-ins/JSON 34/140 (24.29%) parse/builtin.js {unsupported: [Reflect.construct]} parse/revived-proxy.js {unsupported: [Proxy]} parse/revived-proxy-revoked.js {unsupported: [Proxy]} @@ -715,7 +715,6 @@ built-ins/JSON 35/140 (25.0%) parse/text-negative-zero.js stringify/builtin.js {unsupported: [Reflect.construct]} stringify/replacer-array-abrupt.js {unsupported: [Proxy]} - stringify/replacer-array-number.js stringify/replacer-array-proxy.js {unsupported: [Proxy]} stringify/replacer-array-proxy-revoked.js {unsupported: [Proxy]} stringify/replacer-array-proxy-revoked-realm.js {unsupported: [Proxy]}