You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2019/10/01 06:37:12 UTC
[struts] branch master updated: Merge pull request #366 from
JCgH4164838Gh792C124B5/local_25x_SendErrorEnh
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 <lu...@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");
+ }
}