Skip to content

Commit 00b7649

Browse files
committed
Remove error logging on ClientAbortException
Update `ErrorPageFilter` so that a Tomcat `ClientAbortException` no longer causes "Cannot forward to error page for request" logging for committed responses. Since a `ClientAbortException` indicates that the client is no longer available it's of no consequence that the request has been committed and the forward will fail. Closes gh-13221
1 parent 43071b9 commit 00b7649

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

spring-boot/src/main/java/org/springframework/boot/web/support/ErrorPageFilter.java

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,12 @@
1818

1919
import java.io.IOException;
2020
import java.io.PrintWriter;
21+
import java.util.Collection;
22+
import java.util.Collections;
2123
import java.util.HashMap;
24+
import java.util.HashSet;
2225
import java.util.Map;
26+
import java.util.Set;
2327

2428
import javax.servlet.Filter;
2529
import javax.servlet.FilterChain;
@@ -40,6 +44,7 @@
4044
import org.springframework.boot.web.servlet.ErrorPageRegistry;
4145
import org.springframework.core.Ordered;
4246
import org.springframework.core.annotation.Order;
47+
import org.springframework.util.ClassUtils;
4348
import org.springframework.web.filter.OncePerRequestFilter;
4449
import org.springframework.web.util.NestedServletException;
4550

@@ -77,6 +82,14 @@ public class ErrorPageFilter implements Filter, ErrorPageRegistry {
7782

7883
private static final String ERROR_STATUS_CODE = "javax.servlet.error.status_code";
7984

85+
private static final Set<Class<?>> CLIENT_ABORT_EXCEPTIONS;
86+
static {
87+
Set<Class<?>> clientAbortExceptions = new HashSet<>();
88+
addClassIfPresent(clientAbortExceptions,
89+
"org.apache.catalina.connector.ClientAbortException");
90+
CLIENT_ABORT_EXCEPTIONS = Collections.unmodifiableSet(clientAbortExceptions);
91+
}
92+
8093
private String global;
8194

8295
private final Map<Integer, String> statuses = new HashMap<Integer, String>();
@@ -164,7 +177,6 @@ private void handleException(HttpServletRequest request, HttpServletResponse res
164177
handleCommittedResponse(request, ex);
165178
return;
166179
}
167-
168180
forwardToErrorPage(errorPath, request, wrapped, ex);
169181
}
170182

@@ -200,6 +212,9 @@ protected String getDescription(HttpServletRequest request) {
200212
}
201213

202214
private void handleCommittedResponse(HttpServletRequest request, Throwable ex) {
215+
if (isClientAbortException(ex)) {
216+
return;
217+
}
203218
String message = "Cannot forward to error page for request "
204219
+ getDescription(request) + " as the response has already been"
205220
+ " committed. As a result, the response may have the wrong status"
@@ -216,6 +231,18 @@ private void handleCommittedResponse(HttpServletRequest request, Throwable ex) {
216231
}
217232
}
218233

234+
private boolean isClientAbortException(Throwable ex) {
235+
if (ex == null) {
236+
return false;
237+
}
238+
for (Class<?> candidate : CLIENT_ABORT_EXCEPTIONS) {
239+
if (candidate.isInstance(ex)) {
240+
return true;
241+
}
242+
}
243+
return isClientAbortException(ex.getCause());
244+
}
245+
219246
private String getErrorPath(Map<Integer, String> map, Integer status) {
220247
if (map.containsKey(status)) {
221248
return map.get(status);
@@ -276,6 +303,15 @@ else if (errorPage.getStatus() != null) {
276303
public void destroy() {
277304
}
278305

306+
private static void addClassIfPresent(Collection<Class<?>> collection,
307+
String className) {
308+
try {
309+
collection.add(ClassUtils.forName(className, null));
310+
}
311+
catch (Throwable ex) {
312+
}
313+
}
314+
279315
private static class ErrorWrapperResponse extends HttpServletResponseWrapper {
280316

281317
private int status;

spring-boot/src/test/java/org/springframework/boot/web/support/ErrorPageFilterTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import javax.servlet.http.HttpServletResponse;
2929
import javax.servlet.http.HttpServletResponseWrapper;
3030

31+
import org.apache.catalina.connector.ClientAbortException;
3132
import org.junit.Rule;
3233
import org.junit.Test;
3334

@@ -150,6 +151,25 @@ public void doFilter(ServletRequest request, ServletResponse response)
150151
assertThat(this.response.isCommitted()).isTrue();
151152
}
152153

154+
@Test
155+
public void responseCommittedWhenFromClientAbortException() throws Exception {
156+
this.filter.addErrorPages(new ErrorPage("/error"));
157+
this.response.setCommitted(true);
158+
this.chain = new MockFilterChain() {
159+
160+
@Override
161+
public void doFilter(ServletRequest request, ServletResponse response)
162+
throws IOException, ServletException {
163+
super.doFilter(request, response);
164+
throw new ClientAbortException();
165+
}
166+
167+
};
168+
this.filter.doFilter(this.request, this.response, this.chain);
169+
assertThat(this.response.isCommitted()).isTrue();
170+
assertThat(this.output.toString()).doesNotContain("Cannot forward");
171+
}
172+
153173
@Test
154174
public void responseUncommittedWithoutErrorPage() throws Exception {
155175
this.chain = new MockFilterChain() {

0 commit comments

Comments
 (0)