Skip to content

Commit

Permalink
Make handling of object indices more in line with the spec
Browse files Browse the repository at this point in the history 
* In more cases, treat out-of-range numeric indices as strings
* For typed arrays, correctly implement index semantics
  • Loading branch information
@gbrail
gbrail committed Oct 6, 2023
1 parent d421fe3 commit 50b3e74
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 18 deletions.
45 changes: 39 additions & 6 deletions src/org/mozilla/javascript/ScriptRuntime.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Double> 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__";
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
66 changes: 65 additions & 1 deletion src/org/mozilla/javascript/typedarrays/NativeTypedArrayView.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Double> 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<Double> 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<Double> 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<Double> 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];
Expand All @@ -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) {
Expand Down
10 changes: 1 addition & 9 deletions testsrc/jstests/harmony/typedarrays.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
59 changes: 59 additions & 0 deletions testsrc/org/mozilla/javascript/tests/IndexTest.java
View file
Original file line number Diff line number Diff line change
@@ -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");
}
}
3 changes: 1 addition & 2 deletions testsrc/test262.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]}
Expand All @@ -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]}
Expand Down

0 comments on commit 50b3e74

Please sign in to comment.