From b201ff53461d2b3a1129d4d425972e6280b6291d Mon Sep 17 00:00:00 2001
From: spacewander <spacewanderlzx@gmail.com>
Date: Mon, 20 Feb 2023 16:05:19 +0800
Subject: [PATCH 1/2] fix(proxy-rewrite): escape args part if it's not from
 user conf

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
---
 apisix/core/utils.lua            |   8 ++-
 apisix/plugins/proxy-rewrite.lua |  30 ++++++---
 t/plugin/proxy-rewrite3.t        | 111 +++++++++++++++++++++++++++++++
 3 files changed, 139 insertions(+), 10 deletions(-)

diff --git a/apisix/core/utils.lua b/apisix/core/utils.lua
index f72996b78d99..01c8b34c8503 100644
--- a/apisix/core/utils.lua
+++ b/apisix/core/utils.lua
@@ -293,6 +293,7 @@ do
     local _ctx
     local n_resolved
     local pat = [[(?<!\\)\$\{?(\w+)\}?]]
+    local _escaper
 
     local function resolve(m)
         local v = _ctx[m[1]]
@@ -300,10 +301,13 @@ do
             return ""
         end
         n_resolved = n_resolved + 1
+        if _escaper then
+            return _escaper(tostring(v))
+        end
         return tostring(v)
     end
 
-    function resolve_var(tpl, ctx)
+    function resolve_var(tpl, ctx, escaper)
         n_resolved = 0
         if not tpl then
             return tpl, nil, n_resolved
@@ -316,8 +320,10 @@ do
 
         -- avoid creating temporary function
         _ctx = ctx
+        _escaper = escaper
         local res, _, err = re_gsub(tpl, pat, resolve, "jo")
         _ctx = nil
+        _escaper = nil
         if not res then
             return nil, err
         end
diff --git a/apisix/plugins/proxy-rewrite.lua b/apisix/plugins/proxy-rewrite.lua
index fdb8c81840d0..74f936c6b97a 100644
--- a/apisix/plugins/proxy-rewrite.lua
+++ b/apisix/plugins/proxy-rewrite.lua
@@ -253,18 +253,24 @@ do
     end
 
 
-    function _M.rewrite(conf, ctx)
-        for _, name in ipairs(upstream_names) do
-            if conf[name] then
-                ctx.var[upstream_vars[name]] = conf[name]
-            end
+    local function escape_separator(s)
+        return re_sub(s, [[\?]], "%3F", "jo")
+    end
+
+function _M.rewrite(conf, ctx)
+    for _, name in ipairs(upstream_names) do
+        if conf[name] then
+            ctx.var[upstream_vars[name]] = conf[name]
         end
+    end
 
     local upstream_uri = ctx.var.uri
+    local separator_escaped = false
     if conf.use_real_request_uri_unsafe then
         upstream_uri = ctx.var.real_request_uri
     elseif conf.uri ~= nil then
-        upstream_uri = core.utils.resolve_var(conf.uri, ctx.var)
+        separator_escaped = true
+        upstream_uri = core.utils.resolve_var(conf.uri, ctx.var, escape_separator)
     elseif conf.regex_uri ~= nil then
         local uri, _, err = re_sub(ctx.var.uri, conf.regex_uri[1],
                                    conf.regex_uri[2], "jo")
@@ -280,11 +286,17 @@ do
     end
 
     if not conf.use_real_request_uri_unsafe then
-        local index = str_find(upstream_uri, "?")
+        local index
+        if separator_escaped then
+            index = str_find(upstream_uri, "?")
+        end
+
         if index then
-            upstream_uri = core.utils.uri_safe_encode(sub_str(upstream_uri, 1, index-1)) ..
-                           sub_str(upstream_uri, index)
+            upstream_uri = core.utils.uri_safe_encode(sub_str(upstream_uri, 1, index - 1)) ..
+                sub_str(upstream_uri, index)
         else
+            -- The '?' may come from client request '%3f' when we use ngx.var.uri directly or
+            -- via regex_uri
             upstream_uri = core.utils.uri_safe_encode(upstream_uri)
         end
 
diff --git a/t/plugin/proxy-rewrite3.t b/t/plugin/proxy-rewrite3.t
index 621c06557c56..1fdf4c0077f3 100644
--- a/t/plugin/proxy-rewrite3.t
+++ b/t/plugin/proxy-rewrite3.t
@@ -455,3 +455,114 @@ passed
 GET /echo HTTP/1.1
 --- response_headers
 test: test_in_set
