fix(tonic-xds): match routes on request path and anchor safe_regex

tonic-xds/src/client/route.rs

@@ -40,6 +40,8 @@ use tower::{BoxError, Layer, Service};
 pub(crate) struct RouteInput<'a> {
     /// The authority (host) of the request URI.
     pub authority: &'a str,
+    /// The request path (e.g. `/pkg.Service/Method`), used for path matching.
+    pub path: &'a str,
     /// The HTTP headers of the request. These can be used for header-based routing decisions.
     pub headers: &'a http::HeaderMap,
 }
@@ -210,6 +212,7 @@ where
                 }
                 let route_input = RouteInput {
                     authority: &authority,
+                    path: request.uri().path(),
                     headers: request.headers(),
                 };
                 router.route(&route_input, &config)?
@@ -293,6 +296,77 @@ mod tests {
         }
     }
 
+    #[tokio::test]
+    async fn matches_routes_on_the_request_path() {
+        use crate::xds::cache::XdsCache;
+        use crate::xds::resource::route_config::{
+            PathSpecifierConfig, RouteConfig, RouteConfigAction, RouteConfigMatch,
+            RouteConfigResource, VirtualHostConfig,
+        };
+        use crate::xds::routing::XdsRouter;
+
+        fn prefix_route(prefix: &str, cluster: &str) -> RouteConfig {
+            RouteConfig {
+                match_criteria: RouteConfigMatch {
+                    path_specifier: PathSpecifierConfig::Prefix(prefix.into()),
+                    headers: vec![],
+                    case_sensitive: true,
+                    match_fraction: None,
+                },
+                action: RouteConfigAction::Cluster(cluster.into()),
+                retry_config: None,
+            }
+        }
+
+        let cache = XdsCache::new();
+        cache.update_route_config(Arc::new(RouteConfigResource {
+            name: "rc".into(),
+            virtual_hosts: vec![VirtualHostConfig {
+                name: "vh".into(),
+                domains: vec!["greeter.svc:50051".into()],
+                // Most specific first; "/" matches anything, so picking it means
+                // the real path never reached matching.
+                routes: vec![
+                    prefix_route("/pkg.Greeter/", "greeter-cluster"),
+                    prefix_route("/", "fallback-cluster"),
+                ],
+            }],
+            metadata: Default::default(),
+        }));
+        let xds_router = XdsRouter::new(&cache);
+        while xds_router.snapshot().is_none() {
+            tokio::task::yield_now().await;
+        }
+        let router: Arc<dyn Router> = Arc::new(xds_router);
+
+        let captured: Arc<Mutex<Option<String>>> = Arc::new(Mutex::new(None));
+        let sink = captured.clone();
+        let inner = service_fn(move |req: Request<()>| {
+            let sink = sink.clone();
+            async move {
+                *sink.lock().unwrap() = req
+                    .extensions()
+                    .get::<RouteDecision>()
+                    .map(|d| d.cluster.clone());
+                Ok::<_, BoxError>(http::Response::new(()))
+            }
+        });
+        let svc = XdsRoutingLayer::new(router, None, Arc::from("greeter.svc:50051")).layer(inner);
+
+        // The URI a tonic-generated client produces.
+        let req = Request::builder()
+            .uri("/pkg.Greeter/SayHello")
+            .body(())
+            .unwrap();
+        svc.oneshot(req).await.unwrap();
+
+        assert_eq!(
+            captured.lock().unwrap().as_deref(),
+            Some("greeter-cluster"),
+            "route matching did not see the request path",
+        );
+    }
+
     #[tokio::test]
     async fn interceptor_runs_when_config_arrives_late() {
         use crate::xds::cache::XdsCache;

tonic-xds/src/xds/resource/mod.rs

@@ -41,6 +41,7 @@ pub(crate) mod hash_policy;
 pub(crate) mod listener;
 pub(crate) mod outlier_detection;
 pub(crate) mod route_config;
+pub(crate) mod safe_regex;
 pub(crate) mod san_matcher;
 pub(crate) mod security;
 pub(crate) mod string_matcher;

tonic-xds/src/xds/resource/route_config.rs

@@ -34,12 +34,11 @@ use envoy_types::pb::envoy::config::route::v3::{
     RetryPolicy, RouteConfiguration, RouteMatch, route, route_action, route_match,
 };
 use prost::Message;
-use regex::Regex;
 use xds_client::resource::TypeUrl;
 use xds_client::{Error, Resource};
 
+use super::safe_regex::SafeRegex;
 use super::string_matcher::StringMatcher;
-
 /// A `typed_filter_metadata` entry — a `google.protobuf.Any` (a type URL plus an
 /// encoded message value).
 #[derive(Debug, Clone)]
@@ -276,7 +275,7 @@ pub(crate) struct RouteConfigMatch {
 pub(crate) enum PathSpecifierConfig {
     Prefix(String),
     Path(String),
-    SafeRegex(Regex),
+    SafeRegex(SafeRegex),
 }
 
 /// Header matching criteria.
@@ -443,7 +442,7 @@ fn validate_route_match(rm: RouteMatch) -> xds_client::Result<RouteConfigMatch>
         Some(route_match::PathSpecifier::Prefix(p)) => PathSpecifierConfig::Prefix(p),
         Some(route_match::PathSpecifier::Path(p)) => PathSpecifierConfig::Path(p),
         Some(route_match::PathSpecifier::SafeRegex(r)) => {
-            let re = Regex::new(&r.regex)
+            let re = SafeRegex::new(&r.regex)
                 .map_err(|e| Error::Validation(format!("invalid path regex '{}': {e}", r.regex)))?;
             PathSpecifierConfig::SafeRegex(re)
         }
@@ -516,7 +515,7 @@ fn validate_header_matcher(
         // SafeRegexMatch is deprecated in favor of StringMatch, which is handled below.
         #[allow(deprecated)]
         Some(HeaderMatchSpecifier::SafeRegexMatch(r)) => {
-            let re = Regex::new(&r.regex).map_err(|e| {
+            let re = SafeRegex::new(&r.regex).map_err(|e| {
                 Error::Validation(format!("invalid header regex '{}': {e}", r.regex))
             })?;
             HeaderMatchSpecifierConfig::String(StringMatcher::SafeRegex(re))
@@ -784,6 +783,89 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn safe_regex_path_matcher_requires_a_full_match() {
+        use envoy_types::pb::envoy::r#type::matcher::v3::RegexMatcher;
+
+        let unanchored_rm = RouteMatch {
+            path_specifier: Some(route_match::PathSpecifier::SafeRegex(RegexMatcher {
+                regex: r"/pkg\.Greeter/SayHello".to_string(),
+                ..Default::default()
+            })),
+            ..Default::default()
+        };
+
+        let matched = validate_route_match(unanchored_rm).expect("valid regex");
+        let PathSpecifierConfig::SafeRegex(re) = matched.path_specifier else {
+            panic!("expected a SafeRegex path specifier");
+        };
+
+        assert!(
+            re.is_match("/pkg.Greeter/SayHello"),
+            "exact path must match"
+        );
+        assert!(
+            !re.is_match("/pkg.Greeter/SayHelloAgain"),
+            "a longer method sharing the prefix must not match"
+        );
+        assert!(
+            !re.is_match("/other.Svc/x/pkg.Greeter/SayHello"),
+            "the pattern must not match as a substring of a longer path"
+        );
+    }
+
+    #[test]
+    fn safe_regex_path_matcher_anchors_each_alternation_branch() {
+        use envoy_types::pb::envoy::r#type::matcher::v3::RegexMatcher;
+
+        let rm = RouteMatch {
+            path_specifier: Some(route_match::PathSpecifier::SafeRegex(RegexMatcher {
+                regex: "/a|/b".to_string(),
+                ..Default::default()
+            })),
+            ..Default::default()
+        };
+
+        let matched = validate_route_match(rm).expect("valid regex");
+        let PathSpecifierConfig::SafeRegex(re) = matched.path_specifier else {
+            panic!("expected a SafeRegex path specifier");
+        };
+
+        assert!(re.is_match("/a"));
+        assert!(re.is_match("/b"));
+        assert!(
+            !re.is_match("/aX"),
+            "an alternation branch must not match a longer path"
+        );
+    }
+
+    #[test]
+    fn safe_regex_header_matcher_requires_a_full_match() {
+        use envoy_types::pb::envoy::config::route::v3::HeaderMatcher;
+        use envoy_types::pb::envoy::config::route::v3::header_matcher::HeaderMatchSpecifier;
+        use envoy_types::pb::envoy::r#type::matcher::v3::RegexMatcher;
+
+        #[allow(deprecated)]
+        let hm = HeaderMatcher {
+            name: "x-version".into(),
+            header_match_specifier: Some(HeaderMatchSpecifier::SafeRegexMatch(RegexMatcher {
+                regex: "v[0-9]+".into(),
+                ..Default::default()
+            })),
+            ..Default::default()
+        };
+
+        let matched = validate_header_matcher(hm).expect("valid regex");
+        let HeaderMatchSpecifierConfig::String(m) = matched.match_specifier else {
+            panic!("expected a string matcher");
+        };
+        assert!(m.is_match("v2"));
+        assert!(
+            !m.is_match("v2-beta"),
+            "a longer value sharing the prefix must not match"
+        );
+    }
+
     #[test]
     fn test_cascade_weighted_clusters() {
         use envoy_types::pb::envoy::config::route::v3::{