Browse Source

[client] Fix decompressed response headers (#63419)

When a gzip-encoded response is decompressed the response should no more
have a content-encoding header and content-length should be set to
"unknown". GzipDecompressingEntity correctly does this for the entity
but the response still reported the original response's content-encoding
and content-length headers.
Sylvain Wallez 5 years ago
parent
commit
4f29e3e1ba

+ 11 - 7
client/rest/src/main/java/org/elasticsearch/client/RestClient.java

@@ -49,6 +49,7 @@ import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
 import org.apache.http.nio.client.methods.HttpAsyncMethods;
 import org.apache.http.nio.protocol.HttpAsyncRequestProducer;
 import org.apache.http.nio.protocol.HttpAsyncResponseConsumer;
+import org.apache.http.protocol.HTTP;
 
 import javax.net.ssl.SSLHandshakeException;
 import java.io.ByteArrayInputStream;
@@ -73,7 +74,6 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -308,12 +308,16 @@ public class RestClient implements Closeable {
         RequestLogger.logResponse(logger, request.httpRequest, node.getHost(), httpResponse);
         int statusCode = httpResponse.getStatusLine().getStatusCode();
 
-        Optional.ofNullable(httpResponse.getEntity())
-            .map(HttpEntity::getContentEncoding)
-            .map(Header::getValue)
-            .filter("gzip"::equalsIgnoreCase)
-            .map(gzipHeaderValue -> new GzipDecompressingEntity(httpResponse.getEntity()))
-            .ifPresent(httpResponse::setEntity);
+        HttpEntity entity = httpResponse.getEntity();
+        if (entity != null) {
+            Header header = entity.getContentEncoding();
+            if (header != null && "gzip".equals(header.getValue())) {
+                // Decompress and cleanup response headers
+                httpResponse.setEntity(new GzipDecompressingEntity(entity));
+                httpResponse.removeHeaders(HTTP.CONTENT_ENCODING);
+                httpResponse.removeHeaders(HTTP.CONTENT_LEN);
+            }
+        }
 
         Response response = new Response(request.httpRequest.getRequestLine(), node.getHost(), httpResponse);
         if (isSuccessfulResponse(statusCode) || request.ignoreErrorCodes.contains(response.getStatusLine().getStatusCode())) {

+ 55 - 15
client/rest/src/test/java/org/elasticsearch/client/RestClientGzipCompressionTests.java

@@ -26,6 +26,7 @@ import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
 import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
+import org.apache.http.protocol.HTTP;
 import org.elasticsearch.mocksocket.MockHttpServer;
 import org.junit.AfterClass;
 import org.junit.Assert;
@@ -80,10 +81,9 @@ public class RestClientGzipCompressionTests extends RestClientTestCase {
                 exchange.getResponseHeaders().add("Content-Encoding", "gzip");
             }
 
-            exchange.sendResponseHeaders(200, 0);
-
             // Encode response if needed
-            OutputStream out = exchange.getResponseBody();
+            ByteArrayOutputStream bao = new ByteArrayOutputStream();
+            OutputStream out = bao;
             if (compress) {
                 out = new GZIPOutputStream(out);
             }
@@ -96,6 +96,11 @@ public class RestClientGzipCompressionTests extends RestClientTestCase {
             out.write(bytes);
             out.close();
 
+            bytes = bao.toByteArray();
+
+            exchange.sendResponseHeaders(200, bytes.length);
+
+            exchange.getResponseBody().write(bytes);
             exchange.close();
         }
     }
@@ -119,6 +124,22 @@ public class RestClientGzipCompressionTests extends RestClientTestCase {
             .build();
     }
 
+    public void testUncompressedSync() throws Exception {
+        RestClient restClient = createClient(false);
+
+        // Send non-compressed request, expect non-compressed response
+        Request request = new Request("POST", "/");
+        request.setEntity(new StringEntity("plain request, plain response", ContentType.TEXT_PLAIN));
+
+        Response response = restClient.performRequest(request);
+
+        // Server sends a content-length which should be kept
+        Assert.assertTrue(response.getEntity().getContentLength() > 0);
+        checkResponse("null#null#plain request, plain response", response);
+
+        restClient.close();
+    }
+
     public void testGzipHeaderSync() throws Exception {
         RestClient restClient = createClient(false);
 
@@ -129,9 +150,9 @@ public class RestClientGzipCompressionTests extends RestClientTestCase {
 
         Response response = restClient.performRequest(request);
 
-        HttpEntity entity = response.getEntity();
-        String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8);
-        Assert.assertEquals("null#gzip#plain request, gzip response", content);
+        // Content-length is unknown because of ungzip. Do not just test -1 as it returns "a negative number if unknown"
+        Assert.assertTrue(response.getEntity().getContentLength() < 0);
+        checkResponse("null#gzip#plain request, gzip response", response);
 
         restClient.close();
     }
@@ -148,9 +169,8 @@ public class RestClientGzipCompressionTests extends RestClientTestCase {
         restClient.performRequestAsync(request, futureResponse);
         Response response = futureResponse.get();
 
-        HttpEntity entity = response.getEntity();
-        String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8);
-        Assert.assertEquals("null#gzip#plain request, gzip response", content);
+        Assert.assertTrue(response.getEntity().getContentLength() < 0);
+        checkResponse("null#gzip#plain request, gzip response", response);
 
         restClient.close();
     }
@@ -163,9 +183,8 @@ public class RestClientGzipCompressionTests extends RestClientTestCase {
 
         Response response = restClient.performRequest(request);
 
-        HttpEntity entity = response.getEntity();
-        String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8);
-        Assert.assertEquals("gzip#gzip#compressing client", content);
+        Assert.assertTrue(response.getEntity().getContentLength() < 0);
+        checkResponse("gzip#gzip#compressing client", response);
 
         restClient.close();
     }
