From f53dd6371022338073c9f3ea525f294ca3c6e18f Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 5 Dec 2024 13:37:21 +0100 Subject: [PATCH 01/20] Fixes #12612 - Use Compression classes for client decoding. Now using Compression classes for client response decoding. Removed oej.client.GZIPContentDecoder, now using only oeh.http.GZIPContentDecoder where necessary. Signed-off-by: Simone Bordet --- jetty-core/jetty-client/pom.xml | 8 + .../src/main/java/module-info.java | 2 + .../eclipse/jetty/client/ContentDecoder.java | 100 ++++---- .../jetty/client/GZIPContentDecoder.java | 135 ----------- .../org/eclipse/jetty/client/HttpClient.java | 25 +- .../jetty/client/transport/HttpReceiver.java | 218 +++++++++--------- .../internal/HttpReceiverOverHTTP.java | 2 +- ...HttpClientContentDecoderFactoriesTest.java | 48 ++-- .../brotli/internal/BrotliDecoderSource.java | 13 +- .../jetty/compression/DecoderSource.java | 116 +--------- .../compression/gzip/GzipCompression.java | 2 +- .../compression/gzip/GzipDecoderConfig.java | 10 +- .../gzip/internal/GzipDecoderSource.java | 90 ++++---- .../zstandard/ZstandardCompression.java | 2 +- .../internal/ZstandardDecoderSource.java | 16 +- .../io/content/ContentSourceTransformer.java | 153 +++++++----- .../io/ContentSourceTransformerTest.java | 14 +- .../ee10/proxy/AsyncMiddleManServlet.java | 23 +- .../ee11/proxy/AsyncMiddleManServlet.java | 23 +- .../ee9/proxy/AsyncMiddleManServlet.java | 23 +- 20 files changed, 454 insertions(+), 569 deletions(-) delete mode 100644 jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java diff --git a/jetty-core/jetty-client/pom.xml b/jetty-core/jetty-client/pom.xml index a04f1b5cf9b7..2143de36defa 100644 --- a/jetty-core/jetty-client/pom.xml +++ b/jetty-core/jetty-client/pom.xml @@ -33,6 +33,14 @@ jetty-jmx true + + org.eclipse.jetty.compression + jetty-compression-common + + + org.eclipse.jetty.compression + jetty-compression-gzip + org.slf4j slf4j-api diff --git a/jetty-core/jetty-client/src/main/java/module-info.java b/jetty-core/jetty-client/src/main/java/module-info.java index ab16064533c2..69e26d46e12f 100644 --- a/jetty-core/jetty-client/src/main/java/module-info.java +++ b/jetty-core/jetty-client/src/main/java/module-info.java @@ -16,6 +16,8 @@ requires org.eclipse.jetty.alpn.client; requires org.slf4j; + requires transitive org.eclipse.jetty.compression; + requires transitive org.eclipse.jetty.compression.gzip; requires transitive org.eclipse.jetty.http; // Only required if using JMX. diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java index 474d3515fd1c..2e94348f70fb 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/ContentDecoder.java @@ -13,68 +13,50 @@ package org.eclipse.jetty.client; -import java.nio.ByteBuffer; +import java.text.DecimalFormat; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Objects; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.io.RetainableByteBuffer; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.util.component.ContainerLifeCycle; /** - * {@link ContentDecoder} decodes content bytes of a response. + *

Groups abstractions related to response content decoding.

* * @see Factory + * @see Factories + * @see HttpClient#getContentDecoderFactories() */ public interface ContentDecoder { /** - *

Processes the response 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.

+ *

A factory for {@link Content.Source} that decode response content.

+ *

