Commit ac88a600 authored by Andy Wilkinson's avatar Andy Wilkinson

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
parent 1805cc56
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
package org.springframework.boot.web.support; package org.springframework.boot.web.support;
import java.io.IOException; import java.io.IOException;
import java.io.PrintWriter;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
...@@ -24,6 +25,7 @@ import javax.servlet.Filter; ...@@ -24,6 +25,7 @@ import javax.servlet.Filter;
import javax.servlet.FilterChain; import javax.servlet.FilterChain;
import javax.servlet.FilterConfig; import javax.servlet.FilterConfig;
import javax.servlet.ServletException; import javax.servlet.ServletException;
import javax.servlet.ServletOutputStream;
import javax.servlet.ServletRequest; import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse; import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
...@@ -311,11 +313,15 @@ public class ErrorPageFilter implements Filter, ErrorPageRegistry { ...@@ -311,11 +313,15 @@ public class ErrorPageFilter implements Filter, ErrorPageRegistry {
@Override @Override
public void flushBuffer() throws IOException { public void flushBuffer() throws IOException {
sendErrorIfNecessary();
super.flushBuffer();
}
private void sendErrorIfNecessary() throws IOException {
if (this.hasErrorToSend && !isCommitted()) { if (this.hasErrorToSend && !isCommitted()) {
((HttpServletResponse) getResponse()).sendError(this.status, ((HttpServletResponse) getResponse()).sendError(this.status,
this.message); this.message);
} }
super.flushBuffer();
} }
public String getMessage() { public String getMessage() {
...@@ -326,6 +332,19 @@ public class ErrorPageFilter implements Filter, ErrorPageRegistry { ...@@ -326,6 +332,19 @@ public class ErrorPageFilter implements Filter, ErrorPageRegistry {
return this.hasErrorToSend; return this.hasErrorToSend;
} }
@Override
public PrintWriter getWriter() throws IOException {
sendErrorIfNecessary();
return super.getWriter();
}
@Override
public ServletOutputStream getOutputStream() throws IOException {
sendErrorIfNecessary();
return super.getOutputStream();
}
} }
} }
/* /*
* 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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
...@@ -521,6 +521,22 @@ public class ErrorPageFilterTests { ...@@ -521,6 +521,22 @@ public class ErrorPageFilterTests {
assertThat(this.response.getForwardedUrl()).isEqualTo("/500"); 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 { private void setUpAsyncDispatch() throws Exception {
this.request.setAsyncSupported(true); this.request.setAsyncSupported(true);
this.request.setAsyncStarted(true); this.request.setAsyncStarted(true);
......
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