Skip to content

feat(grpc): Add resolver wrapper to handle HTTP CONNECT proxy configuration#2679

Merged
arjan-bal merged 27 commits into
grpc:masterfrom
arjan-bal:http-proxy-resolver
Jul 3, 2026
Merged

feat(grpc): Add resolver wrapper to handle HTTP CONNECT proxy configuration#2679
arjan-bal merged 27 commits into
grpc:masterfrom
arjan-bal:http-proxy-resolver

Conversation

@arjan-bal

@arjan-bal arjan-bal commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

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 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
Transport changes: #2698

@arjan-bal

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the gRPC client to support arbitrary byte-based addresses (via ByteStr) instead of requiring valid UTF-8 strings, enabling support for non-UTF-8 unix socket paths. It also introduces an HTTP CONNECT proxy resolver that intercepts DNS resolution and applies proxy configurations based on environment variables. The review feedback suggests bypassing proxy resolution entirely for non-DNS schemes (like unix or inmemory), safely handling empty NO_PROXY environment variables, and updating tests to verify that unix paths correctly bypass the proxy.

Comment thread grpc/src/client/name_resolution/proxy_resolver.rs
Comment thread grpc/src/client/name_resolution/proxy_resolver.rs
Comment thread grpc/src/client/name_resolution/proxy_resolver.rs
options: ResolverOptions,
matcher: Option<&Matcher>,
) -> Result<Box<dyn Resolver>, (String, ResolverOptions)> {
// Skip proxy lookup for known non-TCP schemes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd expect to skip it for anything that isn't dns. I think there had been a little discussion around that, but I don't know where it ended.

The host will be passed to the proxy and the proxy then has to do the resolution, but we've discarded the scheme. It seems we are assuming dns formatting of the target string as well. We shouldn't be parsing the target string without knowledge of the specific scheme in use. Things would be better if we delegated to the name resolver to parse the target string into an authority.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answering inline:

I'd expect to skip it for anything that isn't dns. I think there had been a little discussion around that, but I don't know where it ended.

The initial intention behind implementing proxy resolution as a separate resolver builder was to allow all custom resolvers to get HTTP CONNECT support for free. For context, the C++ implementation skips proxying for unix URLs, while in Go, target host resolution can only be skipped for dns URLs. Java simply implements the proxy logic inside the DNS resolver itself.

I've now updated the Rust implementation to skip proxying for all non-dns URLs.

Things would be better if we delegated to the name resolver to parse the target string into an authority.

I've updated the code to use the target resolver builder for authority parsing. Based on the previous point, this will inherently be a DNS resolver.

@arjan-bal arjan-bal assigned ejona86 and unassigned arjan-bal Jun 22, 2026
@arjan-bal arjan-bal requested a review from ejona86 June 23, 2026 06:24
Comment on lines +33 to +34
use super::ResolverOptions;
use super::Target;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: please try to avoid use super except in test mods.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed and noted.

fn build(&self, target: &Target, options: ResolverOptions) -> Box<dyn Resolver> {
match self.new_resolver(target, options, MATCHER.as_ref()) {
Ok(resolver) => resolver,
Err((err, options)) => NopResolver::new_with_err(err, options),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: consider making new_resolver infallible and return the NopResolver itself instead. It avoids needing to pass the options back and forth.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

struct HttpsProxyResolver {
child: Box<dyn Resolver>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly this is always a dns::DnsResolver. Should we put that type in here directly instead of a dyn?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DNS resolver builder returns a NopResolver if the target is an IP address here:

Host::Ipv4(ipv4) => {
return nop_resolver_for_ip(IpAddr::V4(ipv4), endpoint.port, options);
}
Host::Ipv6(ipv6) => {
return nop_resolver_for_ip(IpAddr::V6(ipv6), endpoint.port, options);
}

To return a single concrete type from dns::Builder, we could introduce a dns::Resolver enum with NopResolver and DnsResolver variants and expose it via a pub(crate) method.

I'm wondering if this refactoring provides any real benefit. Since HttpsProxyResolver only requires the methods already defined on the Resolver trait, staying with the trait might be cleaner. What do you think?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this refactoring provides any real benefit.

No, I think this is fine. If this was always one type I think it would make sense to just name it and have it there, but since it's more, then this way is better.

/// Returns the address of the proxy server to connect to (host:port).
/// This is Punycode-encoded, i.e., it's a valid URL host:port.
pub(crate) fn target_authority(&self) -> &str {
&self.connect_addr

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better if the names matches?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed both, the field and the getter to connect_authority.

if let Ok(endpoints) = &mut update.endpoints {
for endpoint in endpoints {
for address in &mut endpoint.addresses {
address.attributes = address.attributes.add(self.proxy_options.clone());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the attribute should be somewhere more central, like in grpc/client/transport maybe?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the ProxyOptions and its helpers into the transport module.

@dfawley dfawley assigned arjan-bal and unassigned dfawley Jun 30, 2026
@arjan-bal arjan-bal force-pushed the http-proxy-resolver branch from 5395e05 to b2f47f5 Compare July 1, 2026 10:21
@arjan-bal arjan-bal assigned dfawley and unassigned ejona86 and arjan-bal Jul 1, 2026
Comment thread grpc/src/client/transport/mod.rs Outdated
pub(crate) handshake_info: ClientHandshakeInfo,
}

/// Options for establishing an HTTP CONNECT proxy tunnel.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe mention that this can be added to addresses' attributes, and transports that support it will read it from there and use the address as a CONNECT proxy instead of a backend.

What about transports that don't support it, though? Should those error? (It seems like they should...) We can deal with this later, but something to think about. Maybe we should have a separate address type for a proxy address or something.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the latest revision of the design, the subchannel will wrap the channel credentials in an HTTP CONNECT handshaker, making it transparent to transports. I've added this information in the rustdoc.

/// Must be a valid hostname.
/// * `proxy_authorization_header` - The value of the `Proxy-Authorization` header, if present.
pub(crate) fn new(
target_authority: String,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

connect_authority?

I'm not sure what the best name to use for this is (consider also target_address or backend_address maybe?), but it should match between field, constructor, and accessor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to target_authority throughout.

@arjan-bal arjan-bal assigned arjan-bal and unassigned dfawley Jul 3, 2026
@arjan-bal arjan-bal merged commit b68df9f into grpc:master Jul 3, 2026
26 checks passed
@arjan-bal arjan-bal deleted the http-proxy-resolver branch July 3, 2026 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants