struts-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lukaszlen...@apache.org
Subject [struts] branch master updated: Merge pull request #366 from JCgH4164838Gh792C124B5/local_25x_SendErrorEnh
Date Tue, 01 Oct 2019 06:37:12 GMT
This is an automated email from the ASF dual-hosted git repository.

lukaszlenart 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 1a93a30  Merge pull request #366 from JCgH4164838Gh792C124B5/local_25x_SendErrorEnh
     new 544c353  Merge pull request #369 from JCgH4164838Gh792C124B5/local_26x_CPickPR366
1a93a30 is described below

commit 1a93a300bb75184179bc223441d9d6684f0434f1
Author: Lukasz Lenart <lukaszlenart@apache.org>
AuthorDate: Mon Sep 23 08:54:02 2019 +0200

    Merge pull request #366 from JCgH4164838Gh792C124B5/local_25x_SendErrorEnh
    
    Improved logging for DefaultDispatcherErrorHandler, DefaultStaticContentLoader
    
    (cherry picked from commit 12d4feaf8f90b3af3853eb53263410e8e6f1e956)
---
 .../dispatcher/DefaultDispatcherErrorHandler.java  |   8 ++
 .../dispatcher/DefaultStaticContentLoader.java     |  10 +-
 .../DefaultDispatcherErrorHandlerTest.java         | 138 +++++++++++++++++++++
 .../dispatcher/DefaultStaticContentLoaderTest.java |  62 +++++++++
 4 files changed, 217 insertions(+), 1 deletion(-)

diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java
b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java
index 0769771..551ff1e 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java
@@ -97,6 +97,10 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler
{
             response.sendError(code, e.getMessage());
         } catch (IOException e1) {
             // we're already sending an error, not much else we can do if more stuff breaks
+            LOG.warn("Unable to send error response, code: {};", code, e1);
+        } catch (IllegalStateException ise) {
+            // Log illegalstate instead of passing unrecoverable exception to calling thread
+            LOG.warn("Unable to send error response, code: {}; isCommited: {};", code, response.isCommitted(),
ise);
         }
     }
 
@@ -122,6 +126,10 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler
{
                 response.sendError(code, "Unable to show problem report:\n" + exp + "\n\n"
+ LocationUtils.getLocation(exp));
             } catch (IOException ex) {
                 // we're already sending an error, not much else we can do if more stuff
breaks
+                LOG.warn("Unable to send error response, code: {};", code, ex);
+            } catch (IllegalStateException ise) {
+                // Log illegalstate instead of passing unrecoverable exception to calling
thread
+                LOG.warn("Unable to send error response, code: {}; isCommited: {};", code,
response.isCommitted(), ise);
             }
         }
     }
diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
index eeedbf5..42f1716 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultStaticContentLoader.java
@@ -218,7 +218,15 @@ public class DefaultStaticContentLoader implements StaticContentLoader
{
             }
         }
 
