Skip to content

Commit

Permalink
Add support for request body gzip Content-Encoding (#2247)
Browse files Browse the repository at this point in the history
* Add support for request body gzip Content-Encoding

* do not use 'deflate' in unit test. It is not supported for requests

---------

Co-authored-by: Fabien Bussiron <[email protected]>
Co-authored-by: Marvin <[email protected]>
  • Loading branch information
3 people authored Dec 7, 2023
1 parent 4f4ccab commit 6f4226d
Show file tree
Hide file tree
Showing 4 changed files with 137 additions and 7 deletions.
15 changes: 13 additions & 2 deletions hc5/src/main/java/feign/hc5/ApacheHttp5Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -121,6 +122,7 @@ ClassicHttpRequest toClassicHttpRequest(Request request, Request.Options options

// request headers
boolean hasAcceptHeader = false;
boolean isGzip = false;
for (final Map.Entry<String, Collection<String>> headerEntry : request.headers().entrySet()) {
final String headerName = headerEntry.getKey();
if (headerName.equalsIgnoreCase(ACCEPT_HEADER_NAME)) {
Expand All @@ -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);
}
Expand All @@ -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));
Expand Down
26 changes: 24 additions & 2 deletions hc5/src/main/java/feign/hc5/AsyncApacheHttp5Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -114,6 +117,7 @@ SimpleHttpRequest toClassicHttpRequest(Request request,

// request headers
boolean hasAcceptHeader = false;
boolean isGzip = false;
for (final Map.Entry<String, Collection<String>> headerEntry : request.headers().entrySet()) {
final String headerName = headerEntry.getKey();
if (headerName.equalsIgnoreCase(ACCEPT_HEADER_NAME)) {
Expand All @@ -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);
}
Expand All @@ -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));
}
Expand Down
6 changes: 3 additions & 3 deletions hc5/src/test/java/feign/hc5/AsyncApacheHttp5ClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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);
Expand Down Expand Up @@ -880,7 +880,7 @@ CompletableFuture<Void> expandArray(@Param(value = "clock",
CompletableFuture<Void> headerMap(@HeaderMap Map<String, Object> headerMap);

@RequestLine("GET /")
@Headers("Content-Encoding: deflate")
@Headers("Content-Encoding: gzip")
CompletableFuture<Void> headerMapWithHeaderAnnotations(@HeaderMap Map<String, Object> headerMap);

@RequestLine("GET /")
Expand Down
97 changes: 97 additions & 0 deletions hc5/src/test/java/feign/hc5/GzipHttp5ClientTest.java
Original file line number Diff line number Diff line change
@@ -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);

}
}

0 comments on commit 6f4226d

Please sign in to comment.