Repository: envoyproxy/gateway

Issue:
Unsupported special characters in Httproute paths with root substitution
*Description*:
When `spec.rules.matches.value` contains a special character  and the desired substitution is root '/', the routing does not work.
When the substitution is a root target, EG handles the matcher as a PathPrefix, but the UrlRewrite as RegexRewrite. This causes an issue if the matcher contains a $, because:
- If the $ remained un-escaped in the matcher, it is taken as is to the regexRewrite and there it is treated as an end of regex (which drops everything after the $).
- If the $ is escaped, the matcher doesn't work (because its match type is PathPrefix).

*Relevant code*:
https://github.com/envoyproxy/gateway/blob/cba91e11708b46a11f8b2c751e04fe8e67028956/internal/xds/translator/route.go#L443


Discussion:

Solution patch:
diff --git a/internal/gatewayapi/contexts.go b/internal/gatewayapi/contexts.go
index 8841e48a7..81e5967a0 100644
--- a/internal/gatewayapi/contexts.go
+++ b/internal/gatewayapi/contexts.go
@@ -6,8 +6,6 @@
 package gatewayapi
 
 import (
-	"reflect"
-
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -456,7 +454,7 @@ func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentRefere
 	var parentRef *gwapiv1.ParentReference
 	specParentRefs := route.GetParentReferences()
 	for _, p := range specParentRefs {
-		if isParentRefEqual(p, forParentRef, route.GetNamespace()) {
+		if IsParentRefEqual(p, forParentRef, route.GetNamespace()) {
 			parentRef = &p
 			break
 		}
@@ -470,7 +468,7 @@ func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentRefere
 	routeStatus := route.GetRouteStatus()
 
 	for i, parent := range routeStatus.Parents {
-		if isParentRefEqual(parent.ParentRef, *parentRef, route.GetNamespace()) {
+		if IsParentRefEqual(parent.ParentRef, *parentRef, route.GetNamespace()) {
 			routeParentStatusIdx = i
 			break
 		}
@@ -510,32 +508,74 @@ func GetRouteParentContext(route RouteContext, forParentRef gwapiv1.ParentRefere
 	return ctx
 }
 
-func isParentRefEqual(ref1, ref2 gwapiv1.ParentReference, routeNS string) bool {
+// IsParentRefEqual compares two ParentReference objects for equality without using reflection.
+func IsParentRefEqual(ref1, ref2 gwapiv1.ParentReference, routeNS string) bool {
+	// Compare Group with default handling
 	defaultGroup := (*gwapiv1.Group)(&gwapiv1.GroupVersion.Group)
-	if ref1.Group == nil {
-		ref1.Group = defaultGroup
+	group1 := ref1.Group
+	if group1 == nil {
+		group1 = defaultGroup
+	}
+	group2 := ref2.Group
+	if group2 == nil {
+		group2 = defaultGroup
 	}
-	if ref2.Group == nil {
-		ref2.Group = defaultGroup
+	if *group1 != *group2 {
+		return false
 	}
 
+	// Compare Kind with default handling
 	defaultKind := gwapiv1.Kind(resource.KindGateway)
-	if ref1.Kind == nil {
-		ref1.Kind = &defaultKind
+	kind1 := ref1.Kind
+	if kind1 == nil {
+		kind1 = &defaultKind
 	}
-	if ref2.Kind == nil {
-		ref2.Kind = &defaultKind
+	kind2 := ref2.Kind
+	if kind2 == nil {
+		kind2 = &defaultKind
+	}
+	if *kind1 != *kind2 {
+		return false
 	}
 
-	// If the parent's namespace is not set, default to the namespace of the Route.
+	// Compare Name (required field)
+	if ref1.Name != ref2.Name {
+		return false
+	}
+
+	// Compare Namespace with default handling
 	defaultNS := gwapiv1.Namespace(routeNS)
-	if ref1.Namespace == nil {
-		ref1.Namespace = &defaultNS
+	namespace1 := ref1.Namespace
+	if namespace1 == nil {
+		namespace1 = &defaultNS
+	}
+	namespace2 := ref2.Namespace
+	if namespace2 == nil {
+		namespace2 = &defaultNS
+	}
+	if *namespace1 != *namespace2 {
+		return false
 	}
-	if ref2.Namespace == nil {
-		ref2.Namespace = &defaultNS
+
+	// Compare SectionName (optional field)
+	if ref1.SectionName == nil && ref2.SectionName == nil {
+		return true
+	}
+	if ref1.SectionName == nil || ref2.SectionName == nil {
+		return false
+	}
+	if *ref1.SectionName != *ref2.SectionName {
+		return false
+	}
+
+	// Compare Port (optional field)
+	if ref1.Port == nil && ref2.Port == nil {
+		return true
+	}
+	if ref1.Port == nil || ref2.Port == nil {
+		return false
 	}
-	return reflect.DeepEqual(ref1, ref2)
+	return *ref1.Port == *ref2.Port
 }
 
 // RouteParentContext wraps a ParentReference and provides helper methods for
diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go
index 7a3d90909..b2930b270 100644
--- a/internal/gatewayapi/securitypolicy.go
+++ b/internal/gatewayapi/securitypolicy.go
@@ -56,31 +56,43 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security
 	resources *resource.Resources,
 	xdsIR resource.XdsIRMap,
 ) []*egv1a1.SecurityPolicy {
-	var res []*egv1a1.SecurityPolicy
 	// SecurityPolicies are already sorted by the provider layer
 
 	// First build a map out of the routes and gateways for faster lookup since users might have thousands of routes or more.
 	// For gateways this probably isn't quite as necessary.
-	routeMap := map[policyTargetRouteKey]*policyRouteTargetContext{}
+	routeMapSize := len(routes)
+	gatewayMapSize := len(gateways)
+	policyMapSize := len(securityPolicies)
+
+	// Pre-allocate result slice and maps with estimated capacity to reduce memory allocations
+	res := make([]*egv1a1.SecurityPolicy, 0, len(securityPolicies))
+	routeMap := make(map[policyTargetRouteKey]*policyRouteTargetContext, routeMapSize)
 	for _, route := range routes {
 		key := policyTargetRouteKey{
 			Kind:      string(route.GetRouteType()),
 			Name:      route.GetName(),
 			Namespace: route.GetNamespace(),
 		}
-		routeMap[key] = &policyRouteTargetContext{RouteContext: route, attachedToRouteRules: make(sets.Set[string])}
+		routeMap[key] = &policyRouteTargetContext{
+			RouteContext:         route,
+			attachedToRouteRules: make(sets.Set[string]),
+		}
 	}
-	gatewayMap := map[types.NamespacedName]*policyGatewayTargetContext{}
+
+	gatewayMap := make(map[types.NamespacedName]*policyGatewayTargetContext, gatewayMapSize)
 	for _, gw := range gateways {
 		key := utils.NamespacedName(gw)
-		gatewayMap[key] = &policyGatewayTargetContext{GatewayContext: gw, attachedToListeners: make(sets.Set[string])}
+		gatewayMap[key] = &policyGatewayTargetContext{
+			GatewayContext:      gw,
+			attachedToListeners: make(sets.Set[string]),
+		}
 	}
 
 	// Map of Gateway to the routes attached to it.
 	// The routes are grouped by sectionNames of their targetRefs
-	gatewayRouteMap := make(map[string]map[string]sets.Set[string])
+	gatewayRouteMap := make(map[string]map[string]sets.Set[string], gatewayMapSize)
 
-	handledPolicies := make(map[types.NamespacedName]*egv1a1.SecurityPolicy)
+	handledPolicies := make(map[types.NamespacedName]*egv1a1.SecurityPolicy, policyMapSize)
 
 	// Translate
 	// 1. First translate Policies targeting RouteRules
@@ -622,6 +634,28 @@ func (t *Translator) translateSecurityPolicyForRoute(
 			}
 		}
 
+		// Pre-create security features to avoid repeated allocations
+		securityFeatures := &ir.SecurityFeatures{
+			CORS:          cors,
+			JWT:           jwt,
+			OIDC:          oidc,
+			APIKeyAuth:    apiKeyAuth,
+			BasicAuth:     basicAuth,
+			ExtAuth:       extAuth,
+			Authorization: authorization,
+		}
+
+		// Pre-create error response to avoid repeated allocations
+		var errorResponse *ir.CustomResponse
+		if errs != nil {
+			shouldFailOpen := extAuthErr != nil && !hasNonExtAuthError && ptr.Deref(policy.Spec.ExtAuth.FailOpen, false)
+			if !shouldFailOpen {
+				errorResponse = &ir.CustomResponse{
+					StatusCode: ptr.To(uint32(500)),
+				}
+			}
+		}
+
 		irKey := t.getIRKey(gtwCtx.Gateway)
 		for _, listener := range parentRefCtx.listeners {
 			irListener := xdsIR[irKey].GetHTTPListener(irListenerName(listener))
@@ -640,26 +674,10 @@ func (t *Translator) translateSecurityPolicyForRoute(
 							continue
 						}
 
-						r.Security = &ir.SecurityFeatures{
-							CORS:          cors,
-							JWT:           jwt,
-							OIDC:          oidc,
-							APIKeyAuth:    apiKeyAuth,
-							BasicAuth:     basicAuth,
-							ExtAuth:       extAuth,
-							Authorization: authorization,
-						}
-						if errs != nil {
-							// If there is only error for ext auth and ext auth is set to fail open, then skip the ext auth
-							// and allow the request to go through.
-							// Otherwise, return a 500 direct response to avoid unauthorized access.
-							shouldFailOpen := extAuthErr != nil && !hasNonExtAuthError && ptr.Deref(policy.Spec.ExtAuth.FailOpen, false)
-							if !shouldFailOpen {
-								// Return a 500 direct response to avoid unauthorized access
-								r.DirectResponse = &ir.CustomResponse{
-									StatusCode: ptr.To(uint32(500)),
-								}
-							}
+						r.Security = securityFeatures
+						if errorResponse != nil {
+							// Return a 500 direct response to avoid unauthorized access
+							r.DirectResponse = errorResponse
 						}
 					}
 				}
@@ -760,6 +778,30 @@ func (t *Translator) translateSecurityPolicyForGateway(
 	// Should exist since we've validated this
 	x := xdsIR[irKey]
 
+	// Pre-create security features and error response to avoid repeated allocations
+	securityFeatures := &ir.SecurityFeatures{
+		CORS:          cors,
+		JWT:           jwt,
+		OIDC:          oidc,
+		APIKeyAuth:    apiKeyAuth,
+		BasicAuth:     basicAuth,
+		ExtAuth:       extAuth,
+		Authorization: authorization,
+	}
+
+	var errorResponse *ir.CustomResponse
+	if errs != nil {
+		// If there is only error for ext auth and ext auth is set to fail open, then skip the ext auth
+		// and allow the request to go through.
+		// Otherwise, return a 500 direct response to avoid unauthorized access.
+		shouldFailOpen := extAuthErr != nil && !hasNonExtAuthError && ptr.Deref(policy.Spec.ExtAuth.FailOpen, false)
+		if !shouldFailOpen {
+			errorResponse = &ir.CustomResponse{
+				StatusCode: ptr.To(uint32(500)),
+			}
+		}
+	}
+
 	policyTarget := irStringKey(policy.Namespace, string(target.Name))
 	for _, h := range x.HTTP {
 		// A HTTPListener name has the format namespace/gatewayName/listenerName
@@ -772,6 +814,7 @@ func (t *Translator) translateSecurityPolicyForGateway(
 		if target.SectionName != nil && string(*target.SectionName) != h.Metadata.SectionName {
 			continue
 		}
+
 		// A Policy targeting the specific scope(xRoute rule, xRoute, Gateway listener) wins over a policy
 		// targeting a lesser specific scope(Gateway).
 		for _, r := range h.Routes {
@@ -779,25 +822,9 @@ func (t *Translator) translateSecurityPolicyForGateway(
 			if r.Security != nil {
 				continue
 			}
-			r.Security = &ir.SecurityFeatures{
-				CORS:          cors,
-				JWT:           jwt,
-				OIDC:          oidc,
-				APIKeyAuth:    apiKeyAuth,
-				BasicAuth:     basicAuth,
-				ExtAuth:       extAuth,
-				Authorization: authorization,
-			}
-			if errs != nil {
-				// If there is only error for ext auth and ext auth is set to fail open, then skip the ext auth
-				// and allow the request to go through.
-				// Otherwise, return a 500 direct response to avoid unauthorized access.
-				shouldFailOpen := extAuthErr != nil && !hasNonExtAuthError && ptr.Deref(policy.Spec.ExtAuth.FailOpen, false)
-				if !shouldFailOpen {
-					r.DirectResponse = &ir.CustomResponse{
-						StatusCode: ptr.To(uint32(500)),
-					}
-				}
+			r.Security = securityFeatures
+			if errorResponse != nil {
+				r.DirectResponse = errorResponse
 			}
 		}
 	}
diff --git a/internal/xds/translator/route.go b/internal/xds/translator/route.go
index 9007fa7d5..07a0dbf27 100644
--- a/internal/xds/translator/route.go
+++ b/internal/xds/translator/route.go
@@ -8,6 +8,7 @@ package translator
 import (
 	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 	"time"
 
@@ -440,7 +441,9 @@ func useRegexRewriteForPrefixMatchReplace(pathMatch *ir.StringMatch, prefixMatch
 func prefix2RegexRewrite(prefix string) *matcherv3.RegexMatchAndSubstitute {
 	return &matcherv3.RegexMatchAndSubstitute{
 		Pattern: &matcherv3.RegexMatcher{
-			Regex: "^" + prefix + `\/*`,
+			// Escape prefix for regex metacharacters
+			// https://github.com/envoyproxy/gateway/issues/6857
+			Regex: "^" + regexp.QuoteMeta(prefix) + `\/*`,
 		},
 		Substitution: "/",
 	}
diff --git a/release-notes/current.yaml b/release-notes/current.yaml
index da978b9a0..fe2f34858 100644
--- a/release-notes/current.yaml
+++ b/release-notes/current.yaml
@@ -27,6 +27,7 @@ bug fixes: |
   Fixed ProxyTopologyInjector cache sync race condition that caused injection failures
   Fixed validation for grpc routes with extension ref filters.
   Fixed service account token handling in GatewayNamespaceMode to use SDS for properly refreshing expired token.
+  Fixed handling of regex meta characters in prefix match replace for URL rewrite.
 
 # Enhancements that improve performance.
 performance improvements: |



Test patch:
diff --git a/internal/xds/translator/testdata/in/xds-ir/http-route-rewrite-url-regex.yaml b/internal/xds/translator/testdata/in/xds-ir/http-route-rewrite-url-regex.yaml
index 73b64d7c0..35f0684e3 100644
--- a/internal/xds/translator/testdata/in/xds-ir/http-route-rewrite-url-regex.yaml
+++ b/internal/xds/translator/testdata/in/xds-ir/http-route-rewrite-url-regex.yaml
@@ -25,6 +25,20 @@ http:
         regexMatchReplace:
           pattern: '^/service/([^/]+)(/.*)$'
           substitution: '\2/instance/\1'
+  - name: "rewrite-route-with-special-characters"
+    pathMatch:
+      prefix: "/$env"
+    hostname: gateway.envoyproxy.io
+    destination:
+      name: "rewrite-route"
+      settings:
+      - endpoints:
+        - host: "1.2.3.4"
+          port: 50000
+        name: "rewrite-route/backend/0"
+    urlRewrite:
+      path:
+        prefixMatchReplace: /
   - name: "rewrite-route-timeout"
     pathMatch:
       prefix: "/origin"
diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-url-regex.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-url-regex.routes.yaml
index ee5a5f691..89a1d81ca 100644
--- a/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-url-regex.routes.yaml
+++ b/internal/xds/translator/testdata/out/xds-ir/http-route-rewrite-url-regex.routes.yaml
@@ -16,6 +16,17 @@
           substitution: \2/instance/\1
         upgradeConfigs:
         - upgradeType: websocket
+    - match:
+        pathSeparatedPrefix: /$env
+      name: rewrite-route-with-special-characters
+      route:
+        cluster: rewrite-route
+        regexRewrite:
+          pattern:
+            regex: ^/\$env\/*
+          substitution: /
+        upgradeConfigs:
+        - upgradeType: websocket
     - match:
         pathSeparatedPrefix: /origin
       name: rewrite-route-timeout