Merge pull request #1013 from apache/WW-4062-ognl-exc-cache

WW-4062 Cache OgnlException thrown on compilation
apache · Aug 10, 2024 · 663dd3a · 663dd3a
2 parents a446409 + 6caa932
commit 663dd3a
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 1 deletion.
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -601,12 +601,27 @@ private Object toTree(String expr) throws OgnlException {
         if (enableExpressionCache) {
             tree = expressionCache.get(expr);
         }
+        if (tree instanceof OgnlException) {
+            // OgnlException was cached, rethrow it with updated stack trace
+            OgnlException e = (OgnlException) tree;
+            e.getCause().fillInStackTrace();
+            throw e;
+        }
         if (tree == null) {
-            tree = ognlGuard.parseExpression(expr);
+            try {
+                tree = ognlGuard.parseExpression(expr);
+            } catch (OgnlException e) {
+                tree = e;
+            }
             if (enableExpressionCache) {
                 expressionCache.put(expr, tree);
             }
+            if (tree instanceof OgnlException) {
+                // Rethrow OgnlException after caching
+                throw (OgnlException) tree;
+            }
         }
+
         if (EXPR_BLOCKED.equals(tree)) {
             throw new OgnlException("Expression blocked by OgnlGuard: " + expr);
         }

diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
@@ -1645,6 +1645,21 @@ public void testCustomOgnlMapBlocked() throws Exception {
         assertThrows(OgnlException.class, () -> ognlUtil.getValue(vulnerableExpr, ognlUtil.createDefaultContext(null), null));
     }
 
+    public void testCompilationErrorsCached() throws Exception {
+        OgnlException e = assertThrows(OgnlException.class, () -> ognlUtil.compile(".literal.$something"));
+        StackTraceElement[] stackTrace = e.getStackTrace();
+        assertThat(stackTrace).isEmpty();
+        StackTraceElement[] causeStackTrace = e.getCause().getStackTrace();
+
+        OgnlException e2 = assertThrows(OgnlException.class, () -> ognlUtil.compile(".literal.$something"));
+        StackTraceElement[] stackTrace2 = e.getStackTrace();
+        assertThat(stackTrace2).isEmpty();
+        StackTraceElement[] causeStackTrace2 = e.getCause().getStackTrace();
+
+        assertSame(e, e2); // Exception is cached
+        assertThat(causeStackTrace).isNotEqualTo(causeStackTrace2); // Stack trace refreshed
+    }
+
     /**
      * Generate a new OgnlUtil instance (not configured by the {@link ContainerBuilder}) that can be used for
      * basic tests, with its Expression and BeanInfo factories set to LRU mode.