Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(proxy-rewrite): create use_real_request_uri_unsafe option #7401

Merged
merged 13 commits into from
Jul 19, 2022
Merged

feat(proxy-rewrite): create use_real_request_uri_unsafe option #7401

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
9382f8b
feat(proxy-rewrite): create use_real_request_uri_unsafe option, see h…
ilteriseroglu-ty Jul 6, 2022
66bf02f
chore(t/plugin/proxy-rewrite): fix broken test response
ilteriseroglu-ty Jul 7, 2022
6de6b40
fix(docs/plugins/proxy-rewrite): lint pass
ilteriseroglu-ty Jul 7, 2022
03d10ae
fix(t/admin/global-rules): fix proxy-rewrite tests
ilteriseroglu-ty Jul 7, 2022
cd65cde
docs(proxy-rewrite): emphasize unsafeness of use_real_request_uri_unsafe
ilteriseroglu-ty Jul 7, 2022
9f46f15
chore(plugins/proxy-rewrite): remove redundant dollar sign
ilteriseroglu-ty Jul 9, 2022
7e5e5b4
chore(t/lib/server): remove unnecessary print_uri_detailed_x calls
ilteriseroglu-ty Jul 9, 2022
3a15ff3
chore(t/plugin/proxy-rewrite): apply suggestions for use_real_request…
ilteriseroglu-ty Jul 9, 2022
5bc7119
fix(t/plugin/proxy-rewrite): lint pass
ilteriseroglu-ty Jul 9, 2022
bac34eb
chore(t/plugin/proxy-rewrite): linter pass
ilteriseroglu-ty Jul 11, 2022
655bb6f
chore(plugins/proxy-rewrite): assign real_request_uri directly
ilteriseroglu-ty Jul 11, 2022
9a09018
docs(plugins/proxy-rewrite): apply suggestion
ilteriseroglu-ty Jul 18, 2022
dd5f377
chore(t/plugin/proxy-rewrite): apply suggestion
ilteriseroglu-ty Jul 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 23 additions & 14 deletions apisix/plugins/proxy-rewrite.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ local schema = {
type = "object",
minProperties = 1,
},
use_real_request_uri_unsafe = {
description = "use real_request_uri instead, THIS IS VERY UNSAFE.",
type = "boolean",
default = false,
},
},
minProperties = 1,
}
Expand Down Expand Up @@ -161,7 +166,9 @@ function _M.rewrite(conf, ctx)
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)
ilteriseroglu-ty marked this conversation as resolved.
Show resolved Hide resolved
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],
Expand All @@ -177,22 +184,24 @@ function _M.rewrite(conf, ctx)
end
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)
end

if ctx.var.is_args == "?" then
if not conf.use_real_request_uri_unsafe then
local index = str_find(upstream_uri, "?")
if index then
ctx.var.upstream_uri = upstream_uri .. "&" .. (ctx.var.args or "")
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

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 "")
end
else
ctx.var.upstream_uri = upstream_uri .. "?" .. (ctx.var.args or "")
ctx.var.upstream_uri = upstream_uri
end
else
ctx.var.upstream_uri = upstream_uri
end

if conf.headers then
Expand Down
17 changes: 9 additions & 8 deletions docs/en/latest/plugins/proxy-rewrite.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,15 @@ The `proxy-rewrite` Plugin rewrites Upstream proxy information such as `scheme`,

## Attributes

