From 9121161b34a8ac2274202e32c12e476e287285e1 Mon Sep 17 00:00:00 2001 From: Eeshu-Yadav Date: Fri, 12 Sep 2025 04:03:13 +0530 Subject: [PATCH 1/3] Implement AutoHostRewrite functionality in request handling This commit implements the missing AutoHostRewrite functionality that was configured but not actually used during request processing. The implementation adds support for all AuthorityRewriteSpecifier variants: - AutoHostRewrite: Uses upstream cluster's authority - Authority: Uses a specific authority - Header: Uses value from a request header - Regex: Applies regex transformation to current authority Both the request URI authority and Host header are updated when authority rewriting is applied, ensuring consistent behavior across the request. Fixes the issue mentioned in PR #86 review where autorewrite configuration was parsed but not executed during request handling. Signed-off-by: Eeshu-Yadav --- .../http_connection_manager/route.rs | 108 +++++++++++++++++- .../http_connection_manager/route.rs | 26 ++++- 2 files changed, 128 insertions(+), 6 deletions(-) diff --git a/orion-configuration/src/config/network_filters/http_connection_manager/route.rs b/orion-configuration/src/config/network_filters/http_connection_manager/route.rs index 9537bd48..4a02623d 100644 --- a/orion-configuration/src/config/network_filters/http_connection_manager/route.rs +++ b/orion-configuration/src/config/network_filters/http_connection_manager/route.rs @@ -38,7 +38,6 @@ use std::{ str::FromStr, time::Duration, }; - #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] #[serde(rename_all = "snake_case")] pub enum Action { @@ -132,6 +131,47 @@ impl PathRewriteSpecifier { } } +impl AuthorityRewriteSpecifier { + /// Applies authority rewrite based on the specifier type + pub fn apply( + &self, + uri: &http::Uri, + headers: &http::HeaderMap, + upstream_authority: &Authority, + ) -> Result, InvalidUri> { + match self { + AuthorityRewriteSpecifier::Authority(authority) => Ok(Some(authority.clone())), + + AuthorityRewriteSpecifier::Header(header_name) => { + let Some(header_value) = headers.get(header_name.as_str()) else { + return Ok(None); + }; + let Ok(header_str) = header_value.to_str() else { + return Ok(None); + }; + Authority::from_str(header_str).map(Some).map_err(|e| e.into()) + }, + + AuthorityRewriteSpecifier::Regex(regex) => { + let current_authority = uri + .authority() + .map(Authority::as_str) + .or_else(|| headers.get(http::header::HOST).and_then(|h| h.to_str().ok())) + .unwrap_or_default(); + + let replacement = regex.pattern.replace_all(current_authority, regex.substitution.as_str()); + if let std::borrow::Cow::Borrowed(_) = replacement { + Ok(None) + } else { + Authority::from_str(&replacement).map(Some).map_err(|e| e.into()) + } + }, + + AuthorityRewriteSpecifier::AutoHostRewrite => Ok(Some(upstream_authority.clone())), + } + } +} + #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq)] pub struct RedirectAction { pub response_code: RedirectResponseCode, @@ -649,6 +689,72 @@ mod tests { let result = route_match.match_request(&req); assert!(!result.matched()); } + + #[test] + fn test_authority_rewrite_auto_host_rewrite() { + let authority_rewrite = AuthorityRewriteSpecifier::AutoHostRewrite; + let upstream_authority = Authority::from_str("upstream.example.com:8080").unwrap(); + let uri = "http://original.example.com/path".parse::().unwrap(); + let mut headers = http::HeaderMap::new(); + headers.insert("host", "original.example.com".parse().unwrap()); + + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + assert_eq!(result, Some(upstream_authority)); + } + + #[test] + fn test_authority_rewrite_specific_authority() { + let target_authority = Authority::from_str("target.example.com:9090").unwrap(); + let authority_rewrite = AuthorityRewriteSpecifier::Authority(target_authority.clone()); + let upstream_authority = Authority::from_str("upstream.example.com:8080").unwrap(); + let uri = "http://original.example.com/path".parse::().unwrap(); + let mut headers = http::HeaderMap::new(); + headers.insert("host", "original.example.com".parse().unwrap()); + + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + assert_eq!(result, Some(target_authority)); + } + + #[test] + fn test_authority_rewrite_header() { + let authority_rewrite = AuthorityRewriteSpecifier::Header("x-target-host".into()); + let upstream_authority = Authority::from_str("upstream.example.com:8080").unwrap(); + let uri = "http://original.example.com/path".parse::().unwrap(); + let mut headers = http::HeaderMap::new(); + headers.insert("host", "original.example.com".parse().unwrap()); + headers.insert("x-target-host", "header.example.com:7070".parse().unwrap()); + + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + assert_eq!(result, Some(Authority::from_str("header.example.com:7070").unwrap())); + } + + #[test] + fn test_authority_rewrite_header_missing() { + let authority_rewrite = AuthorityRewriteSpecifier::Header("x-missing-header".into()); + let upstream_authority = Authority::from_str("upstream.example.com:8080").unwrap(); + let uri = "http://original.example.com/path".parse::().unwrap(); + let mut headers = http::HeaderMap::new(); + headers.insert("host", "original.example.com".parse().unwrap()); + + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + assert_eq!(result, None); + } + + #[test] + fn test_authority_rewrite_regex() { + let regex = RegexMatchAndSubstitute { + pattern: Regex::new(r"^([^.]+)\.original\.com$").unwrap(), + substitution: "${1}.rewritten.com".into(), + }; + let authority_rewrite = AuthorityRewriteSpecifier::Regex(regex); + let upstream_authority = Authority::from_str("upstream.example.com:8080").unwrap(); + let uri = "http://api.original.com/path".parse::().unwrap(); + let mut headers = http::HeaderMap::new(); + headers.insert("host", "api.original.com".parse().unwrap()); + + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + assert_eq!(result, Some(Authority::from_str("api.rewritten.com").unwrap())); + } } #[cfg(feature = "envoy-conversions")] diff --git a/orion-lib/src/listeners/http_connection_manager/route.rs b/orion-lib/src/listeners/http_connection_manager/route.rs index 7e82c620..dd60f769 100644 --- a/orion-lib/src/listeners/http_connection_manager/route.rs +++ b/orion-lib/src/listeners/http_connection_manager/route.rs @@ -97,16 +97,32 @@ impl<'a> RequestHandler<(MatchedRequest<'a>, &HttpConnectionManager)> for &Route } else { None }; - if path_and_query_replacement.is_some() { + + let authority_replacement = if let Some(authority_rewrite) = &self.authority_rewrite { + authority_rewrite + .apply(&parts.uri, &parts.headers, &svc_channel.upstream_authority) + .with_context_msg("invalid authority after rewrite")? + } else { + None + }; + + if path_and_query_replacement.is_some() || authority_replacement.is_some() { parts.uri = { - let UriParts { scheme, authority, path_and_query: _, .. } = parts.uri.into_parts(); + let UriParts { scheme, authority, path_and_query, .. } = parts.uri.into_parts(); let mut new_parts = UriParts::default(); new_parts.scheme = scheme; - new_parts.authority = authority; - new_parts.path_and_query = path_and_query_replacement; - Uri::from_parts(new_parts).with_context_msg("failed to replace request path_and_query")? + new_parts.authority = authority_replacement.clone().or(authority); + new_parts.path_and_query = path_and_query_replacement.or(path_and_query); + Uri::from_parts(new_parts).with_context_msg("failed to replace request URI")? } } + + if let Some(new_authority) = &authority_replacement { + let header_value = http::HeaderValue::from_str(new_authority.as_str()) + .with_context_msg("failed to create Host header value")?; + parts.headers.insert(http::header::HOST, header_value); + } + Request::from_parts(parts, body.map_into()) }; From eed95b89993a2b166cdbc5aa1ab2f2bc877f9f0a Mon Sep 17 00:00:00 2001 From: Eeshu-Yadav Date: Tue, 16 Sep 2025 20:17:03 +0530 Subject: [PATCH 2/3] Add test case for authority rewrite with missing host header This commit adds a test case to cover the edge case where there's no Host header in the request headers. The test verifies that the regex-based authority rewrite handles gracefully when both the URI has no authority and there's no Host header in the headers, ensuring robust behavior in all scenarios. Test: test_authority_rewrite_regex_no_host_header Signed-off-by: Eeshu-Yadav --- .../http_connection_manager/route.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/orion-configuration/src/config/network_filters/http_connection_manager/route.rs b/orion-configuration/src/config/network_filters/http_connection_manager/route.rs index 4a02623d..f127946b 100644 --- a/orion-configuration/src/config/network_filters/http_connection_manager/route.rs +++ b/orion-configuration/src/config/network_filters/http_connection_manager/route.rs @@ -755,6 +755,21 @@ mod tests { let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); assert_eq!(result, Some(Authority::from_str("api.rewritten.com").unwrap())); } + + #[test] + fn test_authority_rewrite_regex_no_host_header() { + let regex = RegexMatchAndSubstitute { + pattern: Regex::new(r"^([^.]+)\.original\.com$").unwrap(), + substitution: "${1}.rewritten.com".into(), + }; + let authority_rewrite = AuthorityRewriteSpecifier::Regex(regex); + let upstream_authority = Authority::from_str("upstream.example.com:8080").unwrap(); + let uri = "/path".parse::().unwrap(); + let headers = http::HeaderMap::new(); + + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + assert_eq!(result, None); + } } #[cfg(feature = "envoy-conversions")] From 2b301f6cd1127c6493985a0489706943052714bf Mon Sep 17 00:00:00 2001 From: Eeshu-Yadav Date: Fri, 19 Sep 2025 23:45:07 +0530 Subject: [PATCH 3/3] Address review comments: remove redundant error conversions - Remove unnecessary .map_err(|e| e.into()) calls - The ? operator handles error conversion automatically for Authority::from_str - Remove redundant InvalidUri error type Signed-off-by: Eeshu-Yadav --- .../http_connection_manager/route.rs | 28 +++++++++---------- .../http_connection_manager/route.rs | 4 +-- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/orion-configuration/src/config/network_filters/http_connection_manager/route.rs b/orion-configuration/src/config/network_filters/http_connection_manager/route.rs index f127946b..3ea4f802 100644 --- a/orion-configuration/src/config/network_filters/http_connection_manager/route.rs +++ b/orion-configuration/src/config/network_filters/http_connection_manager/route.rs @@ -138,18 +138,18 @@ impl AuthorityRewriteSpecifier { uri: &http::Uri, headers: &http::HeaderMap, upstream_authority: &Authority, - ) -> Result, InvalidUri> { + ) -> Option { match self { - AuthorityRewriteSpecifier::Authority(authority) => Ok(Some(authority.clone())), + AuthorityRewriteSpecifier::Authority(authority) => Some(authority.clone()), AuthorityRewriteSpecifier::Header(header_name) => { let Some(header_value) = headers.get(header_name.as_str()) else { - return Ok(None); + return None; }; let Ok(header_str) = header_value.to_str() else { - return Ok(None); + return None; }; - Authority::from_str(header_str).map(Some).map_err(|e| e.into()) + Authority::from_str(header_str).ok() }, AuthorityRewriteSpecifier::Regex(regex) => { @@ -161,13 +161,13 @@ impl AuthorityRewriteSpecifier { let replacement = regex.pattern.replace_all(current_authority, regex.substitution.as_str()); if let std::borrow::Cow::Borrowed(_) = replacement { - Ok(None) + None } else { - Authority::from_str(&replacement).map(Some).map_err(|e| e.into()) + Authority::from_str(&replacement).ok() } }, - AuthorityRewriteSpecifier::AutoHostRewrite => Ok(Some(upstream_authority.clone())), + AuthorityRewriteSpecifier::AutoHostRewrite => Some(upstream_authority.clone()), } } } @@ -698,7 +698,7 @@ mod tests { let mut headers = http::HeaderMap::new(); headers.insert("host", "original.example.com".parse().unwrap()); - let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority); assert_eq!(result, Some(upstream_authority)); } @@ -711,7 +711,7 @@ mod tests { let mut headers = http::HeaderMap::new(); headers.insert("host", "original.example.com".parse().unwrap()); - let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority); assert_eq!(result, Some(target_authority)); } @@ -724,7 +724,7 @@ mod tests { headers.insert("host", "original.example.com".parse().unwrap()); headers.insert("x-target-host", "header.example.com:7070".parse().unwrap()); - let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority); assert_eq!(result, Some(Authority::from_str("header.example.com:7070").unwrap())); } @@ -736,7 +736,7 @@ mod tests { let mut headers = http::HeaderMap::new(); headers.insert("host", "original.example.com".parse().unwrap()); - let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority); assert_eq!(result, None); } @@ -752,7 +752,7 @@ mod tests { let mut headers = http::HeaderMap::new(); headers.insert("host", "api.original.com".parse().unwrap()); - let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority); assert_eq!(result, Some(Authority::from_str("api.rewritten.com").unwrap())); } @@ -767,7 +767,7 @@ mod tests { let uri = "/path".parse::().unwrap(); let headers = http::HeaderMap::new(); - let result = authority_rewrite.apply(&uri, &headers, &upstream_authority).unwrap(); + let result = authority_rewrite.apply(&uri, &headers, &upstream_authority); assert_eq!(result, None); } } diff --git a/orion-lib/src/listeners/http_connection_manager/route.rs b/orion-lib/src/listeners/http_connection_manager/route.rs index dd60f769..1b054f6d 100644 --- a/orion-lib/src/listeners/http_connection_manager/route.rs +++ b/orion-lib/src/listeners/http_connection_manager/route.rs @@ -99,9 +99,7 @@ impl<'a> RequestHandler<(MatchedRequest<'a>, &HttpConnectionManager)> for &Route }; let authority_replacement = if let Some(authority_rewrite) = &self.authority_rewrite { - authority_rewrite - .apply(&parts.uri, &parts.headers, &svc_channel.upstream_authority) - .with_context_msg("invalid authority after rewrite")? + authority_rewrite.apply(&parts.uri, &parts.headers, &svc_channel.upstream_authority) } else { None };