Skip to content

feat: Expose an option for (real_)request_uri to be used in proxy-rewrite plugin #7386

Description

@ilteriseroglu-ty

Description

Hello!

During our tests we have noticed that APISIX always uses the normalized/decoded URI coming from nginx (as per its design, which isn't an issue), and this creates an edge case where a backend that requires URL Encoded URIs to not work properly (i.e. GitLab API calls that reference paths inside a project use %2F instead of /).

Looking at the codebase, we've also seen the native request_uri variable being moved to the real_request_uri variable. And because of how proxy-rewrites rewrite schema work; utilizing real_request_uri to bypass the encoding is not possible.

We know that this is a really unsafe behavior to utilize real_request_uri but the cost of the edge cases this causes is critical. We've copied over proxy-rewrite to our downstream custom plugins repo as proxy-rewrite-unsafe with our addition of the real_request_uri override:

--- proxy-rewrite.lua	2022-07-05 09:49:53.000000000 +0300
+++ proxy-rewrite-unsafe.lua	2022-07-05 10:02:39.000000000 +0300
@@ -15,7 +15,7 @@
 -- limitations under the License.
 --
 local core        = require("apisix.core")
-local plugin_name = "proxy-rewrite"
+local plugin_name = "proxy-rewrite-unsafe"
 local pairs       = pairs
 local ipairs      = ipairs
 local ngx         = ngx
@@ -40,6 +40,11 @@
 local schema = {
     type = "object",
     properties = {
+        use_real_request_uri_unsafe = {
+            description = "use real_request_uri (request_uri in nginx), THIS IS UNSAFE.)",
+            type        = "boolean",
+            default     = false,
+        },
         uri = {
             description = "new uri for upstream",
             type        = "string",
@@ -161,7 +166,9 @@
     end
 
     local upstream_uri = ctx.var.uri
-    if conf.uri ~= nil then
+    if conf.use_real_request_uri_unsafe then
+        upstream_uri = core.utils.resolve_var("$real_request_uri", ctx.var)
+    elseif conf.uri ~= nil then
         upstream_uri = core.utils.resolve_var(conf.uri, ctx.var)
     elseif conf.regex_uri ~= nil then
         local uri, _, err = re_sub(ctx.var.uri, conf.regex_uri[1],
@@ -178,18 +185,22 @@
     end
 
     local index = str_find(upstream_uri, "?")
-    if index then
-        upstream_uri = core.utils.uri_safe_encode(sub_str(upstream_uri, 1, index-1)) ..
-                       sub_str(upstream_uri, index)
-    else
-        upstream_uri = core.utils.uri_safe_encode(upstream_uri)
+    if not conf.use_real_request_uri_unsafe then
+        if index then
+            upstream_uri = core.utils.uri_safe_encode(sub_str(upstream_uri, 1, index-1)) ..
+                           sub_str(upstream_uri, index)
+        else
+            upstream_uri = core.utils.uri_safe_encode(upstream_uri)
+        end
     end
 
     if ctx.var.is_args == "?" then
-        if index then
-            ctx.var.upstream_uri = upstream_uri .. "&" .. (ctx.var.args or "")
-        else
-            ctx.var.upstream_uri = upstream_uri .. "?" .. (ctx.var.args or "")
+        if not conf.use_real_request_uri_unsafe then
+            if index then
+                ctx.var.upstream_uri = upstream_uri .. "&" .. (ctx.var.args or "")
+            else
+                ctx.var.upstream_uri = upstream_uri .. "?" .. (ctx.var.args or "")
+            end
         end
     else
         ctx.var.upstream_uri = upstream_uri

Is it possible to expose an option on upstream like in the diff above?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions