feat(router): add match_uri_encoded_slash to keep %2F in path parameters#13626
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.
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.
|
[P1] The encoded-slash match URI currently leaks beyond route matching. When Why this blocks merge: the PR says this only affects route matching and that upstream behavior is unchanged, but many existing plugins use Suggested fix: keep the encoded URI as a router-match-only value, restore the normalized |
Previously the encoded-slash uri was written into api_ctx.var.uri (and request_uri) and never restored, so rewrite/access-phase plugins and the upstream observed the encoded uri instead of the normalized one -- the option leaked beyond route matching (e.g. proxy-rewrite / uri-based policy plugins). Now the encoded uri is swapped in only for router dispatch and restored right after, so plugins and the upstream keep seeing the normalized ctx.var.uri; only the matched route and its captured params (uri_param_*) retain %2F. Add a regression test: with the option on, a serverless rewrite hook observes the normalized uri while uri_param_id still captures the encoded slash.
…cases - drop the redundant real_request_uri guard (always set for HTTP requests) - use core.string.find instead of str_find, dropping the extra import - document the upper-case %2F requirement and the composition with delete_uri_tail_slash / normalize_uri_like_servlet (fail-closed fallback) - add a test asserting the upstream receives the decoded uri
nginx forwards the original request line through the variable proxy_pass, so the upstream receives %2F unchanged rather than the decoded uri. Fix the docs, the config comment and the TEST 13 assertion accordingly. The router-match-only handling of ctx.var.uri (plugins still see the decoded uri) is unchanged.
Encoded-slash forwarding to the upstream is verified independently (nginx variable proxy_pass with an empty upstream_uri forwards the original request line, so the upstream receives %2F unchanged, confirmed via a standalone nginx probe). Asserting the upstream-received uri from inside test-nginx proved unreliable, so drop the test instead of keeping a flaky assertion; the docs describe the forwarding behavior.
Add an echo endpoint on the fake upstream (returns its own request_uri) and a route /echo/:id, then assert that GET /echo/a%2Fb reaches the upstream as /echo/a%2Fb unchanged. This both proves the feature matches the %2F request and pins the upstream-forwarding contract, using a response_body assertion instead of the unreliable error-log approach that was dropped earlier.
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)