From 18eb2a607386460f44c98dae008e58d47213dbad Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 25 Aug 2025 19:06:25 +0200 Subject: [PATCH] Polishing contribution Closes gh-35225 --- .../http/client/JdkClientHttpRequest.java | 28 ++++++------- .../client/JdkClientHttpRequestFactory.java | 12 +++--- .../client/AbstractMockWebServerTests.java | 39 ++++++++++++------- .../JdkClientHttpRequestFactoryTests.java | 33 +++++++++------- .../client/JdkClientHttpRequestTests.java | 2 +- 5 files changed, 60 insertions(+), 54 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java index 930d5bdbfdd..4c9bef04a6f 100644 --- a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequest.java @@ -67,7 +67,7 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { private static final Set DISALLOWED_HEADERS = disallowedHeaders(); - private static final List ALLOWED_ENCODINGS = List.of("gzip", "deflate"); + private static final List SUPPORTED_ENCODINGS = List.of("gzip", "deflate"); private final HttpClient httpClient; @@ -80,18 +80,18 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { private final @Nullable Duration timeout; - private final boolean compressionEnabled; + private final boolean compression; - public JdkClientHttpRequest(HttpClient httpClient, URI uri, HttpMethod method, Executor executor, - @Nullable Duration readTimeout, boolean compressionEnabled) { + JdkClientHttpRequest(HttpClient httpClient, URI uri, HttpMethod method, Executor executor, + @Nullable Duration readTimeout, boolean compression) { this.httpClient = httpClient; this.uri = uri; this.method = method; this.executor = executor; this.timeout = readTimeout; - this.compressionEnabled = compressionEnabled; + this.compression = compression; } @@ -164,13 +164,10 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { private HttpRequest buildRequest(HttpHeaders headers, @Nullable Body body) { HttpRequest.Builder builder = HttpRequest.newBuilder().uri(this.uri); - // When compression is enabled and valid encoding is absent, we add gzip as standard encoding - if (this.compressionEnabled) { - if (headers.containsHeader(HttpHeaders.ACCEPT_ENCODING) && - !ALLOWED_ENCODINGS.contains(headers.getFirst(HttpHeaders.ACCEPT_ENCODING))) { - headers.remove(HttpHeaders.ACCEPT_ENCODING); + if (this.compression) { + if (!headers.containsHeader(HttpHeaders.ACCEPT_ENCODING)) { + headers.addAll(HttpHeaders.ACCEPT_ENCODING, SUPPORTED_ENCODINGS); } - headers.add(HttpHeaders.ACCEPT_ENCODING, "gzip"); } headers.forEach((headerName, headerValues) -> { @@ -310,16 +307,15 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { } /** - * Custom BodyHandler that checks the Content-Encoding header and applies the appropriate decompression algorithm. + * BodyHandler that checks the Content-Encoding header and applies the appropriate decompression algorithm. * Supports Gzip and Deflate encoded responses. */ - public static final class DecompressingBodyHandler implements BodyHandler { + private static final class DecompressingBodyHandler implements BodyHandler { @Override public BodySubscriber apply(ResponseInfo responseInfo) { String contentEncoding = responseInfo.headers().firstValue(HttpHeaders.CONTENT_ENCODING).orElse(""); if (contentEncoding.equalsIgnoreCase("gzip")) { - // If the content is gzipped, wrap the InputStream with a GZIPInputStream return BodySubscribers.mapping( BodySubscribers.ofInputStream(), (InputStream is) -> { @@ -327,18 +323,16 @@ class JdkClientHttpRequest extends AbstractStreamingClientHttpRequest { return new GZIPInputStream(is); } catch (IOException ex) { - throw new UncheckedIOException(ex); // Propagate IOExceptions + throw new UncheckedIOException(ex); } }); } else if (contentEncoding.equalsIgnoreCase("deflate")) { - // If the content is encoded using deflate, wrap the InputStream with a InflaterInputStream return BodySubscribers.mapping( BodySubscribers.ofInputStream(), InflaterInputStream::new); } else { - // Otherwise, return a standard InputStream BodySubscriber return BodySubscribers.ofInputStream(); } } diff --git a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java index 01cce828239..14dec7ffa51 100644 --- a/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java +++ b/spring-web/src/main/java/org/springframework/http/client/JdkClientHttpRequestFactory.java @@ -43,7 +43,7 @@ public class JdkClientHttpRequestFactory implements ClientHttpRequestFactory { private @Nullable Duration readTimeout; - private boolean compressionEnabled; + private boolean compression = true; /** @@ -99,17 +99,17 @@ public class JdkClientHttpRequestFactory implements ClientHttpRequestFactory { } /** - * Sets custom {@link BodyHandler} that can handle gzip encoded {@link HttpClient}'s response. - * @param compressionEnabled to enable compression by default for all {@link HttpClient}'s requests. + * Set whether support for uncompressing "gzip" and "deflate" HTTP responses is enabled. + * @since 7.0 */ - public void setCompressionEnabled(boolean compressionEnabled) { - this.compressionEnabled = compressionEnabled; + public void enableCompression(boolean enable) { + this.compression = enable; } @Override public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException { - return new JdkClientHttpRequest(this.httpClient, uri, httpMethod, this.executor, this.readTimeout, this.compressionEnabled); + return new JdkClientHttpRequest(this.httpClient, uri, httpMethod, this.executor, this.readTimeout, this.compression); } } diff --git a/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java b/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java index 922b8c00ba7..7730e95c212 100644 --- a/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/AbstractMockWebServerTests.java @@ -16,21 +16,21 @@ package org.springframework.http.client; +import java.io.ByteArrayOutputStream; +import java.util.zip.DeflaterOutputStream; +import java.util.zip.GZIPOutputStream; + import mockwebserver3.Dispatcher; import mockwebserver3.MockResponse; import mockwebserver3.MockWebServer; import mockwebserver3.RecordedRequest; +import okio.Buffer; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.springframework.http.HttpHeaders; import org.springframework.util.StringUtils; -import java.io.ByteArrayOutputStream; -import java.nio.charset.StandardCharsets; -import java.util.zip.DeflaterOutputStream; -import java.util.zip.GZIPOutputStream; - import static org.assertj.core.api.Assertions.assertThat; /** @@ -112,25 +112,34 @@ public abstract class AbstractMockWebServerTests { String headerName = request.getTarget().replace("/header/",""); return new MockResponse.Builder().body(headerName + ":" + request.getHeaders().get(headerName)).code(200).build(); } - else if(request.getTarget().startsWith("/compress/")) { + else if(request.getTarget().startsWith("/compress/") && request.getBody() != null) { String encoding = request.getTarget().replace("/compress/",""); - ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); + String requestBody = request.getBody().utf8(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); if (encoding.equals("gzip")) { - try(GZIPOutputStream gzipOutputStream = new GZIPOutputStream(byteArrayOutputStream)) { - gzipOutputStream.write("Test Payload".getBytes()); + try(GZIPOutputStream gzipOutputStream = new GZIPOutputStream(outputStream)) { + gzipOutputStream.write(requestBody.getBytes()); gzipOutputStream.flush(); } } else if(encoding.equals("deflate")) { - try(DeflaterOutputStream deflaterOutputStream = new DeflaterOutputStream(byteArrayOutputStream)) { - deflaterOutputStream.write("Test Payload".getBytes()); + try(DeflaterOutputStream deflaterOutputStream = new DeflaterOutputStream(outputStream)) { + deflaterOutputStream.write(requestBody.getBytes()); deflaterOutputStream.flush(); } - } else { - byteArrayOutputStream.write("Test Payload".getBytes()); } - return new MockResponse.Builder().body(byteArrayOutputStream.toString(StandardCharsets.ISO_8859_1)) - .code(200).setHeader(HttpHeaders.CONTENT_ENCODING, encoding).build(); + else { + outputStream.write(requestBody.getBytes()); + } + Buffer buffer = new Buffer(); + buffer.write(outputStream.toByteArray()); + MockResponse.Builder builder = new MockResponse.Builder() + .body(buffer) + .code(200); + if (!encoding.isEmpty()) { + builder.setHeader(HttpHeaders.CONTENT_ENCODING, encoding); + } + return builder.build(); } return new MockResponse.Builder().code(404).build(); } diff --git a/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestFactoryTests.java index 8f380754c9a..2d9a9c32a11 100644 --- a/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestFactoryTests.java @@ -100,22 +100,22 @@ class JdkClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { void deleteRequestWithBody() throws Exception { URI uri = URI.create(baseUrl + "/echo"); ClientHttpRequest request = this.factory.createRequest(uri, HttpMethod.DELETE); - StreamUtils.copy("body", StandardCharsets.ISO_8859_1, request.getBody()); + StreamUtils.copy("body", StandardCharsets.UTF_8, request.getBody()); try (ClientHttpResponse response = request.execute()) { assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK); - assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.ISO_8859_1)) - .as("Invalid request body").isEqualTo("body"); + assertThat(response.getBody()).as("Invalid request body").hasContent("body"); } } @Test void compressionDisabled() throws IOException { URI uri = URI.create(baseUrl + "/compress/"); - ClientHttpRequest request = this.factory.createRequest(uri, HttpMethod.GET); + ClientHttpRequest request = this.factory.createRequest(uri, HttpMethod.POST); + StreamUtils.copy("Payload to compress", StandardCharsets.UTF_8, request.getBody()); try (ClientHttpResponse response = request.execute()) { assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK); - assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.ISO_8859_1)) - .as("Invalid request body").isEqualTo("Test Payload"); + assertThat(response.getHeaders().containsHeader("Content-Encoding")).isFalse(); + assertThat(response.getBody()).as("Invalid request body").hasContent("Payload to compress"); } } @@ -123,13 +123,14 @@ class JdkClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { void compressionGzip() throws IOException { URI uri = URI.create(baseUrl + "/compress/gzip"); JdkClientHttpRequestFactory requestFactory = (JdkClientHttpRequestFactory) this.factory; - requestFactory.setCompressionEnabled(true); - ClientHttpRequest request = requestFactory.createRequest(uri, HttpMethod.GET); - + requestFactory.enableCompression(true); + ClientHttpRequest request = requestFactory.createRequest(uri, HttpMethod.POST); + StreamUtils.copy("Payload to compress", StandardCharsets.UTF_8, request.getBody()); try (ClientHttpResponse response = request.execute()) { assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK); - assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.ISO_8859_1)) - .as("Invalid request body").isEqualTo("Test Payload"); + assertThat(response.getHeaders().getFirst("Content-Encoding")) + .as("Invalid content encoding").isEqualTo("gzip"); + assertThat(response.getBody()).as("Invalid request body").hasContent("Payload to compress"); } } @@ -137,12 +138,14 @@ class JdkClientHttpRequestFactoryTests extends AbstractHttpRequestFactoryTests { void compressionDeflate() throws IOException { URI uri = URI.create(baseUrl + "/compress/deflate"); JdkClientHttpRequestFactory requestFactory = (JdkClientHttpRequestFactory) this.factory; - requestFactory.setCompressionEnabled(true); - ClientHttpRequest request = requestFactory.createRequest(uri, HttpMethod.GET); + requestFactory.enableCompression(true); + ClientHttpRequest request = requestFactory.createRequest(uri, HttpMethod.POST); + StreamUtils.copy("Payload to compress", StandardCharsets.UTF_8, request.getBody()); try (ClientHttpResponse response = request.execute()) { assertThat(response.getStatusCode()).as("Invalid response status").isEqualTo(HttpStatus.OK); - assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.ISO_8859_1)) - .as("Invalid request body").isEqualTo("Test Payload"); + assertThat(response.getHeaders().getFirst("Content-Encoding")) + .as("Invalid content encoding").isEqualTo("deflate"); + assertThat(response.getBody()).as("Invalid request body").hasContent("Payload to compress"); } } diff --git a/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTests.java b/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTests.java index 300af1ea221..5b2f0bdc42b 100644 --- a/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/JdkClientHttpRequestTests.java @@ -72,7 +72,7 @@ class JdkClientHttpRequestTests { } private JdkClientHttpRequest createRequest(Duration timeout) { - return new JdkClientHttpRequest(client, URI.create("https://abc.com"), HttpMethod.GET, executor, timeout); + return new JdkClientHttpRequest(client, URI.create("https://abc.com"), HttpMethod.GET, executor, timeout, false); } }