-        response.sendError(HttpServletResponse.SC_NOT_FOUND);
+        try {
+            response.sendError(HttpServletResponse.SC_NOT_FOUND);
+        } catch (IOException e1) {
+            // we're already sending an error, not much else we can do if more stuff breaks
+            LOG.warn("Unable to send error response, code: {};", HttpServletResponse.SC_NOT_FOUND,
e1);
+        } catch (IllegalStateException ise) {
+            // Log illegalstate instead of passing unrecoverable exception to calling thread
+            LOG.warn("Unable to send error response, code: {}; isCommited: {};", HttpServletResponse.SC_NOT_FOUND,
response.isCommitted(), ise);
+        }
     }
 
     protected void process(InputStream is, String path, HttpServletRequest request, HttpServletResponse
response) throws IOException {
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java
new file mode 100644
index 0000000..e89dd39
--- /dev/null
+++ b/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.dispatcher;
+
+import java.io.IOException;
+import java.util.Collections;
+import org.apache.struts2.StrutsInternalTestCase;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.struts2.views.freemarker.FreemarkerManager;
+import static org.easymock.EasyMock.anyInt;
+import static org.easymock.EasyMock.anyString;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.replay;
+
+public class DefaultDispatcherErrorHandlerTest extends StrutsInternalTestCase {
+    private HttpServletRequest requestMock;
+    private HttpServletResponse responseMock;
+
+    /**
+     * Test to exercise the code path and prove handleError() will output 
+     * the desired log warning when an IOException is thrown with devMode false.
+     */
+    public void testHandleErrorIOException() {
+        DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler();
+        defaultDispatcherErrorHandler.setDevMode("false");
+        defaultDispatcherErrorHandler.setFreemarkerManager(dispatcher.getContainer().getInstance(FreemarkerManager.class));
+        defaultDispatcherErrorHandler.init(dispatcher.servletContext);
+        Exception fakeException = new Exception("Fake Exception, devMode false");
+        try {
+            requestMock.setAttribute("javax.servlet.error.exception", fakeException);
+            expectLastCall();
+            requestMock.setAttribute("javax.servlet.jsp.jspException", fakeException);
+            expectLastCall();
+            responseMock.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException.getMessage());
+            expectLastCall().andStubThrow(new IOException("Fake IO Exception (SC_INTERNAL_SERVER_ERROR,
devMode false)"));
+            replay(responseMock);
+        } catch (IOException ioe) {
+            fail("Mock sendError call setup failed.  Ex: " + ioe);
+        }
+        defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
fakeException);
+    }
+
+    /**
+     * Test to exercise the code path and prove handleError() will output 
+     * the desired log warning when an IOException is thrown with devMode true.
+     */
+    public void testHandleErrorIOExceptionDevMode() {
+        DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler();
+        defaultDispatcherErrorHandler.setDevMode("true");
+        defaultDispatcherErrorHandler.setFreemarkerManager(dispatcher.getContainer().getInstance(FreemarkerManager.class));
+        defaultDispatcherErrorHandler.init(dispatcher.servletContext);
+        Exception fakeException = new Exception("Fake Exception, devMode true");
+        try {
+            responseMock.setContentType("text/html");
+            expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception
(report write)"));  // Fake error during report write
+            responseMock.sendError(anyInt(), anyString());
+            expectLastCall().andStubThrow(new IOException("Fake IO Exception (SC_INTERNAL_SERVER_ERROR,
devMode true)"));
+            replay(responseMock);
+        } catch (IOException ioe) {
+            fail("Mock sendError call setup failed.  Ex: " + ioe);
+        }
+        defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
fakeException);
+    }
+
+    /**
+     * Test to exercise the code path and prove handleError() will output 
+     * the desired log warning when an IllegalStateException is thrown with devMode false.
+     */
+    public void testHandleErrorIllegalStateException() {
+        DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler();
+        defaultDispatcherErrorHandler.setDevMode("false");
+        defaultDispatcherErrorHandler.setFreemarkerManager(dispatcher.getContainer().getInstance(FreemarkerManager.class));
+        defaultDispatcherErrorHandler.init(dispatcher.servletContext);
+        Exception fakeException = new Exception("Fake Exception, devMode false");
+        try {
+            requestMock.setAttribute("javax.servlet.error.exception", fakeException);
+            expectLastCall();
+            requestMock.setAttribute("javax.servlet.jsp.jspException", fakeException);
+            expectLastCall();
+            expect(responseMock.isCommitted()).andStubReturn(Boolean.TRUE);
+            responseMock.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException.getMessage());
+            expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception
(SC_INTERNAL_SERVER_ERROR, devMode false)"));
+            replay(responseMock);
+        } catch (IOException ioe) {
+            fail("Mock sendError call setup failed.  Ex: " + ioe);
+        }
+        defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
fakeException);
+    }
+
+    /**
+     * Test to exercise the code path and prove handleError() will output 
+     * the desired log warning when an IllegalStateException is thrown with devMode true.
+     */
+    public void testHandleErrorIllegalStateExceptionDevMode() {
+        DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler();
+        defaultDispatcherErrorHandler.setDevMode("true");
+        defaultDispatcherErrorHandler.setFreemarkerManager(dispatcher.getContainer().getInstance(FreemarkerManager.class));
+        defaultDispatcherErrorHandler.init(dispatcher.servletContext);
+        Exception fakeException = new Exception("Fake Exception, devMode true");
+        try {
+            expect(responseMock.isCommitted()).andStubReturn(Boolean.TRUE);
+            responseMock.setContentType("text/html");
+            expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception
(report write)"));  // Fake error during report write
+            responseMock.sendError(anyInt(), anyString());
+            expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception
(SC_INTERNAL_SERVER_ERROR, devMode true)"));
+            replay(responseMock);
+        } catch (IOException ioe) {
+            fail("Mock sendError call setup failed.  Ex: " + ioe);
+        }
+        defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
fakeException);
+    }
+
+    protected void setUp() {
+        requestMock = (HttpServletRequest) createMock(HttpServletRequest.class);
+        responseMock = (HttpServletResponse) createMock(HttpServletResponse.class);
+        dispatcher = initDispatcher(Collections.<String, String>emptyMap());
+    }
+}
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
index e8d077d..8f8eda1 100644
--- a/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
+++ b/core/src/test/java/org/apache/struts2/dispatcher/DefaultStaticContentLoaderTest.java
@@ -18,11 +18,22 @@
  */
 package org.apache.struts2.dispatcher;
 
