not log user exceptions as missing properties (WW-4999)

Also reaks loop on user method exceptions - but continue to next objects in stack on NoSuchMethodException. (cherry picked from commit 0999fba)
melanj · Jun 3, 2019 · 382124c · 382124c
1 parent b657a27
commit 382124c
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 4 deletions.
diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStack.java
@@ -339,7 +339,8 @@ protected Object handleOgnlException(String expr, boolean throwExceptionOnFailur
     }
 
     protected boolean shouldLogMissingPropertyWarning(OgnlException e) {
-        return (e instanceof NoSuchPropertyException || e instanceof MethodFailedException)
+        return (e instanceof NoSuchPropertyException ||
+                (e instanceof MethodFailedException && e.getReason() instanceof NoSuchMethodException))
         		&& devMode && logMissingProperties;
     }
 

diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java b/core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java
@@ -218,13 +218,13 @@ public Object callMethod(Map context, Object target, String name, Object[] objec
         }
 
         Throwable reason = null;
+        Class[] argTypes = getArgTypes(objects);
         for (Object o : root) {
             if (o == null) {
                 continue;
             }
 
             Class clazz = o.getClass();
-            Class[] argTypes = getArgTypes(objects);
 
             MethodCall mc = null;
 
@@ -236,12 +236,17 @@ public Object callMethod(Map context, Object target, String name, Object[] objec
                 try {
                     return OgnlRuntime.callMethod((OgnlContext) context, o, name, objects);
                 } catch (OgnlException e) {
-                    // try the next one
                     reason = e.getReason();
 
-                    if ((mc != null) && (reason != null) && (reason.getClass() == NoSuchMethodException.class)) {
+                    if (reason != null && !(reason instanceof NoSuchMethodException)) {
+                        // method has found but thrown an exception
+                        break;
+                    }
+
+                    if ((mc != null) && (reason != null)) {
                         invalidMethods.put(mc, Boolean.TRUE);
                     }
+                    // continue and try the next one
                 }
             }
         }

diff --git a/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java b/core/src/test/java/com/opensymphony/xwork2/ognl/OgnlValueStackTest.java
@@ -277,6 +277,43 @@ public void testLogMissingProperties() {
         }
     }
 
+    public void testNotLogUserExceptionsAsMissingProperties() {
+        OgnlValueStack vs = createValueStack();
+        vs.setDevMode("true");
+        vs.setLogMissingProperties("true");
+
+        Dog dog = new Dog();
+        vs.push(dog);
+
+        TestAppender testAppender = new TestAppender();
+        Logger logger = (Logger) LogManager.getLogger(OgnlValueStack.class);
+        logger.addAppender(testAppender);
+        testAppender.start();
+
+        try {
+            vs.setValue("exception", "exceptionValue", false);
+            vs.findValue("exception", false);
+            vs.findValue("exception", String.class, false);
+            vs.findValue("getException()", false);
+            vs.findValue("getException()", String.class, false);
+            vs.findValue("bite", false);
+            vs.findValue("bite", void.class, false);
+            vs.findValue("getBite()", false);
+            vs.findValue("getBite()", void.class, false);
+
+            assertEquals(8, testAppender.logEvents.size());
+            for (int i = 0; i < testAppender.logEvents.size(); i += 2) {
+                assertTrue(testAppender.logEvents.get(i).getMessage().getFormattedMessage()
+                        .startsWith("Caught an exception while evaluating expression '"));
+                assertEquals("NOTE: Previous warning message was issued due to devMode set to true.",
+                        testAppender.logEvents.get(i + 1).getMessage().getFormattedMessage());
+            }
+        } finally {
+            testAppender.stop();
+            logger.removeAppender(testAppender);
+        }
+    }
+
     public void testFailOnMissingMethod() {
         OgnlValueStack vs = createValueStack();