Issue #4550

Do not check the assignability of the arguments. Instead rely on the order of the methods. Signed-off-by: Greg Wilkins <gregw@webtide.com>
jetty · Feb 24, 2020 · d6843a9 · d6843a9
1 parent f35522c
commit d6843a9
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 23 deletions.
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/TypeUtil.java
@@ -65,14 +65,14 @@ public class TypeUtil
     static
     {
         Map<Class<?>, Class<?>> unbox = new HashMap<>();
-        unbox.put(int.class, Integer.class);
-        unbox.put(long.class, Long.class);
+        unbox.put(boolean.class, Boolean.class);
         unbox.put(byte.class, Byte.class);
         unbox.put(char.class, Character.class);
+        unbox.put(short.class, Short.class);
+        unbox.put(int.class, Integer.class);
+        unbox.put(long.class, Long.class);
         unbox.put(float.class, Float.class);
         unbox.put(double.class, Double.class);
-        unbox.put(short.class, Long.class);
-        unbox.put(boolean.class, Boolean.class);
         __unbox = Collections.unmodifiableMap(unbox);
     }
 
@@ -751,8 +751,6 @@ public static boolean isUnboxable(Class<?> type, Object arg)
         if (arg == null)
             return true;
 
-        Class<?> c = __unbox.get(type);
-        Class<?> ac = arg.getClass();
-        return ac == __unbox.get(type) || (Number.class.isAssignableFrom(c) && Number.class.isAssignableFrom(ac));
+        return __unbox.get(type) == arg.getClass();
     }
 }
diff --git a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java
@@ -117,18 +117,19 @@ else if (!va1 && va2)
             else
             {
                 // Rank by assignability of the arguments
-                // This is only a rough guide that will favour call(String) vs call(Object),
-                // but it will not resolve call(String, Object) vs call (Object, String)
                 for (int i = 0; i < count; i++)
                 {
                     Class<?> t1 = p1[i].getType();
                     Class<?> t2 = p2[i].getType();
                     if (t1 != t2)
                     {
                         if (t1.isAssignableFrom(t2))
-                            compare++;
+                            compare = 1;
                         else if (t2.isAssignableFrom(t1))
-                            compare--;
+                            compare = -1;
+                        else
+                            compare = t1.getName().compareTo(t2.getName());
+                        break;
                     }
                 }
             }
@@ -1803,16 +1804,6 @@ Object[] matchArgsToParameters(Executable executable)
                     args = arguments.toArray(new Object[0]);
                 }
 
-                // Check assignable
-                Parameter[] params = executable.getParameters();
-                for (int i = 0; i < args.length; i++)
-                {
-                    if (args[i] != null &&
-                        !params[i].getType().isAssignableFrom(args[i].getClass()) &&
-                        !TypeUtil.isUnboxable(params[i].getType(), args[i]))
-                        return null;
-                }
-
                 return args;
             }
         }
@@ -1835,9 +1826,8 @@ private static List<XmlParser.Node> getNodes(XmlParser.Node node, String element
             }
         }
 
-        for (int i = 0; i < node.size(); i++)
+        for (Object o : node)
         {
-            Object o = node.get(i);
             if (!(o instanceof XmlParser.Node))
                 continue;
             XmlParser.Node n = (XmlParser.Node)o;

diff --git a/jetty-xml/src/test/java/org/eclipse/jetty/xml/TestConfiguration.java b/jetty-xml/src/test/java/org/eclipse/jetty/xml/TestConfiguration.java
@@ -54,6 +54,7 @@ public class TestConfiguration extends HashMap<String, Object>
     private Set set;
     private ConstructorArgTestClass constructorArgTestClass;
     public Map map;
+    public Double number;
 
     public TestConfiguration()
     {
@@ -65,6 +66,16 @@ public TestConfiguration(@Name("name") String n)
         name = n;
     }
 
+    public void setNumber(Object value)
+    {
+        testObject = value;
+    }
+
+    public void setNumber(double value)
+    {
+        number = value;
+    }
+
     public void setTest(Object value)
     {
         testObject = value;

diff --git a/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java b/jetty-xml/src/test/java/org/eclipse/jetty/xml/XmlConfigurationTest.java
@@ -614,6 +614,10 @@ public void call()
         {
         }
 
+        public void call(int o)
+        {
+        }
+
         public void call(Object o)
         {
         }
@@ -639,6 +643,7 @@ public void testMethodOrdering() throws Exception
         Collections.sort(methods, XmlConfiguration.EXECUTABLE_COMPARATOR);
         assertThat(methods, Matchers.contains(
             TestOrder.class.getMethod("call"),
+            TestOrder.class.getMethod("call", int.class),
             TestOrder.class.getMethod("call", String.class),
             TestOrder.class.getMethod("call", Object.class),
             TestOrder.class.getMethod("call", String[].class),
@@ -856,6 +861,45 @@ public void testCallMissingVarArgs() throws Exception
         assertNull(atc.getThird());
     }
 
+    public static class NumberTypeProvider implements ArgumentsProvider
+    {
+        @Override
+        public Stream<? extends Arguments> provideArguments(ExtensionContext context)
+        {
+            return Stream.of(
+                "byte",
+                "int",
+                // "char",
+                "short",
+                "long",
+                "float",
+                "double",
+                "Byte",
+                "Integer",
+                // "Character",
+                "Short",
+                "Long",
+                "Float",
+                "Double"
+            ).map(Arguments::of);
+        }
+    }
+
+    @ParameterizedTest
+    @ArgumentsSource(NumberTypeProvider.class)
+    public void testCallNumberConversion(String type) throws Exception
+    {
+        XmlConfiguration xmlConfiguration = asXmlConfiguration(
+            "<Configure class=\"org.eclipse.jetty.xml.TestConfiguration\">" +
+                " <Call name=\"setNumber\">" +
+                "  <Arg type=\"" + type + "\">42</Arg>" +
+                " </Call>" +
+                "</Configure>");
+
+        TestConfiguration tc = (TestConfiguration)xmlConfiguration.configure();
+        assertEquals(42.0D, tc.number);
+    }
+
     @Test
     public void testArgumentsGetIgnoredMissingDTD() throws Exception
     {