From a54527e243d02750098de7358f3e91e4330653dd Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 16 Aug 2023 18:57:03 +0200 Subject: [PATCH 1/6] Fixes #8926 - HttpClient GZIPContentDecoder should remove Content-Length 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 --- .../eclipse/jetty/client/ContentDecoder.java | 8 ++++ .../jetty/client/GZIPContentDecoder.java | 42 +++++++++++++++++++ .../eclipse/jetty/client/HttpReceiver.java | 37 ++++++++-------- .../eclipse/jetty/client/HttpResponse.java | 5 +++ .../org/eclipse/jetty/client/api/Request.java | 6 +++ .../eclipse/jetty/client/api/Response.java | 12 ++++++ .../jetty/client/HttpClientGZIPTest.java | 15 +++++++ 7 files changed, 106 insertions(+), 19 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java index 0c51cae37dee..98b6957b3ee3 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java @@ -22,6 +22,10 @@ */ public interface ContentDecoder { + public default void beforeDecoding(HttpExchange exchange) + { + } + /** *

Decodes the bytes in the given {@code buffer} and returns decoded bytes, if any.

* @@ -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()}. *

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java b/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java index 517edfd9861d..8acf93c39942 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java +++ b/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 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. */ diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java index 03a20de81cec..c57c5075d878 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java +++ b/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 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 responseListeners = exchange.getConversation().getResponseListeners(); notifier.notifyHeaders(responseListeners, response); contentListeners.reset(responseListeners); contentListeners.notifyBeforeContent(response); - if (!contentListeners.isEmpty()) - { - List 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(); } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java index a7b9a64600a0..a01a8d147672 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java +++ b/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(); diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java index e2919eb86b87..1766b0a35510 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Request.java +++ b/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); /** + *

Registers a listener for the headers event.

+ *

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()}.

+ * * @param listener a listener for response headers event * @return this request object */ diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java index fa5740b9d21e..fd5225ce70e0 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/api/Response.java @@ -65,6 +65,18 @@ public interface Response String getReason(); /** + *

Returns the headers of this response.

+ *

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.

+ *

Similarly, the {@code Content-Encoding} header + * may be removed or modified, as the content is + * decoded by the client.

+ * * @return the headers of this response */ HttpFields getHeaders(); diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java index 02e1cd408975..de7a8e7eabcc 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java +++ b/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,6 +304,12 @@ 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()) @@ -304,6 +317,8 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r 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 From eed61fa46028435981e1fac539b07a11ddabc71e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 17 Aug 2023 12:03:07 +0200 Subject: [PATCH 2/6] Updates after review. Signed-off-by: Simone Bordet --- .../eclipse/jetty/client/ContentDecoder.java | 16 +++++++ .../jetty/client/GZIPContentDecoder.java | 32 ++++++------- .../eclipse/jetty/client/HttpReceiver.java | 48 +++++++++++++------ .../http2/client/http/ContentLengthTest.java | 11 +++-- 4 files changed, 73 insertions(+), 34 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java index 98b6957b3ee3..5df48e441514 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java @@ -22,6 +22,14 @@ */ public interface ContentDecoder { + /** + *

Processes the exchange just before the decoding of the response content.

+ *

Typical processing may involve modifying the response headers, for example + * by temporarily removing the {@code Content-Length} header, or modifying the + * {@code Content-Encoding} header.

+ * + * @param exchange the exchange to process before decoding the response content + */ public default void beforeDecoding(HttpExchange exchange) { } @@ -43,6 +51,14 @@ public default void release(ByteBuffer decoded) { } + /** + *

Processes the exchange after the response content has been decoded.

+ *

Typical processing may involve modifying the response headers, for example + * updating the {@code Content-Length} header to the length of the decoded + * response content. + * + * @param exchange the exchange to process after decoding the response content + */ public default void afterDecoding(HttpExchange exchange) { } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java b/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java index 8acf93c39942..09fb88eecfcc 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java @@ -14,9 +14,7 @@ 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; @@ -50,23 +48,25 @@ public GZIPContentDecoder(ByteBufferPool byteBufferPool, int 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 fields = headers.listIterator(); - while (fields.hasNext()) + ListIterator iterator = exchange.getResponse().headers().listIterator(); + while (iterator.hasNext()) { - HttpField field = fields.next(); - if (field.getHeader() == HttpHeader.CONTENT_ENCODING) + HttpField field = iterator.next(); + HttpHeader header = field.getHeader(); + if (header == HttpHeader.CONTENT_LENGTH) + { + // Content-Length is not valid anymore while we are decoding. + iterator.remove(); + } + else if (header == HttpHeader.CONTENT_ENCODING) { - String value = Arrays.stream(field.getValues()) - .filter(encoding -> !"gzip".equalsIgnoreCase(encoding)) - .collect(Collectors.joining(", ")); - if (value.isEmpty()) - fields.remove(); + // Content-Encoding should be removed/modified as the content will be decoded. + String value = field.getValue(); + int comma = value.lastIndexOf(","); + if (comma < 0) + iterator.remove(); else - fields.set(new HttpField(HttpHeader.CONTENT_ENCODING, value)); + iterator.set(new HttpField(HttpHeader.CONTENT_ENCODING, value.substring(0, comma))); break; } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java index c57c5075d878..8f628edb4641 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java @@ -31,8 +31,11 @@ import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.QuotedCSV; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.MathUtils; @@ -286,22 +289,37 @@ protected boolean responseHeaders(HttpExchange exchange) return false; HttpResponse response = exchange.getResponse(); + HttpFields responseHeaders = response.getHeaders(); 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 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); + LOG.debug("Response headers {}{}{}", response, System.lineSeparator(), responseHeaders.toString().trim()); + + // HEAD responses may have Content-Encoding + // and Content-Length, but have no content. + if (!HttpMethod.HEAD.is(exchange.getRequest().getMethod())) + { + // Content-Encoding may have multiple values in the order they + // are applied, but we only support one decoding pass, the last one. + String contentEncoding = responseHeaders.get(HttpHeader.CONTENT_ENCODING); + if (contentEncoding != null) + { + int comma = contentEncoding.indexOf(","); + if (comma > 0) + { + QuotedCSV parser = new QuotedCSV(false); + parser.addValue(contentEncoding); + List values = parser.getValues(); + contentEncoding = values.get(values.size() - 1); + } + // If there is a matching content decoder factory, build a decoder. + for (ContentDecoder.Factory factory : getHttpDestination().getHttpClient().getContentDecoderFactories()) + { + if (factory.getEncoding().equalsIgnoreCase(contentEncoding)) + { + decoder = new Decoder(exchange, factory.newContentDecoder()); + break; + } + } + } } ResponseNotifier notifier = getHttpDestination().getResponseNotifier(); diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ContentLengthTest.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ContentLengthTest.java index 560ff4a37819..3bf436a3ff88 100644 --- a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ContentLengthTest.java +++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/ContentLengthTest.java @@ -20,13 +20,15 @@ import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.gzip.GzipHandler; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.lessThan; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; public class ContentLengthTest extends AbstractTest { @@ -94,7 +96,10 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r .send(); HttpFields responseHeaders = response.getHeaders(); - long contentLength = responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH.asString()); - assertTrue(0 < contentLength && contentLength < data.length); + long contentLength = responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH); + if (HttpMethod.HEAD.is(method)) + assertThat(contentLength, lessThan((long)data.length)); + else + assertEquals(data.length, contentLength); } } From e0e304ef91a46a88ff58939c8a71758f1db763ec Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 17 Aug 2023 12:24:32 +0200 Subject: [PATCH 3/6] Updates after review. Signed-off-by: Simone Bordet --- .../jetty/client/GZIPContentDecoder.java | 48 ++++++------ .../eclipse/jetty/client/HttpResponse.java | 5 -- .../jetty/client/HttpClientGZIPTest.java | 73 +++++++++++++++++++ 3 files changed, 99 insertions(+), 27 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java b/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java index 09fb88eecfcc..f34860db63ad 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java @@ -17,7 +17,6 @@ import java.util.ListIterator; import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.io.ByteBufferPool; @@ -48,28 +47,31 @@ public GZIPContentDecoder(ByteBufferPool byteBufferPool, int bufferSize) @Override public void beforeDecoding(HttpExchange exchange) { - ListIterator iterator = exchange.getResponse().headers().listIterator(); - while (iterator.hasNext()) + exchange.getResponse().headers(headers -> { - HttpField field = iterator.next(); - HttpHeader header = field.getHeader(); - if (header == HttpHeader.CONTENT_LENGTH) + ListIterator iterator = headers.listIterator(); + while (iterator.hasNext()) { - // Content-Length is not valid anymore while we are decoding. - iterator.remove(); - } - else if (header == HttpHeader.CONTENT_ENCODING) - { - // Content-Encoding should be removed/modified as the content will be decoded. - String value = field.getValue(); - int comma = value.lastIndexOf(","); - if (comma < 0) + HttpField field = iterator.next(); + HttpHeader header = field.getHeader(); + if (header == HttpHeader.CONTENT_LENGTH) + { + // Content-Length is not valid anymore while we are decoding. iterator.remove(); - else - iterator.set(new HttpField(HttpHeader.CONTENT_ENCODING, value.substring(0, comma))); - break; + } + else if (header == HttpHeader.CONTENT_ENCODING) + { + // Content-Encoding should be removed/modified as the content will be decoded. + String value = field.getValue(); + int comma = value.lastIndexOf(","); + if (comma < 0) + iterator.remove(); + else + iterator.set(new HttpField(HttpHeader.CONTENT_ENCODING, value.substring(0, comma))); + break; + } } - } + }); } @Override @@ -83,9 +85,11 @@ protected boolean decodedChunk(ByteBuffer chunk) @Override public void afterDecoding(HttpExchange exchange) { - HttpFields.Mutable headers = exchange.getResponse().headers(); - headers.remove(HttpHeader.TRANSFER_ENCODING); - headers.putLongField(HttpHeader.CONTENT_LENGTH, decodedLength); + exchange.getResponse().headers(headers -> + { + headers.remove(HttpHeader.TRANSFER_ENCODING); + headers.putLongField(HttpHeader.CONTENT_LENGTH, decodedLength); + }); } /** diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java index a01a8d147672..a7b9a64600a0 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpResponse.java @@ -87,11 +87,6 @@ public HttpFields getHeaders() return headers.asImmutable(); } - protected HttpFields.Mutable headers() - { - return headers; - } - public void clearHeaders() { headers.clear(); diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java index de7a8e7eabcc..6300d4fe31e7 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.client; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -22,6 +23,8 @@ import java.util.Random; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletRequest; @@ -81,6 +84,76 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r assertEquals(data.length, responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH)); } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testMultipleContentEncodingsFooGZIP(Scenario scenario) throws Exception + { + final byte[] data = "HELLO WORLD".getBytes(StandardCharsets.UTF_8); + start(scenario, new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setHeader("Content-Encoding", "foo,gzip"); + GZIPOutputStream gzipOutput = new GZIPOutputStream(response.getOutputStream()); + gzipOutput.write(data); + gzipOutput.finish(); + } + }); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(200, response.getStatus()); + assertArrayEquals(data, response.getContent()); + HttpFields responseHeaders = response.getHeaders(); + // The content has been decoded, so Content-Encoding must be only be "foo". + assertEquals("foo", responseHeaders.get(HttpHeader.CONTENT_ENCODING)); + // The Content-Length must be the decoded one. + assertEquals(data.length, responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH)); + } + + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testMultipleContentEncodingsGZIPFoo(Scenario scenario) throws Exception + { + final byte[] data = "HELLO WORLD".getBytes(StandardCharsets.UTF_8); + start(scenario, new EmptyServerHandler() + { + @Override + protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + response.setHeader("Content-Encoding", "gzip,foo"); + GZIPOutputStream gzipOutput = new GZIPOutputStream(response.getOutputStream()); + gzipOutput.write(data); + gzipOutput.finish(); + } + }); + + // There is no "foo" content decoder factory, so content must remain gzipped. + AtomicLong encodedContentLength = new AtomicLong(); + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .onResponseHeader((r, field) -> + { + if (field.getHeader() == HttpHeader.CONTENT_LENGTH) + encodedContentLength.set(field.getLongValue()); + return true; + }) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(200, response.getStatus()); + + byte[] content = IO.readBytes(new GZIPInputStream(new ByteArrayInputStream(response.getContent()))); + assertArrayEquals(data, content); + HttpFields responseHeaders = response.getHeaders(); + assertEquals("gzip,foo", responseHeaders.get(HttpHeader.CONTENT_ENCODING)); + assertEquals(encodedContentLength.get(), responseHeaders.getLongField(HttpHeader.CONTENT_LENGTH)); + } + @ParameterizedTest @ArgumentsSource(ScenarioProvider.class) public void testGZIPContentOneByteAtATime(Scenario scenario) throws Exception From e54187c73de61adfd92e239e9bcf8101acd26231 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 17 Aug 2023 13:49:01 +0200 Subject: [PATCH 4/6] Fixed test. Signed-off-by: Simone Bordet --- .../eclipse/jetty/demos/ManyHandlersTest.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/demos/embedded/src/test/java/org/eclipse/jetty/demos/ManyHandlersTest.java b/demos/embedded/src/test/java/org/eclipse/jetty/demos/ManyHandlersTest.java index dd9329dba47a..3dda46ad6342 100644 --- a/demos/embedded/src/test/java/org/eclipse/jetty/demos/ManyHandlersTest.java +++ b/demos/embedded/src/test/java/org/eclipse/jetty/demos/ManyHandlersTest.java @@ -15,10 +15,10 @@ import java.net.URI; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Server; @@ -53,9 +53,15 @@ public void testGetParams() throws Exception { URI uri = server.getURI().resolve("/params?a=b&foo=bar"); + AtomicReference contentEncoding = new AtomicReference<>(); ContentResponse response = client.newRequest(uri) .method(HttpMethod.GET) - .headers(headers -> headers.put(HttpHeader.ACCEPT_ENCODING, HttpHeaderValue.GZIP)) + .onResponseHeader((r, field) -> + { + if (field.getHeader() == HttpHeader.CONTENT_ENCODING) + contentEncoding.set(field.getValue()); + return true; + }) .send(); assertThat("HTTP Response Status", response.getStatus(), is(HttpStatus.OK_200)); @@ -63,8 +69,7 @@ public void testGetParams() throws Exception // test gzip // Test that Gzip was used to produce the response - String contentEncoding = response.getHeaders().get(HttpHeader.CONTENT_ENCODING); - assertThat("Content-Encoding", contentEncoding, containsString("gzip")); + assertThat("Content-Encoding", contentEncoding.get(), containsString("gzip")); // test response content String responseBody = response.getContentAsString(); @@ -78,18 +83,25 @@ public void testGetParams() throws Exception public void testGetHello() throws Exception { URI uri = server.getURI().resolve("/hello"); + + AtomicReference contentEncoding = new AtomicReference<>(); ContentResponse response = client.newRequest(uri) .method(HttpMethod.GET) - .headers(headers -> headers.put(HttpHeader.ACCEPT_ENCODING, HttpHeaderValue.GZIP)) + .onResponseHeader((r, field) -> + { + if (field.getHeader() == HttpHeader.CONTENT_ENCODING) + contentEncoding.set(field.getValue()); + return true; + }) .send(); + assertThat("HTTP Response Status", response.getStatus(), is(HttpStatus.OK_200)); // dumpResponseHeaders(response); // test gzip // Test that Gzip was used to produce the response - String contentEncoding = response.getHeaders().get(HttpHeader.CONTENT_ENCODING); - assertThat("Content-Encoding", contentEncoding, containsString("gzip")); + assertThat("Content-Encoding", contentEncoding.get(), containsString("gzip")); // test expected header from wrapper String welcome = response.getHeaders().get("X-Welcome"); From 6c906965557c4921dd7fef32e2fa0969a48a10b1 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 17 Aug 2023 15:55:09 +0200 Subject: [PATCH 5/6] Fixed test. Signed-off-by: Simone Bordet --- .../jetty/tests/distribution/DemoModulesTests.java | 5 ----- .../jetty/tests/distribution/GzipModuleTests.java | 9 +++------ 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DemoModulesTests.java b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DemoModulesTests.java index a5bd12439a9d..e8539ea509b5 100644 --- a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DemoModulesTests.java +++ b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/DemoModulesTests.java @@ -105,7 +105,6 @@ public void testJspDump() throws Exception String[] argsStart = { "jetty.http.port=" + httpPort, - "jetty.httpConfig.port=" + httpsPort, "jetty.ssl.port=" + httpsPort }; @@ -149,7 +148,6 @@ public void testAsyncRest() throws Exception String[] argsStart = { "jetty.http.port=" + httpPort, - "jetty.httpConfig.port=" + httpsPort, "jetty.ssl.port=" + httpsPort }; @@ -205,7 +203,6 @@ public void testSpec() throws Exception String[] argsStart = { "jetty.http.port=" + httpPort, - "jetty.httpConfig.port=" + httpsPort, "jetty.ssl.port=" + httpsPort }; @@ -260,7 +257,6 @@ public void testJPMS() throws Exception String[] argsStart = { "--jpms", "jetty.http.port=" + httpPort, - "jetty.httpConfig.port=" + httpsPort, "jetty.ssl.port=" + httpsPort }; try (JettyHomeTester.Run runStart = distribution.start(argsStart)) @@ -301,7 +297,6 @@ public void testSessionDump() throws Exception int httpsPort = distribution.freePort(); String[] argsStart = { "jetty.http.port=" + httpPort, - "jetty.httpConfig.port=" + httpsPort, "jetty.ssl.port=" + httpsPort }; try (JettyHomeTester.Run runStart = distribution.start(argsStart)) diff --git a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/GzipModuleTests.java b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/GzipModuleTests.java index 0c5155a2a792..5b465e0e8152 100644 --- a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/GzipModuleTests.java +++ b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/GzipModuleTests.java @@ -55,8 +55,7 @@ public void testGzipDefault() throws Exception assertEquals(0, runConfig.getExitValue()); String[] argsStart = { - "jetty.http.port=" + httpPort, - "jetty.httpConfig.port=" + httpPort + "jetty.http.port=" + httpPort }; File war = distribution.resolveArtifact("org.eclipse.jetty.demos:demo-simple-webapp:war:" + jettyVersion); @@ -70,7 +69,7 @@ public void testGzipDefault() throws Exception ContentResponse response = client.GET("http://localhost:" + httpPort + "/demo/index.html"); String responseDetails = toResponseDetails(response); assertEquals(HttpStatus.OK_200, response.getStatus(), responseDetails); - assertThat(responseDetails, response.getHeaders().get(HttpHeader.CONTENT_ENCODING), containsString("gzip")); + assertThat(responseDetails, containsString("Hello")); } } } @@ -99,8 +98,7 @@ public void testGzipDefaultExcludedMimeType() throws Exception assertEquals(0, runConfig.getExitValue()); String[] argsStart = { - "jetty.http.port=" + httpPort, - "jetty.httpConfig.port=" + httpPort + "jetty.http.port=" + httpPort }; File war = distribution.resolveArtifact("org.eclipse.jetty.demos:demo-simple-webapp:war:" + jettyVersion); @@ -145,7 +143,6 @@ public void testGzipAddWebappSpecificExcludeMimeType() throws Exception String[] argsStart = { "jetty.http.port=" + httpPort, - "jetty.httpConfig.port=" + httpPort, "jetty.gzip.excludedMimeTypeList=image/vnd.microsoft.icon" }; From d9f9de7e04b0dd711d12b7bd3d7caad4fc15b28e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 21 Aug 2023 08:57:24 +0200 Subject: [PATCH 6/6] Fixed typo from review. Signed-off-by: Simone Bordet --- .../test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java index 6300d4fe31e7..6daa5d003c4e 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientGZIPTest.java @@ -390,7 +390,7 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r IO.copy(input, output); } assertArrayEquals(content, output.toByteArray()); - // After the content has been coded, the length is known again. + // After the content has been decoded, the length is known again. assertEquals(content.length, response.getHeaders().getLongField(HttpHeader.CONTENT_LENGTH)); }