From 7dcab84b91375579ac701c22ee77ff5e8ca04a19 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 24 Nov 2023 01:25:03 +1100 Subject: [PATCH] Fix jetty 12.0.x transient timeouts (#10844) Fixes #10234 * Introduced transient failures in reads where a failure chunk has last=false. * Transient failure now do not fail the handler callback. * Improve eeN ContentProducer to more carefully assert transient and terminal errors + enable HttpInputIntegrationTest * Do not add connection: close to the response when the error is transient * Rework ChunksContentSource to support null chunks * Added tests to verify the new transient failure cases * Review all code that handles failure, and handling correctly transient failure, either by making them fatal, and/or by failing Content.Source. Signed-off-by: Ludovic Orban Signed-off-by: Olivier Lamy Signed-off-by: Simone Bordet Co-authored-by: Ludovic Orban Co-authored-by: Olivier Lamy Co-authored-by: Joakim Erdfelt Co-authored-by: Chad Wilson Co-authored-by: Simone Bordet --- .../asciidoc/programming-guide/arch-io.adoc | 35 +- .../jetty/docs/programming/ContentDocs.java | 33 +- .../eclipse/jetty/client/ContentDecoder.java | 6 +- .../org/eclipse/jetty/client/Response.java | 3 + .../jetty/client/transport/HttpReceiver.java | 30 +- .../jetty/client/transport/HttpSender.java | 6 +- .../client/transport/ResponseListeners.java | 19 +- .../client/AsyncContentListenerTest.java | 97 ++++ ...HttpClientContentDecoderFactoriesTest.java | 117 +++++ .../client/HttpClientContentFailuresTest.java | 233 ++++++++++ .../transport/ResponseListenersTest.java | 207 ++++++--- .../server/internal/HttpStreamOverFCGI.java | 4 +- .../org/eclipse/jetty/http/MultiPart.java | 17 +- .../jetty/http/MultiPartFormDataTest.java | 107 +++++ .../eclipse/jetty/io/ChunkAccumulator.java | 4 +- .../java/org/eclipse/jetty/io/Content.java | 34 +- .../jetty/io/content/ChunksContentSource.java | 29 +- .../ContentSourceCompletableFuture.java | 18 +- .../io/content/ContentSourceInputStream.java | 14 +- .../io/content/ContentSourcePublisher.java | 2 + .../io/content/ContentSourceTransformer.java | 7 +- .../jetty/io/internal/ContentCopier.java | 9 +- .../io/internal/ContentSourceByteBuffer.java | 2 + .../io/internal/ContentSourceConsumer.java | 2 + .../io/internal/ContentSourceString.java | 2 + .../jetty/io/ChunkAccumulatorTest.java | 62 +++ .../ContentSourceCompletableFutureTest.java | 123 +++++ .../io/ContentSourceInputStreamTest.java | 127 ++++++ .../io/ContentSourceTransformerTest.java | 85 ++++ .../org/eclipse/jetty/io/ContentTest.java | 25 + .../java/org/eclipse/jetty/io/TestSink.java | 39 ++ .../java/org/eclipse/jetty/io/TestSource.java | 38 ++ .../jetty/io/internal/ContentCopierTest.java | 72 +++ .../internal/ContentSourceByteBufferTest.java | 64 +++ .../internal/ContentSourceConsumerTest.java | 64 +++ .../io/internal/ContentSourceStringTest.java | 65 +++ .../server/NegotiatingServerConnection.java | 5 +- .../server/handler/gzip/GzipRequest.java | 18 +- .../server/internal/HttpChannelState.java | 40 +- .../jetty/server/internal/HttpConnection.java | 19 +- .../eclipse/jetty/server/HttpStreamTest.java | 164 +++++++ .../handler/gzip/GzipTransformerTest.java | 129 ++++++ .../client/transport/ServerTimeoutsTest.java | 35 +- .../org/eclipse/jetty/util/BufferUtil.java | 2 +- .../jetty/util/thread/QueuedThreadPool.java | 7 +- .../jetty/ee10/proxy/ProxyServlet.java | 2 + jetty-ee10/jetty-ee10-servlet/pom.xml | 5 + .../ee10/servlet/AsyncContentProducer.java | 47 +- .../ee10/servlet/BlockingContentProducer.java | 11 +- .../eclipse/jetty/ee10/servlet/HttpInput.java | 7 +- .../jetty/ee10/servlet/ServletChannel.java | 2 +- .../servlet/AbstractContentProducerTest.java | 148 ++++++ .../servlet/AsyncContentProducerTest.java | 208 +++++++++ .../servlet/BlockingContentProducerTest.java | 184 ++++++++ .../ee10/servlet/MockConnectionMetaData.java | 132 ++++++ .../jetty/ee10/servlet/MockRequest.java | 173 +++++++ .../test/client/transport/AbstractTest.java | 8 + .../transport/HttpClientContinueTest.java | 49 ++ .../client/transport/ServerTimeoutsTest.java | 44 +- .../jetty-ee10-test-integration/pom.xml | 5 + .../ee10/test/HttpInputIntegrationTest.java | 61 ++- .../ee10/test/HttpInputInterceptorTest.java | 342 -------------- .../test/HttpInputTransientErrorTest.java | 430 ++++++++++++++++++ .../eclipse/jetty/ee9/proxy/ProxyServlet.java | 2 + .../jetty-ee9-test-integration/pom.xml | 5 + .../ee9/test/HttpInputIntegrationTest.java | 59 +-- .../ee9/test/HttpInputTransientErrorTest.java | 223 +++++++++ 67 files changed, 3736 insertions(+), 631 deletions(-) create mode 100644 jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/AsyncContentListenerTest.java create mode 100644 jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentDecoderFactoriesTest.java create mode 100644 jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentFailuresTest.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ChunkAccumulatorTest.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceCompletableFutureTest.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceInputStreamTest.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/TestSink.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/TestSource.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentCopierTest.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceByteBufferTest.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceConsumerTest.java create mode 100644 jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceStringTest.java create mode 100644 jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpStreamTest.java create mode 100644 jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipTransformerTest.java create mode 100644 jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AbstractContentProducerTest.java create mode 100644 jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducerTest.java create mode 100644 jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducerTest.java create mode 100644 jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MockConnectionMetaData.java create mode 100644 jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MockRequest.java delete mode 100644 jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputInterceptorTest.java create mode 100644 jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputTransientErrorTest.java create mode 100644 jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputTransientErrorTest.java diff --git a/documentation/jetty-documentation/src/main/asciidoc/programming-guide/arch-io.adoc b/documentation/jetty-documentation/src/main/asciidoc/programming-guide/arch-io.adoc index 85752793f36..6155fae3a4e 100644 --- a/documentation/jetty-documentation/src/main/asciidoc/programming-guide/arch-io.adoc +++ b/documentation/jetty-documentation/src/main/asciidoc/programming-guide/arch-io.adoc @@ -234,22 +234,39 @@ The high-level abstraction that Jetty offers to read bytes is `org.eclipse.jetty A `Content.Chunk` groups the following information: * A `ByteBuffer` with the bytes that have been read; it may be empty. -* Whether the read reached end-of-file. -* A failure that might have happened during the read. +* Whether the read reached end-of-file, via its `last` flag. +* A failure that might have happened during the read, via its `getFailure()` method. -The ``Content.Chunk``'s `ByteBuffer` is typically a slice of a different `ByteBuffer` that has been read by a lower layer. -There may be multiple layers between the bottom layer (where the initial read typically happens) and the application layer. +The `Content.Chunk` returned from `Content.Source.read()` can either be a _normal_ chunk (a chunk containing a `ByteBuffer` and a `null` failure), or a _failure_ chunk (a chunk containing an empty `ByteBuffer` and a non-`null` failure). -By slicing the `ByteBuffer` (rather than copying its bytes), there is no copy of the bytes between the layers. +A failure chunk also indicates (via the `last` flag) whether the failure is a fatal (when `last=true`) or transient (when `last=false`) failure. + +A transient failure is a temporary failure that happened during the read, it may be ignored, and it is recoverable: it is possible to call `read()` again and obtain a normal chunk (or a `null` chunk). +Typical cases of transient failures are idle timeout failures, where the read timed out, but the application may decide to insist reading until some other event happens. +The application may convert a transient failure into a fatal failure by calling `Content.Source.fail(Throwable)`. + +A `Content.Source` must be fully consumed by reading all its content, or failed by calling `Content.Source.fail(Throwable)` to signal that the reader is not interested in reading anymore, otherwise it may leak underlying resources. + +Fully consuming a `Content.Source` means reading from it until it returns a `Content.Chunk` whose `last` flag is `true`. +Reading or demanding from an already fully consumed `Content.Source` is always immediately serviced with the last state of the `Content.Source`: a `Content.Chunk` with the `last` flag set to `true`, either an end-of-file chunk, or a failure chunk. + +Once failed, a `Content.Source` is considered fully consumed. +Further attempts to read from a failed `Content.Source` return a failure chunk whose `getFailure()` method returns the exception passed to `Content.Source.fail(Throwable)`. + +When reading a normal chunk, its `ByteBuffer` is typically a slice of a different `ByteBuffer` that has been read by a lower layer. +There may be multiple layers between the bottom layer (where the initial read typically happens) and the application layer that calls `Content.Source.read()`. + +By slicing the `ByteBuffer` (rather than copying its bytes), there is no copy of the bytes between the layers, which yields greater performance. However, this comes with the cost that the `ByteBuffer`, and the associated `Content.Chunk`, have an intrinsic lifecycle: the final consumer of a `Content.Chunk` at the application layer must indicate when it has consumed the chunk, so that the bottom layer may reuse/recycle the `ByteBuffer`. Consuming the chunk means that the bytes in the `ByteBuffer` are read (or ignored), and that the application will not look at or reference that `ByteBuffer` ever again. `Content.Chunk` offers a retain/release model to deal with the `ByteBuffer` lifecycle, with a simple rule: -IMPORTANT: A `Content.Chunk` returned by a call to `Content.Source.read()` **must** be released. +IMPORTANT: A `Content.Chunk` returned by a call to `Content.Source.read()` **must** be released, except for ``Content.Chunk``s that are failure chunks. +Failure chunks _may_ be released, but they do not _need_ to be. -The example below is the idiomatic way to read from a `Content.Source`: +The example below is the idiomatic way of reading from a `Content.Source`: [source,java,indent=0] ---- @@ -258,7 +275,7 @@ include::{doc_code}/org/eclipse/jetty/docs/programming/ContentDocs.java[tags=idi <1> The `read()` that must be paired with a `release()`. <2> The `release()` that pairs the `read()`. -Note how the reads happens in a loop, consuming the `Content.Source` as soon as it has content available to be read, and therefore no backpressure is applied to the reads. +Note how the reads happen in a loop, consuming the `Content.Source` as soon as it has content available to be read, and therefore no backpressure is applied to the reads. An alternative way to read from a `Content.Source`, to use when the chunk is consumed asynchronously, and you don't want to read again until the `Content.Chunk` is consumed, is the following: @@ -273,7 +290,7 @@ Note how the reads do not happen in a loop, and therefore backpressure is applie Since the `Chunk` is consumed asynchronously, you may need to retain it to extend its lifecycle, as explained in xref:pg-arch-io-content-source-chunk[this section]. -You can use `Content.Source` static methods to conveniently read (in a blocking way or non-blocking way), for example via `static Content.Source.asStringAsync(Content.Source, Charset)`, or via an `InputStream` using `static Content.Source.asInputStream(Content.Source source)`. +You can use `Content.Source` static methods to conveniently read (in a blocking way or non-blocking way), for example via `static Content.Source.asStringAsync(Content.Source, Charset)`, or via an `InputStream` using `static Content.Source.asInputStream(Content.Source)`. Refer to the `Content.Source` link:{javadoc-url}/org/eclipse/jetty/io/Content.Source.html[`javadocs`] for further details. diff --git a/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java index 28e0507ef07..eec843bba4b 100644 --- a/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java +++ b/documentation/jetty-documentation/src/main/java/org/eclipse/jetty/docs/programming/ContentDocs.java @@ -57,8 +57,17 @@ public class ContentDocs // If there is a failure reading, handle it. if (Content.Chunk.isFailure(chunk)) { - handleFailure(chunk.getFailure()); - return; + boolean fatal = chunk.isLast(); + if (fatal) + { + handleFatalFailure(chunk.getFailure()); + return; + } + else + { + handleTransientFailure(chunk.getFailure()); + continue; + } } // A normal chunk of content, consume it. @@ -93,10 +102,16 @@ public class ContentDocs return; } - // If there is a failure reading, handle it. + // If there is a failure reading, always treat it as fatal. if (Content.Chunk.isFailure(chunk)) { - handleFailure(chunk.getFailure()); + // If the failure is transient, fail the source + // to indicate that there will be no more reads. + if (!chunk.isLast()) + source.fail(chunk.getFailure()); + + // Handle the failure and stop reading by not demanding. + handleFatalFailure(chunk.getFailure()); return; } @@ -120,7 +135,7 @@ public class ContentDocs { // If there is a failure reading, handle it, // and stop reading by not demanding. - handleFailure(failure); + handleFatalFailure(failure); } }); } @@ -132,7 +147,11 @@ public class ContentDocs } } - private static void handleFailure(Throwable failure) + private static void handleFatalFailure(Throwable failure) + { + } + + private static void handleTransientFailure(Throwable failure) { } @@ -189,7 +208,7 @@ public class ContentDocs if (Content.Chunk.isFailure(chunk)) { - handleFailure(chunk.getFailure()); + handleFatalFailure(chunk.getFailure()); return; } 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 eb3464c9223..474d3515fd1 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 @@ -41,11 +41,11 @@ public interface ContentDecoder /** *

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

- *

The returned {@link RetainableByteBuffer} containing the decoded bytes may - * be empty and must be released via {@link RetainableByteBuffer#release()}.

+ *

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 that must be released + * @return a buffer containing decoded bytes */ public abstract RetainableByteBuffer decode(ByteBuffer buffer); diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Response.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Response.java index d3008194922..10b0db2a60e 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Response.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Response.java @@ -190,6 +190,8 @@ public interface Response if (Content.Chunk.isFailure(chunk)) { response.abort(chunk.getFailure()); + if (!chunk.isLast()) + contentSource.fail(chunk.getFailure()); return; } if (chunk.isLast() && !chunk.hasRemaining()) @@ -207,6 +209,7 @@ public interface Response { chunk.release(); response.abort(x); + contentSource.fail(x); } } } 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 a8808c65abb..4d719a3e687 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 @@ -31,6 +31,7 @@ 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; @@ -587,7 +588,11 @@ public abstract class HttpReceiver if (_chunk == null) return null; if (Content.Chunk.isFailure(_chunk)) - return _chunk; + { + Content.Chunk failure = _chunk; + _chunk = Content.Chunk.next(failure); + return failure; + } // Retain the input chunk because its ByteBuffer will be referenced by the Inflater. if (retain) @@ -602,14 +607,25 @@ public abstract class HttpReceiver { // The decoded ByteBuffer is a transformed "copy" of the // compressed one, so it has its own reference counter. - if (LOG.isDebugEnabled()) - LOG.debug("returning decoded content"); - return Content.Chunk.asChunk(decodedBuffer.getByteBuffer(), false, decodedBuffer); + 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.hasRemaining()) { @@ -788,7 +804,13 @@ public abstract class HttpReceiver try (AutoLock ignored = lock.lock()) { if (Content.Chunk.isFailure(currentChunk)) + { + Throwable cause = currentChunk.getFailure(); + if (!currentChunk.isLast()) + currentChunk = Content.Chunk.from(cause, true); + ExceptionUtil.addSuppressedIfNotAssociated(cause, failure); return false; + } if (currentChunk != null) currentChunk.release(); currentChunk = Content.Chunk.from(failure); diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpSender.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpSender.java index 328ee0df920..9e7d4dcd634 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpSender.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpSender.java @@ -504,7 +504,11 @@ public abstract class HttpSender } if (Content.Chunk.isFailure(chunk)) - throw chunk.getFailure(); + { + Content.Chunk failure = chunk; + chunk = Content.Chunk.next(failure); + throw failure.getFailure(); + } ByteBuffer buffer = chunk.getByteBuffer(); contentBuffer = buffer.asReadOnlyBuffer(); diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/ResponseListeners.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/ResponseListeners.java index cd56720e4c1..de70d3b29fd 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/ResponseListeners.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/ResponseListeners.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.client.Result; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.content.ByteBufferContentSource; +import org.eclipse.jetty.util.ExceptionUtil; import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -679,12 +680,20 @@ public class ResponseListeners if (LOG.isDebugEnabled()) LOG.debug("Content source #{} fail while current chunk is {}", index, currentChunk); if (Content.Chunk.isFailure(currentChunk)) - return; - if (currentChunk != null && currentChunk != ALREADY_READ_CHUNK) - currentChunk.release(); - this.chunk = Content.Chunk.from(failure); - onDemandCallback(); + { + Throwable cause = currentChunk.getFailure(); + if (!currentChunk.isLast()) + chunk = Content.Chunk.from(cause, true); + ExceptionUtil.addSuppressedIfNotAssociated(cause, failure); + } + else + { + if (currentChunk != null && currentChunk != ALREADY_READ_CHUNK) + currentChunk.release(); + this.chunk = Content.Chunk.from(failure); + } registerFailure(this, failure); + onDemandCallback(); } @Override diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/AsyncContentListenerTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/AsyncContentListenerTest.java new file mode 100644 index 00000000000..01d302fa475 --- /dev/null +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/AsyncContentListenerTest.java @@ -0,0 +1,97 @@ +// +// ======================================================================== +// 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.io.Closeable; +import java.net.URI; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.eclipse.jetty.client.transport.HttpConversation; +import org.eclipse.jetty.client.transport.HttpRequest; +import org.eclipse.jetty.client.transport.HttpResponse; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.content.ChunksContentSource; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; + +public class AsyncContentListenerTest +{ + @Test + public void testTransientFailureBecomesTerminal() + { + TestSource originalSource = new TestSource( + Content.Chunk.from(ByteBuffer.wrap(new byte[] {1}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[] {2}), false), + Content.Chunk.from(new NumberFormatException(), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[] {3}), true) + ); + + List collectedChunks = new ArrayList<>(); + Response.AsyncContentListener asyncContentListener = (response, chunk, demander) -> + { + chunk.retain(); + collectedChunks.add(chunk); + demander.run(); + }; + + HttpResponse response = new HttpResponse(new HttpRequest(new HttpClient(), new HttpConversation(), URI.create("http://localhost"))); + asyncContentListener.onContentSource(response, originalSource); + + assertThat(collectedChunks.size(), is(2)); + assertThat(collectedChunks.get(0).isLast(), is(false)); + assertThat(collectedChunks.get(0).getByteBuffer().get(), is((byte)1)); + assertThat(collectedChunks.get(0).getByteBuffer().hasRemaining(), is(false)); + assertThat(collectedChunks.get(1).isLast(), is(false)); + assertThat(collectedChunks.get(1).getByteBuffer().get(), is((byte)2)); + assertThat(collectedChunks.get(1).getByteBuffer().hasRemaining(), is(false)); + + Content.Chunk chunk = originalSource.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), instanceOf(NumberFormatException.class)); + + collectedChunks.forEach(Content.Chunk::release); + originalSource.close(); + } + + private static class TestSource extends ChunksContentSource implements Closeable + { + private Content.Chunk[] chunks; + + public TestSource(Content.Chunk... chunks) + { + super(Arrays.asList(chunks)); + this.chunks = chunks; + } + + @Override + public void close() + { + if (chunks != null) + { + for (Content.Chunk chunk : chunks) + { + if (chunk != null) + chunk.release(); + } + chunks = null; + } + } + } +} 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 new file mode 100644 index 00000000000..fbc58bc298e --- /dev/null +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentDecoderFactoriesTest.java @@ -0,0 +1,117 @@ +// +// ======================================================================== +// 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.nio.ByteBuffer; + +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.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; +import org.junit.jupiter.params.provider.ArgumentsSource; + +import static java.nio.charset.StandardCharsets.US_ASCII; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +public class HttpClientContentDecoderFactoriesTest extends AbstractHttpClientServerTest +{ + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testContentDecoderReturningEmptyRetainableDecodedBuffer(Scenario scenario) throws Exception + { + ArrayByteBufferPool.Tracking bufferPool = new ArrayByteBufferPool.Tracking(); + start(scenario, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + response.getHeaders().add(HttpHeader.CONTENT_ENCODING, "UPPERCASE"); + response.write(true, ByteBuffer.wrap("**THE ANSWER IS FORTY TWO**".getBytes(US_ASCII)), callback); + return true; + } + }); + + client.getContentDecoderFactories().put(new ContentDecoder.Factory("UPPERCASE") + { + @Override + public ContentDecoder newContentDecoder() + { + return byteBuffer -> + { + byte b = byteBuffer.get(); + if (b == '*') + return bufferPool.acquire(0, true); + + 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; + }; + } + }); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .send(); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.getContentAsString(), is("the answer is forty two")); + + assertThat("Decoder leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0)); + } + + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testContentDecoderReturningNonRetainableDecodedBuffer(Scenario scenario) throws Exception + { + start(scenario, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + response.getHeaders().add(HttpHeader.CONTENT_ENCODING, "UPPERCASE"); + response.write(true, ByteBuffer.wrap("THE ANSWER IS FORTY TWO".getBytes(US_ASCII)), callback); + return true; + } + }); + + client.getContentDecoderFactories().put(new ContentDecoder.Factory("UPPERCASE") + { + @Override + public ContentDecoder newContentDecoder() + { + return byteBuffer -> + { + String uppercase = US_ASCII.decode(byteBuffer).toString(); + String lowercase = StringUtil.asciiToLowerCase(uppercase); + return RetainableByteBuffer.wrap(ByteBuffer.wrap(lowercase.getBytes(US_ASCII))); + }; + } + }); + + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .send(); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.getContentAsString(), is("the answer is forty two")); + } +} diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentFailuresTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentFailuresTest.java new file mode 100644 index 00000000000..fcd47fa140c --- /dev/null +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientContentFailuresTest.java @@ -0,0 +1,233 @@ +// +// ======================================================================== +// 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.io.Closeable; +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; + +import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.content.ChunksContentSource; +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.ArgumentsSource; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.fail; + +public class HttpClientContentFailuresTest extends AbstractHttpClientServerTest +{ + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testTerminalFailureInContentMakesSendThrow(Scenario scenario) throws Exception + { + start(scenario, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + callback.succeeded(); + return true; + } + }); + + Exception failure = new NumberFormatException(); + TestContent content = new TestContent( + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), false), + Content.Chunk.from(failure, true) + ); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .method(HttpMethod.POST) + .body(content) + .send(); + fail(); + } + catch (ExecutionException e) + { + assertThat(e.getCause(), sameInstance(failure)); + } + + Content.Chunk chunk = content.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), sameInstance(failure)); + + content.close(); + } + + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testTransientFailureInContentConsideredTerminalAndMakesSendThrow(Scenario scenario) throws Exception + { + start(scenario, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + callback.succeeded(); + return true; + } + }); + + Exception failure = new NumberFormatException(); + TestContent content = new TestContent( + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), false), + Content.Chunk.from(failure, false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{3}), true) + ); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .method(HttpMethod.POST) + .body(content) + .send(); + fail(); + } + catch (ExecutionException e) + { + assertThat(e.getCause(), sameInstance(failure)); + } + + Content.Chunk chunk = content.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), sameInstance(failure)); + + content.close(); + } + + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testTransientTimeoutFailureMakesSendThrowTimeoutException(Scenario scenario) throws Exception + { + start(scenario, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + callback.succeeded(); + return true; + } + }); + + Exception failure = new TimeoutException(); + TestContent content = new TestContent( + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), false), + Content.Chunk.from(failure, false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{3}), true) + ); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .method(HttpMethod.POST) + .body(content) + .send(); + fail(); + } + catch (TimeoutException e) + { + assertThat(e, sameInstance(failure)); + } + + Content.Chunk chunk = content.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), sameInstance(failure)); + + content.close(); + } + + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testTerminalTimeoutFailureMakesSendThrowTimeoutException(Scenario scenario) throws Exception + { + start(scenario, new Handler.Abstract() + { + @Override + public boolean handle(Request request, Response response, Callback callback) + { + callback.succeeded(); + return true; + } + }); + + Exception failure = new TimeoutException(); + TestContent content = new TestContent( + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), false), + Content.Chunk.from(failure, true) + ); + + try + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .method(HttpMethod.POST) + .body(content) + .send(); + fail(); + } + catch (TimeoutException e) + { + assertThat(e, sameInstance(failure)); + } + + Content.Chunk chunk = content.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), sameInstance(failure)); + + content.close(); + } + + public static class TestContent extends ChunksContentSource implements Closeable, org.eclipse.jetty.client.Request.Content + { + private Content.Chunk[] chunks; + + public TestContent(Content.Chunk... chunks) + { + super(Arrays.asList(chunks)); + this.chunks = chunks; + } + + @Override + public void close() + { + if (chunks != null) + { + for (Content.Chunk chunk : chunks) + { + if (chunk != null) + chunk.release(); + } + chunks = null; + } + } + } +} diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/transport/ResponseListenersTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/transport/ResponseListenersTest.java index b7b35f294d2..bafe19599ff 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/transport/ResponseListenersTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/transport/ResponseListenersTest.java @@ -13,18 +13,20 @@ package org.eclipse.jetty.client.transport; +import java.io.Closeable; import java.nio.ByteBuffer; import java.util.Arrays; import java.util.List; -import java.util.Queue; -import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeoutException; import org.eclipse.jetty.client.Response; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.content.ChunksContentSource; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; public class ResponseListenersTest @@ -32,13 +34,13 @@ public class ResponseListenersTest @Test public void testContentSourceDemultiplexerSpuriousWakeup() { - SimpleSource contentSource = new SimpleSource(Arrays.asList( + TestSource contentSource = new TestSource( Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), null, Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), false), null, Content.Chunk.from(ByteBuffer.wrap(new byte[]{3}), true) - )); + ); List chunks = new CopyOnWriteArrayList<>(); @@ -57,7 +59,6 @@ public class ResponseListenersTest source.demand(this); return; } - chunk.release(); if (!chunk.isLast()) source.demand(this); } @@ -83,66 +84,170 @@ public class ResponseListenersTest assertThat(chunks.get(4).getByteBuffer().get(), is((byte)3)); assertThat(chunks.get(5).isLast(), is(true)); assertThat(chunks.get(5).getByteBuffer().get(), is((byte)3)); + + chunks.forEach(Content.Chunk::release); + contentSource.close(); } - private static class SimpleSource implements Content.Source + @Test + public void testContentSourceDemultiplexerFailOnTransientException() { - private static final Content.Chunk SPURIOUS_WAKEUP = new Content.Chunk() - { - @Override - public ByteBuffer getByteBuffer() - { - return null; - } + TestSource contentSource = new TestSource( + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), false), + null, + Content.Chunk.from(new TimeoutException("timeout"), false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{3}), true) + ); - @Override - public boolean isLast() + List chunks = new CopyOnWriteArrayList<>(); + ResponseListeners responseListeners = new ResponseListeners(); + Response.ContentSourceListener contentSourceListener = (r, source) -> + { + Runnable runnable = new Runnable() { - return false; - } + @Override + public void run() + { + Content.Chunk chunk = source.read(); + chunks.add(chunk); + if (chunk == null) + { + source.demand(this); + return; + } + if (Content.Chunk.isFailure(chunk, false)) + source.fail(new NumberFormatException()); + if (!chunk.isLast()) + source.demand(this); + } + }; + source.demand(runnable); }; - private final Queue chunks = new ConcurrentLinkedQueue<>(); - private Runnable demand; + // Add 2 ContentSourceListeners to enable the use of ContentSourceDemultiplexer. + responseListeners.addContentSourceListener(contentSourceListener); + responseListeners.addContentSourceListener(contentSourceListener); - public SimpleSource(List chunks) + responseListeners.notifyContentSource(null, contentSource); + + assertThat(chunks.size(), is(8)); + assertThat(chunks.get(0).getByteBuffer().get(), is((byte)1)); + assertThat(chunks.get(0).isLast(), is(false)); + assertThat(chunks.get(1).getByteBuffer().get(), is((byte)1)); + assertThat(chunks.get(1).isLast(), is(false)); + assertThat(chunks.get(2).getByteBuffer().get(), is((byte)2)); + assertThat(chunks.get(2).isLast(), is(false)); + assertThat(chunks.get(3).getByteBuffer().get(), is((byte)2)); + assertThat(chunks.get(3).isLast(), is(false)); + + // Failures are not alternated because ContentSourceDemultiplexer is failed, + // it immediately services demands. + assertThat(Content.Chunk.isFailure(chunks.get(4), false), is(true)); + assertThat(chunks.get(4).getFailure(), instanceOf(TimeoutException.class)); + assertThat(Content.Chunk.isFailure(chunks.get(5), true), is(true)); + assertThat(chunks.get(5).getFailure(), instanceOf(NumberFormatException.class)); + assertThat(Content.Chunk.isFailure(chunks.get(6), false), is(true)); + assertThat(chunks.get(6).getFailure(), instanceOf(TimeoutException.class)); + assertThat(Content.Chunk.isFailure(chunks.get(7), true), is(true)); + assertThat(chunks.get(7).getFailure(), instanceOf(NumberFormatException.class)); + + Content.Chunk chunk = contentSource.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), instanceOf(NumberFormatException.class)); + + chunks.forEach(Content.Chunk::release); + contentSource.close(); + } + + @Test + public void testContentSourceDemultiplexerFailOnTerminalException() + { + TestSource contentSource = new TestSource( + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), false), + null, + Content.Chunk.from(new ArithmeticException(), true) + ); + + List chunks = new CopyOnWriteArrayList<>(); + ResponseListeners responseListeners = new ResponseListeners(); + Response.ContentSourceListener contentSourceListener = (r, source) -> { - for (Content.Chunk chunk : chunks) + Runnable runnable = new Runnable() { - this.chunks.add(chunk != null ? chunk : SPURIOUS_WAKEUP); - } + @Override + public void run() + { + Content.Chunk chunk = source.read(); + chunks.add(chunk); + if (chunk == null) + { + source.demand(this); + return; + } + if (Content.Chunk.isFailure(chunk)) + source.fail(new NumberFormatException()); + if (!chunk.isLast()) + source.demand(this); + } + }; + source.demand(runnable); + }; + // Add 2 ContentSourceListeners to enable the use of ContentSourceDemultiplexer. + responseListeners.addContentSourceListener(contentSourceListener); + responseListeners.addContentSourceListener(contentSourceListener); + + responseListeners.notifyContentSource(null, contentSource); + + assertThat(chunks.size(), is(6)); + assertThat(chunks.get(0).getByteBuffer().get(), is((byte)1)); + assertThat(chunks.get(0).isLast(), is(false)); + assertThat(chunks.get(1).getByteBuffer().get(), is((byte)1)); + assertThat(chunks.get(1).isLast(), is(false)); + assertThat(chunks.get(2).getByteBuffer().get(), is((byte)2)); + assertThat(chunks.get(2).isLast(), is(false)); + assertThat(chunks.get(3).getByteBuffer().get(), is((byte)2)); + assertThat(chunks.get(3).isLast(), is(false)); + assertThat(Content.Chunk.isFailure(chunks.get(4), true), is(true)); + assertThat(chunks.get(4).getFailure(), instanceOf(ArithmeticException.class)); + assertThat(Content.Chunk.isFailure(chunks.get(5), true), is(true)); + assertThat(chunks.get(5).getFailure(), instanceOf(ArithmeticException.class)); + + Content.Chunk chunk = contentSource.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), instanceOf(ArithmeticException.class)); + assertThat(chunk.getFailure().getSuppressed().length, is(2)); + assertThat(chunk.getFailure().getSuppressed()[0], instanceOf(NumberFormatException.class)); + assertThat(chunk.getFailure().getSuppressed()[1], instanceOf(NumberFormatException.class)); + + chunks.forEach(Content.Chunk::release); + contentSource.close(); + } + + private static class TestSource extends ChunksContentSource implements Closeable + { + private Content.Chunk[] chunks; + + public TestSource(Content.Chunk... chunks) + { + super(Arrays.asList(chunks)); + this.chunks = chunks; } @Override - public Content.Chunk read() + public void close() { - if (demand != null) - throw new IllegalStateException(); - - Content.Chunk chunk = chunks.poll(); - return chunk == SPURIOUS_WAKEUP ? null : chunk; - } - - @Override - public void demand(Runnable demandCallback) - { - if (demand != null) - throw new IllegalStateException(); - - if (!chunks.isEmpty()) - demandCallback.run(); - else - demand = demandCallback; - } - - @Override - public void fail(Throwable failure) - { - demand = null; - while (!chunks.isEmpty()) + if (chunks != null) { - Content.Chunk chunk = chunks.poll(); - if (chunk != null) - chunk.release(); + for (Content.Chunk chunk : chunks) + { + if (chunk != null) + chunk.release(); + } + chunks = null; } } } diff --git a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java index 680531847e5..dad5be63bae 100644 --- a/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java +++ b/jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java @@ -196,7 +196,9 @@ public class HttpStreamOverFCGI implements HttpStream { if (_chunk == null) _chunk = Content.Chunk.EOF; - else if (!_chunk.isLast() && !(Content.Chunk.isFailure(_chunk))) + else if (Content.Chunk.isFailure(_chunk, false)) + _chunk = Content.Chunk.from(_chunk.getFailure(), true); + else if (!_chunk.isLast()) throw new IllegalStateException(); } diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index 0cf5727dc6e..a6758b5b076 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java @@ -407,7 +407,12 @@ public class MultiPart // because the content sources may not be read, or their chunks could be // further retained, so those chunks must not be linked to the original ones. List chunks = content.stream() - .map(chunk -> Content.Chunk.from(chunk.getByteBuffer().slice(), chunk.isLast())) + .map(chunk -> + { + if (Content.Chunk.isFailure(chunk)) + return chunk; + return Content.Chunk.from(chunk.getByteBuffer().slice(), chunk.isLast()); + }) .toList(); ChunksContentSource newContentSource = new ChunksContentSource(chunks); chunks.forEach(Content.Chunk::release); @@ -759,8 +764,16 @@ public class MultiPart case CONTENT -> { Content.Chunk chunk = part.getContentSource().read(); - if (chunk == null || Content.Chunk.isFailure(chunk)) + if (chunk == null) + yield null; + if (Content.Chunk.isFailure(chunk, true)) + { + try (AutoLock ignored = lock.lock()) + { + errorChunk = chunk; + } yield chunk; + } if (!chunk.isLast()) yield chunk; state = State.MIDDLE; diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java index 5ccff5406a6..c96c2035b65 100644 --- a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java +++ b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/MultiPartFormDataTest.java @@ -29,6 +29,7 @@ import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.content.AsyncContent; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -36,6 +37,7 @@ import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static java.nio.charset.StandardCharsets.ISO_8859_1; +import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsStringIgnoringCase; @@ -805,6 +807,111 @@ public class MultiPartFormDataTest } } + @Test + public void testContentSourceCanBeFailed() + { + MultiPartFormData.ContentSource source = new MultiPartFormData.ContentSource("boundary"); + source.addPart(new MultiPart.ChunksPart("part1", "file1", HttpFields.EMPTY, List.of( + Content.Chunk.from(ByteBuffer.wrap("the answer".getBytes(US_ASCII)), false), + Content.Chunk.from(new NumberFormatException(), false), + Content.Chunk.from(ByteBuffer.wrap(" is 42".getBytes(US_ASCII)), true) + ))); + source.close(); + + Content.Chunk chunk; + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("--boundary\r\n")); + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("Content-Disposition: form-data; name=\"part1\"; filename=\"file1\"\r\n\r\n")); + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("the answer")); + + chunk = source.read(); + assertThat(Content.Chunk.isFailure(chunk, false), is(true)); + assertThat(chunk.getFailure(), instanceOf(NumberFormatException.class)); + source.fail(chunk.getFailure()); + + chunk = source.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), instanceOf(NumberFormatException.class)); + } + + @Test + public void testTransientFailuresAreReturned() + { + MultiPartFormData.ContentSource source = new MultiPartFormData.ContentSource("boundary"); + source.addPart(new MultiPart.ChunksPart("part1", "file1", HttpFields.EMPTY, List.of( + Content.Chunk.from(ByteBuffer.wrap("the answer".getBytes(US_ASCII)), false), + Content.Chunk.from(new NumberFormatException(), false), + Content.Chunk.from(ByteBuffer.wrap(" is 42".getBytes(US_ASCII)), true) + ))); + source.close(); + + Content.Chunk chunk; + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("--boundary\r\n")); + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("Content-Disposition: form-data; name=\"part1\"; filename=\"file1\"\r\n\r\n")); + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("the answer")); + + chunk = source.read(); + assertThat(Content.Chunk.isFailure(chunk, false), is(true)); + assertThat(chunk.getFailure(), instanceOf(NumberFormatException.class)); + + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is(" is 42")); + chunk = source.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("\r\n--boundary--\r\n")); + + chunk = source.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(Content.Chunk.isFailure(chunk), is(false)); + assertThat(chunk.hasRemaining(), is(false)); + } + + @Test + public void testTerminalFailureIsTerminal() + { + MultiPartFormData.ContentSource source = new MultiPartFormData.ContentSource("boundary"); + source.addPart(new MultiPart.ChunksPart("part1", "file1", HttpFields.EMPTY, List.of( + Content.Chunk.from(ByteBuffer.wrap("the answer".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap(" is 42".getBytes(US_ASCII)), false), + Content.Chunk.from(new NumberFormatException(), true) + ))); + source.close(); + + Content.Chunk chunk; + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("--boundary\r\n")); + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("Content-Disposition: form-data; name=\"part1\"; filename=\"file1\"\r\n\r\n")); + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is("the answer")); + chunk = source.read(); + assertThat(chunk.isLast(), is(false)); + assertThat(BufferUtil.toString(chunk.getByteBuffer(), UTF_8), is(" is 42")); + + chunk = source.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), instanceOf(NumberFormatException.class)); + + chunk = source.read(); + assertThat(Content.Chunk.isFailure(chunk, true), is(true)); + assertThat(chunk.getFailure(), instanceOf(NumberFormatException.class)); + } + private class TestContent extends AsyncContent { @Override diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java index cb036274c09..0358f858396 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java @@ -51,7 +51,7 @@ public class ChunkAccumulator chunk.retain(); return _chunks.add(chunk); } - return _chunks.add(Chunk.from(BufferUtil.copy(chunk.getByteBuffer()), chunk.isLast(), () -> {})); + return _chunks.add(Chunk.from(BufferUtil.copy(chunk.getByteBuffer()), chunk.isLast())); } else if (Chunk.isFailure(chunk)) { @@ -191,6 +191,8 @@ public class ChunkAccumulator if (Chunk.isFailure(chunk)) { completeExceptionally(chunk.getFailure()); + if (!chunk.isLast()) + _source.fail(chunk.getFailure()); break; } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index 9cc3a3f4cfd..e53a802597d 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -95,6 +95,11 @@ public class Content /** *

