Copilot commented on code in PR #12914:
URL: https://github.com/apache/apisix/pull/12914#discussion_r2735077959
##########
apisix/plugins/authz-keycloak.lua:
##########
@@ -567,7 +567,7 @@ local function evaluate_permissions(conf, ctx, token)
end
-- Resolve URI to resource(s).
- permission, err = authz_keycloak_resolve_resource(conf,
ctx.var.request_uri,
+ permission, err = authz_keycloak_resolve_resource(conf, ctx.var.uri,
sa_access_token)
Review Comment:
Using `ctx.var.uri` does strip the query string, but it also changes
semantics vs `ctx.var.request_uri`: `$uri` is normalized/decoded and can be
modified by internal rewrites (e.g., proxy-rewrite), while `$request_uri` is
the original raw request (path+args). If the intent is only to drop the query
string while preserving the original request path used for Keycloak resource
matching, consider extracting the path component from `ctx.var.request_uri`
(split on `?`) instead of switching variables, or update the PR
description/tests to cover the rewrite/normalization behavior change.
--
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]
