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 07697714f..551ff1e6a 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 eeedbf543..42f17160e 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 000000000..e89dd3913 --- /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.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 e8d077df0..8f8eda1eb 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"); + } }