Fixes #8926 - HttpClient GZIPContentDecoder should remove Content-Len…

…gth and Content-Encoding: gzip

Now Content-Length and Content-Encoding are removed/modified by the decoder.
In this way, applications have a correct sets of headers to decide whether to decode the content themselves.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>

jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java

@@ -22,6 +22,10 @@
  */
 public interface ContentDecoder
 {
+    public default void beforeDecoding(HttpExchange exchange)
+    {
+    }
+
     /**
      * <p>Decodes the bytes in the given {@code buffer} and returns decoded bytes, if any.</p>
      *
@@ -39,6 +43,10 @@ public default void release(ByteBuffer decoded)
     {
     }
 
+    public default void afterDecoding(HttpExchange exchange)
+    {
+    }
+
     /**
      * Factory for {@link ContentDecoder}s; subclasses must implement {@link #newContentDecoder()}.
      * <p>

jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java

@@ -14,7 +14,13 @@
 package org.eclipse.jetty.client;
 
 import java.nio.ByteBuffer;
+import java.util.Arrays;
+import java.util.ListIterator;
+import java.util.stream.Collectors;
 
+import org.eclipse.jetty.http.HttpField;
+import org.eclipse.jetty.http.HttpFields;
+import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.io.ByteBufferPool;
 
 /**
@@ -24,6 +30,8 @@ public class GZIPContentDecoder extends org.eclipse.jetty.http.GZIPContentDecode
 {
     public static final int DEFAULT_BUFFER_SIZE = 8192;
 
+    private long decodedLength;
+
     public GZIPContentDecoder()
     {
         this(DEFAULT_BUFFER_SIZE);
@@ -39,13 +47,47 @@ public GZIPContentDecoder(ByteBufferPool byteBufferPool, int bufferSize)
         super(byteBufferPool, bufferSize);
     }
 
+    @Override
+    public void beforeDecoding(HttpExchange exchange)
+    {
+        HttpFields.Mutable headers = exchange.getResponse().headers();
+        // Content-Length is not valid anymore while we are decoding.
+        headers.remove(HttpHeader.CONTENT_LENGTH);
+        // Content-Encoding should be removed/modified as the content will be decoded.
+        ListIterator<HttpField> fields = headers.listIterator();
+        while (fields.hasNext())
+        {
+            HttpField field = fields.next();
+            if (field.getHeader() == HttpHeader.CONTENT_ENCODING)
+            {
+                String value = Arrays.stream(field.getValues())
+                    .filter(encoding -> !"gzip".equalsIgnoreCase(encoding))
+                    .collect(Collectors.joining(", "));
+                if (value.isEmpty())
+                    fields.remove();
+                else
+                    fields.set(new HttpField(HttpHeader.CONTENT_ENCODING, value));
+                break;
+            }
+        }
+    }
+
     @Override
     protected boolean decodedChunk(ByteBuffer chunk)
     {
+        decodedLength += chunk.remaining();
         super.decodedChunk(chunk);
         return true;
     }
 
+    @Override
+    public void afterDecoding(HttpExchange exchange)
+    {
+        HttpFields.Mutable headers = exchange.getResponse().headers();
+        headers.remove(HttpHeader.TRANSFER_ENCODING);
+        headers.putLongField(HttpHeader.CONTENT_LENGTH, decodedLength);
+    }
+
     /**
      * Specialized {@link ContentDecoder.Factory} for the "gzip" encoding.
      */

jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java

@@ -288,31 +288,28 @@ protected boolean responseHeaders(HttpExchange exchange)
         HttpResponse response = exchange.getResponse();
         if (LOG.isDebugEnabled())
             LOG.debug("Response headers {}{}{}", response, System.lineSeparator(), response.getHeaders().toString().trim());
+
+        // Content-Encoding may have multiple values in the order
+        // they are applied, but we only support the last one.
+        List<String> contentEncodings = response.getHeaders().getCSV(HttpHeader.CONTENT_ENCODING.asString(), false);
+        if (contentEncodings != null && !contentEncodings.isEmpty())
+        {
+            // Pick the last content encoding from the server.
+            String contentEncoding = contentEncodings.get(contentEncodings.size() - 1);
+            decoder = getHttpDestination().getHttpClient().getContentDecoderFactories().stream()
+                .filter(f -> f.getEncoding().equalsIgnoreCase(contentEncoding))
+                .findFirst()
+                .map(ContentDecoder.Factory::newContentDecoder)
+                .map(d -> new Decoder(exchange, d))
+                .orElse(null);
+        }
+
         ResponseNotifier notifier = getHttpDestination().getResponseNotifier();
         List<Response.ResponseListener> responseListeners = exchange.getConversation().getResponseListeners();
         notifier.notifyHeaders(responseListeners, response);
         contentListeners.reset(responseListeners);
         contentListeners.notifyBeforeContent(response);
 
-        if (!contentListeners.isEmpty())
-        {
-            List<String> contentEncodings = response.getHeaders().getCSV(HttpHeader.CONTENT_ENCODING.asString(), false);
-            if (contentEncodings != null && !contentEncodings.isEmpty())
-            {
-                for (ContentDecoder.Factory factory : getHttpDestination().getHttpClient().getContentDecoderFactories())
-                {
-                    for (String encoding : contentEncodings)
-                    {
-                        if (factory.getEncoding().equalsIgnoreCase(encoding))
-                        {
-                            decoder = new Decoder(exchange, factory.newContentDecoder());
-                            break;
-                        }
-                    }
-                }
-            }
-        }
-
         if (updateResponseState(ResponseState.TRANSIENT, ResponseState.HEADERS))
         {
             boolean hasDemand = hasDemandOrStall();
@@ -755,6 +752,7 @@ private Decoder(HttpExchange exchange, ContentDecoder decoder)
         {
             this.exchange = exchange;
             this.decoder = Objects.requireNonNull(decoder);
+            decoder.beforeDecoding(exchange);
         }
 
         private boolean decode(ByteBuffer encoded, Callback callback)
@@ -868,6 +866,7 @@ private void resume()
         @Override
         public void destroy()
         {
+            decoder.afterDecoding(exchange);
             if (decoder instanceof Destroyable)
                 ((Destroyable)decoder).destroy();
         }

jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java

@@ -87,6 +87,11 @@ public HttpFields getHeaders()
         return headers.asImmutable();
     }
 
+    protected HttpFields.Mutable headers()
+    {
+        return headers;
+    }
+
     public void clearHeaders()
     {
         headers.clear();

jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java

@@ -422,6 +422,12 @@ default Request port(int port)
     Request onResponseHeader(Response.HeaderListener listener);
 
     /**
+     * <p>Registers a listener for the headers event.</p>
+     * <p>Note that the response headers at this event
+     * may be different from the headers received in
+     * {@link #onResponseHeader(Response.HeaderListener)}
+     * as specified in {@link Response#getHeaders()}.</p>
+     *
      * @param listener a listener for response headers event
      * @return this request object
      */

jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java

@@ -65,6 +65,18 @@ public interface Response
     String getReason();
 
     /**
+     * <p>Returns the headers of this response.</p>
+     * <p>Some headers sent by the server may not be present,
+     * or be present but modified, while the content is being
+     * processed.
+     * A typical example is the {@code Content-Length} header
+     * when the content is sent compressed by the server and
+     * automatically decompressed by the client: the
+     * {@code Content-Length} header will be removed.</p>
+     * <p>Similarly, the {@code Content-Encoding} header
+     * may be removed or modified, as the content is
+     * decoded by the client.</p>
+     *
      * @return the headers of this response
      */
     HttpFields getHeaders();

jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java

@@ -30,6 +30,7 @@
 import org.eclipse.jetty.client.api.ContentResponse;
 import org.eclipse.jetty.client.api.Response;
 import org.eclipse.jetty.client.util.InputStreamResponseListener;
+import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.io.ByteBufferPool;
@@ -43,6 +44,7 @@
 import static org.hamcrest.Matchers.lessThan;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 
@@ -72,6 +74,11 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r
 
         assertEquals(200, response.getStatus());
         assertArrayEquals(data, response.getContent());
+        HttpFields responseHeaders = response.getHeaders();
+        // The content has been decoded, so Content-Encoding must be absent.
+        assertNull(responseHeaders.get(HttpHeader.CONTENT_ENCODING));
+        // The Content-Length must be the decoded one.
+        assertEquals(data.length, responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH));
     }
 
     @ParameterizedTest
@@ -297,13 +304,21 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r
 
         Response response = listener.get(20, TimeUnit.SECONDS);
         assertEquals(HttpStatus.OK_200, response.getStatus());
+        // No Content-Length because HttpClient does not know yet the length of the decoded content.
+        assertNull(response.getHeaders().get(HttpHeader.CONTENT_LENGTH));
+        // No Content-Encoding, because the content will be decoded automatically.
+        // In this way applications will know that the content is already un-gzipped
+        // and will not do the un-gzipping themselves.
+        assertNull(response.getHeaders().get(HttpHeader.CONTENT_ENCODING));
 
         ByteArrayOutputStream output = new ByteArrayOutputStream();
         try (InputStream input = listener.getInputStream())
         {
             IO.copy(input, output);
         }
         assertArrayEquals(content, output.toByteArray());
+        // After the content has been coded, the length is known again.
+        assertEquals(content.length, response.getHeaders().getLongField(HttpHeader.CONTENT_LENGTH));
     }
 
     private static void sleep(long ms) throws IOException