Skip to content
Permalink
Browse files
8236859: WebSocket over authenticating proxy fails with NPE
This change fixes several issues with WebSocket and proxy authentication. The AuthenticationFilter is changed to support an authenticating server accessed through an authenticating proxy. MultiExchange is fixed to close the previous connection if a new connection is necessary to establish the websocket (websocket connections are not cached and must be closed in that case). WebSocket OpeningHandshake is fixed to close the connection (without creating the RawChannel) if the opening handshake doesn't result in 101 upgrade protocol.

Reviewed-by: yan
Backport-of: c6da668
  • Loading branch information
Larry-N authored and Yuri Nesterenko committed Dec 23, 2020
1 parent cc78d10 commit 5d25a8ffa9d1dbcdc839a16898ab7ce51a1e0bd5
@@ -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
@@ -241,20 +241,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);
}
}
return null;
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;
@@ -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);
}
}
@@ -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
@@ -264,7 +264,7 @@ long fixupContentLen(long clen) {
connection.close();
return MinimalFuture.completedFuture(null); // not treating as error
} else {
return readBody(discarding(), true, executor);
return readBody(discarding(), !request.isWebSocket(), executor);
}
}

@@ -394,6 +394,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);

@@ -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
@@ -131,6 +131,30 @@ public synchronized RawChannel rawChannel() throws IOException {
return rawChannelProvider.rawChannel();
}

/**
* 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 UnsupportedOperationException if getting a RawChannel over
* this connection is not supported.
* @throws IOException if an I/O exception occurs while closing
* the channel.
*/
@Override
public synchronized void closeRawChannel() throws IOException {
if (rawChannelProvider == null) {
throw new UnsupportedOperationException(
"RawChannel is only supported for WebSocket creation");
}
rawChannelProvider.closeRawChannel();
}

@Override
public String toString() {
StringBuilder sb = new StringBuilder();
@@ -189,6 +213,13 @@ public synchronized RawChannel rawChannel() {
return rawchan;
}

public synchronized void closeRawChannel() throws IOException {
// close the rawChannel, if created, or the
// connection, if not.
if (rawchan != null) rawchan.close();
else connection.close();
}

private static HttpConnection connection(Response resp, Exchange<?> exch) {
if (exch == null || exch.exchImpl == null) {
assert resp.statusCode == 407;
@@ -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 @@ public void cancel() {
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;
@@ -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 @@
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();
@@ -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
*
@@ -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 Result(String subprotocol, TransportFactory transport) {
//
// 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);
}
@@ -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 @@
interface Provider {

RawChannel rawChannel() throws IOException;
void closeRawChannel() throws IOException;
}

interface RawEvent {

1 comment on commit 5d25a8f

@openjdk-notifier

This comment has been minimized.

Copy link

@openjdk-notifier openjdk-notifier bot commented on 5d25a8f Dec 23, 2020

Please sign in to comment.