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 2d6675dfbc..59217870d9 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,9 +209,6 @@ 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; @@ -475,17 +472,6 @@ 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. @@ -1012,16 +998,12 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic doService(request, response); } catch (ServletException | IOException ex) { - if (!handleFailure(request, response, ex)) { - failureCause = ex; - throw ex; - } + failureCause = ex; + throw ex; } catch (Throwable ex) { - if (!handleFailure(request, response, ex)) { - failureCause = ex; - throw new NestedServletException("Request processing failed", ex); - } + failureCause = ex; + throw new NestedServletException("Request processing failed", ex); } finally { @@ -1087,24 +1069,6 @@ 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 5077764fd9..1c5c90be22 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,7 +281,8 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe inputMessage, outputMessage); if (body != null) { if (logger.isDebugEnabled()) { - logger.debug("Writing [" + formatValue(body) + "]"); + Object formatted = (body instanceof CharSequence ? "\"" + body + "\"" : body); + logger.debug("Writing [" + formatted + "]"); } addContentDispositionHeader(inputMessage, outputMessage); if (genericConverter != null) { @@ -398,10 +399,6 @@ 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 84da9dcdd1..fddcb6eaa5 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 @@ -885,8 +885,8 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter mavContainer = (ModelAndViewContainer) asyncManager.getConcurrentResultContext()[0]; asyncManager.clearConcurrentResult(); if (logger.isDebugEnabled()) { - String formatted = AbstractMessageConverterMethodProcessor.formatValue(result); - logger.debug("Resume with async result [" + formatted + "]"); + logger.debug("Resume with async result [" + + (result instanceof CharSequence ? "\"" + result + "\"" : result) + "]"); } 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 7eff968f68..ea9b7fd74a 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-2018 the original author or authors. + * Copyright 2002-2017 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,32 +614,6 @@ 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");