Bug report: JavaScript arrays are not coerced into Java arrays when calling Java functions that expect arrays
Tal Liron
tal.liron at threecrickets.com
Fri Oct 11 03:59:55 PDT 2013
Attached is my working patch. I've signed the developer agreement and
sent it to the Oracle as required.
I've marked all my changes with a "// TAL" comment so you can easily
find them.
Here is the explanation:
JavaArgumentConverters.java: I've added new converters for all primitive
array types, as well as Object[] and String[]. For NativeArrays, it
would convert all the individual elements. For other object types, it
would attempt to create a one-element array, assuming that element is
convertible. I added a few utility methods, too, to help with this. I
also took the liberty to make a change to toString: it will fallback to
Object.toString. (I have a feeling some of you will not like this
change, as it's not directly related to the feature, but I see no harm
in it, and it much helped my testing. In fact, it's even Java's behavior
when working with non-string objects. Without this change, JavaScript
programmers would still have to work hard to set up arrays before
sending them to Java.)
NashornPrimitiveLinker.java: canLinkTypeStatic now recognizes all the
supported primitive array types. In compareConversion, I've also added a
check to prefer conversion to arrays over conversion to primitives.
Without this, an ambiguous exception could be thrown in cases where two
similar method signatures exist. (For example, java.lang.Runtime.execute
has a version that accepts a string array and another that accepts a
string.) I think this can be further improved to prefer Object[] over
String[] if there is such ambiguity. What I need help with from you guys
is in getGuardedInvocation: I currently return null for NativeArray,
because I'm not sure how to get an appropriate guard. Currently
primitiveLookup only supports non-array primitive types. I think what
happens in this case is that a default guard is provided, but I'm not
entirely sure.
Guards.java: I've added extra checks to avoid the
isInstanceGuardAlwaysFalse log warning for array conversions.
On 10/09/2013 10:28 PM, Jim Laskey wrote:
> If you can get the changes to me by Tuesday Oct 15th, I'll take a look. No guarantees, but if the changes are small, are correct, and do not restrict future enhancements in this area then I'll run the changes up the approval chain.
>
>
-------------- next part --------------
diff --git a/src/jdk/internal/dynalink/support/Guards.java b/src/jdk/internal/dynalink/support/Guards.java
--- a/src/jdk/internal/dynalink/support/Guards.java
+++ b/src/jdk/internal/dynalink/support/Guards.java
@@ -155,7 +155,18 @@
LOG.log(Level.WARNING, "isInstanceGuardAlwaysTrue", new Object[] { clazz.getName(), pos, type });
return constantTrue(type);
}
- if(!declaredType.isAssignableFrom(clazz)) {
+ if(!declaredType.isAssignableFrom(clazz) &&
+ // TAL
+ clazz != Object[].class &&
+ clazz != String[].class &&
+ clazz != boolean[].class &&
+ clazz != float[].class &&
+ clazz != double[].class &&
+ clazz != byte[].class &&
+ clazz != short[].class &&
+ clazz != int[].class &&
+ clazz != long[].class &&
+ clazz != char[].class) {
LOG.log(Level.WARNING, "isInstanceGuardAlwaysFalse", new Object[] { clazz.getName(), pos, type });
return constantFalse(type);
}
diff --git a/src/jdk/nashorn/internal/runtime/linker/JavaArgumentConverters.java b/src/jdk/nashorn/internal/runtime/linker/JavaArgumentConverters.java
--- a/src/jdk/nashorn/internal/runtime/linker/JavaArgumentConverters.java
+++ b/src/jdk/nashorn/internal/runtime/linker/JavaArgumentConverters.java
@@ -25,18 +25,21 @@
package jdk.nashorn.internal.runtime.linker;
+import static jdk.nashorn.internal.lookup.Lookup.MH;
import static jdk.nashorn.internal.runtime.ECMAErrors.typeError;
import static jdk.nashorn.internal.runtime.ScriptRuntime.UNDEFINED;
-import static jdk.nashorn.internal.lookup.Lookup.MH;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.util.HashMap;
import java.util.Map;
+
import jdk.internal.dynalink.support.TypeUtilities;
+import jdk.nashorn.internal.objects.NativeArray;
import jdk.nashorn.internal.runtime.ConsString;
import jdk.nashorn.internal.runtime.JSType;
import jdk.nashorn.internal.runtime.ScriptObject;
+import jdk.nashorn.internal.runtime.arrays.ArrayData;
/**
* Utility class shared by {@code NashornLinker} and {@code NashornPrimitiveLinker} for converting JS values to Java
@@ -53,6 +56,18 @@
private static final MethodHandle TO_CHAR = findOwnMH("toChar", Character.class, Object.class);
private static final MethodHandle TO_CHAR_PRIMITIVE = findOwnMH("toCharPrimitive", char.class, Object.class);
+ // TAL
+ private static final MethodHandle TO_OBJECT_ARRAY = findOwnMH("toObjectArray", Object[].class, Object.class);
+ private static final MethodHandle TO_STRING_ARRAY = findOwnMH("toStringArray", String[].class, Object.class);
+ private static final MethodHandle TO_BOOLEAN_PRIMITIVE_ARRAY = findOwnMH("toBooleanPrimitiveArray", boolean[].class, Object.class);
+ private static final MethodHandle TO_FLOAT_PRIMITIVE_ARRAY = findOwnMH("toFloatPrimitiveArray", float[].class, Object.class);
+ private static final MethodHandle TO_DOUBLE_PRIMITIVE_ARRAY = findOwnMH("toDoublePrimitiveArray", double[].class, Object.class);
+ private static final MethodHandle TO_BYTE_PRIMITIVE_ARRAY = findOwnMH("toBytePrimitiveArray", byte[].class, Object.class);
+ private static final MethodHandle TO_SHORT_PRIMITIVE_ARRAY = findOwnMH("toShortPrimitiveArray", short[].class, Object.class);
+ private static final MethodHandle TO_INT_PRIMITIVE_ARRAY = findOwnMH("toIntPrimitiveArray", int[].class, Object.class);
+ private static final MethodHandle TO_LONG_PRIMITIVE_ARRAY = findOwnMH("toLongPrimitiveArray", long[].class, Object.class);
+ private static final MethodHandle TO_CHAR_PRIMITIVE_ARRAY = findOwnMH("toCharPrimitiveArray", char[].class, Object.class);
+
private JavaArgumentConverters() {
}
@@ -60,7 +75,6 @@
return CONVERTERS.get(targetType);
}
- @SuppressWarnings("unused")
private static Boolean toBoolean(final Object obj) {
if (obj instanceof Boolean) {
return (Boolean) obj;
@@ -98,6 +112,12 @@
throw assertUnexpectedType(obj);
}
+ // TAL
+ private static boolean toBooleanPrimitive(final Object obj0) {
+ final Boolean b = toBoolean(obj0);
+ return b == null ? false : b;
+ }
+
private static Character toChar(final Object o) {
if (o == null) {
return null;
@@ -124,7 +144,6 @@
return s.charAt(0);
}
- @SuppressWarnings("unused")
private static char toCharPrimitive(final Object obj0) {
final Character c = toChar(obj0);
return c == null ? (char)0 : c;
@@ -150,11 +169,12 @@
obj = JSType.toPrimitive(obj, String.class);
continue;
}
- throw assertUnexpectedType(obj);
+ // TAL
+ return obj.toString();
+ //throw assertUnexpectedType(obj);
}
}
- @SuppressWarnings("unused")
private static Double toDouble(final Object obj0) {
// TODO - Order tests for performance.
for (Object obj = obj0; ;) {
@@ -180,6 +200,12 @@
}
}
+ // TAL
+ private static double toDoublePrimitive(final Object obj0) {
+ final Double d = toDouble(obj0);
+ return d == null ? 0.0D : d;
+ }
+
@SuppressWarnings("unused")
private static Number toNumber(final Object obj0) {
// TODO - Order tests for performance.
@@ -227,16 +253,194 @@
}
}
- private static AssertionError assertUnexpectedType(final Object obj) {
- return new AssertionError("Unexpected type" + obj.getClass().getName() + ". Guards should have prevented this");
- }
-
- @SuppressWarnings("unused")
private static long toLongPrimitive(final Object obj0) {
final Long l = toLong(obj0);
return l == null ? 0L : l;
}
+ // TAL
+ @SuppressWarnings("unused")
+ private static Object[] toObjectArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ Object[] r = new Object[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = data.getObject(i);
+ }
+ return r;
+ } else {
+ return obj0 != null ? new Object[] { obj0 } : null;
+ }
+ }
+
+ // TAL
+ @SuppressWarnings("unused")
+ private static String[] toStringArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ String[] r = new String[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = toString(data.getObject(i));
+ }
+ return r;
+ } else {
+ String r = toString(obj0);
+ return r != null ? new String[] { r } : null;
+ }
+ }
+
+ // TAL
+ @SuppressWarnings("unused")
+ private static boolean[] toBooleanPrimitiveArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ boolean[] r = new boolean[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = toBooleanPrimitive(data.getObject(i));
+ }
+ return r;
+ } else {
+ Boolean r = toBoolean(obj0);
+ return r != null ? new boolean[] { r } : null;
+ }
+ }
+
+ // TAL
+ @SuppressWarnings("unused")
+ private static float[] toFloatPrimitiveArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ float[] r = new float[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = (float) toDoublePrimitive(data.getObject(i));
+ }
+ return r;
+ } else {
+ Double r = toDouble(obj0);
+ return r != null ? new float[] { r.floatValue() } : null;
+ }
+ }
+
+ // TAL
+ @SuppressWarnings("unused")
+ private static double[] toDoublePrimitiveArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ double[] r = new double[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = toDoublePrimitive(data.getObject(i));
+ }
+ return r;
+ } else {
+ Double r = toDouble(obj0);
+ return r != null ? new double[] { r } : null;
+ }
+ }
+
+ // TAL
+ @SuppressWarnings("unused")
+ private static byte[] toBytePrimitiveArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ byte[] r = new byte[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = (byte) toLongPrimitive(data.getObject(i));
+ }
+ return r;
+ } else {
+ Long r = toLong(obj0);
+ return r != null ? new byte[] { r.byteValue() } : null;
+ }
+ }
+
+ // TAL
+ @SuppressWarnings("unused")
+ private static short[] toShortPrimitiveArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ short[] r = new short[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = (short) toLongPrimitive(data.getObject(i));
+ }
+ return r;
+ } else {
+ Long r = toLong(obj0);
+ return r != null ? new short[] { r.shortValue() } : null;
+ }
+ }
+
+ // TAL
+ @SuppressWarnings("unused")
+ private static int[] toIntPrimitiveArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ int[] r = new int[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = (int) toLongPrimitive(data.getObject(i));
+ }
+ return r;
+ } else {
+ Long r = toLong(obj0);
+ return r != null ? new int[] { r.intValue() } : null;
+ }
+ }
+
+ // TAL
+ @SuppressWarnings("unused")
+ private static long[] toLongPrimitiveArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ long[] r = new long[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = toLongPrimitive(data.getObject(i));
+ }
+ return r;
+ } else {
+ Long r = toLong(obj0);
+ return r != null ? new long[] { r } : null;
+ }
+ }
+
+ // TAL
+ @SuppressWarnings("unused")
+ private static char[] toCharPrimitiveArray(final Object obj0)
+ {
+ if (obj0 instanceof NativeArray) {
+ ArrayData data = ((NativeArray) obj0).getArray();
+ int length = (int) data.length();
+ char[] r = new char[length];
+ for (int i = 0; i < length; i++) {
+ r[i] = toCharPrimitive(data.getObject(i));
+ }
+ return r;
+ } else {
+ Character r = toChar(obj0);
+ return r != null ? new char[] { r } : null;
+ }
+ }
+
+ private static AssertionError assertUnexpectedType(final Object obj) {
+ return new AssertionError("Unexpected type" + obj.getClass().getName() + ". Guards should have prevented this");
+ }
+
private static MethodHandle findOwnMH(final String name, final Class<?> rtype, final Class<?>... types) {
return MH.findStatic(MethodHandles.lookup(), JavaArgumentConverters.class, name, MH.type(rtype, types));
}
@@ -259,6 +463,18 @@
CONVERTERS.put(long.class, TO_LONG_PRIMITIVE);
CONVERTERS.put(Long.class, TO_LONG);
+ // TAL
+ CONVERTERS.put(Object[].class, TO_OBJECT_ARRAY);
+ CONVERTERS.put(String[].class, TO_STRING_ARRAY);
+ CONVERTERS.put(boolean[].class, TO_BOOLEAN_PRIMITIVE_ARRAY);
+ CONVERTERS.put(float[].class, TO_FLOAT_PRIMITIVE_ARRAY);
+ CONVERTERS.put(double[].class, TO_DOUBLE_PRIMITIVE_ARRAY);
+ CONVERTERS.put(byte[].class, TO_BYTE_PRIMITIVE_ARRAY);
+ CONVERTERS.put(short[].class, TO_SHORT_PRIMITIVE_ARRAY);
+ CONVERTERS.put(int[].class, TO_INT_PRIMITIVE_ARRAY);
+ CONVERTERS.put(long[].class, TO_LONG_PRIMITIVE_ARRAY);
+ CONVERTERS.put(char[].class, TO_CHAR_PRIMITIVE_ARRAY);
+
putLongConverter(Byte.class);
putLongConverter(Short.class);
putLongConverter(Integer.class);
diff --git a/src/jdk/nashorn/internal/runtime/linker/NashornPrimitiveLinker.java b/src/jdk/nashorn/internal/runtime/linker/NashornPrimitiveLinker.java
--- a/src/jdk/nashorn/internal/runtime/linker/NashornPrimitiveLinker.java
+++ b/src/jdk/nashorn/internal/runtime/linker/NashornPrimitiveLinker.java
@@ -29,6 +29,7 @@
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
+
import jdk.internal.dynalink.linker.ConversionComparator;
import jdk.internal.dynalink.linker.GuardedInvocation;
import jdk.internal.dynalink.linker.GuardingTypeConverterFactory;
@@ -36,6 +37,7 @@
import jdk.internal.dynalink.linker.LinkerServices;
import jdk.internal.dynalink.linker.TypeBasedGuardingDynamicLinker;
import jdk.internal.dynalink.support.TypeUtilities;
+import jdk.nashorn.internal.objects.NativeArray;
import jdk.nashorn.internal.runtime.ConsString;
import jdk.nashorn.internal.runtime.Context;
import jdk.nashorn.internal.runtime.GlobalObject;
@@ -52,7 +54,12 @@
}
private static boolean canLinkTypeStatic(final Class<?> type) {
- return type == String.class || type == Boolean.class || type == ConsString.class || Number.class.isAssignableFrom(type);
+ return type == String.class || type == Boolean.class || type == ConsString.class || Number.class.isAssignableFrom(type) ||
+ // TAL
+ type == String[].class || type == Object[].class || type == boolean[].class ||
+ type == float[].class || type == double[].class ||
+ type == byte[].class || type == short[].class || type == int[].class || type == long[].class ||
+ type == char[].class;
}
@Override
@@ -63,6 +70,12 @@
final Object self = request.getReceiver();
final GlobalObject global = (GlobalObject) Context.getGlobal();
final NashornCallSiteDescriptor desc = (NashornCallSiteDescriptor) request.getCallSiteDescriptor();
+
+ // TAL
+ if (self.getClass().isArray()) {
+ // TODO
+ return null;
+ }
return Bootstrap.asType(global.primitiveLookup(request, self), linkerServices, desc);
}
@@ -148,6 +161,16 @@
return Comparison.TYPE_2_BETTER;
}
}
+
+ // TAL
+ if (NativeArray.class.isAssignableFrom(sourceType)) {
+ if (targetType1.isArray() && !targetType2.isArray()) {
+ return Comparison.TYPE_1_BETTER;
+ }
+ if (targetType2.isArray() && !targetType1.isArray()) {
+ return Comparison.TYPE_2_BETTER;
+ }
+ }
return Comparison.INDETERMINATE;
}
More information about the nashorn-dev
mailing list