From f2dde3c28a5a080471a125f3eca7b6d4693ae28f Mon Sep 17 00:00:00 2001 From: Lukas Niemeier Date: Sat, 16 Nov 2019 15:28:21 +0100 Subject: [PATCH] Back off if encoding is already given --- riptide-compression/README.md | 21 ++++++++++++ .../compression/RequestCompressionPlugin.java | 2 +- .../riptide/compression/GzipClientDriver.java | 4 +-- .../RequestCompressionPluginTest.java | 33 +++++++++++++++---- 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/riptide-compression/README.md b/riptide-compression/README.md index 962b37ac0..c94bd81a5 100644 --- a/riptide-compression/README.md +++ b/riptide-compression/README.md @@ -50,6 +50,27 @@ In order to specify the compression algorithm you can pass in a custom `Compress new RequestCompressionPlugin(Compression.of("br", BrotliOutputStream::new)); ``` +## Usage + +```java +http.post("/events") + .contentType(MediaType.APPLICATION_JSON) + .body(asList(events)) + .call(pass()) + .join(); +``` + +All request bodies will be compressed using the configured compression method using `chunked` transfer-encoding. + +If there is already a `Content-Encoding` specified on the request, the plugin does nothing. + +### Limitations + +* You must only configure a single `RequestCompressionPlugin` as only a single encoding is applied currently. +* Starting with Spring 4.3 the `Netty4ClientHttpRequestFactory` unconditionally adds a `Content-Length` header, +which breaks if used together with `RequestCompressionPlugin`. Use `riptide-httpclient` instead. + + ## Getting Help If you have questions, concerns, bug reports, etc., please file an issue in this repository's [Issue Tracker](../../../../issues). diff --git a/riptide-compression/src/main/java/org/zalando/riptide/compression/RequestCompressionPlugin.java b/riptide-compression/src/main/java/org/zalando/riptide/compression/RequestCompressionPlugin.java index afd157675..31b50bc2a 100644 --- a/riptide-compression/src/main/java/org/zalando/riptide/compression/RequestCompressionPlugin.java +++ b/riptide-compression/src/main/java/org/zalando/riptide/compression/RequestCompressionPlugin.java @@ -36,7 +36,7 @@ public RequestExecution aroundNetwork(final RequestExecution execution) { return arguments -> { final Entity entity = arguments.getEntity(); - if (entity.isEmpty()) { + if (entity.isEmpty() || arguments.getHeaders().containsKey(CONTENT_ENCODING)) { return execution.execute(arguments); } diff --git a/riptide-compression/src/test/java/org/zalando/riptide/compression/GzipClientDriver.java b/riptide-compression/src/test/java/org/zalando/riptide/compression/GzipClientDriver.java index 14e959688..0d19cfa87 100644 --- a/riptide-compression/src/test/java/org/zalando/riptide/compression/GzipClientDriver.java +++ b/riptide-compression/src/test/java/org/zalando/riptide/compression/GzipClientDriver.java @@ -13,12 +13,12 @@ @Hack final class GzipClientDriver extends ClientDriver { + private int port; + static ClientDriver create() { return new GzipClientDriver(new DefaultClientDriverJettyHandler(new DefaultRequestMatcher())); } - private int port; - private GzipClientDriver(ClientDriverJettyHandler handler) { super(handler); } diff --git a/riptide-compression/src/test/java/org/zalando/riptide/compression/RequestCompressionPluginTest.java b/riptide-compression/src/test/java/org/zalando/riptide/compression/RequestCompressionPluginTest.java index b537a0e18..582d0f8fb 100644 --- a/riptide-compression/src/test/java/org/zalando/riptide/compression/RequestCompressionPluginTest.java +++ b/riptide-compression/src/test/java/org/zalando/riptide/compression/RequestCompressionPluginTest.java @@ -30,6 +30,7 @@ import static java.util.concurrent.Executors.newSingleThreadExecutor; import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.equalTo; +import static org.springframework.http.HttpHeaders.CONTENT_ENCODING; import static org.zalando.riptide.PassRoute.pass; class RequestCompressionPluginTest { @@ -46,7 +47,7 @@ void tearDown() throws Exception { @ParameterizedTest @ArgumentsSource(RequestFactorySource.class) - void shouldCompressRequestUsingFactory(final ClientHttpRequestFactory factory) { + void shouldCompressRequestBody(final ClientHttpRequestFactory factory) { driver.addExpectation(onRequestTo("/") .withMethod(POST) .withHeader("X-Content-Encoding", "gzip") // written by Jetty's GzipHandler @@ -63,16 +64,17 @@ void shouldCompressRequestUsingFactory(final ClientHttpRequestFactory factory) { @ParameterizedTest @ArgumentsSource(RequestFactorySource.class) - void shouldNotCompressEmptyBody(final ClientHttpRequestFactory factory) { + void shouldNotCompressEmptyRequestBody(final ClientHttpRequestFactory factory) { driver.addExpectation(onRequestTo("/") - .withBody(emptyString(), "text/plain") + .withMethod(POST) + .withBody(emptyString(), "application/json") .withoutHeader("Content-Encoding") .withoutHeader("X-Content-Encoding"), giveResponse("", "text/plain")); final Http http = buildHttp(factory, new RequestCompressionPlugin()); - http.get("/") - .contentType(MediaType.TEXT_PLAIN) + http.post("/") + .contentType(MediaType.APPLICATION_JSON) .call(pass()) .join(); } @@ -95,6 +97,25 @@ void shouldCompressWithGivenAlgorithm(final ClientHttpRequestFactory factory) { .join(); } + @ParameterizedTest + @ArgumentsSource(RequestFactorySource.class) + void shouldBackOffIfAlreadyEncoded(final ClientHttpRequestFactory factory) { + driver.addExpectation(onRequestTo("/") + .withMethod(POST) + .withHeader("Content-Encoding", "custom") // not handled by Jetty + .withoutHeader("X-Content-Encoding") + .withBody(equalTo("{}"), "application/json"), + giveResponse("", "text/plain")); + + final Http http = buildHttp(factory, new RequestCompressionPlugin()); + http.post("/") + .header(CONTENT_ENCODING, "custom") + .contentType(MediaType.APPLICATION_JSON) + .body(new HashMap<>()) + .call(pass()) + .join(); + } + private Http buildHttp(final ClientHttpRequestFactory factory, final Plugin... plugins) { return Http.builder() .executor(executor) @@ -109,7 +130,7 @@ static class RequestFactorySource implements ArgumentsProvider { public Stream provideArguments(ExtensionContext extensionContext) { return Stream.of( new SimpleClientHttpRequestFactory(), - new Netty4ClientHttpRequestFactory(), + // new Netty4ClientHttpRequestFactory(), # broken, see #823 new BufferingClientHttpRequestFactory(new SimpleClientHttpRequestFactory()), new ApacheClientHttpRequestFactory(HttpClients.createDefault(), Mode.BUFFERING), new ApacheClientHttpRequestFactory(HttpClients.createDefault(), Mode.STREAMING)