grpc/client: HTTP CONNECT proxy support in transport#2698
Conversation
This reverts commit 0428123.
…ration (#2679) This PR implements a resolver wrapper that determines proxy routing by checking the `HTTPS_PROXY` and `NO_PROXY` environment variables. By default, `hyper-util` matches cURL's behavior by falling back to `ALL_PROXY` when `HTTPS_PROXY` is unset. However, because Go's [`FromEnvironment`](https://pkg.go.dev/golang.org/x/net/http/httpproxy#FromEnvironment) and gRPC C++ ignore `ALL_PROXY`, gRPC Rust manually configures the `Matcher` to bypass it, ensuring cross-language consistency. **Resolution Flow** When `HttpsProxyResolver::Builder` builds a resolver for a target URI, it uses `hyper_util::client::proxy::Matcher` to evaluate the environment variables: 1. **Direct Connection:** If no proxy is required, it delegates resolution entirely to the wrapped child builder, bypassing the proxy resolver. 2. **Proxied Connection:** If a proxy is required, it creates an `HttpsProxyResolver`, which uses the child DNS resolver to resolve the *proxy server's* hostname instead of the target. 3. **Attribute Injection:** The `HttpsProxyResolver` intercepts resolution updates from the child resolver. It attaches the necessary proxy configuration (the original target authority and basic auth credentials) as attributes to each resolved proxy address. In follow-up PRs, the subchannel will read these address attributes to wrap the channel credentials and carry out the HTTP `CONNECT` handshake prior to the standard credential handshake. Internal design doc: [go/grpc-rust-http-connect](http://go/grpc-rust-http-connect) Transport changes: #2698
1b5c061 to
bbde691
Compare
dfawley
left a comment
There was a problem hiding this comment.
I still need to look at the tests, but here's a first round of comments (all pretty minor I think).
| // TODO(nathanielford): Return errors here instead of panicking. | ||
| let target = Target::from_str(&target.into()).unwrap(); | ||
| let resolver_builder = global_registry().get(target.scheme()).unwrap(); | ||
| let resolver_builder = Arc::new(proxy_resolver::Builder::new(resolver_builder)); |
There was a problem hiding this comment.
Wondering if we should have a proxy_resolver::Builder::new_arc instead that returns child_builder directly if child_builder.scheme() != "dns"?
| ) -> Arc<dyn Subchannel> { | ||
| let on_drop = Arc::new(Notify::new()); | ||
| let address_string = address.address.to_string(); | ||
| let transport_options = TransportOptions::default(); |
There was a problem hiding this comment.
This is unused -- remove or change line 335 to use it (and transfer that comment here)?
|
|
||
| /// A wrapper around an asynchronous I/O stream that implements | ||
| /// [GrpcEndpoint]. | ||
| pub(crate) struct TokioIoStream<T> { |
There was a problem hiding this comment.
Should this still mention Tokio if it's not tokio-specific?
| // Note: This file contains modifications by the gRPC authors; see revision | ||
| // history for details. |
There was a problem hiding this comment.
I don't think the revision history is going to help here, is it? Because our first version will have the edits.
| } | ||
|
|
||
| impl HttpConnectHandshaker { | ||
| /// Constructs a new `ProxyChannelCredentials` wrapping the inner credentials. |
There was a problem hiding this comment.
ProxyChannelCredentials -> HttpConnectHandshaker?
This PR introduces an
HttpConnectHandshakerthat wraps theChannelCredentialsto complete the HTTP CONNECT handshake based on theAddressattributes introduced in #2679. This allows the subchannel to configure proxying transparently without requiring changes to the Transport API.