From 1ba945b39b1ca61aeb293e218ce9d1441c65cd22 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 4 Oct 2018 13:47:56 +0200 Subject: [PATCH 01/12] Issue #2941 - JDK 11: UnsupportedOperationException at AnnotationParser.scanClass. Updated to ASM 7.0-beta, and defaulted AnnotationParser to ASM 7. Signed-off-by: Simone Bordet --- .../eclipse/jetty/annotations/AnnotationParser.java | 10 +++++++--- pom.xml | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java index b6d9d35ceff..33e9f9e5f61 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationParser.java @@ -24,7 +24,6 @@ import java.io.InputStream; import java.net.URI; import java.net.URL; import java.nio.file.Path; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -69,8 +68,8 @@ import org.objectweb.asm.Opcodes; public class AnnotationParser { private static final Logger LOG = Log.getLogger(AnnotationParser.class); - protected static int ASM_OPCODE_VERSION = Opcodes.ASM6; //compatibility of api - protected static String ASM_OPCODE_VERSION_STR = "ASM6"; + protected static int ASM_OPCODE_VERSION = Opcodes.ASM7; //compatibility of api + protected static String ASM_OPCODE_VERSION_STR = "ASM7"; /** * Map of classnames scanned and the first location from which scan occurred @@ -118,6 +117,11 @@ public class AnnotationParser asmVersion = Opcodes.ASM6; break; } + case 7: + { + asmVersion = Opcodes.ASM7; + break; + } default: { LOG.warn("Unrecognized runtime asm version, assuming {}", ASM_OPCODE_VERSION_STR); diff --git a/pom.xml b/pom.xml index fae174072ef..9a16af6e4f2 100644 --- a/pom.xml +++ b/pom.xml @@ -26,7 +26,7 @@ undefined 1.1.4 - 6.2 + 7.0-beta 1.21 benchmarks 1.2.0 From cec84cf1bf4edc9bda2ed9b964d1ebe72c6738e0 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 9 Oct 2018 15:53:53 +0200 Subject: [PATCH 02/12] Issue #2796 - Max local stream count exceeded when request fails. Reviewed other possible places where max local stream count may overflow. Fixed handling of HTTP/2 stream idle timeouts. Signed-off-by: Simone Bordet --- .../eclipse/jetty/client/HttpReceiver.java | 4 +-- .../org/eclipse/jetty/client/HttpRequest.java | 3 +- .../org/eclipse/jetty/client/HttpSender.java | 4 +-- .../client/http/HttpConnectionOverHTTP.java | 4 ++- .../fcgi/client/http/HttpChannelOverFCGI.java | 2 +- .../org/eclipse/jetty/http2/HTTP2Session.java | 20 ++++++----- .../org/eclipse/jetty/http2/HTTP2Stream.java | 7 ++-- .../org/eclipse/jetty/http2/api/Stream.java | 15 +++++++++ .../client/http/HttpReceiverOverHTTP2.java | 6 ++-- .../client/http/HttpSenderOverHTTP2.java | 4 ++- .../client/http/MaxConcurrentStreamsTest.java | 33 ++++++++++--------- .../http/client/HttpClientContinueTest.java | 21 ++++++------ 12 files changed, 75 insertions(+), 48 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java index 813672fb72c..1e33255f6fa 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java @@ -550,14 +550,14 @@ public abstract class HttpReceiver // respect to concurrency between request and response. Result result = exchange.terminateResponse(); terminateResponse(exchange, result); + return true; } else { if (LOG.isDebugEnabled()) LOG.debug("Concurrent failure: response termination skipped, performed by helpers"); + return false; } - - return true; } private boolean updateResponseState(ResponseState from, ResponseState to) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java index e2ff05cd6b8..0d473085779 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java @@ -76,7 +76,7 @@ public class HttpRequest implements Request private String query; private String method = HttpMethod.GET.asString(); private HttpVersion version = HttpVersion.HTTP_1_1; - private long idleTimeout; + private long idleTimeout = -1; private long timeout; private long timeoutAt; private ContentProvider content; @@ -99,7 +99,6 @@ public class HttpRequest implements Request extractParams(query); followRedirects(client.isFollowRedirects()); - idleTimeout = client.getIdleTimeout(); HttpField acceptEncodingField = client.getAcceptEncodingField(); if (acceptEncodingField != null) headers.put(acceptEncodingField); diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java index a3093b994b2..5aa9787d610 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpSender.java @@ -579,14 +579,14 @@ public abstract class HttpSender implements AsyncContentProvider.Listener // respect to concurrency between request and response. Result result = exchange.terminateRequest(); terminateRequest(exchange, failure, result); + return true; } else { if (LOG.isDebugEnabled()) LOG.debug("Concurrent failure: request termination skipped, performed by helpers"); + return false; } - - return true; } private boolean updateRequestState(RequestState from, RequestState to) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java index 2e344c3c840..a6e69e8dad7 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java @@ -247,7 +247,9 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements Connec // Save the old idle timeout to restore it. EndPoint endPoint = getEndPoint(); idleTimeout = endPoint.getIdleTimeout(); - endPoint.setIdleTimeout(request.getIdleTimeout()); + long requestIdleTimeout = request.getIdleTimeout(); + if (requestIdleTimeout >= 0) + endPoint.setIdleTimeout(requestIdleTimeout); // One channel per connection, just delegate the send. return send(channel, exchange); diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpChannelOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpChannelOverFCGI.java index 520375ab16f..6ae8eb39fca 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpChannelOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpChannelOverFCGI.java @@ -166,7 +166,7 @@ public class HttpChannelOverFCGI extends HttpChannel { super(connection.getHttpDestination().getHttpClient().getScheduler()); this.connection = connection; - setIdleTimeout(idleTimeout); + setIdleTimeout(idleTimeout >= 0 ? idleTimeout : connection.getEndPoint().getIdleTimeout()); } @Override diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 04279864c5d..83a82d17eb7 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -341,7 +341,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio case SettingsFrame.MAX_CONCURRENT_STREAMS: { if (LOG.isDebugEnabled()) - LOG.debug("Updating max local concurrent streams to {} for {}", maxLocalStreams, this); + LOG.debug("Updating max local concurrent streams to {} for {}", value, this); maxLocalStreams = value; break; } @@ -561,7 +561,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio IStream stream = createLocalStream(streamId); stream.setListener(listener); - ControlEntry entry = new ControlEntry(frame, stream, new PromiseCallback<>(promise, stream)); + ControlEntry entry = new ControlEntry(frame, stream, new StreamPromiseCallback(promise, stream)); queued = flusher.append(entry); } // Iterate outside the synchronized block. @@ -605,7 +605,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio IStream pushStream = createLocalStream(streamId); pushStream.setListener(listener); - ControlEntry entry = new ControlEntry(frame, pushStream, new PromiseCallback<>(promise, pushStream)); + ControlEntry entry = new ControlEntry(frame, pushStream, new StreamPromiseCallback(promise, pushStream)); queued = flusher.append(entry); } // Iterate outside the synchronized block. @@ -779,6 +779,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } else { + localStreamCount.decrementAndGet(); throw new IllegalStateException("Duplicate stream " + streamId); } } @@ -815,6 +816,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } else { + remoteStreamCount.addAndGetHi(-1); onConnectionFailure(ErrorCode.PROTOCOL_ERROR.code, "duplicate_stream"); return null; } @@ -1461,21 +1463,21 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } } - private static class PromiseCallback implements Callback + private static class StreamPromiseCallback implements Callback { - private final Promise promise; - private final C value; + private final Promise promise; + private final IStream stream; - private PromiseCallback(Promise promise, C value) + private StreamPromiseCallback(Promise promise, IStream stream) { this.promise = promise; - this.value = value; + this.stream = stream; } @Override public void succeeded() { - promise.succeeded(value); + promise.succeeded(stream); } @Override diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java index b9609ad4903..265f57aa8a4 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java @@ -139,6 +139,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa { if (writing.compareAndSet(null, callback)) return true; + close(); callback.failed(new WritePendingException()); return false; } @@ -275,8 +276,6 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa private void onHeaders(HeadersFrame frame, Callback callback) { - if (updateClose(frame.isEndStream(), CloseState.Event.RECEIVED)) - session.removeStream(this); MetaData metaData = frame.getMetaData(); if (metaData.isRequest() || metaData.isResponse()) { @@ -286,6 +285,10 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa length = fields.getLongField(HttpHeader.CONTENT_LENGTH.asString()); dataLength = length >= 0 ? length : Long.MIN_VALUE; } + + if (updateClose(frame.isEndStream(), CloseState.Event.RECEIVED)) + session.removeStream(this); + callback.succeeded(); } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java index f76afeaf445..9e879cab891 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java @@ -163,6 +163,13 @@ public interface Stream */ public void onData(Stream stream, DataFrame frame, Callback callback); + /** + *

Callback method invoked when a RST_STREAM frame has been received for this stream.