| Name | Type | Required | Default | Valid values | Description |
|-----------|---------------|----------|---------|----------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| scheme | string | False | "http" | ["http", "https"] | New upstream protocol scheme. This option is deprecated. Instead, it is recommended to set the `scheme` field in the Upstream. |
| uri | string | False | | | New Upstream forwarding address. Value supports [Nginx variables](https://nginx.org/en/docs/http/ngx_http_core_module.html). For example, `$arg_name`. |
| method | string | False | | ["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | Rewrites the HTTP method. |
| regex_uri | array[string] | False | | | New upstream forwarding address. Regular expressions can be used to match the URL from client. If it matches, the URL template is forwarded to the Upstream otherwise, the URL from the client is forwarded. When both `uri` and `regex_uri` are configured, `uri` is used first. For example, `[" ^/iresty/(.*)/(.*)/(.*)", "/$1-$2-$3"]`. Here, the first element is the regular expression to match and the second element is the URL template forwarded to the Upstream. |
| host | string | False | | | New Upstream host address. |
| headers | object | False | | | New Upstream headers. Headers are overwritten if they are already present otherwise, they are added to the present headers. To remove a header, set the header value to an empty string. The values in the header can contain Nginx variables like `$remote_addr` and `$client_addr`. |
| Name | Type | Required | Default | Valid values | Description |
|-----------------------------|---------------|----------|---------|----------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| scheme | string | False | "http" | ["http", "https"] | New upstream protocol scheme. This option is deprecated. Instead, it is recommended to set the `scheme` field in the Upstream. |
| uri | string | False | | | New Upstream forwarding address. Value supports [Nginx variables](https://nginx.org/en/docs/http/ngx_http_core_module.html). For example, `$arg_name`. |
| method | string | False | | ["GET", "POST", "PUT", "HEAD", "DELETE", "OPTIONS","MKCOL", "COPY", "MOVE", "PROPFIND", "PROPFIND","LOCK", "UNLOCK", "PATCH", "TRACE"] | Rewrites the HTTP method. |
| regex_uri | array[string] | False | | | New upstream forwarding address. Regular expressions can be used to match the URL from client. If it matches, the URL template is forwarded to the Upstream otherwise, the URL from the client is forwarded. When both `uri` and `regex_uri` are configured, `uri` is used first. For example, `[" ^/iresty/(.*)/(.*)/(.*)", "/$1-$2-$3"]`. Here, the first element is the regular expression to match and the second element is the URL template forwarded to the Upstream. |
| host | string | False | | | New Upstream host address. |
| headers | object | False | | | New Upstream headers. Headers are overwritten if they are already present otherwise, they are added to the present headers. To remove a header, set the header value to an empty string. The values in the header can contain Nginx variables like `$remote_addr` and `$client_addr`. |
| use_real_request_uri_unsafe | boolean | False | false | | Use real_request_uri (originally request_uri in nginx) to bypass URI normalization. **Enabling this is considered unsafe as it bypasses all URI normalization steps**. |
ilteriseroglu-ty marked this conversation as resolved.
Show resolved Hide resolved

## Enabling the Plugin

Expand Down
6 changes: 3 additions & 3 deletions t/admin/global-rules.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ passed
}
}
--- response_body
{"action":"set","node":{"key":"/apisix/global_rules/1","value":{"id":"1","plugins":{"proxy-rewrite":{"uri":"/"}}}}}
{"action":"set","node":{"key":"/apisix/global_rules/1","value":{"id":"1","plugins":{"proxy-rewrite":{"uri":"/","use_real_request_uri_unsafe":false}}}}}
--- request
GET /t
--- no_error_log
Expand Down Expand Up @@ -485,7 +485,7 @@ GET /t
}
}
--- response_body
{"action":"compareAndSwap","node":{"key":"/apisix/global_rules/1","value":{"id":"1","plugins":{"proxy-rewrite":{"uri":"/"}}}}}
{"action":"compareAndSwap","node":{"key":"/apisix/global_rules/1","value":{"id":"1","plugins":{"proxy-rewrite":{"uri":"/","use_real_request_uri_unsafe":false}}}}}
--- request
GET /t
--- no_error_log
Expand Down Expand Up @@ -521,7 +521,7 @@ GET /t
}
}
--- response_body
{"action":"get","node":{"key":"/apisix/global_rules/1","value":{"id":"1","plugins":{"proxy-rewrite":{"uri":"/"}}}}}
{"action":"get","node":{"key":"/apisix/global_rules/1","value":{"id":"1","plugins":{"proxy-rewrite":{"uri":"/","use_real_request_uri_unsafe":false}}}}}
--- request
GET /t
--- no_error_log
Expand Down
2 changes: 1 addition & 1 deletion t/admin/global-rules2.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ __DATA__
}
}
--- response_body
{"action":"set","node":{"key":"/apisix/global_rules/1","value":{"id":"1","plugins":{"proxy-rewrite":{"uri":"/"}}}}}
{"action":"set","node":{"key":"/apisix/global_rules/1","value":{"id":"1","plugins":{"proxy-rewrite":{"uri":"/","use_real_request_uri_unsafe":false}}}}}



Expand Down
4 changes: 4 additions & 0 deletions t/lib/server.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,10 @@ for i = 1, 100 do
_M["print_uri_" .. i] = print_uri
end

function _M.print_uri_detailed()
ngx.say("ngx.var.uri: ", ngx.var.uri)
ngx.say("ngx.var.request_uri: ", ngx.var.request_uri)
end

function _M.headers()
local args = ngx.req.get_uri_args()
Expand Down
2 changes: 1 addition & 1 deletion t/plugin/proxy-rewrite.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,7 @@ q: apisix)
--- request
GET /t
--- response_body
{"proxy-rewrite":{"headers":{"X-Api":"v2"},"uri":"/uri/plugin_proxy_rewrite"}}
{"proxy-rewrite":{"headers":{"X-Api":"v2"},"uri":"/uri/plugin_proxy_rewrite","use_real_request_uri_unsafe":false}}
--- no_error_log
[error]

Expand Down
100 changes: 100 additions & 0 deletions t/plugin/proxy-rewrite3.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,103 @@ passed
GET /hello
--- error_log
plugin_proxy_rewrite get method: POST



=== TEST 10: set route(unsafe uri not normalized at request)
--- 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,
[[{
"methods": ["GET"],
"plugins": {
"proxy-rewrite": {
"use_real_request_uri_unsafe": true
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/print_uri_detailed"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
ilteriseroglu-ty marked this conversation as resolved.
Show resolved Hide resolved
GET /t
--- response_body
passed
--- no_error_log
[error]



=== TEST 11: unsafe uri not normalized at request
--- request
GET /print%5Furi%5Fdetailed HTTP/1.1
--- response_body
ngx.var.uri: /print_uri_detailed
ngx.var.request_uri: /print%5Furi%5Fdetailed
--- no_error_log
[error]



=== TEST 12: set route(safe uri not normalized at request)
--- 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,
[[{
"methods": ["GET"],
"plugins": {
"proxy-rewrite": {
"use_real_request_uri_unsafe": true
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/print_uri_detailed"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed
--- no_error_log
[error]



=== TEST 13: safe uri not normalized at request
--- request
GET /print_uri_detailed HTTP/1.1
--- response_body
ngx.var.uri: /print_uri_detailed
ngx.var.request_uri: /print_uri_detailed
--- no_error_log
[error]
soulbird marked this conversation as resolved.
Show resolved Hide resolved