Author: markt
Date: Mon Dec  2 14:57:53 2013
New Revision: 1547046

URL: http://svn.apache.org/r1547046
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55691
ArrayELResolver needs to handle the case where the base object is an array of 
primitives.

Added:
    tomcat/tc6.0.x/trunk/java/javax/el/Util.java   (with props)
    tomcat/tc6.0.x/trunk/test/javax/
    tomcat/tc6.0.x/trunk/test/javax/el/
    tomcat/tc6.0.x/trunk/test/javax/el/TestArrayELResolver.java   (with props)
Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/javax/el/ArrayELResolver.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1547046&r1=1547045&r2=1547046&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Mon Dec  2 14:57:53 2013
@@ -115,13 +115,6 @@ PATCHES PROPOSED TO BACKPORT:
   +1: markt, kkolinko
   -1:
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55691
-  ArrayELResolver needs to handle the case where the base object is an array of
-  primitives.
-  http://people.apache.org/~markt/patches/2013-10-28-bug55691-tc6.patch
-  +1: markt, schultz, kkolinko
-  -1:
-
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55749
   Improve the error message when SSLEngine is disabled in AprLifecycleListener
   and SSL is configured for an APR/native connector.

Modified: tomcat/tc6.0.x/trunk/java/javax/el/ArrayELResolver.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/javax/el/ArrayELResolver.java?rev=1547046&r1=1547045&r2=1547046&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/javax/el/ArrayELResolver.java (original)
+++ tomcat/tc6.0.x/trunk/java/javax/el/ArrayELResolver.java Mon Dec  2 14:57:53 
2013
@@ -86,19 +86,18 @@ public class ArrayELResolver extends ELR
                                                                .getName() }));
                        }
 
-                       int idx = coerce(property);
-                       checkBounds(base, idx);
-                       if (value != null &&
-                               
!base.getClass().getComponentType().isAssignableFrom(
-                                       value.getClass())) {
-                           throw new ClassCastException(message(context,
-                                   "objectNotAssignable",
-                                   new Object[] {value.getClass().getName(),
-                                   
base.getClass().getComponentType().getName()}));
-                       }
-                       Array.set(base, idx, value);
-               }
-       }
+            int idx = coerce(property);
+            checkBounds(base, idx);
+            if (value != null && !Util.isAssignableFrom(value.getClass(),
+                    base.getClass().getComponentType())) {
+                throw new ClassCastException(message(context,
+                        "objectNotAssignable",
+                        new Object[] {value.getClass().getName(),
+                        base.getClass().getComponentType().getName()}));
+            }
+            Array.set(base, idx, value);
+        }
+    }
 
        public boolean isReadOnly(ELContext context, Object base, Object 
property)
                        throws NullPointerException, PropertyNotFoundException, 
ELException {

Added: tomcat/tc6.0.x/trunk/java/javax/el/Util.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/javax/el/Util.java?rev=1547046&view=auto
==============================================================================
--- tomcat/tc6.0.x/trunk/java/javax/el/Util.java (added)
+++ tomcat/tc6.0.x/trunk/java/javax/el/Util.java Mon Dec  2 14:57:53 2013
@@ -0,0 +1,57 @@
+/*
+ * 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;
+
+class Util {
+
+    /*
+     * This method duplicates code in org.apache.el.util.ReflectionUtil. When
+     * making changes keep the code in sync.
+     */
+    static boolean isAssignableFrom(Class<?> src, Class<?> target) {
+        // src will always be an object
+        // Short-cut. null is always assignable to an object and in EL null
+        // can always be coerced to a valid value for a primitive
+        if (src == null) {
+            return true;
+        }
+
+        Class<?> targetClass;
+        if (target.isPrimitive()) {
+            if (target == Boolean.TYPE) {
+                targetClass = Boolean.class;
+            } else if (target == Character.TYPE) {
+                targetClass = Character.class;
+            } else if (target == Byte.TYPE) {
+                targetClass = Byte.class;
+            } else if (target == Short.TYPE) {
+                targetClass = Short.class;
+            } else if (target == Integer.TYPE) {
+                targetClass = Integer.class;
+            } else if (target == Long.TYPE) {
+                targetClass = Long.class;
+            } else if (target == Float.TYPE) {
+                targetClass = Float.class;
+            } else {
+                targetClass = Double.class;
+            }
+        } else {
+            targetClass = target;
+        }
+        return targetClass.isAssignableFrom(src);
+    }
+}

Propchange: tomcat/tc6.0.x/trunk/java/javax/el/Util.java
------------------------------------------------------------------------------
    svn:eol-style = native

