Repository navigation
feat: add optional TLS handshake timeout for CONNECT-tunneled HTTPS - #9
Merged
skreborn merged 2 commits intoAug 12, 2026
Merged
Conversation
ProxyConnector performs its own TLS handshake over the CONNECT tunnel for HTTPS destinations, with no way to bound it. A stalled TLS handshake with a proxied target could hang a request indefinitely. Add `ProxyConnector::set_tls_handshake_timeout` to bound that handshake, off by default to preserve existing behavior.
Merged
1 task done
skreborn
requested changes
Aug 12, 2026
skreborn
left a comment
skreborn
left a comment
Contributor
There was a problem hiding this comment.
Welcome, and thank you for your contribution!
It would be great if you could change a few things to increase test coverage and get rid of some warnings.
To exercise the full matrix of features, you can run a couple of test rounds with different backends enabled.
# This one correctly excludes the new test but raises warnings
cargo test --no-default-features
# These two skip the new test but emit no warnings
cargo test --no-default-features --features native-tls
cargo test --no-default-features --features native-tls-vendored
# These two correctly run all tests and emit no warnings
cargo test --no-default-features --features rustls-tls-native-roots
cargo test --no-default-features --features rustls-tls-webpki-rootsLooks good otherwise, and we're happy to have this merged in once it's fixed up.
Gate the timeout plumbing and test on the shared __tls feature instead of __rustls, since none of it is rustls-specific, and use io::ErrorKind::TimedOut for the timeout error for precision. Feature- gate tokio's time feature behind __tls via Cargo feature unification instead of enabling it unconditionally.
Contributor
Author
|
@skreborn thanks for the quick review and guidance! I made the suggested changes and tried out |
Contributor
|
Thanks for taking the time! Released as 1.2.0, so you can go on ahead and start using it right away. |
jszwedko
added a commit
to DataDog/saluki
that referenced
this pull request
Aug 12, 2026
Points Cargo.lock at the fork commit that addresses upstream review feedback on the TLS handshake timeout PR (metalbear-co/hyper-http-proxy#9).
Contributor
Author
|
Awesome, thank you @skreborn ! |
jszwedko
added a commit
to DataDog/saluki
that referenced
this pull request
Aug 12, 2026
metalbear-co/hyper-http-proxy#9 merged and released as 1.2.0, so the fork patch is no longer needed.
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
to DataDog/saluki
that referenced
this pull request
Aug 12, 2026
…gh a proxy (#2309) ## Human Summary Use the upstream `hyper-http-proxy` crate, which now [supports setting a timeout on TLS handshakes](metalbear-co/hyper-http-proxy#9). This started out as a fork while the change was upstream, but that PR has since merged and been released as 1.2.0, so we no longer need it. ## AI Summary When a proxy is configured, `HttpClient`'s TLS handshake timeout only bounds direct connections — `hyper_http_proxy::ProxyConnector` performs a separate TLS handshake over the CONNECT tunnel to reach HTTPS destinations, and that handshake was previously unbounded, so a stalled or unresponsive proxy could hang a request indefinitely. This adds a `set_tls_handshake_timeout` API to `hyper-http-proxy` (contributed upstream via [metalbear-co/hyper-http-proxy#9](metalbear-co/hyper-http-proxy#9), merged and released as 1.2.0) and wires it up using the same handshake timeout already configured for direct connections. ## Test plan - [x] Added `tls_handshake_timeout_fires_against_a_stalled_proxy_tunnel`, which emulates a proxy that completes the CONNECT tunnel but never speaks TLS, and asserts the client's configured handshake timeout ends the request. Co-authored-by: jesse.szwedko <jesse.szwedko@datadoghq.com>
dd-octo-sts Bot
pushed a commit
to DataDog/saluki
that referenced
this pull request
Aug 12, 2026
…gh a proxy (#2309) ## Human Summary Use the upstream `hyper-http-proxy` crate, which now [supports setting a timeout on TLS handshakes](metalbear-co/hyper-http-proxy#9). This started out as a fork while the change was upstream, but that PR has since merged and been released as 1.2.0, so we no longer need it. ## AI Summary When a proxy is configured, `HttpClient`'s TLS handshake timeout only bounds direct connections — `hyper_http_proxy::ProxyConnector` performs a separate TLS handshake over the CONNECT tunnel to reach HTTPS destinations, and that handshake was previously unbounded, so a stalled or unresponsive proxy could hang a request indefinitely. This adds a `set_tls_handshake_timeout` API to `hyper-http-proxy` (contributed upstream via [metalbear-co/hyper-http-proxy#9](metalbear-co/hyper-http-proxy#9), merged and released as 1.2.0) and wires it up using the same handshake timeout already configured for direct connections. ## Test plan - [x] Added `tls_handshake_timeout_fires_against_a_stalled_proxy_tunnel`, which emulates a proxy that completes the CONNECT tunnel but never speaks TLS, and asserts the client's configured handshake timeout ends the request. Co-authored-by: jesse.szwedko <jesse.szwedko@datadoghq.com> f76d1fb
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey!
In DataDog/saluki#178 we have a desire to implement a timeout just for TLS handshake. We've implemented it when a proxy is not in use (see how rustls documents support in rustls/tokio-rustls#187), but we leverage this crate for proxy support and we'd like to support the handshake timeout in this case as well.
The default is no handshake-level timeout, which matches current behavior.