[struts] branch master updated: (Amended commit based on feedback) Proposed fix for WW-5028 for the 2.5.x branch: - Disable printing stacktrace on exceptions by the Dispatcher by default. - Printing stacktrace on exception is only enabled with devMode set to true, as suggested by L. Lenart. - Now prints stacktrace on exception using LOG, as suggested by A. Mashchenko and the the JIRA reporter. Log level set to debug as recommended by Y. Zamani. - Added two additional unit tests for Dispatcher devMode and handleExceptio [...]

[ya...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

yasserzamani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/struts.git


The following commit(s) were added to refs/heads/master by this push:
     new f0776ae  (Amended commit based on feedback) Proposed fix for WW-5028 for the 2.5.x branch: - Disable printing stacktrace on exceptions by the Dispatcher by default. - Printing stacktrace on exception is only enabled with devMode set to true, as suggested by L. Lenart. - Now prints stacktrace on exception using LOG, as suggested by A. Mashchenko and the the JIRA reporter.   Log level set to debug as recommended by Y. Zamani. - Added two additional unit tests for Dispatcher devMode [...]
f0776ae is described below

commit f0776aeac6297c849b14921debec4c9cb5653c37
Author: JCgH4164838Gh792C124B5 <43...@users.noreply.github.com>
AuthorDate: Sat Apr 13 21:50:00 2019 +0430

    (Amended commit based on feedback)
    Proposed fix for WW-5028 for the 2.5.x branch:
    - Disable printing stacktrace on exceptions by the Dispatcher by default.
    - Printing stacktrace on exception is only enabled with devMode set to true, as suggested by L. Lenart.
    - Now prints stacktrace on exception using LOG, as suggested by A. Mashchenko and the the JIRA reporter.
      Log level set to debug as recommended by Y. Zamani.
    - Added two additional unit tests for Dispatcher devMode and handleException states.
    
    (cherry picked from commit 4815744)
---
 .../org/apache/struts2/dispatcher/Dispatcher.java  |  4 ++-
 .../apache/struts2/dispatcher/DispatcherTest.java  | 32 ++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java
index 67a9754..92b8ac0 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java
@@ -614,8 +614,10 @@ public class Dispatcher {
             logConfigurationException(request, e);
             sendError(request, response, HttpServletResponse.SC_NOT_FOUND, e);
         } catch (Exception e) {
-            e.printStackTrace();
             if (handleException || devMode) {
+                if (devMode) {
+                    LOG.debug("Dispatcher serviceAction failed", e);
+                }
                 sendError(request, response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e);
             } else {
                 throw new ServletException(e);
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java
index cef1534..43f0f9b 100644
--- a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java
+++ b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java
@@ -367,6 +367,38 @@ public class DispatcherTest extends StrutsInternalTestCase {
         assertTrue("should execute previous proxy", actionProxy.isExecutedCalled());
     }
 
+    /**
+     * Verify proper default (true) handleExceptionState for Dispatcher and that
+     * it properly reflects a manually configured change to false.
+     *
+     * @throws Exception
+     */
+    public void testHandleException() throws Exception {
+        Dispatcher du = initDispatcher(new HashMap<String, String>());
+        assertTrue("Default Dispatcher handleException state not true ?", du.isHandleException());
+
+        Dispatcher du2 = initDispatcher(new HashMap<String, String>() {{
+            put(StrutsConstants.STRUTS_HANDLE_EXCEPTION, "false");
+        }});
+        assertFalse("Modified Dispatcher handleException state not false ?", du2.isHandleException());
+    }
+
+    /**
+     * Verify proper default (false) devMode for Dispatcher and that
+     * it properly reflects a manually configured change to true.
+     *
+     * @throws Exception
+     */
+    public void testDevMode() throws Exception {
+        Dispatcher du = initDispatcher(new HashMap<String, String>());
+        assertFalse("Default Dispatcher devMode state not false ?", du.isDevMode());
+
+        Dispatcher du2 = initDispatcher(new HashMap<String, String>() {{
+            put(StrutsConstants.STRUTS_DEVMODE, "true");
+        }});
+        assertTrue("Modified Dispatcher devMode state not true ?", du2.isDevMode());
+    }
+
     class InternalConfigurationManager extends ConfigurationManager {
     	public boolean destroyConfiguration = false;