8336655: java/net/httpclient/DigestEchoClient.java IOException: HTTP/1.1 header parser received no bytes

Reviewed-by: jpai

src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -44,6 +44,7 @@ import java.util.stream.Collectors;
 
 import jdk.internal.net.http.common.Deadline;
 import jdk.internal.net.http.common.FlowTube;
+import jdk.internal.net.http.common.Log;
 import jdk.internal.net.http.common.Logger;
 import jdk.internal.net.http.common.TimeLine;
 import jdk.internal.net.http.common.TimeSource;
@@ -492,13 +493,13 @@ final class ConnectionPool {
 
     // Remove a connection from the pool.
     // should only be called while holding the ConnectionPool stateLock.
-    private void removeFromPool(HttpConnection c) {
+    private boolean removeFromPool(HttpConnection c) {
         assert stateLock.isHeldByCurrentThread();
         if (c instanceof PlainHttpConnection) {
-            removeFromPool(c, plainPool);
+            return removeFromPool(c, plainPool);
         } else {
             assert c.isSecure() : "connection " + c + " is not secure!";
-            removeFromPool(c, sslPool);
+            return removeFromPool(c, sslPool);
         }
     }
 
@@ -529,13 +530,29 @@ final class ConnectionPool {
             debug.log("%s : ConnectionPool.cleanup(%s)",
                     String.valueOf(c.getConnectionFlow()), error);
         stateLock.lock();
+        boolean removed;
         try {
-            removeFromPool(c);
+            removed = removeFromPool(c);
             expiryList.remove(c);
         } finally {
             stateLock.unlock();
         }
-        c.close();
+        if (!removed) {
+            // this should not happen; the cleanup may have consumed
+            // some data that wasn't supposed to be consumed, so
+            // the only thing we can do is log it and close the
+            // connection.
+            if (Log.errors()) {
+                Log.logError("WARNING: CleanupTrigger triggered for" +
+                        " a connection not found in the pool: closing {0}", c);
+            } else if (debug.on()) {
+                debug.log("WARNING: CleanupTrigger triggered for" +
+                        " a connection not found in the pool: closing %s", c);
+            }
+            c.close(new IOException("Unexpected cleanup triggered for non pooled connection"));
+        } else {
+            c.close();
+        }
     }
 
     /**
@@ -549,6 +566,7 @@ final class ConnectionPool {
 
         private final HttpConnection connection;
         private volatile boolean done;
+        private volatile boolean dropped;
 
         public CleanupTrigger(HttpConnection connection) {
             this.connection = connection;
@@ -566,6 +584,7 @@ final class ConnectionPool {
 
         @Override
         public void onSubscribe(Flow.Subscription subscription) {
+            if (dropped || done) return;
             subscription.request(1);
         }
         @Override
@@ -586,5 +605,10 @@ final class ConnectionPool {
         public String toString() {
             return "CleanupTrigger(" + connection.getConnectionFlow() + ")";
         }
+
+        @Override
+        public void dropSubscription() {
+            dropped = true;
+        }
     }
 }

src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -573,6 +573,8 @@ final class SocketTube implements FlowTube {
                     debug.log("read publisher: dropping pending subscriber: "
                               + previous.subscriber);
                 previous.errorRef.compareAndSet(null, errorRef.get());
+                // make sure no data will be routed to the old subscriber.
+                previous.stopReading();
                 previous.signalOnSubscribe();
                 if (subscriptionImpl.completed) {
                     previous.signalCompletion();
@@ -606,6 +608,7 @@ final class SocketTube implements FlowTube {
             volatile boolean subscribed;
             volatile boolean cancelled;
             volatile boolean completed;
+            volatile boolean stopped;
 
             public ReadSubscription(InternalReadSubscription impl,
                                     TubeSubscriber subscriber) {
@@ -623,11 +626,11 @@ final class SocketTube implements FlowTube {
 
             @Override
             public void request(long n) {
-                if (!cancelled) {
+                if (!cancelled && !stopped) {
                     impl.request(n);
                 } else {
                     if (debug.on())
-                        debug.log("subscription cancelled, ignoring request %d", n);
+                        debug.log("subscription stopped or cancelled, ignoring request %d", n);
                 }
             }
 
@@ -661,6 +664,20 @@ final class SocketTube implements FlowTube {
                     signalCompletion();
                 }
             }
+
+            /**
+             * Called when switching subscriber on the {@link InternalReadSubscription}.
+             * This subscriber is the old subscriber. Demand on the internal
+             * subscription will be reset and reading will be paused until the
+             * new subscriber is subscribed.
+             * This should ensure that no data is routed to this subscriber
+             * until the new subscriber is subscribed.
+             */
+            void stopReading() {
+                stopped = true;
+                impl.demand.reset();
+                impl.pauseReadEvent();
+            }
         }
 
         final class InternalReadSubscription implements Flow.Subscription {

test/jdk/java/net/httpclient/DigestEchoClient.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -64,7 +64,7 @@ import static java.lang.String.format;
  * @test
  * @summary this test verifies that a client may provides authorization
  *          headers directly when connecting with a server.
- * @bug 8087112
+ * @bug 8087112 8336655
  * @library /test/lib /test/jdk/java/net/httpclient/lib
  * @build jdk.httpclient.test.lib.common.HttpServerAdapters jdk.test.lib.net.SimpleSSLContext
  *        DigestEchoServer ReferenceTracker DigestEchoClient