Skip to content

Commit 6d2bb68

Browse files
committed
Ensure that ErrorPageFilter handles errors during async dispatch
Previously, the ErrorPageFilter was not invoked for async dispatches. This meant that an error that was set during an async dispatch would go undetected and a 200 response with an empty body would be returned. This commit updates ErrorPageFilter to configure its OncePerRequestFilter so that async dispatches are filtered and the correct error handling is performed. Closes gh-2711
1 parent c3a040a commit 6d2bb68

File tree

2 files changed

+75
-0
lines changed

2 files changed

+75
-0
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ protected void doFilterInternal(HttpServletRequest request,
8888
ErrorPageFilter.this.doFilter(request, response, chain);
8989
}
9090

91+
@Override
92+
protected boolean shouldNotFilterAsyncDispatch() {
93+
return false;
94+
}
95+
9196
};
9297

9398
@Override

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@
3434
import org.springframework.mock.web.MockFilterConfig;
3535
import org.springframework.mock.web.MockHttpServletRequest;
3636
import org.springframework.mock.web.MockHttpServletResponse;
37+
import org.springframework.web.context.request.async.DeferredResult;
38+
import org.springframework.web.context.request.async.StandardServletAsyncWebRequest;
39+
import org.springframework.web.context.request.async.WebAsyncManager;
40+
import org.springframework.web.context.request.async.WebAsyncUtils;
3741

3842
import static org.hamcrest.Matchers.containsString;
3943
import static org.hamcrest.Matchers.equalTo;
@@ -373,6 +377,62 @@ public void doFilter(ServletRequest request, ServletResponse response)
373377
assertTrue(this.response.isCommitted());
374378
}
375379

380+
@Test
381+
public void responseIsNotCommitedDuringAsyncDispatch() throws Exception {
382+
setUpAsyncDispatch();
383+
384+
this.filter.doFilter(this.request, this.response, this.chain);
385+
386+
assertThat(this.chain.getRequest(), equalTo((ServletRequest) this.request));
387+
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getResponse(),
388+
equalTo((ServletResponse) this.response));
389+
assertFalse(this.response.isCommitted());
390+
}
391+
392+
@Test
393+
public void responseIsCommitedWhenExceptionIsThrownDuringAsyncDispatch()
394+
throws Exception {
395+
this.filter.addErrorPages(new ErrorPage("/error"));
396+
setUpAsyncDispatch();
397+
this.chain = new MockFilterChain() {
398+
@Override
399+
public void doFilter(ServletRequest request, ServletResponse response)
400+
throws IOException, ServletException {
401+
super.doFilter(request, response);
402+
throw new RuntimeException("BAD");
403+
}
404+
};
405+
406+
this.filter.doFilter(this.request, this.response, this.chain);
407+
408+
assertThat(this.chain.getRequest(), equalTo((ServletRequest) this.request));
409+
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getResponse(),
410+
equalTo((ServletResponse) this.response));
411+
assertTrue(this.response.isCommitted());
412+
}
413+
414+
@Test
415+
public void responseIsCommitedWhenStatusIs400PlusDuringAsyncDispatch()
416+
throws Exception {
417+
this.filter.addErrorPages(new ErrorPage("/error"));
418+
setUpAsyncDispatch();
419+
this.chain = new MockFilterChain() {
420+
@Override
421+
public void doFilter(ServletRequest request, ServletResponse response)
422+
throws IOException, ServletException {
423+
super.doFilter(request, response);
424+
((HttpServletResponse) response).sendError(400, "BAD");
425+
}
426+
};
427+
428+
this.filter.doFilter(this.request, this.response, this.chain);
429+
430+
assertThat(this.chain.getRequest(), equalTo((ServletRequest) this.request));
431+
assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getResponse(),
432+
equalTo((ServletResponse) this.response));
433+
assertTrue(this.response.isCommitted());
434+
}
435+
376436
@Test
377437
public void responseIsNotFlushedIfStatusIsLessThan400AndItHasAlreadyBeenCommitted()
378438
throws Exception {
@@ -419,4 +479,14 @@ public void doFilter(ServletRequest request, ServletResponse response)
419479
assertThat(this.output.toString(), containsString("request [/test/alpha]"));
420480
}
421481

482+
private void setUpAsyncDispatch() throws Exception {
483+
this.request.setAsyncSupported(true);
484+
this.request.setAsyncStarted(true);
485+
DeferredResult<String> result = new DeferredResult<String>();
486+
WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(this.request);
487+
asyncManager.setAsyncWebRequest(new StandardServletAsyncWebRequest(this.request,
488+
this.response));
489+
asyncManager.startDeferredResultProcessing(result);
490+
}
491+
422492
}

0 commit comments

Comments
 (0)