From 054f074331fe22f06bd6cf7d018e6ec87bb2cee6 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Fri, 5 Jun 2026 00:21:16 +0530 Subject: [PATCH 01/21] allow non-utf8 addresses in Address struct --- grpc/src/byte_str.rs | 61 ++++++++++++++++--- .../client/load_balancing/graceful_switch.rs | 8 +-- grpc/src/client/load_balancing/pick_first.rs | 55 +++++++---------- grpc/src/client/load_balancing/round_robin.rs | 8 +-- grpc/src/client/name_resolution/mod.rs | 15 +++-- grpc/src/client/subchannel.rs | 5 +- grpc/src/client/transport/mod.rs | 7 ++- grpc/src/client/transport/tonic/mod.rs | 17 ++++-- grpc/src/client/transport/tonic/test.rs | 6 +- grpc/src/inmemory/mod.rs | 13 ++-- 10 files changed, 122 insertions(+), 73 deletions(-) diff --git a/grpc/src/byte_str.rs b/grpc/src/byte_str.rs index 971d3587c..1d6c734d9 100644 --- a/grpc/src/byte_str.rs +++ b/grpc/src/byte_str.rs @@ -21,28 +21,24 @@ * IN THE SOFTWARE. * */ - -use core::str; use std::ops::Deref; use bytes::Bytes; /// A cheaply cloneable and sliceable chunk of contiguous memory. +/// +/// The bytes held by `ByteStr` are arbitrary and may not be valid UTF-8. #[derive(Debug, Default, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct ByteStr { - // Invariant: bytes contains valid UTF-8 bytes: Bytes, } impl Deref for ByteStr { - type Target = str; + type Target = [u8]; #[inline] - fn deref(&self) -> &str { - let b: &[u8] = self.bytes.as_ref(); - // The invariant of `bytes` is that it contains valid UTF-8 allows us - // to unwrap. - str::from_utf8(b).unwrap() + fn deref(&self) -> &[u8] { + &self.bytes } } @@ -55,3 +51,50 @@ impl From for ByteStr { } } } + +impl<'a> TryFrom<&'a ByteStr> for &'a str { + type Error = std::str::Utf8Error; + + #[inline] + fn try_from(value: &'a ByteStr) -> Result { + std::str::from_utf8(value) + } +} + +impl TryFrom for String { + type Error = std::str::Utf8Error; + + #[inline] + fn try_from(value: ByteStr) -> Result { + let s = std::str::from_utf8(&value)?; + Ok(s.to_owned()) + } +} + +impl PartialEq for ByteStr { + #[inline] + fn eq(&self, other: &str) -> bool { + self.bytes == other.as_bytes() + } +} + +impl<'a> PartialEq<&'a str> for ByteStr { + #[inline] + fn eq(&self, other: &&'a str) -> bool { + self.bytes == other.as_bytes() + } +} + +impl PartialEq for str { + #[inline] + fn eq(&self, other: &ByteStr) -> bool { + self.as_bytes() == other.bytes + } +} + +impl<'a> PartialEq for &'a str { + #[inline] + fn eq(&self, other: &ByteStr) -> bool { + self.as_bytes() == other.bytes + } +} diff --git a/grpc/src/client/load_balancing/graceful_switch.rs b/grpc/src/client/load_balancing/graceful_switch.rs index 70bc784f3..444263602 100644 --- a/grpc/src/client/load_balancing/graceful_switch.rs +++ b/grpc/src/client/load_balancing/graceful_switch.rs @@ -451,12 +451,10 @@ mod test { let PickResult::Pick(pick) = pick else { panic!("unexpected pick result: {:?}", pick); }; - let received_address = &pick.subchannel.address().address.to_string(); - // It's good practice to create the expected value once. - let expected_address = name.to_string(); + let received_address = &pick.subchannel.address().address; // Check for inequality and panic with a detailed message if they don't match. - assert_eq!(received_address, &expected_address); + assert_eq!(received_address, name); } fn move_subchannel_to_state( @@ -655,7 +653,7 @@ mod test { .resolver_update(update.clone(), Some(&parsed_config2), &mut *tcc) .unwrap(); let subchannel = verify_subchannel_creation_from_policy(&mut rx_events); - assert_eq!(&*subchannel.address().address, "127.0.0.1:1234"); + assert_eq!(subchannel.address().address, "127.0.0.1:1234"); assert_channel_empty(&mut rx_events); } diff --git a/grpc/src/client/load_balancing/pick_first.rs b/grpc/src/client/load_balancing/pick_first.rs index e21fa0c38..541355169 100644 --- a/grpc/src/client/load_balancing/pick_first.rs +++ b/grpc/src/client/load_balancing/pick_first.rs @@ -424,7 +424,7 @@ impl PickFirstPolicy { // Partition by family (Basic IPv6 detection via colon). let (ipv6, ipv4): (Vec
, Vec
) = tcp_addresses .into_iter() - .partition(|addr| addr.address.contains(':')); + .partition(|addr| addr.address.contains(&b':')); // Interleave the two lists so ipv6 and ipv4 addresses are alternated. let mut interleaved = Vec::with_capacity(ipv6.len() + ipv4.len() + unknown.len()); @@ -736,9 +736,9 @@ mod test { use std::time::Duration; use super::*; - use crate::client::load_balancing::test_utils::{ - TestChannelController, TestEvent, TestWorkScheduler, - }; + use crate::client::load_balancing::test_utils::TestChannelController; + use crate::client::load_balancing::test_utils::TestEvent; + use crate::client::load_balancing::test_utils::TestWorkScheduler; const DEFAULT_TEST_DURATION: Duration = Duration::from_secs(10); @@ -926,7 +926,7 @@ mod test { let res = state.picker.pick(&RequestHeaders::default()); match res { PickResult::Pick(pick) => { - assert_eq!(pick.subchannel.address().address.to_string(), "addr1") + assert_eq!(pick.subchannel.address().address, "addr1") } other => panic!("unexpected pick result {:?}", other), } @@ -940,7 +940,7 @@ mod test { // Should connect to addr2. let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr2"); + assert_eq!(addr.address, "addr2"); // Simulate addr2 succeeding. let sc2 = policy.subchannels[1].clone(); @@ -984,25 +984,16 @@ mod test { // Should create new subchannel for addr2 (was cleared by cleanup). let sc2 = expect_new_subchannel(&rx); - assert_eq!(sc2.address().address.to_string(), "addr2"); + assert_eq!(sc2.address().address, "addr2"); // Should create new subchannel for addr3 (was not in previous list). let sc3 = expect_new_subchannel(&rx); - assert_eq!(sc3.address().address.to_string(), "addr3"); + assert_eq!(sc3.address().address, "addr3"); // Should NOT have any more events (no Connect, no UpdatePicker), // because it stuck to the original selected subchannel. assert!(rx.try_recv().is_err(), "unexpected event"); - assert_eq!( - policy - .selected - .as_ref() - .unwrap() - .address() - .address - .to_string(), - "addr1" - ); + assert_eq!(policy.selected.as_ref().unwrap().address().address, "addr1"); } // If all addresses fail during a connection pass, the LB should update to @@ -1086,7 +1077,7 @@ mod test { let mut resulting_addrs = Vec::with_capacity(NUM_ADDRS); for _ in 0..NUM_ADDRS { let sc = expect_new_subchannel(&rx); - resulting_addrs.push(sc.address().address.to_string()); + resulting_addrs.push(sc.address().address); } // Mock shuffler reverses endpoints: [EP3, EP2, EP1] @@ -1155,9 +1146,9 @@ mod test { // Should only create subchannels for addr1 and addr2 (2 unique addrs). let sc1 = expect_new_subchannel(&rx); - assert_eq!(sc1.address().address.to_string(), "addr1"); + assert_eq!(sc1.address().address, "addr1"); let sc2 = expect_new_subchannel(&rx); - assert_eq!(sc2.address().address.to_string(), "addr2"); + assert_eq!(sc2.address().address, "addr2"); // Verify no 3rd subchannel was created. while let Ok(event) = rx.try_recv() { @@ -1224,7 +1215,7 @@ mod test { // Expect Connect event for addr2 due to timer expiration. let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr2"); + assert_eq!(addr.address, "addr2"); } // If all addresses fail during a connection pass, the LB should enter @@ -1257,7 +1248,7 @@ mod test { // Should automatically call connect() again. let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr1"); + assert_eq!(addr.address, "addr1"); } // If the LB is in steady state, and a new address becomes ready, it should @@ -1272,7 +1263,7 @@ mod test { // Should failover to addr2: expect Connect(addr2). let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr2"); + assert_eq!(addr.address, "addr2"); // While addr2 is connecting, simulate addr1 going IDLE (backoff over). policy.subchannel_update( @@ -1307,7 +1298,7 @@ mod test { ); expect_request_resolution(&rx); let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr1"); + assert_eq!(addr.address, "addr1"); // Confirm LB is in steady state. assert!(policy.steady_state.is_some()); @@ -1324,7 +1315,7 @@ mod test { // Now it should automatically call connect() again. let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr1"); + assert_eq!(addr.address, "addr1"); // Simulate addr1 successfully connecting and becoming READY. policy.subchannel_update( @@ -1342,7 +1333,7 @@ mod test { let res = state.picker.pick(&RequestHeaders::default()); match res { PickResult::Pick(pick) => { - assert_eq!(pick.subchannel.address().address.to_string(), "addr1"); + assert_eq!(pick.subchannel.address().address, "addr1"); } other => panic!("unexpected pick result {:?}", other), } @@ -1361,7 +1352,7 @@ mod test { // Expect Connect(addr2). let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr2"); + assert_eq!(addr.address, "addr2"); // Simulate addr1 backing off and transitioning to IDLE early // (while addr2 is still connecting). @@ -1400,7 +1391,7 @@ mod test { // Expect an immediate Connect(addr1) event triggered by the exhaustion // loop sweeping up the early IDLE subchannel. let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr1"); + assert_eq!(addr.address, "addr1"); } // This test is meant to validate that if a new address with different @@ -1499,7 +1490,7 @@ mod test { policy.work(controller.as_mut()); let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr2"); + assert_eq!(addr.address, "addr2"); // 2. Simulate addr2 failing first while addr1 is still in flight. let sc2 = policy.subchannels[1].clone(); @@ -1581,7 +1572,7 @@ mod test { let res = state.picker.pick(&RequestHeaders::default()); let sc1 = match res { PickResult::Pick(pick) => { - assert_eq!(pick.subchannel.address().address.to_string(), "addr1"); + assert_eq!(pick.subchannel.address().address, "addr1"); pick.subchannel } other => panic!("unexpected pick result {:?}", other), @@ -1617,7 +1608,7 @@ mod test { // 7. Verify that the policy initiates a reconnection to addr1. let addr = expect_connect(&rx); - assert_eq!(addr.address.to_string(), "addr1"); + assert_eq!(addr.address, "addr1"); // And the picker goes to Connecting. let state = expect_picker_update(&rx); diff --git a/grpc/src/client/load_balancing/round_robin.rs b/grpc/src/client/load_balancing/round_robin.rs index efa2ce61a..ec82e2a2c 100644 --- a/grpc/src/client/load_balancing/round_robin.rs +++ b/grpc/src/client/load_balancing/round_robin.rs @@ -1017,11 +1017,11 @@ mod test { let all_subchannels = verify_subchannel_creation(&mut rx_events, 2); let subchannel_one = all_subchannels .iter() - .find(|sc| sc.address().address == "subchannel_one".to_string().into()) + .find(|sc| sc.address().address == "subchannel_one") .unwrap(); let subchannel_two = all_subchannels .iter() - .find(|sc| sc.address().address == "subchannel_two".to_string().into()) + .find(|sc| sc.address().address == "subchannel_two") .unwrap(); move_subchannel_to_state( @@ -1080,11 +1080,11 @@ mod test { let new_subchannels = verify_subchannel_creation(&mut rx_events, 2); let new_sc = new_subchannels .iter() - .find(|sc| sc.address().address == "new".to_string().into()) + .find(|sc| sc.address().address == "new") .unwrap(); let old_sc = new_subchannels .iter() - .find(|sc| sc.address().address == "subchannel_two".to_string().into()) + .find(|sc| sc.address().address == "subchannel_two") .unwrap(); move_subchannel_to_state( diff --git a/grpc/src/client/name_resolution/mod.rs b/grpc/src/client/name_resolution/mod.rs index 9cecab56c..005a2b39f 100644 --- a/grpc/src/client/name_resolution/mod.rs +++ b/grpc/src/client/name_resolution/mod.rs @@ -323,7 +323,12 @@ impl Hash for Address { impl Display for Address { #[allow(clippy::to_string_in_format_args)] fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!(f, "{}:{}", self.network_type, self.address.to_string()) + write!( + f, + "{}:{}", + self.network_type, + String::from_utf8_lossy(&self.address) + ) } } @@ -379,13 +384,15 @@ impl NopResolver { #[cfg(test)] mod test { + use std::collections::HashMap; + use std::collections::hash_map::DefaultHasher; + use std::hash::Hash; + use std::hash::Hasher; + use super::Target; use crate::attributes::Attributes; use crate::byte_str::ByteStr; use crate::client::name_resolution::Address; - use std::collections::HashMap; - use std::collections::hash_map::DefaultHasher; - use std::hash::{Hash, Hasher}; #[test] pub fn parse_target() { diff --git a/grpc/src/client/subchannel.rs b/grpc/src/client/subchannel.rs index 9f95e6d2b..f820d8ff8 100644 --- a/grpc/src/client/subchannel.rs +++ b/grpc/src/client/subchannel.rs @@ -39,6 +39,7 @@ use tonic::async_trait; use crate::StatusCodeError; use crate::StatusError; +use crate::byte_str::ByteStr; use crate::client::CallOptions; use crate::client::ConnectivityState; use crate::client::DynInvoke; @@ -243,7 +244,7 @@ pub(crate) struct InternalSubchannel { } struct InternalSubchannelData { - address: String, + address: ByteStr, state: InternalSubchannelState, work_queue: WorkQueueTx, on_drop: Arc, @@ -310,7 +311,7 @@ impl InternalSubchannel { work_queue: WorkQueueTx, ) -> Arc { let on_drop = Arc::new(Notify::new()); - let address_string = address.address.to_string(); + let address_string = address.address.clone(); let this = Arc::new_cyclic(|weak_self| Self { address, on_drop: on_drop.clone(), diff --git a/grpc/src/client/transport/mod.rs b/grpc/src/client/transport/mod.rs index 35b1ac421..107eb7f83 100644 --- a/grpc/src/client/transport/mod.rs +++ b/grpc/src/client/transport/mod.rs @@ -26,6 +26,7 @@ use std::sync::Arc; use std::time::Duration; use std::time::Instant; +use crate::byte_str::ByteStr; use crate::client::DynInvoke; use crate::client::Invoke; use crate::credentials::client::ClientHandshakeInfo; @@ -71,7 +72,7 @@ pub(crate) trait Transport: Sync { async fn connect( &self, - address: String, + address: ByteStr, runtime: GrpcRuntime, security_opts: &SecurityOpts, opts: &TransportOptions, @@ -89,7 +90,7 @@ pub(crate) trait Transport: Sync { pub(crate) trait DynTransport: Send + Sync { async fn dyn_connect( &self, - address: String, + address: ByteStr, runtime: GrpcRuntime, security_opts: &SecurityOpts, opts: &TransportOptions, @@ -107,7 +108,7 @@ pub(crate) trait DynTransport: Send + Sync { impl DynTransport for T { async fn dyn_connect( &self, - address: String, + address: ByteStr, runtime: GrpcRuntime, security_opts: &SecurityOpts, opts: &TransportOptions, diff --git a/grpc/src/client/transport/tonic/mod.rs b/grpc/src/client/transport/tonic/mod.rs index c736d9a6d..d6fecee1b 100644 --- a/grpc/src/client/transport/tonic/mod.rs +++ b/grpc/src/client/transport/tonic/mod.rs @@ -23,8 +23,10 @@ */ use std::error::Error; +use std::ffi::OsStr; use std::future::Future; use std::net::SocketAddr; +use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; use std::pin::Pin; use std::str::FromStr; @@ -70,6 +72,7 @@ use tower_service::Service as TowerService; use crate::StatusCodeError; use crate::StatusError; +use crate::byte_str::ByteStr; use crate::client::CallOptions; use crate::client::Invoke; use crate::client::RecvStream; @@ -363,7 +366,7 @@ impl Transport for TransportBuilder { async fn connect( &self, - address: String, + address: ByteStr, runtime: GrpcRuntime, security_info: &SecurityOpts, opts: &TransportOptions, @@ -405,8 +408,11 @@ impl Transport for TransportBuilder { let transport_fut = match self.network_type { NetworkType::Tcp => { + let addr_str: &str = (&address) + .try_into() + .map_err(|err| format!("address contains non-utf8 symbols: {err}"))?; let addr: SocketAddr = - SocketAddr::from_str(&address).map_err(|err| err.to_string())?; + SocketAddr::from_str(addr_str).map_err(|err| err.to_string())?; runtime.tcp_stream( addr, TcpOptions { @@ -415,9 +421,10 @@ impl Transport for TransportBuilder { }, ) } - NetworkType::Unix => { - runtime.unix_stream(PathBuf::from(&address), UnixSocketOptions::default()) - } + NetworkType::Unix => runtime.unix_stream( + PathBuf::from(OsStr::from_bytes(&address)), + UnixSocketOptions::default(), + ), }; let transport = if let Some(deadline) = opts.connect_deadline { let timeout = deadline.saturating_duration_since(Instant::now()); diff --git a/grpc/src/client/transport/tonic/test.rs b/grpc/src/client/transport/tonic/test.rs index 6dc3fe478..f26438b2d 100644 --- a/grpc/src/client/transport/tonic/test.rs +++ b/grpc/src/client/transport/tonic/test.rs @@ -157,7 +157,7 @@ pub(crate) async fn tonic_transport_rpc() { }; let (conn, _sec_info, mut disconnection_listener) = builder .dyn_connect( - addr.to_string(), + addr.to_string().into(), GrpcRuntime::new(TokioRuntime::default()), &securty_opts, &config, @@ -674,7 +674,7 @@ async fn tonic_transport_invalid_base64_headers() { }; let (conn, _sec_info, _disconnection_listener) = builder .dyn_connect( - addr.to_string(), + addr.to_string().into(), GrpcRuntime::new(TokioRuntime::default()), &securty_opts, &config, @@ -749,7 +749,7 @@ async fn tonic_transport_recv_drop_cancels_send() { }; let (conn, _sec_info, _disconnection_listener) = builder .dyn_connect( - addr.to_string(), + addr.to_string().into(), GrpcRuntime::new(TokioRuntime::default()), &securty_opts, &config, diff --git a/grpc/src/inmemory/mod.rs b/grpc/src/inmemory/mod.rs index 24354d2d9..e735e9e29 100644 --- a/grpc/src/inmemory/mod.rs +++ b/grpc/src/inmemory/mod.rs @@ -38,6 +38,7 @@ use tokio::sync::oneshot; use crate::StatusCodeError; use crate::StatusError; use crate::attributes::Attributes; +use crate::byte_str::ByteStr; use crate::client::CallOptions; use crate::client::DynRecvStream as ClientDynRecvStream; use crate::client::DynSendStream as ClientDynSendStream; @@ -78,7 +79,7 @@ use crate::server::ResponseStreamItem as ServerResponseStreamItem; use crate::server::SendOptions as ServerSendOptions; use crate::server::SendStream as ServerSendStream; -static LISTENERS: LazyLock>>> = +static LISTENERS: LazyLock>>> = LazyLock::new(|| Mutex::new(HashMap::new())); static NEXT_ID: AtomicU64 = AtomicU64::new(0); @@ -106,7 +107,7 @@ pub struct InMemoryListener { } struct InMemoryListenerInner { - id: String, + id: ByteStr, r: TokioMutex>, close_notify: Arc, drop_notify: Arc, @@ -126,7 +127,7 @@ impl Default for InMemoryListener { impl InMemoryListener { pub fn new() -> Self { - let id = NEXT_ID.fetch_add(1, Ordering::Relaxed).to_string(); + let id: ByteStr = NEXT_ID.fetch_add(1, Ordering::Relaxed).to_string().into(); let (s, r) = mpsc::channel(1); let mut listeners = LISTENERS.lock().unwrap(); listeners.insert(id.clone(), s); @@ -140,7 +141,7 @@ impl InMemoryListener { } } - pub fn id(&self) -> String { + pub fn id(&self) -> ByteStr { self.inner.id.clone() } @@ -343,7 +344,7 @@ impl Transport for InMemoryTransport { async fn connect( &self, - target: String, + target: ByteStr, _runtime: GrpcRuntime, _security_opts: &SecurityOpts, _options: &TransportOptions, @@ -358,7 +359,7 @@ impl Transport for InMemoryTransport { let listeners = LISTENERS.lock().unwrap(); let s = listeners .get(&target) - .ok_or_else(|| format!("no listener for target: {}", target))?; + .ok_or_else(|| format!("no listener for target: {:?}", target))?; let (closed_tx, closed_rx) = oneshot::channel(); let conn = InMemoryConnection { From dea8d574b4d5d92bd4918bbdca241a0c811ba2b9 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Fri, 5 Jun 2026 16:11:47 +0530 Subject: [PATCH 02/21] precent decode path in target URL --- grpc/Cargo.toml | 1 + grpc/src/byte_str.rs | 35 +++++++++++++++- grpc/src/client/channel.rs | 4 +- grpc/src/client/name_resolution/dns/mod.rs | 7 +++- grpc/src/client/name_resolution/mod.rs | 42 ++++++++++++++----- grpc/src/client/name_resolution/unix.rs | 7 ++-- .../client/name_resolution/unix_abstract.rs | 10 +++-- grpc/src/client/transport/tonic/test.rs | 10 ++++- grpc/src/inmemory/mod.rs | 11 +++-- 9 files changed, 96 insertions(+), 31 deletions(-) diff --git a/grpc/Cargo.toml b/grpc/Cargo.toml index c0c2434b0..a3e55a986 100644 --- a/grpc/Cargo.toml +++ b/grpc/Cargo.toml @@ -59,6 +59,7 @@ http-body = "1.0.1" hyper = { version = "1.6.0", features = ["client", "http2"] } itoa = "1.0" parking_lot = "0.12.4" +percent-encoding = "2" pin-project-lite = "0.2.16" rand = "0.9" rustls = { version = "0.23", optional = true, default-features = false, features = [ diff --git a/grpc/src/byte_str.rs b/grpc/src/byte_str.rs index 1d6c734d9..cf349538c 100644 --- a/grpc/src/byte_str.rs +++ b/grpc/src/byte_str.rs @@ -33,6 +33,20 @@ pub struct ByteStr { bytes: Bytes, } +impl ByteStr { + /// Strips a prefix, returning a new zero-copy ByteStr. + #[inline] + pub(crate) fn strip_prefix(&self, prefix: &[u8]) -> Option { + if self.starts_with(prefix) { + Some(ByteStr { + bytes: self.bytes.slice(prefix.len()..), + }) + } else { + None + } + } +} + impl Deref for ByteStr { type Target = [u8]; @@ -46,7 +60,6 @@ impl From for ByteStr { #[inline] fn from(src: String) -> ByteStr { ByteStr { - // Invariant: src is a String so contains valid UTF-8. bytes: Bytes::from(src), } } @@ -92,9 +105,27 @@ impl PartialEq for str { } } -impl<'a> PartialEq for &'a str { +impl PartialEq for &str { #[inline] fn eq(&self, other: &ByteStr) -> bool { self.as_bytes() == other.bytes } } + +impl FromIterator for ByteStr { + #[inline] + fn from_iter>(iter: T) -> Self { + ByteStr { + bytes: Bytes::from_iter(iter), + } + } +} + +impl From<&'static str> for ByteStr { + #[inline] + fn from(src: &'static str) -> ByteStr { + ByteStr { + bytes: Bytes::from_static(src.as_bytes()), + } + } +} diff --git a/grpc/src/client/channel.rs b/grpc/src/client/channel.rs index c93697fc3..b8a8d1a64 100644 --- a/grpc/src/client/channel.rs +++ b/grpc/src/client/channel.rs @@ -614,8 +614,8 @@ impl WatcherIter { } } -/// Parses the host and port from a URL-encoded string. When the input can not -/// be parsed as (host, port) pair, it returns the entire input as the host. +/// Parses the host and port from a string. When the input can not be parsed +/// as (host, port) pair, it returns the entire input as the host. fn parse_authority(host_and_port: &str) -> Authority { // Handle bracketed IPv6 addresses (e.g., "[::1]:80"). if let Some(stripped) = host_and_port.strip_prefix('[') diff --git a/grpc/src/client/name_resolution/dns/mod.rs b/grpc/src/client/name_resolution/dns/mod.rs index ce47dfc42..4cedfd46c 100644 --- a/grpc/src/client/name_resolution/dns/mod.rs +++ b/grpc/src/client/name_resolution/dns/mod.rs @@ -315,8 +315,11 @@ struct ParseResult { fn parse_endpoint_and_authority(target: &Target) -> Result { // Parse the endpoint. let endpoint = target.path(); - let endpoint = endpoint.strip_prefix("/").unwrap_or(endpoint); - let parse_result = parse_host_port(endpoint, DEFAULT_PORT) + let endpoint = endpoint.strip_prefix(b"/").unwrap_or(endpoint); + let host_port = (&endpoint) + .try_into() + .map_err(|err| format!("target hostname contains invalid utf8 symbols: {err}"))?; + let parse_result = parse_host_port(host_port, DEFAULT_PORT) .map_err(|err| format!("Failed to parse target {target}: {err}"))?; let endpoint = parse_result.ok_or("Received empty endpoint host.".to_string())?; diff --git a/grpc/src/client/name_resolution/mod.rs b/grpc/src/client/name_resolution/mod.rs index 005a2b39f..eb55e550a 100644 --- a/grpc/src/client/name_resolution/mod.rs +++ b/grpc/src/client/name_resolution/mod.rs @@ -34,6 +34,7 @@ use std::hash::Hash; use std::str::FromStr; use std::sync::Arc; +use percent_encoding::percent_decode_str; use url::Url; use crate::attributes::Attributes; @@ -123,9 +124,9 @@ impl Target { } } - /// Retrieves endpoint from `Url.path()`. - pub fn path(&self) -> &str { - self.url.path() + /// Retrieves the percent-decoded endpoint from `Url.path()`. + pub fn path(&self) -> ByteStr { + percent_decode_str(self.url.path()).collect() } } @@ -136,7 +137,7 @@ impl Display for Target { "{}://{}{}", self.scheme(), self.authority_host_port(), - self.path() + String::from_utf8_lossy(&self.path()) ) } } @@ -163,7 +164,8 @@ pub(crate) trait ResolverBuilder: Send + Sync { /// with the leading prefix removed. fn default_authority(&self, target: &Target) -> String { let path = target.path(); - path.strip_prefix("/").unwrap_or(path).to_string() + let path = path.strip_prefix(b"/").unwrap_or(path); + String::from_utf8_lossy(&path).into() } /// Returns a bool indicating whether the input uri is valid to create a @@ -403,7 +405,7 @@ mod test { want_host: &'static str, want_port: Option, want_host_port: &'static str, - want_path: &'static str, + want_path: &'static [u8], want_str: &'static str, } let test_cases = vec![ @@ -413,7 +415,7 @@ mod test { want_host_port: "", want_host: "", want_port: None, - want_path: "/grpc.io", + want_path: b"/grpc.io", want_str: "dns:///grpc.io", }, TestCase { @@ -422,7 +424,7 @@ mod test { want_host_port: "8.8.8.8:53", want_host: "8.8.8.8", want_port: Some(53), - want_path: "/grpc.io/docs", + want_path: b"/grpc.io/docs", want_str: "dns://8.8.8.8:53/grpc.io/docs", }, TestCase { @@ -431,7 +433,7 @@ mod test { want_host_port: "", want_host: "", want_port: None, - want_path: "path/to/file", + want_path: b"path/to/file", want_str: "unix://path/to/file", }, TestCase { @@ -440,9 +442,27 @@ mod test { want_host_port: "", want_host: "", want_port: None, - want_path: "/run/containerd/containerd.sock", + want_path: b"/run/containerd/containerd.sock", want_str: "unix:///run/containerd/containerd.sock", }, + TestCase { + input: "dns:///foo%20bar", + want_scheme: "dns", + want_host_port: "", + want_host: "", + want_port: None, + want_path: b"/foo bar", + want_str: "dns:///foo bar", + }, + TestCase { + input: "dns:///foo%FFbar", + want_scheme: "dns", + want_host_port: "", + want_host: "", + want_port: None, + want_path: b"/foo\xffbar", + want_str: "dns:///foo\u{FFFD}bar", + }, ]; for tc in test_cases { @@ -451,7 +471,7 @@ mod test { assert_eq!(target.authority_host(), tc.want_host); assert_eq!(target.authority_port(), tc.want_port); assert_eq!(target.authority_host_port(), tc.want_host_port); - assert_eq!(target.path(), tc.want_path); + assert_eq!(&*target.path(), tc.want_path); assert_eq!(&target.to_string(), tc.want_str); } } diff --git a/grpc/src/client/name_resolution/unix.rs b/grpc/src/client/name_resolution/unix.rs index e87b12ff8..0b0b40619 100644 --- a/grpc/src/client/name_resolution/unix.rs +++ b/grpc/src/client/name_resolution/unix.rs @@ -23,7 +23,6 @@ */ use crate::attributes::Attributes; -use crate::byte_str::ByteStr; use crate::client::name_resolution::Address; use crate::client::name_resolution::NopResolver; use crate::client::name_resolution::ResolverBuilder; @@ -59,7 +58,7 @@ impl ResolverBuilder for Builder { } fn default_authority(&self, target: &Target) -> String { - "localhost".to_owned() + "localhost".into() } } @@ -76,10 +75,9 @@ fn parse_target(target: &Target) -> Result { if !host_port.is_empty() { return Err(format!("invalid (non-empty) authority: {host_port}")); } - let addr_string = target.path().to_owned(); Ok(Address { network_type: UNIX_NETWORK_TYPE, - address: ByteStr::from(addr_string), + address: target.path(), attributes: Attributes::new(), }) } @@ -87,6 +85,7 @@ fn parse_target(target: &Target) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::byte_str::ByteStr; use crate::client::name_resolution::Endpoint; use crate::client::name_resolution::ResolverOptions; use crate::client::name_resolution::test_utils::TestChannelController; diff --git a/grpc/src/client/name_resolution/unix_abstract.rs b/grpc/src/client/name_resolution/unix_abstract.rs index b74cabd5a..f11144bcd 100644 --- a/grpc/src/client/name_resolution/unix_abstract.rs +++ b/grpc/src/client/name_resolution/unix_abstract.rs @@ -23,7 +23,6 @@ */ use crate::attributes::Attributes; -use crate::byte_str::ByteStr; use crate::client::name_resolution::Address; use crate::client::name_resolution::NopResolver; use crate::client::name_resolution::ResolverBuilder; @@ -59,7 +58,7 @@ impl ResolverBuilder for Builder { } fn default_authority(&self, _target: &Target) -> String { - "localhost".to_owned() + "localhost".into() } } @@ -78,10 +77,12 @@ fn parse_target(target: &Target) -> Result { if !host_port.is_empty() { return Err(format!("invalid (non-empty) authority: {host_port}")); } - let addr_string = format!("\0{}", target.path()); + let addr = std::iter::once(b'\0') + .chain(target.path().iter().copied()) + .collect(); Ok(Address { network_type: UNIX_NETWORK_TYPE, - address: ByteStr::from(addr_string), + address: addr, attributes: Attributes::new(), }) } @@ -89,6 +90,7 @@ fn parse_target(target: &Target) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::byte_str::ByteStr; use crate::client::name_resolution::Endpoint; use crate::client::name_resolution::ResolverOptions; use crate::client::name_resolution::test_utils::TestChannelController; diff --git a/grpc/src/client/transport/tonic/test.rs b/grpc/src/client/transport/tonic/test.rs index f26438b2d..f55f90208 100644 --- a/grpc/src/client/transport/tonic/test.rs +++ b/grpc/src/client/transport/tonic/test.rs @@ -316,9 +316,15 @@ mod unix_tests { #[tokio::test] async fn unix_absolute_path() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + let dir = tempdir().expect("failed to create temp dir"); - let socket_path = dir.path().join("absolute.sock"); - let target = format!("unix://{}", socket_path.to_str().unwrap()); + let mut socket_path_bytes = dir.path().as_os_str().as_bytes().to_vec(); + // Use an invalid UTF-8 byte sequence to ensure it is handled correctly. + socket_path_bytes.extend_from_slice(b"/absolute\xff.sock"); + let socket_path = PathBuf::from(OsStr::from_bytes(&socket_path_bytes)); + let target = format!("unix://{}/absolute%FF.sock", dir.path().to_str().unwrap()); run_unix_test(&socket_path, &target).await; } diff --git a/grpc/src/inmemory/mod.rs b/grpc/src/inmemory/mod.rs index e735e9e29..ac517e4eb 100644 --- a/grpc/src/inmemory/mod.rs +++ b/grpc/src/inmemory/mod.rs @@ -392,8 +392,11 @@ pub struct InMemoryResolverBuilder {} impl ResolverBuilder for InMemoryResolverBuilder { fn build(&self, target: &Target, options: ResolverOptions) -> Box { - let path = target.path().strip_prefix('/').unwrap_or(target.path()); - let ids: Vec = path.split(',').map(|s| s.to_string()).collect(); + let path = target.path().strip_prefix(b"/").unwrap_or(target.path()); + let ids: Vec = path + .split(|&b| b == b',') + .map(|chunk| chunk.iter().copied().collect()) + .collect(); options.work_scheduler.schedule_work(); Box::new(InMemoryResolver { ids }) } @@ -408,7 +411,7 @@ impl ResolverBuilder for InMemoryResolverBuilder { } struct InMemoryResolver { - ids: Vec, + ids: Vec, } impl Resolver for InMemoryResolver { @@ -421,7 +424,7 @@ impl Resolver for InMemoryResolver { .map(|id| Endpoint { addresses: vec![Address { network_type: "inmemory", - address: crate::byte_str::ByteStr::from(id.clone()), + address: id.clone(), ..Default::default() }], ..Default::default() From 4cb16938f6b79fda56e2452c02cca215049a26df Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Fri, 5 Jun 2026 16:41:54 +0530 Subject: [PATCH 03/21] polish --- grpc/src/client/name_resolution/dns/mod.rs | 2 +- grpc/src/client/name_resolution/mod.rs | 1 - grpc/src/client/transport/tonic/mod.rs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/grpc/src/client/name_resolution/dns/mod.rs b/grpc/src/client/name_resolution/dns/mod.rs index 4cedfd46c..cfab549a6 100644 --- a/grpc/src/client/name_resolution/dns/mod.rs +++ b/grpc/src/client/name_resolution/dns/mod.rs @@ -318,7 +318,7 @@ fn parse_endpoint_and_authority(target: &Target) -> Result let endpoint = endpoint.strip_prefix(b"/").unwrap_or(endpoint); let host_port = (&endpoint) .try_into() - .map_err(|err| format!("target hostname contains invalid utf8 symbols: {err}"))?; + .map_err(|err| format!("target hostname contains invalid UTF-8 symbols: {err}"))?; let parse_result = parse_host_port(host_port, DEFAULT_PORT) .map_err(|err| format!("Failed to parse target {target}: {err}"))?; let endpoint = parse_result.ok_or("Received empty endpoint host.".to_string())?; diff --git a/grpc/src/client/name_resolution/mod.rs b/grpc/src/client/name_resolution/mod.rs index eb55e550a..91a900e43 100644 --- a/grpc/src/client/name_resolution/mod.rs +++ b/grpc/src/client/name_resolution/mod.rs @@ -323,7 +323,6 @@ impl Hash for Address { } impl Display for Address { - #[allow(clippy::to_string_in_format_args)] fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!( f, diff --git a/grpc/src/client/transport/tonic/mod.rs b/grpc/src/client/transport/tonic/mod.rs index d6fecee1b..75728b75c 100644 --- a/grpc/src/client/transport/tonic/mod.rs +++ b/grpc/src/client/transport/tonic/mod.rs @@ -410,7 +410,7 @@ impl Transport for TransportBuilder { NetworkType::Tcp => { let addr_str: &str = (&address) .try_into() - .map_err(|err| format!("address contains non-utf8 symbols: {err}"))?; + .map_err(|err| format!("address contains non-UTF-8 symbols: {err}"))?; let addr: SocketAddr = SocketAddr::from_str(addr_str).map_err(|err| err.to_string())?; runtime.tcp_stream( From 991204ab03b0f88a7685478dc856bafad205c482 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Fri, 5 Jun 2026 17:10:01 +0530 Subject: [PATCH 04/21] fix non-unix build --- grpc/src/client/transport/tonic/mod.rs | 27 ++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/grpc/src/client/transport/tonic/mod.rs b/grpc/src/client/transport/tonic/mod.rs index 75728b75c..819dd14b9 100644 --- a/grpc/src/client/transport/tonic/mod.rs +++ b/grpc/src/client/transport/tonic/mod.rs @@ -23,10 +23,8 @@ */ use std::error::Error; -use std::ffi::OsStr; use std::future::Future; use std::net::SocketAddr; -use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; use std::pin::Pin; use std::str::FromStr; @@ -421,10 +419,27 @@ impl Transport for TransportBuilder { }, ) } - NetworkType::Unix => runtime.unix_stream( - PathBuf::from(OsStr::from_bytes(&address)), - UnixSocketOptions::default(), - ), + NetworkType::Unix => { + #[cfg(unix)] + { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + runtime.unix_stream( + PathBuf::from(OsStr::from_bytes(&address)), + UnixSocketOptions::default(), + ) + } + #[cfg(not(unix))] + { + match std::str::from_utf8(&address) { + Ok(valid_str) => runtime + .unix_stream(PathBuf::from(valid_str), UnixSocketOptions::default()), + Err(_) => Box::pin(async move { + Err("socket path contains non-UTF-8 characters".to_string()) + }), + } + } + } }; let transport = if let Some(deadline) = opts.connect_deadline { let timeout = deadline.saturating_duration_since(Instant::now()); From 060c3457865a839f1a7fffede6884c800a996d41 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Fri, 5 Jun 2026 23:06:14 +0530 Subject: [PATCH 05/21] fix darwin test --- grpc/src/client/transport/tonic/test.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/grpc/src/client/transport/tonic/test.rs b/grpc/src/client/transport/tonic/test.rs index f55f90208..d2f82f45f 100644 --- a/grpc/src/client/transport/tonic/test.rs +++ b/grpc/src/client/transport/tonic/test.rs @@ -316,6 +316,16 @@ mod unix_tests { #[tokio::test] async fn unix_absolute_path() { + let dir = tempdir().expect("failed to create temp dir"); + let socket_path = dir.path().join("absolute.sock"); + let target = format!("unix://{}", socket_path.to_str().unwrap()); + + run_unix_test(&socket_path, &target).await; + } + + #[tokio::test] + #[cfg(target_os = "linux")] + async fn unix_absolute_path_non_utf8() { use std::ffi::OsStr; use std::os::unix::ffi::OsStrExt; From 6ce704cd4514957f05e5cc2330eacc6216fef24f Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 8 Jun 2026 17:45:58 +0530 Subject: [PATCH 06/21] add proxy resolver --- grpc/Cargo.toml | 1 + grpc/src/client/name_resolution/dns/mod.rs | 2 +- grpc/src/client/name_resolution/mod.rs | 1 + .../client/name_resolution/proxy_resolver.rs | 641 ++++++++++++++++++ 4 files changed, 644 insertions(+), 1 deletion(-) create mode 100644 grpc/src/client/name_resolution/proxy_resolver.rs diff --git a/grpc/Cargo.toml b/grpc/Cargo.toml index a3e55a986..efc05a389 100644 --- a/grpc/Cargo.toml +++ b/grpc/Cargo.toml @@ -57,6 +57,7 @@ hickory-resolver = { version = "0.25.1", optional = true } http = "1.1.0" http-body = "1.0.1" hyper = { version = "1.6.0", features = ["client", "http2"] } +hyper-util = { version = "0.1", features = ["client", "client-proxy"] } itoa = "1.0" parking_lot = "0.12.4" percent-encoding = "2" diff --git a/grpc/src/client/name_resolution/dns/mod.rs b/grpc/src/client/name_resolution/dns/mod.rs index cfab549a6..fa15345a8 100644 --- a/grpc/src/client/name_resolution/dns/mod.rs +++ b/grpc/src/client/name_resolution/dns/mod.rs @@ -110,7 +110,7 @@ pub(crate) fn reg() { global_registry().add_builder(Box::new(Builder {})); } -struct Builder {} +pub(crate) struct Builder {} struct DnsOptions { min_resolution_interval: Duration, diff --git a/grpc/src/client/name_resolution/mod.rs b/grpc/src/client/name_resolution/mod.rs index 91a900e43..15680bfc7 100644 --- a/grpc/src/client/name_resolution/mod.rs +++ b/grpc/src/client/name_resolution/mod.rs @@ -54,6 +54,7 @@ pub(crate) mod unix; #[cfg(target_os = "linux")] pub(crate) mod unix_abstract; pub(crate) use registry::global_registry; +pub(crate) mod proxy_resolver; /// Target represents a target for gRPC, as specified in: /// https://github.com/grpc/grpc/blob/master/doc/naming.md. diff --git a/grpc/src/client/name_resolution/proxy_resolver.rs b/grpc/src/client/name_resolution/proxy_resolver.rs new file mode 100644 index 000000000..4e700adf5 --- /dev/null +++ b/grpc/src/client/name_resolution/proxy_resolver.rs @@ -0,0 +1,641 @@ +/* + * + * Copyright 2026 gRPC authors. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +use std::fmt::Debug; +use std::sync::Arc; +use std::sync::LazyLock; + +use http::HeaderValue; +use hyper_util::client::proxy::matcher::Matcher; +use url::Url; + +use super::ResolverOptions; +use super::Target; +use crate::client::name_resolution::Address; +use crate::client::name_resolution::ChannelController; +use crate::client::name_resolution::NopResolver; +use crate::client::name_resolution::Resolver; +use crate::client::name_resolution::ResolverBuilder; +use crate::client::name_resolution::ResolverUpdate; +use crate::client::name_resolution::dns; +use crate::client::service_config::ServiceConfig; + +static MATCHER: LazyLock> = LazyLock::new(build_matcher); + +fn build_matcher() -> Option { + // Avoid using a proxy in a Common Gateway Interface (CGI) environment. + if std::env::var_os("REQUEST_METHOD").is_some() { + return None; + } + + let https_proxy = get_first_env(&["HTTPS_PROXY", "https_proxy"]); + if https_proxy.is_empty() { + return None; + } + + let builder = Matcher::builder(); + // Only read NO_PROXY and HTTPS_PROXY. This avoids reading ALL_PROXY, + // which is not read by gRPC Go and C++. + Some( + builder + .no(get_first_env(&["NO_PROXY", "no_proxy"])) + .https(https_proxy) + .build(), + ) +} + +/// A resolver builder that wraps another `ResolverBuilder` and applies proxy +/// configuration. +/// +/// This builder checks if the target URI should be proxied based on environment +/// variables (like `HTTPS_PROXY`, `NO_PROXY`). If a proxy is needed, it creates +/// a resolver that resolves the proxy address and injects proxy options into +/// the resolved addresses. +pub(crate) struct Builder { + child_builder: Arc, +} + +impl ResolverBuilder for Builder { + fn build(&self, target: &Target, options: ResolverOptions) -> Box { + match self.new_resolver(target, options, MATCHER.as_ref()) { + Ok(resolver) => resolver, + Err((err, options)) => NopResolver::new_with_err(err, options), + } + } + + fn scheme(&self) -> &str { + self.child_builder.scheme() + } + + fn is_valid_uri(&self, uri: &Target) -> bool { + self.child_builder.is_valid_uri(uri) + } + + fn default_authority(&self, target: &Target) -> String { + self.child_builder.default_authority(target) + } +} + +impl Builder { + /// Creates a new `Builder` that wraps the given `child_builder`. + pub(crate) fn new(child_builder: Arc) -> Self { + Self { child_builder } + } + + fn new_resolver( + &self, + target: &Target, + options: ResolverOptions, + matcher: Option<&Matcher>, + ) -> Result, (String, ResolverOptions)> { + // If HTTPS_PROXY is unset, avoid parsing the target as a DNS hostname. + let Some(matcher) = matcher else { + return Ok(self.child_builder.build(target, options)); + }; + let path = target.path(); + let path = path.strip_prefix(b"/").unwrap_or(path); + + // Attempt to convert the path to a UTF-8 string, then parse it as a + // URL host. + let url_obj = match (&path) + .try_into() + .map_err(|err| format!("non-UTF-8 symbol in target host: {err}")) + .and_then(|target_host: &str| { + Url::parse(&format!("https://{target_host}")) + .map_err(|err| format!("invalid target host in URL: {err}")) + }) { + Ok(url) => url, + Err(err) => { + return Err((err, options)); + } + }; + + // Extract host and port, adding the 443 default. + let host = url_obj.host_str().unwrap_or(""); + let port = url_obj.port().unwrap_or(443); + let explicit_authority = format!("{host}:{port}"); + + // Safely build `http::Uri` with the explicit authority (guaranteed ASCII/Punycode). + let uri = match http::Uri::builder() + .scheme("https") + .authority(explicit_authority.as_str()) + .path_and_query("/") + .build() + { + Ok(uri) => uri, + Err(err) => { + // This should not error since the url crate parsed the host. + return Err(( + format!("failed to parse target authority: {}", err), + options, + )); + } + }; + + let Some(intercept) = matcher.intercept(&uri) else { + return Ok(self.child_builder.build(target, options)); + }; + + let mut proxy_authorization_header = intercept.basic_auth().cloned(); + if let Some(ref mut header) = proxy_authorization_header { + header.set_sensitive(true); + } + + let proxy_options = ProxyOptions { + proxy_authorization_header, + connect_addr: explicit_authority, + }; + + let Some(proxy_host) = intercept.uri().authority() else { + return Err(( + format!("proxy URI missing authority: {}", intercept.uri()), + options, + )); + }; + + // `proxy_host` must be a valid URL authority. Because the `url` crate + // implements the WHATWG standard, it leaves `[]` unescaped in the path. + // Therefore, we don't need to explicitly percent-decode the host string. + let target_str = format!("dns:///{}", proxy_host); + let proxy_target: Target = match target_str.parse() { + Ok(t) => t, + Err(e) => { + return Err(( + format!("failed to parse proxy target {target_str}: {e}"), + options, + )); + } + }; + + let child = dns::Builder {}.build(&proxy_target, options); + + Ok(Box::new(HttpsProxyResolver { + child, + proxy_options: Arc::new(proxy_options), + })) + } +} + +struct HttpsProxyResolver { + child: Box, + proxy_options: Arc, +} + +/// Options for establishing an HTTP CONNECT proxy tunnel. +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] +pub(crate) struct ProxyOptions { + proxy_authorization_header: Option, + connect_addr: String, +} + +impl ProxyOptions { + /// Returns the value of the `Proxy-Authorization` header, if present. + pub(crate) fn proxy_authorization_header(&self) -> Option<&http::HeaderValue> { + self.proxy_authorization_header.as_ref() + } + + /// 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 + } +} + +impl Resolver for HttpsProxyResolver { + fn resolve_now(&mut self) { + self.child.resolve_now(); + } + + fn work(&mut self, channel_controller: &mut dyn ChannelController) { + let mut interceptor = InterceptingController { + inner: channel_controller, + proxy_options: &self.proxy_options, + }; + self.child.work(&mut interceptor); + } +} + +struct InterceptingController<'a> { + inner: &'a mut dyn ChannelController, + proxy_options: &'a Arc, +} + +impl<'a> ChannelController for InterceptingController<'a> { + fn update(&mut self, mut update: ResolverUpdate) -> Result<(), String> { + 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()); + } + } + } + self.inner.update(update) + } + + fn parse_service_config(&self, config: &str) -> Result { + self.inner.parse_service_config(config) + } +} + +/// Extracts `ProxyOptions` from the given `Address` attributes, if present. +pub(crate) fn proxy_options_for_addr(addr: &Address) -> Option<&ProxyOptions> { + addr.attributes + .get::>() + .map(AsRef::as_ref) +} + +fn get_first_env(names: &[&str]) -> String { + for name in names { + if let Ok(val) = std::env::var(name) { + return val; + } + } + + String::new() +} + +#[cfg(test)] +mod tests { + use std::future::Future; + use std::net::IpAddr; + use std::pin::Pin; + use std::sync::Arc; + + use super::*; + use crate::attributes::Attributes; + use crate::byte_str::ByteStr; + use crate::client::name_resolution::Address; + use crate::client::name_resolution::test_utils::TestChannelController; + use crate::client::name_resolution::test_utils::TestWorkScheduler; + use crate::rt; + use crate::rt::GrpcEndpoint; + use crate::rt::GrpcRuntime; + use crate::rt::Runtime; + use crate::rt::Sleep; + use crate::rt::TaskHandle; + use crate::rt::TcpOptions; + use crate::rt::tokio::TokioRuntime; + + const DIRECT_ADDRESS: &str = "1.2.3.4:5678"; + + #[derive(Clone, Debug)] + struct FakeDns { + lookup_result: Result, String>, + } + + #[tonic::async_trait] + impl rt::DnsResolver for FakeDns { + async fn lookup_host_name(&self, _: &str) -> Result, String> { + self.lookup_result.clone() + } + + async fn lookup_txt(&self, _: &str) -> Result, String> { + Err("unimplemented".to_string()) + } + } + + #[derive(Debug)] + struct FakeRuntime { + inner: TokioRuntime, + dns: FakeDns, + } + + impl Runtime for FakeRuntime { + fn spawn( + &self, + task: Pin + Send + 'static>>, + ) -> Box { + self.inner.spawn(task) + } + + fn get_dns_resolver( + &self, + _: rt::ResolverOptions, + ) -> Result, String> { + Ok(Box::new(self.dns.clone())) + } + + fn sleep(&self, duration: std::time::Duration) -> Pin> { + self.inner.sleep(duration) + } + + fn tcp_stream( + &self, + target: std::net::SocketAddr, + opts: TcpOptions, + ) -> Pin, String>> + Send>> { + self.inner.tcp_stream(target, opts) + } + } + + struct MockResolverBuilder { + scheme: &'static str, + } + + impl ResolverBuilder for MockResolverBuilder { + fn build(&self, _target: &Target, options: ResolverOptions) -> Box { + let addr = Address { + network_type: "tcp", + address: ByteStr::from(DIRECT_ADDRESS.to_string()), + attributes: Attributes::new(), + }; + NopResolver::new_with_addr(addr, options) + } + + fn scheme(&self) -> &str { + self.scheme + } + + fn is_valid_uri(&self, _uri: &Target) -> bool { + true + } + } + + async fn run_resolver_and_get_addresses( + target_uri: &str, + dns_ips: Vec, + matcher: Option<&Matcher>, + ) -> Vec
{ + let child_builder = Arc::new(MockResolverBuilder { scheme: "dns" }); + let builder = Builder::new(child_builder); + + let target: Target = target_uri.parse().unwrap(); + let (work_scheduler, mut work_rx) = TestWorkScheduler::new_pair(); + let runtime = FakeRuntime { + inner: TokioRuntime::default(), + dns: FakeDns { + lookup_result: Ok(dns_ips), + }, + }; + let options = ResolverOptions { + authority: target.authority_host_port(), + runtime: GrpcRuntime::new(runtime), + work_scheduler, + }; + + let mut resolver = match builder.new_resolver(&target, options, matcher) { + Ok(resolver) => resolver, + Err((err, _)) => panic!("failed to build resolver: {err}"), + }; + + work_rx.recv().await.unwrap(); + + let (mut channel_controller, mut update_rx) = TestChannelController::new_pair(); + resolver.work(&mut channel_controller); + + let update = update_rx.recv().await.unwrap(); + let endpoints = update.endpoints.unwrap(); + + let mut addresses = Vec::new(); + for endpoint in endpoints { + for address in endpoint.addresses { + addresses.push(address); + } + } + addresses + } + + #[tokio::test] + async fn test_proxy_matched() { + let matcher = Matcher::builder() + .https("http://user:password@proxy.example.com:8080") + .build(); + + let dns_ips = vec!["127.0.0.1".parse().unwrap(), "::1".parse().unwrap()]; + let addresses = + run_resolver_and_get_addresses("dns:///target.example.com", dns_ips, Some(&matcher)) + .await; + + assert_eq!(addresses.len(), 2); + assert_eq!(addresses[0].address, "127.0.0.1:8080"); + assert_eq!(addresses[1].address, "[::1]:8080"); + + let mut expected_header = HeaderValue::from_static("Basic dXNlcjpwYXNzd29yZA=="); + expected_header.set_sensitive(true); + let expected_proxy_opts = ProxyOptions { + proxy_authorization_header: Some(expected_header), + connect_addr: "target.example.com:443".to_string(), + }; + + for address in &addresses { + let proxy_opts = proxy_options_for_addr(address).expect("ProxyOptions not found"); + assert_eq!(proxy_opts, &expected_proxy_opts); + } + } + + #[tokio::test] + async fn test_proxy_non_matched() { + let matcher = Matcher::builder() + .https("http://proxy.example.com:8080") + .no("target.example.com") + .build(); + + let addresses = run_resolver_and_get_addresses( + "dns:///target.example.com", + vec!["127.0.0.1".parse().unwrap()], + Some(&matcher), + ) + .await; + + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert!(proxy_options_for_addr(&addresses[0]).is_none()); + } + + #[tokio::test] + async fn test_punycode_encoding() { + let matcher = Matcher::builder() + .https("http://proxy.example.com:8080") + .build(); + + let addresses = run_resolver_and_get_addresses( + "dns:///täst.example.com", + vec!["127.0.0.1".parse().unwrap()], + Some(&matcher), + ) + .await; + + assert_eq!(addresses.len(), 1); + let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + assert_eq!( + proxy_opts, + &ProxyOptions { + proxy_authorization_header: None, + connect_addr: "xn--tst-qla.example.com:443".to_string(), + } + ); + } + + #[tokio::test] + async fn test_invalid_unix_path_with_proxy_errors() { + let matcher = Matcher::builder() + .https("http://proxy.example.com:8080") + .build(); + + // The path has a space in the first segment of the path, which makes it + // an invalid hostname. + let target_uri = "unix:///var%20/run/grpc.sock"; + + let child_builder = Arc::new(MockResolverBuilder { scheme: "unix" }); + let builder = Builder::new(child_builder); + + let target: Target = target_uri.parse().unwrap(); + let (work_scheduler, mut work_rx) = TestWorkScheduler::new_pair(); + let runtime = FakeRuntime { + inner: TokioRuntime::default(), + dns: FakeDns { + lookup_result: Ok(vec!["127.0.0.1".parse().unwrap()]), + }, + }; + let options = ResolverOptions { + authority: target.authority_host_port(), + runtime: GrpcRuntime::new(runtime), + work_scheduler, + }; + + let mut resolver = match builder.new_resolver(&target, options, Some(&matcher)) { + Ok(resolver) => resolver, + Err((err, options)) => NopResolver::new_with_err(err, options), + }; + + work_rx.recv().await.unwrap(); + + let (mut channel_controller, mut update_rx) = TestChannelController::new_pair(); + resolver.work(&mut channel_controller); + + let update = update_rx.recv().await.unwrap(); + let err = update.endpoints.unwrap_err(); + assert!(err.contains("invalid target host in URL")); + } + + #[tokio::test] + async fn test_invalid_unix_path_without_proxy_works() { + // When matcher is None, it should bypass URL parsing and succeed, + // returning the child resolver's addresses. + let addresses = run_resolver_and_get_addresses( + "unix:///var%20/run/grpc.sock", + vec!["127.0.0.1".parse().unwrap()], + None, + ) + .await; + + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert!(proxy_options_for_addr(&addresses[0]).is_none()); + } + + #[tokio::test] + async fn test_matcher_behavior_configured_manually() { + let dns_ips = || vec!["127.0.0.1".parse().unwrap()]; + + // Case 1: http proxy is set, but destination is HTTPS. + // It should NOT be matched. + let matcher = Matcher::builder() + .http("http://proxy.example.com:8080") + .build(); + let addresses = + run_resolver_and_get_addresses("dns:///target.example.com", dns_ips(), Some(&matcher)) + .await; + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert!( + proxy_options_for_addr(&addresses[0]).is_none(), + "HTTP proxy should not match HTTPS destinations" + ); + + // Case 2: https proxy is set, destination is HTTPS. + // It should be matched. + let matcher = Matcher::builder() + .https("http://proxy.example.com:8080") + .build(); + let addresses = + run_resolver_and_get_addresses("dns:///target.example.com", dns_ips(), Some(&matcher)) + .await; + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, "127.0.0.1:8080"); + assert!( + proxy_options_for_addr(&addresses[0]).is_some(), + "HTTPS proxy should match HTTPS destinations" + ); + + // Case 3: https proxy and no proxy are configured. + let matcher = Matcher::builder() + .https("http://proxy.example.com:8080") + .no("target.example.com") + .build(); + + // Target A: target.example.com (matched by no_proxy) -> should bypass proxy + let addresses = + run_resolver_and_get_addresses("dns:///target.example.com", dns_ips(), Some(&matcher)) + .await; + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert!(proxy_options_for_addr(&addresses[0]).is_none()); + + // Target B: other.example.com (NOT matched by no_proxy) -> should proxy + let addresses = + run_resolver_and_get_addresses("dns:///other.example.com", dns_ips(), Some(&matcher)) + .await; + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, "127.0.0.1:8080"); + assert!(proxy_options_for_addr(&addresses[0]).is_some()); + } + + #[tokio::test] + async fn test_no_matcher_returns_child_resolver() { + let addresses = run_resolver_and_get_addresses( + "unix:///invalid/but/doesnt/matter/since/no/matcher", + vec!["127.0.0.1".parse().unwrap()], + None, + ) + .await; + + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert!(proxy_options_for_addr(&addresses[0]).is_none()); + } + + #[tokio::test] + async fn test_proxy_ipv6_address() { + let matcher = Matcher::builder().https("http://[::1]:8080").build(); + + let addresses = run_resolver_and_get_addresses( + "dns:///target.example.com", + vec!["127.0.0.1".parse().unwrap()], + Some(&matcher), + ) + .await; + + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, "[::1]:8080"); + + let expected_proxy_opts = ProxyOptions { + proxy_authorization_header: None, + connect_addr: "target.example.com:443".to_string(), + }; + + let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + assert_eq!(proxy_opts, &expected_proxy_opts); + } +} From 0ffbba6873abdadc5bbc3348f900dd1dd74cc9bc Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 8 Jun 2026 17:48:28 +0530 Subject: [PATCH 07/21] add minor version to dep --- grpc/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/Cargo.toml b/grpc/Cargo.toml index a3e55a986..373e7a2f6 100644 --- a/grpc/Cargo.toml +++ b/grpc/Cargo.toml @@ -59,7 +59,7 @@ http-body = "1.0.1" hyper = { version = "1.6.0", features = ["client", "http2"] } itoa = "1.0" parking_lot = "0.12.4" -percent-encoding = "2" +percent-encoding = "2.3" pin-project-lite = "0.2.16" rand = "0.9" rustls = { version = "0.23", optional = true, default-features = false, features = [ From b6a4bcd8bf79c690cce98d0fbda65c5d06a20765 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 8 Jun 2026 17:50:22 +0530 Subject: [PATCH 08/21] Add minor version in dep --- grpc/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grpc/Cargo.toml b/grpc/Cargo.toml index efc05a389..5fce485a2 100644 --- a/grpc/Cargo.toml +++ b/grpc/Cargo.toml @@ -60,7 +60,7 @@ hyper = { version = "1.6.0", features = ["client", "http2"] } hyper-util = { version = "0.1", features = ["client", "client-proxy"] } itoa = "1.0" parking_lot = "0.12.4" -percent-encoding = "2" +percent-encoding = "2.3" pin-project-lite = "0.2.16" rand = "0.9" rustls = { version = "0.23", optional = true, default-features = false, features = [ From e502faf24afed2b606345221806b38cb7cec3ded Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 8 Jun 2026 23:04:28 +0530 Subject: [PATCH 09/21] skip proxy for unix schemes --- .../client/name_resolution/proxy_resolver.rs | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/grpc/src/client/name_resolution/proxy_resolver.rs b/grpc/src/client/name_resolution/proxy_resolver.rs index 4e700adf5..547802440 100644 --- a/grpc/src/client/name_resolution/proxy_resolver.rs +++ b/grpc/src/client/name_resolution/proxy_resolver.rs @@ -109,6 +109,10 @@ impl Builder { options: ResolverOptions, matcher: Option<&Matcher>, ) -> Result, (String, ResolverOptions)> { + // Skip proxy lookup for known non-TCP schemes. + if matches!(target.scheme(), "unix" | "unix-abstract") { + return Ok(self.child_builder.build(target, options)); + } // If HTTPS_PROXY is unset, avoid parsing the target as a DNS hostname. let Some(matcher) = matcher else { return Ok(self.child_builder.build(target, options)); @@ -417,7 +421,7 @@ mod tests { } #[tokio::test] - async fn test_proxy_matched() { + async fn proxy_matched() { let matcher = Matcher::builder() .https("http://user:password@proxy.example.com:8080") .build(); @@ -445,7 +449,7 @@ mod tests { } #[tokio::test] - async fn test_proxy_non_matched() { + async fn proxy_non_matched() { let matcher = Matcher::builder() .https("http://proxy.example.com:8080") .no("target.example.com") @@ -464,7 +468,7 @@ mod tests { } #[tokio::test] - async fn test_punycode_encoding() { + async fn punycode_encoding() { let matcher = Matcher::builder() .https("http://proxy.example.com:8080") .build(); @@ -488,16 +492,16 @@ mod tests { } #[tokio::test] - async fn test_invalid_unix_path_with_proxy_errors() { + async fn invalid_custom_scheme_path_with_proxy_errors() { let matcher = Matcher::builder() .https("http://proxy.example.com:8080") .build(); // The path has a space in the first segment of the path, which makes it // an invalid hostname. - let target_uri = "unix:///var%20/run/grpc.sock"; + let target_uri = "custom:///var%20/run/grpc.sock"; - let child_builder = Arc::new(MockResolverBuilder { scheme: "unix" }); + let child_builder = Arc::new(MockResolverBuilder { scheme: "custom" }); let builder = Builder::new(child_builder); let target: Target = target_uri.parse().unwrap(); @@ -530,13 +534,28 @@ mod tests { } #[tokio::test] - async fn test_invalid_unix_path_without_proxy_works() { - // When matcher is None, it should bypass URL parsing and succeed, - // returning the child resolver's addresses. + async fn invalid_unix_path_works() { + let matcher = Matcher::builder() + .https("http://proxy.example.com:8080") + .build(); + + // Proxy lookup for unix targets should be skipped. let addresses = run_resolver_and_get_addresses( "unix:///var%20/run/grpc.sock", vec!["127.0.0.1".parse().unwrap()], - None, + Some(&matcher), + ) + .await; + + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert!(proxy_options_for_addr(&addresses[0]).is_none()); + + // Check for abstract-unix scheme. + let addresses = run_resolver_and_get_addresses( + "unix-abstract:grpc.sock", + vec!["127.0.0.1".parse().unwrap()], + Some(&matcher), ) .await; @@ -546,7 +565,7 @@ mod tests { } #[tokio::test] - async fn test_matcher_behavior_configured_manually() { + async fn matcher_behavior_configured_manually() { let dns_ips = || vec!["127.0.0.1".parse().unwrap()]; // Case 1: http proxy is set, but destination is HTTPS. @@ -603,7 +622,7 @@ mod tests { } #[tokio::test] - async fn test_no_matcher_returns_child_resolver() { + async fn no_matcher_returns_child_resolver() { let addresses = run_resolver_and_get_addresses( "unix:///invalid/but/doesnt/matter/since/no/matcher", vec!["127.0.0.1".parse().unwrap()], @@ -617,7 +636,7 @@ mod tests { } #[tokio::test] - async fn test_proxy_ipv6_address() { + async fn proxy_ipv6_address() { let matcher = Matcher::builder().https("http://[::1]:8080").build(); let addresses = run_resolver_and_get_addresses( From 2164fc4624810d6b93476325eebd91d315c279bd Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 8 Jun 2026 23:44:52 +0530 Subject: [PATCH 10/21] Move authority parsing constructor --- grpc/src/client/channel.rs | 182 +----------------------------------- grpc/src/credentials/mod.rs | 176 ++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 181 deletions(-) diff --git a/grpc/src/client/channel.rs b/grpc/src/client/channel.rs index 3a3234290..e696e97df 100644 --- a/grpc/src/client/channel.rs +++ b/grpc/src/client/channel.rs @@ -258,7 +258,7 @@ impl PersistentChannel { .unwrap_or_else(|| resolver_builder.default_authority(&target).to_owned()); let security_opts = SecurityOpts { credentials, - authority: parse_authority(&authority), + authority: Authority::from_host_port_str(&authority), handshake_info: ClientHandshakeInfo::default(), }; @@ -600,183 +600,3 @@ impl WatcherIter { Some(self.rx.borrow_and_update().clone()) } } - -/// Parses the host and port from a string. When the input can not be parsed -/// as (host, port) pair, it returns the entire input as the host. -fn parse_authority(host_and_port: &str) -> Authority { - // Handle bracketed IPv6 addresses (e.g., "[::1]:80"). - if let Some(stripped) = host_and_port.strip_prefix('[') - && let Some((host, port_str)) = stripped.split_once("]:") - && let Ok(port) = port_str.parse::() - { - return Authority::new(host, Some(port)); - } - // Handle unbracketed addresses (IPv4 or hostnames, e.g., "localhost:8080"). - if let Some((host, port_str)) = host_and_port.rsplit_once(':') - && !host.contains(':') - && let Ok(port) = port_str.parse::() - { - return Authority::new(host, Some(port)); - } - Authority::new(host_and_port.to_string(), None) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_parse_authority() { - struct TestCase { - input: &'static str, - expected: Authority, - } - - let cases = [ - TestCase { - input: "localhost:http", - expected: Authority::new("localhost:http", None), - }, - TestCase { - input: "localhost:80", - expected: Authority::new("localhost", Some(80)), - }, - // host name with zone identifier. - TestCase { - input: "localhost%lo0:80", - expected: Authority::new("localhost%lo0", Some(80)), - }, - TestCase { - input: "localhost%lo0:http", - expected: Authority::new("localhost%lo0:http", None), - }, - TestCase { - input: "[localhost%lo0]:http", - expected: Authority::new("[localhost%lo0]:http", None), - }, - TestCase { - input: "[localhost%lo0]:80", - expected: Authority::new("localhost%lo0", Some(80)), - }, - // IP literal - TestCase { - input: "127.0.0.1:http", - expected: Authority::new("127.0.0.1:http", None), - }, - TestCase { - input: "127.0.0.1:80", - expected: Authority::new("127.0.0.1", Some(80)), - }, - TestCase { - input: "[::1]:http", - expected: Authority::new("[::1]:http", None), - }, - TestCase { - input: "[::1]:80", - expected: Authority::new("::1", Some(80)), - }, - // IP literal with zone identifier. - TestCase { - input: "[::1%lo0]:http", - expected: Authority::new("[::1%lo0]:http", None), - }, - TestCase { - input: "[::1%lo0]:80", - expected: Authority::new("::1%lo0", Some(80)), - }, - TestCase { - input: ":http", - expected: Authority::new(":http", None), - }, - TestCase { - input: ":80", - expected: Authority::new("", Some(80)), - }, - TestCase { - input: "grpc.io:", - expected: Authority::new("grpc.io:", None), - }, - TestCase { - input: "127.0.0.1:", - expected: Authority::new("127.0.0.1:", None), - }, - TestCase { - input: "[::1]:", - expected: Authority::new("[::1]:", None), - }, - TestCase { - input: "grpc.io:https%foo", - expected: Authority::new("grpc.io:https%foo", None), - }, - TestCase { - input: "grpc.io", - expected: Authority::new("grpc.io", None), - }, - TestCase { - input: "127.0.0.1", - expected: Authority::new("127.0.0.1", None), - }, - TestCase { - input: "[::1]", - expected: Authority::new("[::1]", None), - }, - TestCase { - input: "[fe80::1%lo0]", - expected: Authority::new("[fe80::1%lo0]", None), - }, - TestCase { - input: "[localhost%lo0]", - expected: Authority::new("[localhost%lo0]", None), - }, - TestCase { - input: "localhost%lo0", - expected: Authority::new("localhost%lo0", None), - }, - TestCase { - input: "::1", - expected: Authority::new("::1", None), - }, - TestCase { - input: "fe80::1%lo0", - expected: Authority::new("fe80::1%lo0", None), - }, - TestCase { - input: "fe80::1%lo0:80", - expected: Authority::new("fe80::1%lo0:80", None), - }, - TestCase { - input: "[foo:bar]", - expected: Authority::new("[foo:bar]", None), - }, - TestCase { - input: "[foo:bar]baz", - expected: Authority::new("[foo:bar]baz", None), - }, - TestCase { - input: "[foo]bar:baz", - expected: Authority::new("[foo]bar:baz", None), - }, - TestCase { - input: "[foo]:[bar]:baz", - expected: Authority::new("[foo]:[bar]:baz", None), - }, - TestCase { - input: "[foo]:[bar]baz", - expected: Authority::new("[foo]:[bar]baz", None), - }, - TestCase { - input: "foo[bar]:baz", - expected: Authority::new("foo[bar]:baz", None), - }, - TestCase { - input: "foo]bar:baz", - expected: Authority::new("foo]bar:baz", None), - }, - ]; - - for TestCase { input, expected } in cases { - let auth = parse_authority(input); - assert_eq!(auth, expected, "authority mismatch for {}", input); - } - } -} diff --git a/grpc/src/credentials/mod.rs b/grpc/src/credentials/mod.rs index 68a56600f..15450d32b 100644 --- a/grpc/src/credentials/mod.rs +++ b/grpc/src/credentials/mod.rs @@ -161,6 +161,27 @@ pub(crate) mod common { } } + /// Parses the host and port from a string. When the input can not be parsed + /// as (host, port) pair, it returns the entire input as the host. + pub(crate) fn from_host_port_str(host_and_port: &str) -> Self { + // Handle bracketed IPv6 addresses (e.g., "[::1]:80"). + if let Some(stripped) = host_and_port.strip_prefix('[') + && let Some((host, port_str)) = stripped.split_once("]:") + && let Ok(port) = port_str.parse::() + { + return Self::new(host, Some(port)); + } + // Handle unbracketed addresses (IPv4 or hostnames, e.g., + // "localhost:8080"). + if let Some((host, port_str)) = host_and_port.rsplit_once(':') + && !host.contains(':') + && let Ok(port) = port_str.parse::() + { + return Self::new(host, Some(port)); + } + Self::new(host_and_port.to_string(), None) + } + pub fn host(&self) -> &str { &self.host } @@ -218,4 +239,159 @@ mod tests { let authority = Authority::new("::1", None); assert_eq!(&authority.host_port_string(), "::1"); } + + #[test] + fn test_parse_authority() { + struct TestCase { + input: &'static str, + expected: Authority, + } + + let cases = [ + TestCase { + input: "localhost:http", + expected: Authority::new("localhost:http", None), + }, + TestCase { + input: "localhost:80", + expected: Authority::new("localhost", Some(80)), + }, + // host name with zone identifier. + TestCase { + input: "localhost%lo0:80", + expected: Authority::new("localhost%lo0", Some(80)), + }, + TestCase { + input: "localhost%lo0:http", + expected: Authority::new("localhost%lo0:http", None), + }, + TestCase { + input: "[localhost%lo0]:http", + expected: Authority::new("[localhost%lo0]:http", None), + }, + TestCase { + input: "[localhost%lo0]:80", + expected: Authority::new("localhost%lo0", Some(80)), + }, + // IP literal + TestCase { + input: "127.0.0.1:http", + expected: Authority::new("127.0.0.1:http", None), + }, + TestCase { + input: "127.0.0.1:80", + expected: Authority::new("127.0.0.1", Some(80)), + }, + TestCase { + input: "[::1]:http", + expected: Authority::new("[::1]:http", None), + }, + TestCase { + input: "[::1]:80", + expected: Authority::new("::1", Some(80)), + }, + // IP literal with zone identifier. + TestCase { + input: "[::1%lo0]:http", + expected: Authority::new("[::1%lo0]:http", None), + }, + TestCase { + input: "[::1%lo0]:80", + expected: Authority::new("::1%lo0", Some(80)), + }, + TestCase { + input: ":http", + expected: Authority::new(":http", None), + }, + TestCase { + input: ":80", + expected: Authority::new("", Some(80)), + }, + TestCase { + input: "grpc.io:", + expected: Authority::new("grpc.io:", None), + }, + TestCase { + input: "127.0.0.1:", + expected: Authority::new("127.0.0.1:", None), + }, + TestCase { + input: "[::1]:", + expected: Authority::new("[::1]:", None), + }, + TestCase { + input: "grpc.io:https%foo", + expected: Authority::new("grpc.io:https%foo", None), + }, + TestCase { + input: "grpc.io", + expected: Authority::new("grpc.io", None), + }, + TestCase { + input: "127.0.0.1", + expected: Authority::new("127.0.0.1", None), + }, + TestCase { + input: "[::1]", + expected: Authority::new("[::1]", None), + }, + TestCase { + input: "[fe80::1%lo0]", + expected: Authority::new("[fe80::1%lo0]", None), + }, + TestCase { + input: "[localhost%lo0]", + expected: Authority::new("[localhost%lo0]", None), + }, + TestCase { + input: "localhost%lo0", + expected: Authority::new("localhost%lo0", None), + }, + TestCase { + input: "::1", + expected: Authority::new("::1", None), + }, + TestCase { + input: "fe80::1%lo0", + expected: Authority::new("fe80::1%lo0", None), + }, + TestCase { + input: "fe80::1%lo0:80", + expected: Authority::new("fe80::1%lo0:80", None), + }, + TestCase { + input: "[foo:bar]", + expected: Authority::new("[foo:bar]", None), + }, + TestCase { + input: "[foo:bar]baz", + expected: Authority::new("[foo:bar]baz", None), + }, + TestCase { + input: "[foo]bar:baz", + expected: Authority::new("[foo]bar:baz", None), + }, + TestCase { + input: "[foo]:[bar]:baz", + expected: Authority::new("[foo]:[bar]:baz", None), + }, + TestCase { + input: "[foo]:[bar]baz", + expected: Authority::new("[foo]:[bar]baz", None), + }, + TestCase { + input: "foo[bar]:baz", + expected: Authority::new("foo[bar]:baz", None), + }, + TestCase { + input: "foo]bar:baz", + expected: Authority::new("foo]bar:baz", None), + }, + ]; + + for TestCase { input, expected } in cases { + let auth = Authority::from_host_port_str(input); + assert_eq!(auth, expected, "authority mismatch for {}", input); + } + } } From 5f7c97d4331163c6fe6c31cf03f10c4c394f14a5 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Tue, 9 Jun 2026 00:07:27 +0530 Subject: [PATCH 11/21] fix handling of unbrackated IPv6 in target address --- .../client/name_resolution/proxy_resolver.rs | 43 +++++++++++++++++-- grpc/src/credentials/mod.rs | 4 ++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/grpc/src/client/name_resolution/proxy_resolver.rs b/grpc/src/client/name_resolution/proxy_resolver.rs index 547802440..ce318338d 100644 --- a/grpc/src/client/name_resolution/proxy_resolver.rs +++ b/grpc/src/client/name_resolution/proxy_resolver.rs @@ -40,6 +40,7 @@ use crate::client::name_resolution::ResolverBuilder; use crate::client::name_resolution::ResolverUpdate; use crate::client::name_resolution::dns; use crate::client::service_config::ServiceConfig; +use crate::credentials::common::Authority; static MATCHER: LazyLock> = LazyLock::new(build_matcher); @@ -126,6 +127,7 @@ impl Builder { .try_into() .map_err(|err| format!("non-UTF-8 symbol in target host: {err}")) .and_then(|target_host: &str| { + let target_host = authority_with_default_port(target_host, 443); Url::parse(&format!("https://{target_host}")) .map_err(|err| format!("invalid target host in URL: {err}")) }) { @@ -135,12 +137,14 @@ impl Builder { } }; - // Extract host and port, adding the 443 default. + // The URL omits the default port for the scheme (443 for HTTPS), so we + // must explicitly add it. let host = url_obj.host_str().unwrap_or(""); let port = url_obj.port().unwrap_or(443); let explicit_authority = format!("{host}:{port}"); - // Safely build `http::Uri` with the explicit authority (guaranteed ASCII/Punycode). + // Safely build `http::Uri` with the explicit authority (guaranteed + // ASCII/Punycode). let uri = match http::Uri::builder() .scheme("https") .authority(explicit_authority.as_str()) @@ -279,6 +283,14 @@ fn get_first_env(names: &[&str]) -> String { String::new() } +fn authority_with_default_port(host_port: &str, default_port: u16) -> String { + let mut authority = Authority::from_host_port_str(host_port); + if authority.port().is_none() { + authority.set_port(Some(default_port)); + } + authority.host_port_string() +} + #[cfg(test)] mod tests { use std::future::Future; @@ -636,7 +648,7 @@ mod tests { } #[tokio::test] - async fn proxy_ipv6_address() { + async fn ipv6_proxy_address() { let matcher = Matcher::builder().https("http://[::1]:8080").build(); let addresses = run_resolver_and_get_addresses( @@ -657,4 +669,29 @@ mod tests { let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); } + + #[tokio::test] + async fn ipv6_target_address() { + let matcher = Matcher::builder() + .https("http://proxy.example.com:8080") + .build(); + + let addresses = run_resolver_and_get_addresses( + "dns:///::1", + vec!["127.0.0.1".parse().unwrap()], + Some(&matcher), + ) + .await; + + assert_eq!(addresses.len(), 1); + assert_eq!(addresses[0].address, "127.0.0.1:8080"); + + let expected_proxy_opts = ProxyOptions { + proxy_authorization_header: None, + connect_addr: "[::1]:443".to_string(), + }; + + let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + assert_eq!(proxy_opts, &expected_proxy_opts); + } } diff --git a/grpc/src/credentials/mod.rs b/grpc/src/credentials/mod.rs index 15450d32b..5d0d44ca5 100644 --- a/grpc/src/credentials/mod.rs +++ b/grpc/src/credentials/mod.rs @@ -190,6 +190,10 @@ pub(crate) mod common { self.port } + pub fn set_port(&mut self, port: Option) { + self.port = port; + } + pub fn host_port_string(&self) -> String { let host_str = &self.host; match self.port() { From 5cc591e815a3a1bec2c5b0b19e9faa5498de4edf Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Tue, 9 Jun 2026 11:38:22 +0530 Subject: [PATCH 12/21] disallow non-UTF-8 symbols in path --- grpc/src/byte_str.rs | 94 ++----------------- grpc/src/client/channel.rs | 6 +- .../client/load_balancing/graceful_switch.rs | 8 +- grpc/src/client/load_balancing/pick_first.rs | 55 ++++++----- grpc/src/client/load_balancing/round_robin.rs | 8 +- grpc/src/client/name_resolution/dns/mod.rs | 7 +- grpc/src/client/name_resolution/dns/test.rs | 2 +- grpc/src/client/name_resolution/mod.rs | 69 +++++++------- grpc/src/client/name_resolution/unix.rs | 7 +- .../client/name_resolution/unix_abstract.rs | 10 +- grpc/src/client/subchannel.rs | 5 +- grpc/src/client/transport/mod.rs | 7 +- grpc/src/client/transport/tonic/mod.rs | 28 +----- grpc/src/client/transport/tonic/test.rs | 22 +---- grpc/src/inmemory/mod.rs | 24 ++--- 15 files changed, 117 insertions(+), 235 deletions(-) diff --git a/grpc/src/byte_str.rs b/grpc/src/byte_str.rs index cf349538c..971d3587c 100644 --- a/grpc/src/byte_str.rs +++ b/grpc/src/byte_str.rs @@ -21,38 +21,28 @@ * IN THE SOFTWARE. * */ + +use core::str; use std::ops::Deref; use bytes::Bytes; /// A cheaply cloneable and sliceable chunk of contiguous memory. -/// -/// The bytes held by `ByteStr` are arbitrary and may not be valid UTF-8. #[derive(Debug, Default, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct ByteStr { + // Invariant: bytes contains valid UTF-8 bytes: Bytes, } -impl ByteStr { - /// Strips a prefix, returning a new zero-copy ByteStr. - #[inline] - pub(crate) fn strip_prefix(&self, prefix: &[u8]) -> Option { - if self.starts_with(prefix) { - Some(ByteStr { - bytes: self.bytes.slice(prefix.len()..), - }) - } else { - None - } - } -} - impl Deref for ByteStr { - type Target = [u8]; + type Target = str; #[inline] - fn deref(&self) -> &[u8] { - &self.bytes + fn deref(&self) -> &str { + let b: &[u8] = self.bytes.as_ref(); + // The invariant of `bytes` is that it contains valid UTF-8 allows us + // to unwrap. + str::from_utf8(b).unwrap() } } @@ -60,72 +50,8 @@ impl From for ByteStr { #[inline] fn from(src: String) -> ByteStr { ByteStr { + // Invariant: src is a String so contains valid UTF-8. bytes: Bytes::from(src), } } } - -impl<'a> TryFrom<&'a ByteStr> for &'a str { - type Error = std::str::Utf8Error; - - #[inline] - fn try_from(value: &'a ByteStr) -> Result { - std::str::from_utf8(value) - } -} - -impl TryFrom for String { - type Error = std::str::Utf8Error; - - #[inline] - fn try_from(value: ByteStr) -> Result { - let s = std::str::from_utf8(&value)?; - Ok(s.to_owned()) - } -} - -impl PartialEq for ByteStr { - #[inline] - fn eq(&self, other: &str) -> bool { - self.bytes == other.as_bytes() - } -} - -impl<'a> PartialEq<&'a str> for ByteStr { - #[inline] - fn eq(&self, other: &&'a str) -> bool { - self.bytes == other.as_bytes() - } -} - -impl PartialEq for str { - #[inline] - fn eq(&self, other: &ByteStr) -> bool { - self.as_bytes() == other.bytes - } -} - -impl PartialEq for &str { - #[inline] - fn eq(&self, other: &ByteStr) -> bool { - self.as_bytes() == other.bytes - } -} - -impl FromIterator for ByteStr { - #[inline] - fn from_iter>(iter: T) -> Self { - ByteStr { - bytes: Bytes::from_iter(iter), - } - } -} - -impl From<&'static str> for ByteStr { - #[inline] - fn from(src: &'static str) -> ByteStr { - ByteStr { - bytes: Bytes::from_static(src.as_bytes()), - } - } -} diff --git a/grpc/src/client/channel.rs b/grpc/src/client/channel.rs index 3a3234290..b0531d3c2 100644 --- a/grpc/src/client/channel.rs +++ b/grpc/src/client/channel.rs @@ -36,7 +36,6 @@ use std::vec; use serde_json::json; use tokio::sync::mpsc; use tokio::sync::watch; -use url::Url; // NOTE: http::Uri requires non-empty authority portion of URI use crate::StatusCodeError; use crate::StatusError; @@ -248,10 +247,9 @@ impl PersistentChannel { options: ChannelOptions, credentials: Arc, ) -> Self { - // TODO(arjan-bal): Return errors here instead of panicking. - let target = Url::from_str(&target.into()).unwrap(); + // 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 target = name_resolution::Target::from(target); let authority = options .channel_authority .clone() diff --git a/grpc/src/client/load_balancing/graceful_switch.rs b/grpc/src/client/load_balancing/graceful_switch.rs index dcd1ea87e..eb38f6f1b 100644 --- a/grpc/src/client/load_balancing/graceful_switch.rs +++ b/grpc/src/client/load_balancing/graceful_switch.rs @@ -452,10 +452,12 @@ mod test { let PickResult::Pick(pick) = pick else { panic!("unexpected pick result: {:?}", pick); }; - let received_address = &pick.subchannel.address().address; + let received_address = &pick.subchannel.address().address.to_string(); + // It's good practice to create the expected value once. + let expected_address = name.to_string(); // Check for inequality and panic with a detailed message if they don't match. - assert_eq!(received_address, name); + assert_eq!(received_address, &expected_address); } fn move_subchannel_to_state( @@ -654,7 +656,7 @@ mod test { .resolver_update(update.clone(), Some(&parsed_config2), &mut *tcc) .unwrap(); let subchannel = verify_subchannel_creation_from_policy(&mut rx_events); - assert_eq!(subchannel.address().address, "127.0.0.1:1234"); + assert_eq!(&*subchannel.address().address, "127.0.0.1:1234"); assert_channel_empty(&mut rx_events); } diff --git a/grpc/src/client/load_balancing/pick_first.rs b/grpc/src/client/load_balancing/pick_first.rs index b4a2be1e4..48157ecbc 100644 --- a/grpc/src/client/load_balancing/pick_first.rs +++ b/grpc/src/client/load_balancing/pick_first.rs @@ -425,7 +425,7 @@ impl PickFirstPolicy { // Partition by family (Basic IPv6 detection via colon). let (ipv6, ipv4): (Vec
, Vec
) = tcp_addresses .into_iter() - .partition(|addr| addr.address.contains(&b':')); + .partition(|addr| addr.address.contains(':')); // Interleave the two lists so ipv6 and ipv4 addresses are alternated. let mut interleaved = Vec::with_capacity(ipv6.len() + ipv4.len() + unknown.len()); @@ -738,9 +738,9 @@ mod test { use std::time::Duration; use super::*; - use crate::client::load_balancing::test_utils::TestChannelController; - use crate::client::load_balancing::test_utils::TestEvent; - use crate::client::load_balancing::test_utils::TestWorkScheduler; + use crate::client::load_balancing::test_utils::{ + TestChannelController, TestEvent, TestWorkScheduler, + }; const DEFAULT_TEST_DURATION: Duration = Duration::from_secs(10); @@ -928,7 +928,7 @@ mod test { let res = state.picker.pick(&RequestHeaders::default()); match res { PickResult::Pick(pick) => { - assert_eq!(pick.subchannel.address().address, "addr1") + assert_eq!(pick.subchannel.address().address.to_string(), "addr1") } other => panic!("unexpected pick result {:?}", other), } @@ -942,7 +942,7 @@ mod test { // Should connect to addr2. let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr2"); + assert_eq!(addr.address.to_string(), "addr2"); // Simulate addr2 succeeding. let sc2 = policy.subchannels[1].clone(); @@ -986,16 +986,25 @@ mod test { // Should create new subchannel for addr2 (was cleared by cleanup). let sc2 = expect_new_subchannel(&rx); - assert_eq!(sc2.address().address, "addr2"); + assert_eq!(sc2.address().address.to_string(), "addr2"); // Should create new subchannel for addr3 (was not in previous list). let sc3 = expect_new_subchannel(&rx); - assert_eq!(sc3.address().address, "addr3"); + assert_eq!(sc3.address().address.to_string(), "addr3"); // Should NOT have any more events (no Connect, no UpdatePicker), // because it stuck to the original selected subchannel. assert!(rx.try_recv().is_err(), "unexpected event"); - assert_eq!(policy.selected.as_ref().unwrap().address().address, "addr1"); + assert_eq!( + policy + .selected + .as_ref() + .unwrap() + .address() + .address + .to_string(), + "addr1" + ); } // If all addresses fail during a connection pass, the LB should update to @@ -1079,7 +1088,7 @@ mod test { let mut resulting_addrs = Vec::with_capacity(NUM_ADDRS); for _ in 0..NUM_ADDRS { let sc = expect_new_subchannel(&rx); - resulting_addrs.push(sc.address().address); + resulting_addrs.push(sc.address().address.to_string()); } // Mock shuffler reverses endpoints: [EP3, EP2, EP1] @@ -1148,9 +1157,9 @@ mod test { // Should only create subchannels for addr1 and addr2 (2 unique addrs). let sc1 = expect_new_subchannel(&rx); - assert_eq!(sc1.address().address, "addr1"); + assert_eq!(sc1.address().address.to_string(), "addr1"); let sc2 = expect_new_subchannel(&rx); - assert_eq!(sc2.address().address, "addr2"); + assert_eq!(sc2.address().address.to_string(), "addr2"); // Verify no 3rd subchannel was created. while let Ok(event) = rx.try_recv() { @@ -1217,7 +1226,7 @@ mod test { // Expect Connect event for addr2 due to timer expiration. let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr2"); + assert_eq!(addr.address.to_string(), "addr2"); } // If all addresses fail during a connection pass, the LB should enter @@ -1250,7 +1259,7 @@ mod test { // Should automatically call connect() again. let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr1"); + assert_eq!(addr.address.to_string(), "addr1"); } // If the LB is in steady state, and a new address becomes ready, it should @@ -1265,7 +1274,7 @@ mod test { // Should failover to addr2: expect Connect(addr2). let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr2"); + assert_eq!(addr.address.to_string(), "addr2"); // While addr2 is connecting, simulate addr1 going IDLE (backoff over). policy.subchannel_update( @@ -1300,7 +1309,7 @@ mod test { ); expect_request_resolution(&rx); let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr1"); + assert_eq!(addr.address.to_string(), "addr1"); // Confirm LB is in steady state. assert!(policy.steady_state.is_some()); @@ -1317,7 +1326,7 @@ mod test { // Now it should automatically call connect() again. let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr1"); + assert_eq!(addr.address.to_string(), "addr1"); // Simulate addr1 successfully connecting and becoming READY. policy.subchannel_update( @@ -1335,7 +1344,7 @@ mod test { let res = state.picker.pick(&RequestHeaders::default()); match res { PickResult::Pick(pick) => { - assert_eq!(pick.subchannel.address().address, "addr1"); + assert_eq!(pick.subchannel.address().address.to_string(), "addr1"); } other => panic!("unexpected pick result {:?}", other), } @@ -1354,7 +1363,7 @@ mod test { // Expect Connect(addr2). let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr2"); + assert_eq!(addr.address.to_string(), "addr2"); // Simulate addr1 backing off and transitioning to IDLE early // (while addr2 is still connecting). @@ -1393,7 +1402,7 @@ mod test { // Expect an immediate Connect(addr1) event triggered by the exhaustion // loop sweeping up the early IDLE subchannel. let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr1"); + assert_eq!(addr.address.to_string(), "addr1"); } // This test is meant to validate that if a new address with different @@ -1492,7 +1501,7 @@ mod test { policy.work(None, controller.as_mut()); let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr2"); + assert_eq!(addr.address.to_string(), "addr2"); // 2. Simulate addr2 failing first while addr1 is still in flight. let sc2 = policy.subchannels[1].clone(); @@ -1574,7 +1583,7 @@ mod test { let res = state.picker.pick(&RequestHeaders::default()); let sc1 = match res { PickResult::Pick(pick) => { - assert_eq!(pick.subchannel.address().address, "addr1"); + assert_eq!(pick.subchannel.address().address.to_string(), "addr1"); pick.subchannel } other => panic!("unexpected pick result {:?}", other), @@ -1610,7 +1619,7 @@ mod test { // 7. Verify that the policy initiates a reconnection to addr1. let addr = expect_connect(&rx); - assert_eq!(addr.address, "addr1"); + assert_eq!(addr.address.to_string(), "addr1"); // And the picker goes to Connecting. let state = expect_picker_update(&rx); diff --git a/grpc/src/client/load_balancing/round_robin.rs b/grpc/src/client/load_balancing/round_robin.rs index 9cc24f73a..505fb9a87 100644 --- a/grpc/src/client/load_balancing/round_robin.rs +++ b/grpc/src/client/load_balancing/round_robin.rs @@ -1018,11 +1018,11 @@ mod test { let all_subchannels = verify_subchannel_creation(&mut rx_events, 2); let subchannel_one = all_subchannels .iter() - .find(|sc| sc.address().address == "subchannel_one") + .find(|sc| sc.address().address == "subchannel_one".to_string().into()) .unwrap(); let subchannel_two = all_subchannels .iter() - .find(|sc| sc.address().address == "subchannel_two") + .find(|sc| sc.address().address == "subchannel_two".to_string().into()) .unwrap(); move_subchannel_to_state( @@ -1081,11 +1081,11 @@ mod test { let new_subchannels = verify_subchannel_creation(&mut rx_events, 2); let new_sc = new_subchannels .iter() - .find(|sc| sc.address().address == "new") + .find(|sc| sc.address().address == "new".to_string().into()) .unwrap(); let old_sc = new_subchannels .iter() - .find(|sc| sc.address().address == "subchannel_two") + .find(|sc| sc.address().address == "subchannel_two".to_string().into()) .unwrap(); move_subchannel_to_state( diff --git a/grpc/src/client/name_resolution/dns/mod.rs b/grpc/src/client/name_resolution/dns/mod.rs index cfab549a6..ce47dfc42 100644 --- a/grpc/src/client/name_resolution/dns/mod.rs +++ b/grpc/src/client/name_resolution/dns/mod.rs @@ -315,11 +315,8 @@ struct ParseResult { fn parse_endpoint_and_authority(target: &Target) -> Result { // Parse the endpoint. let endpoint = target.path(); - let endpoint = endpoint.strip_prefix(b"/").unwrap_or(endpoint); - let host_port = (&endpoint) - .try_into() - .map_err(|err| format!("target hostname contains invalid UTF-8 symbols: {err}"))?; - let parse_result = parse_host_port(host_port, DEFAULT_PORT) + let endpoint = endpoint.strip_prefix("/").unwrap_or(endpoint); + let parse_result = parse_host_port(endpoint, DEFAULT_PORT) .map_err(|err| format!("Failed to parse target {target}: {err}"))?; let endpoint = parse_result.ok_or("Received empty endpoint host.".to_string())?; diff --git a/grpc/src/client/name_resolution/dns/test.rs b/grpc/src/client/name_resolution/dns/test.rs index eb6f823a3..f085f695a 100644 --- a/grpc/src/client/name_resolution/dns/test.rs +++ b/grpc/src/client/name_resolution/dns/test.rs @@ -124,7 +124,7 @@ pub(crate) fn target_parsing() { }), }, TestCase { - input: "dns:///[fe80::1%80]:5678/abc", + input: "dns:///[fe80::1%2580]:5678/abc", want_result: Err("SocketAddr doesn't support IPv6 addresses with zones".to_string()), }, TestCase { diff --git a/grpc/src/client/name_resolution/mod.rs b/grpc/src/client/name_resolution/mod.rs index 91a900e43..d2493e833 100644 --- a/grpc/src/client/name_resolution/mod.rs +++ b/grpc/src/client/name_resolution/mod.rs @@ -68,22 +68,19 @@ pub(crate) use registry::global_registry; #[derive(Debug, Clone)] pub(crate) struct Target { url: Url, + decoded_path: String, } impl FromStr for Target { type Err = String; fn from_str(s: &str) -> Result { - match s.parse::() { - Ok(url) => Ok(Target { url }), - Err(err) => Err(err.to_string()), - } - } -} - -impl From for Target { - fn from(url: url::Url) -> Self { - Target { url } + let url = s.parse::().map_err(|err| err.to_string())?; + let decoded_path = percent_decode_str(url.path()) + .decode_utf8() + .map_err(|err| format!("invalid UTF-8 character in target path: {err}"))? + .into_owned(); + Ok(Target { url, decoded_path }) } } @@ -125,8 +122,8 @@ impl Target { } /// Retrieves the percent-decoded endpoint from `Url.path()`. - pub fn path(&self) -> ByteStr { - percent_decode_str(self.url.path()).collect() + pub fn path(&self) -> &str { + &self.decoded_path } } @@ -137,7 +134,7 @@ impl Display for Target { "{}://{}{}", self.scheme(), self.authority_host_port(), - String::from_utf8_lossy(&self.path()) + self.decoded_path ) } } @@ -164,8 +161,7 @@ pub(crate) trait ResolverBuilder: Send + Sync { /// with the leading prefix removed. fn default_authority(&self, target: &Target) -> String { let path = target.path(); - let path = path.strip_prefix(b"/").unwrap_or(path); - String::from_utf8_lossy(&path).into() + path.strip_prefix("/").unwrap_or(path).to_string() } /// Returns a bool indicating whether the input uri is valid to create a @@ -323,13 +319,9 @@ impl Hash for Address { } impl Display for Address { + #[allow(clippy::to_string_in_format_args)] fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { - write!( - f, - "{}:{}", - self.network_type, - String::from_utf8_lossy(&self.address) - ) + write!(f, "{}:{}", self.network_type, self.address.to_string()) } } @@ -404,7 +396,7 @@ mod test { want_host: &'static str, want_port: Option, want_host_port: &'static str, - want_path: &'static [u8], + want_path: &'static str, want_str: &'static str, } let test_cases = vec![ @@ -414,7 +406,7 @@ mod test { want_host_port: "", want_host: "", want_port: None, - want_path: b"/grpc.io", + want_path: "/grpc.io", want_str: "dns:///grpc.io", }, TestCase { @@ -423,7 +415,7 @@ mod test { want_host_port: "8.8.8.8:53", want_host: "8.8.8.8", want_port: Some(53), - want_path: b"/grpc.io/docs", + want_path: "/grpc.io/docs", want_str: "dns://8.8.8.8:53/grpc.io/docs", }, TestCase { @@ -432,7 +424,7 @@ mod test { want_host_port: "", want_host: "", want_port: None, - want_path: b"path/to/file", + want_path: "path/to/file", want_str: "unix://path/to/file", }, TestCase { @@ -441,7 +433,7 @@ mod test { want_host_port: "", want_host: "", want_port: None, - want_path: b"/run/containerd/containerd.sock", + want_path: "/run/containerd/containerd.sock", want_str: "unix:///run/containerd/containerd.sock", }, TestCase { @@ -450,18 +442,9 @@ mod test { want_host_port: "", want_host: "", want_port: None, - want_path: b"/foo bar", + want_path: "/foo bar", want_str: "dns:///foo bar", }, - TestCase { - input: "dns:///foo%FFbar", - want_scheme: "dns", - want_host_port: "", - want_host: "", - want_port: None, - want_path: b"/foo\xffbar", - want_str: "dns:///foo\u{FFFD}bar", - }, ]; for tc in test_cases { @@ -470,11 +453,23 @@ mod test { assert_eq!(target.authority_host(), tc.want_host); assert_eq!(target.authority_port(), tc.want_port); assert_eq!(target.authority_host_port(), tc.want_host_port); - assert_eq!(&*target.path(), tc.want_path); + assert_eq!(target.path(), tc.want_path); assert_eq!(&target.to_string(), tc.want_str); } } + #[test] + fn parse_target_invalid_utf8() { + let input = "dns:///foo%FFbar"; + let target: Result = input.parse(); + assert!(target.is_err()); + assert!( + target + .unwrap_err() + .contains("invalid UTF-8 character in target path") + ); + } + // This test ensures that the Address struct correctly maintains its // asymmetric PartialEq and Hash contracts. // Specifically, two addresses with the same physical coordinates but diff --git a/grpc/src/client/name_resolution/unix.rs b/grpc/src/client/name_resolution/unix.rs index 0b0b40619..e87b12ff8 100644 --- a/grpc/src/client/name_resolution/unix.rs +++ b/grpc/src/client/name_resolution/unix.rs @@ -23,6 +23,7 @@ */ use crate::attributes::Attributes; +use crate::byte_str::ByteStr; use crate::client::name_resolution::Address; use crate::client::name_resolution::NopResolver; use crate::client::name_resolution::ResolverBuilder; @@ -58,7 +59,7 @@ impl ResolverBuilder for Builder { } fn default_authority(&self, target: &Target) -> String { - "localhost".into() + "localhost".to_owned() } } @@ -75,9 +76,10 @@ fn parse_target(target: &Target) -> Result { if !host_port.is_empty() { return Err(format!("invalid (non-empty) authority: {host_port}")); } + let addr_string = target.path().to_owned(); Ok(Address { network_type: UNIX_NETWORK_TYPE, - address: target.path(), + address: ByteStr::from(addr_string), attributes: Attributes::new(), }) } @@ -85,7 +87,6 @@ fn parse_target(target: &Target) -> Result { #[cfg(test)] mod tests { use super::*; - use crate::byte_str::ByteStr; use crate::client::name_resolution::Endpoint; use crate::client::name_resolution::ResolverOptions; use crate::client::name_resolution::test_utils::TestChannelController; diff --git a/grpc/src/client/name_resolution/unix_abstract.rs b/grpc/src/client/name_resolution/unix_abstract.rs index f11144bcd..b74cabd5a 100644 --- a/grpc/src/client/name_resolution/unix_abstract.rs +++ b/grpc/src/client/name_resolution/unix_abstract.rs @@ -23,6 +23,7 @@ */ use crate::attributes::Attributes; +use crate::byte_str::ByteStr; use crate::client::name_resolution::Address; use crate::client::name_resolution::NopResolver; use crate::client::name_resolution::ResolverBuilder; @@ -58,7 +59,7 @@ impl ResolverBuilder for Builder { } fn default_authority(&self, _target: &Target) -> String { - "localhost".into() + "localhost".to_owned() } } @@ -77,12 +78,10 @@ fn parse_target(target: &Target) -> Result { if !host_port.is_empty() { return Err(format!("invalid (non-empty) authority: {host_port}")); } - let addr = std::iter::once(b'\0') - .chain(target.path().iter().copied()) - .collect(); + let addr_string = format!("\0{}", target.path()); Ok(Address { network_type: UNIX_NETWORK_TYPE, - address: addr, + address: ByteStr::from(addr_string), attributes: Attributes::new(), }) } @@ -90,7 +89,6 @@ fn parse_target(target: &Target) -> Result { #[cfg(test)] mod tests { use super::*; - use crate::byte_str::ByteStr; use crate::client::name_resolution::Endpoint; use crate::client::name_resolution::ResolverOptions; use crate::client::name_resolution::test_utils::TestChannelController; diff --git a/grpc/src/client/subchannel.rs b/grpc/src/client/subchannel.rs index f820d8ff8..9f95e6d2b 100644 --- a/grpc/src/client/subchannel.rs +++ b/grpc/src/client/subchannel.rs @@ -39,7 +39,6 @@ use tonic::async_trait; use crate::StatusCodeError; use crate::StatusError; -use crate::byte_str::ByteStr; use crate::client::CallOptions; use crate::client::ConnectivityState; use crate::client::DynInvoke; @@ -244,7 +243,7 @@ pub(crate) struct InternalSubchannel { } struct InternalSubchannelData { - address: ByteStr, + address: String, state: InternalSubchannelState, work_queue: WorkQueueTx, on_drop: Arc, @@ -311,7 +310,7 @@ impl InternalSubchannel { work_queue: WorkQueueTx, ) -> Arc { let on_drop = Arc::new(Notify::new()); - let address_string = address.address.clone(); + let address_string = address.address.to_string(); let this = Arc::new_cyclic(|weak_self| Self { address, on_drop: on_drop.clone(), diff --git a/grpc/src/client/transport/mod.rs b/grpc/src/client/transport/mod.rs index 107eb7f83..35b1ac421 100644 --- a/grpc/src/client/transport/mod.rs +++ b/grpc/src/client/transport/mod.rs @@ -26,7 +26,6 @@ use std::sync::Arc; use std::time::Duration; use std::time::Instant; -use crate::byte_str::ByteStr; use crate::client::DynInvoke; use crate::client::Invoke; use crate::credentials::client::ClientHandshakeInfo; @@ -72,7 +71,7 @@ pub(crate) trait Transport: Sync { async fn connect( &self, - address: ByteStr, + address: String, runtime: GrpcRuntime, security_opts: &SecurityOpts, opts: &TransportOptions, @@ -90,7 +89,7 @@ pub(crate) trait Transport: Sync { pub(crate) trait DynTransport: Send + Sync { async fn dyn_connect( &self, - address: ByteStr, + address: String, runtime: GrpcRuntime, security_opts: &SecurityOpts, opts: &TransportOptions, @@ -108,7 +107,7 @@ pub(crate) trait DynTransport: Send + Sync { impl DynTransport for T { async fn dyn_connect( &self, - address: ByteStr, + address: String, runtime: GrpcRuntime, security_opts: &SecurityOpts, opts: &TransportOptions, diff --git a/grpc/src/client/transport/tonic/mod.rs b/grpc/src/client/transport/tonic/mod.rs index 819dd14b9..c736d9a6d 100644 --- a/grpc/src/client/transport/tonic/mod.rs +++ b/grpc/src/client/transport/tonic/mod.rs @@ -70,7 +70,6 @@ use tower_service::Service as TowerService; use crate::StatusCodeError; use crate::StatusError; -use crate::byte_str::ByteStr; use crate::client::CallOptions; use crate::client::Invoke; use crate::client::RecvStream; @@ -364,7 +363,7 @@ impl Transport for TransportBuilder { async fn connect( &self, - address: ByteStr, + address: String, runtime: GrpcRuntime, security_info: &SecurityOpts, opts: &TransportOptions, @@ -406,11 +405,8 @@ impl Transport for TransportBuilder { let transport_fut = match self.network_type { NetworkType::Tcp => { - let addr_str: &str = (&address) - .try_into() - .map_err(|err| format!("address contains non-UTF-8 symbols: {err}"))?; let addr: SocketAddr = - SocketAddr::from_str(addr_str).map_err(|err| err.to_string())?; + SocketAddr::from_str(&address).map_err(|err| err.to_string())?; runtime.tcp_stream( addr, TcpOptions { @@ -420,25 +416,7 @@ impl Transport for TransportBuilder { ) } NetworkType::Unix => { - #[cfg(unix)] - { - use std::ffi::OsStr; - use std::os::unix::ffi::OsStrExt; - runtime.unix_stream( - PathBuf::from(OsStr::from_bytes(&address)), - UnixSocketOptions::default(), - ) - } - #[cfg(not(unix))] - { - match std::str::from_utf8(&address) { - Ok(valid_str) => runtime - .unix_stream(PathBuf::from(valid_str), UnixSocketOptions::default()), - Err(_) => Box::pin(async move { - Err("socket path contains non-UTF-8 characters".to_string()) - }), - } - } + runtime.unix_stream(PathBuf::from(&address), UnixSocketOptions::default()) } }; let transport = if let Some(deadline) = opts.connect_deadline { diff --git a/grpc/src/client/transport/tonic/test.rs b/grpc/src/client/transport/tonic/test.rs index d2f82f45f..6dc3fe478 100644 --- a/grpc/src/client/transport/tonic/test.rs +++ b/grpc/src/client/transport/tonic/test.rs @@ -157,7 +157,7 @@ pub(crate) async fn tonic_transport_rpc() { }; let (conn, _sec_info, mut disconnection_listener) = builder .dyn_connect( - addr.to_string().into(), + addr.to_string(), GrpcRuntime::new(TokioRuntime::default()), &securty_opts, &config, @@ -323,22 +323,6 @@ mod unix_tests { run_unix_test(&socket_path, &target).await; } - #[tokio::test] - #[cfg(target_os = "linux")] - async fn unix_absolute_path_non_utf8() { - use std::ffi::OsStr; - use std::os::unix::ffi::OsStrExt; - - let dir = tempdir().expect("failed to create temp dir"); - let mut socket_path_bytes = dir.path().as_os_str().as_bytes().to_vec(); - // Use an invalid UTF-8 byte sequence to ensure it is handled correctly. - socket_path_bytes.extend_from_slice(b"/absolute\xff.sock"); - let socket_path = PathBuf::from(OsStr::from_bytes(&socket_path_bytes)); - let target = format!("unix://{}/absolute%FF.sock", dir.path().to_str().unwrap()); - - run_unix_test(&socket_path, &target).await; - } - #[tokio::test] async fn unix_relative_path() { let dir = tempdir().expect("failed to create temp dir"); @@ -690,7 +674,7 @@ async fn tonic_transport_invalid_base64_headers() { }; let (conn, _sec_info, _disconnection_listener) = builder .dyn_connect( - addr.to_string().into(), + addr.to_string(), GrpcRuntime::new(TokioRuntime::default()), &securty_opts, &config, @@ -765,7 +749,7 @@ async fn tonic_transport_recv_drop_cancels_send() { }; let (conn, _sec_info, _disconnection_listener) = builder .dyn_connect( - addr.to_string().into(), + addr.to_string(), GrpcRuntime::new(TokioRuntime::default()), &securty_opts, &config, diff --git a/grpc/src/inmemory/mod.rs b/grpc/src/inmemory/mod.rs index ac517e4eb..24354d2d9 100644 --- a/grpc/src/inmemory/mod.rs +++ b/grpc/src/inmemory/mod.rs @@ -38,7 +38,6 @@ use tokio::sync::oneshot; use crate::StatusCodeError; use crate::StatusError; use crate::attributes::Attributes; -use crate::byte_str::ByteStr; use crate::client::CallOptions; use crate::client::DynRecvStream as ClientDynRecvStream; use crate::client::DynSendStream as ClientDynSendStream; @@ -79,7 +78,7 @@ use crate::server::ResponseStreamItem as ServerResponseStreamItem; use crate::server::SendOptions as ServerSendOptions; use crate::server::SendStream as ServerSendStream; -static LISTENERS: LazyLock>>> = +static LISTENERS: LazyLock>>> = LazyLock::new(|| Mutex::new(HashMap::new())); static NEXT_ID: AtomicU64 = AtomicU64::new(0); @@ -107,7 +106,7 @@ pub struct InMemoryListener { } struct InMemoryListenerInner { - id: ByteStr, + id: String, r: TokioMutex>, close_notify: Arc, drop_notify: Arc, @@ -127,7 +126,7 @@ impl Default for InMemoryListener { impl InMemoryListener { pub fn new() -> Self { - let id: ByteStr = NEXT_ID.fetch_add(1, Ordering::Relaxed).to_string().into(); + let id = NEXT_ID.fetch_add(1, Ordering::Relaxed).to_string(); let (s, r) = mpsc::channel(1); let mut listeners = LISTENERS.lock().unwrap(); listeners.insert(id.clone(), s); @@ -141,7 +140,7 @@ impl InMemoryListener { } } - pub fn id(&self) -> ByteStr { + pub fn id(&self) -> String { self.inner.id.clone() } @@ -344,7 +343,7 @@ impl Transport for InMemoryTransport { async fn connect( &self, - target: ByteStr, + target: String, _runtime: GrpcRuntime, _security_opts: &SecurityOpts, _options: &TransportOptions, @@ -359,7 +358,7 @@ impl Transport for InMemoryTransport { let listeners = LISTENERS.lock().unwrap(); let s = listeners .get(&target) - .ok_or_else(|| format!("no listener for target: {:?}", target))?; + .ok_or_else(|| format!("no listener for target: {}", target))?; let (closed_tx, closed_rx) = oneshot::channel(); let conn = InMemoryConnection { @@ -392,11 +391,8 @@ pub struct InMemoryResolverBuilder {} impl ResolverBuilder for InMemoryResolverBuilder { fn build(&self, target: &Target, options: ResolverOptions) -> Box { - let path = target.path().strip_prefix(b"/").unwrap_or(target.path()); - let ids: Vec = path - .split(|&b| b == b',') - .map(|chunk| chunk.iter().copied().collect()) - .collect(); + let path = target.path().strip_prefix('/').unwrap_or(target.path()); + let ids: Vec = path.split(',').map(|s| s.to_string()).collect(); options.work_scheduler.schedule_work(); Box::new(InMemoryResolver { ids }) } @@ -411,7 +407,7 @@ impl ResolverBuilder for InMemoryResolverBuilder { } struct InMemoryResolver { - ids: Vec, + ids: Vec, } impl Resolver for InMemoryResolver { @@ -424,7 +420,7 @@ impl Resolver for InMemoryResolver { .map(|id| Endpoint { addresses: vec![Address { network_type: "inmemory", - address: id.clone(), + address: crate::byte_str::ByteStr::from(id.clone()), ..Default::default() }], ..Default::default() From 9d2964b92b9c534e4b69ba11915449f967bd0a48 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 15 Jun 2026 23:38:46 +0530 Subject: [PATCH 13/21] fix merge --- .../client/name_resolution/proxy_resolver.rs | 38 ++++++++----------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/grpc/src/client/name_resolution/proxy_resolver.rs b/grpc/src/client/name_resolution/proxy_resolver.rs index ce318338d..86ae5b08e 100644 --- a/grpc/src/client/name_resolution/proxy_resolver.rs +++ b/grpc/src/client/name_resolution/proxy_resolver.rs @@ -119,21 +119,15 @@ impl Builder { return Ok(self.child_builder.build(target, options)); }; let path = target.path(); - let path = path.strip_prefix(b"/").unwrap_or(path); + let path = path.strip_prefix('/').unwrap_or(path); // Attempt to convert the path to a UTF-8 string, then parse it as a // URL host. - let url_obj = match (&path) - .try_into() - .map_err(|err| format!("non-UTF-8 symbol in target host: {err}")) - .and_then(|target_host: &str| { - let target_host = authority_with_default_port(target_host, 443); - Url::parse(&format!("https://{target_host}")) - .map_err(|err| format!("invalid target host in URL: {err}")) - }) { + let target_host = authority_with_default_port(path, 443); + let url_obj = match Url::parse(&format!("https://{target_host}")) { Ok(url) => url, Err(err) => { - return Err((err, options)); + return Err((format!("invalid target host in URL: {err}"), options)); } }; @@ -444,8 +438,8 @@ mod tests { .await; assert_eq!(addresses.len(), 2); - assert_eq!(addresses[0].address, "127.0.0.1:8080"); - assert_eq!(addresses[1].address, "[::1]:8080"); + assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); + assert_eq!(&*addresses[1].address, "[::1]:8080"); let mut expected_header = HeaderValue::from_static("Basic dXNlcjpwYXNzd29yZA=="); expected_header.set_sensitive(true); @@ -475,7 +469,7 @@ mod tests { .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); assert!(proxy_options_for_addr(&addresses[0]).is_none()); } @@ -560,7 +554,7 @@ mod tests { .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); assert!(proxy_options_for_addr(&addresses[0]).is_none()); // Check for abstract-unix scheme. @@ -572,7 +566,7 @@ mod tests { .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); assert!(proxy_options_for_addr(&addresses[0]).is_none()); } @@ -589,7 +583,7 @@ mod tests { run_resolver_and_get_addresses("dns:///target.example.com", dns_ips(), Some(&matcher)) .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); assert!( proxy_options_for_addr(&addresses[0]).is_none(), "HTTP proxy should not match HTTPS destinations" @@ -604,7 +598,7 @@ mod tests { run_resolver_and_get_addresses("dns:///target.example.com", dns_ips(), Some(&matcher)) .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, "127.0.0.1:8080"); + assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); assert!( proxy_options_for_addr(&addresses[0]).is_some(), "HTTPS proxy should match HTTPS destinations" @@ -621,7 +615,7 @@ mod tests { run_resolver_and_get_addresses("dns:///target.example.com", dns_ips(), Some(&matcher)) .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); assert!(proxy_options_for_addr(&addresses[0]).is_none()); // Target B: other.example.com (NOT matched by no_proxy) -> should proxy @@ -629,7 +623,7 @@ mod tests { run_resolver_and_get_addresses("dns:///other.example.com", dns_ips(), Some(&matcher)) .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, "127.0.0.1:8080"); + assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); assert!(proxy_options_for_addr(&addresses[0]).is_some()); } @@ -643,7 +637,7 @@ mod tests { .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, DIRECT_ADDRESS); + assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); assert!(proxy_options_for_addr(&addresses[0]).is_none()); } @@ -659,7 +653,7 @@ mod tests { .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, "[::1]:8080"); + assert_eq!(&*addresses[0].address, "[::1]:8080"); let expected_proxy_opts = ProxyOptions { proxy_authorization_header: None, @@ -684,7 +678,7 @@ mod tests { .await; assert_eq!(addresses.len(), 1); - assert_eq!(addresses[0].address, "127.0.0.1:8080"); + assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); let expected_proxy_opts = ProxyOptions { proxy_authorization_header: None, From 9306284bca416595d3e4258adfd5eb5ebf4d9449 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Tue, 16 Jun 2026 00:10:00 +0530 Subject: [PATCH 14/21] disable unecessary feature --- grpc/Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/grpc/Cargo.toml b/grpc/Cargo.toml index 5fce485a2..fe5040c9b 100644 --- a/grpc/Cargo.toml +++ b/grpc/Cargo.toml @@ -55,9 +55,10 @@ bytes = "1.10.1" futures = { version = "0.3", default-features = false, optional = true } hickory-resolver = { version = "0.25.1", optional = true } http = "1.1.0" +httparse = "1.9" http-body = "1.0.1" hyper = { version = "1.6.0", features = ["client", "http2"] } -hyper-util = { version = "0.1", features = ["client", "client-proxy"] } +hyper-util = { version = "0.1", features = ["client-proxy"] } itoa = "1.0" parking_lot = "0.12.4" percent-encoding = "2.3" From 04281233b29716eb7bb98ef6c7c879207f590469 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Thu, 18 Jun 2026 23:39:18 +0530 Subject: [PATCH 15/21] fix udeps --- grpc/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/grpc/Cargo.toml b/grpc/Cargo.toml index 339f2ac06..4e78c8b61 100644 --- a/grpc/Cargo.toml +++ b/grpc/Cargo.toml @@ -55,7 +55,6 @@ bytes = "1.10.1" futures = { version = "0.3", default-features = false, optional = true } hickory-resolver = { version = "0.25.1", optional = true } http = "1.1.0" -httparse = "1.9" http-body = "1.0.1" hyper = { version = "1.6.0", features = ["client", "http2"] } hyper-util = { version = "0.1", features = ["client-proxy"] } From dd4f6bc3686714b276531cfd33e6365fd343a1d8 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Fri, 19 Jun 2026 00:04:20 +0530 Subject: [PATCH 16/21] fixes --- .../client/name_resolution/proxy_resolver.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/grpc/src/client/name_resolution/proxy_resolver.rs b/grpc/src/client/name_resolution/proxy_resolver.rs index 86ae5b08e..b10cf3ca0 100644 --- a/grpc/src/client/name_resolution/proxy_resolver.rs +++ b/grpc/src/client/name_resolution/proxy_resolver.rs @@ -118,11 +118,11 @@ impl Builder { let Some(matcher) = matcher else { return Ok(self.child_builder.build(target, options)); }; + let path = target.path(); let path = path.strip_prefix('/').unwrap_or(path); - // Attempt to convert the path to a UTF-8 string, then parse it as a - // URL host. + // Use the URL crate to validate the authority and punycode encode it. let target_host = authority_with_default_port(path, 443); let url_obj = match Url::parse(&format!("https://{target_host}")) { Ok(url) => url, @@ -137,8 +137,6 @@ impl Builder { let port = url_obj.port().unwrap_or(443); let explicit_authority = format!("{host}:{port}"); - // Safely build `http::Uri` with the explicit authority (guaranteed - // ASCII/Punycode). let uri = match http::Uri::builder() .scheme("https") .authority(explicit_authority.as_str()) @@ -176,9 +174,10 @@ impl Builder { )); }; - // `proxy_host` must be a valid URL authority. Because the `url` crate - // implements the WHATWG standard, it leaves `[]` unescaped in the path. - // Therefore, we don't need to explicitly percent-decode the host string. + // `proxy_host` is be a valid URL authority. Because the `url` crate + // parses the target using the WHATWG standard, it allows unescaped `[]` + // characters in the path. Therefore, we don't need to explicitly + // percent-encode the host string when adding it to the target path. let target_str = format!("dns:///{}", proxy_host); let proxy_target: Target = match target_str.parse() { Ok(t) => t, @@ -295,7 +294,6 @@ mod tests { use super::*; use crate::attributes::Attributes; use crate::byte_str::ByteStr; - use crate::client::name_resolution::Address; use crate::client::name_resolution::test_utils::TestChannelController; use crate::client::name_resolution::test_utils::TestWorkScheduler; use crate::rt; @@ -455,7 +453,7 @@ mod tests { } #[tokio::test] - async fn proxy_non_matched() { + async fn proxy_not_matched() { let matcher = Matcher::builder() .https("http://proxy.example.com:8080") .no("target.example.com") @@ -540,7 +538,7 @@ mod tests { } #[tokio::test] - async fn invalid_unix_path_works() { + async fn unix_path_bypass() { let matcher = Matcher::builder() .https("http://proxy.example.com:8080") .build(); From c1af9e6c572c01f2ad32aa8b00e798041ff42887 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 22 Jun 2026 14:18:36 +0530 Subject: [PATCH 17/21] use builder to resolver authority --- .../client/name_resolution/proxy_resolver.rs | 96 ++++++++++++++++--- 1 file changed, 84 insertions(+), 12 deletions(-) diff --git a/grpc/src/client/name_resolution/proxy_resolver.rs b/grpc/src/client/name_resolution/proxy_resolver.rs index b10cf3ca0..773d34ad7 100644 --- a/grpc/src/client/name_resolution/proxy_resolver.rs +++ b/grpc/src/client/name_resolution/proxy_resolver.rs @@ -119,12 +119,10 @@ impl Builder { return Ok(self.child_builder.build(target, options)); }; - let path = target.path(); - let path = path.strip_prefix('/').unwrap_or(path); - + let target_authority = self.child_builder.default_authority(target); // Use the URL crate to validate the authority and punycode encode it. - let target_host = authority_with_default_port(path, 443); - let url_obj = match Url::parse(&format!("https://{target_host}")) { + let target_authority = authority_with_default_port(&target_authority, 443); + let url_obj = match Url::parse(&format!("https://{target_authority}")) { Ok(url) => url, Err(err) => { return Err((format!("invalid target host in URL: {err}"), options)); @@ -380,12 +378,12 @@ mod tests { } } - async fn run_resolver_and_get_addresses( + async fn run_resolver_and_get_addresses_with_builder( target_uri: &str, dns_ips: Vec, matcher: Option<&Matcher>, + child_builder: Arc, ) -> Vec
{ - let child_builder = Arc::new(MockResolverBuilder { scheme: "dns" }); let builder = Builder::new(child_builder); let target: Target = target_uri.parse().unwrap(); @@ -424,6 +422,16 @@ mod tests { addresses } + async fn run_resolver_and_get_addresses( + target_uri: &str, + dns_ips: Vec, + matcher: Option<&Matcher>, + ) -> Vec
{ + let child_builder = Arc::new(MockResolverBuilder { scheme: "dns" }); + run_resolver_and_get_addresses_with_builder(target_uri, dns_ips, matcher, child_builder) + .await + } + #[tokio::test] async fn proxy_matched() { let matcher = Matcher::builder() @@ -597,10 +605,12 @@ mod tests { .await; assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); - assert!( - proxy_options_for_addr(&addresses[0]).is_some(), - "HTTPS proxy should match HTTPS destinations" - ); + let expected_proxy_opts = ProxyOptions { + proxy_authorization_header: None, + connect_addr: "target.example.com:443".to_string(), + }; + let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + assert_eq!(proxy_opts, &expected_proxy_opts); // Case 3: https proxy and no proxy are configured. let matcher = Matcher::builder() @@ -622,7 +632,12 @@ mod tests { .await; assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); - assert!(proxy_options_for_addr(&addresses[0]).is_some()); + let expected_proxy_opts = ProxyOptions { + proxy_authorization_header: None, + connect_addr: "other.example.com:443".to_string(), + }; + let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + assert_eq!(proxy_opts, &expected_proxy_opts); } #[tokio::test] @@ -686,4 +701,61 @@ mod tests { let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); } + + struct CustomAuthorityBuilder { + default_authority: String, + } + + impl ResolverBuilder for CustomAuthorityBuilder { + fn build(&self, _target: &Target, options: ResolverOptions) -> Box { + let addr = Address { + network_type: "tcp", + address: ByteStr::from(DIRECT_ADDRESS.to_string()), + attributes: Attributes::new(), + }; + NopResolver::new_with_addr(addr, options) + } + + fn scheme(&self) -> &str { + "custom" + } + + fn is_valid_uri(&self, _uri: &Target) -> bool { + true + } + + fn default_authority(&self, _target: &Target) -> String { + self.default_authority.clone() + } + } + + #[tokio::test] + async fn custom_resolver_builder_default_authority() { + let matcher = Matcher::builder() + .https("http://proxy.example.com:8080") + .build(); + + let custom_authority = "custom.authority.example.com:1234".to_string(); + let child_builder = Arc::new(CustomAuthorityBuilder { + default_authority: custom_authority.clone(), + }); + + let dns_ips = vec!["127.0.0.1".parse().unwrap()]; + let addresses = run_resolver_and_get_addresses_with_builder( + "custom:///whatever", + dns_ips, + Some(&matcher), + child_builder, + ) + .await; + + assert_eq!(addresses.len(), 1); + let expected_proxy_opts = ProxyOptions { + proxy_authorization_header: None, + connect_addr: "custom.authority.example.com:1234".to_string(), + }; + + let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + assert_eq!(proxy_opts, &expected_proxy_opts); + } } From b0c82896a639afadb5b554fc9ab550c046e73869 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 22 Jun 2026 15:47:40 +0530 Subject: [PATCH 18/21] restrict proxy to dns --- .../client/name_resolution/proxy_resolver.rs | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/grpc/src/client/name_resolution/proxy_resolver.rs b/grpc/src/client/name_resolution/proxy_resolver.rs index 773d34ad7..2ba4f0e1f 100644 --- a/grpc/src/client/name_resolution/proxy_resolver.rs +++ b/grpc/src/client/name_resolution/proxy_resolver.rs @@ -110,8 +110,8 @@ impl Builder { options: ResolverOptions, matcher: Option<&Matcher>, ) -> Result, (String, ResolverOptions)> { - // Skip proxy lookup for known non-TCP schemes. - if matches!(target.scheme(), "unix" | "unix-abstract") { + // Skip proxy lookup for non-DNS targets. + if target.scheme() != "dns" { return Ok(self.child_builder.build(target, options)); } // If HTTPS_PROXY is unset, avoid parsing the target as a DNS hostname. @@ -355,9 +355,7 @@ mod tests { } } - struct MockResolverBuilder { - scheme: &'static str, - } + struct MockResolverBuilder {} impl ResolverBuilder for MockResolverBuilder { fn build(&self, _target: &Target, options: ResolverOptions) -> Box { @@ -370,7 +368,7 @@ mod tests { } fn scheme(&self) -> &str { - self.scheme + "dns" } fn is_valid_uri(&self, _uri: &Target) -> bool { @@ -427,7 +425,7 @@ mod tests { dns_ips: Vec, matcher: Option<&Matcher>, ) -> Vec
{ - let child_builder = Arc::new(MockResolverBuilder { scheme: "dns" }); + let child_builder = Arc::new(MockResolverBuilder {}); run_resolver_and_get_addresses_with_builder(target_uri, dns_ips, matcher, child_builder) .await } @@ -504,16 +502,16 @@ mod tests { } #[tokio::test] - async fn invalid_custom_scheme_path_with_proxy_errors() { + async fn invalid_path_with_proxy_errors() { let matcher = Matcher::builder() .https("http://proxy.example.com:8080") .build(); // The path has a space in the first segment of the path, which makes it // an invalid hostname. - let target_uri = "custom:///var%20/run/grpc.sock"; + let target_uri = "dns:///var%20/run/grpc.sock"; - let child_builder = Arc::new(MockResolverBuilder { scheme: "custom" }); + let child_builder = Arc::new(MockResolverBuilder {}); let builder = Builder::new(child_builder); let target: Target = target_uri.parse().unwrap(); @@ -717,7 +715,7 @@ mod tests { } fn scheme(&self) -> &str { - "custom" + "dns" } fn is_valid_uri(&self, _uri: &Target) -> bool { @@ -742,7 +740,7 @@ mod tests { let dns_ips = vec!["127.0.0.1".parse().unwrap()]; let addresses = run_resolver_and_get_addresses_with_builder( - "custom:///whatever", + "dns:///whatever", dns_ips, Some(&matcher), child_builder, From 09dfb948832fb1723b9da5a2c5d1b856fab4de61 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Wed, 1 Jul 2026 15:30:50 +0530 Subject: [PATCH 19/21] no super import, infallible constructor --- .../client/name_resolution/proxy_resolver.rs | 70 +++++++++---------- 1 file changed, 32 insertions(+), 38 deletions(-) diff --git a/grpc/src/client/name_resolution/proxy_resolver.rs b/grpc/src/client/name_resolution/proxy_resolver.rs index 2ba4f0e1f..a6a968918 100644 --- a/grpc/src/client/name_resolution/proxy_resolver.rs +++ b/grpc/src/client/name_resolution/proxy_resolver.rs @@ -30,14 +30,14 @@ use http::HeaderValue; use hyper_util::client::proxy::matcher::Matcher; use url::Url; -use super::ResolverOptions; -use super::Target; use crate::client::name_resolution::Address; use crate::client::name_resolution::ChannelController; use crate::client::name_resolution::NopResolver; use crate::client::name_resolution::Resolver; use crate::client::name_resolution::ResolverBuilder; +use crate::client::name_resolution::ResolverOptions; use crate::client::name_resolution::ResolverUpdate; +use crate::client::name_resolution::Target; use crate::client::name_resolution::dns; use crate::client::service_config::ServiceConfig; use crate::credentials::common::Authority; @@ -79,10 +79,7 @@ pub(crate) struct Builder { impl ResolverBuilder for Builder { fn build(&self, target: &Target, options: ResolverOptions) -> Box { - match self.new_resolver(target, options, MATCHER.as_ref()) { - Ok(resolver) => resolver, - Err((err, options)) => NopResolver::new_with_err(err, options), - } + self.new_resolver(target, options, MATCHER.as_ref()) } fn scheme(&self) -> &str { @@ -109,14 +106,14 @@ impl Builder { target: &Target, options: ResolverOptions, matcher: Option<&Matcher>, - ) -> Result, (String, ResolverOptions)> { + ) -> Box { // Skip proxy lookup for non-DNS targets. if target.scheme() != "dns" { - return Ok(self.child_builder.build(target, options)); + return self.child_builder.build(target, options); } // If HTTPS_PROXY is unset, avoid parsing the target as a DNS hostname. let Some(matcher) = matcher else { - return Ok(self.child_builder.build(target, options)); + return self.child_builder.build(target, options); }; let target_authority = self.child_builder.default_authority(target); @@ -125,7 +122,10 @@ impl Builder { let url_obj = match Url::parse(&format!("https://{target_authority}")) { Ok(url) => url, Err(err) => { - return Err((format!("invalid target host in URL: {err}"), options)); + return NopResolver::new_with_err( + format!("invalid target host in URL: {err}"), + options, + ); } }; @@ -144,15 +144,15 @@ impl Builder { Ok(uri) => uri, Err(err) => { // This should not error since the url crate parsed the host. - return Err(( + return NopResolver::new_with_err( format!("failed to parse target authority: {}", err), options, - )); + ); } }; let Some(intercept) = matcher.intercept(&uri) else { - return Ok(self.child_builder.build(target, options)); + return self.child_builder.build(target, options); }; let mut proxy_authorization_header = intercept.basic_auth().cloned(); @@ -162,14 +162,14 @@ impl Builder { let proxy_options = ProxyOptions { proxy_authorization_header, - connect_addr: explicit_authority, + connect_authority: explicit_authority, }; let Some(proxy_host) = intercept.uri().authority() else { - return Err(( + return NopResolver::new_with_err( format!("proxy URI missing authority: {}", intercept.uri()), options, - )); + ); }; // `proxy_host` is be a valid URL authority. Because the `url` crate @@ -180,19 +180,19 @@ impl Builder { let proxy_target: Target = match target_str.parse() { Ok(t) => t, Err(e) => { - return Err(( + return NopResolver::new_with_err( format!("failed to parse proxy target {target_str}: {e}"), options, - )); + ); } }; let child = dns::Builder {}.build(&proxy_target, options); - Ok(Box::new(HttpsProxyResolver { + Box::new(HttpsProxyResolver { child, proxy_options: Arc::new(proxy_options), - })) + }) } } @@ -205,7 +205,7 @@ struct HttpsProxyResolver { #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] pub(crate) struct ProxyOptions { proxy_authorization_header: Option, - connect_addr: String, + connect_authority: String, } impl ProxyOptions { @@ -216,8 +216,8 @@ impl ProxyOptions { /// 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 + pub(crate) fn connect_authority(&self) -> &str { + &self.connect_authority } } @@ -398,10 +398,7 @@ mod tests { work_scheduler, }; - let mut resolver = match builder.new_resolver(&target, options, matcher) { - Ok(resolver) => resolver, - Err((err, _)) => panic!("failed to build resolver: {err}"), - }; + let mut resolver = builder.new_resolver(&target, options, matcher); work_rx.recv().await.unwrap(); @@ -449,7 +446,7 @@ mod tests { expected_header.set_sensitive(true); let expected_proxy_opts = ProxyOptions { proxy_authorization_header: Some(expected_header), - connect_addr: "target.example.com:443".to_string(), + connect_authority: "target.example.com:443".to_string(), }; for address in &addresses { @@ -496,7 +493,7 @@ mod tests { proxy_opts, &ProxyOptions { proxy_authorization_header: None, - connect_addr: "xn--tst-qla.example.com:443".to_string(), + connect_authority: "xn--tst-qla.example.com:443".to_string(), } ); } @@ -528,10 +525,7 @@ mod tests { work_scheduler, }; - let mut resolver = match builder.new_resolver(&target, options, Some(&matcher)) { - Ok(resolver) => resolver, - Err((err, options)) => NopResolver::new_with_err(err, options), - }; + let mut resolver = builder.new_resolver(&target, options, Some(&matcher)); work_rx.recv().await.unwrap(); @@ -605,7 +599,7 @@ mod tests { assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); let expected_proxy_opts = ProxyOptions { proxy_authorization_header: None, - connect_addr: "target.example.com:443".to_string(), + connect_authority: "target.example.com:443".to_string(), }; let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); @@ -632,7 +626,7 @@ mod tests { assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); let expected_proxy_opts = ProxyOptions { proxy_authorization_header: None, - connect_addr: "other.example.com:443".to_string(), + connect_authority: "other.example.com:443".to_string(), }; let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); @@ -668,7 +662,7 @@ mod tests { let expected_proxy_opts = ProxyOptions { proxy_authorization_header: None, - connect_addr: "target.example.com:443".to_string(), + connect_authority: "target.example.com:443".to_string(), }; let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); @@ -693,7 +687,7 @@ mod tests { let expected_proxy_opts = ProxyOptions { proxy_authorization_header: None, - connect_addr: "[::1]:443".to_string(), + connect_authority: "[::1]:443".to_string(), }; let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); @@ -750,7 +744,7 @@ mod tests { assert_eq!(addresses.len(), 1); let expected_proxy_opts = ProxyOptions { proxy_authorization_header: None, - connect_addr: "custom.authority.example.com:1234".to_string(), + connect_authority: "custom.authority.example.com:1234".to_string(), }; let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); From b2f47f591e9f7a74243c6997b99dbd311babfc6d Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Wed, 1 Jul 2026 15:49:16 +0530 Subject: [PATCH 20/21] move proxy opts to transport mod --- .../client/name_resolution/proxy_resolver.rs | 104 +++++------------- grpc/src/client/transport/mod.rs | 49 +++++++++ 2 files changed, 77 insertions(+), 76 deletions(-) diff --git a/grpc/src/client/name_resolution/proxy_resolver.rs b/grpc/src/client/name_resolution/proxy_resolver.rs index a6a968918..6e6488794 100644 --- a/grpc/src/client/name_resolution/proxy_resolver.rs +++ b/grpc/src/client/name_resolution/proxy_resolver.rs @@ -22,15 +22,12 @@ * */ -use std::fmt::Debug; use std::sync::Arc; use std::sync::LazyLock; -use http::HeaderValue; use hyper_util::client::proxy::matcher::Matcher; use url::Url; -use crate::client::name_resolution::Address; use crate::client::name_resolution::ChannelController; use crate::client::name_resolution::NopResolver; use crate::client::name_resolution::Resolver; @@ -40,6 +37,7 @@ use crate::client::name_resolution::ResolverUpdate; use crate::client::name_resolution::Target; use crate::client::name_resolution::dns; use crate::client::service_config::ServiceConfig; +use crate::client::transport::ProxyOptions; use crate::credentials::common::Authority; static MATCHER: LazyLock> = LazyLock::new(build_matcher); @@ -160,10 +158,7 @@ impl Builder { header.set_sensitive(true); } - let proxy_options = ProxyOptions { - proxy_authorization_header, - connect_authority: explicit_authority, - }; + let proxy_options = ProxyOptions::new(explicit_authority, proxy_authorization_header); let Some(proxy_host) = intercept.uri().authority() else { return NopResolver::new_with_err( @@ -201,26 +196,6 @@ struct HttpsProxyResolver { proxy_options: Arc, } -/// Options for establishing an HTTP CONNECT proxy tunnel. -#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] -pub(crate) struct ProxyOptions { - proxy_authorization_header: Option, - connect_authority: String, -} - -impl ProxyOptions { - /// Returns the value of the `Proxy-Authorization` header, if present. - pub(crate) fn proxy_authorization_header(&self) -> Option<&http::HeaderValue> { - self.proxy_authorization_header.as_ref() - } - - /// 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 connect_authority(&self) -> &str { - &self.connect_authority - } -} - impl Resolver for HttpsProxyResolver { fn resolve_now(&mut self) { self.child.resolve_now(); @@ -245,7 +220,7 @@ impl<'a> ChannelController for InterceptingController<'a> { 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()); + ProxyOptions::add_to_addr(address, self.proxy_options.clone()); } } } @@ -257,13 +232,6 @@ impl<'a> ChannelController for InterceptingController<'a> { } } -/// Extracts `ProxyOptions` from the given `Address` attributes, if present. -pub(crate) fn proxy_options_for_addr(addr: &Address) -> Option<&ProxyOptions> { - addr.attributes - .get::>() - .map(AsRef::as_ref) -} - fn get_first_env(names: &[&str]) -> String { for name in names { if let Ok(val) = std::env::var(name) { @@ -289,9 +257,12 @@ mod tests { use std::pin::Pin; use std::sync::Arc; + use http::HeaderValue; + use super::*; use crate::attributes::Attributes; use crate::byte_str::ByteStr; + use crate::client::name_resolution::Address; use crate::client::name_resolution::test_utils::TestChannelController; use crate::client::name_resolution::test_utils::TestWorkScheduler; use crate::rt; @@ -444,13 +415,11 @@ mod tests { let mut expected_header = HeaderValue::from_static("Basic dXNlcjpwYXNzd29yZA=="); expected_header.set_sensitive(true); - let expected_proxy_opts = ProxyOptions { - proxy_authorization_header: Some(expected_header), - connect_authority: "target.example.com:443".to_string(), - }; + let expected_proxy_opts = + ProxyOptions::new("target.example.com:443".to_string(), Some(expected_header)); for address in &addresses { - let proxy_opts = proxy_options_for_addr(address).expect("ProxyOptions not found"); + let proxy_opts = ProxyOptions::from_addr(address).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); } } @@ -471,7 +440,7 @@ mod tests { assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); - assert!(proxy_options_for_addr(&addresses[0]).is_none()); + assert!(ProxyOptions::from_addr(&addresses[0]).is_none()); } #[tokio::test] @@ -488,13 +457,10 @@ mod tests { .await; assert_eq!(addresses.len(), 1); - let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + let proxy_opts = ProxyOptions::from_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!( proxy_opts, - &ProxyOptions { - proxy_authorization_header: None, - connect_authority: "xn--tst-qla.example.com:443".to_string(), - } + &ProxyOptions::new("xn--tst-qla.example.com:443".to_string(), None) ); } @@ -553,7 +519,7 @@ mod tests { assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); - assert!(proxy_options_for_addr(&addresses[0]).is_none()); + assert!(ProxyOptions::from_addr(&addresses[0]).is_none()); // Check for abstract-unix scheme. let addresses = run_resolver_and_get_addresses( @@ -565,7 +531,7 @@ mod tests { assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); - assert!(proxy_options_for_addr(&addresses[0]).is_none()); + assert!(ProxyOptions::from_addr(&addresses[0]).is_none()); } #[tokio::test] @@ -583,7 +549,7 @@ mod tests { assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); assert!( - proxy_options_for_addr(&addresses[0]).is_none(), + ProxyOptions::from_addr(&addresses[0]).is_none(), "HTTP proxy should not match HTTPS destinations" ); @@ -597,11 +563,8 @@ mod tests { .await; assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); - let expected_proxy_opts = ProxyOptions { - proxy_authorization_header: None, - connect_authority: "target.example.com:443".to_string(), - }; - let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + let expected_proxy_opts = ProxyOptions::new("target.example.com:443".to_string(), None); + let proxy_opts = ProxyOptions::from_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); // Case 3: https proxy and no proxy are configured. @@ -616,7 +579,7 @@ mod tests { .await; assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); - assert!(proxy_options_for_addr(&addresses[0]).is_none()); + assert!(ProxyOptions::from_addr(&addresses[0]).is_none()); // Target B: other.example.com (NOT matched by no_proxy) -> should proxy let addresses = @@ -624,11 +587,8 @@ mod tests { .await; assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); - let expected_proxy_opts = ProxyOptions { - proxy_authorization_header: None, - connect_authority: "other.example.com:443".to_string(), - }; - let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + let expected_proxy_opts = ProxyOptions::new("other.example.com:443".to_string(), None); + let proxy_opts = ProxyOptions::from_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); } @@ -643,7 +603,7 @@ mod tests { assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, DIRECT_ADDRESS); - assert!(proxy_options_for_addr(&addresses[0]).is_none()); + assert!(ProxyOptions::from_addr(&addresses[0]).is_none()); } #[tokio::test] @@ -660,12 +620,9 @@ mod tests { assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, "[::1]:8080"); - let expected_proxy_opts = ProxyOptions { - proxy_authorization_header: None, - connect_authority: "target.example.com:443".to_string(), - }; + let expected_proxy_opts = ProxyOptions::new("target.example.com:443".to_string(), None); - let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + let proxy_opts = ProxyOptions::from_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); } @@ -685,12 +642,9 @@ mod tests { assert_eq!(addresses.len(), 1); assert_eq!(&*addresses[0].address, "127.0.0.1:8080"); - let expected_proxy_opts = ProxyOptions { - proxy_authorization_header: None, - connect_authority: "[::1]:443".to_string(), - }; + let expected_proxy_opts = ProxyOptions::new("[::1]:443".to_string(), None); - let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + let proxy_opts = ProxyOptions::from_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); } @@ -742,12 +696,10 @@ mod tests { .await; assert_eq!(addresses.len(), 1); - let expected_proxy_opts = ProxyOptions { - proxy_authorization_header: None, - connect_authority: "custom.authority.example.com:1234".to_string(), - }; + let expected_proxy_opts = + ProxyOptions::new("custom.authority.example.com:1234".to_string(), None); - let proxy_opts = proxy_options_for_addr(&addresses[0]).expect("ProxyOptions not found"); + let proxy_opts = ProxyOptions::from_addr(&addresses[0]).expect("ProxyOptions not found"); assert_eq!(proxy_opts, &expected_proxy_opts); } } diff --git a/grpc/src/client/transport/mod.rs b/grpc/src/client/transport/mod.rs index 33987a3ac..b53cb0c09 100644 --- a/grpc/src/client/transport/mod.rs +++ b/grpc/src/client/transport/mod.rs @@ -25,8 +25,11 @@ use std::sync::Arc; use std::time::Duration; +use http::HeaderValue; + use crate::client::DynInvoke; use crate::client::Invoke; +use crate::client::name_resolution::Address; use crate::credentials::client::ClientHandshakeInfo; use crate::credentials::client::DynClientConnectionSecurityInfo; use crate::credentials::common::Authority; @@ -128,3 +131,49 @@ pub(crate) struct SecurityOpts { pub(crate) authority: Authority, pub(crate) handshake_info: ClientHandshakeInfo, } + +/// Options for establishing an HTTP CONNECT proxy tunnel. +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] +pub(crate) struct ProxyOptions { + proxy_authorization_header: Option, + connect_authority: String, +} + +impl ProxyOptions { + /// Creates a new `ProxyOptions`. + /// + /// # Arguments + /// * `target_authority` - The address of the target server to connect to (host:port). + /// Must be a valid hostname. + /// * `proxy_authorization_header` - The value of the `Proxy-Authorization` header, if present. + pub(crate) fn new( + target_authority: String, + proxy_authorization_header: Option, + ) -> Self { + Self { + proxy_authorization_header, + connect_authority: target_authority, + } + } + + /// Returns the value of the `Proxy-Authorization` header, if present. + pub(crate) fn proxy_authorization_header(&self) -> Option<&HeaderValue> { + self.proxy_authorization_header.as_ref() + } + + /// 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 connect_authority(&self) -> &str { + &self.connect_authority + } + + /// Extracts `ProxyOptions` from the given `Address` attributes, if present. + pub(crate) fn from_addr(addr: &Address) -> Option<&Self> { + addr.attributes.get::>().map(AsRef::as_ref) + } + + /// Adds these `ProxyOptions` to the given `Address` attributes. + pub(crate) fn add_to_addr(addr: &mut Address, options: Arc) { + addr.attributes = addr.attributes.add(options); + } +} From 8a1e72a386737a80b7e83fa3c47cea282e397504 Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Fri, 3 Jul 2026 14:43:06 +0530 Subject: [PATCH 21/21] rustdoc and variable rename --- grpc/src/client/transport/mod.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/grpc/src/client/transport/mod.rs b/grpc/src/client/transport/mod.rs index d65ff1cf8..8bce87acb 100644 --- a/grpc/src/client/transport/mod.rs +++ b/grpc/src/client/transport/mod.rs @@ -132,19 +132,23 @@ pub(crate) struct SecurityOpts { pub(crate) handshake_info: ClientHandshakeInfo, } -/// Options for establishing an HTTP CONNECT proxy tunnel. +/// Configuration options for establishing an HTTP `CONNECT` proxy tunnel. +/// +/// This may be added as an [`Address`] attribute by a +/// [`crate::client::name_resolution::Resolver`]. If present, the subchannel +/// will automatically handle the HTTP `CONNECT` handshake. #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] pub(crate) struct ProxyOptions { proxy_authorization_header: Option, - connect_authority: String, + target_authority: String, } impl ProxyOptions { /// Creates a new `ProxyOptions`. /// /// # Arguments - /// * `target_authority` - The address of the target server to connect to (host:port). - /// Must be a valid hostname. + /// * `target_authority` - The address of the target server to connect to + /// (host:port). Must be a valid hostname. /// * `proxy_authorization_header` - The value of the `Proxy-Authorization` header, if present. pub(crate) fn new( target_authority: String, @@ -152,7 +156,7 @@ impl ProxyOptions { ) -> Self { Self { proxy_authorization_header, - connect_authority: target_authority, + target_authority, } } @@ -163,8 +167,8 @@ impl ProxyOptions { /// 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 connect_authority(&self) -> &str { - &self.connect_authority + pub(crate) fn target_authority(&self) -> &str { + &self.target_authority } /// Extracts `ProxyOptions` from the given `Address` attributes, if present.