@@ -184,9 +203,8 @@ public class RestClientGzipCompressionTests extends RestClientTestCase {
         Response response = futureResponse.get();
 
         // Server should report it had a compressed request and sent back a compressed response
-        HttpEntity entity = response.getEntity();
-        String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8);
-        Assert.assertEquals("gzip#gzip#compressing client", content);
+        Assert.assertTrue(response.getEntity().getContentLength() < 0);
+        checkResponse("gzip#gzip#compressing client", response);
 
         restClient.close();
     }
@@ -202,4 +220,26 @@ public class RestClientGzipCompressionTests extends RestClientTestCase {
             this.completeExceptionally(exception);
         }
     }
+
+    private static void checkResponse(String expected, Response response) throws Exception {
+        HttpEntity entity = response.getEntity();
+        Assert.assertNotNull(entity);
+
+        String content = new String(readAll(entity.getContent()), StandardCharsets.UTF_8);
+        Assert.assertEquals(expected, content);
+
+        // Original Content-Encoding should be removed on both entity and response
+        Assert.assertNull(entity.getContentEncoding());
+        Assert.assertNull(response.getHeader(HTTP.CONTENT_ENCODING));
+
+        // Content-length must be consistent between entity and response
+        long entityContentLength = entity.getContentLength();
+        String headerContentLength = response.getHeader(HTTP.CONTENT_LEN);
+
+        if (entityContentLength < 0) {
+            Assert.assertNull(headerContentLength);
+        } else {
+            Assert.assertEquals(String.valueOf(entityContentLength), headerContentLength);
+        }
+    }
 }

+ 1 - 1
qa/smoke-test-http/src/test/java/org/elasticsearch/http/HttpCompressionIT.java

@@ -59,7 +59,7 @@ public class HttpCompressionIT extends ESRestTestCase {
         request.setOptions(requestOptions);
         response = client().performRequest(request);
         assertEquals(200, response.getStatusLine().getStatusCode());
-        assertEquals(GZIP_ENCODING, response.getHeader(HttpHeaders.CONTENT_ENCODING));
+        assertNull(response.getHeader(HttpHeaders.CONTENT_ENCODING));
         assertThat(response.getEntity(), instanceOf(GzipDecompressingEntity.class));
 
         String body = EntityUtils.toString(response.getEntity());