+import java.io.IOException;
 import org.apache.struts2.StrutsInternalTestCase;
 
 import java.util.List;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.replay;
 
 public class DefaultStaticContentLoaderTest extends StrutsInternalTestCase {
+    private HttpServletRequest requestMock;
+    private HttpServletResponse responseMock;
+    private HostConfig hostConfigMock;
+    private DefaultStaticContentLoader defaultStaticContentLoader;
 
     public void testParsePackages() throws Exception {
 
@@ -50,4 +61,55 @@ public class DefaultStaticContentLoaderTest extends StrutsInternalTestCase
{
         assertEquals(result4.get(3), "foo/bar/package4/");
     }
 
+    /**
+     * Test to exercise the code path and prove findStaticResource() will output 
+     * the desired log warning when an IOException is thrown.
+     */
+    public void testFindStaticResourceIOException() {
+        expect(requestMock.getDateHeader("If-Modified-Since")).andStubReturn(0L);
+        try {
+            responseMock.sendError(HttpServletResponse.SC_NOT_FOUND);
+            expectLastCall().andStubThrow(new IOException("Fake IO Exception (SC_NOT_FOUND)"));
+            replay(responseMock);
+        } catch (IOException ioe) {
+            fail("Mock sendError call setup failed.  Ex: " + ioe);
+        }
+        try {
+            defaultStaticContentLoader.findStaticResource("/static/fake.html", requestMock,
responseMock);
+        } catch (IOException ioe) {
+            fail("DefaultStaticContentLoader.findStaticResource() call failed.  Ex: " + ioe);
+        }
+    }
+
+    /**
+     * Test to exercise the code path and prove findStaticResource() will output 
+     * the desired log warning when an IllegalStateException is thrown.
+     */
+    public void testFindStaticResourceIllegalStateException() {
+        expect(requestMock.getDateHeader("If-Modified-Since")).andStubReturn(0L);
+        try {
+            expect(responseMock.isCommitted()).andStubReturn(Boolean.TRUE);
+            responseMock.sendError(HttpServletResponse.SC_NOT_FOUND);
+            expectLastCall().andStubThrow(new IllegalStateException("Fake IllegalState Exception
(SC_NOT_FOUND)"));
+            replay(responseMock);
+        } catch (IOException ioe) {
+            fail("Mock sendError call setup failed.  Ex: " + ioe);
+        }
+        try {
+            defaultStaticContentLoader.findStaticResource("/static/fake.html", requestMock,
responseMock);
+        } catch (IOException ioe) {
+            fail("DefaultStaticContentLoader.findStaticResource() call failed.  Ex: " + ioe);
+        }
+    }
+
+    protected void setUp() {
+        requestMock = (HttpServletRequest) createMock(HttpServletRequest.class);
+        responseMock = (HttpServletResponse) createMock(HttpServletResponse.class);
+        hostConfigMock = (HostConfig) createMock(HostConfig.class);
+        expect(hostConfigMock.getInitParameter("packages")).andStubReturn(null);
+        expect(hostConfigMock.getInitParameter("loggerFactory")).andStubReturn(null);
+        defaultStaticContentLoader = new DefaultStaticContentLoader();
+        defaultStaticContentLoader.setHostConfig(hostConfigMock);
+        defaultStaticContentLoader.setEncoding("UTF-8");
+    }
 }


Mime
View raw message