From 79673abc085400ef58721032d41417732c45c2e2 Mon Sep 17 00:00:00 2001 From: Fabien Bussiron Date: Mon, 27 Nov 2023 17:07:25 +0100 Subject: [PATCH 1/2] Add support for request body gzip Content-Encoding --- .../java/feign/hc5/ApacheHttp5Client.java | 15 ++- .../feign/hc5/AsyncApacheHttp5Client.java | 26 ++++- .../java/feign/hc5/GzipHttp5ClientTest.java | 97 +++++++++++++++++++ 3 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 hc5/src/test/java/feign/hc5/GzipHttp5ClientTest.java diff --git a/hc5/src/main/java/feign/hc5/ApacheHttp5Client.java b/hc5/src/main/java/feign/hc5/ApacheHttp5Client.java index 89910fd563..9f7d235431 100644 --- a/hc5/src/main/java/feign/hc5/ApacheHttp5Client.java +++ b/hc5/src/main/java/feign/hc5/ApacheHttp5Client.java @@ -34,6 +34,7 @@ import org.apache.hc.client5.http.classic.HttpClient; import org.apache.hc.client5.http.config.Configurable; import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.entity.GzipCompressingEntity; import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.core5.http.ClassicHttpRequest; @@ -121,6 +122,7 @@ ClassicHttpRequest toClassicHttpRequest(Request request, Request.Options options // request headers boolean hasAcceptHeader = false; + boolean isGzip = false; for (final Map.Entry> headerEntry : request.headers().entrySet()) { final String headerName = headerEntry.getKey(); if (headerName.equalsIgnoreCase(ACCEPT_HEADER_NAME)) { @@ -132,7 +134,14 @@ ClassicHttpRequest toClassicHttpRequest(Request request, Request.Options options // doesn't like us to set it as well. continue; } - + if (headerName.equalsIgnoreCase(Util.CONTENT_ENCODING)) { + isGzip = headerEntry.getValue().stream().anyMatch(Util.ENCODING_GZIP::equalsIgnoreCase); + boolean isDeflate = headerEntry.getValue().stream().anyMatch(Util.ENCODING_DEFLATE::equalsIgnoreCase); + if (isDeflate) { + // DeflateCompressingEntity not available in hc5 yet + throw new IllegalArgumentException("Deflate Content-Encoding is not supported by feign-hc5"); + } + } for (final String headerValue : headerEntry.getValue()) { requestBuilder.addHeader(headerName, headerValue); } @@ -159,7 +168,9 @@ ClassicHttpRequest toClassicHttpRequest(Request request, Request.Options options } entity = new StringEntity(content, contentType); } - + if(isGzip) { + entity = new GzipCompressingEntity(entity); + } requestBuilder.setEntity(entity); } else { requestBuilder.setEntity(new ByteArrayEntity(new byte[0], null)); diff --git a/hc5/src/main/java/feign/hc5/AsyncApacheHttp5Client.java b/hc5/src/main/java/feign/hc5/AsyncApacheHttp5Client.java index 35217376f8..8e2accad08 100644 --- a/hc5/src/main/java/feign/hc5/AsyncApacheHttp5Client.java +++ b/hc5/src/main/java/feign/hc5/AsyncApacheHttp5Client.java @@ -24,7 +24,10 @@ import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.io.CloseMode; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.util.*; +import java.util.zip.GZIPOutputStream; import java.util.concurrent.CompletableFuture; import feign.*; import feign.Request.Options; @@ -114,6 +117,7 @@ SimpleHttpRequest toClassicHttpRequest(Request request, // request headers boolean hasAcceptHeader = false; + boolean isGzip = false; for (final Map.Entry> headerEntry : request.headers().entrySet()) { final String headerName = headerEntry.getKey(); if (headerName.equalsIgnoreCase(ACCEPT_HEADER_NAME)) { @@ -125,7 +129,15 @@ SimpleHttpRequest toClassicHttpRequest(Request request, // doesn't like us to set it as well. continue; } - + if (headerName.equalsIgnoreCase(Util.CONTENT_ENCODING)) { + isGzip = headerEntry.getValue().stream().anyMatch(Util.ENCODING_GZIP::equalsIgnoreCase); + boolean isDeflate = headerEntry.getValue().stream().anyMatch(Util.ENCODING_DEFLATE::equalsIgnoreCase); + if(isDeflate) { + // DeflateCompressingEntity not available in hc5 yet + throw new IllegalArgumentException("Deflate Content-Encoding is not supported by feign-hc5"); + } + } + for (final String headerValue : headerEntry.getValue()) { httpRequest.addHeader(headerName, headerValue); } @@ -137,7 +149,17 @@ SimpleHttpRequest toClassicHttpRequest(Request request, // request body // final Body requestBody = request.requestBody(); - final byte[] data = request.body(); + byte[] data = request.body(); + if(isGzip && data != null && data.length > 0) { + // compress if needed + try(ByteArrayOutputStream baos = new ByteArrayOutputStream(); + GZIPOutputStream gzipOs = new GZIPOutputStream(baos, true)) { + gzipOs.write(data); + gzipOs.flush(); + data = baos.toByteArray(); + } catch (IOException suppressed) { // NOPMD + } + } if (data != null) { httpRequest.setBody(data, getContentType(request)); } diff --git a/hc5/src/test/java/feign/hc5/GzipHttp5ClientTest.java b/hc5/src/test/java/feign/hc5/GzipHttp5ClientTest.java new file mode 100644 index 0000000000..4c064428b3 --- /dev/null +++ b/hc5/src/test/java/feign/hc5/GzipHttp5ClientTest.java @@ -0,0 +1,97 @@ +/* + * Copyright 2012-2023 The Feign Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package feign.hc5; + +import static org.junit.Assume.assumeTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.zip.GZIPInputStream; + +import org.junit.jupiter.api.Test; + +import feign.Feign; +import feign.Feign.Builder; +import feign.RequestLine; +import feign.client.AbstractClientTest; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.RecordedRequest; + +/** + * Tests that 'Content-Encoding: gzip' is handled correctly + */ +public class GzipHttp5ClientTest extends AbstractClientTest { + + @Override + public Builder newBuilder() { + return Feign.builder().client(new ApacheHttp5Client()); + } + + @Test + public void testWithCompressedBody() throws InterruptedException, IOException { + final TestInterface testInterface = buildTestInterface(true); + + server.enqueue(new MockResponse().setBody("foo")); + + assertEquals("foo", testInterface.withBody("bar")); + final RecordedRequest request1 = server.takeRequest(); + assertEquals("/test", request1.getPath()); + + ByteArrayInputStream bodyContentIs = new ByteArrayInputStream(request1.getBody().readByteArray()); + byte[] uncompressed = new GZIPInputStream(bodyContentIs).readAllBytes(); + + assertEquals("bar", new String(uncompressed, StandardCharsets.UTF_8)); + + } + + @Test + public void testWithUncompressedBody() throws InterruptedException, IOException { + final TestInterface testInterface = buildTestInterface(false); + + server.enqueue(new MockResponse().setBody("foo")); + + assertEquals("foo", testInterface.withBody("bar")); + final RecordedRequest request1 = server.takeRequest(); + assertEquals("/test", request1.getPath()); + + assertEquals("bar", request1.getBody().readString(StandardCharsets.UTF_8)); + } + + + private TestInterface buildTestInterface(boolean compress) { + return newBuilder() + .requestInterceptor(req -> req.header("Content-Encoding", compress ? "gzip" : "")) + .target(TestInterface.class, "http://localhost:" + server.getPort()); + } + + + @Override + public void testVeryLongResponseNullLength() { + assumeTrue("HC5 client seems to hang with response size equalto Long.MAX", false); + } + + @Override + public void testContentTypeDefaultsToRequestCharset() throws Exception { + assumeTrue("this test is flaky on windows, but works fine.", false); + } + + public interface TestInterface { + + @RequestLine("POST /test") + String withBody(String body); + + } +} From 5be16d85965d6dcf00e2ca5851efadbd93978991 Mon Sep 17 00:00:00 2001 From: Bussiron Date: Tue, 28 Nov 2023 10:21:29 +0100 Subject: [PATCH 2/2] do not use 'deflate' in unit test. It is not supported for requests --- hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java b/hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java index 02661aff73..8f6764ab51 100644 --- a/hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java +++ b/hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java @@ -243,7 +243,7 @@ public void headerMapWithHeaderAnnotations() throws Exception { api.headerMapWithHeaderAnnotations(headerMap); // header map should be additive for headers provided by annotations - assertThat(server.takeRequest()).hasHeaders(entry("Content-Encoding", Arrays.asList("deflate")), + assertThat(server.takeRequest()).hasHeaders(entry("Content-Encoding", Arrays.asList("gzip")), entry("Custom-Header", Arrays.asList("fooValue"))); server.enqueue(new MockResponse()); @@ -256,7 +256,7 @@ public void headerMapWithHeaderAnnotations() throws Exception { * valid to have more than one value for a header. */ assertThat(server.takeRequest()).hasHeaders( - entry("Content-Encoding", Arrays.asList("deflate", "overrideFromMap")), + entry("Content-Encoding", Arrays.asList("gzip", "overrideFromMap")), entry("Custom-Header", Arrays.asList("fooValue"))); checkCFCompletedSoon(cf); @@ -880,7 +880,7 @@ CompletableFuture expandArray(@Param(value = "clock", CompletableFuture headerMap(@HeaderMap Map headerMap); @RequestLine("GET /") - @Headers("Content-Encoding: deflate") + @Headers("Content-Encoding: gzip") CompletableFuture headerMapWithHeaderAnnotations(@HeaderMap Map headerMap); @RequestLine("GET /")