A source of content that can be read with a read/demand model.

+ *

To avoid leaking its resources, a source must either:

+ *
    + *
  • be read until it returns a {@link Chunk#isLast() last chunk}, either EOF or a terminal failure
    • + *
  • be {@link #fail(Throwable) failed}
    • + *
*

Idiomatic usage

*

The read/demand model typical usage is the following:

* 
{@code
@@ -110,12 +115,19 @@ public class Content
      *         }
      *
      *         // The chunk is a failure.
-     *         if (Content.Chunk.isFailure(chunk)) {
-     *             // Handle the failure.
-     *             Throwable cause = chunk.getFailure();
-     *             boolean transient = !chunk.isLast();
-     *             // ...
-     *             return;
+     *         if (Content.Chunk.isFailure(chunk))
+     *         {
+     *             boolean fatal = chunk.isLast();
+     *             if (fatal)
+     *             {
+     *                 handleFatalFailure(chunk.getFailure());
+     *                 return;
+     *             }
+     *             else
+     *             {
+     *                 handleTransientFailure(chunk.getFailure());
+     *                 continue;
+     *             }
      *         }
      *
      *         // It's a valid chunk, consume the chunk's bytes.
@@ -124,6 +136,10 @@ public class Content
      *
      *         // Release the chunk when it has been consumed.
      *         chunk.release();
+     *
+     *         // Exit if the Content.Source is fully consumed.
+     *         if (chunk.isLast())
+     *             break;
      *     }
      * }
      * }
