Skip to content

Commit e0b54e2

Browse files
bclozelbclozel
committed
Ignore flushes on ServletServerHttpResponse output stream
Prior to this commit, flush calls on the output stream returned by `ServletServerHttpResponse#getBody` would be delegated to the Servlet response output stream. This can cause performance issues when `HttpMessageConverter` and other web components write and flush multiple times to the response body. Here, the Servlet container is in a better position to flush to the network at the optimal time and buffer the response body until then. This is particularly true for `HttpMessageConverters` when they flush many times the output stream, sometimes due to the underlying codec library. Instead of revisiting the entire message converter contract, we are here ignoring flush calls to that output stream. This change does not affect the client side, nor the `ServletServerHttpResponse#flush` calls. This commit also introduces a new Spring property `"spring.http.response.flush.enabled"` that reverts this behavior change if necessary. Closes gh-36385
1 parent f2b64ad commit e0b54e2

5 files changed

Lines changed: 74 additions & 9 deletions

File tree

‎framework-docs/modules/ROOT/pages/appendix.adoc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ resolvable otherwise. See
8181
{spring-framework-api}++/core/env/AbstractEnvironment.html#IGNORE_GETENV_PROPERTY_NAME++[`AbstractEnvironment`]
8282
for details.
8383

84+
| `spring.http.response.flush.enabled`
85+
| Configures the Spring MVC `ServletServerHttpResponse` to allow flushing on the `OutputStream`
86+
returned by `ServletServerHttpResponse#getBody()`. By default, such flush calls are ignored and
87+
only `ServletServerHttpResponse#flush()` will actually flush the response to the network.
88+
8489
| `spring.jdbc.getParameterType.ignore`
8590
| Instructs Spring to ignore `java.sql.ParameterMetaData.getParameterType` completely.
8691
See the note in xref:data-access/jdbc/advanced.adoc#jdbc-batch-list[Batch Operations with a List of Objects].

‎spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,35 @@
2323
import jakarta.servlet.http.HttpServletResponse;
2424
import org.jspecify.annotations.Nullable;
2525

26+
import org.springframework.core.SpringProperties;
2627
import org.springframework.http.HttpHeaders;
2728
import org.springframework.http.HttpStatusCode;
2829
import org.springframework.http.MediaType;
2930
import org.springframework.util.Assert;
31+
import org.springframework.util.StreamUtils;
3032

3133
/**
3234
* {@link ServerHttpResponse} implementation that is based on a {@link HttpServletResponse}.
3335
*
3436
* @author Arjen Poutsma
3537
* @author Rossen Stoyanchev
38+
* @author Brian Clozel
3639
* @since 3.0
3740
*/
3841
public class ServletServerHttpResponse implements ServerHttpResponse {
3942

43+
/**
44+
* System property that indicates whether {@code response.getBody().flush()}
45+
* should effectively flush to the network. This is by default disabled,
46+
* and developers must set the {@code "spring.http.response.flush.enabled"}
47+
* {@link org.springframework.core.SpringProperties Spring property} to
48+
* turn it on.
49+
* <p>Applications should instead {@link #flush()} on the response directly.
50+
*/
51+
public static final String BODY_FLUSH_ENABLED = "spring.http.response.flush.enabled";
52+
53+
private final boolean flushEnabled = SpringProperties.getFlag(BODY_FLUSH_ENABLED);
54+
4055
private final HttpServletResponse servletResponse;
4156

4257
private final HttpHeaders headers;
@@ -86,11 +101,20 @@ else if (this.headersWritten) {
86101
}
87102
}
88103

104+
/**
105+
* Return the body of the message as an output stream.
106+
* <p>By default, flushing the output stream has no effect
107+
* (see {@link #BODY_FLUSH_ENABLED}) and should be performed
108+
* using {@link #flush()} instead.
109+
* @return the output stream body (never {@code null})
110+
* @throws IOException in case of I/O errors
111+
*/
89112
@Override
90113
public OutputStream getBody() throws IOException {
91114
this.bodyUsed = true;
92115
writeHeaders();
93-
return this.servletResponse.getOutputStream();
116+
return (this.flushEnabled) ? this.servletResponse.getOutputStream() :
117+
StreamUtils.nonFlushing(this.servletResponse.getOutputStream());
94118
}
95119

96120
@Override

‎spring-web/src/test/java/org/springframework/http/server/ServletServerHttpResponseTests.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,26 @@
1919
import java.nio.charset.StandardCharsets;
2020
import java.util.List;
2121

22+
import jakarta.servlet.ServletOutputStream;
23+
import jakarta.servlet.http.HttpServletResponse;
2224
import org.junit.jupiter.api.BeforeEach;
2325
import org.junit.jupiter.api.Test;
2426

