From ac88a60071a063456da9c868e7745eb6709bc1e5 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 4 Apr 2018 13:38:19 +0100 Subject: [PATCH] Ensure error is sent before Writer or OutputStream is used Previously, ErrorPageFilter's ErrorResponseWrapper would delaying sending an error back to the client. In cases where the response's Writer or OutputStream was accessed and flushed or closed, this could lead to the wrong response status being sent. This commit updates ErrorResponseWrapper so that it will send any capture error to the client before returning the response's Writer or OutputStream. This ensures that closing the Writer or OutputStream does not cause the response to be committed with the default response status rather than the previously captured error status. Such responses will now include the correct status, but will not be forwarded to the error controller. Such forwarding is not possible due to the response already having been committed. Closes gh-11814 --- .../boot/web/support/ErrorPageFilter.java | 21 ++++++++++++++++++- .../web/support/ErrorPageFilterTests.java | 18 +++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/web/support/ErrorPageFilter.java b/spring-boot/src/main/java/org/springframework/boot/web/support/ErrorPageFilter.java index 5eb14c3a46d..44663914017 100644 --- a/spring-boot/src/main/java/org/springframework/boot/web/support/ErrorPageFilter.java +++ b/spring-boot/src/main/java/org/springframework/boot/web/support/ErrorPageFilter.java @@ -17,6 +17,7 @@ package org.springframework.boot.web.support; import java.io.IOException; +import java.io.PrintWriter; import java.util.HashMap; import java.util.Map; @@ -24,6 +25,7 @@ import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; @@ -311,11 +313,15 @@ public class ErrorPageFilter implements Filter, ErrorPageRegistry { @Override public void flushBuffer() throws IOException { + sendErrorIfNecessary(); + super.flushBuffer(); + } + + private void sendErrorIfNecessary() throws IOException { if (this.hasErrorToSend && !isCommitted()) { ((HttpServletResponse) getResponse()).sendError(this.status, this.message); } - super.flushBuffer(); } public String getMessage() { @@ -326,6 +332,19 @@ public class ErrorPageFilter implements Filter, ErrorPageRegistry { return this.hasErrorToSend; } + @Override + public PrintWriter getWriter() throws IOException { + sendErrorIfNecessary(); + return super.getWriter(); + + } + + @Override + public ServletOutputStream getOutputStream() throws IOException { + sendErrorIfNecessary(); + return super.getOutputStream(); + } + } } diff --git a/spring-boot/src/test/java/org/springframework/boot/web/support/ErrorPageFilterTests.java b/spring-boot/src/test/java/org/springframework/boot/web/support/ErrorPageFilterTests.java index 53182442d70..ed691645e32 100644 --- a/spring-boot/src/test/java/org/springframework/boot/web/support/ErrorPageFilterTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/web/support/ErrorPageFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -521,6 +521,22 @@ public class ErrorPageFilterTests { assertThat(this.response.getForwardedUrl()).isEqualTo("/500"); } + @Test + public void whenErrorIsSentAndWriterIsFlushedErrorIsSentToTheClient() + throws Exception { + this.chain = new MockFilterChain() { + @Override + public void doFilter(ServletRequest request, ServletResponse response) + throws IOException, ServletException { + ((HttpServletResponse) response).sendError(400); + response.getWriter().flush(); + super.doFilter(request, response); + } + }; + this.filter.doFilter(this.request, this.response, this.chain); + assertThat(this.response.getStatus()).isEqualTo(400); + } + private void setUpAsyncDispatch() throws Exception { this.request.setAsyncSupported(true); this.request.setAsyncStarted(true);