@@ -859,9 +875,11 @@ public class Content */ default Chunk asReadOnly() { - if (!canRetain()) + if (getByteBuffer().isReadOnly()) return this; - return asChunk(getByteBuffer().asReadOnlyBuffer(), isLast(), this); + if (canRetain()) + return asChunk(getByteBuffer().asReadOnlyBuffer(), isLast(), this); + return from(getByteBuffer().asReadOnlyBuffer(), isLast()); } /** diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java index 766732ea616..76ee447421f 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ChunksContentSource.java @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; import java.util.List; +import java.util.Objects; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.util.thread.AutoLock; @@ -40,9 +41,23 @@ public class ChunksContentSource implements Content.Source public ChunksContentSource(Collection chunks) { - chunks.forEach(Content.Chunk::retain); + long sum = 0L; + Iterator it = chunks.iterator(); + while (it.hasNext()) + { + Content.Chunk chunk = it.next(); + if (chunk != null) + { + if (it.hasNext() && chunk.isLast()) + throw new IllegalArgumentException("Collection cannot contain a last Content.Chunk that is not at the last position: " + chunk); + sum += chunk.getByteBuffer().remaining(); + } + } + // Only retain after the previous loop checked the collection is valid. + chunks.stream().filter(Objects::nonNull).forEach(Content.Chunk::retain); + this.chunks = chunks; - this.length = chunks.stream().mapToLong(c -> c.getByteBuffer().remaining()).sum(); + this.length = sum; } public Collection getChunks() @@ -60,18 +75,16 @@ public class ChunksContentSource implements Content.Source public Content.Chunk read() { Content.Chunk chunk; - boolean last; try (AutoLock ignored = lock.lock()) { if (terminated != null) return terminated; if (iterator == null) iterator = chunks.iterator(); - if (!iterator.hasNext()) - return terminated = Content.Chunk.EOF; chunk = iterator.next(); - last = !iterator.hasNext(); - if (last) + if (chunk != null && chunk.isLast()) + terminated = Content.Chunk.next(chunk); + if (terminated == null && !iterator.hasNext()) terminated = Content.Chunk.EOF; } return chunk; @@ -132,6 +145,6 @@ public class ChunksContentSource implements Content.Source chunksToRelease = List.copyOf(chunks); } } - chunksToRelease.forEach(Content.Chunk::release); + chunksToRelease.stream().filter(Objects::nonNull).forEach(Content.Chunk::release); } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java index f44d9283261..6461f9489c1 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceCompletableFuture.java @@ -48,7 +48,9 @@ import org.eclipse.jetty.io.Content; * } * } * - * new CompletableUTF8String(source).thenAccept(System.err::println); + * CompletableUTF8String cs = new CompletableUTF8String(source); + * cs.parse(); + * String s = cs.get(); * } */ public abstract class ContentSourceCompletableFuture extends CompletableFuture @@ -83,9 +85,17 @@ public abstract class ContentSourceCompletableFuture extends CompletableFutur } if (Content.Chunk.isFailure(chunk)) { - if (!chunk.isLast() && onTransientFailure(chunk.getFailure())) - continue; - completeExceptionally(chunk.getFailure()); + if (chunk.isLast()) + { + completeExceptionally(chunk.getFailure()); + } + else + { + if (onTransientFailure(chunk.getFailure())) + continue; + _content.fail(chunk.getFailure()); + completeExceptionally(chunk.getFailure()); + } return; } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceInputStream.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceInputStream.java index d25137481cf..46dfbf74f32 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceInputStream.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourceInputStream.java @@ -56,9 +56,9 @@ public class ContentSourceInputStream extends InputStream { if (Content.Chunk.isFailure(chunk)) { - Content.Chunk c = chunk; - chunk = Content.Chunk.next(c); - throw IO.rethrow(c.getFailure()); + Content.Chunk failure = chunk; + chunk = Content.Chunk.next(failure); + throw IO.rethrow(failure.getFailure()); } ByteBuffer byteBuffer = chunk.getByteBuffer(); @@ -125,9 +125,11 @@ public class ContentSourceInputStream extends InputStream // Handle a failure as read would if (Content.Chunk.isFailure(chunk)) { - Content.Chunk c = chunk; - chunk = Content.Chunk.next(c); - throw IO.rethrow(c.getFailure()); + Content.Chunk failure = chunk; + chunk = Content.Chunk.next(failure); + if (!failure.isLast()) + content.fail(failure.getFailure()); + throw IO.rethrow(failure.getFailure()); } contentSkipped = chunk.hasRemaining(); diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourcePublisher.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourcePublisher.java index 15858a928ff..95435d53c59 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourcePublisher.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/ContentSourcePublisher.java @@ -128,6 +128,8 @@ public class ContentSourcePublisher implements Flow.Publisher if (Content.Chunk.isFailure(chunk)) { terminate(); + if (!chunk.isLast()) + content.fail(chunk.getFailure()); subscriber.onError(chunk.getFailure()); return; } 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 7f2d6330f89..e34d5903c6c 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 @@ -66,7 +66,12 @@ public abstract class ContentSourceTransformer implements Content.Source } if (Content.Chunk.isFailure(rawChunk)) - return rawChunk; + { + Content.Chunk failure = rawChunk; + rawChunk = Content.Chunk.next(rawChunk); + needsRawRead = rawChunk == null; + return failure; + } if (Content.Chunk.isFailure(transformedChunk)) return transformedChunk; diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentCopier.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentCopier.java index ed294dea85f..c57fae5cc02 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentCopier.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentCopier.java @@ -61,14 +61,7 @@ public class ContentCopier extends IteratingNestedCallback return Action.SCHEDULED; if (Content.Chunk.isFailure(current)) - { - if (current.isLast()) - throw current.getFailure(); - if (LOG.isDebugEnabled()) - LOG.debug("ignored transient failure", current.getFailure()); - succeeded(); - return Action.SCHEDULED; - } + throw current.getFailure(); sink.write(current.isLast(), current.getByteBuffer(), this); return Action.SCHEDULED; diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceByteBuffer.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceByteBuffer.java index 9d286e42596..9db5923b287 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceByteBuffer.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceByteBuffer.java @@ -47,6 +47,8 @@ public class ContentSourceByteBuffer implements Runnable if (Content.Chunk.isFailure(chunk)) { promise.failed(chunk.getFailure()); + if (!chunk.isLast()) + source.fail(chunk.getFailure()); return; } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceConsumer.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceConsumer.java index 5992ee06e21..7b802ae2fbb 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceConsumer.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceConsumer.java @@ -44,6 +44,8 @@ public class ContentSourceConsumer implements Invocable.Task if (Content.Chunk.isFailure(chunk)) { callback.failed(chunk.getFailure()); + if (!chunk.isLast()) + source.fail(chunk.getFailure()); return; } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceString.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceString.java index 68096a00571..9ae9605dd25 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceString.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceString.java @@ -45,6 +45,8 @@ public class ContentSourceString if (Content.Chunk.isFailure(chunk)) { promise.failed(chunk.getFailure()); + if (!chunk.isLast()) + content.fail(chunk.getFailure()); return; } text.append(chunk.getByteBuffer()); diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ChunkAccumulatorTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ChunkAccumulatorTest.java new file mode 100644 index 00000000000..8a7a47a7d37 --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ChunkAccumulatorTest.java @@ -0,0 +1,62 @@ +// +// ======================================================================== +// 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.io; + +import java.nio.ByteBuffer; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.fail; + +public class ChunkAccumulatorTest +{ + @Test + public void testTransientErrorsBecomeTerminalErrors() throws Exception + { + TimeoutException originalFailure = new TimeoutException("timeout 1"); + TestSource originalSource = new TestSource( + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + null, + Content.Chunk.from(originalFailure, false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), true) + ); + + ChunkAccumulator chunkAccumulator = new ChunkAccumulator(); + CompletableFuture completableFuture = chunkAccumulator.readAll(originalSource); + + try + { + completableFuture.get(); + fail(); + } + catch (ExecutionException e) + { + assertThat(e.getCause(), sameInstance(originalFailure)); + } + + Content.Chunk chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.getFailure(), sameInstance(originalFailure)); + + originalSource.close(); + } +} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceCompletableFutureTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceCompletableFutureTest.java new file mode 100644 index 00000000000..4b97ded5af4 --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceCompletableFutureTest.java @@ -0,0 +1,123 @@ +// +// ======================================================================== +// 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.io; + +import java.nio.ByteBuffer; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; + +import org.eclipse.jetty.io.content.ContentSourceCompletableFuture; +import org.eclipse.jetty.util.Utf8StringBuilder; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.fail; + +public class ContentSourceCompletableFutureTest +{ + @Test + public void testTransientErrorsBecomeTerminalErrors() throws Exception + { + TimeoutException originalFailure = new TimeoutException("timeout 1"); + TestSource originalSource = new TestSource( + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'1'}), false), + null, + Content.Chunk.from(originalFailure, false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'2'}), true) + ); + + ContentSourceCompletableFuture contentSourceCompletableFuture = new ContentSourceCompletableFuture<>(originalSource) + { + final Utf8StringBuilder builder = new Utf8StringBuilder(); + + @Override + protected String parse(Content.Chunk chunk) + { + if (chunk.hasRemaining()) + builder.append(chunk.getByteBuffer()); + if (!chunk.isLast()) + return null; + return builder.takeCompleteString(IllegalStateException::new); + } + }; + + try + { + contentSourceCompletableFuture.parse(); + contentSourceCompletableFuture.get(); + fail(); + } + catch (ExecutionException e) + { + assertThat(e.getCause(), sameInstance(originalFailure)); + } + + Content.Chunk chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.getFailure(), sameInstance(originalFailure)); + + originalSource.close(); + } + + @Test + public void testTransientErrorsAreIgnored() throws Exception + { + TestSource originalSource = new TestSource( + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'1'}), false), + null, + Content.Chunk.from(new TimeoutException("timeout 1"), false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'2'}), false), + null, + Content.Chunk.from(new TimeoutException("timeout 2"), false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'3'}), true) + ); + + ContentSourceCompletableFuture contentSourceCompletableFuture = new ContentSourceCompletableFuture<>(originalSource) + { + final Utf8StringBuilder builder = new Utf8StringBuilder(); + + @Override + protected String parse(Content.Chunk chunk) + { + if (chunk.hasRemaining()) + builder.append(chunk.getByteBuffer()); + if (!chunk.isLast()) + return null; + return builder.takeCompleteString(IllegalStateException::new); + } + + @Override + protected boolean onTransientFailure(Throwable cause) + { + return true; + } + }; + + contentSourceCompletableFuture.parse(); + assertThat(contentSourceCompletableFuture.get(), is("123")); + + Content.Chunk chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.hasRemaining(), is(false)); + + originalSource.close(); + } +} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceInputStreamTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceInputStreamTest.java new file mode 100644 index 00000000000..bab24f35cbc --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceInputStreamTest.java @@ -0,0 +1,127 @@ +// +// ======================================================================== +// 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.io; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.concurrent.TimeoutException; + +import org.eclipse.jetty.io.content.ContentSourceInputStream; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.fail; + +public class ContentSourceInputStreamTest +{ + @Test + public void testTransientErrorsAreRethrownOnRead() throws Exception + { + TimeoutException originalFailure1 = new TimeoutException("timeout 1"); + TimeoutException originalFailure2 = new TimeoutException("timeout 2"); + TestSource originalSource = new TestSource( + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'1'}), false), + null, + Content.Chunk.from(originalFailure1, false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'2'}), false), + null, + Content.Chunk.from(originalFailure2, false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'3'}), true) + ); + + ContentSourceInputStream contentSourceInputStream = new ContentSourceInputStream(originalSource); + + byte[] buf = new byte[16]; + + int read = contentSourceInputStream.read(buf); + assertThat(read, is(1)); + assertThat(buf[0], is((byte)'1')); + try + { + contentSourceInputStream.read(); + fail(); + } + catch (IOException e) + { + assertThat(e.getCause(), sameInstance(originalFailure1)); + } + read = contentSourceInputStream.read(buf); + assertThat(read, is(1)); + assertThat(buf[0], is((byte)'2')); + try + { + contentSourceInputStream.read(); + fail(); + } + catch (IOException e) + { + assertThat(e.getCause(), sameInstance(originalFailure2)); + } + read = contentSourceInputStream.read(buf); + assertThat(read, is(1)); + assertThat(buf[0], is((byte)'3')); + + read = contentSourceInputStream.read(buf); + assertThat(read, is(-1)); + + Content.Chunk chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.hasRemaining(), is(false)); + assertThat(Content.Chunk.isFailure(chunk), is(false)); + + contentSourceInputStream.close(); + + originalSource.close(); + } + + @Test + public void testNextTransientErrorIsRethrownOnClose() throws Exception + { + TimeoutException originalFailure = new TimeoutException("timeout"); + TestSource originalSource = new TestSource( + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'1'}), false), + Content.Chunk.from(originalFailure, false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'2'}), true) + ); + + ContentSourceInputStream contentSourceInputStream = new ContentSourceInputStream(originalSource); + + byte[] buf = new byte[16]; + + int read = contentSourceInputStream.read(buf); + assertThat(read, is(1)); + assertThat(buf[0], is((byte)'1')); + try + { + contentSourceInputStream.close(); + fail(); + } + catch (IOException e) + { + assertThat(e.getCause(), sameInstance(originalFailure)); + } + + Content.Chunk chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.getFailure(), sameInstance(originalFailure)); + + originalSource.close(); + } +} 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 820b09b9428..2a71e9a494b 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 @@ -14,10 +14,12 @@ package org.eclipse.jetty.io; import java.io.IOException; +import java.nio.ByteBuffer; import java.util.ArrayDeque; import java.util.List; import java.util.Locale; import java.util.Queue; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -32,6 +34,8 @@ import org.junit.jupiter.params.provider.ValueSource; import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -312,6 +316,87 @@ public class ContentSourceTransformerTest assertTrue(Content.Chunk.isFailure(chunk, true)); } + @Test + public void testTransientFailuresFromOriginalSourceAreReturned() + { + TimeoutException originalFailure1 = new TimeoutException("timeout 1"); + TimeoutException originalFailure2 = new TimeoutException("timeout 2"); + TestSource originalSource = new TestSource( + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'A'}), false), + Content.Chunk.from(originalFailure1, false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'B'}), false), + Content.Chunk.from(originalFailure2, false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'C'}), true) + ); + + WordSplitLowCaseTransformer transformer = new WordSplitLowCaseTransformer(originalSource); + + assertEquals('a', (char)transformer.read().getByteBuffer().get()); + Content.Chunk chunk = transformer.read(); + assertThat(chunk.getFailure(), sameInstance(originalFailure1)); + assertThat(chunk.isLast(), is(false)); + assertEquals('b', (char)transformer.read().getByteBuffer().get()); + chunk = transformer.read(); + assertThat(chunk.getFailure(), sameInstance(originalFailure2)); + assertThat(chunk.isLast(), is(false)); + assertEquals('c', (char)transformer.read().getByteBuffer().get()); + + chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.hasRemaining(), is(false)); + assertThat(Content.Chunk.isFailure(chunk), is(false)); + + originalSource.close(); + } + + @Test + public void testTransientFailuresFromTransformationAreReturned() + { + TimeoutException originalFailure1 = new TimeoutException("timeout 1"); + TimeoutException originalFailure2 = new TimeoutException("timeout 2"); + TestSource originalSource = new TestSource( + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'A'}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'B'}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'C'}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'D'}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'E'}), true) + ); + + ContentSourceTransformer transformer = new ContentSourceTransformer(originalSource) + { + @Override + protected Content.Chunk transform(Content.Chunk rawChunk) + { + if (rawChunk == null) + return null; + String decoded = UTF_8.decode(rawChunk.getByteBuffer().duplicate()).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()); + }; + } + }; + + assertEquals('A', (char)transformer.read().getByteBuffer().get()); + Content.Chunk chunk = transformer.read(); + assertThat(chunk.getFailure(), sameInstance(originalFailure1)); + assertThat(chunk.isLast(), is(false)); + assertEquals('C', (char)transformer.read().getByteBuffer().get()); + chunk = transformer.read(); + assertThat(chunk.getFailure(), sameInstance(originalFailure2)); + assertThat(chunk.isLast(), is(false)); + assertEquals('E', (char)transformer.read().getByteBuffer().get()); + + chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.hasRemaining(), is(false)); + assertThat(Content.Chunk.isFailure(chunk), is(false)); + + originalSource.close(); + } + private static class WordSplitLowCaseTransformer extends ContentSourceTransformer { private final Queue chunks = new ArrayDeque<>(); diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentTest.java index add174fb4dd..211914a4b80 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentTest.java @@ -14,18 +14,43 @@ package org.eclipse.jetty.io; import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import org.eclipse.jetty.util.BufferUtil; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.sameInstance; public class ContentTest { + @Test + public void testAsReadOnly() + { + assertThat(Content.Chunk.EOF.asReadOnly(), sameInstance(Content.Chunk.EOF)); + assertThat(Content.Chunk.EMPTY.asReadOnly(), sameInstance(Content.Chunk.EMPTY)); + + assertThat(Content.Chunk.from(BufferUtil.EMPTY_BUFFER, true).asReadOnly(), sameInstance(Content.Chunk.EOF)); + assertThat(Content.Chunk.from(BufferUtil.EMPTY_BUFFER, false).asReadOnly(), sameInstance(Content.Chunk.EMPTY)); + + Content.Chunk failureChunk = Content.Chunk.from(new NumberFormatException()); + assertThat(failureChunk.asReadOnly(), sameInstance(failureChunk)); + + Content.Chunk chunk = Content.Chunk.from(ByteBuffer.wrap(new byte[1]).asReadOnlyBuffer(), false); + assertThat(chunk.asReadOnly(), sameInstance(chunk)); + + Content.Chunk rwChunk = Content.Chunk.from(ByteBuffer.wrap("abc".getBytes(StandardCharsets.US_ASCII)), false); + Content.Chunk roChunk = rwChunk.asReadOnly(); + assertThat(rwChunk, not(sameInstance(roChunk))); + assertThat(BufferUtil.toString(rwChunk.getByteBuffer(), StandardCharsets.US_ASCII), equalTo(BufferUtil.toString(roChunk.getByteBuffer(), StandardCharsets.US_ASCII))); + } + @Test public void testFromEmptyByteBufferWithoutReleaser() { diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/TestSink.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/TestSink.java new file mode 100644 index 00000000000..e270e2bd0fc --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/TestSink.java @@ -0,0 +1,39 @@ +// +// ======================================================================== +// 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.io; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; + +import org.eclipse.jetty.util.Callback; + +public class TestSink implements Content.Sink +{ + private List accumulatedChunks = new ArrayList<>(); + + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + accumulatedChunks.add(Content.Chunk.from(byteBuffer, last)); + callback.succeeded(); + } + + public List takeAccumulatedChunks() + { + List chunks = accumulatedChunks; + accumulatedChunks = null; + return chunks; + } +} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/TestSource.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/TestSource.java new file mode 100644 index 00000000000..05743d296a9 --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/TestSource.java @@ -0,0 +1,38 @@ +// +// ======================================================================== +// 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.io; + +import java.io.Closeable; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; + +import org.eclipse.jetty.io.content.ChunksContentSource; + +public class TestSource extends ChunksContentSource implements Closeable +{ + private final List chunks; + + public TestSource(Content.Chunk... chunks) + { + super(Arrays.asList(chunks)); + this.chunks = Arrays.asList(chunks); + } + + @Override + public void close() + { + chunks.stream().filter(Objects::nonNull).forEach(Content.Chunk::release); + } +} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentCopierTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentCopierTest.java new file mode 100644 index 00000000000..029b953a038 --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentCopierTest.java @@ -0,0 +1,72 @@ +// +// ======================================================================== +// 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.io.internal; + +import java.nio.ByteBuffer; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; + +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.TestSink; +import org.eclipse.jetty.io.TestSource; +import org.eclipse.jetty.util.Callback; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.fail; + +public class ContentCopierTest +{ + @Test + public void testTransientErrorsBecomeTerminalErrors() throws Exception + { + TimeoutException originalFailure = new TimeoutException("timeout"); + TestSource originalSource = new TestSource( + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + null, + Content.Chunk.from(originalFailure, false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), true) + ); + + Callback.Completable callback = new Callback.Completable(); + TestSink resultSink = new TestSink(); + ContentCopier contentCopier = new ContentCopier(originalSource, resultSink, null, callback); + contentCopier.iterate(); + try + { + callback.get(); + fail(); + } + catch (ExecutionException e) + { + assertThat(e.getCause(), sameInstance(originalFailure)); + } + + List accumulatedChunks = resultSink.takeAccumulatedChunks(); + assertThat(accumulatedChunks.size(), is(1)); + assertThat(accumulatedChunks.get(0).isLast(), is(false)); + assertThat(accumulatedChunks.get(0).getByteBuffer().get(), is((byte)1)); + + Content.Chunk chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.getFailure(), sameInstance(originalFailure)); + + originalSource.close(); + } +} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceByteBufferTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceByteBufferTest.java new file mode 100644 index 00000000000..d8d6637dd21 --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceByteBufferTest.java @@ -0,0 +1,64 @@ +// +// ======================================================================== +// 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.io.internal; + +import java.nio.ByteBuffer; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; + +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.TestSource; +import org.eclipse.jetty.util.Promise; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.fail; + +public class ContentSourceByteBufferTest +{ + @Test + public void testTransientErrorsBecomeTerminalErrors() throws Exception + { + TimeoutException originalFailure = new TimeoutException("timeout"); + TestSource originalSource = new TestSource( + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + null, + Content.Chunk.from(originalFailure, false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), true) + ); + + Promise.Completable promise = new Promise.Completable<>(); + ContentSourceByteBuffer contentSourceByteBuffer = new ContentSourceByteBuffer(originalSource, promise); + contentSourceByteBuffer.run(); + try + { + promise.get(); + fail(); + } + catch (ExecutionException e) + { + assertThat(e.getCause(), sameInstance(originalFailure)); + } + + Content.Chunk chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.getFailure(), sameInstance(originalFailure)); + + originalSource.close(); + } +} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceConsumerTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceConsumerTest.java new file mode 100644 index 00000000000..a1d41453c77 --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceConsumerTest.java @@ -0,0 +1,64 @@ +// +// ======================================================================== +// 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.io.internal; + +import java.nio.ByteBuffer; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; + +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.TestSource; +import org.eclipse.jetty.util.Callback; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.fail; + +public class ContentSourceConsumerTest +{ + @Test + public void testTransientErrorsBecomeTerminalErrors() throws Exception + { + TimeoutException originalFailure = new TimeoutException("timeout"); + TestSource originalSource = new TestSource( + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{1}), false), + null, + Content.Chunk.from(originalFailure, false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{2}), true) + ); + + Callback.Completable callback = new Callback.Completable(); + ContentSourceConsumer contentSourceConsumer = new ContentSourceConsumer(originalSource, callback); + contentSourceConsumer.run(); + try + { + callback.get(); + fail(); + } + catch (ExecutionException e) + { + assertThat(e.getCause(), sameInstance(originalFailure)); + } + + Content.Chunk chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.getFailure(), sameInstance(originalFailure)); + + originalSource.close(); + } +} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceStringTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceStringTest.java new file mode 100644 index 00000000000..6320a33029b --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/internal/ContentSourceStringTest.java @@ -0,0 +1,65 @@ +// +// ======================================================================== +// 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.io.internal; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; + +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.TestSource; +import org.eclipse.jetty.util.Promise; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.fail; + +public class ContentSourceStringTest +{ + @Test + public void testTransientErrorsBecomeTerminalErrors() throws Exception + { + TimeoutException originalFailure = new TimeoutException("timeout"); + TestSource originalSource = new TestSource( + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'1'}), false), + null, + Content.Chunk.from(originalFailure, false), + null, + Content.Chunk.from(ByteBuffer.wrap(new byte[]{'2'}), true) + ); + + Promise.Completable promise = new Promise.Completable<>(); + ContentSourceString contentSourceString = new ContentSourceString(originalSource, StandardCharsets.US_ASCII, promise); + contentSourceString.convert(); + try + { + promise.get(); + fail(); + } + catch (ExecutionException e) + { + assertThat(e.getCause(), sameInstance(originalFailure)); + } + + Content.Chunk chunk = originalSource.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.getFailure(), sameInstance(originalFailure)); + + originalSource.close(); + } +} diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/NegotiatingServerConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/NegotiatingServerConnection.java index 59b1f6eb981..038e02d48c3 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/NegotiatingServerConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/NegotiatingServerConnection.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.server; import java.io.IOException; +import java.nio.ByteBuffer; import java.util.List; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; @@ -21,13 +22,13 @@ import javax.net.ssl.SSLEngineResult; import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.EndPoint; -import org.eclipse.jetty.util.BufferUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public abstract class NegotiatingServerConnection extends AbstractConnection { private static final Logger LOG = LoggerFactory.getLogger(NegotiatingServerConnection.class); + private static final ByteBuffer EMPTY_WRITABLE_BUFFER = ByteBuffer.allocate(0); public interface CipherDiscriminator { @@ -144,7 +145,7 @@ public abstract class NegotiatingServerConnection extends AbstractConnection { try { - return getEndPoint().fill(BufferUtil.EMPTY_BUFFER); + return getEndPoint().fill(EMPTY_WRITABLE_BUFFER); } catch (IOException x) { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipRequest.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipRequest.java index 424b2d0e677..de2218fec80 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipRequest.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipRequest.java @@ -50,7 +50,7 @@ public class GzipRequest extends Request.Wrapper { Components components = getComponents(); _decoder = new Decoder(__inflaterPool, components.getByteBufferPool(), inflateBufferSize); - _gzipTransformer = new GzipTransformer(getWrapped()); + _gzipTransformer = new GzipTransformer(getWrapped(), _decoder); } } @@ -141,13 +141,15 @@ public class GzipRequest extends Request.Wrapper _decoder.destroy(); } - private class GzipTransformer extends ContentSourceTransformer + static class GzipTransformer extends ContentSourceTransformer { + private final Decoder _decoder; private Content.Chunk _chunk; - public GzipTransformer(Content.Source source) + GzipTransformer(Content.Source source, Decoder decoder) { super(source); + _decoder = decoder; } @Override @@ -159,7 +161,11 @@ public class GzipRequest extends Request.Wrapper if (_chunk == null) return null; if (Content.Chunk.isFailure(_chunk)) - return _chunk; + { + Content.Chunk failure = _chunk; + _chunk = Content.Chunk.next(failure); + return failure; + } if (_chunk.isLast() && !_chunk.hasRemaining()) return Content.Chunk.EOF; @@ -187,11 +193,11 @@ public class GzipRequest extends Request.Wrapper } } - private static class Decoder extends GZIPContentDecoder + static class Decoder extends GZIPContentDecoder { private RetainableByteBuffer _decoded; - private Decoder(InflaterPool inflaterPool, ByteBufferPool bufferPool, int bufferSize) + Decoder(InflaterPool inflaterPool, ByteBufferPool bufferPool, int bufferSize) { super(inflaterPool, bufferPool, bufferSize); } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java index 1b6cc02cd8f..6093fab1b47 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java @@ -336,23 +336,13 @@ public class HttpChannelState implements HttpChannel, Components Runnable invokeOnContentAvailable = _onContentAvailable; _onContentAvailable = null; + // If demand was in process, then arrange for the next read to return the idle timeout, if no other error + if (invokeOnContentAvailable != null) + _failure = Content.Chunk.from(t, false); + // If a write call is in progress, take the writeCallback to fail below Runnable invokeWriteFailure = _response.lockedFailWrite(t); - // If demand was in process, then arrange for the next read to return the idle timeout, if no other error - // TODO to make IO timeouts transient, remove the invokeWriteFailure test below. - // Probably writes cannot be made transient as it will be unclear how much of the buffer has actually - // been written. So write timeouts might always be persistent... but then we should call the listener - // before calling lockedFailedWrite above. - if (invokeOnContentAvailable != null || invokeWriteFailure != null) - { - // TODO The chunk here should be last==false, so that IO timeout is a transient failure. - // However AsyncContentProducer has been written on the assumption of no transient - // failures, so it needs to be updated before we can make timeouts transients. - // See ServerTimeoutTest.testAsyncReadHttpIdleTimeoutOverridesIdleTimeout - _failure = Content.Chunk.from(t, true); - } - // If there was an IO operation, just deliver the idle timeout via them if (invokeOnContentAvailable != null || invokeWriteFailure != null) return _serializedInvoker.offer(invokeOnContentAvailable, invokeWriteFailure); @@ -432,34 +422,20 @@ public class HttpChannelState implements HttpChannel, Components Runnable invokeWriteFailure = _response.lockedFailWrite(x); // Create runnable to invoke any onError listeners - ChannelRequest request = _request; - Runnable invokeOnFailureListeners = () -> - { - Consumer onFailure; - try (AutoLock ignore = _lock.lock()) - { - onFailure = _onFailure; - } + Consumer onFailure = _onFailure; + Runnable invokeOnFailureListeners = onFailure == null ? null : () -> + { try { if (LOG.isDebugEnabled()) LOG.debug("invokeListeners {} {}", HttpChannelState.this, onFailure, x); - if (onFailure != null) - onFailure.accept(x); + onFailure.accept(x); } catch (Throwable throwable) { ExceptionUtil.addSuppressedIfNotAssociated(x, throwable); } - - // If the application has not been otherwise informed of the failure - if (invokeOnContentAvailable == null && invokeWriteFailure == null && onFailure == null) - { - if (LOG.isDebugEnabled()) - LOG.debug("failing callback in {}", this, x); - request._callback.failed(x); - } }; // Serialize all the error actions. diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index 08a83cd567d..e61ed5a0615 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -1051,13 +1051,24 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab if (stream != null) { BadMessageException bad = new BadMessageException("Early EOF"); + Content.Chunk chunk = stream._chunk; - if (Content.Chunk.isFailure(stream._chunk)) - stream._chunk.getFailure().addSuppressed(bad); + if (Content.Chunk.isFailure(chunk)) + { + if (chunk.isLast()) + { + chunk.getFailure().addSuppressed(bad); + } + else + { + bad.addSuppressed(chunk.getFailure()); + stream._chunk = Content.Chunk.from(bad); + } + } else { - if (stream._chunk != null) - stream._chunk.release(); + if (chunk != null) + chunk.release(); stream._chunk = Content.Chunk.from(bad); } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpStreamTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpStreamTest.java new file mode 100644 index 00000000000..8048be766d8 --- /dev/null +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/HttpStreamTest.java @@ -0,0 +1,164 @@ +// +// ======================================================================== +// 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.server; + +import java.nio.ByteBuffer; +import java.util.ArrayDeque; +import java.util.Arrays; +import java.util.Queue; + +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.util.Callback; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; + +public class HttpStreamTest +{ + @Test + public void testNoContentReturnsContentNotConsumed() + { + HttpConfiguration httpConfig = new HttpConfiguration(); + httpConfig.setMaxUnconsumedRequestContentReads(2); + TestHttpStream httpStream = new TestHttpStream(); + Throwable throwable = HttpStream.consumeAvailable(httpStream, httpConfig); + assertThat(throwable, notNullValue()); + } + + @Test + public void testTooMuchContentReturnsContentNotConsumed() + { + HttpConfiguration httpConfig = new HttpConfiguration(); + httpConfig.setMaxUnconsumedRequestContentReads(2); + TestHttpStream httpStream = new TestHttpStream( + Content.Chunk.from(ByteBuffer.wrap(new byte[] {1}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[] {2}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[] {3}), true) + ); + Throwable throwable = HttpStream.consumeAvailable(httpStream, httpConfig); + assertThat(throwable, notNullValue()); + } + + @Test + public void testLastContentReturnsNull() + { + HttpConfiguration httpConfig = new HttpConfiguration(); + httpConfig.setMaxUnconsumedRequestContentReads(5); + TestHttpStream httpStream = new TestHttpStream( + Content.Chunk.from(ByteBuffer.wrap(new byte[] {1}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[] {2}), false), + Content.Chunk.from(ByteBuffer.wrap(new byte[] {3}), true) + ); + Throwable throwable = HttpStream.consumeAvailable(httpStream, httpConfig); + assertThat(throwable, nullValue()); + } + + @Test + public void testTerminalFailureReturnsFailure() + { + HttpConfiguration httpConfig = new HttpConfiguration(); + httpConfig.setMaxUnconsumedRequestContentReads(5); + NumberFormatException failure = new NumberFormatException(); + TestHttpStream httpStream = new TestHttpStream( + Content.Chunk.from(ByteBuffer.wrap(new byte[] {1}), false), + Content.Chunk.from(failure, true) + ); + Throwable throwable = HttpStream.consumeAvailable(httpStream, httpConfig); + assertThat(throwable, sameInstance(failure)); + } + + @Test + public void testTransientFailureReturnsFailure() + { + HttpConfiguration httpConfig = new HttpConfiguration(); + httpConfig.setMaxUnconsumedRequestContentReads(5); + NumberFormatException failure = new NumberFormatException(); + TestHttpStream httpStream = new TestHttpStream( + Content.Chunk.from(ByteBuffer.wrap(new byte[] {1}), false), + Content.Chunk.from(failure, false), + Content.Chunk.from(ByteBuffer.wrap(new byte[] {2}), true) + ); + Throwable throwable = HttpStream.consumeAvailable(httpStream, httpConfig); + assertThat(throwable, sameInstance(failure)); + } + + private static class TestHttpStream implements HttpStream + { + private final Queue chunks = new ArrayDeque<>(); + + public TestHttpStream(Content.Chunk... chunks) + { + this.chunks.addAll(Arrays.asList(chunks)); + } + + @Override + public Content.Chunk read() + { + return chunks.poll(); + } + + @Override + public String getId() + { + throw new UnsupportedOperationException(); + } + + @Override + public void demand() + { + throw new UnsupportedOperationException(); + } + + @Override + public void prepareResponse(HttpFields.Mutable headers) + { + throw new UnsupportedOperationException(); + } + + @Override + public void send(MetaData.Request request, MetaData.Response response, boolean last, ByteBuffer content, Callback callback) + { + throw new UnsupportedOperationException(); + } + + @Override + public long getIdleTimeout() + { + throw new UnsupportedOperationException(); + } + + @Override + public void setIdleTimeout(long idleTimeoutMs) + { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isCommitted() + { + throw new UnsupportedOperationException(); + } + + @Override + public Throwable consumeAvailable() + { + throw new UnsupportedOperationException(); + } + } +} diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipTransformerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipTransformerTest.java new file mode 100644 index 00000000000..c82e654df36 --- /dev/null +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/gzip/GzipTransformerTest.java @@ -0,0 +1,129 @@ +// +// ======================================================================== +// 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.server.handler.gzip; + +import java.io.ByteArrayOutputStream; +import java.io.Closeable; +import java.io.IOException; +import java.util.Arrays; +import java.util.concurrent.TimeoutException; +import java.util.zip.GZIPOutputStream; + +import org.eclipse.jetty.io.ArrayByteBufferPool; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.RetainableByteBuffer; +import org.eclipse.jetty.io.content.ChunksContentSource; +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.compression.InflaterPool; +import org.junit.jupiter.api.Test; + +import static java.nio.charset.StandardCharsets.US_ASCII; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; + +public class GzipTransformerTest +{ + @Test + public void testTransientFailuresFromOriginalSourceAreReturned() throws Exception + { + ArrayByteBufferPool.Tracking bufferPool = new ArrayByteBufferPool.Tracking(); + TimeoutException originalFailure1 = new TimeoutException("timeout 1"); + TimeoutException originalFailure2 = new TimeoutException("timeout 2"); + TestSource originalSource = new TestSource( + gzipChunk(bufferPool, "AAA".getBytes(US_ASCII), false), + Content.Chunk.from(originalFailure1, false), + gzipChunk(bufferPool, "BBB".getBytes(US_ASCII), false), + Content.Chunk.from(originalFailure2, false), + gzipChunk(bufferPool, "CCC".getBytes(US_ASCII), true) + ); + + GzipRequest.GzipTransformer transformer = new GzipRequest.GzipTransformer(originalSource, new GzipRequest.Decoder(new InflaterPool(1, true), bufferPool, 1)); + + + Content.Chunk chunk; + chunk = transformer.read(); + assertThat(US_ASCII.decode(chunk.getByteBuffer()).toString(), is("AAA")); + assertThat(chunk.getByteBuffer().hasRemaining(), is(false)); + assertThat(chunk.isLast(), is(false)); + chunk.release(); + + chunk = transformer.read(); + assertThat(Content.Chunk.isFailure(chunk, false), is(true)); + assertThat(chunk.getFailure(), sameInstance(originalFailure1)); + + chunk = transformer.read(); + assertThat(US_ASCII.decode(chunk.getByteBuffer()).toString(), is("BBB")); + assertThat(chunk.getByteBuffer().hasRemaining(), is(false)); + assertThat(chunk.isLast(), is(false)); + chunk.release(); + + chunk = transformer.read(); + assertThat(Content.Chunk.isFailure(chunk, false), is(true)); + assertThat(chunk.getFailure(), sameInstance(originalFailure2)); + + chunk = transformer.read(); + assertThat(US_ASCII.decode(chunk.getByteBuffer()).toString(), is("CCC")); + assertThat(chunk.getByteBuffer().hasRemaining(), is(false)); + assertThat(chunk.isLast(), is(false)); + chunk.release(); + + chunk = transformer.read(); + assertThat(Content.Chunk.isFailure(chunk), is(false)); + assertThat(chunk.getByteBuffer().hasRemaining(), is(false)); + assertThat(chunk.isLast(), is(true)); + + originalSource.close(); + assertThat("Leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0)); + } + + private static Content.Chunk gzipChunk(ArrayByteBufferPool.Tracking bufferPool, byte[] bytes, boolean last) throws IOException + { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + GZIPOutputStream gzos = new GZIPOutputStream(baos); + gzos.write(bytes); + gzos.close(); + byte[] gzippedBytes = baos.toByteArray(); + + RetainableByteBuffer buffer = bufferPool.acquire(gzippedBytes.length, false); + int pos = BufferUtil.flipToFill(buffer.getByteBuffer()); + buffer.getByteBuffer().put(gzippedBytes); + BufferUtil.flipToFlush(buffer.getByteBuffer(), pos); + return Content.Chunk.asChunk(buffer.getByteBuffer(), last, buffer); + } + + private static class TestSource extends ChunksContentSource implements Closeable + { + private Content.Chunk[] chunks; + + public TestSource(Content.Chunk... chunks) + { + super(Arrays.asList(chunks)); + this.chunks = chunks; + } + + @Override + public void close() + { + if (chunks != null) + { + for (Content.Chunk chunk : chunks) + { + chunk.release(); + } + chunks = null; + } + } + } +} diff --git a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ServerTimeoutsTest.java b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ServerTimeoutsTest.java index 4c7778934ce..b47a079db3d 100644 --- a/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ServerTimeoutsTest.java +++ b/jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/ServerTimeoutsTest.java @@ -42,6 +42,7 @@ import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; public class ServerTimeoutsTest extends AbstractTest @@ -55,7 +56,7 @@ public class ServerTimeoutsTest extends AbstractTest setStreamIdleTimeout(IDLE_TIMEOUT); } - public static Stream transportsAndTrueIdleTimeoutListeners() + public static Stream transportsAndIdleTimeoutListener() { Collection transports = transports(); return Stream.concat( @@ -64,8 +65,8 @@ public class ServerTimeoutsTest extends AbstractTest } @ParameterizedTest - @MethodSource("transportsAndTrueIdleTimeoutListeners") - public void testIdleTimeout(Transport transport, boolean listener) throws Exception + @MethodSource("transportsAndIdleTimeoutListener") + public void testIdleTimeout(Transport transport, boolean addIdleTimeoutListener) throws Exception { AtomicBoolean listenerCalled = new AtomicBoolean(); start(transport, new Handler.Abstract() @@ -73,9 +74,11 @@ public class ServerTimeoutsTest extends AbstractTest @Override public boolean handle(Request request, Response response, Callback callback) { - - if (listener) + if (addIdleTimeoutListener) + { request.addIdleTimeoutListener(t -> listenerCalled.compareAndSet(false, true)); + request.addFailureListener(callback::failed); + } // Do not complete the callback, so it idle times out. return true; @@ -88,13 +91,13 @@ public class ServerTimeoutsTest extends AbstractTest assertThat(response.getStatus(), is(HttpStatus.INTERNAL_SERVER_ERROR_500)); assertThat(response.getContentAsString(), containsStringIgnoringCase("HTTP ERROR 500 java.util.concurrent.TimeoutException: Idle timeout")); - if (listener) + if (addIdleTimeoutListener) assertTrue(listenerCalled.get()); } @ParameterizedTest - @MethodSource("transportsAndTrueIdleTimeoutListeners") - public void testIdleTimeoutWithDemand(Transport transport, boolean listener) throws Exception + @MethodSource("transportsAndIdleTimeoutListener") + public void testIdleTimeoutWithDemand(Transport transport, boolean addIdleTimeoutListener) throws Exception { AtomicBoolean listenerCalled = new AtomicBoolean(); CountDownLatch demanded = new CountDownLatch(1); @@ -105,8 +108,7 @@ public class ServerTimeoutsTest extends AbstractTest @Override public boolean handle(Request request, Response response, Callback callback) { - - if (listener) + if (addIdleTimeoutListener) request.addIdleTimeoutListener(t -> listenerCalled.compareAndSet(false, true)); requestRef.set(request); callbackRef.set(callback); @@ -130,15 +132,12 @@ public class ServerTimeoutsTest extends AbstractTest // Reads should yield the idle timeout. Content.Chunk chunk = requestRef.get().read(); - // TODO change last to false in the next line if timeouts are transients - assertTrue(Content.Chunk.isFailure(chunk, true)); + assertTrue(Content.Chunk.isFailure(chunk, false)); Throwable cause = chunk.getFailure(); assertThat(cause, instanceOf(TimeoutException.class)); - /* TODO if transient timeout failures are supported then add this check // Can read again assertNull(requestRef.get().read()); - */ // Complete the callback as the error listener promised. callbackRef.get().failed(cause); @@ -187,10 +186,9 @@ public class ServerTimeoutsTest extends AbstractTest } @ParameterizedTest - @MethodSource("transportsNoFCGI") + @MethodSource("transports") public void testIdleTimeoutErrorListenerReturnsFalseThenTrue(Transport transport) throws Exception { - // TODO fix FCGI for multiple timeouts AtomicReference error = new AtomicReference<>(); start(transport, new Handler.Abstract() { @@ -198,6 +196,7 @@ public class ServerTimeoutsTest extends AbstractTest public boolean handle(Request request, Response response, Callback callback) { request.addIdleTimeoutListener(t -> error.getAndSet(t) != null); + request.addFailureListener(callback::failed); return true; } }); @@ -206,9 +205,9 @@ public class ServerTimeoutsTest extends AbstractTest .timeout(IDLE_TIMEOUT * 5, TimeUnit.MILLISECONDS) .send(); - // The first time the listener returns true, but does not complete the callback, + // The first time the listener returns false, but does not complete the callback, // so another idle timeout elapses. - // The second time the listener returns false and the implementation produces the response. + // The second time the listener returns true and the implementation produces the response. assertThat(response.getStatus(), is(HttpStatus.INTERNAL_SERVER_ERROR_500)); assertThat(response.getContentAsString(), containsStringIgnoringCase("HTTP ERROR 500 java.util.concurrent.TimeoutException: Idle timeout")); assertThat(error.get(), instanceOf(TimeoutException.class)); diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java index f8f516bccff..6f78d3300cc 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java @@ -105,7 +105,7 @@ public class BufferUtil (byte)'E', (byte)'F' }; - public static final ByteBuffer EMPTY_BUFFER = ByteBuffer.wrap(new byte[0]); + public static final ByteBuffer EMPTY_BUFFER = ByteBuffer.wrap(new byte[0]).asReadOnlyBuffer(); /** * Allocate ByteBuffer in flush mode. diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index 3baf9e88c1b..c4f5d191c81 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -971,6 +971,11 @@ public class QueuedThreadPool extends ContainerLifeCycle implements ThreadFactor job.run(); } + protected void onJobFailure(Throwable x) + { + LOG.warn("Job failed", x); + } + /** *

Determines whether to evict the current thread from the pool.

* @@ -1197,7 +1202,7 @@ public class QueuedThreadPool extends ContainerLifeCycle implements ThreadFactor } catch (Throwable e) { - LOG.warn("Job failed", e); + onJobFailure(e); } finally { diff --git a/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/ProxyServlet.java b/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/ProxyServlet.java index b422a681dc9..c8e0f532b97 100644 --- a/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/ProxyServlet.java +++ b/jetty-ee10/jetty-ee10-proxy/src/main/java/org/eclipse/jetty/ee10/proxy/ProxyServlet.java @@ -259,6 +259,8 @@ public class ProxyServlet extends AbstractProxyServlet Content.Chunk chunk = super.read(); if (Content.Chunk.isFailure(chunk)) { + if (!chunk.isLast()) + fail(chunk.getFailure()); onClientRequestFailure(request, proxyRequest, response, chunk.getFailure()); } else diff --git a/jetty-ee10/jetty-ee10-servlet/pom.xml b/jetty-ee10/jetty-ee10-servlet/pom.xml index c3039c7a44f..a62d93dadc2 100644 --- a/jetty-ee10/jetty-ee10-servlet/pom.xml +++ b/jetty-ee10/jetty-ee10-servlet/pom.xml @@ -47,6 +47,11 @@ org.slf4j slf4j-api + + org.awaitility + awaitility + test + org.eclipse.jetty jetty-client diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java index 719bd9d17d0..4be2ca17b15 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java @@ -51,6 +51,11 @@ class AsyncContentProducer implements ContentProducer _lock = lock; } + ServletChannel getServletChannel() + { + return _servletChannel; + } + @Override public void recycle() { @@ -101,7 +106,7 @@ class AsyncContentProducer implements ContentProducer public boolean isError() { assertLocked(); - boolean failure = Content.Chunk.isFailure(_chunk); + boolean failure = Content.Chunk.isFailure(_chunk, true); if (LOG.isDebugEnabled()) LOG.debug("isFailure = {} {}", failure, this); return failure; @@ -200,7 +205,11 @@ class AsyncContentProducer implements ContentProducer if (LOG.isDebugEnabled()) LOG.debug("nextChunk = {} {}", chunk, this); if (chunk != null) + { _servletChannel.getServletRequestState().onReadIdle(); + if (Content.Chunk.isFailure(chunk, false)) + _chunk = Content.Chunk.next(chunk); + } return chunk; } @@ -244,6 +253,9 @@ class AsyncContentProducer implements ContentProducer { if (LOG.isDebugEnabled()) LOG.debug("isReady() demand callback {}", this); + // We could call this.onContentProducible() directly but this + // would mean we would need to take the lock here while it + // is the responsibility of the HttpInput to take it. if (_servletChannel.getHttpInput().onContentProducible()) _servletChannel.handle(); }); @@ -267,19 +279,24 @@ class AsyncContentProducer implements ContentProducer { if (_chunk != null) { + if (Content.Chunk.isFailure(_chunk, false)) + { + // We return the transient failure here without _chunk = Content.Chunk.next(_chunk) + // because this method may be called by available() or isReady(), which do not consume the + // chunk. Only a call from nextChunk() consumes the chunk produced here, so the call to next + // is done there. + return _chunk; + } if (_chunk.isLast() || _chunk.hasRemaining()) { if (LOG.isDebugEnabled()) LOG.debug("chunk not yet depleted, returning it {}", this); return _chunk; } - else - { - if (LOG.isDebugEnabled()) - LOG.debug("current chunk depleted {}", this); - _chunk.release(); - _chunk = null; - } + if (LOG.isDebugEnabled()) + LOG.debug("current chunk depleted {}", this); + _chunk.release(); + _chunk = null; } else { @@ -292,19 +309,7 @@ class AsyncContentProducer implements ContentProducer LOG.debug("channel has no new chunk {}", this); return null; } - else - { - _servletChannel.getServletRequestState().onContentAdded(); - } - } - - // Release the chunk immediately, if it is empty. - if (_chunk != null && !_chunk.hasRemaining() && !_chunk.isLast()) - { - if (LOG.isDebugEnabled()) - LOG.debug("releasing empty chunk {}", this); - _chunk.release(); - _chunk = null; + _servletChannel.getServletRequestState().onContentAdded(); } } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducer.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducer.java index d63b5a5c5f9..e0ea0e34e32 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducer.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducer.java @@ -103,7 +103,7 @@ class BlockingContentProducer implements ContentProducer if (chunk != null) return chunk; - // IFF isReady() returns false then HttpChannel.needContent() has been called, + // IFF isReady() returns false then Request.demand() has been called, // thus we know that eventually a call to onContentProducible will come. if (_asyncContentProducer.isReady()) { @@ -149,10 +149,9 @@ class BlockingContentProducer implements ContentProducer // This is why this method always returns false. // But async errors can occur while the dispatched thread is NOT blocked reading (i.e.: in state WAITING), // so the WAITING to WOKEN transition must be done by the error-notifying thread which then has to reschedule - // the dispatched thread after HttpChannelState.asyncError() is called. + // the dispatched thread. // Calling _asyncContentProducer.onContentProducible() changes the channel state from WAITING to WOKEN which - // would prevent the subsequent call to HttpChannelState.asyncError() from rescheduling the thread. - // AsyncServletTest.testStartAsyncThenClientStreamIdleTimeout() tests this. + // would prevent the async error thread from noticing that a redispatching is needed. boolean unready = _asyncContentProducer.isUnready(); if (LOG.isDebugEnabled()) LOG.debug("onContentProducible releasing semaphore {} unready={}", _semaphore, unready); @@ -160,8 +159,8 @@ class BlockingContentProducer implements ContentProducer // just after having received the request, not only when they have read all the available content. if (unready) { - // Call nextChunk() to switch the input state back to IDLE, otherwise we would stay UNREADY. - _asyncContentProducer.nextChunk(); + // Switch the input state back to IDLE, otherwise we would stay UNREADY. + _asyncContentProducer.getServletChannel().getServletRequestState().onReadIdle(); _semaphore.release(); } return false; diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpInput.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpInput.java index 8e741e55b96..6dfff5f3388 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpInput.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/HttpInput.java @@ -20,7 +20,6 @@ import java.util.concurrent.atomic.LongAdder; import jakarta.servlet.ReadListener; import jakarta.servlet.ServletInputStream; -import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.server.Context; import org.eclipse.jetty.util.thread.AutoLock; @@ -42,8 +41,8 @@ public class HttpInput extends ServletInputStream implements Runnable private final ServletChannel _servletChannel; private final ServletChannelState _channelState; private final byte[] _oneByteBuffer = new byte[1]; - private final BlockingContentProducer _blockingContentProducer; - private final AsyncContentProducer _asyncContentProducer; + final BlockingContentProducer _blockingContentProducer; + final AsyncContentProducer _asyncContentProducer; private final LongAdder _contentConsumed = new LongAdder(); private volatile ContentProducer _contentProducer; private volatile boolean _consumedEof; @@ -348,8 +347,6 @@ public class HttpInput extends ServletInputStream implements Runnable Throwable failure = chunk.getFailure(); if (LOG.isDebugEnabled()) LOG.debug("running failure={} {}", failure, this); - // TODO is this necessary to add here? - _servletChannel.getServletContextResponse().getHeaders().add(HttpFields.CONNECTION_CLOSE); _readListener.onError(failure); } else if (chunk.isLast() && !chunk.hasRemaining()) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java index c8ac14515e0..07a573702ad 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java @@ -76,7 +76,7 @@ public class ServletChannel private final ServletContextHandler.ServletContextApi _servletContextApi; private final ConnectionMetaData _connectionMetaData; private final AtomicLong _requests = new AtomicLong(); - private final HttpInput _httpInput; + final HttpInput _httpInput; private final HttpOutput _httpOutput; private ServletContextRequest _servletContextRequest; private Request _request; diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AbstractContentProducerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AbstractContentProducerTest.java new file mode 100644 index 00000000000..67216059c6b --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AbstractContentProducerTest.java @@ -0,0 +1,148 @@ +// +// ======================================================================== +// 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.ee10.servlet; + +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.function.BooleanSupplier; + +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.thread.AutoLock; +import org.eclipse.jetty.util.thread.TimerScheduler; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; + +import static java.nio.charset.StandardCharsets.US_ASCII; + +public abstract class AbstractContentProducerTest +{ + private TimerScheduler _scheduler; + + @BeforeEach + public void setUp() throws Exception + { + _scheduler = new TimerScheduler(); + _scheduler.start(); + } + + @AfterEach + public void tearDown() throws Exception + { + _scheduler.stop(); + } + + static int countRemaining(List chunks) + { + int total = 0; + for (Content.Chunk chunk : chunks) + { + total += chunk.remaining(); + } + return total; + } + + static String asString(List chunks) + { + StringBuilder sb = new StringBuilder(); + for (Content.Chunk chunk : chunks) + { + byte[] b = new byte[chunk.remaining()]; + chunk.getByteBuffer().duplicate().get(b); + sb.append(new String(b, US_ASCII)); + } + return sb.toString(); + } + + class ArrayDelayedServletChannel extends ServletChannel + { + ArrayDelayedServletChannel(List chunks) + { + super(new ServletContextHandler(), new MockConnectionMetaData()); + associate(new ArrayDelayedServletChannelRequest(chunks), null, Callback.NOOP); + } + + ContentProducer getAsyncContentProducer() + { + return _httpInput._asyncContentProducer; + } + + ContentProducer getBlockingContentProducer() + { + return _httpInput._blockingContentProducer; + } + + BooleanSupplier getContentPresenceCheckSupplier() + { + return () -> !getServletRequestState().isInputUnready(); + } + + AutoLock getLock() + { + return _httpInput._lock; + } + } + + private class ArrayDelayedServletChannelRequest extends MockRequest + { + private final List chunks; + private int counter; + private volatile Content.Chunk nextContent; + + ArrayDelayedServletChannelRequest(List chunks) + { + for (int i = 0; i < chunks.size() - 1; i++) + { + Content.Chunk chunk = chunks.get(i); + if (chunk.isLast()) + throw new AssertionError("Only the last of the given chunks may be marked as last"); + } + if (!chunks.get(chunks.size() - 1).isLast()) + throw new AssertionError("The last of the given chunks must be marked as last"); + this.chunks = chunks; + } + + @Override + public void fail(Throwable failure) + { + nextContent = Content.Chunk.from(failure, true); + counter = chunks.size(); + } + + @Override + public void demand(Runnable demandCallback) + { + if (nextContent != null) + { + demandCallback.run(); + return; + } + + _scheduler.schedule(() -> + { + int idx = counter < chunks.size() ? counter++ : chunks.size() - 1; + nextContent = chunks.get(idx); + demandCallback.run(); + }, 50, TimeUnit.MILLISECONDS); + } + + @Override + public Content.Chunk read() + { + Content.Chunk result = nextContent; + nextContent = null; + return result; + } + } +} diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducerTest.java new file mode 100644 index 00000000000..4f9ff1a6b2e --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducerTest.java @@ -0,0 +1,208 @@ +// +// ======================================================================== +// 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.ee10.servlet; + +import java.nio.ByteBuffer; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.function.BooleanSupplier; +import java.util.function.Consumer; + +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.EofException; +import org.eclipse.jetty.util.thread.AutoLock; +import org.junit.jupiter.api.Test; + +import static java.nio.charset.StandardCharsets.US_ASCII; +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.fail; + +public class AsyncContentProducerTest extends AbstractContentProducerTest +{ + @Test + public void testSimple() + { + List chunks = List.of( + Content.Chunk.from(ByteBuffer.wrap("1 hello 1".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("2 howdy 2".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("3 hey ya 3".getBytes(US_ASCII)), true) + ); + int totalContentBytesCount = countRemaining(chunks); + String originalContentString = asString(chunks); + + ArrayDelayedServletChannel servletChannel = new ArrayDelayedServletChannel(chunks); + ContentProducer contentProducer = servletChannel.getAsyncContentProducer(); + + Throwable error = readAndAssertContent(contentProducer, servletChannel.getLock(), servletChannel.getContentPresenceCheckSupplier(), totalContentBytesCount, originalContentString, + chunks.size() * 2, 0, 3, c -> fail(c.getFailure())); + assertThat(error, nullValue()); + } + + @Test + public void testSimpleWithEof() + { + List chunks = List.of( + Content.Chunk.from(ByteBuffer.wrap("1 hello 1".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("2 howdy 2".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("3 hey ya 3".getBytes(US_ASCII)), false), + Content.Chunk.EOF + ); + int totalContentBytesCount = countRemaining(chunks); + String originalContentString = asString(chunks); + + ArrayDelayedServletChannel servletChannel = new ArrayDelayedServletChannel(chunks); + ContentProducer contentProducer = servletChannel.getAsyncContentProducer(); + + Throwable error = readAndAssertContent(contentProducer, servletChannel.getLock(), servletChannel.getContentPresenceCheckSupplier(), + totalContentBytesCount, originalContentString, + chunks.size() * 2, 0, 4, c -> fail(c.getFailure())); + assertThat(error, nullValue()); + } + + @Test + public void testWithLastError() + { + Throwable expectedError = new EofException("Early EOF"); + List chunks = List.of( + Content.Chunk.from(ByteBuffer.wrap("1 hello 1".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("2 howdy 2".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("3 hey ya 3".getBytes(US_ASCII)), false), + Content.Chunk.from(expectedError, true) + ); + int totalContentBytesCount = countRemaining(chunks); + String originalContentString = asString(chunks); + + ArrayDelayedServletChannel servletChannel = new ArrayDelayedServletChannel(chunks); + ContentProducer contentProducer = servletChannel.getAsyncContentProducer(); + + Throwable error = readAndAssertContent(contentProducer, servletChannel.getLock(), servletChannel.getContentPresenceCheckSupplier(), + totalContentBytesCount, originalContentString, + chunks.size() * 2, 0, 4, c -> fail(c.getFailure())); + assertThat(error, is(expectedError)); + } + + @Test + public void testWithTransientErrors() + { + List chunks = List.of( + Content.Chunk.from(ByteBuffer.wrap("1 hello 1".getBytes(US_ASCII)), false), + Content.Chunk.from(new TimeoutException("timeout 1"), false), + Content.Chunk.from(ByteBuffer.wrap("2 howdy 2".getBytes(US_ASCII)), false), + Content.Chunk.from(new TimeoutException("timeout 2"), false), + Content.Chunk.from(ByteBuffer.wrap("3 hey ya 3".getBytes(US_ASCII)), false), + Content.Chunk.from(new TimeoutException("timeout 3"), false), + Content.Chunk.EOF + ); + int totalContentBytesCount = countRemaining(chunks); + String originalContentString = asString(chunks); + + ArrayDelayedServletChannel servletChannel = new ArrayDelayedServletChannel(chunks); + ContentProducer contentProducer = servletChannel.getAsyncContentProducer(); + + Throwable error = readAndAssertContent(contentProducer, servletChannel.getLock(), servletChannel.getContentPresenceCheckSupplier(), + totalContentBytesCount, originalContentString, + chunks.size() * 2, 0, 7, new Consumer<>() + { + int counter; + + @Override + public void accept(Content.Chunk chunk) + { + assertThat(chunk.isLast(), is(false)); + assertThat(Content.Chunk.isFailure(chunk, true), is(false)); + assertThat(Content.Chunk.isFailure(chunk, false), is(true)); + + Throwable x = chunk.getFailure(); + assertThat(x, instanceOf(TimeoutException.class)); + assertThat(x.getMessage(), equalTo("timeout " + ++counter)); + assertThat(counter, lessThanOrEqualTo(3)); + + try (AutoLock ignore = servletChannel.getLock().lock()) + { + assertThat(contentProducer.isError(), is(false)); + } + } + }); + assertThat(error, nullValue()); + } + + private Throwable readAndAssertContent(ContentProducer contentProducer, AutoLock lock, BooleanSupplier isThereContent, int totalContentBytesCount, String originalContentString, int totalContentCount, int readyCount, int notReadyCount, Consumer transientErrorConsumer) + { + int readBytes = 0; + String consumedString = ""; + int nextContentCount = 0; + int isReadyFalseCount = 0; + int isReadyTrueCount = 0; + Throwable failure; + + while (true) + { + try (AutoLock ignore = lock.lock()) + { + if (contentProducer.isReady()) + isReadyTrueCount++; + else + isReadyFalseCount++; + } + + Content.Chunk content; + try (AutoLock ignore = lock.lock()) + { + content = contentProducer.nextChunk(); + } + nextContentCount++; + if (content == null) + { + await().atMost(5, TimeUnit.SECONDS).until(isThereContent::getAsBoolean); + try (AutoLock ignore = lock.lock()) + { + content = contentProducer.nextChunk(); + } + nextContentCount++; + assertThat(nextContentCount, lessThanOrEqualTo(totalContentCount)); + } + assertThat(content, notNullValue()); + + if (Content.Chunk.isFailure(content, false)) + transientErrorConsumer.accept(content); + + byte[] b = new byte[content.remaining()]; + readBytes += b.length; + content.getByteBuffer().get(b); + consumedString += new String(b, US_ASCII); + content.skip(content.remaining()); + + if (content.isLast()) + { + failure = content.getFailure(); + break; + } + } + + assertThat(nextContentCount, is(totalContentCount)); + assertThat(readBytes, is(totalContentBytesCount)); + assertThat(consumedString, is(originalContentString)); + assertThat(isReadyFalseCount, is(notReadyCount)); + assertThat(isReadyTrueCount, is(readyCount)); + return failure; + } +} diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducerTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducerTest.java new file mode 100644 index 00000000000..ec8e3580db1 --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducerTest.java @@ -0,0 +1,184 @@ +// +// ======================================================================== +// 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.ee10.servlet; + +import java.nio.ByteBuffer; +import java.util.List; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; + +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.EofException; +import org.eclipse.jetty.util.thread.AutoLock; +import org.junit.jupiter.api.Test; + +import static java.nio.charset.StandardCharsets.US_ASCII; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.fail; + +public class BlockingContentProducerTest extends AbstractContentProducerTest +{ + @Test + public void testSimple() + { + List chunks = List.of( + Content.Chunk.from(ByteBuffer.wrap("1 hello 1".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("2 howdy 2".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("3 hey ya 3".getBytes(US_ASCII)), true) + ); + int totalContentBytesCount = countRemaining(chunks); + String originalContentString = asString(chunks); + + ArrayDelayedServletChannel servletChannel = new ArrayDelayedServletChannel(chunks); + ContentProducer contentProducer = servletChannel.getBlockingContentProducer(); + + Throwable error = readAndAssertContent(contentProducer, servletChannel.getLock(), + totalContentBytesCount, originalContentString, + chunks.size(), c -> fail(c.getFailure())); + assertThat(error, nullValue()); + } + + @Test + public void testSimpleWithEof() + { + List chunks = List.of( + Content.Chunk.from(ByteBuffer.wrap("1 hello 1".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("2 howdy 2".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("3 hey ya 3".getBytes(US_ASCII)), false), + Content.Chunk.EOF + ); + int totalContentBytesCount = countRemaining(chunks); + String originalContentString = asString(chunks); + + ArrayDelayedServletChannel servletChannel = new ArrayDelayedServletChannel(chunks); + ContentProducer contentProducer = servletChannel.getBlockingContentProducer(); + + Throwable error = readAndAssertContent(contentProducer, servletChannel.getLock(), + totalContentBytesCount, originalContentString, + chunks.size(), c -> fail(c.getFailure())); + assertThat(error, nullValue()); + } + + @Test + public void testWithLastError() + { + Throwable expectedError = new EofException("Early EOF"); + List chunks = List.of( + Content.Chunk.from(ByteBuffer.wrap("1 hello 1".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("2 howdy 2".getBytes(US_ASCII)), false), + Content.Chunk.from(ByteBuffer.wrap("3 hey ya 3".getBytes(US_ASCII)), false), + Content.Chunk.from(expectedError, true) + ); + int totalContentBytesCount = countRemaining(chunks); + String originalContentString = asString(chunks); + + ArrayDelayedServletChannel servletChannel = new ArrayDelayedServletChannel(chunks); + ContentProducer contentProducer = servletChannel.getBlockingContentProducer(); + + Throwable error = readAndAssertContent(contentProducer, servletChannel.getLock(), + totalContentBytesCount, originalContentString, + chunks.size(), c -> fail(c.getFailure())); + assertThat(error, is(expectedError)); + } + + @Test + public void testWithTransientErrors() + { + List chunks = List.of( + Content.Chunk.from(ByteBuffer.wrap("1 hello 1".getBytes(US_ASCII)), false), + Content.Chunk.from(new TimeoutException("timeout 1"), false), + Content.Chunk.from(ByteBuffer.wrap("2 howdy 2".getBytes(US_ASCII)), false), + Content.Chunk.from(new TimeoutException("timeout 2"), false), + Content.Chunk.from(ByteBuffer.wrap("3 hey ya 3".getBytes(US_ASCII)), false), + Content.Chunk.from(new TimeoutException("timeout 3"), false), + Content.Chunk.EOF + ); + int totalContentBytesCount = countRemaining(chunks); + String originalContentString = asString(chunks); + + ArrayDelayedServletChannel servletChannel = new ArrayDelayedServletChannel(chunks); + ContentProducer contentProducer = servletChannel.getBlockingContentProducer(); + + Throwable error = readAndAssertContent(contentProducer, servletChannel.getLock(), + totalContentBytesCount, originalContentString, + chunks.size(), new Consumer<>() + { + int counter; + + @Override + public void accept(Content.Chunk chunk) + { + assertThat(chunk.isLast(), is(false)); + assertThat(Content.Chunk.isFailure(chunk, true), is(false)); + assertThat(Content.Chunk.isFailure(chunk, false), is(true)); + + Throwable x = chunk.getFailure(); + assertThat(x, instanceOf(TimeoutException.class)); + assertThat(x.getMessage(), equalTo("timeout " + ++counter)); + assertThat(counter, lessThanOrEqualTo(3)); + + try (AutoLock ignore = servletChannel.getLock().lock()) + { + assertThat(contentProducer.isError(), is(false)); + } + } + }); + assertThat(error, nullValue()); + } + + private Throwable readAndAssertContent(ContentProducer contentProducer, AutoLock lock, int totalContentBytesCount, String originalContentString, int totalContentCount, Consumer transientErrorConsumer) + { + int readBytes = 0; + String consumedString = ""; + int nextContentCount = 0; + Throwable failure; + + while (true) + { + Content.Chunk content; + try (AutoLock ignore = lock.lock()) + { + content = contentProducer.nextChunk(); + } + nextContentCount++; + assertThat(content, notNullValue()); + + if (Content.Chunk.isFailure(content, false)) + transientErrorConsumer.accept(content); + + byte[] b = new byte[content.remaining()]; + readBytes += b.length; + content.getByteBuffer().get(b); + consumedString += new String(b, US_ASCII); + content.skip(content.remaining()); + + if (content.isLast()) + { + failure = content.getFailure(); + break; + } + } + + assertThat(nextContentCount, is(totalContentCount)); + assertThat(readBytes, is(totalContentBytesCount)); + assertThat(consumedString, is(originalContentString)); + return failure; + } +} diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MockConnectionMetaData.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MockConnectionMetaData.java new file mode 100644 index 00000000000..f96266e9cec --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MockConnectionMetaData.java @@ -0,0 +1,132 @@ +// +// ======================================================================== +// 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.ee10.servlet; + +import java.net.InetSocketAddress; +import java.net.SocketAddress; + +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.io.AbstractConnection; +import org.eclipse.jetty.io.ByteArrayEndPoint; +import org.eclipse.jetty.io.Connection; +import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.server.ConnectionMetaData; +import org.eclipse.jetty.server.Connector; +import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.util.Attributes; +import org.eclipse.jetty.util.HostPort; + +// TODO shared copy of this class +public class MockConnectionMetaData extends Attributes.Mapped implements ConnectionMetaData +{ + private final HttpConfiguration _httpConfig = new HttpConfiguration(); + private final Connector _connector; + private final EndPoint _endPoint; + private final Connection _connection; + private boolean _persistent = true; + + public MockConnectionMetaData() + { + this(null); + } + + public MockConnectionMetaData(Connector connector) + { + this(connector, null); + } + + public MockConnectionMetaData(Connector connector, EndPoint endPoint) + { + _connector = connector; + _endPoint = endPoint == null ? new ByteArrayEndPoint() : endPoint; + _connection = new AbstractConnection(_endPoint, Runnable::run) + { + @Override + public void onFillable() + { + } + }; + } + + public void notPersistent() + { + _persistent = false; + } + + @Override + public String getId() + { + return "test"; + } + + @Override + public HttpConfiguration getHttpConfiguration() + { + return _httpConfig; + } + + @Override + public HttpVersion getHttpVersion() + { + return HttpVersion.HTTP_1_1; + } + + @Override + public String getProtocol() + { + return "http"; + } + + @Override + public Connection getConnection() + { + return _connection; + } + + @Override + public Connector getConnector() + { + return _connector; + } + + @Override + public boolean isPersistent() + { + return _persistent; + } + + @Override + public boolean isSecure() + { + return false; + } + + @Override + public SocketAddress getRemoteSocketAddress() + { + return InetSocketAddress.createUnresolved("localhost", 12345); + } + + @Override + public SocketAddress getLocalSocketAddress() + { + return InetSocketAddress.createUnresolved("localhost", 80); + } + + @Override + public HostPort getServerAuthority() + { + return null; + } +} diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MockRequest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MockRequest.java new file mode 100644 index 00000000000..14f470b9d7e --- /dev/null +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/MockRequest.java @@ -0,0 +1,173 @@ +// +// ======================================================================== +// 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.ee10.servlet; + +import java.util.Set; +import java.util.concurrent.TimeoutException; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Predicate; + +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.server.Components; +import org.eclipse.jetty.server.ConnectionMetaData; +import org.eclipse.jetty.server.Context; +import org.eclipse.jetty.server.HttpStream; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Session; +import org.eclipse.jetty.server.TunnelSupport; + +public class MockRequest implements Request +{ + @Override + public void fail(Throwable failure) + { + } + + @Override + public String getId() + { + return null; + } + + @Override + public Components getComponents() + { + return null; + } + + @Override + public ConnectionMetaData getConnectionMetaData() + { + return null; + } + + @Override + public String getMethod() + { + return null; + } + + @Override + public HttpURI getHttpURI() + { + return null; + } + + @Override + public Context getContext() + { + return null; + } + + @Override + public HttpFields getHeaders() + { + return null; + } + + @Override + public void demand(Runnable demandCallback) + { + } + + @Override + public HttpFields getTrailers() + { + return null; + } + + @Override + public long getBeginNanoTime() + { + return 0; + } + + @Override + public long getHeadersNanoTime() + { + return 0; + } + + @Override + public boolean isSecure() + { + return false; + } + + @Override + public Content.Chunk read() + { + return null; + } + + @Override + public boolean consumeAvailable() + { + return false; + } + + @Override + public void addIdleTimeoutListener(Predicate onIdleTimeout) + { + } + + @Override + public void addFailureListener(Consumer onFailure) + { + } + + @Override + public TunnelSupport getTunnelSupport() + { + return null; + } + + @Override + public void addHttpStreamWrapper(Function wrapper) + { + } + + @Override + public Session getSession(boolean create) + { + return null; + } + + @Override + public Object removeAttribute(String name) + { + return null; + } + + @Override + public Object setAttribute(String name, Object attribute) + { + return null; + } + + @Override + public Object getAttribute(String name) + { + return null; + } + + @Override + public Set getAttributeNameSet() + { + return null; + } +} diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AbstractTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AbstractTest.java index e8f7583bc31..05a1b755a02 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AbstractTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/AbstractTest.java @@ -21,6 +21,7 @@ import java.security.KeyStore; import java.util.Collection; import java.util.EnumSet; import java.util.List; +import java.util.function.Consumer; import jakarta.servlet.http.HttpServlet; import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; @@ -170,12 +171,19 @@ public class AbstractTest } protected void startClient(Transport transport) throws Exception + { + startClient(transport, null); + } + + protected void startClient(Transport transport, Consumer consumer) throws Exception { QueuedThreadPool clientThreads = new QueuedThreadPool(); clientThreads.setName("client"); client = new HttpClient(newHttpClientTransport(transport)); client.setExecutor(clientThreads); client.setSocketAddressResolver(new SocketAddressResolver.Sync()); + if (consumer != null) + consumer.accept(client); client.start(); } diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/HttpClientContinueTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/HttpClientContinueTest.java index 294d7442590..43d09a083a8 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/HttpClientContinueTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/HttpClientContinueTest.java @@ -676,6 +676,55 @@ public class HttpClientContinueTest extends AbstractTest assertTrue(latch.await(5, TimeUnit.SECONDS)); } + @ParameterizedTest + @MethodSource("transportsNoFCGI") + public void test100ContinueThenTimeoutThenSendError(Transport transport) throws Exception + { + long idleTimeout = 1000; + + CountDownLatch serverLatch = new CountDownLatch(1); + startServer(transport, new HttpServlet() + { + @Override + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException + { + // Send the 100 Continue. + ServletInputStream input = request.getInputStream(); + try + { + // Echo the content. + IO.copy(input, response.getOutputStream()); + } + catch (IOException x) + { + // The copy failed b/c of idle timeout, time to try + // to send an error which should have no effect. + response.sendError(HttpStatus.IM_A_TEAPOT_418); + serverLatch.countDown(); + } + } + }); + startClient(transport, httpClient -> httpClient.setIdleTimeout(idleTimeout)); + + AsyncRequestContent requestContent = new AsyncRequestContent(); + requestContent.write(ByteBuffer.wrap(new byte[512]), Callback.NOOP); + CountDownLatch clientLatch = new CountDownLatch(1); + client.newRequest(newURI(transport)) + .headers(headers -> headers.put(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE.asString())) + .body(requestContent) + .send(result -> + { + if (result.isFailed() && result.getResponse().getStatus() == HttpStatus.CONTINUE_100) + clientLatch.countDown(); + }); + + // Wait more than the idle timeout to break the connection. + Thread.sleep(2 * idleTimeout); + + assertTrue(serverLatch.await(5, TimeUnit.SECONDS)); + assertTrue(clientLatch.await(5, TimeUnit.SECONDS)); + } + @Test public void testExpect100ContinueWithTwoResponsesInOneRead() throws Exception { diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/ServerTimeoutsTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/ServerTimeoutsTest.java index 49a2f377d0f..fbe33e19868 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/ServerTimeoutsTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-client-transports/src/test/java/org/eclipse/jetty/ee10/test/client/transport/ServerTimeoutsTest.java @@ -52,6 +52,7 @@ import org.junit.jupiter.params.provider.MethodSource; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -413,8 +414,6 @@ public class ServerTimeoutsTest extends AbstractTest @MethodSource("transportsNoFCGI") public void testBlockingReadHttpIdleTimeoutOverridesIdleTimeout(Transport transport) throws Exception { - assumeTrue(transport != Transport.H3); // TODO Fix H3 - long httpIdleTimeout = 2500; long idleTimeout = 3 * httpIdleTimeout; httpConfig.setIdleTimeout(httpIdleTimeout); @@ -444,7 +443,19 @@ public class ServerTimeoutsTest extends AbstractTest @ParameterizedTest @MethodSource("transportsNoFCGI") - public void testAsyncReadHttpIdleTimeoutOverridesIdleTimeout(Transport transport) throws Exception + public void testAsyncReadHttpIdleTimeoutOverridesIdleTimeoutIsReadyFirst(Transport transport) throws Exception + { + testAsyncReadHttpIdleTimeoutOverridesIdleTimeout(transport, true); + } + + @ParameterizedTest + @MethodSource("transportsNoFCGI") + public void testAsyncReadHttpIdleTimeoutOverridesIdleTimeoutReadFirst(Transport transport) throws Exception + { + testAsyncReadHttpIdleTimeoutOverridesIdleTimeout(transport, false); + } + + private void testAsyncReadHttpIdleTimeoutOverridesIdleTimeout(Transport transport, boolean isReadyFirst) throws Exception { long httpIdleTimeout = 2000; long idleTimeout = 3 * httpIdleTimeout; @@ -463,6 +474,8 @@ public class ServerTimeoutsTest extends AbstractTest @Override public void onDataAvailable() throws IOException { + if (isReadyFirst) + assertTrue(input.isReady()); assertEquals(0, input.read()); assertFalse(input.isReady()); } @@ -477,17 +490,10 @@ public class ServerTimeoutsTest extends AbstractTest { if (failure instanceof TimeoutException) { - response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500); + response.setStatus(HttpStatus.GATEWAY_TIMEOUT_504); handlerLatch.countDown(); } - // TODO the problem here is that timeout failures are currently persistent and affect reads - // and writes. So after the 500 is set above, the complete below tries to commit the response, - // but the write/send for that fails with the same timeout exception. Thus the 500 is never - // sent and the connection is just closed. - // This was not apparent until the change in HttpOutput#onWriteComplete to not always abort on - // failure (as this prevents async handling completing on its own terms). - // We can "fix" this here by doing a response.sendError(-1); asyncContext.complete(); } }); @@ -501,7 +507,7 @@ public class ServerTimeoutsTest extends AbstractTest .body(content) .send(result -> { - if (result.getResponse().getStatus() == HttpStatus.INTERNAL_SERVER_ERROR_500) + if (result.getResponse().getStatus() == HttpStatus.GATEWAY_TIMEOUT_504) resultLatch.countDown(); }); @@ -648,19 +654,13 @@ public class ServerTimeoutsTest extends AbstractTest } @Override - protected void service(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void service(HttpServletRequest request, HttpServletResponse response) throws IOException { ServletInputStream input = request.getInputStream(); assertEquals(0, input.read()); - try - { - input.read(); - } - catch (IOException x) - { - handlerLatch.countDown(); - throw x; - } + IOException x = assertThrows(IOException.class, input::read); + handlerLatch.countDown(); + throw x; } } } diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/pom.xml b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/pom.xml index afeea1be151..942b077b16a 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/pom.xml +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/pom.xml @@ -44,6 +44,11 @@ org.slf4j slf4j-api + + org.awaitility + awaitility + test + org.eclipse.jetty jetty-alpn-server diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputIntegrationTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputIntegrationTest.java index e816fc9fdd1..ffb025e2e2f 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputIntegrationTest.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputIntegrationTest.java @@ -34,36 +34,43 @@ import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.ee10.servlet.ServletApiRequest; +import org.eclipse.jetty.ee10.servlet.ServletChannelState; import org.eclipse.jetty.ee10.servlet.ServletContextHandler; +import org.eclipse.jetty.ee10.servlet.ServletContextRequest; import org.eclipse.jetty.ee10.servlet.ServletHolder; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory; +import org.eclipse.jetty.io.ArrayByteBufferPool; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.LocalConnector.LocalEndPoint; import org.eclipse.jetty.server.NetworkConnector; -import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.SecureRequestCustomizer; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -@Disabled //TODO needs investigation public class HttpInputIntegrationTest { enum Mode @@ -74,13 +81,15 @@ public class HttpInputIntegrationTest private static Server __server; private static HttpConfiguration __config; private static SslContextFactory.Server __sslContextFactory; + private static ArrayByteBufferPool.Tracking __bufferPool; @BeforeAll public static void beforeClass() throws Exception { __config = new HttpConfiguration(); - __server = new Server(); + __bufferPool = new ArrayByteBufferPool.Tracking(); + __server = new Server(null, null, __bufferPool); LocalConnector local = new LocalConnector(__server, new HttpConnectionFactory(__config)); local.setIdleTimeout(4000); __server.addConnector(local); @@ -128,9 +137,16 @@ public class HttpInputIntegrationTest } @AfterAll - public static void afterClass() throws Exception + public static void afterClass() { - __server.stop(); + try + { + assertThat("Server leaks: " + __bufferPool.dumpLeaks(), __bufferPool.getLeaks().size(), Matchers.is(0)); + } + finally + { + LifeCycle.stop(__server); + } } interface TestClient @@ -199,7 +215,7 @@ public class HttpInputIntegrationTest return tests.stream().map(Arguments::of); } - private static void runMode(Mode mode, Request request, Runnable test) + private static void runMode(Mode mode, ServletContextRequest request, Runnable test) { switch (mode) { @@ -237,27 +253,24 @@ public class HttpInputIntegrationTest case ASYNC_OTHER_WAIT: { CountDownLatch latch = new CountDownLatch(1); - //TODO - /* HttpChannel.State state = request.getHttpChannelState().getState(); + ServletChannelState servletRequestState = request.getServletChannel().getServletRequestState(); + ServletChannelState.State state = servletRequestState.getState(); new Thread(() -> { try { if (!latch.await(5, TimeUnit.SECONDS)) fail("latch expired"); - - // Spin until state change - while (request.getHttpChannelState().getState() == state) - { - Thread.yield(); - } + + // Wait until the state changes. + await().atMost(5, TimeUnit.SECONDS).until(servletRequestState::getState, not(state)); test.run(); } catch (Exception e) { e.printStackTrace(); } - }).start();*/ + }).start(); // ensure other thread running before trying to return latch.countDown(); break; @@ -291,6 +304,7 @@ public class HttpInputIntegrationTest assertTrue(response.contains("sum=" + sum)); } + @Tag("stress") @ParameterizedTest(name = "[{index}] STRESS {0}") @MethodSource("scenarios") public void testStress(Scenario scenario) throws Exception @@ -373,7 +387,7 @@ public class HttpInputIntegrationTest catch (Exception e) { e.printStackTrace(); - resp.setStatus(500); + resp.setStatus(599); resp.getWriter().println("read=" + e); resp.getWriter().println("sum=-1"); } @@ -384,12 +398,11 @@ public class HttpInputIntegrationTest AsyncContext context = req.startAsync(); context.setTimeout(10000); ServletInputStream in = req.getInputStream(); - //TODO - //Request request = Request.getBaseRequest(req); + ServletContextRequest request = (ServletContextRequest)((ServletApiRequest)req).getRequest(); AtomicInteger read = new AtomicInteger(0); AtomicInteger sum = new AtomicInteger(0); - runMode(mode, /* request */ null, () -> in.setReadListener(new ReadListener() + runMode(mode, request, () -> in.setReadListener(new ReadListener() { @Override public void onError(Throwable t) @@ -397,7 +410,7 @@ public class HttpInputIntegrationTest t.printStackTrace(); try { - resp.sendError(500); + resp.sendError(599); } catch (IOException e) { @@ -410,7 +423,7 @@ public class HttpInputIntegrationTest @Override public void onDataAvailable() { - runMode(mode, /* request */ null, () -> + runMode(mode, request, () -> { while (in.isReady() && !in.isFinished()) { @@ -423,9 +436,7 @@ public class HttpInputIntegrationTest int i = read.getAndIncrement(); if (b != expected.charAt(i)) { - /*System.err.printf("XXX '%c'!='%c' at %d%n", expected.charAt(i), (char)b, i); - System.err.println(" " + request.getHttpChannel()); - System.err.println(" " + request.getHttpChannel().getHttpTransport());*/ + onError(new AssertionError("'%c'!='%c' at %d".formatted(expected.charAt(i), (char)b, i))); } } catch (IOException e) diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputInterceptorTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputInterceptorTest.java deleted file mode 100644 index 13cd05ac2ed..00000000000 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputInterceptorTest.java +++ /dev/null @@ -1,342 +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.ee10.test; - -import java.io.IOException; -import java.nio.ByteBuffer; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; - -import jakarta.servlet.AsyncContext; -import jakarta.servlet.ReadListener; -import jakarta.servlet.ServletInputStream; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; -import org.eclipse.jetty.client.AsyncRequestContent; -import org.eclipse.jetty.client.BytesRequestContent; -import org.eclipse.jetty.client.ContentResponse; -import org.eclipse.jetty.client.HttpClient; -import org.eclipse.jetty.http.HttpMethod; -import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.server.Handler; -import org.eclipse.jetty.server.HttpConnectionFactory; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.ServerConnector; -import org.eclipse.jetty.util.IO; -import org.eclipse.jetty.util.component.LifeCycle; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.core.Is.is; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; - -@Disabled //TODO needs investigation -public class HttpInputInterceptorTest -{ - private Server server; - private HttpConnectionFactory httpConnectionFactory = new HttpConnectionFactory(); - private ServerConnector connector; - private HttpClient client; - - private void start(Handler handler) throws Exception - { - server = new Server(); - connector = new ServerConnector(server, 1, 1, httpConnectionFactory); - server.addConnector(connector); - - server.setHandler(handler); - - client = new HttpClient(); - server.addBean(client); - - server.start(); - } - - @AfterEach - public void dispose() - { - LifeCycle.stop(server); - } - - @Test - public void testBlockingReadInterceptorThrows() throws Exception - { - CountDownLatch serverLatch = new CountDownLatch(1); - //TODO - /* start(new AbstractHandler() - { - @Override - public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) - { - jettyRequest.setHandled(true); - - // Throw immediately from the interceptor. - jettyRequest.getHttpInput().addInterceptor(content -> - { - throw new RuntimeException(); - }); - - assertThrows(IOException.class, () -> IO.readBytes(request.getInputStream())); - serverLatch.countDown(); - response.setStatus(HttpStatus.NO_CONTENT_204); - } - });*/ - - ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) - .method(HttpMethod.POST) - .body(new BytesRequestContent(new byte[1])) - .timeout(5, TimeUnit.SECONDS) - .send(); - - assertTrue(serverLatch.await(5, TimeUnit.SECONDS)); - assertEquals(HttpStatus.NO_CONTENT_204, response.getStatus()); - } - - @Test - public void testBlockingReadInterceptorConsumesHalfThenThrows() throws Exception - { - CountDownLatch serverLatch = new CountDownLatch(1); - //TODO - /* start(new AbstractHandler() - { - @Override - public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) - { - jettyRequest.setHandled(true); - - // Consume some and then throw. - AtomicInteger readCount = new AtomicInteger(); - jettyRequest.getHttpInput().addInterceptor(content -> - { - int reads = readCount.incrementAndGet(); - if (reads == 1) - { - ByteBuffer buffer = content.getByteBuffer(); - int half = buffer.remaining() / 2; - int limit = buffer.limit(); - buffer.limit(buffer.position() + half); - ByteBuffer chunk = buffer.slice(); - buffer.position(buffer.limit()); - buffer.limit(limit); - return new HttpInput.Content(chunk); - } - throw new RuntimeException(); - }); - - assertThrows(IOException.class, () -> IO.readBytes(request.getInputStream())); - serverLatch.countDown(); - response.setStatus(HttpStatus.NO_CONTENT_204); - } - });*/ - - ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) - .method(HttpMethod.POST) - .body(new BytesRequestContent(new byte[1024])) - .timeout(5, TimeUnit.SECONDS) - .send(); - - assertTrue(serverLatch.await(5, TimeUnit.SECONDS)); - assertEquals(HttpStatus.NO_CONTENT_204, response.getStatus()); - } - - @Test - public void testAvailableReadInterceptorThrows() throws Exception - { - CountDownLatch interceptorLatch = new CountDownLatch(1); - //TODO - /* start(new AbstractHandler() - { - @Override - public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException - { - jettyRequest.setHandled(true); - - // Throw immediately from the interceptor. - jettyRequest.getHttpInput().addInterceptor(content -> - { - interceptorLatch.countDown(); - throw new RuntimeException(); - }); - - int available = request.getInputStream().available(); - assertEquals(0, available); - } - });*/ - - ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) - .method(HttpMethod.POST) - .body(new BytesRequestContent(new byte[1])) - .timeout(5, TimeUnit.SECONDS) - .send(); - - assertTrue(interceptorLatch.await(5, TimeUnit.SECONDS)); - assertEquals(HttpStatus.OK_200, response.getStatus()); - } - - @Test - public void testIsReadyReadInterceptorThrows() throws Exception - { - AsyncRequestContent asyncRequestContent = new AsyncRequestContent(ByteBuffer.wrap(new byte[1])); - CountDownLatch interceptorLatch = new CountDownLatch(1); - CountDownLatch readFailureLatch = new CountDownLatch(1); - //TODO - /* start(new AbstractHandler() - { - @Override - public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException - { - jettyRequest.setHandled(true); - - AtomicBoolean onDataAvailable = new AtomicBoolean(); - jettyRequest.getHttpInput().addInterceptor(content -> - { - if (onDataAvailable.get()) - { - interceptorLatch.countDown(); - throw new RuntimeException(); - } - else - { - return content; - } - }); - - AsyncContext asyncContext = request.startAsync(); - ServletInputStream input = request.getInputStream(); - input.setReadListener(new ReadListener() - { - @Override - public void onDataAvailable() - { - onDataAvailable.set(true); - - // The input.setReadListener() call called the interceptor so there is content for read(). - assertThat(input.isReady(), is(true)); - assertDoesNotThrow(() -> assertEquals(0, input.read())); - - // Make the client send more content so that the interceptor will be called again. - asyncRequestContent.offer(ByteBuffer.wrap(new byte[1])); - asyncRequestContent.close(); - sleep(500); // Wait a little to make sure the content arrived by next isReady() call. - - // The interceptor should throw, but isReady() should not. - assertThat(input.isReady(), is(true)); - assertThrows(IOException.class, () -> assertEquals(0, input.read())); - readFailureLatch.countDown(); - response.setStatus(HttpStatus.NO_CONTENT_204); - asyncContext.complete(); - } - - @Override - public void onAllDataRead() - { - } - - @Override - public void onError(Throwable error) - { - error.printStackTrace(); - } - }); - } - });*/ - - ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) - .method(HttpMethod.POST) - .body(asyncRequestContent) - .timeout(5, TimeUnit.SECONDS) - .send(); - - assertTrue(interceptorLatch.await(5, TimeUnit.SECONDS)); - assertTrue(readFailureLatch.await(5, TimeUnit.SECONDS)); - assertEquals(HttpStatus.NO_CONTENT_204, response.getStatus()); - } - - @Test - public void testSetReadListenerReadInterceptorThrows() throws Exception - { - RuntimeException failure = new RuntimeException(); - CountDownLatch interceptorLatch = new CountDownLatch(1); - //TODO - /* start(new AbstractHandler() - { - @Override - public void handle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException - { - jettyRequest.setHandled(true); - - // Throw immediately from the interceptor. - jettyRequest.getHttpInput().addInterceptor(content -> - { - interceptorLatch.countDown(); - failure.addSuppressed(new Throwable()); - throw failure; - }); - - AsyncContext asyncContext = request.startAsync(); - ServletInputStream input = request.getInputStream(); - input.setReadListener(new ReadListener() - { - @Override - public void onDataAvailable() - { - } - - @Override - public void onAllDataRead() - { - } - - @Override - public void onError(Throwable error) - { - assertSame(failure, error.getCause()); - response.setStatus(HttpStatus.NO_CONTENT_204); - asyncContext.complete(); - } - }); - } - });*/ - - ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) - .method(HttpMethod.POST) - .body(new BytesRequestContent(new byte[1])) - .timeout(5, TimeUnit.SECONDS) - .send(); - - assertTrue(interceptorLatch.await(5, TimeUnit.SECONDS)); - assertEquals(HttpStatus.NO_CONTENT_204, response.getStatus()); - } - - private static void sleep(long time) - { - try - { - Thread.sleep(time); - } - catch (InterruptedException x) - { - throw new RuntimeException(x); - } - } -} diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputTransientErrorTest.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputTransientErrorTest.java new file mode 100644 index 00000000000..ca7658ce2d5 --- /dev/null +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/HttpInputTransientErrorTest.java @@ -0,0 +1,430 @@ +// +// ======================================================================== +// 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.ee10.test; + +import java.io.IOException; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import jakarta.servlet.AsyncContext; +import jakarta.servlet.ReadListener; +import jakarta.servlet.ServletInputStream; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.ee10.servlet.ServletContextHandler; +import org.eclipse.jetty.ee10.servlet.ServletHolder; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.io.ArrayByteBufferPool; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.component.LifeCycle; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +//TODO test all protocols +public class HttpInputTransientErrorTest +{ + private static final int IDLE_TIMEOUT = 250; + + private LocalConnector connector; + private Server server; + private ArrayByteBufferPool.Tracking bufferPool; + + @AfterEach + public void tearDown() + { + try + { + if (bufferPool != null) + assertThat("Server leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0)); + } + finally + { + LifeCycle.stop(server); + } + } + + private void startServer(HttpServlet servlet) throws Exception + { + bufferPool = new ArrayByteBufferPool.Tracking(); + server = new Server(null, null, bufferPool); + connector = new LocalConnector(server, new HttpConnectionFactory()); + connector.setIdleTimeout(IDLE_TIMEOUT); + server.addConnector(connector); + + ServletContextHandler context = new ServletContextHandler("/ctx"); + server.setHandler(context); + ServletHolder holder = new ServletHolder(servlet); + holder.setAsyncSupported(true); + context.addServlet(holder, "/*"); + + server.start(); + } + + @Test + public void testAsyncServletHandleError() throws Exception + { + List events = new CopyOnWriteArrayList<>(); + AtomicReference failure = new AtomicReference<>(); + startServer(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException + { + AsyncContext asyncContext = req.startAsync(req, resp); + asyncContext.setTimeout(0); + resp.setContentType("text/plain;charset=UTF-8"); + + // Since the client sends a request with a content-length header, but sends + // the content only after idle timeout expired, this ReadListener will have + // onError() executed first, then since onError() charges on and reads the content, + // onDataAvailable and onAllDataRead are called afterwards. + req.getInputStream().setReadListener(new ReadListener() + { + final AtomicInteger counter = new AtomicInteger(); + + @Override + public void onDataAvailable() throws IOException + { + ServletInputStream input = req.getInputStream(); + while (true) + { + if (!input.isReady()) + break; + int read = input.read(); + if (read < 0) + break; + else + counter.incrementAndGet(); + } + } + + @Override + public void onAllDataRead() throws IOException + { + events.add("onAllDataRead"); + resp.setStatus(HttpStatus.OK_200); + resp.setContentType("text/plain;charset=UTF-8"); + resp.getWriter().println("read=" + counter.get()); + asyncContext.complete(); + } + + @Override + public void onError(Throwable t) + { + events.add("onError"); + if (failure.compareAndSet(null, t)) + { + try + { + // The first error is transient, just try to read normally. + onDataAvailable(); + } + catch (IOException e) + { + resp.setStatus(599); + asyncContext.complete(); + } + } + else + { + resp.setStatus(598); + t.printStackTrace(); + asyncContext.complete(); + } + } + }); + } + }); + + try (LocalConnector.LocalEndPoint localEndPoint = connector.connect()) + { + String request = """ + POST /ctx/post HTTP/1.1 + Host: local + Content-Length: 10 + + """; + localEndPoint.addInput(request); + Thread.sleep((long)(IDLE_TIMEOUT * 1.5)); + localEndPoint.addInput("1234567890"); + HttpTester.Response response = HttpTester.parseResponse(localEndPoint.getResponse(false, 5, TimeUnit.SECONDS)); + + assertThat("Unexpected response status\n" + response + response.getContent(), response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.get(HttpHeader.CONNECTION), nullValue()); + assertThat(response.get(HttpHeader.CONTENT_TYPE), is("text/plain;charset=UTF-8")); + assertThat(response.getContent(), containsString("read=10")); + assertInstanceOf(TimeoutException.class, failure.get()); + assertThat(events, contains("onError", "onAllDataRead")); + } + } + + @Test + public void testAsyncTimeoutThenSetReadListenerThenRead() throws Exception + { + CountDownLatch doPostlatch = new CountDownLatch(1); + + AtomicReference failure = new AtomicReference<>(); + startServer(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) + { + AsyncContext asyncContext = req.startAsync(req, resp); + asyncContext.setTimeout(0); + resp.setContentType("text/plain;charset=UTF-8"); + + // Not calling setReadListener will make Jetty set the ServletChannelState + // in state WAITING upon doPost return, so idle timeouts are ignored. + new Thread(() -> + { + try + { + doPostlatch.await(5, TimeUnit.SECONDS); + + req.getInputStream().setReadListener(new ReadListener() + { + final AtomicInteger counter = new AtomicInteger(); + + @Override + public void onDataAvailable() throws IOException + { + ServletInputStream input = req.getInputStream(); + while (true) + { + if (!input.isReady()) + break; + int read = input.read(); + if (read < 0) + break; + else + counter.incrementAndGet(); + } + } + + @Override + public void onAllDataRead() throws IOException + { + resp.setStatus(HttpStatus.OK_200); + resp.setContentType("text/plain;charset=UTF-8"); + resp.getWriter().println("read=" + counter.get()); + asyncContext.complete(); + } + + @Override + public void onError(Throwable t) + { + failure.set(t); + resp.setStatus(598); + t.printStackTrace(); + asyncContext.complete(); + } + }); + } + catch (Exception e) + { + throw new AssertionError(e); + } + }).start(); + } + }); + + try (LocalConnector.LocalEndPoint localEndPoint = connector.connect()) + { + String request = """ + POST /ctx/post HTTP/1.1 + Host: local + Content-Length: 10 + + """; + localEndPoint.addInput(request); + Thread.sleep((long)(IDLE_TIMEOUT * 1.5)); + localEndPoint.addInput("1234567890"); + doPostlatch.countDown(); + HttpTester.Response response = HttpTester.parseResponse(localEndPoint.getResponse(false, 5, TimeUnit.SECONDS)); + + assertThat("Unexpected response status\n" + response + response.getContent(), response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.get(HttpHeader.CONTENT_TYPE), is("text/plain;charset=UTF-8")); + assertThat(response.getContent(), containsString("read=10")); + assertThat(failure.get(), nullValue()); + } + } + + @Test + public void testAsyncServletStopOnError() throws Exception + { + AtomicReference failure = new AtomicReference<>(); + startServer(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException + { + AsyncContext asyncContext = req.startAsync(req, resp); + asyncContext.setTimeout(0); + resp.setContentType("text/plain;charset=UTF-8"); + + req.getInputStream().setReadListener(new ReadListener() + { + @Override + public void onDataAvailable() + { + throw new AssertionError(); + } + + @Override + public void onAllDataRead() + { + throw new AssertionError(); + } + + @Override + public void onError(Throwable t) + { + if (failure.compareAndSet(null, t)) + { + resp.setStatus(HttpStatus.IM_A_TEAPOT_418); + asyncContext.complete(); + } + else + { + resp.setStatus(599); + t.printStackTrace(); + asyncContext.complete(); + } + } + }); + } + }); + + try (LocalConnector.LocalEndPoint localEndPoint = connector.connect()) + { + String request = """ + POST /ctx/post HTTP/1.1 + Host: local + Content-Length: 10 + + """; + localEndPoint.addInput(request); + Thread.sleep((long)(IDLE_TIMEOUT * 1.5)); + localEndPoint.addInput("1234567890"); + HttpTester.Response response = HttpTester.parseResponse(localEndPoint.getResponse(false, 5, TimeUnit.SECONDS)); + + assertThat("Unexpected response status\n" + response + response.getContent(), response.getStatus(), is(HttpStatus.IM_A_TEAPOT_418)); + assertInstanceOf(TimeoutException.class, failure.get()); + } + } + + @Test + public void testBlockingServletHandleError() throws Exception + { + AtomicReference failure = new AtomicReference<>(); + startServer(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException + { + try + { + IO.toString(req.getInputStream()); + } + catch (IOException e) + { + failure.set(e); + } + + String content = IO.toString(req.getInputStream()); + resp.setStatus(HttpStatus.OK_200); + resp.setContentType("text/plain;charset=UTF-8"); + resp.getWriter().println("read=" + content.length()); + } + }); + + try (LocalConnector.LocalEndPoint localEndPoint = connector.connect()) + { + String request = """ + POST /ctx/post HTTP/1.1 + Host: local + Content-Length: 10 + + """; + localEndPoint.addInput(request); + Thread.sleep((long)(IDLE_TIMEOUT * 1.5)); + localEndPoint.addInput("1234567890"); + HttpTester.Response response = HttpTester.parseResponse(localEndPoint.getResponse(false, 5, TimeUnit.SECONDS)); + + assertThat("Unexpected response status\n" + response + response.getContent(), response.getStatus(), is(HttpStatus.OK_200)); + assertThat(response.get(HttpHeader.CONTENT_TYPE), is("text/plain;charset=UTF-8")); + assertThat(response.getContent(), containsString("read=10")); + assertInstanceOf(IOException.class, failure.get()); + assertInstanceOf(TimeoutException.class, failure.get().getCause()); + } + } + + @Test + public void testBlockingServletStopOnError() throws Exception + { + AtomicReference failure = new AtomicReference<>(); + startServer(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) + { + try + { + IO.toString(req.getInputStream()); + } + catch (IOException e) + { + failure.set(e); + resp.setStatus(HttpStatus.IM_A_TEAPOT_418); + } + } + }); + + try (LocalConnector.LocalEndPoint localEndPoint = connector.connect()) + { + String request = """ + POST /ctx/post HTTP/1.1 + Host: local + Content-Length: 10 + + """; + localEndPoint.addInput(request); + Thread.sleep((long)(IDLE_TIMEOUT * 1.5)); + localEndPoint.addInput("1234567890"); + HttpTester.Response response = HttpTester.parseResponse(localEndPoint.getResponse(false, 5, TimeUnit.SECONDS)); + + assertThat("Unexpected response status\n" + response + response.getContent(), response.getStatus(), is(HttpStatus.IM_A_TEAPOT_418)); + assertInstanceOf(IOException.class, failure.get()); + assertInstanceOf(TimeoutException.class, failure.get().getCause()); + } + } +} diff --git a/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/ProxyServlet.java b/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/ProxyServlet.java index a58bf1ef72e..f9520096bcf 100644 --- a/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/ProxyServlet.java +++ b/jetty-ee9/jetty-ee9-proxy/src/main/java/org/eclipse/jetty/ee9/proxy/ProxyServlet.java @@ -259,6 +259,8 @@ public class ProxyServlet extends AbstractProxyServlet Content.Chunk chunk = super.read(); if (Content.Chunk.isFailure(chunk)) { + if (!chunk.isLast()) + fail(chunk.getFailure()); onClientRequestFailure(request, proxyRequest, response, chunk.getFailure()); } else diff --git a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/pom.xml b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/pom.xml index 3dae358c226..1b513e6478d 100644 --- a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/pom.xml +++ b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/pom.xml @@ -44,6 +44,11 @@ org.slf4j slf4j-api + + org.awaitility + awaitility + test + org.eclipse.jetty jetty-alpn-server diff --git a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputIntegrationTest.java b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputIntegrationTest.java index 80d53eb1ae1..759af89dd66 100644 --- a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputIntegrationTest.java +++ b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputIntegrationTest.java @@ -34,38 +34,42 @@ import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.ee9.nested.HttpChannelState; +import org.eclipse.jetty.ee9.nested.Request; import org.eclipse.jetty.ee9.servlet.ServletContextHandler; import org.eclipse.jetty.ee9.servlet.ServletHolder; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory; +import org.eclipse.jetty.io.ArrayByteBufferPool; import org.eclipse.jetty.server.Connector; -import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.LocalConnector.LocalEndPoint; import org.eclipse.jetty.server.NetworkConnector; -import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.SecureRequestCustomizer; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Tag; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import static org.awaitility.Awaitility.await; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -@Disabled //TODO needs investigation public class HttpInputIntegrationTest { enum Mode @@ -76,13 +80,15 @@ public class HttpInputIntegrationTest private static Server __server; private static HttpConfiguration __config; private static SslContextFactory.Server __sslContextFactory; + private static ArrayByteBufferPool.Tracking __bufferPool; @BeforeAll public static void beforeClass() throws Exception { __config = new HttpConfiguration(); - __server = new Server(); + __bufferPool = new ArrayByteBufferPool.Tracking(); + __server = new Server(null, null, __bufferPool); LocalConnector local = new LocalConnector(__server, new HttpConnectionFactory(__config)); local.setIdleTimeout(4000); __server.addConnector(local); @@ -129,9 +135,16 @@ public class HttpInputIntegrationTest } @AfterAll - public static void afterClass() throws Exception + public static void afterClass() { - __server.stop(); + try + { + assertThat("Server leaks: " + __bufferPool.dumpLeaks(), __bufferPool.getLeaks().size(), Matchers.is(0)); + } + finally + { + LifeCycle.stop(__server); + } } interface TestClient @@ -238,26 +251,23 @@ public class HttpInputIntegrationTest case ASYNC_OTHER_WAIT: { CountDownLatch latch = new CountDownLatch(1); - /* HttpChannel.State state = request.getHttpChannelState().getState(); + HttpChannelState.State state = request.getHttpChannelState().getState(); new Thread(() -> { try { if (!latch.await(5, TimeUnit.SECONDS)) fail("latch expired"); - - // Spin until state change - while (request.getHttpChannelState().getState() == state) - { - Thread.yield(); - } + + // Wait until the state changes. + await().atMost(5, TimeUnit.SECONDS).until(request.getHttpChannelState()::getState, not(state)); test.run(); } catch (Exception e) { e.printStackTrace(); } - }).start();*/ + }).start(); // ensure other thread running before trying to return latch.countDown(); break; @@ -374,7 +384,7 @@ public class HttpInputIntegrationTest catch (Exception e) { e.printStackTrace(); - resp.setStatus(500); + resp.setStatus(599); resp.getWriter().println("read=" + e); resp.getWriter().println("sum=-1"); } @@ -385,12 +395,11 @@ public class HttpInputIntegrationTest AsyncContext context = req.startAsync(); context.setTimeout(10000); ServletInputStream in = req.getInputStream(); - //TODO - //Request request = Request.getBaseRequest(req); + Request request = (Request)req; AtomicInteger read = new AtomicInteger(0); AtomicInteger sum = new AtomicInteger(0); - runMode(mode, null /*request*/, () -> in.setReadListener(new ReadListener() + runMode(mode, request, () -> in.setReadListener(new ReadListener() { @Override public void onError(Throwable t) @@ -398,7 +407,7 @@ public class HttpInputIntegrationTest t.printStackTrace(); try { - resp.sendError(500); + resp.sendError(599); } catch (IOException e) { @@ -411,7 +420,7 @@ public class HttpInputIntegrationTest @Override public void onDataAvailable() { - runMode(mode, null/*request*/, () -> + runMode(mode, request, () -> { while (in.isReady() && !in.isFinished()) { @@ -422,12 +431,10 @@ public class HttpInputIntegrationTest return; sum.addAndGet(b); int i = read.getAndIncrement(); - /* if (b != expected.charAt(i)) + if (b != expected.charAt(i)) { - System.err.printf("XXX '%c'!='%c' at %d%n", expected.charAt(i), (char)b, i); - System.err.println(" " + request.getHttpChannel()); - System.err.println(" " + request.getHttpChannel().getHttpTransport()); - }*/ + onError(new AssertionError("'%c'!='%c' at %d".formatted(expected.charAt(i), (char)b, i))); + } } catch (IOException e) { diff --git a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputTransientErrorTest.java b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputTransientErrorTest.java new file mode 100644 index 00000000000..950eb29e31f --- /dev/null +++ b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/HttpInputTransientErrorTest.java @@ -0,0 +1,223 @@ +// +// ======================================================================== +// 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.ee9.test; + +import java.io.IOException; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import jakarta.servlet.AsyncContext; +import jakarta.servlet.ReadListener; +import jakarta.servlet.ServletInputStream; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.ee9.servlet.ServletContextHandler; +import org.eclipse.jetty.ee9.servlet.ServletHolder; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpTester; +import org.eclipse.jetty.io.ArrayByteBufferPool; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.LocalConnector; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.component.LifeCycle; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +//TODO test all protocols +public class HttpInputTransientErrorTest +{ + private static final int IDLE_TIMEOUT = 250; + + private LocalConnector connector; + private Server server; + private ArrayByteBufferPool.Tracking bufferPool; + + @AfterEach + public void tearDown() + { + try + { + if (bufferPool != null) + assertThat("Server leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0)); + } + finally + { + LifeCycle.stop(server); + } + } + + private void startServer(HttpServlet servlet) throws Exception + { + bufferPool = new ArrayByteBufferPool.Tracking(); + server = new Server(null, null, bufferPool); + connector = new LocalConnector(server, new HttpConnectionFactory()); + connector.setIdleTimeout(IDLE_TIMEOUT); + server.addConnector(connector); + + ServletContextHandler context = new ServletContextHandler(server, "/ctx"); + ServletHolder holder = new ServletHolder(servlet); + holder.setAsyncSupported(true); + context.addServlet(holder, "/*"); + + server.start(); + } + + @Test + public void testAsyncServletTimeoutErrorIsTerminal() throws Exception + { + List failures = new CopyOnWriteArrayList<>(); + startServer(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException + { + AsyncContext asyncContext = req.startAsync(req, resp); + asyncContext.setTimeout(0); + resp.setContentType("text/plain;charset=UTF-8"); + + req.getInputStream().setReadListener(new ReadListener() + { + @Override + public void onDataAvailable() + { + + } + + @Override + public void onAllDataRead() + { + + } + + @Override + public void onError(Throwable t) + { + failures.add(t); + try + { + ServletInputStream input = req.getInputStream(); + if (!input.isReady()) + { + resp.setStatus(597); + asyncContext.complete(); + return; + } + try + { + input.read(); + resp.setStatus(598); + asyncContext.complete(); + return; + } + catch (IOException e) + { + failures.add(e); + } + + resp.setStatus(HttpStatus.OK_200); + asyncContext.complete(); + } + catch (IOException e) + { + resp.setStatus(599); + e.printStackTrace(); + asyncContext.complete(); + } + } + }); + } + }); + + try (LocalConnector.LocalEndPoint localEndPoint = connector.connect()) + { + String request = """ + POST /ctx/post HTTP/1.1 + Host: local + Content-Length: 10 + + """; + localEndPoint.addInput(request); + Thread.sleep((long)(IDLE_TIMEOUT * 1.5)); + localEndPoint.addInput("1234567890"); + HttpTester.Response response = HttpTester.parseResponse(localEndPoint.getResponse(false, 5, TimeUnit.SECONDS)); + + assertThat("Unexpected response status\n" + response + response.getContent(), response.getStatus(), is(HttpStatus.OK_200)); + assertThat(failures.size(), is(2)); + assertInstanceOf(TimeoutException.class, failures.get(0)); + assertInstanceOf(IOException.class, failures.get(1)); + assertThat(failures.get(1).getCause(), sameInstance(failures.get(0))); + } + } + + @Test + public void testBlockingServletTimeoutErrorIsTerminal() throws Exception + { + List failures = new CopyOnWriteArrayList<>(); + startServer(new HttpServlet() + { + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) + { + try + { + IO.toString(req.getInputStream()); + } + catch (IOException e) + { + failures.add(e); + } + try + { + IO.toString(req.getInputStream()); + } + catch (IOException e) + { + failures.add(e); + } + + resp.setStatus(HttpStatus.OK_200); + } + }); + + try (LocalConnector.LocalEndPoint localEndPoint = connector.connect()) + { + String request = """ + POST /ctx/post HTTP/1.1 + Host: local + Content-Length: 10 + + """; + localEndPoint.addInput(request); + Thread.sleep((long)(IDLE_TIMEOUT * 1.5)); + localEndPoint.addInput("1234567890"); + HttpTester.Response response = HttpTester.parseResponse(localEndPoint.getResponse(false, 5, TimeUnit.SECONDS)); + + assertThat("Unexpected response status\n" + response + response.getContent(), response.getStatus(), is(HttpStatus.OK_200)); + assertThat(failures.size(), is(2)); + assertInstanceOf(IOException.class, failures.get(0)); + assertInstanceOf(TimeoutException.class, failures.get(0).getCause()); + assertInstanceOf(IOException.class, failures.get(1)); + assertInstanceOf(TimeoutException.class, failures.get(1).getCause()); + } + } +}