From e643833d363bc958b002521cf24e70e5a01b5ea6 Mon Sep 17 00:00:00 2001 From: ArturoSalazarB16 Date: Mon, 28 Sep 2026 18:34:07 -0700 Subject: [PATCH 1/5] okhttp: Set socket read timeout during TLS handshake In `OkHttpClientTransport`, direct TLS connection setup calls `OkHttpTlsUpgrader.upgrade(...)` without configuring a socket read timeout (`setSoTimeout`). When a middlebox or unresponsive server completes the TCP three-way handshake and acknowledges the TLS `ClientHello` without sending a `ServerHello` or `RST`, `sslSocket.startHandshake()` blocks indefinitely in socket read. Because `this.socket` and `asyncSink` are only assigned after `OkHttpTlsUpgrader.upgrade(...)` returns, channel shutdown cannot close the underlying socket either, leaving the subchannel permanently stuck in `CONNECTING`. Configure `sock.setSoTimeout(proxySocketTimeout)` before `OkHttpTlsUpgrader.upgrade(...)` and reset it to `0` once the TLS upgrade completes, matching the timeout handling in `createHttpProxySocket`. Also close `sock` via `GrpcUtil.closeQuietly(sock)` in `catch (Exception e)` so the socket is not leaked when handshake setup fails before `asyncSink.becomeConnected(...)` takes ownership. --- .../io/grpc/okhttp/OkHttpClientTransport.java | 3 ++ .../okhttp/OkHttpClientTransportTest.java | 29 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java index 4764a6a1387..02b45dc66e9 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java @@ -739,9 +739,12 @@ public void close() { } } if (sslSocketFactory != null) { + sock.setSoTimeout(proxySocketTimeout); SSLSocket sslSocket = OkHttpTlsUpgrader.upgrade( sslSocketFactory, hostnameVerifier, sock, getOverridenHost(), getOverridenPort(), connectionSpec); + // As the socket will be used for RPCs from here on, we want the socket timeout back to zero. + sock.setSoTimeout(0); sslSession = sslSocket.getSession(); sock = sslSocket; } diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java index 0b571530db4..d0d3f29a182 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java @@ -101,6 +101,7 @@ import java.net.ServerSocket; import java.net.Socket; import java.net.SocketAddress; +import java.net.SocketTimeoutException; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; @@ -2048,6 +2049,34 @@ public void proxy_serverHangs() throws Exception { sock.close(); } + @Test + public void tls_serverHangs() throws Exception { + ServerSocket serverSocket = new ServerSocket(0); + clientTransport = + new OkHttpClientTransport( + channelBuilder.useTransportSecurity().buildTransportFactory(), + new InetSocketAddress("localhost", serverSocket.getLocalPort()), + "authority", + "userAgent", + EAG_ATTRS, + NO_PROXY, + tooManyPingsRunnable, + null); + clientTransport.proxySocketTimeout = 10; + clientTransport.start(transportListener); + + Socket sock = serverSocket.accept(); + serverSocket.close(); + + ArgumentCaptor statusCaptor = ArgumentCaptor.forClass(Status.class); + verify(transportListener, timeout(200)) + .transportShutdown(statusCaptor.capture(), any(DisconnectError.class)); + verify(transportListener, timeout(TIME_OUT_MS)).transportTerminated(); + assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.UNAVAILABLE); + assertThat(statusCaptor.getValue().getCause()).isInstanceOf(SocketTimeoutException.class); + sock.close(); + } + @Test public void goAway_notUtf8() throws Exception { initTransport(); From 14ed009055ac86de52e5ad8a0498fedb04e636d5 Mon Sep 17 00:00:00 2001 From: ArturoSalazarB16 Date: Mon, 28 Sep 2026 18:45:47 -0700 Subject: [PATCH 2/5] Update OkHttpClientTransport.java --- okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java | 1 + 1 file changed, 1 insertion(+) diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java index 02b45dc66e9..e12a9adb8d2 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java @@ -764,6 +764,7 @@ sslSocketFactory, hostnameVerifier, sock, getOverridenHost(), getOverridenPort() startGoAway(0, ErrorCode.INTERNAL_ERROR, e.getStatus()); return; } catch (Exception e) { + GrpcUtil.closeQuietly(sock); onException(e); return; } finally { From 8c8e299fa07ee0c1d9103337c874496e4c9eff19 Mon Sep 17 00:00:00 2001 From: ArturoSalazarB16 Date: Mon, 28 Sep 2026 19:38:57 -0700 Subject: [PATCH 3/5] Update OkHttpClientTransport.java --- okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java index e12a9adb8d2..0464cb46efd 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java @@ -743,7 +743,8 @@ public void close() { SSLSocket sslSocket = OkHttpTlsUpgrader.upgrade( sslSocketFactory, hostnameVerifier, sock, getOverridenHost(), getOverridenPort(), connectionSpec); - // As the socket will be used for RPCs from here on, we want the socket timeout back to zero. + // As the socket will be used for RPCs from here on, we want the socket + // timeout back to zero. sock.setSoTimeout(0); sslSession = sslSocket.getSession(); sock = sslSocket; From ccecccca2360e270ebf501269dcbc1988ccbbf33 Mon Sep 17 00:00:00 2001 From: ArturoSalazarB16 Date: Tue, 29 Sep 2026 08:47:23 -0700 Subject: [PATCH 4/5] Update OkHttpClientTransportTest.java --- .../test/java/io/grpc/okhttp/OkHttpClientTransportTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java index d0d3f29a182..db7fec19e54 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java @@ -2073,7 +2073,8 @@ public void tls_serverHangs() throws Exception { .transportShutdown(statusCaptor.capture(), any(DisconnectError.class)); verify(transportListener, timeout(TIME_OUT_MS)).transportTerminated(); assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.UNAVAILABLE); - assertThat(statusCaptor.getValue().getCause()).isInstanceOf(SocketTimeoutException.class); + assertThat(Throwables.getRootCause(statusCaptor.getValue().getCause())) + .isInstanceOf(SocketTimeoutException.class); sock.close(); } From 626526b6d35bf41936d3531b459a6db51e229ca7 Mon Sep 17 00:00:00 2001 From: ArturoSalazarB16 Date: Tue, 29 Sep 2026 09:13:10 -0700 Subject: [PATCH 5/5] Update OkHttpClientTransportTest.java --- .../src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java index db7fec19e54..ec459332156 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java @@ -49,6 +49,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; import com.google.common.base.Supplier; +import com.google.common.base.Throwables; import com.google.common.base.Ticker; import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.Futures;