feat(router): add match_uri_encoded_slash to keep %2F in path parameters#13626
Open
AlinsRan wants to merge 9 commits into
Open
feat(router): add match_uri_encoded_slash to keep %2F in path parameters#13626AlinsRan wants to merge 9 commits into
AlinsRan wants to merge 9 commits into
Conversation
6ca9581 to
fa757bc
Compare
3df89d7 to
b9da49e
Compare
Nginx decodes an URL-encoded slash (%2F) into a real '/' in $uri before route matching, so a request like /v1/te%2Fst/products/electronics/list is seen as /v1/te/st/products/electronics/list and fails to match a route with path parameters such as /v1/:id/products/:type/list. Add an opt-in apisix.preserve_encoded_slash option. When enabled, the route matching uri is rebuilt from the raw request_uri with every percent-encoding decoded except %2F, which stays encoded so it is treated as part of a path parameter instead of a separator. The encoded slash is then forwarded to the upstream as is. '.' and '..' segments are still normalized to prevent path traversal, and a decoded null byte is rejected. The rebuild only runs when an encoded slash is present, so normal requests keep using the normalized $uri with no overhead. Closes apache#11810
b9da49e to
6e47607
Compare
Address review feedback on the preserve_encoded_slash option: - remove_dot_segments: ngx.re.split drops trailing empty fields, which turned "/", "//" and paths that resolve to the root into an empty string and silently stripped a trailing slash. Rebuild the path with a leading "/", merge consecutive slashes like Nginx, and re-add the trailing slash only when at least one segment remains. - access phase: only inspect the path, so an encoded slash in the query string no longer triggers a rebuild. - docs: warn that the option is global and clarify that only real, decoded path separators are normalized. - tests: assert route matching vs route-not-found, upstream forwarding of %2F, and cover root, trailing-slash and query-string cases.
Reorder the segment handling so there is no empty if branch (luacheck 542), keeping the same merge/skip/pop behaviour.
Address review feedback: - decode_uri_keep_encoded_slash now propagates the ngx.re.gsub error instead of silently passing nil to the null-byte check. - run the preserve_encoded_slash rebuild before delete_uri_tail_slash and normalize_uri_like_servlet, and feed its result into them, so those options are no longer undone when an encoded slash is present.
…ative-match cases The upstream-forwarding assertion and the no-match negative case depend on cross-file etcd state in the shared test suite and are not deterministic. Keep the path-parameter matching cases, which cover the fix for issue apache#11810.
… preserve_encoded_slash preserve_encoded_slash only affects route matching; the request forwarded to the upstream still uses APISIX's normal (decoded) URI. Remove the inaccurate 'forwarded to the upstream as is' wording and add the Chinese documentation.
…_encoded_slash The option only affects how the matching URI is normalized (it keeps %2F encoded instead of decoding it to a separator); it does not preserve the encoded slash end-to-end. Rename it to sit alongside normalize_uri_like_servlet and reflect that it is a URI-normalization variant used for route matching.
Name it in the delete_uri_tail_slash / normalize_uri_like_servlet family (verb_uri_object) and scope it to route matching. The option only changes how %2F is treated when matching the URI; it does not preserve the encoded slash end-to-end. The internal helper keeps the descriptive name normalize_uri_keep_encoded_slash.
shreemaan-abhishek
previously approved these changes
Jul 2, 2026
Rebuilding the matching URI from the raw request_uri forced us to re-implement Nginx's whole URI normalization (dot segments, slash merge, NUL, trailing slash) in Lua, which was the entire safety surface. Instead, use the already-normalized $uri as an oracle: keep %2F encoded only when a plain full decode of the request path reproduces $uri exactly (proving Nginx did nothing beyond decoding). The matching URI is then provably either $uri or $uri with some '/' shown as %2F. Any request that also needed normalization (traversal, //, %00, absolute-form, fragment) fails the equivalence check and falls back to $uri, so no bypass is possible even if the decode is wrong. The kept slash is normalized to upper-case %2F. Requests combining %2F with dot segments or consecutive slashes now fall back to $uri matching (previously matched via the hand-rolled normalization); such ambiguous requests are handled conservatively. Tests updated accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Nginx decodes an URL-encoded slash (
%2F) into a real/in$uribeforeroute matching happens. As a result a request like
/v1/te%2Fst/products/electronics/listis seen by the router as/v1/te/st/products/electronics/list, which has a different number of pathsegments and therefore fails to match a route defined with path parameters such
as
/v1/:id/products/:type/list(returns404 Route Not Found). This is theproblem reported in #11810, and it cannot be fixed in
lua-resty-radixtree(the parameter pattern
[^/]+must stop at a real slash to keep segmentationcorrect) because the information is already lost by the time the router runs.
This PR adds an opt-in
apisix.match_uri_encoded_slashoption (defaultfalse).When enabled, the route matching uri keeps
%2F/%2fencoded so it is treatedas part of a path parameter instead of a separator.
This only affects route matching: the request forwarded to the upstream
still uses APISIX's normal (decoded) URI, so upstream behaviour is unchanged.
How it stays safe
Rather than re-implementing Nginx's URI normalization in Lua (dot-segment
resolution, slash merging, NUL rejection, …), the option uses the
already-normalized
$urias an oracle:$uri.../, nomerged
//, no fragment, not an absolute-form request line). Only then doesit build the matching uri by decoding everything except
%2F. The resultprovably differs from
$urionly by showing some/as%2F.niland matching falls back to the normalized$uri.So traversal (
..%2F..%2F,%2e%2e), consecutive slashes,%00and exoticrequest lines all fail the equivalence check and fall back — no bypass is
possible even if the decode itself is wrong. The kept slash is normalized to
upper-case
%2F. Requests without%2Fin the path skip the check entirely andkeep using
$uriwith no overhead.Combining
%2Fwith dot segments or consecutive slashes therefore falls back tostandard
$urimatching (such ambiguous requests are handled conservatively,similar to Envoy's default for escaped-slash normalization).
Which issue(s) this PR fixes:
Fixes #11810
Checklist
false)