Added: tomcat/tc6.0.x/trunk/test/javax/el/TestArrayELResolver.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/test/javax/el/TestArrayELResolver.java?rev=1547046&view=auto
==============================================================================
--- tomcat/tc6.0.x/trunk/test/javax/el/TestArrayELResolver.java (added)
+++ tomcat/tc6.0.x/trunk/test/javax/el/TestArrayELResolver.java Mon Dec  2 
14:57:53 2013
@@ -0,0 +1,386 @@
+/*
+ * 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 junit.framework.Assert;
+import junit.framework.TestCase;
+
+import org.apache.jasper.el.ELContextImpl;
+
+public class TestArrayELResolver extends TestCase {
+
+    /**
+     * Tests that a null context results in an NPE as per EL Javadoc.
+     */
+    public void testGetType01() throws Exception {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            resolver.getType(null, new Object(), new Object());
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof NullPointerException);
+    }
+
+    /**
+     * Tests that a valid property is not resolved if base is not an array.
+     */
+    public void testGetType02() {
+        doNegativeTest(new Object(), new Object(), MethodUnderTest.GET_TYPE,
+                true);
+    }
+
+    /**
+     * Tests that a valid property is resolved.
+     */
+    public void testGetType03() {
+        ArrayELResolver resolver = new ArrayELResolver();
+        ELContext context = new ELContextImpl();
+
+        String[] base = new String[] { "element" };
+        Class<?> result = resolver.getType(context, base, new Integer(0));
+
+        Assert.assertEquals(base.getClass().getComponentType(), result);
+        Assert.assertTrue(context.isPropertyResolved());
+    }
+
+    /**
+     * Tests that the key is out of bounds and exception will be thrown.
+     */
+    public void testGetType04() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            ELContext context = new ELContextImpl();
+
+            String[] base = new String[] { "element" };
+            resolver.getType(context, base, new Integer(1));
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof PropertyNotFoundException);
+    }
+
+    /**
+     * Tests that a null context results in an NPE as per EL Javadoc.
+     */
+    public void testGetValue01() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            resolver.getValue(null, new Object(), new Object());
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof NullPointerException);
+    }
+
+    /**
+     * Tests that a valid property is not resolved if base is not an array.
+     */
+    public void testGetValue02() {
+        doNegativeTest(new Object(), new Object(), MethodUnderTest.GET_VALUE,
+                true);
+    }
+
+    /**
+     * Tests that a valid property is resolved.
+     */
+    public void testGetValue03() {
+        ArrayELResolver resolver = new ArrayELResolver();
+        ELContext context = new ELContextImpl();
+
+        String[] base = new String[] { "element" };
+        Object result = resolver.getValue(context, base, new Integer(0));
+
+        Assert.assertEquals("element", result);
+        Assert.assertTrue(context.isPropertyResolved());
+    }
+
+    /**
+     * Tests a coercion cannot be performed as the key is not integer.
+     */
+    public void testGetValue04() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            ELContext context = new ELContextImpl();
+
+            String[] base = new String[] { "element" };
+            resolver.getValue(context, base, "key");
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof IllegalArgumentException);
+    }
+
+    /**
+     * Tests that the key is out of bounds and null will be returned.
+     */
+    public void testGetValue05() {
+        ArrayELResolver resolver = new ArrayELResolver();
+        ELContext context = new ELContextImpl();
+
+        String[] base = new String[] { "element" };
+        Object result = resolver.getValue(context, base, new Integer(1));
+
+        Assert.assertNull(result);
+        Assert.assertTrue(context.isPropertyResolved());
+
+        result = resolver.getValue(context, base, new Integer(-1));
+
+        Assert.assertNull(result);
+        Assert.assertTrue(context.isPropertyResolved());
+    }
+
+    /**
+     * Tests that a null context results in an NPE as per EL Javadoc.
+     */
+    public void testSetValue01() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            resolver.setValue(null, new Object(), new Object(), new Object());
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof NullPointerException);
+    }
+
+    /**
+     * Tests that a valid property is not set if base is not an array.
+     */
+    public void testSetValue02() {
+        doNegativeTest(new Object(), new Object(), MethodUnderTest.SET_VALUE,
+                false);
+    }
+
+    /**
+     * Tests that an exception is thrown when readOnly is true.
+     */
+    public void testSetValue03() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver(true);
+            ELContext context = new ELContextImpl();
+
+            resolver.setValue(context, new String[] {}, new Object(), new 
Object());
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof PropertyNotWritableException);
+    }
+
+    /**
+     * Tests that a valid property is set.
+     */
+    //@Test
+    public void testSetValue04() {
+        ArrayELResolver resolver = new ArrayELResolver();
+        ELContext context = new ELContextImpl();
+
+        String[] base = new String[] { "element" };
+        resolver.setValue(context, base, new Integer(0), "new-element");
+
+        Assert.assertEquals("new-element",
+                resolver.getValue(context, base, new Integer(0)));
+        Assert.assertTrue(context.isPropertyResolved());
+
+        resolver.setValue(context, base, new Integer(0), null);
+
+        Assert.assertEquals(null,
+                resolver.getValue(context, base, new Integer(0)));
+        Assert.assertTrue(context.isPropertyResolved());
+    }
+
+    /**
+     * Tests a coercion cannot be performed as the key is not integer.
+     */
+    public void testSetValue05() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            ELContext context = new ELContextImpl();
+
+            String[] base = new String[] { "element" };
+            resolver.setValue(context, base, "key", "new-element");
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof IllegalArgumentException);
+    }
+
+    /**
+     * Tests that the key is out of bounds and exception will be thrown.
+     */
+    public void testSetValue06() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            ELContext context = new ELContextImpl();
+
+            String[] base = new String[] { "element" };
+            resolver.setValue(context, base, new Integer(1), "new-element");
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof PropertyNotFoundException);
+    }
+
+    /**
+     * Tests that an exception will be thrown if the value is not from the
+     * corresponding type.
+     */
+    public void testSetValue07() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            ELContext context = new ELContextImpl();
+
+            String[] base = new String[] { "element" };
+            resolver.setValue(context, base, new Integer(0), new Integer(1));
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof ClassCastException);
+    }
+
+    /**
+     * Tests setting arrays of primitives.
+     * https://issues.apache.org/bugzilla/show_bug.cgi?id=55691
+     */
+    public void testSetValue08() {
+        ArrayELResolver resolver = new ArrayELResolver();
+        ELContext context = new ELContextImpl();
+
+        int[] base = new int[] { 1, 2, 3 };
+        resolver.setValue(context, base, new Integer(1), Integer.valueOf(4));
+
+        Assert.assertEquals(Integer.valueOf(base[1]), Integer.valueOf(4));
+    }
+
+    /**
+     * Tests that a null context results in an NPE as per EL Javadoc.
+     */
+    public void testIsReadOnly01() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            resolver.isReadOnly(null, new Object(), new Object());
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof NullPointerException);
+    }
+
+    /**
+     * Tests that the propertyResolved is false if base is not an array.
+     */
+    public void testIsReadOnly02() {
+        ArrayELResolver resolver = new ArrayELResolver();
+        ELContext context = new ELContextImpl();
+
+        boolean result = resolver.isReadOnly(context, new Object(),
+                new Object());
+
+        Assert.assertFalse(result);
+        Assert.assertFalse(context.isPropertyResolved());
+
+        resolver = new ArrayELResolver(true);
+
+        result = resolver.isReadOnly(context, new Object(), new Object());
+
+        Assert.assertTrue(result);
+        Assert.assertFalse(context.isPropertyResolved());
+    }
+
+    /**
+     * Tests that if the ArrayELResolver is constructed with readOnly the 
method
+     * will return always true, otherwise false.
+     */
+    public void testIsReadOnly03() {
+        ArrayELResolver resolver = new ArrayELResolver();
+        ELContext context = new ELContextImpl();
+
+        String[] base = new String[] { "element" };
+        boolean result = resolver.isReadOnly(context, base, new Integer(0));
+
+        Assert.assertFalse(result);
+        Assert.assertTrue(context.isPropertyResolved());
+
+        resolver = new ArrayELResolver(true);
+
+        result = resolver.isReadOnly(context, base, new Integer(0));
+
+        Assert.assertTrue(result);
+        Assert.assertTrue(context.isPropertyResolved());
+    }
+
+    /**
+     * Tests that the key is out of bounds and exception will be thrown.
+     */
+    public void testIsReadOnly04() {
+        Exception expected = null;
+        try {
+            ArrayELResolver resolver = new ArrayELResolver();
+            ELContext context = new ELContextImpl();
+
+            String[] base = new String[] { "element" };
+            resolver.isReadOnly(context, base, new Integer(1));
+        } catch (Exception e) {
+            expected = e;
+        }
+        Assert.assertTrue(expected instanceof PropertyNotFoundException);
+    }
+
+    private void doNegativeTest(Object base, Object trigger,
+            MethodUnderTest method, boolean checkResult) {
+        ArrayELResolver resolver = new ArrayELResolver();
+        ELContext context = new ELContextImpl();
+
+        Object result = null;
+        switch (method) {
+        case GET_VALUE: {
+            result = resolver.getValue(context, base, trigger);
+            break;
+        }
+        case SET_VALUE: {
+            resolver.setValue(context, base, trigger, new Object());
+            break;
+        }
+        case GET_TYPE: {
+            result = resolver.getType(context, base, trigger);
+            break;
+        }
+        default: {
+            // Should never happen
+            Assert.fail("Missing case for method");
+        }
+        }
+
+        if (checkResult) {
+            Assert.assertNull(result);
+        }
+        Assert.assertFalse(context.isPropertyResolved());
+    }
+
+    private static enum MethodUnderTest {
+        GET_VALUE, SET_VALUE, GET_TYPE
+    }
+
+}

Propchange: tomcat/tc6.0.x/trunk/test/javax/el/TestArrayELResolver.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1547046&r1=1547045&r2=1547046&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Mon Dec  2 14:57:53 2013
@@ -86,6 +86,14 @@
       </fix>
     </changelog>
   </subsection>
+  <subsection name="Jasper">
+    <changelog>
+      <fix>
+        <bug>55691</bug>: Fix <code>javax.el.ArrayELResolver</code> to 
correctly
+        handle the case where the base object is an array of primitives. 
(markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Web applications">
     <changelog>
       <add>



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

Reply via email to