8268617: [11u REDO] - WebSocket over authenticating proxy fails with NPE

Larry-N · RealCLanger · commit 8a40d251e18d · 2021-06-28T11:29:31.000Z
Reviewed-by: clanger
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/AuthenticationFilter.java b/src/java.net.http/share/classes/jdk/internal/net/http/AuthenticationFilter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -240,20 +240,23 @@ public HttpRequestImpl response(Response r) throws IOException {
         HttpHeaders hdrs = r.headers();
         HttpRequestImpl req = r.request();
 
-        if (status != UNAUTHORIZED && status != PROXY_UNAUTHORIZED) {
-            // check if any authentication succeeded for first time
-            if (exchange.serverauth != null && !exchange.serverauth.fromcache) {
-                AuthInfo au = exchange.serverauth;
-                cache.store(au.scheme, req.uri(), false, au.credentials);
-            }
+        if (status != PROXY_UNAUTHORIZED) {
             if (exchange.proxyauth != null && !exchange.proxyauth.fromcache) {
                 AuthInfo au = exchange.proxyauth;
                 URI proxyURI = getProxyURI(req);
                 if (proxyURI != null) {
+                    exchange.proxyauth = null;
                     cache.store(au.scheme, proxyURI, true, au.credentials);
                 }
             }
+            if (status != UNAUTHORIZED) {
+            // check if any authentication succeeded for first time
+                if (exchange.serverauth != null && !exchange.serverauth.fromcache) {
+                    AuthInfo au = exchange.serverauth;
+                    cache.store(au.scheme, req.uri(), false, au.credentials);
+                }
             return null;
+            }
         }
 
         boolean proxy = status == PROXY_UNAUTHORIZED;
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java b/src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -142,6 +142,7 @@ synchronized HttpConnection getConnection(boolean secure,
         HttpConnection c = secure ? findConnection(key, sslPool)
                                   : findConnection(key, plainPool);
         //System.out.println ("getConnection returning: " + c);
+        assert c == null || c.isSecure() == secure;
         return c;
     }
 
@@ -155,6 +156,10 @@ void returnToPool(HttpConnection conn) {
     // Called also by whitebox tests
     void returnToPool(HttpConnection conn, Instant now, long keepAlive) {
 
+        assert (conn instanceof PlainHttpConnection) || conn.isSecure()
+            : "Attempting to return unsecure connection to SSL pool: "
+                + conn.getClass();
+
         // Don't call registerCleanupTrigger while holding a lock,
         // but register it before the connection is added to the pool,
         // since we don't want to trigger the cleanup if the connection
@@ -450,7 +455,7 @@ private void removeFromPool(HttpConnection c) {
         if (c instanceof PlainHttpConnection) {
             removeFromPool(c, plainPool);
         } else {
-            assert c.isSecure();
+            assert c.isSecure() : "connection " + c + " is not secure!";
             removeFromPool(c, sslPool);
         }
     }
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java b/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -263,7 +263,7 @@ public CompletableFuture<Void> ignoreBody(Executor executor) {
             connection.close();
             return MinimalFuture.completedFuture(null); // not treating as error
         } else {
-            return readBody(discarding(), true, executor);
+            return readBody(discarding(), !request.isWebSocket(), executor);
         }
     }
 
@@ -387,6 +387,14 @@ public void onComplete() {
     public <U> CompletableFuture<U> readBody(HttpResponse.BodySubscriber<U> p,
                                          boolean return2Cache,
                                          Executor executor) {
+        if (debug.on()) {
+            debug.log("readBody: return2Cache: " + return2Cache);
+            if (request.isWebSocket() && return2Cache && connection != null) {
+                debug.log("websocket connection will be returned to cache: "
+                        + connection.getClass() + "/" + connection );
+            }
+        }
+        assert !return2Cache || !request.isWebSocket();
         this.return2Cache = return2Cache;
         final Http1BodySubscriber<U> subscriber = new Http1BodySubscriber<>(p);
 
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/HttpResponseImpl.java b/src/java.net.http/share/classes/jdk/internal/net/http/HttpResponseImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -160,6 +160,26 @@ public synchronized RawChannel rawChannel() throws IOException {
         return rawchan;
     }
 
+    /**
+     * Closes the RawChannel that may have been used for WebSocket protocol.
+     *
+     * @apiNote This method should be called to close the connection
+     * if an exception occurs during the websocket handshake, in cases where
+     * {@link #rawChannel() rawChannel().close()} would have been called.
+     * An unsuccessful handshake may prevent the creation of the RawChannel:
+     * if a RawChannel has already been created, this method wil close it.
+     * Otherwise, it will close the connection.
+     *
+     * @throws IOException if an I/O exception occurs while closing
+     *         the channel.
+     */
+    public synchronized void closeRawChannel() throws IOException {
+        //  close the rawChannel, if created, or the
+        // connection, if not.
+        if (rawchan != null) rawchan.close();
+        else connection.close();
+        }
+
     @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -363,6 +363,10 @@ private CompletableFuture<Response> responseAsyncImpl() {
                             this.response =
                                 new HttpResponseImpl<>(currentreq, response, this.response, null, exch);
                             Exchange<T> oldExch = exch;
+                            if (currentreq.isWebSocket()) {
+                                // need to close the connection and open a new one.
+                                exch.exchImpl.connection().close();
+                            }
                             return exch.ignoreBody().handle((r,t) -> {
                                 previousreq = currentreq;
                                 currentreq = newrequest;
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/RawChannelTube.java b/src/java.net.http/share/classes/jdk/internal/net/http/RawChannelTube.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, 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
@@ -73,7 +73,7 @@ public class RawChannelTube implements RawChannel {
         this.initial = initial;
         this.writePublisher = new WritePublisher();
         this.readSubscriber = new ReadSubscriber();
-        dbgTag = "[WebSocket] RawChannelTube(" + tube.toString() +")";
+        dbgTag = "[WebSocket] RawChannelTube(" + tube +")";
         debug = Utils.getWebSocketLogger(dbgTag::toString, Utils.DEBUG_WS);
         connection.client().webSocketOpen();
         connectFlows();
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/common/Log.java b/src/java.net.http/share/classes/jdk/internal/net/http/common/Log.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -41,7 +41,7 @@
 import javax.net.ssl.SSLParameters;
 
 /**
- * -Djava.net.HttpClient.log=
+ * -Djdk.httpclient.HttpClient.log=
  *          errors,requests,headers,
  *          frames[:control:data:window:all..],content,ssl,trace,channel
  *
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/websocket/OpeningHandshake.java b/src/java.net.http/share/classes/jdk/internal/net/http/websocket/OpeningHandshake.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -215,19 +215,26 @@ private CompletableFuture<Result> resultFrom(HttpResponse<?> response) {
         //
         // See https://tools.ietf.org/html/rfc6455#section-7.4.1
         Result result = null;
-        Exception exception = null;
+        Throwable exception = null;
         try {
             result = handleResponse(response);
         } catch (IOException e) {
             exception = e;
         } catch (Exception e) {
             exception = new WebSocketHandshakeException(response).initCause(e);
+        } catch (Error e) {
+            // We should attempt to close the connection and relay
+            // the error through the completable future even in this
+            // case.
+            exception = e;
         }
         if (exception == null) {
             return MinimalFuture.completedFuture(result);
         }
         try {
-            ((RawChannel.Provider) response).rawChannel().close();
+            // calling this method will close the rawChannel, if created,
+            // or the connection, if not.
+            ((RawChannel.Provider) response).closeRawChannel();
         } catch (IOException e) {
             exception.addSuppressed(e);
         }
diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/websocket/RawChannel.java b/src/java.net.http/share/classes/jdk/internal/net/http/websocket/RawChannel.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -40,6 +40,7 @@ public interface RawChannel extends Closeable {
     interface Provider {
 
         RawChannel rawChannel() throws IOException;
+        void closeRawChannel() throws IOException;
     }
 
     interface RawEvent {
diff --git a/test/jdk/java/net/httpclient/websocket/DummySecureWebSocketServer.java b/test/jdk/java/net/httpclient/websocket/DummySecureWebSocketServer.java
diff --git a/test/jdk/java/net/httpclient/websocket/SecureSupport.java b/test/jdk/java/net/httpclient/websocket/SecureSupport.java
diff --git a/test/jdk/java/net/httpclient/websocket/WebSocketProxyTest.java b/test/jdk/java/net/httpclient/websocket/WebSocketProxyTest.java

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+ * Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`*`
`5`	`5`	`* This code is free software; you can redistribute it and/or modify it`
`@@ -41,7 +41,7 @@`
`41`	`41`	`import javax.net.ssl.SSLParameters;`
`42`	`42`
`43`	`43`	`/**`
`44`		`- * -Djava.net.HttpClient.log=`
	`44`	`+ * -Djdk.httpclient.HttpClient.log=`
`45`	`45`	`* errors,requests,headers,`
`46`	`46`	`* frames[:control:data:window:all..],content,ssl,trace,channel`
`47`	`47`	`*`