diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlReflectionProvider.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlReflectionProvider.java index d21b2f6245..1dbc8c67e5 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlReflectionProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlReflectionProvider.java @@ -21,7 +21,6 @@ import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.util.reflection.ReflectionException; import com.opensymphony.xwork2.util.reflection.ReflectionProvider; -import ognl.Ognl; import ognl.OgnlException; import ognl.OgnlRuntime; @@ -33,9 +32,9 @@ import java.util.Map; public class OgnlReflectionProvider implements ReflectionProvider { - + private OgnlUtil ognlUtil; - + @Inject public void setOgnlUtil(OgnlUtil ognlUtil) { this.ognlUtil = ognlUtil; @@ -69,7 +68,6 @@ public void setProperties(Map props, Object o, Map co public void setProperties(Map props, Object o, Map context, boolean throwPropertyExceptions) throws ReflectionException{ ognlUtil.setProperties(props, o, context, throwPropertyExceptions); - } public void setProperties(Map properties, Object o) { @@ -134,7 +132,7 @@ public Object getValue(String expression, Map context, Object ro public void setValue(String expression, Map context, Object root, Object value) throws ReflectionException { try { - Ognl.setValue(expression, context, root, value); + ognlUtil.setValue(expression, context, root, value); } catch (OgnlException e) { throw new ReflectionException(e); } diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java index bb4d299c0a..4f691543a2 100644 --- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java +++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java @@ -525,8 +525,7 @@ public Object getRealTarget(String property, Map context, Object } /** - * Wrapper around Ognl.setValue() to handle type conversion for collection elements. - * Ideally, this should be handled by OGNL directly. + * Wrapper around Ognl#setValue * * @param name the name * @param context context map @@ -536,16 +535,7 @@ public Object getRealTarget(String property, Map context, Object * @throws OgnlException in case of ognl errors */ public void setValue(final String name, final Map context, final Object root, final Object value) throws OgnlException { - compileAndExecute(name, context, (OgnlTask) tree -> { - if (isEvalExpression(tree, context)) { - throw new OgnlException("Eval expression/chained expressions cannot be used as parameter name"); - } - if (isArithmeticExpression(tree, context)) { - throw new OgnlException("Arithmetic expressions cannot be used as parameter name"); - } - Ognl.setValue(tree, context, root, value); - return null; - }); + ognlSet(name, context, root, value, context, this::checkEvalExpression, this::checkArithmeticExpression); } private boolean isEvalExpression(Object tree, Map context) throws OgnlException { @@ -588,58 +578,55 @@ private boolean isSimpleMethod(Object tree, Map context) throws } public Object getValue(final String name, final Map context, final Object root) throws OgnlException { - return compileAndExecute(name, context, tree -> Ognl.getValue(tree, context, root)); + return getValue(name, context, root, null); } public Object callMethod(final String name, final Map context, final Object root) throws OgnlException { - return compileAndExecuteMethod(name, context, tree -> Ognl.getValue(tree, context, root)); + return ognlGet(name, context, root, null, context, this::checkSimpleMethod); } public Object getValue(final String name, final Map context, final Object root, final Class resultType) throws OgnlException { - return compileAndExecute(name, context, tree -> Ognl.getValue(tree, context, root, resultType)); + return ognlGet(name, context, root, resultType, context, this::checkEnableEvalExpression); } - public Object compile(String expression) throws OgnlException { return compile(expression, null); } - private Object compileAndExecute(String expression, Map context, OgnlTask task) throws OgnlException { - Object tree; - if (enableExpressionCache) { - tree = expressionCache.get(expression); - if (tree == null) { - tree = Ognl.parseExpression(expression); - checkEnableEvalExpression(tree, context); - expressionCache.putIfAbsent(expression, tree); - } - } else { - tree = Ognl.parseExpression(expression); - checkEnableEvalExpression(tree, context); + private void ognlSet(String expr, Map context, Object root, Object value, Map checkContext, TreeValidator... treeValidators) throws OgnlException { + Object tree = toTree(expr); + for (TreeValidator validator : treeValidators) { + validator.validate(tree, checkContext); } + Ognl.setValue(tree, context, root, value); + } - return task.execute(tree); + private T ognlGet(String expr, Map context, Object root, Class resultType, Map checkContext, TreeValidator... treeValidators) throws OgnlException { + Object tree = toTree(expr); + for (TreeValidator validator : treeValidators) { + validator.validate(tree, checkContext); + } + return (T) Ognl.getValue(tree, context, root, resultType); } - private Object compileAndExecuteMethod(String expression, Map context, OgnlTask task) throws OgnlException { - Object tree; + private Object toTree(String expr) throws OgnlException { + Object tree = null; if (enableExpressionCache) { - tree = expressionCache.get(expression); - if (tree == null) { - tree = Ognl.parseExpression(expression); - checkSimpleMethod(tree, context); - expressionCache.putIfAbsent(expression, tree); + tree = expressionCache.get(expr); + } + if (tree == null) { + tree = Ognl.parseExpression(expr); + if (enableExpressionCache) { + expressionCache.put(expr, tree); } - } else { - tree = Ognl.parseExpression(expression); - checkSimpleMethod(tree, context); } - - return task.execute(tree); + return tree; } public Object compile(String expression, Map context) throws OgnlException { - return compileAndExecute(expression, context, tree -> tree); + Object tree = toTree(expression); + checkEnableEvalExpression(tree, context); + return tree; } private void checkEnableEvalExpression(Object tree, Map context) throws OgnlException { @@ -654,6 +641,18 @@ private void checkSimpleMethod(Object tree, Map context) throws } } + private void checkEvalExpression(Object tree, Map context) throws OgnlException { + if (isEvalExpression(tree, context)) { + throw new OgnlException("Eval expression/chained expressions cannot be used as parameter name"); + } + } + + private void checkArithmeticExpression(Object tree, Map context) throws OgnlException { + if (isArithmeticExpression(tree, context)) { + throw new OgnlException("Arithmetic expressions cannot be used as parameter name"); + } + } + /** * Copies the properties in the object "from" and sets them in the object "to" * using specified type converter, or {@link com.opensymphony.xwork2.conversion.impl.XWorkConverter} if none @@ -728,12 +727,8 @@ public void copy(final Object from, final Object to, final Map c PropertyDescriptor toPd = toPdHash.get(fromPd.getName()); if ((toPd != null) && (toPd.getWriteMethod() != null)) { try { - compileAndExecute(fromPd.getName(), context, expr -> { - Object value = Ognl.getValue(expr, contextFrom, from); - Ognl.setValue(expr, contextTo, to, value); - return null; - }); - + Object value = ognlGet(fromPd.getName(), contextFrom, from, null, context, this::checkEnableEvalExpression); + ognlSet(fromPd.getName(), contextTo, to, value, context); } catch (OgnlException e) { LOG.debug("Got OGNL exception", e); } @@ -805,7 +800,7 @@ public Map getBeanMap(final Object source) throws IntrospectionE final String propertyName = propertyDescriptor.getDisplayName(); Method readMethod = propertyDescriptor.getReadMethod(); if (readMethod != null) { - final Object value = compileAndExecute(propertyName, null, expr -> Ognl.getValue(expr, sourceMap, source)); + final Object value = ognlGet(propertyName, sourceMap, source, null, null, this::checkEnableEvalExpression); beanMap.put(propertyName, value); } else { beanMap.put(propertyName, "There is no read method for " + propertyName); @@ -895,8 +890,8 @@ protected Map createDefaultContext(Object root, ClassResolver cl return Ognl.createDefaultContext(root, memberAccess, resolver, defaultConverter); } - private interface OgnlTask { - T execute(Object tree) throws OgnlException; + @FunctionalInterface + private interface TreeValidator { + void validate(Object tree, Map context) throws OgnlException; } - } diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java index 8694d97213..1222fdff4f 100644 --- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java @@ -1314,7 +1314,7 @@ public void testBlockSequenceOfExpressions() { } assertNotNull(expected); assertSame(OgnlException.class, expected.getClass()); - assertEquals(expected.getMessage(), "Eval expressions/chained expressions have been disabled!"); + assertEquals("Eval expression/chained expressions cannot be used as parameter name", expected.getMessage()); } public void testCallMethod() {