From 84aba05b179180b148453a5d131bd52cdf061792 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Thu, 27 Oct 2022 11:40:33 -0400 Subject: [PATCH] Fixed compression support for h2c protocol (#4944) Signed-off-by: Andriy Redko --- CHANGELOG.md | 2 + .../HighLevelRestClientCompressionIT.java | 37 +++++++++++++++++++ .../netty4/Netty4HttpServerTransport.java | 19 +++++----- .../bootstrap/test-framework.policy | 5 +++ 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f948e0f3d8d8..b17b996a5a284 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -174,6 +174,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Fix a bug on handling an invalid array value for point type field #4900([#4900](https://github.com/opensearch-project/OpenSearch/pull/4900)) - [BUG]: Allow decommission to support delay timeout ([#4930](https://github.com/opensearch-project/OpenSearch/pull/4930)) - Fix failing test: VerifyVersionConstantsIT ([#4946](https://github.com/opensearch-project/OpenSearch/pull/4946)) +- Fixed compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944)) + ### Security - CVE-2022-25857 org.yaml:snakeyaml DOS vulnerability ([#4341](https://github.com/opensearch-project/OpenSearch/pull/4341)) diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/HighLevelRestClientCompressionIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/HighLevelRestClientCompressionIT.java index 054d0ae8670b5..6985353806a01 100644 --- a/client/rest-high-level/src/test/java/org/opensearch/client/HighLevelRestClientCompressionIT.java +++ b/client/rest-high-level/src/test/java/org/opensearch/client/HighLevelRestClientCompressionIT.java @@ -31,13 +31,22 @@ package org.opensearch.client; +import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.classic.methods.HttpPut; +import org.apache.hc.client5.http.entity.GzipCompressingEntity; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.io.entity.StringEntity; import org.opensearch.action.search.SearchRequest; import org.opensearch.action.search.SearchResponse; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import static org.hamcrest.Matchers.equalTo; @@ -62,4 +71,32 @@ public void testCompressesResponseIfRequested() throws IOException { assertEquals(SAMPLE_DOCUMENT, searchResponse.getHits().getHits()[0].getSourceAsString()); } + /** + * The default CloseableHttpAsyncClient does not support compression out of the box (so that applies to RestClient + * and RestHighLevelClient). To check the compression works on both sides, crafting the request using CloseableHttpClient + * instead which uses compression by default. + */ + public void testCompressesRequest() throws IOException, URISyntaxException { + try (CloseableHttpClient client = HttpClients.custom().build()) { + final Node node = client().getNodes().iterator().next(); + final URI baseUri = new URI(node.getHost().toURI()); + + final HttpPut index = new HttpPut(baseUri.resolve("/company/_doc/1")); + index.setEntity(new GzipCompressingEntity(new StringEntity(SAMPLE_DOCUMENT, ContentType.APPLICATION_JSON))); + try (CloseableHttpResponse response = client.execute(index)) { + assertThat(response.getCode(), equalTo(201)); + } + + final HttpGet refresh = new HttpGet(baseUri.resolve("/_refresh")); + try (CloseableHttpResponse response = client.execute(refresh)) { + assertThat(response.getCode(), equalTo(200)); + } + + final HttpPost search = new HttpPost(baseUri.resolve("/_search")); + index.setEntity(new GzipCompressingEntity(new StringEntity("{}", ContentType.APPLICATION_JSON))); + try (CloseableHttpResponse response = client.execute(search)) { + assertThat(response.getCode(), equalTo(200)); + } + } + } } diff --git a/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpServerTransport.java b/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpServerTransport.java index e3fde75e5b551..fcc9ab295c6c7 100644 --- a/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpServerTransport.java +++ b/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/Netty4HttpServerTransport.java @@ -413,18 +413,19 @@ protected void channelRead0(ChannelHandlerContext ctx, HttpMessage msg) throws E // If this handler is hit then no upgrade has been attempted and the client is just talking HTTP final ChannelPipeline pipeline = ctx.pipeline(); pipeline.addAfter(ctx.name(), "handler", getRequestHandler()); - pipeline.replace(this, "aggregator", aggregator); + pipeline.replace(this, "decoder_compress", new HttpContentDecompressor()); - ch.pipeline().addLast("decoder_compress", new HttpContentDecompressor()); - ch.pipeline().addLast("encoder", new HttpResponseEncoder()); + pipeline.addAfter("decoder_compress", "aggregator", aggregator); if (handlingSettings.isCompression()) { - ch.pipeline() - .addAfter("aggregator", "encoder_compress", new HttpContentCompressor(handlingSettings.getCompressionLevel())); + pipeline.addAfter( + "aggregator", + "encoder_compress", + new HttpContentCompressor(handlingSettings.getCompressionLevel()) + ); } - ch.pipeline().addBefore("handler", "request_creator", requestCreator); - ch.pipeline().addBefore("handler", "response_creator", responseCreator); - ch.pipeline() - .addBefore("handler", "pipelining", new Netty4HttpPipeliningHandler(logger, transport.pipeliningMaxEvents)); + pipeline.addBefore("handler", "request_creator", requestCreator); + pipeline.addBefore("handler", "response_creator", responseCreator); + pipeline.addBefore("handler", "pipelining", new Netty4HttpPipeliningHandler(logger, transport.pipeliningMaxEvents)); ctx.fireChannelRead(ReferenceCountUtil.retain(msg)); } diff --git a/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy b/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy index 60b704dc12f0c..b6b95a0ac98c8 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/test-framework.policy @@ -111,6 +111,11 @@ grant codeBase "${codebase.httpcore5}" { permission java.net.SocketPermission "*", "connect"; }; +grant codeBase "${codebase.httpclient5}" { + // httpclient5 makes socket connections for rest tests + permission java.net.SocketPermission "*", "connect"; +}; + grant codeBase "${codebase.httpcore-nio}" { // httpcore makes socket connections for rest tests permission java.net.SocketPermission "*", "connect";