From 50b3e7433fff2d012694c9e89e5163e7f634d3ee Mon Sep 17 00:00:00 2001 From: Greg Brail Date: Fri, 6 Oct 2023 16:28:51 -0700 Subject: [PATCH] Make handling of object indices more in line with the spec * In more cases, treat out-of-range numeric indices as strings * For typed arrays, correctly implement index semantics --- src/org/mozilla/javascript/ScriptRuntime.java | 45 +++++++++++-- .../typedarrays/NativeTypedArrayView.java | 66 ++++++++++++++++++- testsrc/jstests/harmony/typedarrays.js | 10 +-- .../mozilla/javascript/tests/IndexTest.java | 59 +++++++++++++++++ testsrc/test262.properties | 3 +- 5 files changed, 165 insertions(+), 18 deletions(-) create mode 100644 testsrc/org/mozilla/javascript/tests/IndexTest.java diff --git a/src/org/mozilla/javascript/ScriptRuntime.java b/src/org/mozilla/javascript/ScriptRuntime.java index 825eb2537c..f6641cf23a 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__"; @@ -1540,7 +1562,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 +1586,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 +1599,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 +1632,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 +1756,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 +1860,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/typedarrays/NativeTypedArrayView.java b/src/org/mozilla/javascript/typedarrays/NativeTypedArrayView.java index 0864fbfc8b..f74c61554f 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.isEmpty()) { + // 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/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]}