This is an automated email from the ASF dual-hosted git repository.
kusal pushed a commit to branch WW-5340-ognl-guard
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/WW-5340-ognl-guard by this
push:
new 02db3683b WW-5340 Refactor OgnlGuard to do the parsing
02db3683b is described below
commit 02db3683bdd201737b78ad3fac55b81c0b3cded5
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Fri Sep 1 00:16:34 2023 +1000
WW-5340 Refactor OgnlGuard to do the parsing
---
.../opensymphony/xwork2/ognl/DefaultOgnlGuard.java | 7 +++-
.../com/opensymphony/xwork2/ognl/OgnlGuard.java | 48 ++++++++++++++++++++--
.../com/opensymphony/xwork2/ognl/OgnlUtil.java | 7 +---
.../xwork2/ognl/DefaultOgnlGuardTest.java | 18 +++-----
4 files changed, 59 insertions(+), 21 deletions(-)
diff --git
a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuard.java
b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuard.java
index 4e45a19ad..55a5d8f74 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuard.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuard.java
@@ -65,7 +65,12 @@ public class DefaultOgnlGuard implements OgnlGuard {
}
@Override
- public boolean isBlocked(String expr, Object tree) {
+ public boolean isRawExpressionBlocked(String expr) {
+ return false;
+ }
+
+ @Override
+ public boolean isParsedTreeBlocked(Object tree) {
return containsExcludedNodeType(tree);
}
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlGuard.java
b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlGuard.java
index b1bcb409e..b0e75f4c1 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlGuard.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlGuard.java
@@ -18,6 +18,9 @@
*/
package com.opensymphony.xwork2.ognl;
+import ognl.Ognl;
+import ognl.OgnlException;
+
/**
* Guards all expressions parsed by Struts Core. It is evaluated by {@link
OgnlUtil} immediately after parsing any
* expression.
@@ -26,12 +29,51 @@ package com.opensymphony.xwork2.ognl;
*/
public interface OgnlGuard {
+ String GUARD_BLOCKED = "_ognl_guard_blocked";
+
/**
- * It is imperative that the parsed tree matches the expression.
+ * Determines whether an OGNL expression should be blocked based on
validation done on both the raw expression and
+ * the parsed tree.
*
* @param expr OGNL expression
- * @param tree parsed tree of expression
* @return whether the expression should be blocked
*/
- boolean isBlocked(String expr, Object tree);
+ default boolean isBlocked(String expr) throws OgnlException {
+ return GUARD_BLOCKED.equals(parseExpression(expr));
+ }
+
+ /**
+ * Parses an OGNL expression and returns the resulting tree only if the
expression is not blocked as per defined
+ * validation rules in {@link #isRawExpressionBlocked} and {@link
#isParsedTreeBlocked}.
+ *
+ * @param expr OGNL expression
+ * @return parsed expression or {@link #GUARD_BLOCKED} if the expression
should be blocked
+ */
+ default Object parseExpression(String expr) throws OgnlException {
+ if (isRawExpressionBlocked(expr)) {
+ return GUARD_BLOCKED;
+ }
+ Object tree = Ognl.parseExpression(expr);
+ if (isParsedTreeBlocked(tree)) {
+ return GUARD_BLOCKED;
+ }
+ return expr;
+ }
+
+ /**
+ * Determines whether an OGNL expression should be blocked based on
validation done on only the raw expression,
+ * without parsing the tree.
+ *
+ * @param expr OGNL expression
+ * @return whether the expression should be blocked
+ */
+ boolean isRawExpressionBlocked(String expr);
+
+ /**
+ * Determines whether a parsed OGNL tree should be blocked based on some
validation rules.
+ *
+ * @param tree parsed OGNL tree
+ * @return whether the parsed tree should be blocked
+ */
+ boolean isParsedTreeBlocked(Object tree);
}
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 8a276435a..8f207541f 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -52,6 +52,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
+import static com.opensymphony.xwork2.ognl.OgnlGuard.GUARD_BLOCKED;
import static
com.opensymphony.xwork2.util.TextParseUtil.commaDelimitedStringToSet;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toSet;
@@ -71,7 +72,6 @@ public class OgnlUtil {
// Flag used to reduce flooding logs with WARNs about using DevMode
excluded packages
private final AtomicBoolean warnReported = new AtomicBoolean(false);
- private static final String GUARD_BLOCKED = "_ognl_guard_blocked";
private final OgnlCache<String, Object> expressionCache;
private final OgnlCache<Class<?>, BeanInfo> beanInfoCache;
private TypeConverter defaultConverter;
@@ -613,10 +613,7 @@ public class OgnlUtil {
tree = expressionCache.get(expr);
}
if (tree == null) {
- tree = Ognl.parseExpression(expr);
- if (ognlGuard.isBlocked(expr, tree)) {
- tree = GUARD_BLOCKED;
- }
+ tree = ognlGuard.parseExpression(expr);
if (enableExpressionCache) {
expressionCache.put(expr, tree);
}
diff --git
a/core/src/test/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuardTest.java
b/core/src/test/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuardTest.java
index 793f26432..d04656ddd 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuardTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/DefaultOgnlGuardTest.java
@@ -19,8 +19,6 @@
package com.opensymphony.xwork2.ognl;
import com.opensymphony.xwork2.config.ConfigurationException;
-import ognl.Ognl;
-import ognl.OgnlException;
import org.junit.Before;
import org.junit.Test;
@@ -38,19 +36,15 @@ public class DefaultOgnlGuardTest {
defaultOgnlGuard = new DefaultOgnlGuard();
}
- private boolean exec(String expr) throws OgnlException {
- return defaultOgnlGuard.isBlocked(expr, Ognl.parseExpression(expr));
- }
-
@Test
public void notConfigured() throws Exception {
String expr = "1+1";
- assertFalse(exec(expr));
+ assertFalse(defaultOgnlGuard.isBlocked(expr));
}
@Test
public void nonNodeTree() throws Exception {
- assertFalse(defaultOgnlGuard.isBlocked("1+1", "String"));
+ assertFalse(defaultOgnlGuard.isParsedTreeBlocked("String"));
}
@Test
@@ -68,18 +62,18 @@ public class DefaultOgnlGuardTest {
@Test
public void addBlocked() throws Exception {
String expr = "1+1";
- assertFalse(exec(expr));
+ assertFalse(defaultOgnlGuard.isBlocked(expr));
defaultOgnlGuard.useExcludedNodeTypes("ognl.ASTAdd");
- assertTrue(exec(expr));
+ assertTrue(defaultOgnlGuard.isBlocked(expr));
}
@Test
public void nestedAddBlocked() throws Exception {
String expr = "{'a',1+1}";
- assertFalse(exec(expr));
+ assertFalse(defaultOgnlGuard.isBlocked(expr));
defaultOgnlGuard.useExcludedNodeTypes("ognl.ASTAdd");
- assertTrue(exec(expr));
+ assertTrue(defaultOgnlGuard.isBlocked(expr));
}
}
