Skip to content

Commit 18eb2a6

Browse files
bclozelbclozel
committed
Polishing contribution
Closes gh-35225
1 parent 4101714 commit 18eb2a6

File tree

5 files changed

+60
-54
lines changed

5 files changed

+60
-54
lines changed

‎spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest {
6767

6868
private static final Set<String> DISALLOWED_HEADERS = disallowedHeaders();
6969

70-
private static final List<String> ALLOWED_ENCODINGS = List.of("gzip", "deflate");
70+
private static final List<String> SUPPORTED_ENCODINGS = List.of("gzip", "deflate");
7171

7272

7373
private final HttpClient httpClient;
@@ -80,18 +80,18 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest {
8080

8181
private final @Nullable Duration timeout;
8282

83-
private final boolean compressionEnabled;
83+
private final boolean compression;
8484

8585

86-
public JdkClientHttpRequest(HttpClient httpClient, URI uri, HttpMethod method, Executor executor,
87-
@Nullable Duration readTimeout, boolean compressionEnabled) {
86+
JdkClientHttpRequest(HttpClient httpClient, URI uri, HttpMethod method, Executor executor,
87+
@Nullable Duration readTimeout, boolean compression) {
8888

8989
this.httpClient = httpClient;
9090
this.uri = uri;
9191
this.method = method;
9292
this.executor = executor;
9393
this.timeout = readTimeout;
94-
this.compressionEnabled = compressionEnabled;
94+
this.compression = compression;
9595
}
9696

9797

@@ -164,13 +164,10 @@ else if (cause instanceof IOException ioEx) {
164164
private HttpRequest buildRequest(HttpHeaders headers, @Nullable Body body) {
165165
HttpRequest.Builder builder = HttpRequest.newBuilder().uri(this.uri);
166166

167-
// When compression is enabled and valid encoding is absent, we add gzip as standard encoding
168-
if (this.compressionEnabled) {
169-
if (headers.containsHeader(HttpHeaders.ACCEPT_ENCODING) &&
170-
!ALLOWED_ENCODINGS.contains(headers.getFirst(HttpHeaders.ACCEPT_ENCODING))) {
171-
headers.remove(HttpHeaders.ACCEPT_ENCODING);
167+
if (this.compression) {
168+
if (!headers.containsHeader(HttpHeaders.ACCEPT_ENCODING)) {
169+
headers.addAll(HttpHeaders.ACCEPT_ENCODING, SUPPORTED_ENCODINGS);
172170
}
173-
headers.add(HttpHeaders.ACCEPT_ENCODING, "gzip");
174171
}
175172

176173
headers.forEach((headerName, headerValues) -> {
@@ -310,35 +307,32 @@ public void handleCancellationException(CancellationException ex) throws HttpTim
310307
}
311308

312309
/**
313-
* Custom BodyHandler that checks the Content-Encoding header and applies the appropriate decompression algorithm.
310+
* BodyHandler that checks the Content-Encoding header and applies the appropriate decompression algorithm.
314311
* Supports Gzip and Deflate encoded responses.
315312
*/
316-
public static final class DecompressingBodyHandler implements BodyHandler<InputStream> {
313+
private static final class DecompressingBodyHandler implements BodyHandler<InputStream> {
317314

318315
@Override
319316
public BodySubscriber<InputStream> apply(ResponseInfo responseInfo) {
320317
String contentEncoding = responseInfo.headers().firstValue(HttpHeaders.CONTENT_ENCODING).orElse("");
321318
if (contentEncoding.equalsIgnoreCase("gzip")) {
322-
// If the content is gzipped, wrap the InputStream with a GZIPInputStream
323319
return BodySubscribers.mapping(
324320
BodySubscribers.ofInputStream(),
325321
(InputStream is) -> {
326322
try {
327323
return new GZIPInputStream(is);
328324
}
329325
catch (IOException ex) {
330-
throw new UncheckedIOException(ex); // Propagate IOExceptions
326+
throw new UncheckedIOException(ex);
331327
}
332328
});
333329
}
334330
else if (contentEncoding.equalsIgnoreCase("deflate")) {
335-
// If the content is encoded using deflate, wrap the InputStream with a InflaterInputStream
336331
return BodySubscribers.mapping(
337332
BodySubscribers.ofInputStream(),
338333
InflaterInputStream::new);
339334
}
340335
else {
341-
// Otherwise, return a standard InputStream BodySubscriber
342336
return BodySubscribers.ofInputStream();
343337
}
344338
}

‎spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public class JdkClientHttpRequestFactory implements ClientHttpRequestFactory {
4343

4444
private @Nullable Duration readTimeout;
4545

46-
private boolean compressionEnabled;
46+
private boolean compression = true;
4747

4848

4949
/**
@@ -99,17 +99,17 @@ public void setReadTimeout(Duration readTimeout) {
9999
}
100100

101101
/**
102-
* Sets custom {@link BodyHandler} that can handle gzip encoded {@link HttpClient}'s response.
103-
* @param compressionEnabled to enable compression by default for all {@link HttpClient}'s requests.
102+
* Set whether support for uncompressing "gzip" and "deflate" HTTP responses is enabled.
103+
* @since 7.0
104104
*/
105-
public void setCompressionEnabled(boolean compressionEnabled) {
106-
this.compressionEnabled = compressionEnabled;
105+
public void enableCompression(boolean enable) {
106+
this.compression = enable;
107107
}
108108

109109

110110
@Override
111111
public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException {
112-
return new JdkClientHttpRequest(this.httpClient, uri, httpMethod, this.executor, this.readTimeout, this.compressionEnabled);
112+
return new JdkClientHttpRequest(this.httpClient, uri, httpMethod, this.executor, this.readTimeout, this.compression);
113113
}
114114

115115
}

‎spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,21 @@
1616

1717
package org.springframework.http.client;
1818

19+
import java.io.ByteArrayOutputStream;
20+
import java.util.zip.DeflaterOutputStream;
21+
import java.util.zip.GZIPOutputStream;
22+
1923
import mockwebserver3.Dispatcher;
2024
import mockwebserver3.MockResponse;
2125
import mockwebserver3.MockWebServer;
2226
import mockwebserver3.RecordedRequest;
27+
import okio.Buffer;
2328
import org.junit.jupiter.api.AfterEach;
2429
import org.junit.jupiter.api.BeforeEach;
2530

2631
import org.springframework.http.HttpHeaders;
2732
import org.springframework.util.StringUtils;
2833

29-
import java.io.ByteArrayOutputStream;
30-
import java.nio.charset.StandardCharsets;
31-
import java.util.zip.DeflaterOutputStream;
32-
import java.util.zip.GZIPOutputStream;
33-
3434
import static org.assertj.core.api.Assertions.assertThat;
3535

3636
/**
@@ -112,25 +112,34 @@ else if(request.getTarget().startsWith("/header/")) {
112112
String headerName = request.getTarget().replace("/header/","");
113113
return new MockResponse.Builder().body(headerName + ":" + request.getHeaders().get(headerName)).code(200).build();
114114
}
115-
else if(request.getTarget().startsWith("/compress/")) {
115+
else if(request.getTarget().startsWith("/compress/") && request.getBody() != null) {
116116
String encoding = request.getTarget().replace("/compress/","");
117-
ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
117+
String requestBody = request.getBody().utf8();
118+
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
118119
if (encoding.equals("gzip")) {
119-
try(GZIPOutputStream gzipOutputStream = new GZIPOutputStream(byteArrayOutputStream)) {
120-
gzipOutputStream.write("Test Payload".getBytes());
120+
try(GZIPOutputStream gzipOutputStream = new GZIPOutputStream(outputStream)) {
121+
gzipOutputStream.write(requestBody.getBytes());
121122
gzipOutputStream.flush();
122123
}
123124
}
124125
else if(encoding.equals("deflate")) {
125-
try(DeflaterOutputStream deflaterOutputStream = new DeflaterOutputStream(byteArrayOutputStream)) {
126-
deflaterOutputStream.write("Test Payload".getBytes());
126+
try(DeflaterOutputStream deflaterOutputStream = new DeflaterOutputStream(outputStream)) {
127+
deflaterOutputStream.write(requestBody.getBytes());
127128
deflaterOutputStream.flush();
128129
}
129-
} else {
130-
byteArrayOutputStream.write("Test Payload".getBytes());
131130
}
132-
return new MockResponse.Builder().body(byteArrayOutputStream.toString(StandardCharsets.ISO_8859_1))
133-
.code(200).setHeader(HttpHeaders.CONTENT_ENCODING, encoding).build();
131+
else {
132+
outputStream.write(requestBody.getBytes());
133+
}
134+
Buffer buffer = new Buffer();
135+
buffer.write(outputStream.toByteArray());
136+
MockResponse.Builder builder = new MockResponse.Builder()
137+
.body(buffer)
138+
.code(200);
139+
if (!encoding.isEmpty()) {
140+
builder.setHeader(HttpHeaders.CONTENT_ENCODING, encoding);
141+
}
142+
return builder.build();
134143
}
135144
return new MockResponse.Builder().code(404).build();
136145
}

‎spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestFactoryTests.java

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -100,49 +100,52 @@ public void contentLength0() throws IOException {
100100
void deleteRequestWithBody() throws Exception {
101101
URI uri = URI.create(baseUrl + "/echo");
102102
ClientHttpRequest request = this.factory.createRequest(uri, HttpMethod.DELETE);
103-
StreamUtils.copy("body", StandardCharsets.ISO_8859_1, request.getBody());
103+
StreamUtils.copy("body", StandardCharsets.UTF_8, request.getBody());
104104
try (ClientHttpResponse response = request.execute()) {
105105
assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK);
106-
assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.ISO_8859_1))
107-
.as("Invalid request body").isEqualTo("body");
106+
assertThat(response.getBody()).as("Invalid request body").hasContent("body");
108107
}
109108
}
110109

111110
@Test
112111
void compressionDisabled() throws IOException {
113112
URI uri = URI.create(baseUrl + "/compress/");
114-
ClientHttpRequest request = this.factory.createRequest(uri, HttpMethod.GET);
113+
ClientHttpRequest request = this.factory.createRequest(uri, HttpMethod.POST);
114+
StreamUtils.copy("Payload to compress", StandardCharsets.UTF_8, request.getBody());
115115
try (ClientHttpResponse response = request.execute()) {
116116
assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK);
117-
assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.ISO_8859_1))
118-
.as("Invalid request body").isEqualTo("Test Payload");
117+
assertThat(response.getHeaders().containsHeader("Content-Encoding")).isFalse();
118+
assertThat(response.getBody()).as("Invalid request body").hasContent("Payload to compress");
119119
}
120120
}
121121

122122
@Test
123123
void compressionGzip() throws IOException {
124124
URI uri = URI.create(baseUrl + "/compress/gzip");
125125
JdkClientHttpRequestFactory requestFactory = (JdkClientHttpRequestFactory) this.factory;
126-
requestFactory.setCompressionEnabled(true);
127-
ClientHttpRequest request = requestFactory.createRequest(uri, HttpMethod.GET);
128-
126+
requestFactory.enableCompression(true);
127+
ClientHttpRequest request = requestFactory.createRequest(uri, HttpMethod.POST);
128+
StreamUtils.copy("Payload to compress", StandardCharsets.UTF_8, request.getBody());
129129
try (ClientHttpResponse response = request.execute()) {
130130
assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK);
131-
assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.ISO_8859_1))
132-
.as("Invalid request body").isEqualTo("Test Payload");
131+
assertThat(response.getHeaders().getFirst("Content-Encoding"))
132+
.as("Invalid content encoding").isEqualTo("gzip");
133+
assertThat(response.getBody()).as("Invalid request body").hasContent("Payload to compress");
133134
}
134135
}
135136

136137
@Test
137138
void compressionDeflate() throws IOException {
138139
URI uri = URI.create(baseUrl + "/compress/deflate");
139140
JdkClientHttpRequestFactory requestFactory = (JdkClientHttpRequestFactory) this.factory;
140-
requestFactory.setCompressionEnabled(true);
141-
ClientHttpRequest request = requestFactory.createRequest(uri, HttpMethod.GET);
141+
requestFactory.enableCompression(true);
142+
ClientHttpRequest request = requestFactory.createRequest(uri, HttpMethod.POST);
143+
StreamUtils.copy("Payload to compress", StandardCharsets.UTF_8, request.getBody());
142144
try (ClientHttpResponse response = request.execute()) {
143145
assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK);
144-
assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.ISO_8859_1))
145-
.as("Invalid request body").isEqualTo("Test Payload");
146+
assertThat(response.getHeaders().getFirst("Content-Encoding"))
147+
.as("Invalid content encoding").isEqualTo("deflate");
148+
assertThat(response.getBody()).as("Invalid request body").hasContent("Payload to compress");
146149
}
147150
}
148151

‎spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ void futureCancelled() {
7272
}
7373

7474
private JdkClientHttpRequest createRequest(Duration timeout) {
75-
return new JdkClientHttpRequest(client, URI.create("https://abc.com"), HttpMethod.GET, executor, timeout);
75+
return new JdkClientHttpRequest(client, URI.create("https://abc.com"), HttpMethod.GET, executor, timeout, false);
7676
}
7777

7878
}

0 commit comments

Comments
 (0)