+
+
+
+=== TEST 21: set route
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                ngx.HTTP_PUT,
+                [[{
+                      "plugins": {
+                          "proxy-rewrite": {
+                              "host": "test.xxxx.com"
+                          }
+                      },
+                      "upstream": {
+                          "nodes": {
+                              "127.0.0.1:8125": 1
+                          },
+                          "type": "roundrobin"
+                      },
+                      "uri": "/hello*"
+                 }]]
+                 )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 22: hit with CRLF
+--- request
+GET /hello%3f0z=700%26a=c%20HTTP/1.1%0D%0AHost:google.com%0d%0a%0d%0a
+--- http_config
+    server {
+        listen 8125;
+        location / {
+            content_by_lua_block {
+                ngx.say(ngx.var.host)
+                ngx.say(ngx.var.request_uri)
+            }
+        }
+    }
+--- response_body
+test.xxxx.com
+/hello%3F0z=700&a=c%20HTTP/1.1%0D%0AHost:google.com%0D%0A%0D%0A
+
+
+
+=== TEST 23: set route with uri
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                ngx.HTTP_PUT,
+                [[{
+                      "plugins": {
+                          "proxy-rewrite": {
+                              "uri": "/$uri/remain",
+                              "host": "test.xxxx.com"
+                          }
+                      },
+                      "upstream": {
+                          "nodes": {
+                              "127.0.0.1:8125": 1
+                          },
+                          "type": "roundrobin"
+                      },
+                      "uri": "/hello*"
+                 }]]
+                 )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 24: hit with CRLF
+--- request
+GET /hello%3f0z=700%26a=c%20HTTP/1.1%0D%0AHost:google.com%0d%0a%0d%0a
+--- http_config
+    server {
+        listen 8125;
+        location / {
+            content_by_lua_block {
+                ngx.say(ngx.var.host)
+                ngx.say(ngx.var.request_uri)
+            }
+        }
+    }
+--- response_body
+test.xxxx.com
+//hello%253F0z=700&a=c%20HTTP/1.1%0D%0AHost:google.com%0D%0A%0D%0A/remain

From 641bc2df135ddbd978e2579f9fde6203aaf01399 Mon Sep 17 00:00:00 2001
From: spacewander <spacewanderlzx@gmail.com>
Date: Thu, 23 Feb 2023 10:15:24 +0800
Subject: [PATCH 2/2] fix regex_uri with args

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
---
 apisix/plugins/proxy-rewrite.lua |  6 +++-
 t/plugin/proxy-rewrite3.t        | 53 ++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/apisix/plugins/proxy-rewrite.lua b/apisix/plugins/proxy-rewrite.lua
index 74f936c6b97a..65ffdf3abd4d 100644
--- a/apisix/plugins/proxy-rewrite.lua
+++ b/apisix/plugins/proxy-rewrite.lua
@@ -272,7 +272,11 @@ function _M.rewrite(conf, ctx)
         separator_escaped = true
         upstream_uri = core.utils.resolve_var(conf.uri, ctx.var, escape_separator)
     elseif conf.regex_uri ~= nil then
-        local uri, _, err = re_sub(ctx.var.uri, conf.regex_uri[1],
+        if not str_find(upstream_uri, "?") then
+            separator_escaped = true
+        end
+
+        local uri, _, err = re_sub(upstream_uri, conf.regex_uri[1],
                                    conf.regex_uri[2], "jo")
         if uri then
             upstream_uri = uri
diff --git a/t/plugin/proxy-rewrite3.t b/t/plugin/proxy-rewrite3.t
index 1fdf4c0077f3..613bd1b1add0 100644
--- a/t/plugin/proxy-rewrite3.t
+++ b/t/plugin/proxy-rewrite3.t
@@ -566,3 +566,56 @@ GET /hello%3f0z=700%26a=c%20HTTP/1.1%0D%0AHost:google.com%0d%0a%0d%0a
 --- response_body
 test.xxxx.com
 //hello%253F0z=700&a=c%20HTTP/1.1%0D%0AHost:google.com%0D%0A%0D%0A/remain
+
+
+
+=== TEST 25: regex_uri with args
+--- config
+    location /t {
+        content_by_lua_block {
+            local t = require("lib.test_admin").test
+            local code, body = t('/apisix/admin/routes/1',
+                ngx.HTTP_PUT,
+                [[{
+                      "plugins": {
+                          "proxy-rewrite": {
+                              "regex_uri": ["^/test/(.*)/(.*)/(.*)", "/$1_$2_$3?a=c"]
+                          }
+                      },
+                      "upstream": {
+                          "nodes": {
+                              "127.0.0.1:8125": 1
+                          },
+                          "type": "roundrobin"
+                      },
+                      "uri": "/test/*"
+                 }]]
+                 )
+
+            if code >= 300 then
+                ngx.status = code
+            end
+            ngx.say(body)
+        }
+    }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 26: hit
+--- request
+GET /test/plugin/proxy/rewrite HTTP/1.1
+--- http_config
+    server {
+        listen 8125;
+        location / {
+            content_by_lua_block {
+                ngx.say(ngx.var.request_uri)
+            }
+        }
+    }
+--- response_body
+/plugin_proxy_rewrite?a=c