membphis commented on PR #13626:
URL: https://github.com/apache/apisix/pull/13626#issuecomment-4888430353

   [P1] The encoded-slash match URI currently leaks beyond route matching.
   
   When `match_uri_encoded_slash` is enabled and 
`build_match_uri_keep_encoded_slash()` succeeds, the implementation writes the 
encoded-slash-preserved value back into `api_ctx.var.uri` and then rebuilds 
`api_ctx.var.request_uri` from it. That value is not restored after router 
dispatch, so route/global plugins in the later rewrite/access phases also 
observe the encoded match URI.
   
   Why this blocks merge: the PR says this only affects route matching and that 
upstream behavior is unchanged, but many existing plugins use `ctx.var.uri` for 
security or rewrite decisions, for example `authz-casbin`, OPA, OAS validation, 
and `proxy-rewrite`. A request such as `/v1/te%2Fst/products/electronics/list` 
can therefore make those plugins see `/v1/te%2Fst/...` instead of the 
normalized `/v1/te/st/...`, which changes the behavior of opt-in deployments 
beyond routing.
   
   Suggested fix: keep the encoded URI as a router-match-only value, restore 
the normalized `ctx.var.uri` / `ctx.var.request_uri` immediately after 
matching, and retain only the matched route/params from the encoded match. 
Please add a regression test showing that with `match_uri_encoded_slash=true`, 
a plugin or serverless hook still sees the normalized URI while route params 
can still capture the encoded slash.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to