# HG changeset patch # User hannesw # Date 1384557826 -3600 # Node ID a165c0fb5be6bb7dd9ebb416c99a6bd2e3c5849a # Parent fea9f0f9bbde562b4808eee724d4f93996c6d519 8028210: Missing conversions on array index expression Reviewed-by: attila, jlaskey, lagergren diff -r fea9f0f9bbde -r a165c0fb5be6 src/jdk/nashorn/internal/objects/NativeArguments.java --- a/src/jdk/nashorn/internal/objects/NativeArguments.java Thu Nov 14 15:53:49 2013 +0530 +++ b/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); } /** diff -r fea9f0f9bbde -r a165c0fb5be6 src/jdk/nashorn/internal/objects/NativeObject.java --- a/src/jdk/nashorn/internal/objects/NativeObject.java Thu Nov 14 15:53:49 2013 +0530 +++ b/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); } /** diff -r fea9f0f9bbde -r a165c0fb5be6 src/jdk/nashorn/internal/objects/NativeString.java --- a/src/jdk/nashorn/internal/objects/NativeString.java Thu Nov 14 15:53:49 2013 +0530 +++ b/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) { diff -r fea9f0f9bbde -r a165c0fb5be6 src/jdk/nashorn/internal/runtime/ScriptObject.java --- a/src/jdk/nashorn/internal/runtime/ScriptObject.java Thu Nov 14 15:53:49 2013 +0530 +++ b/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) { diff -r fea9f0f9bbde -r a165c0fb5be6 src/jdk/nashorn/internal/runtime/arrays/ArrayIndex.java --- a/src/jdk/nashorn/internal/runtime/arrays/ArrayIndex.java Thu Nov 14 15:53:49 2013 +0530 +++ b/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 + *

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.

+ * + * @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); + } + } diff -r fea9f0f9bbde -r a165c0fb5be6 test/script/basic/JDK-8028210.js --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/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]); diff -r fea9f0f9bbde -r a165c0fb5be6 test/script/basic/JDK-8028210.js.EXPECTED --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/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