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

WW-4062 Further optimisation of OgnlException caching
apache · Aug 13, 2024 · 9b259ae · 9b259ae
2 parents 8d07694 + 0fd8551
commit 9b259ae
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 8 deletions.
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -602,10 +602,8 @@ private Object toTree(String expr) throws OgnlException {
             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;
+            // OgnlException was cached, rethrow it with empty stack trace (refilling the stack trace is expensive)
+            clearStackTraceAndRethrow(tree);
         }
         if (tree == null) {
             try {
@@ -621,13 +619,21 @@ private Object toTree(String expr) throws OgnlException {
                 throw (OgnlException) tree;
             }
         }
-
         if (EXPR_BLOCKED.equals(tree)) {
             throw new OgnlException("Expression blocked by OgnlGuard: " + expr);
         }
         return tree;
     }
 
+    private void clearStackTraceAndRethrow(Object ognlException) throws OgnlException {
+        OgnlException e = (OgnlException) ognlException;
+        e.setStackTrace(new StackTraceElement[0]);
+        if (e.getCause() != null) {
+            e.getCause().setStackTrace(new StackTraceElement[0]);
+        }
+        throw e;
+    }
+
     public Object compile(String expression, Map<String, Object> context) throws OgnlException {
         Object tree = toTree(expression);
         checkEnableEvalExpression(tree, context);

diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
@@ -1650,14 +1650,15 @@ public void testCompilationErrorsCached() throws Exception {
         StackTraceElement[] stackTrace = e.getStackTrace();
         assertThat(stackTrace).isEmpty();
         StackTraceElement[] causeStackTrace = e.getCause().getStackTrace();
+        assertThat(causeStackTrace).isNotEmpty();
 
         OgnlException e2 = assertThrows(OgnlException.class, () -> ognlUtil.compile(".literal.$something"));
-        StackTraceElement[] stackTrace2 = e.getStackTrace();
+        StackTraceElement[] stackTrace2 = e2.getStackTrace();
         assertThat(stackTrace2).isEmpty();
-        StackTraceElement[] causeStackTrace2 = e.getCause().getStackTrace();
+        StackTraceElement[] causeStackTrace2 = e2.getCause().getStackTrace();
 
+        assertThat(causeStackTrace2).isEmpty(); // Stack trace cleared before rethrow
         assertSame(e, e2); // Exception is cached
-        assertThat(causeStackTrace).isNotEqualTo(causeStackTrace2); // Stack trace refreshed
     }
 
     /**