jython · jeff5 · Apr 17, 2022 · Jun 24, 2021 · Mar 14, 2022 · Mar 19, 2022
diff --git a/NEWS b/NEWS
@@ -17,6 +17,7 @@ Jython 2.7.3a1
     - [ GH-119 ] array.array itemsize and serialisation anomalies for unsigned types
     - [ GH-122 ] Upgrade Apache commons-compress to 1.21
     - [ GH-102 ] Fix CRC32.update calls in zlib.py
+    - [ GH-101 ] Improve boxing of arguments to vararg methods
     - [ GH-98 ] CVE-2019-9740: CRLF injection by url parameter
     - [ GH-97 ] Failures in SNI support affecting pip
     - [ GH-57 ] JavaWebStart compatibility broken
@@ -29,6 +30,10 @@ Jython 2.7.3a1
 
   New Features
 
+    - Under PR GH-101, the matching of sequence arguments to the parameters of Java methods is
+      improved. A Java varargs parameter does not match a final sequence in the call
+      if an array parameter could do so. (Thanks Peter Holloway.)
+
     - array.array itemsize of unsigned types is now the same as their signed counterparts, where 
       previously it was mostly double. Internal representations have changed. Anomalies have been
       eliminated between itemsize and the serialisation (tostring() etc.) for unsigned types,

diff --git a/src/org/python/core/ReflectedArgs.java b/src/org/python/core/ReflectedArgs.java
@@ -164,8 +164,8 @@ private PyObject[] ensureBoxedVarargs(PyObject[] pyArgs, int n) {
             return new PyObject[] {new PyList()};
         }
         PyObject lastArg = pyArgs[pyArgs.length - 1];
