fix(grpc): percent-decode target URI path and reject non-UTF-8 characters

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.3"
 pin-project-lite = "0.2.16"
 rand = "0.9"
 rustls = { version = "0.23", optional = true, default-features = false, features = [

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<dyn DynChannelCredentials>,
     ) -> 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()
@@ -601,8 +599,8 @@ impl<T: Clone> WatcherIter<T> {
     }
 }
 
-/// 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('[')

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 {

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;
@@ -67,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<Self, Self::Err> {
-        match s.parse::<Url>() {
-            Ok(url) => Ok(Target { url }),
-            Err(err) => Err(err.to_string()),
-        }
-    }
-}
-
-impl From<url::Url> for Target {
-    fn from(url: url::Url) -> Self {
-        Target { url }
+        let url = s.parse::<Url>().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 })
     }
 }
 
@@ -123,9 +121,9 @@ impl Target {
         }
     }
 
-    /// Retrieves endpoint from `Url.path()`.
+    /// Retrieves the percent-decoded endpoint from `Url.path()`.
     pub fn path(&self) -> &str {
-        self.url.path()
+        &self.decoded_path
     }
 }
 
@@ -136,7 +134,7 @@ impl Display for Target {
             "{}://{}{}",
             self.scheme(),
             self.authority_host_port(),
-            self.path()
+            self.decoded_path
         )
     }
 }
@@ -379,13 +377,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() {
@@ -436,6 +436,15 @@ mod test {
                 want_path: "/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: "/foo bar",
+                want_str: "dns:///foo bar",
+            },
         ];
 
         for tc in test_cases {
@@ -449,6 +458,18 @@ mod test {
         }
     }
 
+    #[test]
+    fn parse_target_invalid_utf8() {
+        let input = "dns:///foo%FFbar";
+        let target: Result<Target, _> = 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