Skip to content

Commit

Permalink
Merge pull request #746 from apache/WW-5340-ognlutil-refactor
Browse files Browse the repository at this point in the history 
WW-5340 Preliminary refactor of OgnlUtil
  • Loading branch information
@lukaszlenart
lukaszlenart committed Sep 26, 2023
2 parents fde2b70 + 90adbfb commit 19d26b2
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 58 deletions.
8 changes: 3 additions & 5 deletions core/src/main/java/com/opensymphony/xwork2/ognl/OgnlReflectionProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -69,7 +68,6 @@ public void setProperties(Map<String, ?> props, Object o, Map<String, Object> co

public void setProperties(Map<String, ?> props, Object o, Map<String, Object> context, boolean throwPropertyExceptions) throws ReflectionException{
ognlUtil.setProperties(props, o, context, throwPropertyExceptions);

}

public void setProperties(Map<String, ?> properties, Object o) {
Expand Down Expand Up @@ -134,7 +132,7 @@ public Object getValue(String expression, Map<String, Object> context, Object ro
public void setValue(String expression, Map<String, Object> 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);
}
Expand Down
99 changes: 47 additions & 52 deletions core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,7 @@ public Object getRealTarget(String property, Map<String, Object> 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
Expand All @@ -536,16 +535,7 @@ public Object getRealTarget(String property, Map<String, Object> context, Object
* @throws OgnlException in case of ognl errors
*/
public void setValue(final String name, final Map<String, Object> context, final Object root, final Object value) throws OgnlException {
compileAndExecute(name, context, (OgnlTask<Void>) 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<String, Object> context) throws OgnlException {
Expand Down Expand Up @@ -588,58 +578,55 @@ private boolean isSimpleMethod(Object tree, Map<String, Object> context) throws
}

public Object getValue(final String name, final Map<String, Object> 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<String, Object> 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<String, Object> 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 <T> Object compileAndExecute(String expression, Map<String, Object> context, OgnlTask<T> 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<String, Object> context, Object root, Object value, Map<String, Object> 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> T ognlGet(String expr, Map<String, Object> context, Object root, Class<T> resultType, Map<String, Object> 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 <T> Object compileAndExecuteMethod(String expression, Map<String, Object> context, OgnlTask<T> 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<String, Object> context) throws OgnlException {
return compileAndExecute(expression, context, tree -> tree);
Object tree = toTree(expression);
checkEnableEvalExpression(tree, context);
return tree;
}

private void checkEnableEvalExpression(Object tree, Map<String, Object> context) throws OgnlException {
Expand All @@ -654,6 +641,18 @@ private void checkSimpleMethod(Object tree, Map<String, Object> context) throws
}
}

private void checkEvalExpression(Object tree, Map<String, Object> 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<String, Object> 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
Expand Down Expand Up @@ -728,12 +727,8 @@ public void copy(final Object from, final Object to, final Map<String, Object> 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);
}
Expand Down Expand Up @@ -805,7 +800,7 @@ public Map<String, Object> 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);
Expand Down Expand Up @@ -895,8 +890,8 @@ protected Map<String, Object> createDefaultContext(Object root, ClassResolver cl
return Ognl.createDefaultContext(root, memberAccess, resolver, defaultConverter);
}

private interface OgnlTask<T> {
T execute(Object tree) throws OgnlException;
@FunctionalInterface
private interface TreeValidator {
void validate(Object tree, Map<String, Object> context) throws OgnlException;
}

}
2 changes: 1 addition & 1 deletion core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 19d26b2

Please sign in to comment.