27+
import org.springframework.core.SpringProperties;
2528
import org.springframework.http.HttpHeaders;
2629
import org.springframework.http.HttpStatus;
2730
import org.springframework.http.MediaType;
2831
import org.springframework.util.FileCopyUtils;
2932
import org.springframework.web.testfixture.servlet.MockHttpServletResponse;
3033

3134
import static org.assertj.core.api.Assertions.assertThat;
35+
import static org.mockito.Mockito.mock;
36+
import static org.mockito.Mockito.never;
37+
import static org.mockito.Mockito.verify;
38+
import static org.mockito.Mockito.when;
3239

3340
/**
41+
* Tests for {@link ServletServerHttpResponse}.
3442
* @author Arjen Poutsma
3543
* @author Rossen Stoyanchev
3644
* @author Juergen Hoeller
@@ -120,4 +128,33 @@ void getBody() throws Exception {
120128
assertThat(mockResponse.getContentAsByteArray()).as("Invalid content written").isEqualTo(content);
121129
}
122130

131+
@Test
132+
void skipFlushCallsOnOutputStream() throws Exception {
133+
ServletOutputStream mockStream = mock();
134+
HttpServletResponse mockResponse = mock();
135+
when(mockResponse.getOutputStream()).thenReturn(mockStream);
136+
137+
this.response = new ServletServerHttpResponse(mockResponse);
138+
byte[] content = "Hello World".getBytes(StandardCharsets.UTF_8);
139+
FileCopyUtils.copy(content, response.getBody());
140+
response.getBody().flush();
141+
verify(mockStream, never()).flush();
142+
}
143+
144+
@Test
145+
void appliesFlushCallsOnOutputStream() throws Exception {
146+
SpringProperties.setProperty(ServletServerHttpResponse.BODY_FLUSH_ENABLED, Boolean.TRUE.toString());
147+
ServletOutputStream mockStream = mock();
148+
HttpServletResponse mockResponse = mock();
149+
when(mockResponse.getOutputStream()).thenReturn(mockStream);
150+
151+
this.response = new ServletServerHttpResponse(mockResponse);
152+
byte[] content = "Hello World".getBytes(StandardCharsets.UTF_8);
153+
FileCopyUtils.copy(content, response.getBody());
154+
response.getBody().flush();
155+
verify(mockStream).flush();
156+
157+
SpringProperties.setProperty(ServletServerHttpResponse.BODY_FLUSH_ENABLED, null);
158+
}
159+
123160
}

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpHeadersReturnValueHandler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public void handleReturnValue(@Nullable Object returnValue, MethodParameter retu
5555
Assert.state(servletResponse != null, "No HttpServletResponse");
5656
ServletServerHttpResponse outputMessage = new ServletServerHttpResponse(servletResponse);
5757
outputMessage.getHeaders().putAll(headers);
58-
outputMessage.getBody(); // flush headers
58+
outputMessage.flush(); // flush headers
5959
}
6060
}
6161

‎spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/StreamingResponseBodyReturnValueHandler.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.web.servlet.mvc.method.annotation;
1818

19-
import java.io.OutputStream;
2019
import java.util.concurrent.Callable;
2120

2221
import jakarta.servlet.ServletRequest;
@@ -89,26 +88,26 @@ public void handleReturnValue(@Nullable Object returnValue, MethodParameter retu
8988
Assert.isInstanceOf(StreamingResponseBody.class, returnValue, "StreamingResponseBody expected");
9089
StreamingResponseBody streamingBody = (StreamingResponseBody) returnValue;
9190

92-
Callable<Void> callable = new StreamingResponseBodyTask(outputMessage.getBody(), streamingBody);
91+
Callable<Void> callable = new StreamingResponseBodyTask(outputMessage, streamingBody);
9392
WebAsyncUtils.getAsyncManager(webRequest).startCallableProcessing(callable, mavContainer);
9493
}
9594

9695

9796
private static class StreamingResponseBodyTask implements Callable<Void> {
9897

99-
private final OutputStream outputStream;
98+
private final ServerHttpResponse outputMessage;
10099

101100
private final StreamingResponseBody streamingBody;
102101

103-
public StreamingResponseBodyTask(OutputStream outputStream, StreamingResponseBody streamingBody) {
104-
this.outputStream = outputStream;
102+
public StreamingResponseBodyTask(ServerHttpResponse outputMessage, StreamingResponseBody streamingBody) {
103+
this.outputMessage = outputMessage;
105104
this.streamingBody = streamingBody;
106105
}
107106

108107
@Override
109108
public Void call() throws Exception {
110-
this.streamingBody.writeTo(this.outputStream);
111-
this.outputStream.flush();
109+
this.streamingBody.writeTo(this.outputMessage.getBody());
110+
this.outputMessage.flush();
112111
return null;
113112
}
114113
}

0 commit comments

Comments
 (0)