Skip to content

router: checked path rewrite utility and rewrite overrides - #47841

Merged
agrawroh merged 3 commits into
envoyproxy:mainfrom
agrawroh:der-pr4-path-rewrite-utility
Sep 29, 2026
Merged

agrawroh merged 3 commits into
envoyproxy:mainfrom
agrawroh:der-pr4-path-rewrite-utility

Conversation

@agrawroh

Copy link
Copy Markdown
Member

Description

This PR adds a shared checked path rewrite utility for the redirect, formatter and path rewriter callers, and a prefix rewrite callback and a regex rewrite override for the dynamic modules route specifier.


Commit Message: router: checked path rewrite utility and rewrite overrides
Risk Level: Low
Testing: Added Tests
Docs Changes: Added
Release Notes: Added

@repokitteh-read-only

Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @wbpcode
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

ðŸą

Caused by: #47841 was opened by agrawroh.

see: more, trace.

Moves generateNewPath and rewritePathByPrefixOrRegex out of config_impl.cc into
a shared path_rewrite_utility that returns nullopt instead of asserting on a
strip longer than the path and caps the result at a byte bound, so the redirect,
formatter and path rewriter callers can never underflow the reserve size or
throw on an oversized rewrite.

Uses the utility for the dynamic modules route specifier so a module can rewrite
the request path. Adds a set_prefix_rewrite callback and a RouteOverride
regex_rewrite, both computed once against the current path and capped at a new
max_rewritten_path_bytes, and exposes set_prefix_rewrite through the Rust SDK.

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
@agrawroh
agrawroh force-pushed the der-pr4-path-rewrite-utility branch from aec6293 to 15005d6 Compare September 29, 2026 06:52
wbpcode
wbpcode previously approved these changes Sep 29, 2026

@wbpcode wbpcode left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.

@agrawroh

Copy link
Copy Markdown
Member Author

/retest

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
@agrawroh
agrawroh enabled auto-merge (squash) September 29, 2026 18:26
@agrawroh
agrawroh merged commit cf98c97 into envoyproxy:main Sep 29, 2026
27 of 28 checks passed
chentianheng604 added a commit to chentianheng604/envoy that referenced this pull request Oct 12, 2026
In envoyproxy#47841, path rewrites were updated to check that the request path
starts with the matched path before rewriting (rewritePathByPrefixOrRegex),
silently dropping the rewrite if the prefix does not match.

For redirect routes using path_match_policy (UriTemplateMatcher),
rewritePathHeader passes the literal URI template pattern as the matched
path. Because concrete request paths match the pattern rather than the
literal template string, the prefix check always fails and redirect
prefix rewrites silently do nothing at runtime.

Forwarding routes already reject configuring prefix_rewrite together with
a matcher extension (path_matcher_ != nullptr) during config ingestion.
Redirect routes previously lacked this check.

Add the missing validation check to RouteEntryImplBase for redirect
routes: reject redirect.prefix_rewrite when path_matcher_ != nullptr with
absl::InvalidArgumentError("Cannot use redirect.prefix_rewrite with path_match_policy").

Guard this validation check behind the reloadable runtime feature
envoy.reloadable_features.disallow_redirect_prefix_rewrite_with_path_match_policy
(enabled by default) to prevent breaking existing xDS configurations
unexpectedly.

Signed-off-by: Tianheng Chen <tianhengchen@google.com>
chentianheng604 added a commit to chentianheng604/envoy that referenced this pull request Oct 12, 2026
In envoyproxy#47841, path rewrites were updated to check that the request path
starts with the matched path before rewriting (rewritePathByPrefixOrRegex),
silently dropping the rewrite if the prefix does not match.

For redirect routes using path_match_policy (UriTemplateMatcher),
the literal URI template pattern is passed as the matched prefix. Because
concrete request paths match the pattern rather than the literal template
string, the prefix check always fails and redirect prefix rewrites
silently do nothing at runtime.

Forwarding routes already reject configuring prefix_rewrite together with
a matcher extension during config ingestion. Redirect routes previously
lacked this check.

Add the missing validation check for redirect routes, rejecting
configurations combining redirect.prefix_rewrite with path_match_policy
during config ingestion.

Guard this validation check behind the reloadable runtime feature
envoy.reloadable_features.disallow_redirect_prefix_rewrite_with_path_match_policy
(enabled by default) to prevent breaking existing xDS configurations
unexpectedly.

Signed-off-by: Tianheng Chen <tianhengchen@google.com>
chentianheng604 added a commit to chentianheng604/envoy that referenced this pull request Oct 12, 2026
In envoyproxy#47841, path rewrites were updated to check that the request path
starts with the matched path before rewriting (rewritePathByPrefixOrRegex),
silently dropping the rewrite if the prefix does not match.

For redirect routes using path_match_policy (UriTemplateMatcher),
the literal URI template pattern is passed as the matched prefix. Because
concrete request paths match the pattern rather than the literal template
string, the prefix check always fails and redirect prefix rewrites
silently do nothing at runtime.

