From 41cb567894c961b7f1650947bf6f979f6c46759e Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 18 Nov 2014 09:45:21 +0000 Subject: [PATCH] Don't flush in ErrorPageFilter for < 400 response that's committed Previously, for a non-async response with a successful status (< 400), ErrorPageFilter would always call flushBuffer. This triggers an exception in Tomcat if the client has closed the connection before the response has been fully sent. In this case, Tomcat treats the response as successful and commits it before control returns to the filter. This commit updates ErrorPageFilter to only perform the flush if the response has not already been committed, leaving any further flushing that may be necessary to be handled by the servlet container. Fixes gh-1938 --- .../boot/context/web/ErrorPageFilter.java | 4 ++-- .../boot/context/web/ErrorPageFilterTests.java | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java b/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java index ab1e253309..a4340ac752 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java @@ -112,7 +112,7 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple handleErrorStatus(request, response, status, wrapped.getMessage()); response.flushBuffer(); } - else if (!request.isAsyncStarted()) { + else if (!request.isAsyncStarted() && !response.isCommitted()) { response.flushBuffer(); } } @@ -184,7 +184,7 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple + request.getRequestURI() + " as the response has already been" + " committed. As a result, the response may have the wrong status" + " code. If your application is running on WebSphere Application" - + " Server you may be able to resolve this problem by setting " + + " Server you may be able to resolve this problem by setting" + " com.ibm.ws.webcontainer.invokeFlushAfterService to false"; if (ex == null) { logger.error(message); diff --git a/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java b/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java index c6dee4244d..fe73f0b146 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java @@ -40,6 +40,10 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; /** * Tests for {@link ErrorPageFilter}. @@ -343,4 +347,16 @@ public class ErrorPageFilterTests { assertTrue(this.response.isCommitted()); } + @Test + public void responseIsNotFlushedIfStatusIsLessThan400AndItHasAlreadyBeenCommitted() + throws Exception { + HttpServletResponse committedResponse = mock(HttpServletResponse.class); + given(committedResponse.isCommitted()).willReturn(true); + given(committedResponse.getStatus()).willReturn(200); + + this.filter.doFilter(this.request, committedResponse, this.chain); + + verify(committedResponse, times(0)).flushBuffer(); + } + }