Copilot commented on code in PR #12843:
URL: https://github.com/apache/apisix/pull/12843#discussion_r2726779689
##########
apisix/plugins/proxy-rewrite.lua:
##########
@@ -275,15 +275,27 @@ function _M.rewrite(conf, ctx)
end
end
+ -- normalized uri (without query string)
local upstream_uri = ctx.var.uri
local separator_escaped = false
+
+ local query_string = ""
+ -- When use_real_request_uri_unsafe is true, we use real_request_uri
directly
if conf.use_real_request_uri_unsafe then
+ -- Not normalized uri (with query string if available)
upstream_uri = ctx.var.real_request_uri
+ local query_string_index = str_find(upstream_uri, "?")
+ -- If query string exists, extract it. It is needed later when
conf.uri is set.
+ -- Because core.utils.resolve_var output will drop the query string
part.
+ query_string = query_string_index and sub_str(upstream_uri,
query_string_index) or ""
end
+ -- This block determines the upstream URI based on the configuration.
+ -- uri has higher priority than regex_uri.
if conf.uri ~= nil then
+ -- Set the upstream_uri by resolving variables in conf.uri
+ upstream_uri = core.utils.resolve_var(conf.uri, ctx.var,
escape_separator) .. query_string
Review Comment:
When `use_real_request_uri_unsafe` is true and `conf.uri` already contains a
query string (e.g. `/path?x=1`), concatenating `.. query_string` (which starts
with `?`) produces an invalid URI like `/path?x=1?y=2`. This also diverges from
the non-unsafe branch which merges query params using `?`/`&` logic. Please
merge the extracted query into `conf.uri` the same way: if the resolved
`conf.uri` already has `?`, append `&` plus the extracted query without its
leading `?`; otherwise append it as-is. Add/adjust a test to cover `conf.uri`
containing its own query when `use_real_request_uri_unsafe=true`.
```suggestion
local resolved_uri = core.utils.resolve_var(conf.uri, ctx.var,
escape_separator)
if query_string ~= "" then
-- Merge extracted query string into resolved_uri:
-- if resolved_uri already has a query part, append with '&',
-- otherwise append the extracted query as-is.
if str_find(resolved_uri, "?", 1, true) then
upstream_uri = resolved_uri .. "&" .. sub_str(query_string,
2)
else
upstream_uri = resolved_uri .. query_string
end
else
upstream_uri = resolved_uri
end
```
--
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]
