Skip to content

Commit 9b259ae

Browse files
authored
Merge pull request #1021 from apache/WW-4062-ognl-exc-cache
WW-4062 Further optimisation of OgnlException caching
2 parents 8d07694 + 0fd8551 commit 9b259ae

File tree

2 files changed

+15
-8
lines changed

2 files changed

+15
-8
lines changed

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -602,10 +602,8 @@ private Object toTree(String expr) throws OgnlException {
602602
tree = expressionCache.get(expr);
603603
}
604604
if (tree instanceof OgnlException) {
605-
// OgnlException was cached, rethrow it with updated stack trace
606-
OgnlException e = (OgnlException) tree;
607-
e.getCause().fillInStackTrace();
608-
throw e;
605+
// OgnlException was cached, rethrow it with empty stack trace (refilling the stack trace is expensive)
606+
clearStackTraceAndRethrow(tree);
609607
}
610608
if (tree == null) {
611609
try {
@@ -621,13 +619,21 @@ private Object toTree(String expr) throws OgnlException {
621619
throw (OgnlException) tree;
622620
}
623621
}
624-
625622
if (EXPR_BLOCKED.equals(tree)) {
626623
throw new OgnlException("Expression blocked by OgnlGuard: " + expr);
627624
}
628625
return tree;
629626
}
630627

628+
private void clearStackTraceAndRethrow(Object ognlException) throws OgnlException {
629+
OgnlException e = (OgnlException) ognlException;
630+
e.setStackTrace(new StackTraceElement[0]);
631+
if (e.getCause() != null) {
632+
e.getCause().setStackTrace(new StackTraceElement[0]);
633+
}
634+
throw e;
635+
}
636+
631637
public Object compile(String expression, Map<String, Object> context) throws OgnlException {
632638
Object tree = toTree(expression);
633639
checkEnableEvalExpression(tree, context);

core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1650,14 +1650,15 @@ public void testCompilationErrorsCached() throws Exception {
16501650
StackTraceElement[] stackTrace = e.getStackTrace();
16511651
assertThat(stackTrace).isEmpty();
16521652
StackTraceElement[] causeStackTrace = e.getCause().getStackTrace();
1653+
assertThat(causeStackTrace).isNotEmpty();
16531654

16541655
OgnlException e2 = assertThrows(OgnlException.class, () -> ognlUtil.compile(".literal.$something"));
1655-
StackTraceElement[] stackTrace2 = e.getStackTrace();
1656+
StackTraceElement[] stackTrace2 = e2.getStackTrace();
16561657
assertThat(stackTrace2).isEmpty();
1657-
StackTraceElement[] causeStackTrace2 = e.getCause().getStackTrace();
1658+
StackTraceElement[] causeStackTrace2 = e2.getCause().getStackTrace();
16581659

1660+
assertThat(causeStackTrace2).isEmpty(); // Stack trace cleared before rethrow
16591661
assertSame(e, e2); // Exception is cached
1660-
assertThat(causeStackTrace).isNotEqualTo(causeStackTrace2); // Stack trace refreshed
16611662
}
16621663

16631664
/**

0 commit comments

Comments
 (0)