diff --git a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt index e90421ccdc44..af0c6d9102db 100644 --- a/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt +++ b/okhttp/src/commonJvmAndroid/kotlin/okhttp3/internal/connection/ConnectPlan.kt @@ -416,6 +416,10 @@ class ConnectPlan internal constructor( /** * To make an HTTPS connection over an HTTP proxy, send an unencrypted CONNECT request to create * the proxy connection. This may need to be retried if the proxy requires authorization. + * + * Retries on the same connection (when the proxy does not send `Connection: close`) share the + * same [MAX_TUNNEL_ATTEMPTS] budget as reconnects after `Connection: close`, so a misbehaving + * proxy cannot trap this loop indefinitely. See https://github.com/square/okhttp/issues/9477. */ @Throws(IOException::class) private fun createTunnel(): Request? { @@ -423,6 +427,8 @@ class ConnectPlan internal constructor( // Make an SSL Tunnel on the first message pair of each SSL + proxy connection. val url = route.address.url val requestLine = "CONNECT ${url.toHostHeader(includeDefaultPort = true)} HTTP/1.1" + // Count prior reconnects so keep-alive and Connection: close retries share one budget. + var tunnelAttempts = attempt while (true) { val tunnelCodec = Http1ExchangeCodec( @@ -454,6 +460,13 @@ class ConnectPlan internal constructor( if ("close".equals(response.header("Connection"), ignoreCase = true)) { return nextRequest } + + tunnelAttempts++ + if (tunnelAttempts >= MAX_TUNNEL_ATTEMPTS) { + throw ProtocolException( + "Too many tunnel connections attempted: $MAX_TUNNEL_ATTEMPTS", + ) + } } else -> { diff --git a/okhttp/src/jvmTest/kotlin/okhttp3/CallTest.kt b/okhttp/src/jvmTest/kotlin/okhttp3/CallTest.kt index 35c86efecd63..2c6227183b0f 100644 --- a/okhttp/src/jvmTest/kotlin/okhttp3/CallTest.kt +++ b/okhttp/src/jvmTest/kotlin/okhttp3/CallTest.kt @@ -3870,7 +3870,85 @@ open class CallTest { val request = Request("https://android.com/foo".toHttpUrl()) assertFailsWith { client.newCall(request).execute() + }.also { expected -> + assertThat(expected).hasMessage("Too many tunnel connections attempted: 21") + } + } + + /** + * A proxy that keeps the connection open and always returns 407 must not trap OkHttp in an + * infinite loop. https://github.com/square/okhttp/issues/9477 + */ + @Test + fun tooManyProxyAuthFailuresWithoutConnectionClose() { + server.useHttps(handshakeCertificates.sslSocketFactory()) + server.protocols = listOf(Protocol.HTTP_1_1) + for (i in 0..20) { + server.enqueue( + MockResponse + .Builder() + .code(407) + .headers(headersOf("Proxy-Authenticate", "Basic realm=\"localhost\"")) + .inTunnel() + .build(), + ) } + val authenticator = RecordingOkAuthenticator("password", "Basic") + client = + client + .newBuilder() + .sslSocketFactory( + handshakeCertificates.sslSocketFactory(), + handshakeCertificates.trustManager, + ).proxy(server.proxyAddress) + .proxyAuthenticator(authenticator) + .hostnameVerifier(RecordingHostnameVerifier()) + .build() + val request = Request("https://android.com/foo".toHttpUrl()) + assertFailsWith { + client.newCall(request).execute() + }.also { expected -> + assertThat(expected).hasMessage("Too many tunnel connections attempted: 21") + } + // Preemptive challenge + 21 reactive 407s, but only 21 CONNECT attempts are allowed. + assertThat(authenticator.responses.size).isEqualTo(22) + } + + /** Confirm we still succeed after several keep-alive proxy auth challenges (under the limit). */ + @Test + fun proxyAuthenticateOnConnectSucceedsAfterManyChallenges() { + server.useHttps(handshakeCertificates.sslSocketFactory()) + server.protocols = listOf(Protocol.HTTP_1_1) + for (i in 0..19) { + server.enqueue( + MockResponse + .Builder() + .code(407) + .headers(headersOf("Proxy-Authenticate", "Basic realm=\"localhost\"")) + .inTunnel() + .build(), + ) + } + server.enqueue( + MockResponse + .Builder() + .inTunnel() + .build(), + ) + server.enqueue(MockResponse(body = "response body")) + client = + client + .newBuilder() + .sslSocketFactory( + handshakeCertificates.sslSocketFactory(), + handshakeCertificates.trustManager, + ).proxy(server.proxyAddress) + .proxyAuthenticator(RecordingOkAuthenticator("password", "Basic")) + .hostnameVerifier(RecordingHostnameVerifier()) + .build() + val request = Request("https://android.com/foo".toHttpUrl()) + val response = client.newCall(request).execute() + assertThat(response.body.string()).isEqualTo("response body") } /**