8028210: Missing conversions on array index expression
Reviewed-by: attila, jlaskey, lagergren
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeArguments.java Thu Nov 14 15:53:49 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeArguments.java Sat Nov 16 00:23:46 2013 +0100
@@ -35,6 +35,7 @@
import java.util.Arrays;
import java.util.BitSet;
import jdk.nashorn.internal.runtime.AccessorProperty;
+import jdk.nashorn.internal.runtime.JSType;
import jdk.nashorn.internal.runtime.Property;
import jdk.nashorn.internal.runtime.PropertyDescriptor;
import jdk.nashorn.internal.runtime.PropertyMap;
@@ -140,8 +141,9 @@
@Override
public boolean delete(final Object key, final boolean strict) {
- final int index = ArrayIndex.getArrayIndex(key);
- return isMapped(index) ? deleteMapped(index, strict) : super.delete(key, strict);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = ArrayIndex.getArrayIndex(primitiveKey);
+ return isMapped(index) ? deleteMapped(index, strict) : super.delete(primitiveKey, strict);
}
/**
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java Thu Nov 14 15:53:49 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeObject.java Sat Nov 16 00:23:46 2013 +0100
@@ -484,10 +484,12 @@
*/
@Function(attributes = Attribute.NOT_ENUMERABLE)
public static Object hasOwnProperty(final Object self, final Object v) {
- final String str = JSType.toString(v);
+ // Convert ScriptObjects to primitive with String.class hint
+ // but no need to convert other primitives to string.
+ final Object key = JSType.toPrimitive(v, String.class);
final Object obj = Global.toObject(self);
- return (obj instanceof ScriptObject) && ((ScriptObject)obj).hasOwnProperty(str);
+ return (obj instanceof ScriptObject) && ((ScriptObject)obj).hasOwnProperty(key);
}
/**
--- a/nashorn/src/jdk/nashorn/internal/objects/NativeString.java Thu Nov 14 15:53:49 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/objects/NativeString.java Sat Nov 16 00:23:46 2013 +0100
@@ -167,11 +167,12 @@
@SuppressWarnings("unused")
private static Object get(final Object self, final Object key) {
final CharSequence cs = JSType.toCharSequence(self);
- final int index = ArrayIndex.getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = ArrayIndex.getArrayIndex(primitiveKey);
if (index >= 0 && index < cs.length()) {
return String.valueOf(cs.charAt(index));
}
- return ((ScriptObject) Global.toObject(self)).get(key);
+ return ((ScriptObject) Global.toObject(self)).get(primitiveKey);
}
@SuppressWarnings("unused")
@@ -202,11 +203,12 @@
// String characters can be accessed with array-like indexing..
@Override
public Object get(final Object key) {
- final int index = ArrayIndex.getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = ArrayIndex.getArrayIndex(primitiveKey);
if (index >= 0 && index < value.length()) {
return String.valueOf(value.charAt(index));
}
- return super.get(key);
+ return super.get(primitiveKey);
}
@Override
@@ -295,8 +297,9 @@
@Override
public boolean has(final Object key) {
- final int index = ArrayIndex.getArrayIndex(key);
- return isValid(index) || super.has(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = ArrayIndex.getArrayIndex(primitiveKey);
+ return isValid(index) || super.has(primitiveKey);
}
@Override
@@ -318,8 +321,9 @@
@Override
public boolean hasOwnProperty(final Object key) {
- final int index = ArrayIndex.getArrayIndex(key);
- return isValid(index) || super.hasOwnProperty(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = ArrayIndex.getArrayIndex(primitiveKey);
+ return isValid(index) || super.hasOwnProperty(primitiveKey);
}
@Override
@@ -358,8 +362,9 @@
@Override
public boolean delete(final Object key, final boolean strict) {
- final int index = ArrayIndex.getArrayIndex(key);
- return checkDeleteIndex(index, strict)? false : super.delete(key, strict);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = ArrayIndex.getArrayIndex(primitiveKey);
+ return checkDeleteIndex(index, strict)? false : super.delete(primitiveKey, strict);
}
private boolean checkDeleteIndex(final int index, final boolean strict) {
--- a/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java Thu Nov 14 15:53:49 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/ScriptObject.java Sat Nov 16 00:23:46 2013 +0100
@@ -2373,11 +2373,13 @@
private int getInt(final int index, final String key) {
if (isValidArrayIndex(index)) {
- for (ScriptObject object = this; ; ) {
- final FindProperty find = object.findProperty(key, false, false, this);
-
- if (find != null) {
- return getIntValue(find);
+ for (ScriptObject object = this; ; ) {
+ if (object.getMap().containsArrayKeys()) {
+ final FindProperty find = object.findProperty(key, false, false, this);
+
+ if (find != null) {
+ return getIntValue(find);
+ }
}
if ((object = object.getProto()) == null) {
@@ -2389,7 +2391,7 @@
if (array.has(index)) {
return array.getInt(index);
}
- }
+ }
} else {
final FindProperty find = findProperty(key, true);
@@ -2403,14 +2405,15 @@
@Override
public int getInt(final Object key) {
- final int index = getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
final ArrayData array = getArray();
if (array.has(index)) {
return array.getInt(index);
}
- return getInt(index, JSType.toString(key));
+ return getInt(index, JSType.toString(primitiveKey));
}
@Override
@@ -2439,22 +2442,25 @@
@Override
public int getInt(final int key) {
+ final int index = getArrayIndex(key);
final ArrayData array = getArray();
- if (array.has(key)) {
- return array.getInt(key);
+ if (array.has(index)) {
+ return array.getInt(index);
}
- return getInt(key, JSType.toString(key));
+ return getInt(index, JSType.toString(key));
}
private long getLong(final int index, final String key) {
if (isValidArrayIndex(index)) {
for (ScriptObject object = this; ; ) {
- final FindProperty find = object.findProperty(key, false, false, this);
-
- if (find != null) {
- return getLongValue(find);
+ if (object.getMap().containsArrayKeys()) {
+ final FindProperty find = object.findProperty(key, false, false, this);
+
+ if (find != null) {
+ return getLongValue(find);
+ }
}
if ((object = object.getProto()) == null) {
@@ -2466,7 +2472,7 @@
if (array.has(index)) {
return array.getLong(index);
}
- }
+ }
} else {
final FindProperty find = findProperty(key, true);
@@ -2480,14 +2486,15 @@
@Override
public long getLong(final Object key) {
- final int index = getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
final ArrayData array = getArray();
if (array.has(index)) {
return array.getLong(index);
}
- return getLong(index, JSType.toString(key));
+ return getLong(index, JSType.toString(primitiveKey));
}
@Override
@@ -2516,22 +2523,25 @@
@Override
public long getLong(final int key) {
+ final int index = getArrayIndex(key);
final ArrayData array = getArray();
- if (array.has(key)) {
- return array.getLong(key);
+ if (array.has(index)) {
+ return array.getLong(index);
}
- return getLong(key, JSType.toString(key));
+ return getLong(index, JSType.toString(key));
}
private double getDouble(final int index, final String key) {
if (isValidArrayIndex(index)) {
for (ScriptObject object = this; ; ) {
- final FindProperty find = object.findProperty(key, false, false, this);
-
- if (find != null) {
- return getDoubleValue(find);
+ if (object.getMap().containsArrayKeys()) {
+ final FindProperty find = object.findProperty(key, false, false, this);
+
+ if (find != null) {
+ return getDoubleValue(find);
+ }
}
if ((object = object.getProto()) == null) {
@@ -2543,7 +2553,7 @@
if (array.has(index)) {
return array.getDouble(index);
}
- }
+ }
} else {
final FindProperty find = findProperty(key, true);
@@ -2557,14 +2567,15 @@
@Override
public double getDouble(final Object key) {
- final int index = getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
final ArrayData array = getArray();
if (array.has(index)) {
return array.getDouble(index);
}
- return getDouble(index, JSType.toString(key));
+ return getDouble(index, JSType.toString(primitiveKey));
}
@Override
@@ -2593,22 +2604,25 @@
@Override
public double getDouble(final int key) {
+ final int index = getArrayIndex(key);
final ArrayData array = getArray();
- if (array.has(key)) {
- return array.getDouble(key);
+ if (array.has(index)) {
+ return array.getDouble(index);
}
- return getDouble(key, JSType.toString(key));
+ return getDouble(index, JSType.toString(key));
}
private Object get(final int index, final String key) {
if (isValidArrayIndex(index)) {
for (ScriptObject object = this; ; ) {
- final FindProperty find = object.findProperty(key, false, false, this);
-
- if (find != null) {
- return getObjectValue(find);
+ if (object.getMap().containsArrayKeys()) {
+ final FindProperty find = object.findProperty(key, false, false, this);
+
+ if (find != null) {
+ return getObjectValue(find);
+ }
}
if ((object = object.getProto()) == null) {
@@ -2634,14 +2648,15 @@
@Override
public Object get(final Object key) {
- final int index = getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
final ArrayData array = getArray();
if (array.has(index)) {
return array.getObject(index);
}
- return get(index, JSType.toString(key));
+ return get(index, JSType.toString(primitiveKey));
}
@Override
@@ -2670,13 +2685,14 @@
@Override
public Object get(final int key) {
+ final int index = getArrayIndex(key);
final ArrayData array = getArray();
- if (array.has(key)) {
- return array.getObject(key);
+ if (array.has(index)) {
+ return array.getObject(index);
}
- return get(key, JSType.toString(key));
+ return get(index, JSType.toString(key));
}
/**
@@ -2688,7 +2704,7 @@
*/
private void doesNotHave(final int index, final Object value, final boolean strict) {
final long oldLength = getArray().length();
- final long longIndex = index & JSType.MAX_UINT;
+ final long longIndex = ArrayIndex.toLongIndex(index);
if (getMap().containsArrayKeys()) {
final String key = JSType.toString(longIndex);
@@ -2774,7 +2790,8 @@
@Override
public void set(final Object key, final int value, final boolean strict) {
- final int index = getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
if (isValidArrayIndex(index)) {
if (getArray().has(index)) {
@@ -2786,13 +2803,14 @@
return;
}
- final String propName = JSType.toString(key);
+ final String propName = JSType.toString(primitiveKey);
setObject(findProperty(propName, true), strict, propName, JSType.toObject(value));
}
@Override
public void set(final Object key, final long value, final boolean strict) {
- final int index = getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
if (isValidArrayIndex(index)) {
if (getArray().has(index)) {
@@ -2804,13 +2822,14 @@
return;
}
- final String propName = JSType.toString(key);
+ final String propName = JSType.toString(primitiveKey);
setObject(findProperty(propName, true), strict, propName, JSType.toObject(value));
}
@Override
public void set(final Object key, final double value, final boolean strict) {
- final int index = getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
if (isValidArrayIndex(index)) {
if (getArray().has(index)) {
@@ -2822,13 +2841,14 @@
return;
}
- final String propName = JSType.toString(key);
+ final String propName = JSType.toString(primitiveKey);
setObject(findProperty(propName, true), strict, propName, JSType.toObject(value));
}
@Override
public void set(final Object key, final Object value, final boolean strict) {
- final int index = getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
if (isValidArrayIndex(index)) {
if (getArray().has(index)) {
@@ -2840,7 +2860,7 @@
return;
}
- final String propName = JSType.toString(key);
+ final String propName = JSType.toString(primitiveKey);
setObject(findProperty(propName, true), strict, propName, value);
}
@@ -3062,82 +3082,69 @@
@Override
public boolean has(final Object key) {
- final int index = getArrayIndex(key);
-
- if (isValidArrayIndex(index)) {
- for (ScriptObject self = this; self != null; self = self.getProto()) {
- if (self.getArray().has(index)) {
- return true;
- }
- }
- }
-
- return hasProperty(JSType.toString(key), true);
+ final Object primitiveKey = JSType.toPrimitive(key);
+ final int index = getArrayIndex(primitiveKey);
+ return isValidArrayIndex(index) ? hasArrayProperty(index) : hasProperty(JSType.toString(primitiveKey), true);
}
@Override
public boolean has(final double key) {
final int index = getArrayIndex(key);
-
- if (isValidArrayIndex(index)) {
- for (ScriptObject self = this; self != null; self = self.getProto()) {
- if (self.getArray().has(index)) {
- return true;
- }
- }
- }
-
- return hasProperty(JSType.toString(key), true);
+ return isValidArrayIndex(index) ? hasArrayProperty(index) : hasProperty(JSType.toString(key), true);
}
@Override
public boolean has(final long key) {
final int index = getArrayIndex(key);
-
- if (isValidArrayIndex(index)) {
- for (ScriptObject self = this; self != null; self = self.getProto()) {
- if (self.getArray().has(index)) {
- return true;
- }
- }
- }
-
- return hasProperty(JSType.toString(key), true);
+ return isValidArrayIndex(index) ? hasArrayProperty(index) : hasProperty(JSType.toString(key), true);
}
@Override
public boolean has(final int key) {
final int index = getArrayIndex(key);
-
- if (isValidArrayIndex(index)) {
- for (ScriptObject self = this; self != null; self = self.getProto()) {
- if (self.getArray().has(index)) {
- return true;
- }
+ return isValidArrayIndex(index) ? hasArrayProperty(index) : hasProperty(JSType.toString(key), true);
+ }
+
+ private boolean hasArrayProperty(final int index) {
+ boolean hasArrayKeys = false;
+
+ for (ScriptObject self = this; self != null; self = self.getProto()) {
+ if (self.getArray().has(index)) {
+ return true;
}
+ hasArrayKeys = hasArrayKeys || self.getMap().containsArrayKeys();
}
- return hasProperty(JSType.toString(key), true);
+ return hasArrayKeys && hasProperty(ArrayIndex.toKey(index), true);
}
@Override
public boolean hasOwnProperty(final Object key) {
- return getArray().has(getArrayIndex(key)) || hasProperty(JSType.toString(key), false);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
+ return isValidArrayIndex(index) ? hasOwnArrayProperty(index) : hasProperty(JSType.toString(primitiveKey), false);
}
@Override
public boolean hasOwnProperty(final int key) {
- return getArray().has(getArrayIndex(key)) || hasProperty(JSType.toString(key), false);
+ final int index = getArrayIndex(key);
+ return isValidArrayIndex(index) ? hasOwnArrayProperty(index) : hasProperty(JSType.toString(key), false);
}
@Override
public boolean hasOwnProperty(final long key) {
- return getArray().has(getArrayIndex(key)) || hasProperty(JSType.toString(key), false);
+ final int index = getArrayIndex(key);
+ return isValidArrayIndex(index) ? hasOwnArrayProperty(index) : hasProperty(JSType.toString(key), false);
}
@Override
public boolean hasOwnProperty(final double key) {
- return getArray().has(getArrayIndex(key)) || hasProperty(JSType.toString(key), false);
+ final int index = getArrayIndex(key);
+ return isValidArrayIndex(index) ? hasOwnArrayProperty(index) : hasProperty(JSType.toString(key), false);
+ }
+
+ private boolean hasOwnArrayProperty(final int index) {
+ return getArray().has(index) || (getMap().containsArrayKeys() && hasProperty(ArrayIndex.toKey(index), false));
}
@Override
@@ -3190,7 +3197,8 @@
@Override
public boolean delete(final Object key, final boolean strict) {
- final int index = getArrayIndex(key);
+ final Object primitiveKey = JSType.toPrimitive(key, String.class);
+ final int index = getArrayIndex(primitiveKey);
final ArrayData array = getArray();
if (array.has(index)) {
@@ -3201,7 +3209,7 @@
return false;
}
- return deleteObject(key, strict);
+ return deleteObject(primitiveKey, strict);
}
private boolean deleteObject(final Object key, final boolean strict) {
--- a/nashorn/src/jdk/nashorn/internal/runtime/arrays/ArrayIndex.java Thu Nov 14 15:53:49 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/arrays/ArrayIndex.java Sat Nov 16 00:23:46 2013 +0100
@@ -27,6 +27,7 @@
import jdk.nashorn.internal.runtime.ConsString;
import jdk.nashorn.internal.runtime.JSType;
+import jdk.nashorn.internal.runtime.ScriptObject;
/**
* Array index computation helpers. that both throw exceptions or return
@@ -80,7 +81,12 @@
* Returns a valid array index in an int, if the object represents one. This
* routine needs to perform quickly since all keys are tested with it.
*
- * @param key key to check for array index
+ * <p>The {@code key} parameter must be a JavaScript primitive type, i.e. one of
+ * {@code String}, {@code Number}, {@code Boolean}, {@code null}, or {@code undefined}.
+ * {@code ScriptObject} instances should be converted to primitive with
+ * {@code String.class} hint before being passed to this method.</p>
+ *
+ * @param key key to check for array index.
* @return the array index, or {@code -1} if {@code key} does not represent a valid index.
* Note that negative return values other than {@code -1} are considered valid and can be converted to
* the actual index using {@link #toLongIndex(int)}.
@@ -88,18 +94,31 @@
public static int getArrayIndex(final Object key) {
if (key instanceof Integer) {
return getArrayIndex(((Integer) key).intValue());
- } else if (key instanceof Number) {
- return getArrayIndex(((Number) key).doubleValue());
+ } else if (key instanceof Double) {
+ return getArrayIndex(((Double) key).doubleValue());
} else if (key instanceof String) {
return (int)fromString((String) key);
+ } else if (key instanceof Long) {
+ return getArrayIndex(((Long) key).longValue());
} else if (key instanceof ConsString) {
return (int)fromString(key.toString());
}
+ assert !(key instanceof ScriptObject);
return INVALID_ARRAY_INDEX;
}
/**
+ * Returns a valid array index in an int, if {@code key} represents one.
+ *
+ * @param key key to check
+ * @return the array index, or {@code -1} if {@code key} is not a valid array index.
+ */
+ public static int getArrayIndex(final int key) {
+ return (key >= 0) ? key : INVALID_ARRAY_INDEX;
+ }
+
+ /**
* Returns a valid array index in an int, if the long represents one.
*
* @param key key to check
@@ -129,10 +148,7 @@
*/
public static int getArrayIndex(final double key) {
if (JSType.isRepresentableAsInt(key)) {
- final int intKey = (int)key;
- if (intKey >= 0) {
- return intKey;
- }
+ return getArrayIndex((int) key);
} else if (JSType.isRepresentableAsLong(key)) {
return getArrayIndex((long) key);
}
@@ -177,5 +193,16 @@
return index & JSType.MAX_UINT;
}
+ /**
+ * Convert an index to a key string. This is the same as calling {@link #toLongIndex(int)}
+ * and converting the result to String.
+ *
+ * @param index index to convert
+ * @return index as string
+ */
+ public static String toKey(final int index) {
+ return Long.toString(index & JSType.MAX_UINT);
+ }
+
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8028210.js Sat Nov 16 00:23:46 2013 +0100
@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8028210: Missing conversions on array index expression
+ *
+ * @test
+ * @run
+ */
+
+var array = [1, 2];
+var key1 = [[[0]]];
+var key2 = new String("1");
+var key3 = {
+ toString: function() {
+ print("toString called");
+ return "2";
+ }
+};
+
+print(array[key1]);
+print(array[key2]);
+array[key3] = 3;
+print(array[key3]);
+print(key3 in array);
+print(array.hasOwnProperty(key3));
+print(delete array[key3]);
+print(array[key3]);
+
+// string access
+print("abc"[key1]);
+print("abc"[key2]);
+print("abc"[key3]);
+
+// arguments object
+(function(a, b, c) {
+ print(arguments[key3]);
+ delete arguments[key3];
+ print(arguments[key3], c);
+})(1, 2, 3);
+
+// int keys
+array = [];
+array[4294967294] = 1;
+print(array[-2]);
+print(array[4294967294]);
+print(-2 in array);
+print(4294967294 in array);
+print(delete(array[-2]));
+print(array[4294967294]);
+print(delete(array[4294967294]));
+print(array[4294967294]);
+
+array = [];
+array[-2] = 1;
+print(array[-2]);
+print(array[4294967294]);
+print(-2 in array);
+print(4294967294 in array);
+print(delete(array[4294967294]));
+print(array[-2]);
+print(delete(array[-2]));
+print(array[-2]);
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/nashorn/test/script/basic/JDK-8028210.js.EXPECTED Sat Nov 16 00:23:46 2013 +0100
@@ -0,0 +1,38 @@
+1
+2
+toString called
+toString called
+3
+toString called
+true
+toString called
+true
+toString called
+true
+toString called
+undefined
+a
+b
+toString called
+c
+toString called
+3
+toString called
+toString called
+undefined 3
+undefined
+1
+false
+true
+true
+1
+true
+undefined
+1
+undefined
+true
+false
+true
+1
+true
+undefined