From 190af2972b552652f2c6cd4eca084a7dbcd122bd Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 13 Jul 2023 15:12:47 +0200 Subject: [PATCH] Fixes delivery of events to `Response.CompleteListener`s. (#10102) In case of `request.onComplete(Response.CompleteListener l)`, only the complete event should be delivered to `l`, not all the events (in case `l` implements other listener interfaces). Only `Response.CompleteListener` passed to `request.send(Response.CompleteListener)` should receive all events. Signed-off-by: Simone Bordet --- .../eclipse/jetty/client/transport/HttpConnection.java | 2 +- .../org/eclipse/jetty/client/transport/HttpRequest.java | 4 ++-- .../jetty/client/transport/ResponseListeners.java | 7 +------ .../java/org/eclipse/jetty/client/HttpClientTest.java | 9 +++++---- .../client/transport/internal/HttpReceiverOverHTTP2.java | 2 +- 5 files changed, 10 insertions(+), 14 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpConnection.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpConnection.java index c9f26a6806b5..639ee70d906a 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpConnection.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpConnection.java @@ -81,7 +81,7 @@ public void send(Request request, Response.CompleteListener listener) httpRequest.sent(); if (listener != null) - responseListeners.addCompleteListener(listener); + responseListeners.addCompleteListener(listener, true); HttpExchange exchange = new HttpExchange(getHttpDestination(), httpRequest); diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpRequest.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpRequest.java index 814f68802228..116e2656b416 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpRequest.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpRequest.java @@ -549,7 +549,7 @@ public Request onPush(BiFunction pu @Override public Request onComplete(Response.CompleteListener listener) { - responseListeners.addCompleteListener(listener); + responseListeners.addCompleteListener(listener, false); return this; } @@ -674,7 +674,7 @@ public void send(Response.CompleteListener listener) void sendAsync(HttpDestination destination, Response.CompleteListener listener) { if (listener != null) - responseListeners.addCompleteListener(listener); + responseListeners.addCompleteListener(listener, true); destination.send(this); } diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/ResponseListeners.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/ResponseListeners.java index 844727d4ca64..2982cfe0a877 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/ResponseListeners.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/ResponseListeners.java @@ -308,12 +308,7 @@ private static void notifyFailure(Response.FailureListener listener, Response re } } - public boolean addCompleteListener(Response.CompleteListener listener) - { - return addCompleteListener(listener, true); - } - - private boolean addCompleteListener(Response.CompleteListener listener, boolean includeOtherEvents) + public boolean addCompleteListener(Response.CompleteListener listener, boolean includeOtherEvents) { if (listener == null) return false; diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index aa1cc52a27e8..0df32712f23c 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -1013,7 +1013,7 @@ public void testResponseListenerForMultipleEventsIsInvokedOncePerEvent(Scenario start(scenario, new EmptyServerHandler()); AtomicInteger counter = new AtomicInteger(); - CountDownLatch latch = new CountDownLatch(1); + CountDownLatch latch = new CountDownLatch(2); Response.Listener listener = new Response.Listener() { @Override @@ -1079,12 +1079,13 @@ public void onComplete(Result result) .onResponseContentAsync(listener) .onResponseSuccess(listener) .onResponseFailure(listener) + .onComplete(listener) .send(listener); assertTrue(latch.await(5, TimeUnit.SECONDS)); - int expectedEventsTriggeredByOnResponseXXXListeners = 3; - int expectedEventsTriggeredByCompletionListener = 4; - int expected = expectedEventsTriggeredByOnResponseXXXListeners + expectedEventsTriggeredByCompletionListener; + int expectedEventsTriggeredByResponseListeners = 4; + int expectedEventsTriggeredBySendListener = 4; + int expected = expectedEventsTriggeredByResponseListeners + expectedEventsTriggeredBySendListener; assertEquals(expected, counter.get()); } diff --git a/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java b/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java index 153f64d2c897..73ddd6b139b4 100644 --- a/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java +++ b/jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java @@ -186,7 +186,7 @@ Stream.Listener onPush(Stream stream, PushPromiseFrame frame) if (listener != null) { HttpChannelOverHTTP2 pushChannel = getHttpChannel().getHttpConnection().acquireHttpChannel(); - pushRequest.getResponseListeners().addCompleteListener(listener); + pushRequest.getResponseListeners().addCompleteListener(listener, true); HttpExchange pushExchange = new HttpExchange(getHttpDestination(), pushRequest); pushChannel.associate(pushExchange); pushChannel.setStream(stream);