From c8dfb3f4dff328a9aeac2eae1d4c701de632d88c Mon Sep 17 00:00:00 2001
From: Pascal Davoust <pascal.davoust@eptica.com>
Date: Wed, 10 Nov 2021 15:22:38 +0100
Subject: [PATCH] Cache all OGNL expressions (when enabled) irrespective of
 their execution status

---
 .../opensymphony/xwork2/ognl/OgnlUtil.java    | 16 ++------
 .../xwork2/ognl/OgnlUtilTest.java             | 40 +++++++++++++++++++
 2 files changed, 44 insertions(+), 12 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
index c66d5c98c..b891bce6c 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java
@@ -536,18 +536,14 @@ public class OgnlUtil {
             if (tree == null) {
                 tree = Ognl.parseExpression(expression);
                 checkEnableEvalExpression(tree, context);
+                expressions.putIfAbsent(expression, tree);
             }
         } else {
             tree = Ognl.parseExpression(expression);
             checkEnableEvalExpression(tree, context);
         }
 
-        final T exec = task.execute(tree);
-        // if cache is enabled and it's a valid expression, puts it in
-        if (enableExpressionCache) {
-            expressions.putIfAbsent(expression, tree);
-        }
-        return exec;
+        return task.execute(tree);
     }
 
     private <T> Object compileAndExecuteMethod(String expression, Map<String, Object> context, OgnlTask<T> task) throws OgnlException {
@@ -557,18 +553,14 @@ public class OgnlUtil {
             if (tree == null) {
                 tree = Ognl.parseExpression(expression);
                 checkSimpleMethod(tree, context);
+                expressions.putIfAbsent(expression, tree);
             }
         } else {
             tree = Ognl.parseExpression(expression);
             checkSimpleMethod(tree, context);
         }
 
-        final T exec = task.execute(tree);
-        // if cache is enabled and it's a valid expression, puts it in
-        if (enableExpressionCache) {
-            expressions.putIfAbsent(expression, tree);
-        }
-        return exec;
+        return task.execute(tree);
     }
 
     public Object compile(String expression, Map<String, Object> context) throws OgnlException {
diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
index 44a8e6e65..fc8656f72 100644
--- a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
+++ b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java
@@ -128,6 +128,46 @@ public class OgnlUtilTest extends XWorkTestCase {
         assertSame(expr0, expr2);
     }
 
+    public void testExpressionIsCachedIrrespectiveOfItsExecutionStatus() throws OgnlException {
+        Foo foo = new Foo();
+        OgnlContext context = (OgnlContext) ognlUtil.createDefaultContext(foo);
+
+        // Expression which executes with success
+        try {
+            ognlUtil.getValue("@com.opensymphony.xwork2.ognl.OgnlUtilTest@STATIC_FINAL_PUBLIC_ATTRIBUTE", context, foo);
+            assertEquals("Successfully executed expression must have been cached", ognlUtil.expressionCacheSize(), 1);
+        } catch (Exception ex) {
+            fail("Expression execution should have succeeded here. Exception: " + ex);
+        }
+        // Expression which executes with failure
+        try {
+            ognlUtil.getValue("@com.opensymphony.xwork2.ognl.OgnlUtilTest@STATIC_PRIVATE_ATTRIBUTE", context, foo);
+            fail("Expression execution should have failed here");
+        } catch (Exception ex) {
+            assertEquals("Expression with failed execution must have been cached nevertheless", ognlUtil.expressionCacheSize(), 2);
+        }
+    }
+
+    public void testMethodExpressionIsCachedIrrespectiveOfItsExecutionStatus() throws Exception {
+        Foo foo = new Foo();
+        OgnlContext context = (OgnlContext) ognlUtil.createDefaultContext(foo);
+
+        // Method expression which executes with success
+        try {
+            ognlUtil.callMethod("getBar()", context, foo);
+            assertTrue("Successfully executed method expression must have been cached", ognlUtil.expressionCacheSize() == 1);
+        } catch (Exception ex) {
+            fail("Method expression execution should have succeeded here. Exception: " + ex);
+        }
+        // Method expression which executes with failure
+        try {
+            ognlUtil.callMethod("getNonExistingMethod()", context, foo);
+            fail("Expression execution should have failed here");
+        } catch (Exception ex) {
+            assertEquals("Method expression with failed execution must have been cached nevertheless", ognlUtil.expressionCacheSize(), 2);
+        }
+    }
+
     public void testClearExpressionCache() throws OgnlException {
         ognlUtil.setEnableExpressionCache("true");
         // Test that the expression cache is functioning as expected.