-        if (lastArg instanceof PySequenceList || lastArg instanceof PyArray
-                || lastArg instanceof PyXRange || lastArg instanceof PyIterator) {
+        if (pyArgs.length == n && (lastArg instanceof PySequenceList || lastArg instanceof PyArray
+                || lastArg instanceof PyXRange || lastArg instanceof PyIterator)) {
             // NOTE that the check is against PySequenceList, not PySequence,
             // because certain Java <=> Python semantics currently require this
             // additional strictness. Perhaps this can be relaxed.
@@ -175,6 +175,11 @@ private PyObject[] ensureBoxedVarargs(PyObject[] pyArgs, int n) {
             // and AstList, many/most of which seem likely to be problematic for
             // varargs usage.
 
+            // This is only relevant if the number of arguments would be correct if a final sequence
+            // argument was treated as a vararg argument. eg, if two lists are passed to a function
+            // that accepts (Object...), they should be boxed and passed as a single list, even though
+            // the final arg is a sequence.
+
             // FIXME also check if lastArg is sequence-like
             return pyArgs; // will be boxed in an array once __tojava__ is called
         }

diff --git a/tests/java/org/python/core/ReflectedArgsTest.java b/tests/java/org/python/core/ReflectedArgsTest.java
@@ -0,0 +1,167 @@
+package org.python.core;
+
+import static org.junit.Assert.assertArrayEquals;
+
+import junit.framework.TestCase;
+
+/**
+ * Tests for ReflectedArgs, paying particular attention to the processing of arguments to match Java
+ * methods where the signature accepts varargs ({@code T...}) and a sequence type is passed. Recall
+ * that {@code T...} is just syntactic sugar for an array argument.
+ * 
+ * <p>
+ * If a Java method that accepts varargs is called from Jython, the arguments are boxed into a
+ * {@code PyList} first. This should be skipped only if there are already the correct number of
+ * arguments and the final argument is a sequence type.
+ * 
+ * It is not only the final argument that must be checked for being a sequence. Otherwise, we shall
+ * miss valid calls matching the underlying method.
+ * 
+ * E.g. in Java<pre>
+ *  void foo(Object... args) {}
+ * </pre>should be callable from Jython as<pre>
+ *  foo(1, 2, [3, 4])
+ * </pre>where the list and the numbers are all objects and passed as a single array to the varargs
+ * method.
+ * 
+ * <p>
+ * Similarly, if there are too few arguments, a java method such as<pre>
+ * void bar(int a, int[] b, int... c) {}
+ * </pre>should be callable from Jython as<pre>
+ *  bar(1, [2, 3])
+ * </pre>where the vararg argument is empty and [2, 3] is passed as b.
+ * 
+ * <p>
+ * The only remaining ambiguity is where a vararg parameter can take a sequence type as an element,
+ * e.g.<pre>
+ *  void fizz(Object... args) {}
+ * </pre>calling from Jython as<pre>
+ *  fizz([1,2,3])
+ * </pre>would be valid as either args being {@code [1,2,3]} or {@code [[1, 2, 3]]}. This ambiguity
+ * is also present in Java but behaves as if it was the former.
+ */
+public class ReflectedArgsTest extends TestCase {
+
+    /** Example methods for varargs processing. */
+    static class Demo {
+
+        public static void foo(Object... args) {}
+
+        public static void bar(int i, int[] j, int... rest) {}
+
+        public static void baz(Integer i, Integer[] j, Integer... rest) {}
+    }
+
+    /** {@code static void foo(Object... args)} */
+    private static final ReflectedArgs FOO_SIGNATURE =
+            new ReflectedArgs(null, new Class<?>[] {Object[].class}, Demo.class, true, true);
+
+    private static void fooCheck(Object[] args, Object... expected) {
+        // Expect one element that is an array of the expected values
+        assertEquals(1, args.length);
+        Object[] v = (Object[]) args[0];
+        assertEquals(expected.length, v.length);
+        for (int k = 0; k < expected.length; k++) {
+            assertEquals(expected[k], v[k]);
+        }
+    }
+
+    /** {@code static void bar(int i, int[] j, int... rest)} */
+    private static final ReflectedArgs BAR_SIGNATURE = new ReflectedArgs(null,
+            new Class<?>[] {int.class, int[].class, int[].class}, Demo.class, true, true);
+
+    private static void barCheck(Object[] args, int i, int[] j, int... rest) {
+        // Expect three elements conformant to the method arguments
+        assertEquals(3, args.length);
+        assertEquals(i, (int) args[0]);
+        assertArrayEquals(j, (int[]) args[1]);
+        assertArrayEquals(rest, (int[]) args[2]);
+    }
+
+    /** {@code static void baz(Integer i, Integer[] j, Integer... rest)} */
+    private static final ReflectedArgs BAZ_SIGNATURE = new ReflectedArgs(null,
+            new Class<?>[] {Integer.class, Integer[].class, Integer[].class}, Demo.class, true,
+            true);
+
+    private static void bazCheck(Object[] args, Integer i, Integer[] j, Integer... rest) {
+        // Expect three elements conformant to the method arguments
+        assertEquals(3, args.length);
+        assertEquals(i, (Integer) args[0]);
+        assertArrayEquals(j, (Integer[]) args[1]);
+        assertArrayEquals(rest, (Integer[]) args[2]);
+    }
+
+    /*
+     * If a function accepts Object Varargs (or Collection/array etc), the arguments should be boxed
+     * even if the final arg is a PySequenceList if the number of arguments not equal to the
+     * expected number of arguments.
+     *
+     * eg bar(1, [1, 2, 3]) should be valid for a call to void bar(int i, int[] j, int... rest) but
+     * should be boxed as bar(1, [1, 2, 3], [])
+     */
+
+    /**
+     * Calling {@code foo(Object... args)} as Python {@code foo(3, 4, ["bar"])} calls Java
+     * {@code foo(3, 4, strlist)} where {@code strlist} is the representation of Python
+     * {@code ["bar"]}.
+     */
+    public void testVarargsBoxedWithTooManyArgs() {
+        // calling foo(Object... args) as foo(3, 4, ["bar"])
+        PyList strlist = new PyList(new PyObject[] {Py.newString("bar")});
+        PyObject[] pyArgs = {Py.newInteger(3), Py.newInteger(4), strlist};
+        ReflectedCallData callData = new ReflectedCallData();
+        assertTrue(FOO_SIGNATURE.matches(null, pyArgs, Py.NoKeywords, callData));
+        fooCheck(callData.args, new Object[] {3, 4, strlist});
+    }
+
+    /**
+     * Calling {@code bar(int a, int[] b, int... c)} as Python {@code bar(1, [2, 3])} calls Java
+     * {@code bar(1, new int[]{2, 3})}.
+     */
+    public void testPrimitiveVarargsBoxedWithTooFewArguments() {
+        // calling bar(int a, int[] b, int... c) as bar(1, [2, 3])
+        PyObject[] ints = {Py.newInteger(2), Py.newInteger(3)};
+        PyObject[] pyArgs = {Py.newInteger(1), new PyList(ints)};
+        ReflectedCallData callData = new ReflectedCallData();
+        assertTrue(BAR_SIGNATURE.matches(null, pyArgs, Py.NoKeywords, callData));
+        barCheck(callData.args, 1, new int[] {2, 3});
+    }
+
+    /**
+     * Calling {@code baz(Integer a, Integer[] b, Integer... c)} as Python {@code baz(1, [2, 3])}
+     * calls Java {@code baz(1, new Integer[]{2, 3})}.
+     */
+    public void testVarargsBoxedWithTooFewArguments() {
+        // calling baz(Integer a, Integer[] b, Integer... c) as bar(1, [2, 3])
+        PyObject[] ints = {Py.newInteger(2), Py.newInteger(3)};
+        PyObject[] pyArgs = {Py.newInteger(1), new PyList(ints)};
+        ReflectedCallData callData = new ReflectedCallData();
+        assertTrue(BAZ_SIGNATURE.matches(null, pyArgs, Py.NoKeywords, callData));
+        bazCheck(callData.args, 1, new Integer[] {2, 3});
+    }
+
+    /**
+     * Calling {@code foo(Object... args)} as Python {@code foo([1,2,3])} calls Java {@code foo(new
+     * Object[]{1,2,3})}.
+     */
+    public void testVarargsNotBoxedWithCorrectArgs() {
+        // calling foo(Object... args) as foo([1,2,3])
+        PyObject[] ints = {Py.newInteger(1), Py.newInteger(2), Py.newInteger(3)};
+        PyObject[] pyArgs = {new PyList(ints)};
+        ReflectedCallData callData = new ReflectedCallData();
+        assertTrue(FOO_SIGNATURE.matches(null, pyArgs, Py.NoKeywords, callData));
+        fooCheck(callData.args, 1, 2, 3);
+    }
+
+    /**
+     * Calling {@code foo(Object... args)} as Python {@code foo("foo", "bar")} calls Java
+     * {@code foo("foo", "bar")}.
+     */
+    public void testVarargsBoxedWithNoSequences() {
+        // calling foo(Object... args) as foo("foo", "bar")
+        PyObject[] pyArgs = {Py.newString("foo"), Py.newString("bar")};
+        ReflectedCallData callData = new ReflectedCallData();
+        assertTrue(FOO_SIGNATURE.matches(null, pyArgs, Py.NoKeywords, callData));
+        fooCheck(callData.args, "foo", "bar");
+    }
+}