diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 1cb4ab578..3423ec8bd 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -279,4 +279,6 @@ public final class StrutsConstants { public static final String DEFAULT_TEMPLATE_TYPE_CONFIG_KEY = "struts.ui.templateSuffix"; + /** Allows override default DispatcherErrorHandler **/ + public static final String STRUTS_DISPATCHER_ERROR_HANDLER = "struts.dispatcher.errorHandler"; } diff --git a/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java index c4d89032d..b6b5b4590 100644 --- a/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/DefaultBeanSelectionProvider.java @@ -60,6 +60,7 @@ import com.opensymphony.xwork2.util.reflection.ReflectionProvider; import com.opensymphony.xwork2.validator.ActionValidatorManager; import org.apache.struts2.StrutsConstants; import org.apache.struts2.components.UrlRenderer; +import org.apache.struts2.dispatcher.DispatcherErrorHandler; import org.apache.struts2.dispatcher.StaticContentLoader; import org.apache.struts2.dispatcher.mapper.ActionMapper; import org.apache.struts2.dispatcher.multipart.MultiPartRequest; @@ -380,6 +381,8 @@ public class DefaultBeanSelectionProvider extends AbstractBeanSelectionProvider alias(TextParser.class, StrutsConstants.STRUTS_EXPRESSION_PARSER, builder, props); + alias(DispatcherErrorHandler.class, StrutsConstants.STRUTS_DISPATCHER_ERROR_HANDLER, builder, props); + switchDevMode(props); // Convert Struts properties into XWork properties diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java new file mode 100644 index 000000000..c093402f1 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java @@ -0,0 +1,124 @@ +package org.apache.struts2.dispatcher; + +import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.util.location.Location; +import com.opensymphony.xwork2.util.location.LocationUtils; +import com.opensymphony.xwork2.util.logging.Logger; +import com.opensymphony.xwork2.util.logging.LoggerFactory; +import freemarker.template.Template; +import org.apache.struts2.StrutsConstants; +import org.apache.struts2.StrutsException; +import org.apache.struts2.views.freemarker.FreemarkerManager; + +import javax.servlet.ServletContext; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.io.StringWriter; +import java.io.Writer; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + +/** + * Default implementation of {@link org.apache.struts2.dispatcher.DispatcherErrorHandler} + * which sends Error Report in devMode or {@link javax.servlet.http.HttpServletResponse#sendError} otherwise. + */ +public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler { + + private static final Logger LOG = LoggerFactory.getLogger(DefaultDispatcherErrorHandler.class); + + private FreemarkerManager freemarkerManager; + private boolean devMode; + private Template template; + + @Inject + public void setFreemarkerManager(FreemarkerManager freemarkerManager) { + this.freemarkerManager = freemarkerManager; + } + + @Inject(StrutsConstants.STRUTS_DEVMODE) + public void setDevMode(String devMode) { + this.devMode = "true".equalsIgnoreCase(devMode); + } + + public void init(ServletContext ctx) { + try { + freemarker.template.Configuration config = freemarkerManager.getConfiguration(ctx); + template = config.getTemplate("/org/apache/struts2/dispatcher/error.ftl"); + } catch (IOException e) { + throw new StrutsException(e); + } + } + + public void handleError(HttpServletRequest request, HttpServletResponse response, int code, Exception e) { + Boolean devModeOverride = FilterDispatcher.getDevModeOverride(); + if (devModeOverride != null ? devModeOverride : devMode) { + handleErrorInDevMode(response, code, e); + } else { + sendErrorResponse(request, response, code, e); + } + } + + protected void sendErrorResponse(HttpServletRequest request, HttpServletResponse response, int code, Exception e) { + try { + // WW-1977: Only put errors in the request when code is a 500 error + if (code == HttpServletResponse.SC_INTERNAL_SERVER_ERROR) { + // WW-4103: Only logs error when application error occurred, not Struts error + if (LOG.isErrorEnabled()) { + LOG.error("Exception occurred during processing request: #0", e, e.getMessage()); + } + // send a http error response to use the servlet defined error handler + // make the exception available to the web.xml defined error page + request.setAttribute("javax.servlet.error.exception", e); + + // for compatibility + request.setAttribute("javax.servlet.jsp.jspException", e); + } + + // send the error response + response.sendError(code, e.getMessage()); + } catch (IOException e1) { + // we're already sending an error, not much else we can do if more stuff breaks + } + } + + protected void handleErrorInDevMode(HttpServletResponse response, int code, Exception e) { + if (LOG.isDebugEnabled()) { + LOG.debug("Exception occurred during processing request: #0", e, e.getMessage()); + } + try { + List chain = new ArrayList(); + Throwable cur = e; + chain.add(cur); + while ((cur = cur.getCause()) != null) { + chain.add(cur); + } + + Writer writer = new StringWriter(); + template.process(createReportData(e, chain), writer); + + response.setContentType("text/html"); + response.getWriter().write(writer.toString()); + response.getWriter().close(); + } catch (Exception exp) { + try { + if (LOG.isDebugEnabled()) { + LOG.debug("Cannot show problem report!", exp); + } + 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 + } + } + } + + protected HashMap createReportData(Exception e, List chain) { + HashMap data = new HashMap(); + data.put("exception", e); + data.put("unknown", Location.UNKNOWN); + data.put("chain", chain); + data.put("locator", new Dispatcher.Locator()); + return data; + } +} 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 9c490ac32..032df3617 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -21,20 +21,8 @@ package org.apache.struts2.dispatcher; -import com.opensymphony.xwork2.ActionContext; -import com.opensymphony.xwork2.ActionProxy; -import com.opensymphony.xwork2.ActionProxyFactory; -import com.opensymphony.xwork2.FileManager; -import com.opensymphony.xwork2.FileManagerFactory; -import com.opensymphony.xwork2.LocaleProvider; -import com.opensymphony.xwork2.ObjectFactory; -import com.opensymphony.xwork2.Result; -import com.opensymphony.xwork2.config.Configuration; -import com.opensymphony.xwork2.config.ConfigurationException; -import com.opensymphony.xwork2.config.ConfigurationManager; -import com.opensymphony.xwork2.config.ConfigurationProvider; -import com.opensymphony.xwork2.config.FileManagerFactoryProvider; -import com.opensymphony.xwork2.config.FileManagerProvider; +import com.opensymphony.xwork2.*; +import com.opensymphony.xwork2.config.*; import com.opensymphony.xwork2.config.entities.InterceptorMapping; import com.opensymphony.xwork2.config.entities.InterceptorStackConfig; import com.opensymphony.xwork2.config.entities.PackageConfig; @@ -53,7 +41,6 @@ import com.opensymphony.xwork2.util.location.LocationUtils; import com.opensymphony.xwork2.util.logging.Logger; import com.opensymphony.xwork2.util.logging.LoggerFactory; import com.opensymphony.xwork2.util.profiling.UtilTimerStack; -import freemarker.template.Template; import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsConstants; import org.apache.struts2.StrutsException; @@ -68,7 +55,6 @@ import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; import org.apache.struts2.util.AttributeMap; import org.apache.struts2.util.ObjectFactoryDestroyable; import org.apache.struts2.util.fs.JBossFileManager; -import org.apache.struts2.views.freemarker.FreemarkerManager; import javax.servlet.ServletContext; import javax.servlet.ServletException; @@ -76,16 +62,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.File; import java.io.IOException; -import java.io.StringWriter; -import java.io.Writer; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.CopyOnWriteArrayList; /** @@ -93,7 +70,7 @@ import java.util.concurrent.CopyOnWriteArrayList; * of the primary dispatcher holds an instance of this dispatcher to be shared for * all requests. * - * @see org.apache.struts2.dispatcher.FilterDispatcher + * @see org.apache.struts2.dispatcher.ng.InitOperations */ public class Dispatcher { @@ -168,6 +145,11 @@ public class Dispatcher { */ private boolean handleException; + /** + * Interface used to handle internal errors or missing resources + */ + private DispatcherErrorHandler errorHandler; + /** * Provide the dispatcher instance for the current thread. * @@ -280,6 +262,11 @@ public class Dispatcher { this.handleException = Boolean.parseBoolean(handleException); } + @Inject + public void setDispatcherErrorHandler(DispatcherErrorHandler errorHandler) { + this.errorHandler = errorHandler; + } + /** * Releases all instances bound to this dispatcher instance. */ @@ -492,6 +479,10 @@ public class Dispatcher { l.dispatcherInitialized(this); } } + //if (servletContext != null) { + errorHandler.init(servletContext); + //} + } catch (Exception ex) { if (LOG.isErrorEnabled()) LOG.error("Dispatcher initialization failed", ex); @@ -564,10 +555,10 @@ public class Dispatcher { } } catch (ConfigurationException e) { logConfigurationException(request, e); - sendError(request, response, context, HttpServletResponse.SC_NOT_FOUND, e); + sendError(request, response, HttpServletResponse.SC_NOT_FOUND, e); } catch (Exception e) { if (handleException || devMode) { - sendError(request, response, context, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e); + sendError(request, response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e); } else { throw new ServletException(e); } @@ -847,70 +838,26 @@ public class Dispatcher { * @param code the HttpServletResponse error code (see {@link javax.servlet.http.HttpServletResponse} for possible error codes). * @param e the Exception that is reported. * @param ctx the ServletContext object. + * + * @deprecated remove in version 3.0 - use version without ServletContext parameter */ + @Deprecated public void sendError(HttpServletRequest request, HttpServletResponse response, ServletContext ctx, int code, Exception e) { - Boolean devModeOverride = FilterDispatcher.getDevModeOverride(); - if (devModeOverride != null ? devModeOverride : devMode) { - if (LOG.isDebugEnabled()) { - LOG.debug("Exception occurred during processing request: #0", e, e.getMessage()); - } - try { - FreemarkerManager mgr = getContainer().getInstance(FreemarkerManager.class); + sendError(request, response, code, e); + } - freemarker.template.Configuration config = mgr.getConfiguration(ctx); - Template template = config.getTemplate("/org/apache/struts2/dispatcher/error.ftl"); - - List chain = new ArrayList(); - Throwable cur = e; - chain.add(cur); - while ((cur = cur.getCause()) != null) { - chain.add(cur); - } - - HashMap data = new HashMap(); - data.put("exception", e); - data.put("unknown", Location.UNKNOWN); - data.put("chain", chain); - data.put("locator", new Locator()); - - Writer writer = new StringWriter(); - template.process(data, writer); - - response.setContentType("text/html"); - response.getWriter().write(writer.toString()); - response.getWriter().close(); - } catch (Exception exp) { - try { - if (LOG.isDebugEnabled()) { - LOG.debug("Cannot show problem report!", exp); - } - 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 - } - } - } else { - try { - // WW-1977: Only put errors in the request when code is a 500 error - if (code == HttpServletResponse.SC_INTERNAL_SERVER_ERROR) { - // WW-4103: Only logs error when application error occurred, not Struts error - if (LOG.isErrorEnabled()) { - LOG.error("Exception occurred during processing request: #0", e, e.getMessage()); - } - // send a http error response to use the servlet defined error handler - // make the exception availible to the web.xml defined error page - request.setAttribute("javax.servlet.error.exception", e); - - // for compatibility - request.setAttribute("javax.servlet.jsp.jspException", e); - } - - // send the error response - response.sendError(code, e.getMessage()); - } catch (IOException e1) { - // we're already sending an error, not much else we can do if more stuff breaks - } - } + /** + * Send an HTTP error response code. + * + * @param request the HttpServletRequest object. + * @param response the HttpServletResponse object. + * @param code the HttpServletResponse error code (see {@link javax.servlet.http.HttpServletResponse} for possible error codes). + * @param e the Exception that is reported. + * + * @since 2.3.17 + */ + public void sendError(HttpServletRequest request, HttpServletResponse response, int code, Exception e) { + errorHandler.handleError(request, response, code, e); } /** diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DispatcherErrorHandler.java b/core/src/main/java/org/apache/struts2/dispatcher/DispatcherErrorHandler.java new file mode 100644 index 000000000..fe8f6ecc7 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/DispatcherErrorHandler.java @@ -0,0 +1,29 @@ +package org.apache.struts2.dispatcher; + +import javax.servlet.ServletContext; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +/** + * Implementation of this interface is used to handle internal errors or missing resources. + * Basically it sends back HTTP error codes or error page depends on requirements. + */ +public interface DispatcherErrorHandler { + + /** + * Init instance after creating {@link org.apache.struts2.dispatcher.Dispatcher} + * @param ctx current {@link javax.servlet.ServletContext} + */ + public void init(ServletContext ctx); + + /** + * Handle passed error code or exception + * + * @param request current {@link javax.servlet.http.HttpServletRequest} + * @param response current {@link javax.servlet.http.HttpServletResponse} + * @param code HTTP Error Code, see {@link javax.servlet.http.HttpServletResponse} for possible error codes + * @param e Exception to report + */ + public void handleError(HttpServletRequest request, HttpServletResponse response, int code, Exception e); + +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/FilterDispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/FilterDispatcher.java index 6f38026ef..970311ce5 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/FilterDispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/FilterDispatcher.java @@ -409,7 +409,7 @@ public class FilterDispatcher implements StrutsStatics, Filter { mapping = actionMapper.getMapping(request, dispatcher.getConfigurationManager()); } catch (Exception ex) { log.error("error getting ActionMapping", ex); - dispatcher.sendError(request, response, servletContext, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex); + dispatcher.sendError(request, response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex); return; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ng/PrepareOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/ng/PrepareOperations.java index 93e105510..a26a8b2d5 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/ng/PrepareOperations.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/ng/PrepareOperations.java @@ -163,7 +163,7 @@ public class PrepareOperations { request.setAttribute(STRUTS_ACTION_MAPPING_KEY, mapping); } } catch (Exception ex) { - dispatcher.sendError(request, response, servletContext, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex); + dispatcher.sendError(request, response, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, ex); } } diff --git a/core/src/main/resources/struts-default.xml b/core/src/main/resources/struts-default.xml index 635b241cb..c65963af8 100644 --- a/core/src/main/resources/struts-default.xml +++ b/core/src/main/resources/struts-default.xml @@ -132,6 +132,10 @@ + + + + diff --git a/core/src/test/java/org/apache/struts2/dispatcher/FilterDispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/FilterDispatcherTest.java index 0d2845604..2c2d747ba 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/FilterDispatcherTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/FilterDispatcherTest.java @@ -125,6 +125,11 @@ public class FilterDispatcherTest extends StrutsInternalTestCase { public void serviceAction(HttpServletRequest request, HttpServletResponse response, ServletContext context, ActionMapping mapping) throws ServletException { serviceRequest = true; } + + @Override + public void sendError(HttpServletRequest request, HttpServletResponse response, int code, Exception e) { + // NO-OP + } } public static class InnerDispatcher extends Dispatcher { diff --git a/plugins/codebehind/src/test/java/org/apache/struts2/codebehind/CodebehindUnknownHandlerTest.java b/plugins/codebehind/src/test/java/org/apache/struts2/codebehind/CodebehindUnknownHandlerTest.java index dada8161d..e6cff7965 100644 --- a/plugins/codebehind/src/test/java/org/apache/struts2/codebehind/CodebehindUnknownHandlerTest.java +++ b/plugins/codebehind/src/test/java/org/apache/struts2/codebehind/CodebehindUnknownHandlerTest.java @@ -29,6 +29,7 @@ import com.opensymphony.xwork2.config.entities.ResultTypeConfig; import com.opensymphony.xwork2.util.XWorkTestCaseHelper; import org.apache.struts2.StrutsTestCase; import org.apache.struts2.dispatcher.ServletDispatcherResult; +import org.springframework.mock.web.MockServletContext; import javax.servlet.ServletContext; import java.net.MalformedURLException; @@ -46,6 +47,7 @@ public class CodebehindUnknownHandlerTest extends StrutsTestCase { configuration = configurationManager.getConfiguration(); container = configuration.getContainer(); actionProxyFactory = container.getInstance(ActionProxyFactory.class); + servletContext = new MockServletContext(); initDispatcher(Collections.singletonMap("actionPackages", "foo.bar")); mockServletContext = new Mock(ServletContext.class); handler = new CodebehindUnknownHandler("codebehind-default", configuration); diff --git a/plugins/portlet/src/test/java/org/apache/struts2/views/jsp/PortletUrlTagTest.java b/plugins/portlet/src/test/java/org/apache/struts2/views/jsp/PortletUrlTagTest.java index 5f34cfc81..8bdb3fbcb 100644 --- a/plugins/portlet/src/test/java/org/apache/struts2/views/jsp/PortletUrlTagTest.java +++ b/plugins/portlet/src/test/java/org/apache/struts2/views/jsp/PortletUrlTagTest.java @@ -35,21 +35,14 @@ import org.apache.struts2.portlet.util.PortletUrlHelper; import org.jmock.Mock; import org.jmock.cglib.MockObjectTestCase; import org.jmock.core.Constraint; +import org.springframework.mock.web.MockServletContext; -import javax.portlet.PortletContext; -import javax.portlet.PortletMode; -import javax.portlet.PortletURL; -import javax.portlet.RenderRequest; -import javax.portlet.RenderResponse; -import javax.portlet.WindowState; +import javax.portlet.*; +import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.jsp.PageContext; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.Map; +import java.util.*; import static org.apache.struts2.StrutsStatics.STRUTS_PORTLET_CONTEXT; @@ -92,7 +85,8 @@ public class PortletUrlTagTest extends MockObjectTestCase { public void setUp() throws Exception { super.setUp(); - dispatcher = new Dispatcher(null, new HashMap()); + ServletContext servletContext = new MockServletContext(); + dispatcher = new Dispatcher(servletContext, new HashMap()); dispatcher.init(); Dispatcher.setInstance(dispatcher); @@ -100,7 +94,7 @@ public class PortletUrlTagTest extends MockObjectTestCase { stack.getContext().put(ActionContext.CONTAINER, dispatcher.getContainer()); ActionContext context = new ActionContext(stack.getContext()); ActionContext.setContext(context); - + mockActionInvocation = mock(ActionInvocation.class); mockActionProxy = mock(ActionProxy.class); mockHttpReq = mock(HttpServletRequest.class); @@ -111,7 +105,7 @@ public class PortletUrlTagTest extends MockObjectTestCase { mockPortletUrl = mock(PortletURL.class); mockJspWriter = new MockJspWriter(); mockCtx = mock(PortletContext.class); - + mockActionProxy.stubs().method("getNamespace").will(returnValue("/view")); mockActionInvocation.stubs().method("getProxy").will(returnValue( mockActionProxy.proxy())); diff --git a/xwork-core/src/test/java/com/opensymphony/xwork2/TestNGXWorkTestCaseTest.java b/xwork-core/src/test/java/com/opensymphony/xwork2/TestNGXWorkTestCaseTest.java index 7ab7dc221..d13ac13b2 100644 --- a/xwork-core/src/test/java/com/opensymphony/xwork2/TestNGXWorkTestCaseTest.java +++ b/xwork-core/src/test/java/com/opensymphony/xwork2/TestNGXWorkTestCaseTest.java @@ -27,6 +27,7 @@ public class TestNGXWorkTestCaseTest extends TestCase { TestListenerAdapter tla = new TestListenerAdapter(); TestNG testng = new TestNG(); testng.setTestClasses(new Class[] { RunTest.class }); + testng.setOutputDirectory("target/surefire-reports"); testng.addListener(tla); try { testng.run();