You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/09/21 15:33:51 UTC

svn commit: r1173630 - in /tomcat/trunk: java/javax/el/BeanELResolver.java test/javax/el/TestBeanELResolverVarargsInvocation.java

Author: markt
Date: Wed Sep 21 13:33:51 2011
New Revision: 1173630

URL: http://svn.apache.org/viewvc?rev=1173630&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51852
Correct 2 issues in varargs handling
 - Incorrectly constructed varargs arguments (resulting in
ArrayIndexOutOfBoundsExceptions)
 - Incorrectly detected matching varargs methods
Patch (with a test case!) provided by Matt Benson

Added:
    tomcat/trunk/test/javax/el/TestBeanELResolverVarargsInvocation.java   (with props)
Modified:
    tomcat/trunk/java/javax/el/BeanELResolver.java

Modified: tomcat/trunk/java/javax/el/BeanELResolver.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/javax/el/BeanELResolver.java?rev=1173630&r1=1173629&r2=1173630&view=diff
==============================================================================
--- tomcat/trunk/java/javax/el/BeanELResolver.java (original)
+++ tomcat/trunk/java/javax/el/BeanELResolver.java Wed Sep 21 13:33:51 2011
@@ -420,7 +420,8 @@ public class BeanELResolver extends ELRe
                     matchingMethod = getMethod(clazz, m);
                     break;
                 }
-                if (m.isVarArgs()) {
+                if (m.isVarArgs() && methodName.equals(m.getName()) && 
+                            paramCount > m.getParameterTypes().length - 2 ) {
                     matchingMethod = getMethod(clazz, m);
                 }
             }
@@ -440,21 +441,21 @@ public class BeanELResolver extends ELRe
             if (matchingMethod.isVarArgs()) {
                 int varArgIndex = parameterTypes.length - 1;
                 // First argCount-1 parameters are standard
-                for (int i = 0; (i < varArgIndex - 1); i++) {
+                for (int i = 0; (i < varArgIndex); i++) {
                     parameters[i] = factory.coerceToType(params[i],
                             parameterTypes[i]);
                 }
-                // Last parameter is the varags
+                // Last parameter is the varargs
                 Class<?> varArgClass =
                     parameterTypes[varArgIndex].getComponentType();
+                final Object varargs = Array.newInstance(
+                    varArgClass,
+                    (paramCount - varArgIndex));
                 for (int i = (varArgIndex); i < paramCount; i++) {
-                    Object varargs = Array.newInstance(
-                            parameterTypes[paramCount],
-                            (paramCount - varArgIndex));
-                    Array.set(varargs, i,
+                    Array.set(varargs, i - varArgIndex,
                             factory.coerceToType(params[i], varArgClass));
-                    parameters[varArgIndex] = varargs;
                 }
+                parameters[varArgIndex] = varargs;
             } else {
                 parameters = new Object[parameterTypes.length];
                 for (int i = 0; i < parameterTypes.length; i++) {

Added: tomcat/trunk/test/javax/el/TestBeanELResolverVarargsInvocation.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/javax/el/TestBeanELResolverVarargsInvocation.java?rev=1173630&view=auto
==============================================================================
--- tomcat/trunk/test/javax/el/TestBeanELResolverVarargsInvocation.java (added)
+++ tomcat/trunk/test/javax/el/TestBeanELResolverVarargsInvocation.java Wed Sep 21 13:33:51 2011
@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package javax.el;
+
+import java.lang.reflect.Method;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class TestBeanELResolverVarargsInvocation {
+    public static class Foo {
+
+        public String joinDelimited(String delim, String... strings) {
+            StringBuilder result = new StringBuilder();
+            if (strings != null) {
+                for (String s : strings) {
+                    if (delim != null && result.length() > 0) {
+                        result.append(delim);
+                    }
+                    result.append(s);
+                }
+            }
+            return result.toString();
+        }
+
+        public String join(String... strings) {
+            return joinDelimited(null, strings);
+        }
+    }
+
+    private Foo foo;
+    private ELContext elContext;
+    private BeanELResolver beanELResolver;
+
+    @Before
+    public void setup() {
+        foo = new Foo();
+        beanELResolver = new BeanELResolver();
+        elContext = new ELContext() {
+            private VariableMapper variableMapper = new VariableMapper() {
+                private Map<String, ValueExpression> vars =
+                    new HashMap<String, ValueExpression>();
+
+                @Override
+                public ValueExpression setVariable(String arg0,
+                        ValueExpression arg1) {
+                    return vars.put(arg0, arg1);
+                }
+
+                @Override
+                public ValueExpression resolveVariable(String arg0) {
+                    return vars.get(arg0);
+                }
+            };
+            private FunctionMapper functionMapper = new FunctionMapper() {
+
+                @Override
+                public Method resolveFunction(String arg0, String arg1) {
+                    return null;
+                }
+            };
+
+            @Override
+            public VariableMapper getVariableMapper() {
+                return variableMapper;
+            }
+
+            @Override
+            public FunctionMapper getFunctionMapper() {
+                return functionMapper;
+            }
+
+            @Override
+            public ELResolver getELResolver() {
+                return beanELResolver;
+            }
+        };
+    }
+
+    /**
+     * Tests varargs that come after an opening argument.
+     */
+    @Test
+    public void testJoinDelimited() {
+        Assert.assertEquals(foo.joinDelimited("-", "foo", "bar", "baz"),
+            beanELResolver.invoke(elContext, foo, "joinDelimited", null,
+                    new Object[] { "-", "foo", "bar", "baz" }));
+    }
+
+    /**
+     * Tests varargs that constitute a method's only parameters, as well as
+     * bogus results due to improper matching of ANY vararg method, and
+     * depending on the order in which reflected methods are encountered.
+     */
+    @Test
+    public void testJoin() {
+        Assert.assertEquals(foo.join("foo", "bar", "baz"),
+            beanELResolver.invoke(elContext, foo, "join", null,
+                    new Object[] { "foo", "bar", "baz" }));
+    }
+
+}
\ No newline at end of file

Propchange: tomcat/trunk/test/javax/el/TestBeanELResolverVarargsInvocation.java
------------------------------------------------------------------------------
    svn:eol-style = native



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org