+ * + * @param stream the stream + * @param frame the RST_FRAME received + * @param callback the callback to complete when the reset has been handled + */ public default void onReset(Stream stream, ResetFrame frame, Callback callback) { try @@ -214,6 +221,14 @@ public interface Stream return true; } + /** + *

Callback method invoked when the stream failed.

+ * + * @param stream the stream + * @param error the error code + * @param reason the error reason, or null + * @param callback the callback to complete when the failure has been handled + */ public default void onFailure(Stream stream, int error, String reason, Callback callback) { callback.succeeded(); diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java index a0f1817a6bb..94fa9776fa2 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java @@ -171,8 +171,10 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements Stream.Listen @Override public boolean onIdleTimeout(Stream stream, Throwable x) { - responseFailure(x); - return true; + HttpExchange exchange = getHttpExchange(); + if (exchange == null) + return false; + return !exchange.abort(x); } @Override diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java index 4950c949d3d..1635d27f912 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java @@ -67,7 +67,9 @@ public class HttpSenderOverHTTP2 extends HttpSender { channel.setStream(stream); ((IStream)stream).setAttachment(channel); - stream.setIdleTimeout(request.getIdleTimeout()); + long idleTimeout = request.getIdleTimeout(); + if (idleTimeout >= 0) + stream.setIdleTimeout(idleTimeout); if (content.hasContent() && !expects100Continue(request)) { diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/MaxConcurrentStreamsTest.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/MaxConcurrentStreamsTest.java index fe030fa838f..9e981ee1e80 100644 --- a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/MaxConcurrentStreamsTest.java +++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/MaxConcurrentStreamsTest.java @@ -18,6 +18,22 @@ package org.eclipse.jetty.http2.client.http; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.IntStream; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + import org.eclipse.jetty.client.AbstractConnectionPool; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.HttpDestination; @@ -43,21 +59,6 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.Test; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.io.IOException; -import java.net.InetSocketAddress; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import java.util.Queue; -import java.util.concurrent.ConcurrentLinkedQueue; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ForkJoinPool; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.stream.IntStream; - import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -370,7 +371,7 @@ public class MaxConcurrentStreamsTest extends AbstractTest } @Test - public void testTwoConcurrentStreamsFirstTimesOut() throws Exception + public void testTwoStreamsFirstTimesOut() throws Exception { long timeout = 1000; start(1, new EmptyServerHandler() diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java index eb34e6a7e43..a6ff741129c 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientContinueTest.java @@ -18,15 +18,6 @@ package org.eclipse.jetty.http.client; -import static org.eclipse.jetty.http.client.Transport.FCGI; -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.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assumptions.assumeTrue; - import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -65,6 +56,15 @@ import org.junit.jupiter.api.condition.DisabledIfSystemProperty; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; +import static org.eclipse.jetty.http.client.Transport.FCGI; +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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + public class HttpClientContinueTest extends AbstractTest { @Override @@ -344,13 +344,14 @@ public class HttpClientContinueTest extends AbstractTest } }); - scenario.client.setIdleTimeout(idleTimeout); + scenario.client.setIdleTimeout(2 * idleTimeout); byte[] content = new byte[1024]; final CountDownLatch latch = new CountDownLatch(1); scenario.client.newRequest(scenario.newURI()) .header(HttpHeader.EXPECT, HttpHeaderValue.CONTINUE.asString()) .content(new BytesContentProvider(content)) + .idleTimeout(idleTimeout, TimeUnit.MILLISECONDS) .send(new BufferingResponseListener() { @Override From ec2b5b1810e3b6537c86510e31b6efa58418a66e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 11 Oct 2018 15:56:47 +0200 Subject: [PATCH 03/12] Issue #2796 - Max local stream count exceeded when request fails. Bound release of the channel to stream close event. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/http2/HTTP2Session.java | 2 +- .../org/eclipse/jetty/http2/HTTP2Stream.java | 22 ++++++ .../org/eclipse/jetty/http2/api/Stream.java | 9 +++ .../client/http/HttpChannelOverHTTP2.java | 5 ++ .../client/http/HttpConnectionOverHTTP2.java | 17 ++++- .../client/http/HttpReceiverOverHTTP2.java | 6 ++ .../jetty/http/client/HttpClientLoadTest.java | 67 ++++++++++++++----- 7 files changed, 107 insertions(+), 21 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 83a82d17eb7..0b3fff14705 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -763,7 +763,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio int localCount = localStreamCount.get(); int maxCount = getMaxLocalStreams(); if (maxCount >= 0 && localCount >= maxCount) - throw new IllegalStateException("Max local stream count " + maxCount + " exceeded"); + throw new IllegalStateException("Max local stream count " + maxCount + " exceeded" + System.lineSeparator() + dump()); if (localStreamCount.compareAndSet(localCount, localCount + 1)) break; } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java index 265f57aa8a4..4d8e0ae2d4a 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java @@ -510,6 +510,13 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa } } + @Override + public void onClose() + { + super.onClose(); + notifyClosed(this); + } + private void updateStreamCount(int deltaStream, int deltaClosing) { ((HTTP2Session)session).updateStreamCount(isLocal(), deltaStream, deltaClosing); @@ -615,6 +622,21 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa } } + private void notifyClosed(Stream stream) + { + Listener listener = this.listener; + if (listener == null) + return; + try + { + listener.onClosed(stream); + } + catch (Throwable x) + { + LOG.info("Failure while notifying listener " + listener, x); + } + } + @Override public String dump() { diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java index 9e879cab891..6ddbf7b0350 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java @@ -234,6 +234,15 @@ public interface Stream callback.succeeded(); } + /** + *

Callback method invoked after the stream has been closed.

+ * + * @param stream the stream + */ + public default void onClosed(Stream stream) + { + } + /** *

Empty implementation of {@link Listener}

*/ diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java index f9cfa6c2949..4a399926c4c 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java @@ -101,6 +101,11 @@ public class HttpChannelOverHTTP2 extends HttpChannel connection.release(this); } + void onStreamClosed(Stream stream) + { + connection.onStreamClosed(stream, this); + } + @Override public void exchangeTerminated(HttpExchange exchange, Result result) { diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java index 4d4a3cfebe8..e805fe5f938 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java @@ -36,11 +36,16 @@ import org.eclipse.jetty.client.SendFailure; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http2.ErrorCode; import org.eclipse.jetty.http2.api.Session; +import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.Sweeper; public class HttpConnectionOverHTTP2 extends HttpConnection implements Sweeper.Sweepable { + private static final Logger LOG = Log.getLogger(HttpConnection.class); + private final Set activeChannels = ConcurrentHashMap.newKeySet(); private final Queue idleChannels = new ConcurrentLinkedQueue<>(); private final AtomicBoolean closed = new AtomicBoolean(); @@ -87,16 +92,16 @@ public class HttpConnectionOverHTTP2 extends HttpConnection implements Sweeper.S protected void release(HttpChannelOverHTTP2 channel) { + if (LOG.isDebugEnabled()) + LOG.debug("Released {}", channel); // Only non-push channels are released. if (activeChannels.remove(channel)) { - channel.setStream(null); // Recycle only non-failed channels. if (channel.isFailed()) channel.destroy(); else idleChannels.offer(channel); - getHttpDestination().release(this); } else { @@ -104,6 +109,14 @@ public class HttpConnectionOverHTTP2 extends HttpConnection implements Sweeper.S } } + void onStreamClosed(Stream stream, HttpChannelOverHTTP2 channel) + { + if (LOG.isDebugEnabled()) + LOG.debug("{} closed for {}", stream, channel); + channel.setStream(null); + getHttpDestination().release(this); + } + @Override public boolean onIdleTimeout(long idleTimeout) { diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java index 94fa9776fa2..dc4007ea366 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java @@ -184,6 +184,12 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements Stream.Listen callback.succeeded(); } + @Override + public void onClosed(Stream stream) + { + getHttpChannel().onStreamClosed(stream); + } + private void notifyContent(HttpExchange exchange, DataFrame frame, Callback callback) { contentNotifier.offer(new DataInfo(exchange, frame, callback)); diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java index 0d2d9fa0013..65b72262eac 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java @@ -18,10 +18,6 @@ package org.eclipse.jetty.http.client; -import static org.eclipse.jetty.http.client.Transport.UNIX_SOCKET; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.IOException; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -30,11 +26,11 @@ import java.util.Locale; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.IntStream; -import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -68,6 +64,9 @@ import org.hamcrest.Matchers; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class HttpClientLoadTest extends AbstractTest { private final Logger logger = Log.getLogger(HttpClientLoadTest.class); @@ -186,7 +185,7 @@ public class HttpClientLoadTest extends AbstractTest failures) + private void test(String scheme, String host, String method, boolean clientClose, boolean serverClose, long clientTimeout, int contentLength, final boolean checkContentLength, final CountDownLatch latch, final List failures) { long requestId = requestCount.incrementAndGet(); Request request = scenario.client.newRequest(host, scenario.getNetworkConnectorLocalPortInt().orElse(0)) @@ -215,6 +218,12 @@ public class HttpClientLoadTest extends AbstractTest 0) + { + request.header("X-Timeout", String.valueOf(clientTimeout)); + request.idleTimeout(clientTimeout, TimeUnit.MILLISECONDS); + } + switch (method) { case "GET": @@ -254,12 +263,18 @@ public class HttpClientLoadTest extends AbstractTest 0 && failure instanceof TimeoutException)) + { + failure.printStackTrace(); + failures.add("Result failed " + result); + } + } + else + { + if (checkContentLength && contentLength.get() != 0) + failures.add("Content length mismatch " + contentLength); } - - if (checkContentLength && contentLength.get() != 0) - failures.add("Content length mismatch " + contentLength); requestLatch.countDown(); latch.countDown(); @@ -288,8 +303,14 @@ public class HttpClientLoadTest extends AbstractTest Date: Thu, 18 Oct 2018 16:55:33 +0200 Subject: [PATCH 04/12] Issue #2796 - Max local stream count exceeded when request fails. Updates after review. Signed-off-by: Simone Bordet --- .../main/java/org/eclipse/jetty/http2/HTTP2Session.java | 1 + .../jetty/http2/client/http/HttpChannelOverHTTP2.java | 3 ++- .../jetty/http2/client/http/HttpConnectionOverHTTP2.java | 9 +++++---- .../jetty/http2/client/http/HttpReceiverOverHTTP2.java | 3 ++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 0b3fff14705..cbd379b3cc6 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -763,6 +763,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio int localCount = localStreamCount.get(); int maxCount = getMaxLocalStreams(); if (maxCount >= 0 && localCount >= maxCount) + // TODO: remove the dump() in the exception message. throw new IllegalStateException("Max local stream count " + maxCount + " exceeded" + System.lineSeparator() + dump()); if (localStreamCount.compareAndSet(localCount, localCount + 1)) break; diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java index 4a399926c4c..27c6022b642 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java @@ -25,6 +25,7 @@ import org.eclipse.jetty.client.HttpReceiver; import org.eclipse.jetty.client.HttpSender; import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.IStream; import org.eclipse.jetty.http2.api.Session; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.frames.ResetFrame; @@ -101,7 +102,7 @@ public class HttpChannelOverHTTP2 extends HttpChannel connection.release(this); } - void onStreamClosed(Stream stream) + void onStreamClosed(IStream stream) { connection.onStreamClosed(stream, this); } diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java index e805fe5f938..7863d3381ca 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java @@ -35,8 +35,8 @@ import org.eclipse.jetty.client.HttpRequest; import org.eclipse.jetty.client.SendFailure; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.IStream; import org.eclipse.jetty.http2.api.Session; -import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -94,7 +94,6 @@ public class HttpConnectionOverHTTP2 extends HttpConnection implements Sweeper.S { if (LOG.isDebugEnabled()) LOG.debug("Released {}", channel); - // Only non-push channels are released. if (activeChannels.remove(channel)) { // Recycle only non-failed channels. @@ -109,12 +108,14 @@ public class HttpConnectionOverHTTP2 extends HttpConnection implements Sweeper.S } } - void onStreamClosed(Stream stream, HttpChannelOverHTTP2 channel) + void onStreamClosed(IStream stream, HttpChannelOverHTTP2 channel) { if (LOG.isDebugEnabled()) LOG.debug("{} closed for {}", stream, channel); channel.setStream(null); - getHttpDestination().release(this); + // Only non-push channels are released. + if (stream.isLocal()) + getHttpDestination().release(this); } @Override diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java index dc4007ea366..5f1a6751239 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.ErrorCode; +import org.eclipse.jetty.http2.IStream; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.frames.DataFrame; import org.eclipse.jetty.http2.frames.HeadersFrame; @@ -187,7 +188,7 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements Stream.Listen @Override public void onClosed(Stream stream) { - getHttpChannel().onStreamClosed(stream); + getHttpChannel().onStreamClosed((IStream)stream); } private void notifyContent(HttpExchange exchange, DataFrame frame, Callback callback) From 5d837309c322d54fc30291fe6ea13b56beab8e6f Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 30 Oct 2018 17:42:11 +0100 Subject: [PATCH 05/12] Issue #2941 - JDK 11: UnsupportedOperationException at AnnotationParser.scanClass. Downgraded OSGi ASM version to 6.2. Waiting for the SPI Fly library to support JDK 11. Signed-off-by: Simone Bordet --- jetty-osgi/pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/jetty-osgi/pom.xml b/jetty-osgi/pom.xml index 090cc250a42..503b4cba7e3 100644 --- a/jetty-osgi/pom.xml +++ b/jetty-osgi/pom.xml @@ -15,6 +15,7 @@ pom + 6.2 3.6.0.v20100517 3.2.100.v20100503 1.0.0-v20070606 From 31cab3dc0884e1cc26ce514aa9cdf92ae40856d8 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 30 Oct 2018 19:36:02 +0100 Subject: [PATCH 06/12] Issue #2796 - Max local stream count exceeded when request fails. Restored smaller maxContentLength to avoid that the test takes too much time and fails. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/http/client/HttpClientLoadTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java index 65b72262eac..814d9bd011b 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.http.client; import java.io.IOException; +import java.io.InterruptedIOException; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; @@ -199,7 +200,7 @@ public class HttpClientLoadTest extends AbstractTest Date: Thu, 1 Nov 2018 11:43:11 +0100 Subject: [PATCH 07/12] Allows commas to separate cookies in RFC2965 compliance mode (#3045) * Allows commas to separate cookies in RFC2965 compliance mode * cleanup after review Signed-off-by: Greg Wilkins * revert accidental change Signed-off-by: Greg Wilkins --- .../eclipse/jetty/server/CookieCutter.java | 242 ++++++++---------- .../org/eclipse/jetty/server/Request.java | 13 +- .../jetty/server/CookieCutterTest.java | 13 +- .../server/CookieCutter_LenientTest.java | 7 +- 4 files changed, 125 insertions(+), 150 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java index 2a186b18026..d9e7971df05 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java @@ -129,7 +129,6 @@ public class CookieCutter { // Parse the header String name = null; - String value = null; Cookie cookie = null; @@ -139,11 +138,11 @@ public class CookieCutter boolean escaped=false; int tokenstart=-1; int tokenend=-1; - for (int i = 0, length = hdr.length(), last=length-1; i < length; i++) + for (int i = 0, length = hdr.length(); i <= length; i++) { - char c = hdr.charAt(i); + char c = i==length?0:hdr.charAt(i); - // System.err.printf("i=%d c=%s v=%b q=%b e=%b u=%s s=%d e=%d%n" ,i,""+c,invalue,inQuoted,escaped,unquoted,tokenstart,tokenend); + // System.err.printf("i=%d/%d c=%s v=%b q=%b/%b e=%b u=%s s=%d e=%d \t%s=%s%n" ,i,length,c==0?"|":(""+c),invalue,inQuoted,quoted,escaped,unquoted,tokenstart,tokenend,name,value); // Handle quoted values for name or value if (inQuoted) @@ -151,52 +150,39 @@ public class CookieCutter if (escaped) { escaped=false; - unquoted.append(c); + if (c>0) + unquoted.append(c); + else + { + unquoted.setLength(0); + inQuoted = false; + i--; + } continue; } switch (c) { case '"': - inQuoted=false; - if (i==last) - { - value = unquoted.toString(); - unquoted.setLength(0); - } - else - { - quoted=true; - tokenstart=i; - tokenend=-1; - } + inQuoted = false; + quoted = true; + tokenstart = i; + tokenend = -1; break; - + case '\\': - if (i==last) - { - unquoted.setLength(0); - inQuoted = false; - i--; - } - else - { - escaped=true; - } + escaped = true; + continue; + + case 0: + // unterminated quote, let's ignore quotes + unquoted.setLength(0); + inQuoted = false; + i--; continue; default: - if (i==last) - { - // unterminated quote, let's ignore quotes - unquoted.setLength(0); - inQuoted = false; - i--; - } - else - { - unquoted.append(c); - } + unquoted.append(c); continue; } } @@ -211,22 +197,88 @@ public class CookieCutter case ' ': case '\t': break; - + + case ',': + if (_compliance!=CookieCompliance.RFC2965) + { + if (quoted) + { + // must have been a bad internal quote. let's fix as best we can + unquoted.append(hdr,tokenstart,i--); + inQuoted = true; + quoted = false; + continue; + } + if (tokenstart<0) + tokenstart = i; + tokenend=i; + continue; + } + // fall through + case 0: case ';': + { + String value; + if (quoted) { value = unquoted.toString(); unquoted.setLength(0); quoted = false; } - else if(tokenstart>=0 && tokenend>=0) - value = hdr.substring(tokenstart, tokenend+1); + else if(tokenstart>=0) + value = tokenend>=tokenstart?hdr.substring(tokenstart, tokenend+1):hdr.substring(tokenstart); else value = ""; - + + try + { + if (name.startsWith("$")) + { + if (_compliance==CookieCompliance.RFC2965) + { + String lowercaseName = name.toLowerCase(Locale.ENGLISH); + switch(lowercaseName) + { + case "$path": + if (cookie!=null) + cookie.setPath(value); + break; + case "$domain": + if (cookie!=null) + cookie.setDomain(value); + break; + case "$port": + if (cookie!=null) + cookie.setComment("$port="+value); + break; + case "$version": + version = Integer.parseInt(value); + break; + default: + break; + } + } + } + else + { + cookie = new Cookie(name, value); + if (version > 0) + cookie.setVersion(version); + cookies.add(cookie); + } + } + catch (Exception e) + { + LOG.debug(e); + } + + name = null; tokenstart = -1; invalue=false; + break; + } case '"': if (tokenstart<0) @@ -243,20 +295,14 @@ public class CookieCutter if (quoted) { // must have been a bad internal quote. let's fix as best we can - unquoted.append(hdr.substring(tokenstart,i)); + unquoted.append(hdr,tokenstart,i--); inQuoted = true; quoted = false; - i--; continue; } if (tokenstart<0) - tokenstart=i; + tokenstart = i; tokenend=i; - if (i==last) - { - value = hdr.substring(tokenstart, tokenend+1); - break; - } continue; } } @@ -269,21 +315,6 @@ public class CookieCutter case '\t': continue; - case ';': - if (quoted) - { - name = unquoted.toString(); - unquoted.setLength(0); - quoted = false; - } - else if(tokenstart>=0 && tokenend>=0) - { - name = hdr.substring(tokenstart, tokenend+1); - } - - tokenstart = -1; - break; - case '=': if (quoted) { @@ -291,98 +322,29 @@ public class CookieCutter unquoted.setLength(0); quoted = false; } - else if(tokenstart>=0 && tokenend>=0) - { - name = hdr.substring(tokenstart, tokenend+1); - } + else if(tokenstart>=0) + name = tokenend>=tokenstart?hdr.substring(tokenstart, tokenend+1):hdr.substring(tokenstart); + tokenstart = -1; - invalue=true; + invalue = true; break; default: if (quoted) { // must have been a bad internal quote. let's fix as best we can - unquoted.append(hdr.substring(tokenstart,i)); + unquoted.append(hdr,tokenstart,i--); inQuoted = true; quoted = false; - i--; continue; } if (tokenstart<0) tokenstart=i; tokenend=i; - if (i==last) - break; continue; } } } - - if (invalue && i==last && value==null) - { - if (quoted) - { - value = unquoted.toString(); - unquoted.setLength(0); - quoted = false; - } - else if(tokenstart>=0 && tokenend>=0) - { - value = hdr.substring(tokenstart, tokenend+1); - } - else - value = ""; - } - - // If after processing the current character we have a value and a name, then it is a cookie - if (name!=null && value!=null) - { - try - { - if (name.startsWith("$")) - { - String lowercaseName = name.toLowerCase(Locale.ENGLISH); - if (_compliance==CookieCompliance.RFC6265) - { - // Ignore - } - else if ("$path".equals(lowercaseName)) - { - if (cookie!=null) - cookie.setPath(value); - } - else if ("$domain".equals(lowercaseName)) - { - if (cookie!=null) - cookie.setDomain(value); - } - else if ("$port".equals(lowercaseName)) - { - if (cookie!=null) - cookie.setComment("$port="+value); - } - else if ("$version".equals(lowercaseName)) - { - version = Integer.parseInt(value); - } - } - else - { - cookie = new Cookie(name, value); - if (version > 0) - cookie.setVersion(version); - cookies.add(cookie); - } - } - catch (Exception e) - { - LOG.debug(e); - } - - name = null; - value = null; - } } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index ac5198063a5..b13d1cc0c16 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -764,12 +764,15 @@ public class Request implements HttpServletRequest } _cookiesExtracted = true; - - for (String c : metadata.getFields().getValuesList(HttpHeader.COOKIE)) + + for (HttpField field : metadata.getFields()) { - if (_cookies == null) - _cookies = new CookieCutter(getHttpChannel().getHttpConfiguration().getCookieCompliance()); - _cookies.addCookieField(c); + if (field.getHeader()==HttpHeader.COOKIE) + { + if (_cookies==null) + _cookies = new CookieCutter(getHttpChannel().getHttpConfiguration().getCookieCompliance()); + _cookies.addCookieField(field.getValue()); + } } //Javadoc for Request.getCookies() stipulates null for no cookies diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterTest.java index 69962f57967..8419d9462f5 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterTest.java @@ -139,17 +139,22 @@ public class CookieCutterTest * Example from RFC2965 */ @Test - @Disabled("comma separation no longer supported by new RFC6265") public void testRFC2965_CookieSpoofingExample() { String rawCookie = "$Version=\"1\"; session_id=\"1234\", " + "$Version=\"1\"; session_id=\"1111\"; $Domain=\".cracker.edu\""; - - Cookie cookies[] = parseCookieHeaders(CookieCompliance.RFC6265,rawCookie); - + + + Cookie cookies[] = parseCookieHeaders(CookieCompliance.RFC2965,rawCookie); + assertThat("Cookies.length", cookies.length, is(2)); assertCookie("Cookies[0]", cookies[0], "session_id", "1234", 1, null); assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 1, null); + + cookies = parseCookieHeaders(CookieCompliance.RFC6265,rawCookie); + assertThat("Cookies.length", cookies.length, is(2)); + assertCookie("Cookies[0]", cookies[0], "session_id", "1234\", $Version=\"1", 0, null); + assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 0, null); } /** diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java index e22f3961a06..906a0a22b06 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutter_LenientTest.java @@ -144,7 +144,12 @@ public class CookieCutter_LenientTest Arguments.of("GAPS=1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-", "GAPS", "1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-"), // Strong abuse of cookie spec (lots of tspecials) - Arguments.of("$Version=0; rToken=F_TOKEN''!--\"=&{()}", "rToken", "F_TOKEN''!--\"=&{()}") + Arguments.of("$Version=0; rToken=F_TOKEN''!--\"=&{()}", "rToken", "F_TOKEN''!--\"=&{()}"), + + // Commas that were not commas + Arguments.of("name=foo,bar","name","foo,bar"), + Arguments.of("name=foo , bar","name","foo , bar"), + Arguments.of("name=foo , bar, bob","name","foo , bar, bob") ); } From 161f1698cfe0869e855c8da290f8422232142e0a Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 1 Nov 2018 17:06:04 +0100 Subject: [PATCH 08/12] Jetty 9.4.x 3018 request logging bad messages (#3020) Issue #3018 improve logging and handling of slow data rates. * Slow data rates now result in aborted channels, but exception is still thrown. Test for 408 in requestLog * Updated many RequestLog usages to use Server.setRequestLog rather than a RequestLogHandler * Fixed javadoc * removed BadRequestLogHandlerTest (tested in RequestLogTest) * added JMH to show the future of request logging for #113 * copyright header. * Updates from review * Revert to throwing BadMessageException * BME ensures a 408 is logged rather than a 500 Signed-off-by: Greg Wilkins --- .../eclipse/jetty/embedded/LikeJettyXml.java | 5 +- .../eclipse/jetty/embedded/ManyHandlers.java | 20 +- .../eclipse/jetty/ant/ServerProxyImpl.java | 7 +- .../requestlog/jmh/RequestLogBenchmark.java | 222 +++++ .../jetty/maven/plugin/ServerSupport.java | 6 +- .../java/org/eclipse/jetty/runner/Runner.java | 12 +- .../org/eclipse/jetty/server/HttpChannel.java | 15 +- .../org/eclipse/jetty/server/HttpInput.java | 7 +- .../org/eclipse/jetty/server/HttpOutput.java | 7 +- .../server/handler/RequestLogHandler.java | 10 +- .../handler/BadRequestLogHandlerTest.java | 194 ---- .../server/handler/RequestLogHandlerTest.java | 886 ------------------ .../jetty/server/handler/RequestLogTest.java | 509 +++++++++- .../jetty/servlet/AsyncServletTest.java | 8 +- .../jetty/servlet/ServletRequestLogTest.java | 25 +- .../jetty/http/client/ServerTimeoutsTest.java | 26 +- .../jetty/http/client/TransportScenario.java | 13 + .../java/org/eclipse/jetty/TestServer.java | 6 +- 18 files changed, 770 insertions(+), 1208 deletions(-) create mode 100644 jetty-jmh/src/main/java/org/eclipse/jetty/requestlog/jmh/RequestLogBenchmark.java delete mode 100644 jetty-server/src/test/java/org/eclipse/jetty/server/handler/BadRequestLogHandlerTest.java delete mode 100644 jetty-server/src/test/java/org/eclipse/jetty/server/handler/RequestLogHandlerTest.java diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java index 356a9181978..de276e02d68 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/LikeJettyXml.java @@ -44,7 +44,6 @@ import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.DefaultHandler; import org.eclipse.jetty.server.handler.HandlerCollection; -import org.eclipse.jetty.server.handler.RequestLogHandler; import org.eclipse.jetty.server.handler.StatisticsHandler; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; @@ -214,9 +213,7 @@ public class LikeJettyXml requestLog.setExtended(true); requestLog.setLogCookies(false); requestLog.setLogTimeZone("GMT"); - RequestLogHandler requestLogHandler = new RequestLogHandler(); - requestLogHandler.setRequestLog(requestLog); - handlers.addHandler(requestLogHandler); + server.setRequestLog(requestLog); // === jetty-lowresources.xml === diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyHandlers.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyHandlers.java index 337f760af86..244ff29cf68 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyHandlers.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyHandlers.java @@ -35,7 +35,6 @@ import org.eclipse.jetty.server.handler.DefaultHandler; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.server.handler.HandlerList; import org.eclipse.jetty.server.handler.HandlerWrapper; -import org.eclipse.jetty.server.handler.RequestLogHandler; import org.eclipse.jetty.server.handler.gzip.GzipHandler; import org.eclipse.jetty.util.ajax.JSON; @@ -115,13 +114,12 @@ public class ManyHandlers HandlerWrapper wrapper = new WelcomeWrapHandler(); Handler hello = new HelloHandler(); Handler dft = new DefaultHandler(); - RequestLogHandler requestLog = new RequestLogHandler(); // configure request logging File requestLogFile = File.createTempFile("demo", "log"); NCSARequestLog ncsaLog = new NCSARequestLog( requestLogFile.getAbsolutePath()); - requestLog.setRequestLog(ncsaLog); + server.setRequestLog(ncsaLog); // create the handler collections HandlerCollection handlers = new HandlerCollection(); @@ -129,20 +127,8 @@ public class ManyHandlers // link them all together wrapper.setHandler(hello); - list.setHandlers(new Handler[] { param, new GzipHandler(), dft }); - handlers.setHandlers(new Handler[] { list, requestLog }); - - // Handler tree looks like the following - // 
-        // Server
-        // + HandlerCollection
-        // . + HandlerList
-        // . | + param (ParamHandler)
-        // . | + wrapper (WelcomeWrapHandler)
-        // . | | \ hello (HelloHandler)
-        // . | \ dft (DefaultHandler)
-        // . \ requestLog (RequestLogHandler)
-        //
+ list.setHandlers(new Handler[] { param, new GzipHandler() }); + handlers.setHandlers(new Handler[] { list, dft }); server.setHandler(handlers); diff --git a/jetty-ant/src/main/java/org/eclipse/jetty/ant/ServerProxyImpl.java b/jetty-ant/src/main/java/org/eclipse/jetty/ant/ServerProxyImpl.java index 2dd2fdb9ee0..ac62bfc280a 100644 --- a/jetty-ant/src/main/java/org/eclipse/jetty/ant/ServerProxyImpl.java +++ b/jetty-ant/src/main/java/org/eclipse/jetty/ant/ServerProxyImpl.java @@ -40,7 +40,6 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.DefaultHandler; import org.eclipse.jetty.server.handler.HandlerCollection; -import org.eclipse.jetty.server.handler.RequestLogHandler; import org.eclipse.jetty.util.Scanner; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.xml.XmlConfiguration; @@ -407,9 +406,8 @@ public class ServerProxyImpl implements ServerProxy */ private void configureHandlers() { - RequestLogHandler requestLogHandler = new RequestLogHandler(); if (requestLog != null) - requestLogHandler.setRequestLog(requestLog); + server.setRequestLog(requestLog); contexts = (ContextHandlerCollection) server .getChildHandlerByClass(ContextHandlerCollection.class); @@ -422,8 +420,7 @@ public class ServerProxyImpl implements ServerProxy { handlers = new HandlerCollection(); server.setHandler(handlers); - handlers.setHandlers(new Handler[] { contexts, new DefaultHandler(), - requestLogHandler }); + handlers.setHandlers(new Handler[] { contexts, new DefaultHandler() }); } else { diff --git a/jetty-jmh/src/main/java/org/eclipse/jetty/requestlog/jmh/RequestLogBenchmark.java b/jetty-jmh/src/main/java/org/eclipse/jetty/requestlog/jmh/RequestLogBenchmark.java new file mode 100644 index 00000000000..9092a5fca6f --- /dev/null +++ b/jetty-jmh/src/main/java/org/eclipse/jetty/requestlog/jmh/RequestLogBenchmark.java @@ -0,0 +1,222 @@ +// +// ======================================================================== +// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.requestlog.jmh; + +import static java.lang.invoke.MethodHandles.dropArguments; +import static java.lang.invoke.MethodHandles.foldArguments; +import static java.lang.invoke.MethodType.methodType; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.util.TypeUtil; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.profile.GCProfiler; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + + + +@State(Scope.Benchmark) +@Threads(4) +@Warmup(iterations = 7, time = 500, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 7, time = 500, timeUnit = TimeUnit.MILLISECONDS) +public class RequestLogBenchmark +{ + + public static void append(String s, StringBuilder b) + { + b.append(s); + } + + public static void logURI(StringBuilder b, String request) + { + b.append(request); + } + + public static void logLength(StringBuilder b, String request) + { + b.append(request.length()); + } + + public static void logAddr(StringBuilder b, String request) + { + try + { + TypeUtil.toHex(request.hashCode(), b); + } + catch(Exception e) + { + throw new RuntimeException(e); + } + } + + private ThreadLocal buffers = new ThreadLocal() + { + @Override + protected StringBuilder initialValue() + { + return new StringBuilder(256); + } + }; + MethodHandle logHandle; + Object[] iteratedLog; + + public RequestLogBenchmark() + { + try + { + MethodType logType = methodType(Void.TYPE, StringBuilder.class, String.class); + + MethodHandle append = MethodHandles.lookup() + .findStatic(RequestLogBenchmark.class, "append", methodType(Void.TYPE, String.class, StringBuilder.class)); + MethodHandle logURI = MethodHandles.lookup().findStatic(RequestLogBenchmark.class, "logURI", logType); + MethodHandle logAddr = MethodHandles.lookup().findStatic(RequestLogBenchmark.class, "logAddr", logType); + MethodHandle logLength = MethodHandles.lookup().findStatic(RequestLogBenchmark.class, "logLength", logType); + + // setup iteration + iteratedLog = new Object[] + { + logURI, + " - ", + logAddr, + " ", + logLength, + "\n" + }; + + // setup methodHandle + logHandle = dropArguments(append.bindTo("\n"), 1, String.class); + logHandle = foldArguments(logHandle, logLength); + logHandle = foldArguments(logHandle, dropArguments(append.bindTo(" "), 1, String.class)); + logHandle = foldArguments(logHandle, logAddr); + logHandle = foldArguments(logHandle, dropArguments(append.bindTo(" - "), 1, String.class)); + logHandle = foldArguments(logHandle, logURI); + + } + catch (Throwable th) + { + throw new RuntimeException(th); + } + } + + + public String logFixed(String request) + { + StringBuilder b = buffers.get(); + logURI(b,request); + append(" - ",b); + logAddr(b,request); + append(" ",b); + logLength(b,request); + append("\n",b); + String l = b.toString(); + b.setLength(0); + return l; + } + + public String logIterate(String request) + { + try + { + + StringBuilder b = buffers.get(); + for (Object o : iteratedLog) + { + if (o instanceof String) + append((String)o, b); + else if (o instanceof MethodHandle) + ((MethodHandle)o).invoke(b, request); + } + String l = b.toString(); + b.setLength(0); + return l; + } + catch(Throwable th) + { + throw new RuntimeException(th); + } + } + + public String logMethodHandle(String request) + { + try + { + StringBuilder b = buffers.get(); + logHandle.invoke(buffers.get(), request); + String l = b.toString(); + b.setLength(0); + return l; + } + catch (Throwable th) + { + throw new RuntimeException(th); + } + } + + + @Benchmark + @BenchmarkMode({ Mode.Throughput}) + public String testFixed() + { + return logFixed(Long.toString(ThreadLocalRandom.current().nextLong())); + }; + + @Benchmark + @BenchmarkMode({ Mode.Throughput}) + public String testIterate() + { + return logIterate(Long.toString(ThreadLocalRandom.current().nextLong())); + }; + + @Benchmark + @BenchmarkMode({ Mode.Throughput}) + public String testHandle() + { + return logMethodHandle(Long.toString(ThreadLocalRandom.current().nextLong())); + }; + + + public static void main(String[] args) throws RunnerException + { + Options opt = new OptionsBuilder() + .include(RequestLogBenchmark.class.getSimpleName()) + .warmupIterations(20) + .measurementIterations(10) + .addProfiler(GCProfiler.class) + .forks(1) + .threads(100) + .build(); + + new Runner(opt).run(); + } + +} diff --git a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/ServerSupport.java b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/ServerSupport.java index 36eab2e0418..00c8e6c9c4c 100644 --- a/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/ServerSupport.java +++ b/jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/ServerSupport.java @@ -33,7 +33,6 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.DefaultHandler; import org.eclipse.jetty.server.handler.HandlerCollection; -import org.eclipse.jetty.server.handler.RequestLogHandler; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.webapp.Configuration; import org.eclipse.jetty.webapp.WebAppContext; @@ -69,9 +68,8 @@ public class ServerSupport throw new IllegalArgumentException ("Server is null"); DefaultHandler defaultHandler = new DefaultHandler(); - RequestLogHandler requestLogHandler = new RequestLogHandler(); if (requestLog != null) - requestLogHandler.setRequestLog(requestLog); + server.setRequestLog(requestLog); ContextHandlerCollection contexts = findContextHandlerCollection(server); if (contexts == null) @@ -82,7 +80,7 @@ public class ServerSupport { handlers = new HandlerCollection(); server.setHandler(handlers); - handlers.setHandlers(new Handler[]{contexts, defaultHandler, requestLogHandler}); + handlers.setHandlers(new Handler[]{contexts, defaultHandler}); } else { diff --git a/jetty-runner/src/main/java/org/eclipse/jetty/runner/Runner.java b/jetty-runner/src/main/java/org/eclipse/jetty/runner/Runner.java index 12fe3a27ca6..9348f82858f 100644 --- a/jetty-runner/src/main/java/org/eclipse/jetty/runner/Runner.java +++ b/jetty-runner/src/main/java/org/eclipse/jetty/runner/Runner.java @@ -43,7 +43,6 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.DefaultHandler; import org.eclipse.jetty.server.handler.HandlerCollection; -import org.eclipse.jetty.server.handler.RequestLogHandler; import org.eclipse.jetty.server.handler.StatisticsHandler; import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.servlet.ServletContextHandler; @@ -88,7 +87,6 @@ public class Runner protected URLClassLoader _classLoader; protected Classpath _classpath = new Classpath(); protected ContextHandlerCollection _contexts; - protected RequestLogHandler _logHandler; protected String _logFile; protected ArrayList _configFiles; protected boolean _enableStats=false; @@ -392,14 +390,6 @@ public class Runner handlers.addHandler(new DefaultHandler()); } - //ensure a log handler is present - _logHandler = (RequestLogHandler) handlers.getChildHandlerByClass(RequestLogHandler.class); - if (_logHandler == null) - { - _logHandler = new RequestLogHandler(); - handlers.addHandler(_logHandler); - } - //check a connector is configured to listen on Connector[] connectors = _server.getConnectors(); @@ -509,7 +499,7 @@ public class Runner { NCSARequestLog requestLog = new NCSARequestLog(_logFile); requestLog.setExtended(false); - _logHandler.setRequestLog(requestLog); + _server.setRequestLog(requestLog); } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index 43ab02dc183..95012aee8b5 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -888,7 +888,6 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor @Override public void write(ByteBuffer content, boolean complete, Callback callback) { - _written+=BufferUtil.length(content); sendResponse(null,content,complete,callback); } @@ -1226,18 +1225,21 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor private class CommitCallback extends Callback.Nested { private final ByteBuffer _content; + private final int _length; private final boolean _complete; private CommitCallback(Callback callback, ByteBuffer content, boolean complete) { super(callback); - this._content = content == null ? BufferUtil.EMPTY_BUFFER : content.slice(); - this._complete = complete; + _content = content == null ? BufferUtil.EMPTY_BUFFER : content.slice(); + _length = _content.remaining(); + _complete = complete; } @Override public void succeeded() { + _written += _length; super.succeeded(); notifyResponseCommit(_request); if (_content.hasRemaining()) @@ -1299,18 +1301,21 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor private class ContentCallback extends Callback.Nested { private final ByteBuffer _content; + private final int _length; private final boolean _complete; private ContentCallback(Callback callback, ByteBuffer content, boolean complete) { super(callback); - this._content = content == null ? BufferUtil.EMPTY_BUFFER : content.slice(); - this._complete = complete; + _content = content == null ? BufferUtil.EMPTY_BUFFER : content.slice(); + _length = _content.remaining(); + _complete = complete; } @Override public void succeeded() { + _written += _length; super.succeeded(); if (_content.hasRemaining()) notifyResponseContent(_request, _content); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java index 28f6d2622e1..d4592a72607 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java @@ -286,7 +286,12 @@ public class HttpInput extends ServletInputStream implements Runnable { long minimum_data = minRequestDataRate * TimeUnit.NANOSECONDS.toMillis(period) / TimeUnit.SECONDS.toMillis(1); if (_contentArrived < minimum_data) - throw new BadMessageException(HttpStatus.REQUEST_TIMEOUT_408,String.format("Request content data rate < %d B/s",minRequestDataRate)); + { + BadMessageException bad = new BadMessageException(HttpStatus.REQUEST_TIMEOUT_408, + String.format("Request content data rate < %d B/s", minRequestDataRate)); + _channelState.getHttpChannel().abort(bad); + throw bad; + } } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 6fcd6370435..4402d193a93 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -25,6 +25,7 @@ import java.nio.channels.ReadableByteChannel; import java.nio.channels.WritePendingException; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; + import javax.servlet.RequestDispatcher; import javax.servlet.ServletOutputStream; import javax.servlet.ServletRequest; @@ -934,7 +935,11 @@ public class HttpOutput extends ServletOutputStream implements Runnable if (LOG.isDebugEnabled()) LOG.debug("Flushed bytes min/actual {}/{}", minFlushed, _flushed); if (_flushed < minFlushed) - throw new IOException(String.format("Response content data rate < %d B/s", minDataRate)); + { + IOException ioe = new IOException(String.format("Response content data rate < %d B/s", minDataRate)); + _channel.abort(ioe); + throw ioe; + } } public void recycle() diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/RequestLogHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/RequestLogHandler.java index 838d3323422..e30764a2e92 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/RequestLogHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/RequestLogHandler.java @@ -31,11 +31,11 @@ import org.eclipse.jetty.server.Server; /** - * RequestLogHandler. - * This handler can be used to wrap an individual context for context logging. - * To set a {@link RequestLog} instance for the entire {@link Server}, use - * {@link Server#setRequestLog(RequestLog)} instead of this handler. - * + *

This handler provides an alternate way (other than {@link Server#setRequestLog(RequestLog)}) + * to log request, that can be applied to a particular handler (eg context). + * This handler can be used to wrap an individual context for context logging, or can be listed + * prior to a handler. + *

* @see Server#setRequestLog(RequestLog) */ public class RequestLogHandler extends HandlerWrapper diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BadRequestLogHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BadRequestLogHandlerTest.java deleted file mode 100644 index 1c54de4ca0a..00000000000 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/BadRequestLogHandlerTest.java +++ /dev/null @@ -1,194 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.server.handler; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; - -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.OutputStream; -import java.io.OutputStreamWriter; -import java.io.StringReader; -import java.io.StringWriter; -import java.net.InetAddress; -import java.net.InetSocketAddress; -import java.net.Socket; -import java.net.SocketAddress; -import java.nio.charset.StandardCharsets; -import java.time.Duration; -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Stream; - -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.eclipse.jetty.server.Connector; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.RequestLog; -import org.eclipse.jetty.server.Response; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.ServerConnector; -import org.eclipse.jetty.util.IO; -import org.eclipse.jetty.util.component.AbstractLifeCycle; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; -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; - -/** - * Testing oddball request scenarios (like error 400) where the error should - * be logged - */ -@Tag("Unstable") -@Disabled -public class BadRequestLogHandlerTest -{ - private static final Logger LOG = Log.getLogger(BadRequestLogHandlerTest.class); - - public static class CaptureLog extends AbstractLifeCycle implements RequestLog - { - public List captured = new ArrayList<>(); - - @Override - public void log(Request request, Response response) - { - int status = response.getCommittedMetaData().getStatus(); - captured.add(String.format("%s %s %s %03d",request.getMethod(),request.getHttpURI(),request.getProtocol(),status)); - } - } - - private static class HelloHandler extends AbstractHandler - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - response.setContentType("text/plain"); - response.getWriter().print("Hello World"); - baseRequest.setHandled(true); - } - } - - public static Stream data() - { - List data = new ArrayList<>(); - - data.add(new String[]{ "GET /bad VER\r\n" - + "Host: localhost\r\n" - + "Connection: close\r\n\r\n" , - "GET HTTP/1.1 400" }); - data.add(new String[]{ "GET /%%adsasd HTTP/1.1\r\n" - + "Host: localhost\r\n" - + "Connection: close\r\n\r\n" , - "GET HTTP/1.1 400" }); - - return data.stream().map(Arguments::of); - } - - @ParameterizedTest - @MethodSource("data") - public void testLogHandler(String requestHeader, String expectedLog) throws Exception - { - Server server = new Server(); - ServerConnector connector = new ServerConnector(server); - connector.setPort(0); - server.setConnectors(new Connector[] { connector }); - - CaptureLog captureLog = new CaptureLog(); - - RequestLogHandler requestLog = new RequestLogHandler(); - requestLog.setRequestLog(captureLog); - - requestLog.setHandler(new HelloHandler()); - - server.setHandler(requestLog); - - try - { - server.start(); - - String host = connector.getHost(); - if (host == null) - { - host = "localhost"; - } - - InetAddress destAddr = InetAddress.getByName(host); - int port = connector.getLocalPort(); - SocketAddress endpoint = new InetSocketAddress(destAddr,port); - - Socket socket = new Socket(); - socket.setSoTimeout(1000); - socket.connect(endpoint); - - assertTimeoutPreemptively(Duration.ofSeconds(4), ()-> { - try (OutputStream out = socket.getOutputStream(); - OutputStreamWriter writer = new OutputStreamWriter(out, StandardCharsets.UTF_8); - InputStream in = socket.getInputStream(); - InputStreamReader reader = new InputStreamReader(in, StandardCharsets.UTF_8)) - { - StringReader request = new StringReader(requestHeader); - IO.copy(request, writer); - writer.flush(); - StringWriter response = new StringWriter(); - IO.copy(reader, response); - LOG.info("Response: {}", response); - } - finally - { - socket.close(); - } - }); - - assertRequestLog(expectedLog, captureLog); - } - finally - { - server.stop(); - } - } - - private void assertRequestLog(final String expectedLog, CaptureLog captureLog) - { - int captureCount = captureLog.captured.size(); - - if (captureCount != 1) - { - LOG.warn("Capture Log size is {}, expected to be 1",captureCount); - if (captureCount > 1) - { - for (int i = 0; i < captureCount; i++) - { - LOG.warn("[{}] {}",i,captureLog.captured.get(i)); - } - } - assertThat("Capture Log Entry Count",captureLog.captured.size(),is(1)); - } - - String actual = captureLog.captured.get(0); - assertThat("Capture Log",actual,is(expectedLog)); - } -} diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/RequestLogHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/RequestLogHandlerTest.java deleted file mode 100644 index a57c4186c85..00000000000 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/RequestLogHandlerTest.java +++ /dev/null @@ -1,886 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.server.handler; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; - -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.net.HttpURLConnection; -import java.net.URI; -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Stream; - -import javax.servlet.AsyncContext; -import javax.servlet.AsyncEvent; -import javax.servlet.AsyncListener; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.eclipse.jetty.server.Connector; -import org.eclipse.jetty.server.Handler; -import org.eclipse.jetty.server.HttpChannel; -import org.eclipse.jetty.server.HttpChannelState; -import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.RequestLog; -import org.eclipse.jetty.server.Response; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.ServerConnector; -import org.eclipse.jetty.toolchain.test.IO; -import org.eclipse.jetty.util.component.AbstractLifeCycle; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.log.StacklessLogging; -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; - -/** - * Tests for RequestLogHandler behavior. - *

- * Tests different request handler behavior against different server+error configurations - */ -@Tag("Unstable") -@Disabled -public class RequestLogHandlerTest -{ - private static final Logger LOG = Log.getLogger(RequestLogHandlerTest.class); - - public static class CaptureLog extends AbstractLifeCycle implements RequestLog - { - public List captured = new ArrayList<>(); - - @Override - public void log(Request request, Response response) - { - int status = response.getCommittedMetaData().getStatus(); - captured.add(String.format("%s %s %s %03d",request.getMethod(),request.getRequestURI(),request.getProtocol(),status)); - } - } - - private static abstract class AbstractTestHandler extends AbstractHandler - { - @Override - public String toString() - { - return this.getClass().getSimpleName(); - } - } - - private static class HelloHandler extends AbstractTestHandler - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - response.setContentType("text/plain"); - response.getWriter().print("Hello World"); - baseRequest.setHandled(true); - } - } - - private static class ResponseSendErrorHandler extends AbstractTestHandler - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - response.sendError(500,"Whoops"); - baseRequest.setHandled(true); - } - } - - private static class ServletExceptionHandler extends AbstractTestHandler - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - throw new ServletException("Whoops"); - } - } - - private static class IOExceptionHandler extends AbstractTestHandler - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - throw new IOException("Whoops"); - } - } - - private static class RuntimeExceptionHandler extends AbstractTestHandler - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - throw new RuntimeException("Whoops"); - } - } - - private static class AsyncOnTimeoutCompleteHandler extends AbstractTestHandler implements AsyncListener - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - AsyncContext ac = request.startAsync(); - ac.setTimeout(1000); - ac.addListener(this); - baseRequest.setHandled(true); - } - - @Override - public void onTimeout(AsyncEvent event) throws IOException - { - event.getAsyncContext().complete(); - } - - @Override - public void onStartAsync(AsyncEvent event) throws IOException - { - } - - @Override - public void onError(AsyncEvent event) throws IOException - { - } - - @Override - public void onComplete(AsyncEvent event) throws IOException - { - } - } - - private static class AsyncOnTimeoutCompleteUnhandledHandler extends AbstractTestHandler implements AsyncListener - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - AsyncContext ac = request.startAsync(); - ac.setTimeout(1000); - ac.addListener(this); - } - - @Override - public void onTimeout(AsyncEvent event) throws IOException - { - event.getAsyncContext().complete(); - } - - @Override - public void onStartAsync(AsyncEvent event) throws IOException - { - } - - @Override - public void onError(AsyncEvent event) throws IOException - { - } - - @Override - public void onComplete(AsyncEvent event) throws IOException - { - } - } - - private static class AsyncOnTimeoutDispatchHandler extends AbstractTestHandler implements AsyncListener - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - if (request.getAttribute("deep") == null) - { - AsyncContext ac = request.startAsync(); - ac.setTimeout(1000); - ac.addListener(this); - baseRequest.setHandled(true); - request.setAttribute("deep",true); - } - } - - @Override - public void onTimeout(AsyncEvent event) throws IOException - { - event.getAsyncContext().dispatch(); - } - - @Override - public void onStartAsync(AsyncEvent event) throws IOException - { - } - - @Override - public void onError(AsyncEvent event) throws IOException - { - } - - @Override - public void onComplete(AsyncEvent event) throws IOException - { - } - } - - private static class AsyncOnStartIOExceptionHandler extends AbstractTestHandler implements AsyncListener - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException - { - AsyncContext ac = request.startAsync(); - ac.setTimeout(1000); - ac.addListener(this); - baseRequest.setHandled(true); - } - - @Override - public void onTimeout(AsyncEvent event) throws IOException - { - } - - @Override - public void onStartAsync(AsyncEvent event) throws IOException - { - event.getAsyncContext().complete(); - throw new IOException("Whoops"); - } - - @Override - public void onError(AsyncEvent event) throws IOException - { - LOG.warn("onError() -> {}",event); - } - - @Override - public void onComplete(AsyncEvent event) throws IOException - { - } - } - - public static class OKErrorHandler extends ErrorHandler - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException - { - if (baseRequest.isHandled() || response.isCommitted()) - { - return; - } - - // collect error details - String reason = (response instanceof Response)?((Response)response).getReason():null; - int status = response.getStatus(); - - // intentionally set response status to OK (this is a test to see what is actually logged) - response.setStatus(200); - response.setContentType("text/plain"); - PrintWriter out = response.getWriter(); - out.printf("Error %d: %s%n",status,reason); - baseRequest.setHandled(true); - } - } - - public static class DispatchErrorHandler extends ErrorHandler - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException - { - if (baseRequest.isHandled() || response.isCommitted()) - { - return; - } - - RequestDispatcher dispatcher = request.getRequestDispatcher("/errorok/"); - assertThat("Dispatcher", dispatcher, notNullValue()); - - try - { - dispatcher.forward(request,response); - } - catch (ServletException e) - { - throw new IOException("Dispatch.forward failed",e); - } - } - } - - public static class AltErrorHandler extends ErrorHandler - { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException - { - if (baseRequest.isHandled() || response.isCommitted()) - { - return; - } - - // collect error details - String reason = (response instanceof Response)?((Response)response).getReason():null; - int status = response.getStatus(); - - // intentionally set response status to OK (this is a test to see what is actually logged) - response.setContentType("text/plain"); - PrintWriter out = response.getWriter(); - out.printf("Error %d: %s%n",status,reason); - baseRequest.setHandled(true); - } - } - - public static Stream data() - { - List data = new ArrayList<>(); - - data.add(new Object[] { new HelloHandler(), "/test/", "GET /test/ HTTP/1.1 200" }); - data.add(new Object[] { new AsyncOnTimeoutCompleteHandler(), "/test/", "GET /test/ HTTP/1.1 200" }); - data.add(new Object[] { new AsyncOnTimeoutCompleteUnhandledHandler(), "/test/", "GET /test/ HTTP/1.1 200" }); - data.add(new Object[] { new AsyncOnTimeoutDispatchHandler(), "/test/", "GET /test/ HTTP/1.1 200" }); - - data.add(new Object[] { new AsyncOnStartIOExceptionHandler(), "/test/", "GET /test/ HTTP/1.1 500" }); - data.add(new Object[] { new ResponseSendErrorHandler(), "/test/", "GET /test/ HTTP/1.1 500" }); - data.add(new Object[] { new ServletExceptionHandler(), "/test/", "GET /test/ HTTP/1.1 500" }); - data.add(new Object[] { new IOExceptionHandler(), "/test/", "GET /test/ HTTP/1.1 500" }); - data.add(new Object[] { new RuntimeExceptionHandler(), "/test/", "GET /test/ HTTP/1.1 500" }); - - return data.stream().map(Arguments::of); - } - - /** - * Test a RequestLogHandler at the end of a HandlerCollection. all other configuration on server at defaults. - * @throws Exception if test failure - */ - @ParameterizedTest - @MethodSource("data") - public void testLogHandlerCollection(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception - { - Server server = new Server(); - ServerConnector connector = new ServerConnector(server); - connector.setPort(0); - server.setConnectors(new Connector[] { connector }); - - CaptureLog captureLog = new CaptureLog(); - - RequestLogHandler requestLog = new RequestLogHandler(); - requestLog.setRequestLog(captureLog); - - HandlerCollection handlers = new HandlerCollection(); - handlers.setHandlers(new Handler[] { testHandler, requestLog }); - server.setHandler(handlers); - - try - { - server.start(); - - String host = connector.getHost(); - if (host == null) - { - host = "localhost"; - } - int port = connector.getLocalPort(); - - URI serverUri = new URI("http",null,host,port,requestPath,null,null); - - // Make call to test handler - HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection(); - try - { - connection.setAllowUserInteraction(false); - - // log response status code - int statusCode = connection.getResponseCode(); - LOG.debug("Response Status Code: {}",statusCode); - - if (statusCode == 200) - { - // collect response message and log it - String content = getResponseContent(connection); - LOG.debug("Response Content: {}",content); - } - } - finally - { - connection.disconnect(); - } - - assertRequestLog(expectedLogEntry, captureLog); - } - finally - { - server.stop(); - } - } - - @ParameterizedTest - @MethodSource("data") - public void testMultipleLogHandlers(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception - { - Server server = new Server(); - ServerConnector connector = new ServerConnector(server); - connector.setPort(0); - server.setConnectors(new Connector[]{connector}); - - List captureLogs = new ArrayList<>(); - List handlerList = new ArrayList<>(); - handlerList.add(testHandler); - - for (int i = 0; i < 4; ++i) { - CaptureLog captureLog = new CaptureLog(); - captureLogs.add(captureLog); - RequestLogHandler requestLog = new RequestLogHandler(); - requestLog.setRequestLog(captureLog); - handlerList.add(requestLog); - } - - HandlerCollection handlers = new HandlerCollection(); - handlers.setHandlers(handlerList.toArray(new Handler[0])); - server.setHandler(handlers); - - try - { - server.start(); - - String host = connector.getHost(); - if (host == null) - { - host = "localhost"; - } - int port = connector.getLocalPort(); - - URI serverUri = new URI("http",null,host,port,requestPath,null,null); - - // Make call to test handler - HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection(); - try - { - connection.setAllowUserInteraction(false); - - // log response status code - int statusCode = connection.getResponseCode(); - LOG.debug("Response Status Code: {}",statusCode); - - if (statusCode == 200) - { - // collect response message and log it - String content = getResponseContent(connection); - LOG.debug("Response Content: {}",content); - } - } - finally - { - connection.disconnect(); - } - - for (CaptureLog captureLog:captureLogs) - assertRequestLog(expectedLogEntry, captureLog); - } - finally - { - server.stop(); - } - } - - /** - * Test a RequestLogHandler at the end of a HandlerCollection and also with the default ErrorHandler as server bean in place. - * @throws Exception if test failure - */ - @ParameterizedTest - @MethodSource("data") - public void testLogHandlerCollection_ErrorHandler_ServerBean(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception - { - Server server = new Server(); - ServerConnector connector = new ServerConnector(server); - connector.setPort(0); - server.setConnectors(new Connector[] { connector }); - - ErrorHandler errorHandler = new ErrorHandler(); - server.addBean(errorHandler); - - CaptureLog captureLog = new CaptureLog(); - - RequestLogHandler requestLog = new RequestLogHandler(); - requestLog.setRequestLog(captureLog); - - HandlerCollection handlers = new HandlerCollection(); - handlers.setHandlers(new Handler[] { testHandler, requestLog }); - server.setHandler(handlers); - - try - { - server.start(); - - String host = connector.getHost(); - if (host == null) - { - host = "localhost"; - } - int port = connector.getLocalPort(); - - URI serverUri = new URI("http",null,host,port,requestPath,null,null); - - // Make call to test handler - HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection(); - try - { - connection.setAllowUserInteraction(false); - - // log response status code - int statusCode = connection.getResponseCode(); - LOG.debug("Response Status Code: {}",statusCode); - - if (statusCode == 200) - { - // collect response message and log it - String content = getResponseContent(connection); - LOG.debug("Response Content: {}",content); - } - } - finally - { - connection.disconnect(); - } - - assertRequestLog(expectedLogEntry, captureLog); - } - finally - { - server.stop(); - } - } - - /** - * Test a RequestLogHandler at the end of a HandlerCollection and also with the ErrorHandler in place. - * @throws Exception if test failure - */ - @ParameterizedTest - @MethodSource("data") - public void testLogHandlerCollection_AltErrorHandler(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception - { - Server server = new Server(); - ServerConnector connector = new ServerConnector(server); - connector.setPort(0); - server.setConnectors(new Connector[] { connector }); - - AltErrorHandler errorDispatcher = new AltErrorHandler(); - server.addBean(errorDispatcher); - - ContextHandlerCollection contexts = new ContextHandlerCollection(); - ContextHandler errorContext = new ContextHandler("/errorpage"); - errorContext.setHandler(new AltErrorHandler()); - ContextHandler testContext = new ContextHandler("/test"); - testContext.setHandler(testHandler); - contexts.addHandler(errorContext); - contexts.addHandler(testContext); - - RequestLogHandler requestLog = new RequestLogHandler(); - CaptureLog captureLog = new CaptureLog(); - requestLog.setRequestLog(captureLog); - - HandlerCollection handlers = new HandlerCollection(); - handlers.setHandlers(new Handler[] { contexts, requestLog }); - server.setHandler(handlers); - - try - { - server.start(); - - String host = connector.getHost(); - if (host == null) - { - host = "localhost"; - } - int port = connector.getLocalPort(); - - URI serverUri = new URI("http",null,host,port,requestPath,null,null); - - // Make call to test handler - HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection(); - try - { - connection.setAllowUserInteraction(false); - - // log response status code - int statusCode = connection.getResponseCode(); - LOG.debug("Response Status Code: {}",statusCode); - - if (statusCode == 200) - { - // collect response message and log it - String content = getResponseContent(connection); - LOG.debug("Response Content: {}",content); - } - } - finally - { - connection.disconnect(); - } - - assertRequestLog(expectedLogEntry, captureLog); - } - finally - { - server.stop(); - } - } - - /** - * Test a RequestLogHandler at the end of a HandlerCollection and also with the ErrorHandler in place. - * @throws Exception if test failure - */ - @ParameterizedTest - @MethodSource("data") - public void testLogHandlerCollection_OKErrorHandler(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception - { - Server server = new Server(); - ServerConnector connector = new ServerConnector(server); - connector.setPort(0); - server.setConnectors(new Connector[] { connector }); - - OKErrorHandler errorDispatcher = new OKErrorHandler(); - server.addBean(errorDispatcher); - - ContextHandlerCollection contexts = new ContextHandlerCollection(); - ContextHandler errorContext = new ContextHandler("/errorpage"); - errorContext.setHandler(new AltErrorHandler()); - ContextHandler testContext = new ContextHandler("/test"); - testContext.setHandler(testHandler); - contexts.addHandler(errorContext); - contexts.addHandler(testContext); - - RequestLogHandler requestLog = new RequestLogHandler(); - CaptureLog captureLog = new CaptureLog(); - requestLog.setRequestLog(captureLog); - - HandlerCollection handlers = new HandlerCollection(); - handlers.setHandlers(new Handler[] { contexts, requestLog }); - server.setHandler(handlers); - - try - { - server.start(); - - String host = connector.getHost(); - if (host == null) - { - host = "localhost"; - } - int port = connector.getLocalPort(); - - URI serverUri = new URI("http",null,host,port,requestPath,null,null); - - // Make call to test handler - HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection(); - try - { - connection.setAllowUserInteraction(false); - - // log response status code - int statusCode = connection.getResponseCode(); - LOG.debug("Response Status Code: {}",statusCode); - - if (statusCode == 200) - { - // collect response message and log it - String content = getResponseContent(connection); - LOG.debug("Response Content: {}",content); - } - } - finally - { - connection.disconnect(); - } - - assertRequestLog(expectedLogEntry, captureLog); - } - finally - { - server.stop(); - } - } - - /** - * Test a RequestLogHandler at the end of a HandlerCollection and also with the ErrorHandler in place. - * @throws Exception if test failure - */ - @ParameterizedTest - @MethodSource("data") - public void testLogHandlerCollection_DispatchErrorHandler(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception - { - Server server = new Server(); - ServerConnector connector = new ServerConnector(server); - connector.setPort(0); - server.setConnectors(new Connector[] { connector }); - - DispatchErrorHandler errorDispatcher = new DispatchErrorHandler(); - server.addBean(errorDispatcher); - - ContextHandlerCollection contexts = new ContextHandlerCollection(); - ContextHandler errorContext = new ContextHandler("/errorok"); - errorContext.setHandler(new OKErrorHandler()); - ContextHandler testContext = new ContextHandler("/test"); - testContext.setHandler(testHandler); - contexts.addHandler(errorContext); - contexts.addHandler(testContext); - - RequestLogHandler requestLog = new RequestLogHandler(); - CaptureLog captureLog = new CaptureLog(); - requestLog.setRequestLog(captureLog); - - HandlerCollection handlers = new HandlerCollection(); - handlers.setHandlers(new Handler[] { contexts, requestLog }); - server.setHandler(handlers); - - try - { - server.start(); - - String host = connector.getHost(); - if (host == null) - { - host = "localhost"; - } - int port = connector.getLocalPort(); - - URI serverUri = new URI("http",null,host,port,requestPath,null,null); - - // Make call to test handler - HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection(); - try - { - connection.setAllowUserInteraction(false); - - // log response status code - int statusCode = connection.getResponseCode(); - LOG.debug("Response Status Code: {}",statusCode); - - if (statusCode == 200) - { - // collect response message and log it - String content = getResponseContent(connection); - LOG.debug("Response Content: {}",content); - } - } - finally - { - connection.disconnect(); - } - - assertRequestLog(expectedLogEntry, captureLog); - } - finally - { - server.stop(); - } - } - - @ParameterizedTest - @MethodSource("data") - public void testLogHandlerWrapped(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception - { - Server server = new Server(); - ServerConnector connector = new ServerConnector(server); - connector.setPort(0); - server.setConnectors(new Connector[] { connector }); - - CaptureLog captureLog = new CaptureLog(); - - RequestLogHandler requestLog = new RequestLogHandler(); - requestLog.setRequestLog(captureLog); - - requestLog.setHandler(testHandler); - - server.setHandler(requestLog); - - try (StacklessLogging ignore = new StacklessLogging(HttpChannel.class,HttpChannelState.class)) - { - server.start(); - - String host = connector.getHost(); - if (host == null) - { - host = "localhost"; - } - int port = connector.getLocalPort(); - - URI serverUri = new URI("http",null,host,port,requestPath,null,null); - - // Make call to test handler - HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection(); - try - { - connection.setAllowUserInteraction(false); - - // log response status code - int statusCode = connection.getResponseCode(); - LOG.info("Response Status Code: {}",statusCode); - - if (statusCode == 200) - { - // collect response message and log it - String content = getResponseContent(connection); - LOG.info("Response Content: {}",content); - } - } - finally - { - connection.disconnect(); - } - - assertRequestLog(expectedLogEntry, captureLog); - } - finally - { - server.stop(); - } - } - - private void assertRequestLog(final String expectedLogEntry, CaptureLog captureLog) - { - int captureCount = captureLog.captured.size(); - - if (captureCount != 1) - { - LOG.warn("Capture Log size is {}, expected to be 1",captureCount); - if (captureCount > 1) - { - for (int i = 0; i < captureCount; i++) - { - LOG.warn("[{}] {}",i,captureLog.captured.get(i)); - } - } - assertThat("Capture Log Entry Count",captureLog.captured.size(),is(1)); - } - - String actual = captureLog.captured.get(0); - assertThat("Capture Log",actual,is(expectedLogEntry)); - } - - private String getResponseContent(HttpURLConnection connection) throws IOException - { - try (InputStream in = connection.getInputStream(); InputStreamReader reader = new InputStreamReader(in)) - { - StringWriter writer = new StringWriter(); - IO.copy(reader,writer); - return writer.toString(); - } - } -} diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/RequestLogTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/RequestLogTest.java index 040e919d4b0..9c7069daf42 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/RequestLogTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/RequestLogTest.java @@ -18,31 +18,47 @@ package org.eclipse.jetty.server.handler; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; import java.io.IOException; import java.io.InputStream; import java.io.PrintWriter; +import java.util.ArrayList; import java.util.Arrays; -import java.util.concurrent.Exchanger; +import java.util.List; +import java.util.concurrent.BlockingQueue; import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; +import javax.servlet.AsyncContext; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.server.AbstractNCSARequestLog; +import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.RequestLog; +import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.BlockingArrayQueue; +import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; public class RequestLogTest { - Exchanger _log; + Log _log; Server _server; LocalConnector _connector; @@ -50,19 +66,35 @@ public class RequestLogTest @BeforeEach public void before() throws Exception { - _log = new Exchanger(); + _log = new Log(); _server = new Server(); _connector = new LocalConnector(_server); _server.addConnector(_connector); - _server.setRequestLog(new Log()); + + } + + void testHandlerServerStart() throws Exception + { + _server.setRequestLog(_log); _server.setHandler(new TestHandler()); _server.start(); } + private void startServer() throws Exception + { + _server.start(); + } + + private void makeRequest(String requestPath) throws Exception + { + _connector.getResponse("GET "+requestPath+" HTTP/1.0\r\n\r\n"); + } + + + @AfterEach public void after() throws Exception { - _server.stop(); } @@ -70,35 +102,41 @@ public class RequestLogTest @Test public void testNotHandled() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET /foo HTTP/1.0\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo HTTP/1.0\" 404 ")); } @Test public void testRequestLine() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET /foo?data=1 HTTP/1.0\nhost: host:80\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo?data=1 HTTP/1.0\" 200 ")); _connector.getResponse("GET //bad/foo?data=1 HTTP/1.0\n\n"); - log = _log.exchange(null,5,TimeUnit.SECONDS); + log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET //bad/foo?data=1 HTTP/1.0\" 200 ")); _connector.getResponse("GET http://host:80/foo?data=1 HTTP/1.0\n\n"); - log = _log.exchange(null,5,TimeUnit.SECONDS); + log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET http://host:80/foo?data=1 HTTP/1.0\" 200 ")); } @Test public void testHTTP10Host() throws Exception { + testHandlerServerStart(); + _connector.getResponse( "GET /foo?name=value HTTP/1.0\n"+ "Host: servername\n"+ "\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo?name=value")); assertThat(log,containsString(" 200 ")); } @@ -106,11 +144,13 @@ public class RequestLogTest @Test public void testHTTP11() throws Exception { + testHandlerServerStart(); + _connector.getResponse( "GET /foo?name=value HTTP/1.1\n"+ "Host: servername\n"+ "\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo?name=value")); assertThat(log,containsString(" 200 ")); } @@ -118,11 +158,13 @@ public class RequestLogTest @Test public void testAbsolute() throws Exception { + testHandlerServerStart(); + _connector.getResponse( "GET http://hostname:8888/foo?name=value HTTP/1.1\n"+ "Host: servername\n"+ "\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET http://hostname:8888/foo?name=value")); assertThat(log,containsString(" 200 ")); } @@ -130,8 +172,10 @@ public class RequestLogTest @Test public void testQuery() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET /foo?name=value HTTP/1.0\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo?name=value")); assertThat(log,containsString(" 200 ")); } @@ -139,8 +183,10 @@ public class RequestLogTest @Test public void testSmallData() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET /foo?data=42 HTTP/1.0\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo?")); assertThat(log,containsString(" 200 42 ")); } @@ -148,8 +194,10 @@ public class RequestLogTest @Test public void testBigData() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET /foo?data=102400 HTTP/1.0\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo?")); assertThat(log,containsString(" 200 102400 ")); } @@ -157,8 +205,10 @@ public class RequestLogTest @Test public void testStatus() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET /foo?status=206 HTTP/1.0\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo?")); assertThat(log,containsString(" 206 0 ")); } @@ -166,8 +216,10 @@ public class RequestLogTest @Test public void testStatusData() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET /foo?status=206&data=42 HTTP/1.0\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo?")); assertThat(log,containsString(" 206 42 ")); } @@ -175,83 +227,460 @@ public class RequestLogTest @Test public void testBadRequest() throws Exception { + testHandlerServerStart(); + _connector.getResponse("XXXXXXXXXXXX\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("\"- - -\"")); - assertThat(log,containsString(" 400 0 ")); + assertThat(log,containsString(" 400 ")); } @Test public void testBadCharacter() throws Exception { + testHandlerServerStart(); + _connector.getResponse("METHOD /f\00o HTTP/1.0\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("\"- - -\"")); - assertThat(log,containsString(" 400 0 ")); + assertThat(log,containsString(" 400 ")); } @Test public void testBadVersion() throws Exception { + testHandlerServerStart(); + _connector.getResponse("METHOD /foo HTTP/9\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("\"- - -\"")); - assertThat(log,containsString(" 400 0 ")); + assertThat(log,containsString(" 400 ")); } @Test public void testLongURI() throws Exception { + testHandlerServerStart(); + char[] chars = new char[10000]; Arrays.fill(chars,'o'); String ooo = new String(chars); _connector.getResponse("METHOD /f"+ooo+" HTTP/1.0\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("\"- - -\"")); - assertThat(log,containsString(" 414 0 ")); + assertThat(log,containsString(" 414 ")); } @Test public void testLongHeader() throws Exception { + testHandlerServerStart(); + char[] chars = new char[10000]; Arrays.fill(chars,'o'); String ooo = new String(chars); _connector.getResponse("METHOD /foo HTTP/1.0\name: f+"+ooo+"\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("\"METHOD /foo HTTP/1.0\"")); - assertThat(log,containsString(" 431 0 ")); + assertThat(log,containsString(" 431 ")); } @Test public void testBadRequestNoHost() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET /foo HTTP/1.1\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET /foo ")); - assertThat(log,containsString(" 400 0 ")); + assertThat(log,containsString(" 400 ")); } @Test public void testUseragentWithout() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET http://[:1]/foo HTTP/1.1\nReferer: http://other.site\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET http://[:1]/foo ")); - assertThat(log,containsString(" 400 0 \"http://other.site\" \"-\" - ")); + assertThat(log,containsString(" 400 50 \"http://other.site\" \"-\" - ")); } @Test public void testUseragentWith() throws Exception { + testHandlerServerStart(); + _connector.getResponse("GET http://[:1]/foo HTTP/1.1\nReferer: http://other.site\nUser-Agent: Mozilla/5.0 (test)\n\n"); - String log = _log.exchange(null,5,TimeUnit.SECONDS); + String log = _log.entries.poll(5,TimeUnit.SECONDS); assertThat(log,containsString("GET http://[:1]/foo ")); - assertThat(log,containsString(" 400 0 \"http://other.site\" \"Mozilla/5.0 (test)\" - ")); + assertThat(log,containsString(" 400 50 \"http://other.site\" \"Mozilla/5.0 (test)\" - ")); } + + + // Tests from here use these parameters + public static Stream data() + { + List data = new ArrayList<>(); + + data.add(new Object[] { new NoopHandler(), "/noop", "\"GET /noop HTTP/1.0\" 404" }); + data.add(new Object[] { new HelloHandler(), "/hello", "\"GET /hello HTTP/1.0\" 200" }); + data.add(new Object[] { new ResponseSendErrorHandler(), "/sendError", "\"GET /sendError HTTP/1.0\" 599" }); + data.add(new Object[] { new ServletExceptionHandler(), "/sex", "\"GET /sex HTTP/1.0\" 500" }); + data.add(new Object[] { new IOExceptionHandler(), "/ioex", "\"GET /ioex HTTP/1.0\" 500" }); + data.add(new Object[] { new RuntimeExceptionHandler(), "/rtex", "\"GET /rtex HTTP/1.0\" 500" }); + data.add(new Object[] { new BadMessageHandler(), "/bad", "\"GET /bad HTTP/1.0\" 499" }); + data.add(new Object[] { new AbortHandler(), "/bad", "\"GET /bad HTTP/1.0\" 488" }); + + return data.stream().map(Arguments::of); + } + + @ParameterizedTest + @MethodSource("data") + public void testServerRequestLog(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception + { + _server.setRequestLog(_log); + _server.setHandler(testHandler); + startServer(); + makeRequest(requestPath); + assertRequestLog(expectedLogEntry, _log); + } + + @ParameterizedTest + @MethodSource("data") + public void testLogHandlerWrapper(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception + { + RequestLogHandler handler = new RequestLogHandler(); + handler.setRequestLog(_log); + handler.setHandler(testHandler); + _server.setHandler(handler); + startServer(); + makeRequest(requestPath); + assertRequestLog(expectedLogEntry, _log); + } + + @ParameterizedTest + @MethodSource("data") + public void testLogHandlerCollectionFirst(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception + { + RequestLogHandler handler = new RequestLogHandler(); + handler.setRequestLog(_log); + HandlerCollection handlers = new HandlerCollection(); + handlers.setHandlers(new Handler[] { handler, testHandler }); + _server.setHandler(handlers); + startServer(); + makeRequest(requestPath); + assertRequestLog(expectedLogEntry, _log); + } + + + @ParameterizedTest + @MethodSource("data") + public void testLogHandlerCollectionLast(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception + { + RequestLogHandler handler = new RequestLogHandler(); + handler.setRequestLog(_log); + // This is the old ordering of request handler and it cannot well handle thrown exception + Assumptions.assumeTrue( + testHandler instanceof NoopHandler || + testHandler instanceof HelloHandler || + testHandler instanceof ResponseSendErrorHandler + ); + + HandlerCollection handlers = new HandlerCollection(); + handlers.setHandlers(new Handler[] { testHandler, handler }); + _server.setHandler(handlers); + startServer(); + makeRequest(requestPath); + assertRequestLog(expectedLogEntry, _log); + } + + + @ParameterizedTest + @MethodSource("data") + public void testErrorHandler(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception + { + _server.setRequestLog(_log); + AbstractHandler.ErrorDispatchHandler wrapper = new AbstractHandler.ErrorDispatchHandler() + { + @Override + protected void doNonErrorHandle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + throws IOException, ServletException + { + testHandler.handle(target,baseRequest,request,response); + } + }; + + _server.setHandler(wrapper); + + List errors = new ArrayList<>(); + ErrorHandler errorHandler = new ErrorHandler() + { + @Override + public void doError(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + errors.add(baseRequest.getRequestURI()); + super.doError(target, baseRequest, request, response); + } + }; + _server.addBean(errorHandler); + startServer(); + makeRequest(requestPath); + assertRequestLog(expectedLogEntry, _log); + + if (!(testHandler instanceof HelloHandler)) + assertThat(errors,contains(requestPath)); + } + + + @ParameterizedTest + @MethodSource("data") + public void testOKErrorHandler(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception + { + _server.setRequestLog(_log); + AbstractHandler.ErrorDispatchHandler wrapper = new AbstractHandler.ErrorDispatchHandler() + { + @Override + protected void doNonErrorHandle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + throws IOException, ServletException + { + testHandler.handle(target,baseRequest,request,response); + } + }; + + _server.setHandler(wrapper); + + ErrorHandler errorHandler = new OKErrorHandler(); + _server.addBean(errorHandler); + startServer(); + makeRequest(requestPath); + + expectedLogEntry = "\"GET " + requestPath + " HTTP/1.0\" 200"; + assertRequestLog(expectedLogEntry, _log); + } + + + @ParameterizedTest + @MethodSource("data") + public void testAsyncDispatch(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception + { + _server.setRequestLog(_log); + _server.setHandler(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + throws IOException, ServletException + { + if (Boolean.TRUE.equals(request.getAttribute("ASYNC"))) + testHandler.handle(target,baseRequest,request,response); + else + { + request.setAttribute("ASYNC",Boolean.TRUE); + AsyncContext ac = request.startAsync(); + ac.setTimeout(1000); + ac.dispatch(); + baseRequest.setHandled(true); + } + } + }); + startServer(); + makeRequest(requestPath); + + assertRequestLog(expectedLogEntry, _log); + } + + + @ParameterizedTest + @MethodSource("data") + public void testAsyncComplete(Handler testHandler, String requestPath, String expectedLogEntry) throws Exception + { + _server.setRequestLog(_log); + _server.setHandler(new AbstractHandler() + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) + throws IOException, ServletException + { + if (Boolean.TRUE.equals(request.getAttribute("ASYNC"))) + testHandler.handle(target,baseRequest,request,response); + else + { + request.setAttribute("ASYNC",Boolean.TRUE); + AsyncContext ac = request.startAsync(); + ac.setTimeout(1000); + baseRequest.setHandled(true); + _server.getThreadPool().execute(()-> + { + try + { + try + { + baseRequest.setHandled(false); + testHandler.handle(target, baseRequest, request, response); + if (!baseRequest.isHandled()) + response.sendError(404); + } + catch (BadMessageException bad) + { + response.sendError(bad.getCode()); + } + catch (Exception e) + { + response.sendError(500); + } + } + catch(Throwable th) + { + throw new RuntimeException(th); + } + ac.complete(); + }); + } + } + }); + startServer(); + makeRequest(requestPath); + assertRequestLog(expectedLogEntry, _log); + } + + + private void assertRequestLog(final String expectedLogEntry, Log log) throws Exception + { + String line = log.entries.poll(5, TimeUnit.SECONDS); + Assertions.assertNotNull(line); + assertThat(line,containsString(expectedLogEntry)); + Assertions.assertTrue(log.entries.isEmpty()); + } + + public static class CaptureLog extends AbstractLifeCycle implements RequestLog + { + public BlockingQueue log = new BlockingArrayQueue<>(); + + @Override + public void log(Request request, Response response) + { + int status = response.getCommittedMetaData().getStatus(); + log.add(String.format("%s %s %s %03d",request.getMethod(),request.getRequestURI(),request.getProtocol(),status)); + } + } + + private static abstract class AbstractTestHandler extends AbstractHandler + { + @Override + public String toString() + { + return this.getClass().getSimpleName(); + } + } + + private static class NoopHandler extends AbstractTestHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + } + } + + private static class HelloHandler extends AbstractTestHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + response.setContentType("text/plain"); + response.getWriter().print("Hello World"); + if (baseRequest!=null) + baseRequest.setHandled(true); + } + } + + private static class ResponseSendErrorHandler extends AbstractTestHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + response.sendError(599,"expected"); + if (baseRequest!=null) + baseRequest.setHandled(true); + } + } + + private static class ServletExceptionHandler extends AbstractTestHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + throw new ServletException("expected"); + } + } + + private static class IOExceptionHandler extends AbstractTestHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + throw new IOException("expected"); + } + } + + private static class RuntimeExceptionHandler extends AbstractTestHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + throw new RuntimeException("expected"); + } + } + + private static class BadMessageHandler extends AbstractTestHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + throw new BadMessageException(499); + } + } + + private static class AbortHandler extends AbstractTestHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + { + BadMessageException bad = new BadMessageException(488); + baseRequest.getHttpChannel().abort(bad); + throw bad; + } + } + + public static class OKErrorHandler extends ErrorHandler + { + @Override + public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException + { + if (baseRequest.isHandled() || response.isCommitted()) + { + return; + } + + // collect error details + String reason = (response instanceof Response)?((Response)response).getReason():null; + int status = response.getStatus(); + + // intentionally set response status to OK (this is a test to see what is actually logged) + response.setStatus(200); + response.setContentType("text/plain"); + PrintWriter out = response.getWriter(); + out.printf("Error %d: %s%n",status,reason); + baseRequest.setHandled(true); + } + } + private class Log extends AbstractNCSARequestLog { + public BlockingQueue entries = new BlockingArrayQueue<>(); + + Log() { super.setExtended(true); super.setLogLatency(true); @@ -269,7 +698,7 @@ public class RequestLogTest { try { - _log.exchange(requestEntry); + entries.add(requestEntry); } catch(Exception e) { @@ -277,7 +706,7 @@ public class RequestLogTest } } } - + private class TestHandler extends AbstractHandler { @Override @@ -286,7 +715,7 @@ public class RequestLogTest String q = request.getQueryString(); if (q==null) return; - + baseRequest.setHandled(true); for (String action : q.split("\\&")) { @@ -300,12 +729,12 @@ public class RequestLogTest response.setStatus(Integer.parseInt(value)); break; } - + case "data": { int data = Integer.parseInt(value); PrintWriter out = response.getWriter(); - + int w=0; while (w(); RequestLog log=new Log(); - RequestLogHandler logHandler = new RequestLogHandler(); - logHandler.setRequestLog(log); - _server.setHandler(logHandler); + _server.setRequestLog(log); _expectedLogs=1; _expectedCode="200 "; ServletContextHandler context = new ServletContextHandler(ServletContextHandler.NO_SESSIONS); context.setContextPath("/ctx"); - logHandler.setHandler(context); + _server.setHandler(context); context.addEventListener(new DebugListener()); _errorHandler = new ErrorPageErrorHandler(); diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletRequestLogTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletRequestLogTest.java index 32d45af3edf..2ab0f0b5b7a 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletRequestLogTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletRequestLogTest.java @@ -53,7 +53,6 @@ import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.DefaultHandler; import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.server.handler.HandlerCollection; -import org.eclipse.jetty.server.handler.RequestLogHandler; import org.eclipse.jetty.toolchain.test.IO; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.log.Log; @@ -313,12 +312,9 @@ public class ServletRequestLogTest // Next the behavior as defined by etc/jetty-requestlog.xml // the id="RequestLog" - RequestLogHandler requestLog = new RequestLogHandler(); CaptureLog captureLog = new CaptureLog(); - requestLog.setRequestLog(captureLog); + server.setRequestLog(captureLog); - handlers.addHandler(requestLog); - // Lastly, the behavior as defined by deployment of a webapp // Add the Servlet Context ServletContextHandler app = new ServletContextHandler(ServletContextHandler.SESSIONS); @@ -404,12 +400,9 @@ public class ServletRequestLogTest // Next the behavior as defined by etc/jetty-requestlog.xml // the id="RequestLog" - RequestLogHandler requestLog = new RequestLogHandler(); CaptureLog captureLog = new CaptureLog(); - requestLog.setRequestLog(captureLog); + server.setRequestLog(captureLog); - handlers.addHandler(requestLog); - // Lastly, the behavior as defined by deployment of a webapp // Add the Servlet Context ServletContextHandler app = new ServletContextHandler(ServletContextHandler.SESSIONS); @@ -493,12 +486,9 @@ public class ServletRequestLogTest // Next the behavior as defined by etc/jetty-requestlog.xml // the id="RequestLog" - RequestLogHandler requestLog = new RequestLogHandler(); CaptureLog captureLog = new CaptureLog(); - requestLog.setRequestLog(captureLog); + server.setRequestLog(captureLog); - handlers.addHandler(requestLog); - // Lastly, the behavior as defined by deployment of a webapp // Add the Servlet Context ServletContextHandler app = new ServletContextHandler(ServletContextHandler.SESSIONS); @@ -587,14 +577,9 @@ public class ServletRequestLogTest // Next the proposed behavioral change to etc/jetty-requestlog.xml // the id="RequestLog" - RequestLogHandler requestLog = new RequestLogHandler(); CaptureLog captureLog = new CaptureLog(); - requestLog.setRequestLog(captureLog); - - Handler origServerHandler = server.getHandler(); - requestLog.setHandler(origServerHandler); - server.setHandler(requestLog); - + server.setRequestLog(captureLog); + // Lastly, the behavior as defined by deployment of a webapp // Add the Servlet Context ServletContextHandler app = new ServletContextHandler(ServletContextHandler.SESSIONS); diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/ServerTimeoutsTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/ServerTimeoutsTest.java index 399ec3418f7..ba32c40d9fa 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/ServerTimeoutsTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/ServerTimeoutsTest.java @@ -20,9 +20,14 @@ package org.eclipse.jetty.http.client; import static org.eclipse.jetty.http.client.Transport.FCGI; import static org.eclipse.jetty.http.client.Transport.UNIX_SOCKET; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; 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.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; @@ -56,10 +61,10 @@ import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.log.StacklessLogging; -import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; @@ -616,6 +621,7 @@ public class ServerTimeoutsTest extends AbstractTest { init(transport); int bytesPerSecond = 20; + scenario.requestLog.clear(); scenario.httpConfig.setMinRequestDataRate(bytesPerSecond); CountDownLatch handlerLatch = new CountDownLatch(1); scenario.start(new AbstractHandler.ErrorDispatchHandler() @@ -643,13 +649,15 @@ public class ServerTimeoutsTest extends AbstractTest }); DeferredContentProvider contentProvider = new DeferredContentProvider(); - CountDownLatch resultLatch = new CountDownLatch(1); + BlockingQueue results = new BlockingArrayQueue<>(); scenario.client.newRequest(scenario.newURI()) .content(contentProvider) .send(result -> { - if (result.getResponse().getStatus() == HttpStatus.REQUEST_TIMEOUT_408) - resultLatch.countDown(); + if (result.isFailed()) + results.offer(result.getFailure()); + else + results.offer(result.getResponse().getStatus()); }); for (int i = 0; i < 3; ++i) @@ -659,9 +667,17 @@ public class ServerTimeoutsTest extends AbstractTest } contentProvider.close(); + assertThat(scenario.requestLog.poll(5,TimeUnit.SECONDS), containsString(" 408")); + // Request should timeout. assertTrue(handlerLatch.await(5, TimeUnit.SECONDS)); - assertTrue(resultLatch.await(5, TimeUnit.SECONDS)); + + Object result = results.poll(5, TimeUnit.SECONDS); + assertNotNull(result); + if (result instanceof Integer) + assertThat((Integer)result,is(408)); + else + assertThat(result,instanceOf(Throwable.class)); } @ParameterizedTest diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java index 752d03aa382..174c1e51f1c 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java @@ -25,6 +25,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.concurrent.BlockingQueue; import javax.servlet.http.HttpServlet; @@ -56,6 +57,7 @@ import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.unixsocket.UnixSocketConnector; import org.eclipse.jetty.unixsocket.client.HttpClientTransportOverUnixSockets; +import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -75,6 +77,7 @@ public class TransportScenario protected String servletPath = "/servlet"; protected HttpClient client; protected Path sockFile; + protected final BlockingQueue requestLog= new BlockingArrayQueue<>(); public TransportScenario(final Transport transport) throws IOException { @@ -320,7 +323,15 @@ public class TransportScenario server.addBean(mbeanContainer); connector = newServerConnector(server); server.addConnector(connector); + + server.setRequestLog((request, response) -> + { + int status = response.getCommittedMetaData().getStatus(); + requestLog.offer(String.format("%s %s %s %03d",request.getMethod(),request.getRequestURI(),request.getProtocol(),status)); + }); + server.setHandler(handler); + try { server.start(); @@ -375,4 +386,6 @@ public class TransportScenario } } } + + } diff --git a/tests/test-webapps/test-jetty-webapp/src/test/java/org/eclipse/jetty/TestServer.java b/tests/test-webapps/test-jetty-webapp/src/test/java/org/eclipse/jetty/TestServer.java index a078dac760e..ca61e939857 100644 --- a/tests/test-webapps/test-jetty-webapp/src/test/java/org/eclipse/jetty/TestServer.java +++ b/tests/test-webapps/test-jetty-webapp/src/test/java/org/eclipse/jetty/TestServer.java @@ -34,7 +34,6 @@ import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.DefaultHandler; import org.eclipse.jetty.server.handler.HandlerCollection; import org.eclipse.jetty.server.handler.HandlerWrapper; -import org.eclipse.jetty.server.handler.RequestLogHandler; import org.eclipse.jetty.server.handler.ResourceHandler; import org.eclipse.jetty.server.session.DefaultSessionCache; import org.eclipse.jetty.server.session.FileSessionDataStore; @@ -105,9 +104,8 @@ public class TestServer // Handlers HandlerCollection handlers = new HandlerCollection(); ContextHandlerCollection contexts = new ContextHandlerCollection(); - RequestLogHandler requestLogHandler = new RequestLogHandler(); handlers.setHandlers(new Handler[] - { contexts, new DefaultHandler(), requestLogHandler }); + { contexts, new DefaultHandler() }); // Add restart handler to test the ability to save sessions and restart RestartHandler restart = new RestartHandler(); @@ -125,7 +123,7 @@ public class TestServer File log=File.createTempFile("jetty-yyyy_mm_dd", "log"); NCSARequestLog requestLog = new NCSARequestLog(log.toString()); requestLog.setExtended(false); - requestLogHandler.setRequestLog(requestLog); + server.setRequestLog(requestLog); server.setStopAtShutdown(true); From 0fe4b8aa34c997204c6ca90a5ed2ca37bde4a7d7 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 1 Nov 2018 17:35:21 +0100 Subject: [PATCH 09/12] increased timeouts to check for flaky test Signed-off-by: Greg Wilkins --- .../jetty/client/ssl/NeedWantClientAuthTest.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/NeedWantClientAuthTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/NeedWantClientAuthTest.java index 12aca96fec7..53a1262cf16 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/NeedWantClientAuthTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ssl/NeedWantClientAuthTest.java @@ -110,7 +110,7 @@ public class NeedWantClientAuthTest startClient(clientSSL); ContentResponse response = client.newRequest("https://localhost:" + connector.getLocalPort()) - .timeout(5, TimeUnit.SECONDS) + .timeout(10, TimeUnit.SECONDS) .send(); assertEquals(HttpStatus.OK_200, response.getStatus()); @@ -149,11 +149,11 @@ public class NeedWantClientAuthTest startClient(clientSSL); ContentResponse response = client.newRequest("https://localhost:" + connector.getLocalPort()) - .timeout(5, TimeUnit.SECONDS) + .timeout(10, TimeUnit.SECONDS) .send(); assertEquals(HttpStatus.OK_200, response.getStatus()); - assertTrue(handshakeLatch.await(5, TimeUnit.SECONDS)); + assertTrue(handshakeLatch.await(10, TimeUnit.SECONDS)); } @Test @@ -192,7 +192,7 @@ public class NeedWantClientAuthTest CountDownLatch latch = new CountDownLatch(1); client.newRequest("https://localhost:" + connector.getLocalPort()) - .timeout(5, TimeUnit.SECONDS) + .timeout(10, TimeUnit.SECONDS) .send(result -> { if (result.isFailed()) @@ -203,8 +203,8 @@ public class NeedWantClientAuthTest } }); - assertTrue(handshakeLatch.await(5, TimeUnit.SECONDS)); - assertTrue(latch.await(5, TimeUnit.SECONDS)); + assertTrue(handshakeLatch.await(10, TimeUnit.SECONDS)); + assertTrue(latch.await(10, TimeUnit.SECONDS)); } @Test @@ -240,10 +240,10 @@ public class NeedWantClientAuthTest startClient(clientSSL); ContentResponse response = client.newRequest("https://localhost:" + connector.getLocalPort()) - .timeout(5, TimeUnit.SECONDS) + .timeout(10, TimeUnit.SECONDS) .send(); assertEquals(HttpStatus.OK_200, response.getStatus()); - assertTrue(handshakeLatch.await(5, TimeUnit.SECONDS)); + assertTrue(handshakeLatch.await(10, TimeUnit.SECONDS)); } } From 9d7d47f0a8bef61dea81e82b824adfa8339c25e5 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 1 Nov 2018 17:40:15 +0100 Subject: [PATCH 10/12] Issue #2941 - JDK 11: UnsupportedOperationException at AnnotationParser.scanClass. Updated ASM Import-Package version to be "5", which means from ASM 5 onwards. Updated ASM version in the main POM to 7.0. Signed-off-by: Simone Bordet --- jetty-annotations/pom.xml | 2 +- jetty-osgi/jetty-osgi-boot/pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-annotations/pom.xml b/jetty-annotations/pom.xml index 52080ae0f58..c1e4feeaa35 100644 --- a/jetty-annotations/pom.xml +++ b/jetty-annotations/pom.xml @@ -20,7 +20,7 @@ true - org.objectweb.asm;version="[5.0,7)",* + org.objectweb.asm;version="5",* osgi.serviceloader; filter:="(osgi.serviceloader=javax.servlet.ServletContainerInitializer)";resolution:=optional;cardinality:=multiple, osgi.extender; filter:="(osgi.extender=osgi.serviceloader.processor)";resolution:=optional diff --git a/jetty-osgi/jetty-osgi-boot/pom.xml b/jetty-osgi/jetty-osgi-boot/pom.xml index 18457863a7c..bf52d3592a9 100644 --- a/jetty-osgi/jetty-osgi-boot/pom.xml +++ b/jetty-osgi/jetty-osgi-boot/pom.xml @@ -80,7 +80,7 @@ javax.servlet.http;version="[3.1,3.2)", javax.transaction;version="1.1.0";resolution:=optional, javax.transaction.xa;version="1.1.0";resolution:=optional, - org.objectweb.asm;version=4;resolution:=optional, + org.objectweb.asm;version="5";resolution:=optional, org.osgi.framework, org.osgi.service.cm;version="1.2.0", org.osgi.service.packageadmin, From 97c37f998bf887d1fe733c22f769d1bd777edf29 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 1 Nov 2018 17:48:39 +0100 Subject: [PATCH 11/12] Issue #2941 - JDK 11: UnsupportedOperationException at AnnotationParser.scanClass. Updated ASM version in the main POM to 7.0. Signed-off-by: Simone Bordet --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index cc1098231cf..620000f4a80 100644 --- a/pom.xml +++ b/pom.xml @@ -26,7 +26,7 @@ undefined 1.1.4 - 7.0-beta + 7.0 1.21 benchmarks 1.2.0 From 58c1d547a01004a79c00723af45632c57f8d7d11 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 2 Nov 2018 04:14:24 +1100 Subject: [PATCH 12/12] Issue #3044 Use unwrapped session for SessionAsyncListener (#3053) * Issue #3044 Use unwrapped session for SessionAsyncListener --- .../jetty/server/session/SessionHandler.java | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java index e266a243eea..5dde83b28b6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java @@ -158,19 +158,12 @@ public class SessionHandler extends ScopedHandler * has its session completed as the request exits the context. */ public class SessionAsyncListener implements AsyncListener - { - private Session _session; - - public SessionAsyncListener(Session session) - { - _session = session; - } - + { @Override public void onComplete(AsyncEvent event) throws IOException { //An async request has completed, so we can complete the session - complete(((HttpServletRequest)event.getAsyncContext().getRequest()).getSession(false)); + complete(Request.getBaseRequest(event.getAsyncContext().getRequest()).getSession(false)); } @Override @@ -182,7 +175,7 @@ public class SessionHandler extends ScopedHandler @Override public void onError(AsyncEvent event) throws IOException { - complete(((HttpServletRequest)event.getAsyncContext().getRequest()).getSession(false)); + complete(Request.getBaseRequest(event.getAsyncContext().getRequest()).getSession(false)); } @Override @@ -190,7 +183,6 @@ public class SessionHandler extends ScopedHandler { event.getAsyncContext().addListener(this); } - } static final HttpSessionContext __nullSessionContext=new HttpSessionContext() @@ -251,6 +243,7 @@ public class SessionHandler extends ScopedHandler protected Scheduler _scheduler; protected boolean _ownScheduler = false; + protected final SessionAsyncListener _sessionAsyncListener = new SessionAsyncListener(); @@ -381,7 +374,7 @@ public class SessionHandler extends ScopedHandler { if (request.isAsyncStarted() && request.getDispatcherType() == DispatcherType.REQUEST) { - request.getAsyncContext().addListener(new SessionAsyncListener(session)); + request.getAsyncContext().addListener(_sessionAsyncListener); } else {