A {@code Factory} has an {@link #getEncoding() encoding} and a + * {@link #getWeight() weight} that are used in the {@code Accept-Encoding} + * request header and in the {@code Content-Encoding} response headers.

+ *

{@code Factory} instances are configured in {@link HttpClient} via + * {@link HttpClient#getContentDecoderFactories()}.

*/ - public default void beforeDecoding(Response response) - { - } - - /** - *

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

- *

The returned {@link RetainableByteBuffer} will eventually be released via - * {@link RetainableByteBuffer#release()} by the code that called this method.

- * - * @param buffer the buffer containing encoded bytes - * @return a buffer containing decoded bytes - */ - public abstract RetainableByteBuffer decode(ByteBuffer buffer); - - /** - *

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. - */ - public default void afterDecoding(Response response) - { - } - - /** - * Factory for {@link ContentDecoder}s; subclasses must implement {@link #newContentDecoder()}. - *

- * {@link Factory} have an {@link #getEncoding() encoding}, which is the string used in - * {@code Accept-Encoding} request header and in {@code Content-Encoding} response headers. - *

- * {@link Factory} instances are configured in {@link HttpClient} via - * {@link HttpClient#getContentDecoderFactories()}. - */ - public abstract static class Factory + abstract class Factory extends ContainerLifeCycle { private final String encoding; + private final float weight; protected Factory(String encoding) { - this.encoding = encoding; + this(encoding, -1F); + } + + protected Factory(String encoding, float weight) + { + this.encoding = Objects.requireNonNull(encoding); + if (weight != -1F && !(weight >= 0F && weight <= 1F)) + throw new IllegalArgumentException("Invalid weight: " + weight); + this.weight = weight; } /** @@ -85,6 +67,14 @@ public String getEncoding() return encoding; } + /** + * @return the weight (between 0 and 1, at most 3 decimal digits) to use for the {@code Accept-Encoding} request header + */ + public float getWeight() + { + return weight; + } + @Override public boolean equals(Object obj) { @@ -102,14 +92,16 @@ public int hashCode() } /** - * Factory method for {@link ContentDecoder}s + *

Creates a {@link Content.Source} that decodes the + * chunks of the given {@link Content.Source} parameter.

* - * @return a new instance of a {@link ContentDecoder} + * @param contentSource the encoded {@link Content.Source} + * @return the decoded {@link Content.Source} */ - public abstract ContentDecoder newContentDecoder(); + public abstract Content.Source newDecoderContentSource(Content.Source contentSource); } - public static class Factories implements Iterable + class Factories extends ContainerLifeCycle implements Iterable { private final Map factories = new LinkedHashMap<>(); private HttpField acceptEncodingField; @@ -134,8 +126,20 @@ public void clear() public Factory put(Factory factory) { Factory result = factories.put(factory.getEncoding(), factory); - String value = String.join(",", factories.keySet()); - acceptEncodingField = new HttpField(HttpHeader.ACCEPT_ENCODING, value); + updateBean(result, factory, true); + + StringBuilder header = new StringBuilder(); + factories.forEach((encoding, value) -> + { + if (!header.isEmpty()) + header.append(", "); + header.append(encoding); + float weight = value.getWeight(); + if (weight != -1F) + header.append(";q=").append(new DecimalFormat("#.###").format(weight)); + }); + acceptEncodingField = new HttpField(HttpHeader.ACCEPT_ENCODING, header.toString()); + return result; } } diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java deleted file mode 100644 index 95ad573cfec8..000000000000 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/GZIPContentDecoder.java +++ /dev/null @@ -1,135 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.client; - -import java.util.ListIterator; - -import org.eclipse.jetty.client.transport.HttpResponse; -import org.eclipse.jetty.http.HttpField; -import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.io.ByteBufferPool; -import org.eclipse.jetty.io.RetainableByteBuffer; -import org.eclipse.jetty.util.IO; - -/** - * {@link ContentDecoder} for the "gzip" encoding. - */ -public class GZIPContentDecoder extends org.eclipse.jetty.http.GZIPContentDecoder implements ContentDecoder -{ - public static final int DEFAULT_BUFFER_SIZE = IO.DEFAULT_BUFFER_SIZE; - - private long decodedLength; - - public GZIPContentDecoder() - { - this(DEFAULT_BUFFER_SIZE); - } - - public GZIPContentDecoder(int bufferSize) - { - this(null, bufferSize); - } - - public GZIPContentDecoder(ByteBufferPool byteBufferPool, int bufferSize) - { - super(byteBufferPool, bufferSize); - } - - @Override - public void beforeDecoding(Response response) - { - HttpResponse httpResponse = (HttpResponse)response; - httpResponse.headers(headers -> - { - boolean seenContentEncoding = false; - for (ListIterator iterator = headers.listIterator(headers.size()); iterator.hasPrevious();) - { - HttpField field = iterator.previous(); - 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 && !seenContentEncoding) - { - // Last Content-Encoding should be removed/modified as the content will be decoded. - seenContentEncoding = true; - 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))); - } - } - }); - } - - @Override - protected boolean decodedChunk(RetainableByteBuffer chunk) - { - decodedLength += chunk.remaining(); - super.decodedChunk(chunk); - return true; - } - - @Override - public void afterDecoding(Response response) - { - HttpResponse httpResponse = (HttpResponse)response; - httpResponse.headers(headers -> - { - headers.remove(HttpHeader.TRANSFER_ENCODING); - headers.put(HttpHeader.CONTENT_LENGTH, decodedLength); - }); - } - - /** - * Specialized {@link ContentDecoder.Factory} for the "gzip" encoding. - */ - public static class Factory extends ContentDecoder.Factory - { - private final ByteBufferPool byteBufferPool; - private final int bufferSize; - - public Factory() - { - this(DEFAULT_BUFFER_SIZE); - } - - public Factory(int bufferSize) - { - this(null, bufferSize); - } - - public Factory(ByteBufferPool byteBufferPool) - { - this(byteBufferPool, DEFAULT_BUFFER_SIZE); - } - - public Factory(ByteBufferPool byteBufferPool, int bufferSize) - { - super("gzip"); - this.byteBufferPool = byteBufferPool; - this.bufferSize = bufferSize; - } - - @Override - public ContentDecoder newContentDecoder() - { - return new GZIPContentDecoder(byteBufferPool, bufferSize); - } - } -} diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 1d5426e7f588..3415c21ad528 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -37,6 +37,8 @@ import org.eclipse.jetty.client.transport.HttpConversation; import org.eclipse.jetty.client.transport.HttpDestination; import org.eclipse.jetty.client.transport.HttpRequest; +import org.eclipse.jetty.compression.Compression; +import org.eclipse.jetty.compression.gzip.GzipCompression; import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpCookieStore; @@ -50,6 +52,7 @@ import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.ClientConnectionFactory; import org.eclipse.jetty.io.ClientConnector; +import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.Transport; import org.eclipse.jetty.io.ssl.SslClientConnectionFactory; import org.eclipse.jetty.util.Fields; @@ -226,7 +229,9 @@ protected void doStart() throws Exception handlers.put(new ProxyAuthenticationProtocolHandler(this)); handlers.put(new UpgradeProtocolHandler()); - decoderFactories.put(new GZIPContentDecoder.Factory(byteBufferPool)); +// TypeUtil.serviceStream(ServiceLoader.load(Compression.class)) +// .forEach(c -> decoderFactories.put(c)); + decoderFactories.put(new CompressionContentDecoderFactory(new GzipCompression())); if (cookieStore == null) cookieStore = new HttpCookieStore.Default(); @@ -1181,4 +1186,22 @@ public interface Aware { void setHttpClient(HttpClient httpClient); } + + private static class CompressionContentDecoderFactory extends ContentDecoder.Factory + { + private final Compression compression; + + protected CompressionContentDecoderFactory(Compression compression) + { + super(compression.getEncodingName()); + this.compression = Objects.requireNonNull(compression); + installBean(compression); + } + + @Override + public Content.Source newDecoderContentSource(Content.Source contentSource) + { + return compression.newDecoderSource(contentSource); + } + } } diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpReceiver.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpReceiver.java index 6dd4666b52cc..e0184631c611 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpReceiver.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpReceiver.java @@ -15,6 +15,7 @@ import java.net.URI; import java.util.List; +import java.util.ListIterator; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicReference; @@ -30,12 +31,10 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.QuotedCSV; import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.io.RetainableByteBuffer; -import org.eclipse.jetty.io.content.ContentSourceTransformer; import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.Promise; -import org.eclipse.jetty.util.component.Destroyable; import org.eclipse.jetty.util.thread.AutoLock; +import org.eclipse.jetty.util.thread.Invocable; import org.eclipse.jetty.util.thread.SerializedInvoker; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -71,7 +70,8 @@ public abstract class HttpReceiver private final HttpChannel channel; private final SerializedInvoker invoker; private ResponseState responseState = ResponseState.IDLE; - private NotifiableContentSource contentSource; + private ContentSource rawContentSource; + private Content.Source contentSource; private Throwable failure; protected HttpReceiver(HttpChannel channel) @@ -259,7 +259,7 @@ protected void responseHeaders(HttpExchange exchange) // HEAD responses may have Content-Encoding // and Content-Length, but have no content. - ContentDecoder decoder = null; + ContentDecoder.Factory decoderFactory = null; if (!HttpMethod.HEAD.is(exchange.getRequest().getMethod())) { // Content-Encoding may have multiple values in the order they @@ -279,8 +279,8 @@ protected void responseHeaders(HttpExchange exchange) { if (factory.getEncoding().equalsIgnoreCase(contentEncoding)) { - decoder = factory.newContentDecoder(); - decoder.beforeDecoding(response); + decoderFactory = factory; + beforeDecoding(response, contentEncoding); break; } } @@ -301,12 +301,17 @@ protected void responseHeaders(HttpExchange exchange) } responseState = ResponseState.CONTENT; - if (contentSource != null) + if (rawContentSource != null) throw new IllegalStateException(); - contentSource = new ContentSource(); + rawContentSource = new ContentSource(); + contentSource = rawContentSource; - if (decoder != null) - contentSource = new DecodingContentSource(contentSource, invoker, decoder, response); + if (decoderFactory != null) + { + if (LOG.isDebugEnabled()) + LOG.debug("Decoding {} response content", decoderFactory.getEncoding()); + contentSource = new DecodedContentSource(decoderFactory.newDecoderContentSource(rawContentSource), response); + } if (LOG.isDebugEnabled()) LOG.debug("Response content {} {}", response, contentSource); @@ -335,7 +340,7 @@ protected void responseContentAvailable(HttpExchange exchange) if (exchange.isResponseCompleteOrTerminated()) return; - contentSource.onDataAvailable(); + rawContentSource.onDataAvailable(); }); } @@ -472,8 +477,7 @@ protected void dispose() private void cleanup() { - if (contentSource != null) - contentSource.destroy(); + rawContentSource = null; contentSource = null; } @@ -499,7 +503,7 @@ public void abort(HttpExchange exchange, Throwable failure, Promise pro responseState = ResponseState.FAILURE; this.failure = failure; if (contentSource != null) - contentSource.error(failure); + contentSource.fail(failure); dispose(); HttpResponse response = exchange.getResponse(); @@ -514,6 +518,48 @@ public void abort(HttpExchange exchange, Throwable failure, Promise pro }); } + private void beforeDecoding(Response response, String contentEncoding) + { + HttpResponse httpResponse = (HttpResponse)response; + httpResponse.headers(headers -> + { + boolean seenContentEncoding = false; + for (ListIterator iterator = headers.listIterator(headers.size()); iterator.hasPrevious();) + { + HttpField field = iterator.previous(); + 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 && !seenContentEncoding) + { + // Last Content-Encoding should be removed/modified as the content will be decoded. + seenContentEncoding = true; + // TODO: introduce HttpFields.removeLast() or similar. + // Use the contentEncoding parameter. + 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))); + } + } + }); + } + + private void afterDecoding(Response response, long decodedLength) + { + HttpResponse httpResponse = (HttpResponse)response; + httpResponse.headers(headers -> + { + headers.remove(HttpHeader.TRANSFER_ENCODING); + headers.put(HttpHeader.CONTENT_LENGTH, decodedLength); + }); + } + @Override public String toString() { @@ -556,129 +602,77 @@ private enum ResponseState FAILURE } - private interface NotifiableContentSource extends Content.Source, Destroyable - { - boolean error(Throwable failure); - - void onDataAvailable(); - - @Override - default void destroy() - { - } - } - - private static class DecodingContentSource extends ContentSourceTransformer implements NotifiableContentSource + private class DecodedContentSource implements Content.Source { - private static final Logger LOG = LoggerFactory.getLogger(DecodingContentSource.class); + private static final Logger LOG = LoggerFactory.getLogger(DecodedContentSource.class); - private final ContentDecoder _decoder; - private final Response _response; - private volatile Content.Chunk _chunk; + private final Content.Source source; + private final Response response; + private long decodedLength; - private DecodingContentSource(NotifiableContentSource rawSource, SerializedInvoker invoker, ContentDecoder decoder, Response response) - { - super(rawSource, invoker); - _decoder = decoder; - _response = response; - } - - @Override - protected NotifiableContentSource getContentSource() + private DecodedContentSource(Content.Source source, Response response) { - return (NotifiableContentSource)super.getContentSource(); + this.source = source; + this.response = response; } @Override - public void onDataAvailable() + public long getLength() { - getContentSource().onDataAvailable(); + return source.getLength(); } @Override - protected Content.Chunk transform(Content.Chunk inputChunk) + public Content.Chunk read() { while (true) { - boolean retain = _chunk == null; + Content.Chunk chunk = source.read(); + if (LOG.isDebugEnabled()) - LOG.debug("input: {}, chunk: {}, retain? {}", inputChunk, _chunk, retain); - if (_chunk == null) - _chunk = inputChunk; - if (_chunk == null) + LOG.debug("Decoded chunk {}", chunk); + + if (chunk == null) return null; - if (Content.Chunk.isFailure(_chunk)) + + if (chunk.isEmpty() && !chunk.isLast()) { - Content.Chunk failure = _chunk; - _chunk = Content.Chunk.next(failure); - return failure; + chunk.release(); + continue; } - // Retain the input chunk because its ByteBuffer will be referenced by the Inflater. - if (retain) - _chunk.retain(); - if (LOG.isDebugEnabled()) - LOG.debug("decoding: {}", _chunk); - RetainableByteBuffer decodedBuffer = _decoder.decode(_chunk.getByteBuffer()); - if (LOG.isDebugEnabled()) - LOG.debug("decoded: {}", decodedBuffer); + decodedLength += chunk.remaining(); - if (decodedBuffer != null && decodedBuffer.hasRemaining()) - { - // The decoded ByteBuffer is a transformed "copy" of the - // compressed one, so it has its own reference counter. - if (decodedBuffer.canRetain()) - { - if (LOG.isDebugEnabled()) - LOG.debug("returning decoded content"); - return Content.Chunk.asChunk(decodedBuffer.getByteBuffer(), false, decodedBuffer); - } - else - { - if (LOG.isDebugEnabled()) - LOG.debug("returning non-retainable decoded content"); - return Content.Chunk.from(decodedBuffer.getByteBuffer(), false); - } - } - else - { - if (LOG.isDebugEnabled()) - LOG.debug("decoding produced no content"); - if (decodedBuffer != null) - decodedBuffer.release(); + if (chunk.isLast()) + afterDecoding(response, decodedLength); - if (!_chunk.hasRemaining()) - { - Content.Chunk result = _chunk.isLast() ? Content.Chunk.EOF : null; - if (LOG.isDebugEnabled()) - LOG.debug("Could not decode more from this chunk, releasing it, r={}", result); - _chunk.release(); - _chunk = null; - return result; - } - else - { - if (LOG.isDebugEnabled()) - LOG.debug("retrying transformation"); - } - } + return chunk; } } @Override - public boolean error(Throwable failure) + public void demand(Runnable demandCallback) + { + Runnable demand = new Invocable.ReadyTask(Invocable.getInvocationType(demandCallback), () -> invoker.run(demandCallback)); + source.demand(demand); + } + + @Override + public void fail(Throwable failure) + { + source.fail(failure); + } + + @Override + public void fail(Throwable failure, boolean last) { - if (_chunk != null) - _chunk.release(); - _chunk = null; - return getContentSource().error(failure); + source.fail(failure, last); } @Override - public void destroy() + public boolean rewind() { - _decoder.afterDecoding(_response); - getContentSource().destroy(); + return source.rewind(); } } @@ -686,7 +680,7 @@ public void destroy() * This Content.Source implementation guarantees that all {@link #read(boolean)} calls * happening from a {@link #demand(Runnable)} callback must be serialized. */ - private class ContentSource implements NotifiableContentSource + private class ContentSource implements Content.Source { private static final Logger LOG = LoggerFactory.getLogger(ContentSource.class); @@ -729,8 +723,7 @@ public Content.Chunk read() } } - @Override - public void onDataAvailable() + private void onDataAvailable() { if (LOG.isDebugEnabled()) LOG.debug("onDataAvailable on {}", this); @@ -826,8 +819,7 @@ public void fail(Throwable failure) invokeDemandCallback(true); } - @Override - public boolean error(Throwable failure) + private boolean error(Throwable failure) { if (LOG.isDebugEnabled()) LOG.debug("Erroring {}", this); diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpReceiverOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpReceiverOverHTTP.java index da1f86f047e8..29ea530759e8 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpReceiverOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpReceiverOverHTTP.java @@ -524,7 +524,7 @@ private void receiveNext() throw new IllegalStateException(); if (LOG.isDebugEnabled()) - LOG.debug("Receiving next request in {}", this); + LOG.debug("Receiving next response in {}", this); boolean setFillInterest = parseAndFill(true); if (!hasContent() && setFillInterest) fillInterested(); diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentDecoderFactoriesTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentDecoderFactoriesTest.java index fbc58bc298e8..361696865e82 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentDecoderFactoriesTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentDecoderFactoriesTest.java @@ -18,11 +18,11 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.io.ArrayByteBufferPool; -import org.eclipse.jetty.io.RetainableByteBuffer; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.content.ContentSourceTransformer; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; -import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.StringUtil; import org.junit.jupiter.params.ParameterizedTest; @@ -53,19 +53,24 @@ public boolean handle(Request request, Response response, Callback callback) client.getContentDecoderFactories().put(new ContentDecoder.Factory("UPPERCASE") { @Override - public ContentDecoder newContentDecoder() + public Content.Source newDecoderContentSource(Content.Source source) { - return byteBuffer -> + return new ContentSourceTransformer(source) { - byte b = byteBuffer.get(); - if (b == '*') - return bufferPool.acquire(0, true); + @Override + protected Content.Chunk transform(Content.Chunk chunk) + { + if (chunk.isEmpty()) + return chunk.isLast() ? Content.Chunk.EOF : Content.Chunk.EMPTY; - RetainableByteBuffer buffer = bufferPool.acquire(1, true); - int pos = BufferUtil.flipToFill(buffer.getByteBuffer()); - buffer.getByteBuffer().put(StringUtil.asciiToLowerCase(b)); - BufferUtil.flipToFlush(buffer.getByteBuffer(), pos); - return buffer; + ByteBuffer byteBuffer = chunk.getByteBuffer(); + byte b = byteBuffer.get(); + if (b == '*') + return Content.Chunk.EMPTY; + + byte lower = StringUtil.asciiToLowerCase(b); + return Content.Chunk.from(ByteBuffer.wrap(new byte[]{lower}), false); + } }; } }); @@ -97,13 +102,22 @@ public boolean handle(Request request, Response response, Callback callback) client.getContentDecoderFactories().put(new ContentDecoder.Factory("UPPERCASE") { @Override - public ContentDecoder newContentDecoder() + public Content.Source newDecoderContentSource(Content.Source source) { - return byteBuffer -> + return new ContentSourceTransformer(source) { - String uppercase = US_ASCII.decode(byteBuffer).toString(); - String lowercase = StringUtil.asciiToLowerCase(uppercase); - return RetainableByteBuffer.wrap(ByteBuffer.wrap(lowercase.getBytes(US_ASCII))); + @Override + protected Content.Chunk transform(Content.Chunk chunk) + { + if (chunk.isEmpty()) + return chunk.isLast() ? Content.Chunk.EOF : Content.Chunk.EMPTY; + + ByteBuffer byteBuffer = chunk.getByteBuffer(); + String upperCase = US_ASCII.decode(byteBuffer).toString(); + String lowerCase = StringUtil.asciiToLowerCase(upperCase); + + return Content.Chunk.from(US_ASCII.encode(lowerCase), false); + } }; } }); diff --git a/jetty-core/jetty-compression/jetty-compression-brotli/src/main/java/org/eclipse/jetty/compression/brotli/internal/BrotliDecoderSource.java b/jetty-core/jetty-compression/jetty-compression-brotli/src/main/java/org/eclipse/jetty/compression/brotli/internal/BrotliDecoderSource.java index 27099bfbb40f..12f44887f1fc 100644 --- a/jetty-core/jetty-compression/jetty-compression-brotli/src/main/java/org/eclipse/jetty/compression/brotli/internal/BrotliDecoderSource.java +++ b/jetty-core/jetty-compression/jetty-compression-brotli/src/main/java/org/eclipse/jetty/compression/brotli/internal/BrotliDecoderSource.java @@ -40,13 +40,13 @@ public BrotliDecoderSource(Content.Source source, BrotliDecoderConfig config) } @Override - protected Content.Chunk nextChunk(Content.Chunk readChunk) throws IOException + protected Content.Chunk transform(Content.Chunk inputChunk) { - ByteBuffer compressed = readChunk.getByteBuffer(); - if (readChunk.isLast() && !readChunk.hasRemaining()) + ByteBuffer compressed = inputChunk.getByteBuffer(); + if (inputChunk.isLast() && !inputChunk.hasRemaining()) return Content.Chunk.EOF; - boolean last = readChunk.isLast(); + boolean last = inputChunk.isLast(); while (true) { @@ -76,7 +76,10 @@ protected Content.Chunk nextChunk(Content.Chunk readChunk) throws IOException // rely on status.OK to go to EOF return Content.Chunk.from(output, false); } - default -> throw new IOException("Decoder failure: Corrupted input buffer"); + default -> + { + return Content.Chunk.from(new IOException("Decoder failure: Corrupted input buffer")); + } } } } diff --git a/jetty-core/jetty-compression/jetty-compression-common/src/main/java/org/eclipse/jetty/compression/DecoderSource.java b/jetty-core/jetty-compression/jetty-compression-common/src/main/java/org/eclipse/jetty/compression/DecoderSource.java index 642f55b08c8d..a36021fcb32e 100644 --- a/jetty-core/jetty-compression/jetty-compression-common/src/main/java/org/eclipse/jetty/compression/DecoderSource.java +++ b/jetty-core/jetty-compression/jetty-compression-common/src/main/java/org/eclipse/jetty/compression/DecoderSource.java @@ -13,121 +13,13 @@ package org.eclipse.jetty.compression; -import java.io.IOException; - import org.eclipse.jetty.io.Content; -import org.eclipse.jetty.util.ExceptionUtil; +import org.eclipse.jetty.io.content.ContentSourceTransformer; -public abstract class DecoderSource implements Content.Source +public abstract class DecoderSource extends ContentSourceTransformer { - private final Content.Source source; - private Content.Chunk activeChunk; - private Throwable failed; - private boolean terminated = false; - - protected DecoderSource(Content.Source source) - { - this.source = source; - } - - @Override - public void demand(Runnable demandCallback) - { - if (activeChunk != null && activeChunk.hasRemaining()) - demandCallback.run(); - else - source.demand(demandCallback); - } - - @Override - public void fail(Throwable failure) - { - failed = ExceptionUtil.combine(failed, failure); - source.fail(failure); - } - - @Override - public Content.Chunk read() - { - if (failed != null) - return Content.Chunk.from(failed, true); - - if (terminated) - return Content.Chunk.EOF; - - Content.Chunk readChunk = readChunk(); - if (readChunk == null) - return null; - - if (Content.Chunk.isFailure(readChunk)) - { - failed = ExceptionUtil.combine(failed, readChunk.getFailure()); - return readChunk; - } - - try - { - Content.Chunk chunk = nextChunk(readChunk); - if (chunk != null && chunk.isLast()) - { - terminate(); - } - return chunk; - } - catch (Throwable x) - { - fail(x); - return Content.Chunk.from(failed, true); - } - } - - /** - * Process the readChunk and produce a response Chunk. - * - * @param readChunk the active Read Chunk (never null, never a failure) - * @throws IOException if decoder failure occurs. - */ - protected abstract Content.Chunk nextChunk(Content.Chunk readChunk) throws IOException; - - /** - * Place to cleanup and release any resources - * being held by this DecoderSource. - */ - protected void release() - { - } - - private void freeActiveChunk() - { - if (activeChunk != null) - activeChunk.release(); - activeChunk = null; - } - - private Content.Chunk readChunk() - { - if (activeChunk != null) - { - if (activeChunk.hasRemaining()) - return activeChunk; - else - { - activeChunk.release(); - activeChunk = null; - } - } - - activeChunk = source.read(); - return activeChunk; - } - - private void terminate() + public DecoderSource(Content.Source rawSource) { - if (!terminated) - { - terminated = true; - freeActiveChunk(); - release(); - } + super(rawSource); } } diff --git a/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/GzipCompression.java b/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/GzipCompression.java index 8e62827a3c0f..c671fa9f0c87 100644 --- a/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/GzipCompression.java +++ b/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/GzipCompression.java @@ -163,7 +163,7 @@ public InputStream newDecoderInputStream(InputStream in, DecoderConfig config) t public DecoderSource newDecoderSource(Content.Source source, DecoderConfig config) { GzipDecoderConfig gzipDecoderConfig = (GzipDecoderConfig)config; - return new GzipDecoderSource(this, source, gzipDecoderConfig); + return new GzipDecoderSource(source, this, gzipDecoderConfig); } @Override diff --git a/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/GzipDecoderConfig.java b/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/GzipDecoderConfig.java index 70c8a4826cc2..a48a92045ce9 100644 --- a/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/GzipDecoderConfig.java +++ b/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/GzipDecoderConfig.java @@ -14,16 +14,12 @@ package org.eclipse.jetty.compression.gzip; import org.eclipse.jetty.compression.DecoderConfig; +import org.eclipse.jetty.util.IO; public class GzipDecoderConfig implements DecoderConfig { - /** - * Default Buffer Size as found in {@link java.util.zip.GZIPInputStream}. - */ - private static final int DEFAULT_BUFFER_SIZE = 512; - /** - * Minimum buffer size to avoid issues with JDK-8133170 - */ + private static final int DEFAULT_BUFFER_SIZE = IO.DEFAULT_BUFFER_SIZE; + // Minimum buffer size to avoid issues with JDK-8133170. private static final int MIN_BUFFER_SIZE = 32; private int bufferSize = DEFAULT_BUFFER_SIZE; diff --git a/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/internal/GzipDecoderSource.java b/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/internal/GzipDecoderSource.java index 516616baa0cf..e6e6c75fc177 100644 --- a/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/internal/GzipDecoderSource.java +++ b/jetty-core/jetty-compression/jetty-compression-gzip/src/main/java/org/eclipse/jetty/compression/gzip/internal/GzipDecoderSource.java @@ -47,7 +47,7 @@ private enum State private long value; private byte flags; - public GzipDecoderSource(GzipCompression compression, Content.Source source, GzipDecoderConfig config) + public GzipDecoderSource(Content.Source source, GzipCompression compression, GzipDecoderConfig config) { super(source); this.compression = compression; @@ -59,24 +59,21 @@ public GzipDecoderSource(GzipCompression compression, Content.Source source, Gzi } @Override - protected Content.Chunk nextChunk(Content.Chunk readChunk) + protected Content.Chunk transform(Content.Chunk inputChunk) { - ByteBuffer compressed = readChunk.getByteBuffer(); - // parse + ByteBuffer compressed = inputChunk.getByteBuffer(); try { - while (compressed.hasRemaining()) + while (true) { switch (state) { - case INITIAL: + case INITIAL -> { inflater.reset(); state = State.ID; - break; } - - case FLAGS: + case FLAGS -> { if ((flags & 0x04) == 0x04) { @@ -85,9 +82,13 @@ protected Content.Chunk nextChunk(Content.Chunk readChunk) value = 0; } else if ((flags & 0x08) == 0x08) + { state = State.NAME; + } else if ((flags & 0x10) == 0x10) + { state = State.COMMENT; + } else if ((flags & 0x2) == 0x2) { state = State.HCRC; @@ -99,54 +100,51 @@ else if ((flags & 0x2) == 0x2) state = State.DATA; continue; } - break; } - - case DATA: + case DATA -> { - try + while (true) { RetainableByteBuffer buffer = compression.acquireByteBuffer(bufferSize); - ByteBuffer decoded = buffer.getByteBuffer(); - int pos = BufferUtil.flipToFill(decoded); - inflater.inflate(decoded); - BufferUtil.flipToFlush(decoded, pos); - if (buffer.hasRemaining()) + try { - return Content.Chunk.asChunk(decoded, false, buffer); + ByteBuffer decoded = buffer.getByteBuffer(); + int pos = BufferUtil.flipToFill(decoded); + inflater.inflate(decoded); + BufferUtil.flipToFlush(decoded, pos); + if (buffer.hasRemaining()) + return Content.Chunk.asChunk(decoded, false, buffer); + buffer.release(); } - else + catch (DataFormatException x) { buffer.release(); + ZipException failure = new ZipException(); + failure.initCause(x); + throw failure; } - } - catch (DataFormatException x) - { - throw new ZipException(x.getMessage()); - } - if (inflater.needsInput()) - { - if (!compressed.hasRemaining()) + if (inflater.needsInput()) { - return Content.Chunk.EMPTY; + if (!compressed.hasRemaining()) + return Content.Chunk.EMPTY; + inflater.setInput(compressed); + // Loop around and try again to inflate. + } + else if (inflater.finished()) + { + state = State.CRC; + size = 0; + value = 0; + break; } - inflater.setInput(compressed); - } - else if (inflater.finished()) - { - state = State.CRC; - size = 0; - value = 0; - break; } } - continue; - - default: - break; } + if (inputChunk.isEmpty()) + return inputChunk.isLast() ? Content.Chunk.EOF : Content.Chunk.EMPTY; + byte currByte = compressed.get(); switch (state) { @@ -163,14 +161,7 @@ else if (inflater.finished()) if (size == 2) { if (value != 0x8B1F) - { - if (LOG.isDebugEnabled()) - LOG.debug("Skipping rest of input, no gzip magic number detected"); - state = State.INITIAL; - compressed.position(compressed.limit()); - // TODO: need to consumeAll super source? - return Content.Chunk.EOF; - } + throw new ZipException("Invalid gzip bytes"); state = State.CM; } } @@ -281,7 +272,6 @@ else if (inflater.finished()) state = State.ERROR; return Content.Chunk.from(x, true); } - return readChunk.isLast() ? Content.Chunk.EOF : Content.Chunk.EMPTY; } @Override diff --git a/jetty-core/jetty-compression/jetty-compression-zstandard/src/main/java/org/eclipse/jetty/compression/zstandard/ZstandardCompression.java b/jetty-core/jetty-compression/jetty-compression-zstandard/src/main/java/org/eclipse/jetty/compression/zstandard/ZstandardCompression.java index 89471af9c443..852833559d07 100644 --- a/jetty-core/jetty-compression/jetty-compression-zstandard/src/main/java/org/eclipse/jetty/compression/zstandard/ZstandardCompression.java +++ b/jetty-core/jetty-compression/jetty-compression-zstandard/src/main/java/org/eclipse/jetty/compression/zstandard/ZstandardCompression.java @@ -164,7 +164,7 @@ public InputStream newDecoderInputStream(InputStream in, DecoderConfig config) t public DecoderSource newDecoderSource(Content.Source source, DecoderConfig config) { ZstandardDecoderConfig zstandardDecoderConfig = (ZstandardDecoderConfig)config; - return new ZstandardDecoderSource(this, source, zstandardDecoderConfig); + return new ZstandardDecoderSource(source, this, zstandardDecoderConfig); } @Override diff --git a/jetty-core/jetty-compression/jetty-compression-zstandard/src/main/java/org/eclipse/jetty/compression/zstandard/internal/ZstandardDecoderSource.java b/jetty-core/jetty-compression/jetty-compression-zstandard/src/main/java/org/eclipse/jetty/compression/zstandard/internal/ZstandardDecoderSource.java index 170d031994b8..045338ab97dc 100644 --- a/jetty-core/jetty-compression/jetty-compression-zstandard/src/main/java/org/eclipse/jetty/compression/zstandard/internal/ZstandardDecoderSource.java +++ b/jetty-core/jetty-compression/jetty-compression-zstandard/src/main/java/org/eclipse/jetty/compression/zstandard/internal/ZstandardDecoderSource.java @@ -27,30 +27,28 @@ public class ZstandardDecoderSource extends DecoderSource private final ZstandardCompression compression; private final ZstdDecompressCtx decompressCtx; - public ZstandardDecoderSource(ZstandardCompression compression, Content.Source src, ZstandardDecoderConfig config) + public ZstandardDecoderSource(Content.Source source, ZstandardCompression compression, ZstandardDecoderConfig config) { - super(src); + super(source); this.compression = compression; this.decompressCtx = new ZstdDecompressCtx(); this.decompressCtx.setMagicless(config.isMagicless()); } @Override - protected Content.Chunk nextChunk(Content.Chunk readChunk) + protected Content.Chunk transform(Content.Chunk inputChunk) { - ByteBuffer input = readChunk.getByteBuffer(); - if (!readChunk.hasRemaining()) - return readChunk; + ByteBuffer input = inputChunk.getByteBuffer(); + if (!inputChunk.hasRemaining()) + return inputChunk; if (!input.isDirect()) throw new IllegalArgumentException("Read Chunk is not a Direct ByteBuffer"); RetainableByteBuffer dst = compression.acquireByteBuffer(); - boolean last = readChunk.isLast(); + boolean last = inputChunk.isLast(); dst.getByteBuffer().clear(); boolean fullyFlushed = decompressCtx.decompressDirectByteBufferStream(dst.getByteBuffer(), input); if (!fullyFlushed) - { last = false; - } dst.getByteBuffer().flip(); return Content.Chunk.asChunk(dst.getByteBuffer(), last, dst); } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceTransformer.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceTransformer.java index bb2e513c4586..de1fb6ff4f00 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceTransformer.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceTransformer.java @@ -13,12 +13,10 @@ package org.eclipse.jetty.io.content; -import java.util.Objects; - import org.eclipse.jetty.io.Content; import org.eclipse.jetty.util.ExceptionUtil; -import org.eclipse.jetty.util.thread.Invocable; -import org.eclipse.jetty.util.thread.SerializedInvoker; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** *

This abstract {@link Content.Source} wraps another {@link Content.Source} and implementers need only @@ -26,27 +24,23 @@ * read from the wrapped source.

*

The {@link #demand(Runnable)} conversation is passed directly to the wrapped {@link Content.Source}, * which means that transformations that may fully consume bytes read can result in a null return from - * {@link Content.Source#read()} even after a callback to the demand {@link Runnable} (as per spurious + * {@link Content.Source#read()} even after a callback to the demand {@link Runnable}, as per spurious * invocation in {@link Content.Source#demand(Runnable)}.

*/ public abstract class ContentSourceTransformer implements Content.Source { - private final SerializedInvoker invoker; + private static final Logger LOG = LoggerFactory.getLogger(ContentSourceTransformer.class); + private final Content.Source rawSource; private Content.Chunk rawChunk; private Content.Chunk transformedChunk; private volatile boolean needsRawRead; - private volatile Runnable demandCallback; + private boolean finished; protected ContentSourceTransformer(Content.Source rawSource) - { - this(rawSource, new SerializedInvoker(ContentSourceTransformer.class)); - } - - protected ContentSourceTransformer(Content.Source rawSource, SerializedInvoker invoker) { this.rawSource = rawSource; - this.invoker = invoker; + this.needsRawRead = true; } protected Content.Source getContentSource() @@ -57,11 +51,16 @@ protected Content.Source getContentSource() @Override public Content.Chunk read() { + if (LOG.isDebugEnabled()) + LOG.debug("Reading {}", this); + while (true) { if (needsRawRead) { rawChunk = rawSource.read(); + if (LOG.isDebugEnabled()) + LOG.debug("Raw chunk {} {}", rawChunk, this); needsRawRead = rawChunk == null; if (rawChunk == null) return null; @@ -72,54 +71,96 @@ public Content.Chunk read() Content.Chunk failure = rawChunk; rawChunk = Content.Chunk.next(rawChunk); needsRawRead = rawChunk == null; + if (rawChunk != null) + { + finished = true; + release(); + } return failure; } if (Content.Chunk.isFailure(transformedChunk)) return transformedChunk; - transformedChunk = process(rawChunk); + if (finished) + return Content.Chunk.EOF; + + boolean rawLast = rawChunk != null && rawChunk.isLast(); - if (rawChunk != null && rawChunk != transformedChunk) + transformedChunk = process(rawChunk != null ? rawChunk : Content.Chunk.EMPTY); + if (LOG.isDebugEnabled()) + LOG.debug("Transformed chunk {} {}", transformedChunk, this); + + if (rawChunk == null && (transformedChunk == null || transformedChunk == Content.Chunk.EMPTY)) + { + needsRawRead = true; + continue; + } + + // Prevent double release. + if (transformedChunk == rawChunk) + rawChunk = null; + + if (rawChunk != null && rawChunk.isEmpty()) + { rawChunk.release(); - rawChunk = null; + rawChunk = Content.Chunk.next(rawChunk); + } if (transformedChunk != null) { + boolean transformedLast = transformedChunk.isLast(); + boolean transformedFailure = Content.Chunk.isFailure(transformedChunk); + + // Transformation may be complete, but rawSource is not read until EOF, + // return a non-last transformed chunk to force more read() and transform(). + if (transformedLast && !rawLast) + { + if (transformedChunk == Content.Chunk.EOF) + transformedChunk = Content.Chunk.EMPTY; + else if (!transformedFailure) + transformedChunk = Content.Chunk.asChunk(transformedChunk.getByteBuffer(), false, transformedChunk); + } + + boolean terminated = rawLast && transformedLast; + boolean terminalFailure = transformedFailure && transformedLast; + Content.Chunk result = transformedChunk; transformedChunk = Content.Chunk.next(result); + + if (terminated || terminalFailure) + { + finished = true; + release(); + } + return result; } - needsRawRead = true; + needsRawRead = rawChunk == null; } } @Override public void demand(Runnable demandCallback) { - this.demandCallback = Objects.requireNonNull(demandCallback); + if (LOG.isDebugEnabled()) + LOG.debug("Demanding {} {}", demandCallback, this); + if (needsRawRead) - // Inner class used instead of lambda for clarity in stack traces. - rawSource.demand(new DemandTask(Invocable.getInvocationType(demandCallback), this::invokeDemandCallback)); + rawSource.demand(demandCallback); else - invoker.run(this::invokeDemandCallback); + ExceptionUtil.run(demandCallback, this::fail); } @Override public void fail(Throwable failure) { + if (LOG.isDebugEnabled()) + LOG.debug("Failing {}", this, failure); rawSource.fail(failure); } - private void invokeDemandCallback() - { - Runnable demandCallback = this.demandCallback; - this.demandCallback = null; - if (demandCallback != null) - ExceptionUtil.run(demandCallback, this::fail); - } - private Content.Chunk process(Content.Chunk rawChunk) { try @@ -136,44 +177,50 @@ private Content.Chunk process(Content.Chunk rawChunk) /** *

Transforms the input chunk parameter into an output chunk.

*

When this method produces a non-{@code null}, non-last chunk, - * it is subsequently invoked with a {@code null} input chunk to try to + * it is subsequently invoked with either the input chunk (if it has + * remaining bytes), or with {@link Content.Chunk#EMPTY} to try to * produce more output chunks from the previous input chunk. * For example, a single compressed input chunk may be transformed into * multiple uncompressed output chunks.

- *

The input chunk is released as soon as this method returns, so - * implementations that must hold onto the input chunk must arrange to call - * {@link Content.Chunk#retain()} and its correspondent {@link Content.Chunk#release()}.

- *

Implementations should return an {@link Content.Chunk} with non-null - * {@link Content.Chunk#getFailure()} in case - * of transformation errors.

- *

Exceptions thrown by this method are equivalent to returning an error chunk.

+ *

The input chunk is released as soon as this method returns if it + * is fully consumed, so implementations that must hold onto the input + * chunk must arrange to call {@link Content.Chunk#retain()} and its + * correspondent {@link Content.Chunk#release()}.

+ *

Implementations should return a {@link Content.Chunk} with non-null + * {@link Content.Chunk#getFailure()} in case of transformation errors.

+ *

Exceptions thrown by this method are equivalent to returning an + * error chunk.

*

Implementations of this method may return:

*
    - *
  • {@code null}, if more input chunks are necessary to produce an output chunk
  • - *
  • the {@code inputChunk} itself, typically in case of non-null {@link Content.Chunk#getFailure()}, - * or when no transformation is required
  • + *
  • {@code null} or {@link Content.Chunk#EMPTY}, if more input chunks + * are necessary to produce an output chunk
  • + *
  • the {@code inputChunk} itself, typically in case of non-null + * {@link Content.Chunk#getFailure()}, or when no transformation is required
  • *
  • a new {@link Content.Chunk} derived from {@code inputChunk}.
  • *
+ *

The input chunk should be consumed (its position updated) as the + * transformation proceeds.

* * @param inputChunk a chunk read from the wrapped {@link Content.Source} * @return a transformed chunk or {@code null} */ protected abstract Content.Chunk transform(Content.Chunk inputChunk); - private class DemandTask extends Invocable.Task.Abstract + /** + *

Invoked when the transformation is complete to release any resource.

+ */ + protected void release() { - private final Runnable invokeDemandCallback; - - private DemandTask(InvocationType invocationType, Runnable invokeDemandCallback) - { - super(invocationType); - this.invokeDemandCallback = invokeDemandCallback; - } + } - @Override - public void run() - { - invoker.run(invokeDemandCallback); - } + @Override + public String toString() + { + return "%s@%x[finished=%b,source=%s]".formatted( + getClass().getSimpleName(), + hashCode(), + finished, + rawSource + ); } } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTransformerTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTransformerTest.java index 2a71e9a494b1..c397c0e77728 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTransformerTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTransformerTest.java @@ -27,6 +27,7 @@ import org.eclipse.jetty.io.content.ContentSourceTransformer; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FutureCallback; +import org.eclipse.jetty.util.thread.SerializedInvoker; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -367,14 +368,12 @@ public void testTransientFailuresFromTransformationAreReturned() @Override protected Content.Chunk transform(Content.Chunk rawChunk) { - if (rawChunk == null) - return null; - String decoded = UTF_8.decode(rawChunk.getByteBuffer().duplicate()).toString(); + String decoded = UTF_8.decode(rawChunk.getByteBuffer()).toString(); return switch (decoded) { case "B" -> Content.Chunk.from(originalFailure1, false); case "D" -> Content.Chunk.from(originalFailure2, false); - default -> Content.Chunk.from(rawChunk.getByteBuffer(), rawChunk.isLast()); + default -> Content.Chunk.from(UTF_8.encode(decoded), rawChunk.isLast()); }; } }; @@ -399,6 +398,7 @@ protected Content.Chunk transform(Content.Chunk rawChunk) private static class WordSplitLowCaseTransformer extends ContentSourceTransformer { + private final SerializedInvoker invoker = new SerializedInvoker(); private final Queue chunks = new ArrayDeque<>(); private WordSplitLowCaseTransformer(Content.Source rawSource) @@ -406,6 +406,12 @@ private WordSplitLowCaseTransformer(Content.Source rawSource) super(rawSource); } + @Override + public void demand(Runnable demandCallback) + { + super.demand(() -> invoker.run(demandCallback)); + } + @Override protected Content.Chunk transform(Content.Chunk rawChunk) { diff --git a/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/AsyncMiddleManServlet.java b/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/AsyncMiddleManServlet.java index 014652a027e4..c54154dd05f4 100644 --- a/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/AsyncMiddleManServlet.java +++ b/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/AsyncMiddleManServlet.java @@ -35,12 +35,11 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.AsyncRequestContent; -import org.eclipse.jetty.client.ContentDecoder; -import org.eclipse.jetty.client.GZIPContentDecoder; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.Request; import org.eclipse.jetty.client.Response; import org.eclipse.jetty.client.Result; +import org.eclipse.jetty.http.GZIPContentDecoder; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Content; @@ -50,6 +49,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.CountingCallback; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.IteratingCallback; import org.eclipse.jetty.util.component.Destroyable; import org.eclipse.jetty.util.thread.AutoLock; @@ -777,7 +777,7 @@ public static class GZIPContentTransformer implements ContentTransformer private final List buffers = new ArrayList<>(2); private final ContentTransformer transformer; - private final ContentDecoder decoder; + private final GZIPContentDecoder decoder; private final ByteArrayOutputStream out; private final GZIPOutputStream gzipOut; @@ -792,7 +792,7 @@ public GZIPContentTransformer(HttpClient httpClient, ContentTransformer transfor { this.transformer = transformer; ByteBufferPool bufferPool = httpClient == null ? null : httpClient.getByteBufferPool(); - this.decoder = new GZIPContentDecoder(bufferPool, GZIPContentDecoder.DEFAULT_BUFFER_SIZE); + this.decoder = new GZIPDecoder(bufferPool); this.out = new ByteArrayOutputStream(); this.gzipOut = new GZIPOutputStream(out); } @@ -854,6 +854,21 @@ private ByteBuffer gzip(List buffers, boolean finished) throws IOExc out.reset(); return ByteBuffer.wrap(gzipBytes); } + + private static class GZIPDecoder extends GZIPContentDecoder + { + public GZIPDecoder(ByteBufferPool bufferPool) + { + super(bufferPool, IO.DEFAULT_BUFFER_SIZE); + } + + @Override + protected boolean decodedChunk(RetainableByteBuffer chunk) + { + super.decodedChunk(chunk); + return true; + } + } } private class ProxyAsyncRequestContent extends AsyncRequestContent diff --git a/jetty-ee11/jetty-ee11-proxy/src/main/java/org/eclipse/jetty/ee11/proxy/AsyncMiddleManServlet.java b/jetty-ee11/jetty-ee11-proxy/src/main/java/org/eclipse/jetty/ee11/proxy/AsyncMiddleManServlet.java index e74a3bc18119..9988107888ea 100644 --- a/jetty-ee11/jetty-ee11-proxy/src/main/java/org/eclipse/jetty/ee11/proxy/AsyncMiddleManServlet.java +++ b/jetty-ee11/jetty-ee11-proxy/src/main/java/org/eclipse/jetty/ee11/proxy/AsyncMiddleManServlet.java @@ -35,12 +35,11 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.AsyncRequestContent; -import org.eclipse.jetty.client.ContentDecoder; -import org.eclipse.jetty.client.GZIPContentDecoder; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.Request; import org.eclipse.jetty.client.Response; import org.eclipse.jetty.client.Result; +import org.eclipse.jetty.http.GZIPContentDecoder; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Content; @@ -50,6 +49,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.CountingCallback; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.IteratingCallback; import org.eclipse.jetty.util.component.Destroyable; import org.eclipse.jetty.util.thread.AutoLock; @@ -777,7 +777,7 @@ public static class GZIPContentTransformer implements ContentTransformer private final List buffers = new ArrayList<>(2); private final ContentTransformer transformer; - private final ContentDecoder decoder; + private final GZIPContentDecoder decoder; private final ByteArrayOutputStream out; private final GZIPOutputStream gzipOut; @@ -792,7 +792,7 @@ public GZIPContentTransformer(HttpClient httpClient, ContentTransformer transfor { this.transformer = transformer; ByteBufferPool bufferPool = httpClient == null ? null : httpClient.getByteBufferPool(); - this.decoder = new GZIPContentDecoder(bufferPool, GZIPContentDecoder.DEFAULT_BUFFER_SIZE); + this.decoder = new GZIPDecoder(bufferPool); this.out = new ByteArrayOutputStream(); this.gzipOut = new GZIPOutputStream(out); } @@ -854,6 +854,21 @@ private ByteBuffer gzip(List buffers, boolean finished) throws IOExc out.reset(); return ByteBuffer.wrap(gzipBytes); } + + private static class GZIPDecoder extends GZIPContentDecoder + { + public GZIPDecoder(ByteBufferPool bufferPool) + { + super(bufferPool, IO.DEFAULT_BUFFER_SIZE); + } + + @Override + protected boolean decodedChunk(RetainableByteBuffer chunk) + { + super.decodedChunk(chunk); + return true; + } + } } private class ProxyAsyncRequestContent extends AsyncRequestContent diff --git a/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/AsyncMiddleManServlet.java b/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/AsyncMiddleManServlet.java index 7ec105a2aa59..288bc115974d 100644 --- a/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/AsyncMiddleManServlet.java +++ b/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/AsyncMiddleManServlet.java @@ -35,12 +35,11 @@ import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.AsyncRequestContent; -import org.eclipse.jetty.client.ContentDecoder; -import org.eclipse.jetty.client.GZIPContentDecoder; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.Request; import org.eclipse.jetty.client.Response; import org.eclipse.jetty.client.Result; +import org.eclipse.jetty.http.GZIPContentDecoder; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Content; @@ -50,6 +49,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.CountingCallback; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.IteratingCallback; import org.eclipse.jetty.util.component.Destroyable; import org.eclipse.jetty.util.thread.AutoLock; @@ -777,7 +777,7 @@ public static class GZIPContentTransformer implements ContentTransformer private final List buffers = new ArrayList<>(2); private final ContentTransformer transformer; - private final ContentDecoder decoder; + private final GZIPContentDecoder decoder; private final ByteArrayOutputStream out; private final GZIPOutputStream gzipOut; @@ -792,7 +792,7 @@ public GZIPContentTransformer(HttpClient httpClient, ContentTransformer transfor { this.transformer = transformer; ByteBufferPool bufferPool = httpClient == null ? null : httpClient.getByteBufferPool(); - this.decoder = new GZIPContentDecoder(bufferPool, GZIPContentDecoder.DEFAULT_BUFFER_SIZE); + this.decoder = new GZIPDecoder(bufferPool); this.out = new ByteArrayOutputStream(); this.gzipOut = new GZIPOutputStream(out); } @@ -854,6 +854,21 @@ private ByteBuffer gzip(List buffers, boolean finished) throws IOExc out.reset(); return ByteBuffer.wrap(gzipBytes); } + + private static class GZIPDecoder extends GZIPContentDecoder + { + public GZIPDecoder(ByteBufferPool bufferPool) + { + super(bufferPool, IO.DEFAULT_BUFFER_SIZE); + } + + @Override + protected boolean decodedChunk(RetainableByteBuffer chunk) + { + super.decodedChunk(chunk); + return true; + } + } } private class ProxyAsyncRequestContent extends AsyncRequestContent From 7a061035bf4765ed6aaa604fbd4986a30f70c45b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 5 Dec 2024 15:00:52 +0100 Subject: [PATCH 02/20] Updated OSGi dependencies, now that jetty-client depends on jetty-compression-common. Signed-off-by: Simone Bordet --- .../java/org/eclipse/jetty/ee11/osgi/test/TestOSGiUtil.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jetty-ee11/jetty-ee11-osgi/test-jetty-ee11-osgi/src/test/java/org/eclipse/jetty/ee11/osgi/test/TestOSGiUtil.java b/jetty-ee11/jetty-ee11-osgi/test-jetty-ee11-osgi/src/test/java/org/eclipse/jetty/ee11/osgi/test/TestOSGiUtil.java index f0aefe4aeeea..fbbb1763f139 100644 --- a/jetty-ee11/jetty-ee11-osgi/test-jetty-ee11-osgi/src/test/java/org/eclipse/jetty/ee11/osgi/test/TestOSGiUtil.java +++ b/jetty-ee11/jetty-ee11-osgi/test-jetty-ee11-osgi/src/test/java/org/eclipse/jetty/ee11/osgi/test/TestOSGiUtil.java @@ -214,6 +214,8 @@ public static void coreJettyDependencies(List
+ + org.eclipse.jetty.compression + jetty-compression-brotli + test + + + org.eclipse.jetty.compression + jetty-compression-gzip + test + + + org.eclipse.jetty.compression + jetty-compression-server + test + + + org.eclipse.jetty.compression + jetty-compression-zstandard + test + org.eclipse.jetty.fcgi jetty-fcgi-server diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientCompressionTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientCompressionTest.java new file mode 100644 index 000000000000..9ee972221bfb --- /dev/null +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientCompressionTest.java @@ -0,0 +1,242 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.test.client.transport; + +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import org.eclipse.jetty.client.ContentResponse; +import org.eclipse.jetty.compression.gzip.GzipCompression; +import org.eclipse.jetty.compression.server.CompressionHandler; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Handler; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; +import org.eclipse.jetty.util.Callback; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +public class HttpClientCompressionTest extends AbstractTest +{ + private static final String SAMPLE_CONTENT = """ + Lorem ipsum dolor sit amet, consectetur adipiscing elit. In quis felis nunc. + Quisque suscipit mauris et ante auctor ornare rhoncus lacus aliquet. Pellentesque + habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. + Vestibulum sit amet felis augue, vel convallis dolor. Cras accumsan vehicula diam + at faucibus. Etiam in urna turpis, sed congue mi. Morbi et lorem eros. Donec vulputate + velit in risus suscipit lobortis. Aliquam id urna orci, nec sollicitudin ipsum. + Cras a orci turpis. Donec suscipit vulputate cursus. Mauris nunc tellus, fermentum + eu auctor ut, mollis at diam. Quisque porttitor ultrices metus, vitae tincidunt massa + sollicitudin a. Vivamus porttitor libero eget purus hendrerit cursus. Integer aliquam + consequat mauris quis luctus. Cras enim nibh, dignissim eu faucibus ac, mollis nec neque. + Aliquam purus mauris, consectetur nec convallis lacinia, porta sed ante. Suspendisse + et cursus magna. Donec orci enim, molestie a lobortis eu, imperdiet vitae neque. + """; + + @ParameterizedTest + @MethodSource("transports") + public void testEmptyAcceptEncoding(TransportType transportType) throws Exception + { + start(transportType, new CompressionHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + assertEquals("", request.getHeaders().get(HttpHeader.ACCEPT_ENCODING)); + Content.Sink.write(response, true, SAMPLE_CONTENT, callback); + return true; + } + })); + + AtomicReference contentEncodingRef = new AtomicReference<>(); + ContentResponse response = client.newRequest(newURI(transportType)) + .headers(h -> h.put(HttpHeader.ACCEPT_ENCODING, "")) + .onResponseHeader((r, f) -> + { + if (f.getHeader() == HttpHeader.CONTENT_ENCODING) + contentEncodingRef.set(f.getValue()); + return true; + }) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertNull(contentEncodingRef.get()); + assertEquals(SAMPLE_CONTENT, response.getContentAsString()); + } + + @ParameterizedTest + @MethodSource("transports") + public void testZeroQualityAcceptEncoding(TransportType transportType) throws Exception + { + start(transportType, new CompressionHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + Content.Sink.write(response, true, SAMPLE_CONTENT, callback); + return true; + } + })); + + AtomicReference contentEncodingRef = new AtomicReference<>(); + ContentResponse response = client.newRequest(newURI(transportType)) + .headers(h -> h.put(HttpHeader.ACCEPT_ENCODING, "gzip;q=0")) + .onResponseHeader((r, f) -> + { + if (f.getHeader() == HttpHeader.CONTENT_ENCODING) + contentEncodingRef.set(f.getValue()); + return true; + }) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertNull(contentEncodingRef.get()); + assertEquals(SAMPLE_CONTENT, response.getContentAsString()); + } + + @ParameterizedTest + @MethodSource("transports") + public void testIdentityAcceptEncoding(TransportType transportType) throws Exception + { + start(transportType, new CompressionHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + Content.Sink.write(response, true, SAMPLE_CONTENT, callback); + return true; + } + })); + + AtomicReference contentEncodingRef = new AtomicReference<>(); + ContentResponse response = client.newRequest(newURI(transportType)) + .headers(h -> h.put(HttpHeader.ACCEPT_ENCODING, "identity")) + .onResponseHeader((r, f) -> + { + if (f.getHeader() == HttpHeader.CONTENT_ENCODING) + contentEncodingRef.set(f.getValue()); + return true; + }) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertNull(contentEncodingRef.get()); + assertEquals(SAMPLE_CONTENT, response.getContentAsString()); + } + + @ParameterizedTest + @MethodSource("transports") + public void testUnacceptableAcceptEncoding(TransportType transportType) throws Exception + { + CompressionHandler compressionHandler = new CompressionHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + Content.Sink.write(response, true, SAMPLE_CONTENT, callback); + return true; + } + }); + // Support only gzip on the server. + compressionHandler.putCompression(new GzipCompression()); + start(transportType, compressionHandler); + + AtomicReference contentEncodingRef = new AtomicReference<>(); + ContentResponse response = client.newRequest(newURI(transportType)) + // Do not accept gzip. + .headers(h -> h.put(HttpHeader.ACCEPT_ENCODING, "br, gzip;q=0")) + .onResponseHeader((r, f) -> + { + if (f.getHeader() == HttpHeader.CONTENT_ENCODING) + contentEncodingRef.set(f.getValue()); + return true; + }) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertNull(contentEncodingRef.get()); + assertEquals(SAMPLE_CONTENT, response.getContentAsString()); + } + + @ParameterizedTest + @MethodSource("transports") + public void testUnsupportedAcceptEncoding(TransportType transportType) throws Exception + { + CompressionHandler compressionHandler = new CompressionHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + Content.Sink.write(response, true, SAMPLE_CONTENT, callback); + return true; + } + }); + // Support only gzip on the server. + compressionHandler.putCompression(new GzipCompression()); + start(transportType, compressionHandler); + + for (String v : List.of("br, identity;q=0", "br, identity;q=0, *;q=0")) + { + ContentResponse response = client.newRequest(newURI(transportType)) + // Do not accept identity, so the server won't be able to respond. + .headers(h -> h.put(HttpHeader.ACCEPT_ENCODING, v)) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(HttpStatus.UNSUPPORTED_MEDIA_TYPE_415, response.getStatus(), "Accept-Encoding: " + v); + response.getHeaders().contains(HttpHeader.ACCEPT_ENCODING, "gzip"); + } + } + + @ParameterizedTest + @MethodSource("transports") + public void testQualityAcceptEncoding(TransportType transportType) throws Exception + { + start(transportType, new CompressionHandler(new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + Content.Sink.write(response, true, SAMPLE_CONTENT, callback); + return true; + } + })); + + AtomicReference contentEncodingRef = new AtomicReference<>(); + ContentResponse response = client.newRequest(newURI(transportType)) + .headers(h -> h.put(HttpHeader.ACCEPT_ENCODING, "gzip;q=0.5, br;q=1.0")) + .onResponseHeader((r, f) -> + { + if (f.getHeader() == HttpHeader.CONTENT_ENCODING) + contentEncodingRef.set(f.getValue()); + return true; + }) + .timeout(5, TimeUnit.SECONDS) + .send(); + + assertEquals(HttpStatus.OK_200, response.getStatus()); + assertEquals("br", contentEncodingRef.get()); + assertEquals(SAMPLE_CONTENT, response.getContentAsString()); + } +} diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExclude.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExclude.java index 8d8c753c5ce1..7c8cc13d76e7 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExclude.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/IncludeExclude.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.util; +import java.util.Collections; import java.util.Set; import java.util.function.Predicate; @@ -45,7 +46,7 @@ public > IncludeExclude(SET includeSet, Predicate in @Override public IncludeExclude asImmutable() { - return new IncludeExclude<>(Set.copyOf(getIncluded()), getIncludePredicate(), - Set.copyOf(getExcluded()), getExcludePredicate()); + return new IncludeExclude<>(Collections.unmodifiableSet(getIncluded()), getIncludePredicate(), + Collections.unmodifiableSet(getExcluded()), getExcludePredicate()); } } From 50067d7c0f408abb6a140705c330032ae94c3ecf Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sun, 15 Dec 2024 22:01:16 +0100 Subject: [PATCH 19/20] Added module compression-all and correspondent distribution test. Signed-off-by: Simone Bordet --- .../src/main/config/modules/compression-all.mod | 17 +++++++++++++++++ .../tests/distribution/DistributionTests.java | 14 ++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-all.mod diff --git a/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-all.mod b/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-all.mod new file mode 100644 index 000000000000..25a66f7efceb --- /dev/null +++ b/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-all.mod @@ -0,0 +1,17 @@ +# DO NOT EDIT THIS FILE - See: https://jetty.org/docs/ + +[description] +Enables all available compression algorithms in CompressionHandler. + +[tags] +server +handler +compression +brotli +gzip +zstandard + +[depend] +compression-brotli +compression-gzip +compression-zstandard diff --git a/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java b/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java index 4382686406eb..47bbdee7f25b 100644 --- a/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java +++ b/tests/test-distribution/test-distribution-common/src/test/java/org/eclipse/jetty/tests/distribution/DistributionTests.java @@ -2222,7 +2222,7 @@ public void testLimitHandlers(String env) throws Exception } @ParameterizedTest - @ValueSource(strings = {"brotli", "gzip", "zstandard"}) + @ValueSource(strings = {"brotli", "gzip", "zstandard", "all"}) public void testCompressionHandler(String compressionName) throws Exception { String jettyVersion = System.getProperty("jettyVersion"); @@ -2235,6 +2235,16 @@ public void testCompressionHandler(String compressionName) throws Exception case "brotli" -> "br"; case "gzip" -> "gzip"; case "zstandard" -> "zstd"; + case "all" -> "br;q=0.5, gzip;q=1, zstd;q=0.1"; + default -> throw new IllegalArgumentException(); + }; + + String expected = switch (compressionName) + { + case "brotli" -> "br"; + case "gzip" -> "gzip"; + case "zstandard" -> "zstd"; + case "all" -> "gzip"; default -> throw new IllegalArgumentException(); }; @@ -2279,7 +2289,7 @@ public void testCompressionHandler(String compressionName) throws Exception .send(); assertEquals(HttpStatus.OK_200, response.getStatus()); - assertThat(contentEncoding.get(), is(encoding)); + assertThat(contentEncoding.get(), is(expected)); assertThat(response.getContentAsString(), containsStringIgnoringCase("Hello World")); } } From 9660e127a68c650a319f159c5d0c1bfa89de2a93 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 16 Dec 2024 12:37:58 +0100 Subject: [PATCH 20/20] Added module documentation. Signed-off-by: Simone Bordet --- .../server/http/HTTPServerDocs.java | 2 + .../pages/modules/standard.adoc | 51 +++++++++++++++++++ .../config/modules/compression-brotli.mod | 2 + .../main/config/modules/compression-gzip.mod | 2 + .../config/modules/compression-zstandard.mod | 2 + 5 files changed, 59 insertions(+) diff --git a/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java b/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java index 83ff17ced16f..7c1effd79b39 100644 --- a/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java +++ b/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java @@ -1421,6 +1421,8 @@ public void serverCompressionHandler() throws Exception // Do not compress these mime types. .compressExcludeMimeType("font/ttf") .build(); + // Map the request URI path spec '/*' with the compression configuration. + // You can map different path specs with different compression configurations. compressionHandler.putConfiguration("/*", compressionConfig); // Create a ContextHandlerCollection to manage contexts. diff --git a/documentation/jetty/modules/operations-guide/pages/modules/standard.adoc b/documentation/jetty/modules/operations-guide/pages/modules/standard.adoc index e500e5be7426..33d133c6cca2 100644 --- a/documentation/jetty/modules/operations-guide/pages/modules/standard.adoc +++ b/documentation/jetty/modules/operations-guide/pages/modules/standard.adoc @@ -54,6 +54,57 @@ This property allows you to cap the max heap memory retained by the pool. `jetty.byteBufferPool.maxDirectMemory`:: This property allows you to cap the max direct memory retained by the pool. +[[compression]] +== Compression Modules + +The compression modules allow you to configure server-wide request decompression and response compression by installing the `org.eclipse.jetty.compression.server.CompressionHandler` at the root of the `Handler` tree (see also xref:programming-guide:server/http.adoc#handler-use-compression[this section] for further details). + +The supported algorithms are the following: + +* brotli, via the <> module +* gzip, via the <> module +* zstandard, via the <> module + +You can explicitly enable one or more of the compression modules, or all of them with the <> module. + +[[compression-all]] +=== Module `compression-all` + +This module enables request decompression and response compression for all the currently supported algorithms listed in <>. + +[[compression-brotli]] +=== Module `compression-brotli` + +This module enables request decompression and response compression with the link:https://github.com/google/brotli[brotli] algorithm. + +The module properties are: + +---- +include::{jetty-home}/modules/compression-brotli.mod[tags=documentation] +---- + +[[compression-gzip]] +=== Module `compression-gzip` + +This module enables request decompression and response compression with the link:https://en.wikipedia.org/wiki/Gzip[gzip] algorithm. + +The module properties are: + +---- +include::{jetty-home}/modules/compression-gzip.mod[tags=documentation] +---- + +[[compression-zstandard]] +=== Module `compression-zstandard` + +This module enables request decompression and response compression with the link:https://github.com/facebook/zstd[zstdandard] algorithm. + +The module properties are: + +---- +include::{jetty-home}/modules/compression-zstandard.mod[tags=documentation] +---- + [[connectionlimit]] == Module `connectionlimit` diff --git a/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-brotli.mod b/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-brotli.mod index a4cb4a2fada3..428855fbe38c 100644 --- a/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-brotli.mod +++ b/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-brotli.mod @@ -17,6 +17,7 @@ compression etc/jetty-compression-brotli.xml [ini-template] +# tag::documentation[] ## Minimum content length after which brotli is enabled # jetty.compression.brotli.minCompressSize=48 @@ -39,3 +40,4 @@ etc/jetty-compression-brotli.xml ## Brotli log2(LZ window size) for Encoder # valid values from 10 to 24 # jetty.compression.brotli.encoder.lgWindow=22 +# end::documentation[] diff --git a/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-gzip.mod b/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-gzip.mod index 28c72d8687f5..6a422a7aa62a 100644 --- a/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-gzip.mod +++ b/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-gzip.mod @@ -17,6 +17,7 @@ compression etc/jetty-compression-gzip.xml [ini-template] +# tag::documentation[] ## Minimum content length after which gzip is enabled # jetty.compression.gzip.minCompressSize=32 @@ -56,3 +57,4 @@ etc/jetty-compression-gzip.xml ## syncFlush for Encoder ## true for SYNC_FLUSH, false for NO_FLUSH (default) # jetty.compression.gzip.encoder.syncFlush=false +# end::documentation[] diff --git a/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-zstandard.mod b/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-zstandard.mod index 9efd733810b7..ee56f1fc94c4 100644 --- a/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-zstandard.mod +++ b/jetty-core/jetty-compression/jetty-compression-server/src/main/config/modules/compression-zstandard.mod @@ -17,6 +17,7 @@ compression etc/jetty-compression-zstandard.xml [ini-template] +# tag::documentation[] ## Minimum content length after which zstandard is enabled # jetty.compression.zstandard.minCompressSize=48 @@ -52,3 +53,4 @@ etc/jetty-compression-zstandard.xml # Enable/Disable compression checksums # Note: browser zstandard implementations requires this to be false. # jetty.compression.zstandard.encoder.checksum=false +# end::documentation[]