ruanweihong-aaa commented on code in PR #13800:
URL: https://github.com/apache/apisix/pull/13800#discussion_r3755210852


##########
apisix/plugins/proxy-rewrite.lua:
##########
@@ -355,8 +385,14 @@ function _M.rewrite(conf, ctx)
             if captures then
                 ctx.proxy_rewrite_regex_uri_captures = captures
 
+                local replacement = preserve_literal_dollars(conf.regex_uri[i 
+ 1])
+                replacement = 
core.utils.resolve_var_with_captures(replacement, captures)
+                replacement = core.utils.resolve_var(replacement, ctx.var, 
escape_separator)
+                replacement = restore_literal_dollars(replacement)
                 local uri, _, err = re_sub(upstream_uri,
-                    conf.regex_uri[i], conf.regex_uri[i + 1], "jo")
+                    conf.regex_uri[i], function()
+                        return replacement
+                    end, "jo")

Review Comment:
   The additional steps preserve three different replacement semantics:
   
   1. `$1` and `$2` are regex captures.
   2. `$arg_name` and values from `ctx.var` are NGINX variables.
   3. `$$x` must remain the literal string `$x`.
   
   Resolving NGINX variables directly would also consume `$1`, while letting 
`ngx.re.sub` process the replacement after variable resolution could interpret 
dollar signs from resolved values again.
   
   I also tested resolving variables after `ngx.re.sub`, but at that point an 
escaped NGINX variable and an existing literal `$$x` both become `$...`, so 
they can no longer be distinguished.
   
   I can move these stages into a focused helper to make the rewrite path 
easier to read. Would that address the concern, or would you prefer a different 
replacement behavior for literal `$$`?



-- 
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