From 29ce6685cad0f6caf829c49024b1885ab31d62e0 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 1 Aug 2018 22:12:01 +0300 Subject: [PATCH] Add handleFailure property to FrameworkServlet Issue: SPR-17100 --- .../web/servlet/FrameworkServlet.java | 44 +++++++++++++++++-- ...stractMessageConverterMethodProcessor.java | 7 ++- .../RequestMappingHandlerAdapter.java | 4 +- .../web/servlet/DispatcherServletTests.java | 28 +++++++++++- 4 files changed, 74 insertions(+), 9 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java index 59217870d9..2d6675dfbc 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java @@ -209,6 +209,9 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic /** Should we dispatch an HTTP TRACE request to {@link #doService}?. */ private boolean dispatchTraceRequest = false; + /** Should we set the status to 500 for unhandled failures? */ + private boolean shouldHandleFailure = false; + /** WebApplicationContext for this servlet. */ @Nullable private WebApplicationContext webApplicationContext; @@ -472,6 +475,17 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic this.dispatchTraceRequest = dispatchTraceRequest; } + /** + * Whether to handle failures wth {@code response.sendError(500)} as opposed + * to letting them propagate to the container which may log a stacktrace + * even if an error dispatch (e.g. Spring Boot app) handles the exception. + * @param shouldHandleFailure whether to handle failures or propagate + * @since 5.1 + */ + public void setShouldHandleFailure(boolean shouldHandleFailure) { + this.shouldHandleFailure = shouldHandleFailure; + } + /** * Whether to log request params at DEBUG level, and headers at TRACE level. * Both may contain sensitive information. @@ -998,12 +1012,16 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic doService(request, response); } catch (ServletException | IOException ex) { - failureCause = ex; - throw ex; + if (!handleFailure(request, response, ex)) { + failureCause = ex; + throw ex; + } } catch (Throwable ex) { - failureCause = ex; - throw new NestedServletException("Request processing failed", ex); + if (!handleFailure(request, response, ex)) { + failureCause = ex; + throw new NestedServletException("Request processing failed", ex); + } } finally { @@ -1069,6 +1087,24 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic RequestContextHolder.setRequestAttributes(previousAttributes, this.threadContextInheritable); } + private boolean handleFailure(HttpServletRequest request, HttpServletResponse response, Throwable ex) { + if (this.shouldHandleFailure) { + try { + response.sendError(500); + } + catch (IOException ex2) { + if (logger.isDebugEnabled()) { + logger.debug("Handling of failure failed: " + ex2); + } + return false; + } + request.setAttribute(WebUtils.ERROR_STATUS_CODE_ATTRIBUTE, 500); + WebUtils.exposeErrorRequestAttributes(request, ex, getServletName()); + return true; + } + return false; + } + private void logResult(HttpServletRequest request, HttpServletResponse response, @Nullable Throwable failureCause, WebAsyncManager asyncManager) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index 1c5c90be22..5077764fd9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -281,8 +281,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe inputMessage, outputMessage); if (body != null) { if (logger.isDebugEnabled()) { - Object formatted = (body instanceof CharSequence ? "\"" + body + "\"" : body); - logger.debug("Writing [" + formatted + "]"); + logger.debug("Writing [" + formatValue(body) + "]"); } addContentDispositionHeader(inputMessage, outputMessage); if (genericConverter != null) { @@ -399,6 +398,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe return (MediaType.SPECIFICITY_COMPARATOR.compare(acceptType, produceTypeToUse) <= 0 ? acceptType : produceTypeToUse); } + static String formatValue(Object body) { + return (body instanceof CharSequence ? "\"" + body + "\"" : body.toString()); + } + /** * Check if the path has a file extension and whether the extension is * either {@link #WHITELISTED_EXTENSIONS whitelisted} or explicitly diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 99bbcc3dd8..4247fa08d8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -871,8 +871,8 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter mavContainer = (ModelAndViewContainer) asyncManager.getConcurrentResultContext()[0]; asyncManager.clearConcurrentResult(); if (logger.isDebugEnabled()) { - logger.debug("Resume with async result [" + - (result instanceof CharSequence ? "\"" + result + "\"" : result) + "]"); + String formatted = AbstractMessageConverterMethodProcessor.formatValue(result); + logger.debug("Resume with async result [" + formatted + "]"); } invocableMethod = invocableMethod.wrapConcurrentResult(result); } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java index ea9b7fd74a..7eff968f68 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -614,6 +614,32 @@ public class DispatcherServletTests { assertTrue(!ex.toString().contains("bar")); } + @Test // SPR-17100 + public void shouldHandleFailure() throws ServletException, IOException { + + IllegalStateException ex = new IllegalStateException("dummy"); + @SuppressWarnings("serial") + FrameworkServlet servlet = new FrameworkServlet() { + @Override + protected void doService(HttpServletRequest request, HttpServletResponse response) { + throw ex; + } + }; + servlet.setShouldHandleFailure(true); + + MockHttpServletRequest request = new MockHttpServletRequest(getServletContext(), "GET", "/error"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + servlet.service(request, response); + + assertEquals(500, response.getStatus()); + assertEquals(500, request.getAttribute(WebUtils.ERROR_STATUS_CODE_ATTRIBUTE)); + assertEquals(ex.getClass(), request.getAttribute(WebUtils.ERROR_EXCEPTION_TYPE_ATTRIBUTE)); + assertEquals(ex.getMessage(), request.getAttribute(WebUtils.ERROR_MESSAGE_ATTRIBUTE)); + assertEquals(ex, request.getAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE)); + assertEquals(request.getRequestURI(), request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)); + } + @Test public void cleanupAfterIncludeWithRemove() throws ServletException, IOException { MockHttpServletRequest request = new MockHttpServletRequest(getServletContext(), "GET", "/main.do");