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