Forwarding routes already reject configuring prefix_rewrite together with
a matcher extension during config ingestion. Redirect routes previously
lacked this check.

Add the missing validation check for redirect routes, rejecting
configurations combining redirect.prefix_rewrite with path_match_policy
during config ingestion.

Guard this validation check behind the reloadable runtime feature
envoy.reloadable_features.disallow_redirect_prefix_rewrite_with_path_match_policy
(enabled by default) to prevent breaking existing xDS configurations
unexpectedly.

Signed-off-by: Tianheng Chen <tianhengchen@google.com>
chentianheng604 added a commit to chentianheng604/envoy that referenced this pull request Oct 12, 2026
In envoyproxy#47841, path rewrites were updated to check that the request path
starts with the matched path before rewriting (rewritePathByPrefixOrRegex),
silently dropping the rewrite if the prefix does not match.

For redirect routes using path_match_policy (UriTemplateMatcher),
the literal URI template pattern is passed as the matched prefix. Because
concrete request paths match the pattern rather than the literal template
string, the prefix check always fails and redirect prefix rewrites
silently do nothing at runtime.

Forwarding routes already reject configuring prefix_rewrite together with
a matcher extension during config ingestion. Redirect routes previously
lacked this check.

Add the missing validation check for redirect routes, rejecting
configurations combining redirect.prefix_rewrite with path_match_policy
during config ingestion.

Guard this validation check behind the reloadable runtime feature
envoy.reloadable_features.disallow_redirect_prefix_rewrite_with_path_match_policy
(enabled by default) to prevent breaking existing xDS configurations
unexpectedly.

Signed-off-by: Tianheng Chen <tianhengchen@google.com>
yanavlasov pushed a commit that referenced this pull request Oct 13, 2026
Configuring a `prefix_rewrite` requires an identifiable prefix to strip
and replace. Matcher extensions (`path_match_policy`, such as
`UriTemplateMatcher`) match complete paths or structured patterns rather
than simple prefixes, making them fundamentally incompatible with prefix
rewriting.

Envoy already enforces this constraint for forwarding routes:
[`RouteEntryImplBase`](https://github.com/envoyproxy/envoy/blob/main/source/common/router/config_impl.cc#L769-L773)
explicitly rejects `prefix_rewrite` at config ingestion time whenever a
matcher extension is configured (`path_matcher_ != nullptr`). However,
redirect routes (`RedirectAction`) previously missed this validation
check.

Historically, this gap led to runtime crashes:
[`UriTemplateMatcherRouteEntryImpl::rewritePathHeader`](https://github.com/envoyproxy/envoy/blob/main/source/common/router/config_impl.cc#L1453-L1456)
passed the literal template pattern as a pseudo-prefix to the redirect
finalizer, causing debug assertion failures or path corruption. While
[#47841](#47841) introduced a
defensive runtime prefix check in
[`rewritePathByPrefixOrRegex`](https://github.com/envoyproxy/envoy/blob/main/source/common/router/path_rewrite_utility.cc#L35-L39)
to prevent crashes, pairing these fields remains an invalid
configuration that silently fails to rewrite redirects at runtime.

This change closes the validation gap by bringing redirect routes into
alignment with forwarding routes: `RouteEntryImplBase` now rejects route
configurations that combine `redirect.prefix_rewrite` with
`path_match_policy` during config ingestion.

Risk Level: Low
Testing: Unit test
Docs Changes: No
Release Notes: Added
Runtime guard:
envoy.reloadable_features.disallow_redirect_prefix_rewrite_with_path_match_policy

Signed-off-by: Tianheng Chen <tianhengchen@google.com>
chentianheng604 added a commit to chentianheng604/envoy that referenced this pull request Oct 19, 2026
â€Ķenvoyproxy#47831)"

Reverts the route configuration ingestion check introduced in envoyproxy#47831
that rejected combining redirect.prefix_rewrite with path_match_policy,
as existing route configurations in production still use this
combination and fail config ingestion when this check is enabled.

Reverting this ingestion-time rejection is safe because envoyproxy#47841 already
added a defensive runtime prefix check in rewritePathByPrefixOrRegex
(`source/common/router/path_rewrite_utility.cc`) that verifies the
request path starts with the matched prefix before rewriting. When
UriTemplateMatcherRouteEntryImpl::rewritePathHeader passes the literal
URI template pattern as a pseudo-prefix during a redirect, the prefix
check safely skips the rewrite as a no-op instead of triggering debug
assertion failures or corrupting the redirect path.

The runtime guard
envoy.reloadable_features.disallow_redirect_prefix_rewrite_with_path_match_policy
is retained in runtime_features.cc as a no-op so runtime configurations
referencing the v1.40.0 flag continue to load without error until the
flag goes through standard deprecation.

Risk Level: Low
Testing: Existing unit tests
Docs Changes: No
Release Notes: Added

Signed-off-by: Tianheng Chen <tianhengchen@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants