Netty ProxyHandler not configured with connect timeout of vertx client

[Adelrisk]

Version

4.5.12

Context

Due to a bad actor in an external network, my team and I have discovered that the configured ProxyHandler (from netty) does not configure the connection timeout when deploying the proxy handler:

The uncalled method setConnectTimeoutMillis: https://netty.io/4.1/api/io/netty/handler/proxy/ProxyHandler.html#setConnectTimeoutMillis-long-

As far as I know, the ProxyHandler is deployed here in ChannelProvider:

vert.x/vertx-core/src/main/java/io/vertx/core/net/impl/ChannelProvider.java

Line 198 in b9d8551

ProxyHandler proxy;

Problem

The bad actor will randomly/sometimes take 10 seconds to accept a HTTP connection. (We measured from our side of the proxy. Only this one server, and the connect takes either milliseconds, or 10 seconds plus milliseconds.)

This is unfortunate, because the default timeout in netty is 10 seconds:

https://github.com/netty/netty/blob/c269a0116d8911ac7179d4ed55108f14fe618cf4/handler-proxy/src/main/java/io/netty/handler/proxy/ProxyHandler.java#L48

This will just as consistently lead to the following exception:

jakarta.ws.rs.ProcessingException: io.netty.handler.proxy.ProxyConnectException: http, basic, /[<IPv6 address>]:7071 => <the bad actor>, timeout
	at org.jboss.resteasy.reactive.client.handlers.ClientSendRequestHandler$2.accept(ClientSendRequestHandler.java:225)
	at org.jboss.resteasy.reactive.client.handlers.ClientSendRequestHandler$2.accept(ClientSendRequestHandler.java:217)
	at io.smallrye.context.impl.wrappers.SlowContextualConsumer.accept(SlowContextualConsumer.java:21)
	at io.smallrye.mutiny.helpers.UniCallbackSubscriber.onFailure(UniCallbackSubscriber.java:62)
	at io.smallrye.mutiny.operators.uni.UniOperatorProcessor.onFailure(UniOperatorProcessor.java:55)
	at org.jboss.resteasy.reactive.client.AsyncResultUni.lambda$subscribe$1(AsyncResultUni.java:37)
	at io.vertx.core.impl.future.FutureImpl$4.onFailure(FutureImpl.java:188)
	at io.vertx.core.impl.future.FutureBase.lambda$emitFailure$1(FutureBase.java:75)
	at io.vertx.core.impl.ContextImpl.execute(ContextImpl.java:312)
	at io.vertx.core.impl.DuplicatedContext.execute(DuplicatedContext.java:168)
	at io.vertx.core.impl.future.FutureBase.emitFailure(FutureBase.java:72)
	at io.vertx.core.impl.future.FutureImpl.tryFail(FutureImpl.java:278)
	at io.vertx.core.http.impl.HttpClientImpl.lambda$doRequest$4(HttpClientImpl.java:398)
	at io.vertx.core.net.impl.pool.Endpoint.lambda$getConnection$0(Endpoint.java:52)
	at io.vertx.core.http.impl.SharedClientHttpStreamEndpoint$Request.handle(SharedClientHttpStreamEndpoint.java:162)
	at io.vertx.core.http.impl.SharedClientHttpStreamEndpoint$Request.handle(SharedClientHttpStreamEndpoint.java:123)
	at io.vertx.core.impl.ContextImpl.emit(ContextImpl.java:342)
	at io.vertx.core.impl.ContextImpl.emit(ContextImpl.java:335)
	at io.vertx.core.net.impl.pool.SimpleConnectionPool$ConnectFailed$1.run(SimpleConnectionPool.java:380)
	at io.vertx.core.net.impl.pool.Task.runNextTasks(Task.java:43)
	at io.vertx.core.net.impl.pool.CombinerExecutor.submit(CombinerExecutor.java:91)
	at io.vertx.core.net.impl.pool.SimpleConnectionPool.execute(SimpleConnectionPool.java:244)
	at io.vertx.core.net.impl.pool.SimpleConnectionPool.lambda$connect$2(SimpleConnectionPool.java:258)
	at io.vertx.core.http.impl.SharedClientHttpStreamEndpoint.lambda$connect$2(SharedClientHttpStreamEndpoint.java:104)
	at io.vertx.core.impl.future.FutureImpl$4.onFailure(FutureImpl.java:188)
	at io.vertx.core.impl.future.FutureBase.emitFailure(FutureBase.java:81)
	at io.vertx.core.impl.future.FutureImpl.tryFail(FutureImpl.java:278)
	at io.vertx.core.impl.future.Composition$1.onFailure(Composition.java:66)
	at io.vertx.core.impl.future.FutureBase.emitFailure(FutureBase.java:81)
	at io.vertx.core.impl.future.FailedFuture.addListener(FailedFuture.java:98)
	at io.vertx.core.impl.future.Composition.onFailure(Composition.java:55)
	at io.vertx.core.impl.future.FutureBase.emitFailure(FutureBase.java:81)
	at io.vertx.core.impl.future.FutureImpl.tryFail(FutureImpl.java:278)
	at io.vertx.core.impl.ContextImpl.emit(ContextImpl.java:342)
	at io.vertx.core.impl.ContextImpl.emit(ContextImpl.java:335)
	at io.vertx.core.net.impl.NetClientImpl.failed(NetClientImpl.java:351)
	at io.vertx.core.net.impl.NetClientImpl.lambda$connectInternal2$6(NetClientImpl.java:323)
	at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:603)
	at io.netty.util.concurrent.DefaultPromise.notifyListenersNow(DefaultPromise.java:570)
	at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:505)
	at io.netty.util.concurrent.DefaultPromise.setValue0(DefaultPromise.java:649)
	at io.netty.util.concurrent.DefaultPromise.setFailure0(DefaultPromise.java:642)
	at io.netty.util.concurrent.DefaultPromise.setFailure(DefaultPromise.java:123)
	at io.vertx.core.net.impl.ChannelProvider$3$1.exceptionCaught(ChannelProvider.java:235)
	at io.netty.channel.AbstractChannelHandlerContext.invokeExceptionCaught(AbstractChannelHandlerContext.java:346)
	at io.netty.channel.AbstractChannelHandlerContext.invokeExceptionCaught(AbstractChannelHandlerContext.java:325)
	at io.netty.channel.AbstractChannelHandlerContext.fireExceptionCaught(AbstractChannelHandlerContext.java:317)
	at io.netty.handler.proxy.ProxyHandler.failPendingWritesAndClose(ProxyHandler.java:356)
	at io.netty.handler.proxy.ProxyHandler.setConnectFailure(ProxyHandler.java:349)
	at io.netty.handler.proxy.ProxyHandler.access$100(ProxyHandler.java:41)
	at io.netty.handler.proxy.ProxyHandler$2.run(ProxyHandler.java:201)
	at io.netty.util.concurrent.PromiseTask.runTask(PromiseTask.java:98)
	at io.netty.util.concurrent.ScheduledFutureTask.run(ScheduledFutureTask.java:156)
	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:173)
	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:166)
	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:472)
	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:569)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: io.netty.handler.proxy.ProxyConnectException: http, basic, /[<IPv6 address>]:7071 => <the bad actor>, timeout
	... 11 more

The quarkus default timeout of 15 seconds does not play a role, nor does increasing this value to 30 seconds.

https://quarkus.io/guides/rest-client#quarkus-rest-client-config_quarkus-rest-client-connect-timeout

We believe that this issue could be resolved by respecting and using the connect timeout value of the client.

Steps to reproduce

No response

Do you have a reproducer?

No response

[tsegismont]

Looking into it

[tsegismont]

The ProxyHandler throws ProxyConnectException if the connection to the proxy was not marked as successful before connectTimeout expires.

Successful means slightly different things, depending on the proxy type:

• for an HTTP proxy, success means the LastHttpContent message of the proxy response to the CONNECT request has been received, and the status code is 200
• for a SOCKS proxy, success means the SOCKS handshake ended with the SUCCESS status.

To my knowledge, this could happen if:

1. the target server fails to transmit the SYN ACK packet before the timeout fires, or
2. the proxy itself is slow to respond

It would be difficult for us to simulate 1/ in the test suite. Simulating 2/ is much simpler, especially for an HTTP proxy.

The question is now: what kind of timeout should we use to configure the proxy handler.

We could use the connect timeout (socket option) configured in the http client options. Or we could introduce a new field in ProxyOptions.

My preference goes to adding a new field to ProxyOptions because it is specific to this problem.
@vietj any thoughts?

[vietj]

I'm keen also on ProxyOptions for the sake of clarity

[tsegismont]

@Adelrisk @vietj just shared PR #5650

@Adelrisk have you talked about this Quarkus folks or filed an issue in the Quarkus repo? When the PR is merged, I'll backport the fix to the 4.x, but we need the Quarkus REST Client to be aware of the new option.

[Adelrisk]

Not yet. Sounds good, I can open an issue later today if you like.

[tsegismont]

@Adelrisk yes, please go ahead