Fixes delivery of events to Response.CompleteListeners. (#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 <[email protected]>
jetty · Jul 13, 2023 · 190af29 · 190af29
1 parent a08e953
commit 190af29
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 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
@@ -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
@@ -549,7 +549,7 @@ public Request onPush(BiFunction<Request, Request, Response.CompleteListener> 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/...core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/ResponseListeners.java b/...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
@@ -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/...rc/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java b/...rc/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);