Commit 41cb5678 authored by Andy Wilkinson's avatar Andy Wilkinson

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
parent f224c7ac
......@@ -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);
......
......@@ -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();
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment