Skip to content

Commit 4989843

Browse files
committed
Revert "Add handleFailure property to FrameworkServlet"
This reverts commit 29ce668.
1 parent 0dd9e8c commit 4989843

File tree

4 files changed

+9
-74
lines changed

4 files changed

+9
-74
lines changed

spring-webmvc/src/main/java/org/springframework/web/servlet/FrameworkServlet.java

Lines changed: 4 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,6 @@ public abstract class FrameworkServlet extends HttpServletBean implements Applic
209209
/** Should we dispatch an HTTP TRACE request to {@link #doService}?. */
210210
private boolean dispatchTraceRequest = false;
211211

212-
/** Should we set the status to 500 for unhandled failures? */
213-
private boolean shouldHandleFailure = false;
214-
215212
/** WebApplicationContext for this servlet. */
216213
@Nullable
217214
private WebApplicationContext webApplicationContext;
@@ -475,17 +472,6 @@ public void setDispatchTraceRequest(boolean dispatchTraceRequest) {
475472
this.dispatchTraceRequest = dispatchTraceRequest;
476473
}
477474

478-
/**
479-
* Whether to handle failures wth {@code response.sendError(500)} as opposed
480-
* to letting them propagate to the container which may log a stacktrace
481-
* even if an error dispatch (e.g. Spring Boot app) handles the exception.
482-
* @param shouldHandleFailure whether to handle failures or propagate
483-
* @since 5.1
484-
*/
485-
public void setShouldHandleFailure(boolean shouldHandleFailure) {
486-
this.shouldHandleFailure = shouldHandleFailure;
487-
}
488-
489475
/**
490476
* Whether to log request params at DEBUG level, and headers at TRACE level.
491477
* Both may contain sensitive information.
@@ -1012,16 +998,12 @@ protected final void processRequest(HttpServletRequest request, HttpServletRespo
1012998
doService(request, response);
1013999
}
10141000
catch (ServletException | IOException ex) {
1015-
if (!handleFailure(request, response, ex)) {
1016-
failureCause = ex;
1017-
throw ex;
1018-
}
1001+
failureCause = ex;
1002+
throw ex;
10191003
}
10201004
catch (Throwable ex) {
1021-
if (!handleFailure(request, response, ex)) {
1022-
failureCause = ex;
1023-
throw new NestedServletException("Request processing failed", ex);
1024-
}
1005+
failureCause = ex;
1006+
throw new NestedServletException("Request processing failed", ex);
10251007
}
10261008

10271009
finally {
@@ -1087,24 +1069,6 @@ private void resetContextHolders(HttpServletRequest request,
10871069
RequestContextHolder.setRequestAttributes(previousAttributes, this.threadContextInheritable);
10881070
}
10891071

1090-
private boolean handleFailure(HttpServletRequest request, HttpServletResponse response, Throwable ex) {
1091-
if (this.shouldHandleFailure) {
1092-
try {
1093-
response.sendError(500);
1094-
}
1095-
catch (IOException ex2) {
1096-
if (logger.isDebugEnabled()) {
1097-
logger.debug("Handling of failure failed: " + ex2);
1098-
}
1099-
return false;
1100-
}
1101-
request.setAttribute(WebUtils.ERROR_STATUS_CODE_ATTRIBUTE, 500);
1102-
WebUtils.exposeErrorRequestAttributes(request, ex, getServletName());
1103-
return true;
1104-
}
1105-
return false;
1106-
}
1107-
11081072
private void logResult(HttpServletRequest request, HttpServletResponse response,
11091073
@Nullable Throwable failureCause, WebAsyncManager asyncManager) {
11101074

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,8 @@ else if (mediaType.equals(MediaType.ALL) || mediaType.equals(MEDIA_TYPE_APPLICAT
281281
inputMessage, outputMessage);
282282
if (body != null) {
283283
if (logger.isDebugEnabled()) {
284-
logger.debug("Writing [" + formatValue(body) + "]");
284+
Object formatted = (body instanceof CharSequence ? "\"" + body + "\"" : body);
285+
logger.debug("Writing [" + formatted + "]");
285286
}
286287
addContentDispositionHeader(inputMessage, outputMessage);
287288
if (genericConverter != null) {
@@ -398,10 +399,6 @@ private MediaType getMostSpecificMediaType(MediaType acceptType, MediaType produ
398399
return (MediaType.SPECIFICITY_COMPARATOR.compare(acceptType, produceTypeToUse) <= 0 ? acceptType : produceTypeToUse);
399400
}
400401

401-
static String formatValue(Object body) {
402-
return (body instanceof CharSequence ? "\"" + body + "\"" : body.toString());
403-
}
404-
405402
/**
406403
* Check if the path has a file extension and whether the extension is
407404
* either {@link #WHITELISTED_EXTENSIONS whitelisted} or explicitly

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -885,8 +885,8 @@ protected ModelAndView invokeHandlerMethod(HttpServletRequest request,
885885
mavContainer = (ModelAndViewContainer) asyncManager.getConcurrentResultContext()[0];
886886
asyncManager.clearConcurrentResult();
887887
if (logger.isDebugEnabled()) {
888-
String formatted = AbstractMessageConverterMethodProcessor.formatValue(result);
889-
logger.debug("Resume with async result [" + formatted + "]");
888+
logger.debug("Resume with async result [" +
889+
(result instanceof CharSequence ? "\"" + result + "\"" : result) + "]");
890890
}
891891
invocableMethod = invocableMethod.wrapConcurrentResult(result);
892892
}

spring-webmvc/src/test/java/org/springframework/web/servlet/DispatcherServletTests.java

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -614,32 +614,6 @@ public void noHandlerFoundExceptionMessage() {
614614
assertTrue(!ex.toString().contains("bar"));
615615
}
616616

617-
@Test // SPR-17100
618-
public void shouldHandleFailure() throws ServletException, IOException {
619-
620-
IllegalStateException ex = new IllegalStateException("dummy");
621-
@SuppressWarnings("serial")
622-
FrameworkServlet servlet = new FrameworkServlet() {
623-
@Override
624-
protected void doService(HttpServletRequest request, HttpServletResponse response) {
625-
throw ex;
626-
}
627-
};
628-
servlet.setShouldHandleFailure(true);
629-
630-
MockHttpServletRequest request = new MockHttpServletRequest(getServletContext(), "GET", "/error");
631-
MockHttpServletResponse response = new MockHttpServletResponse();
632-
633-
servlet.service(request, response);
634-
635-
assertEquals(500, response.getStatus());
636-
assertEquals(500, request.getAttribute(WebUtils.ERROR_STATUS_CODE_ATTRIBUTE));
637-
assertEquals(ex.getClass(), request.getAttribute(WebUtils.ERROR_EXCEPTION_TYPE_ATTRIBUTE));
638-
assertEquals(ex.getMessage(), request.getAttribute(WebUtils.ERROR_MESSAGE_ATTRIBUTE));
639-
assertEquals(ex, request.getAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE));
640-
assertEquals(request.getRequestURI(), request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE));
641-
}
642-
643617
@Test
644618
public void cleanupAfterIncludeWithRemove() throws ServletException, IOException {
645619
MockHttpServletRequest request = new MockHttpServletRequest(getServletContext(), "GET", "/main.do");

0 commit comments

Comments
 (0)