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.

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: add option to normalize uri like servlet #6984

Merged
merged 1 commit into from
May 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
63 changes: 58 additions & 5 deletions apisix/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ local error = error
local ipairs = ipairs
local ngx_now = ngx.now
local ngx_var = ngx.var
local re_split = require("ngx.re").split
local str_byte = string.byte
local str_sub = string.sub
local tonumber = tonumber
Expand Down Expand Up @@ -251,6 +252,44 @@ local function verify_tls_client(ctx)
end


local function normalize_uri_like_servlet(uri)
local found = core.string.find(uri, ';')
if not found then
return uri
end

local segs, err = re_split(uri, "/", "jo")
if not segs then
return nil, err
end

local len = #segs
for i = 1, len do
local seg = segs[i]
local pos = core.string.find(seg, ';')
if pos then
seg = seg:sub(1, pos - 1)
-- reject bad uri which bypasses with ';'
if seg == "." or seg == ".." then
return nil, "dot segment with parameter"
end
if seg == "" and i < len then
return nil, "empty segment with parameters"
end

segs[i] = seg

seg = seg:lower()
if seg == "%2e" or seg == "%2e%2e" then
return nil, "encoded dot segment"
end
end
end

return core.table.concat(segs, '/')
end


local function common_phase(phase_name)
local api_ctx = ngx.ctx.api_ctx
if not api_ctx then
Expand Down Expand Up @@ -284,11 +323,25 @@ function _M.http_access_phase()
debug.dynamic_debug(api_ctx)

local uri = api_ctx.var.uri
if local_conf.apisix and local_conf.apisix.delete_uri_tail_slash then
if str_byte(uri, #uri) == str_byte("/") then
api_ctx.var.uri = str_sub(api_ctx.var.uri, 1, #uri - 1)
core.log.info("remove the end of uri '/', current uri: ",
api_ctx.var.uri)
if local_conf.apisix then
if local_conf.apisix.delete_uri_tail_slash then
if str_byte(uri, #uri) == str_byte("/") then
api_ctx.var.uri = str_sub(api_ctx.var.uri, 1, #uri - 1)
core.log.info("remove the end of uri '/', current uri: ", api_ctx.var.uri)
end
end

if local_conf.apisix.normalize_uri_like_servlet then
local new_uri, err = normalize_uri_like_servlet(uri)
if not new_uri then
core.log.error("failed to normalize: ", err)
return core.response.exit(400)
end

api_ctx.var.uri = new_uri
-- forward the original uri so the servlet upstream
-- can consume the param after ';'
api_ctx.var.upstream_uri = uri
end
end

Expand Down
5 changes: 5 additions & 0 deletions conf/config-default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ apisix:
role: viewer

delete_uri_tail_slash: false # delete the '/' at the end of the URI
# The URI normalization in servlet is a little different from the RFC's.
# See https://github.com/jakartaee/servlet/blob/master/spec/src/main/asciidoc/servlet-spec-body.adoc#352-uri-path-canonicalization,
# which is used under Tomcat.
# Turn this option on if you want to be compatible with servlet when matching URI path.
normalize_uri_like_servlet: false
router:
http: radixtree_uri # radixtree_uri: match route by uri(base on radixtree)
# radixtree_host_uri: match route by host + uri(base on radixtree)
Expand Down
102 changes: 102 additions & 0 deletions t/router/radixtree-uri-sanity.t
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ worker_connections(256);
no_root_location();
no_shuffle();

our $servlet_yaml_config = <<_EOC_;
apisix:
node_listen: 1984
admin_key: null
normalize_uri_like_servlet: true
_EOC_

run_tests();

__DATA__
Expand Down Expand Up @@ -295,3 +302,98 @@ GET /hello/
--- error_code: 404
--- no_error_log
[error]
--- response_body
{"error_msg":"404 Route Not Found"}



=== TEST 16: match route like servlet
--- yaml_config eval: $::servlet_yaml_config
--- request
GET /hello;world
--- response_body eval
qr/404 Not Found/
--- error_code: 404
--- no_error_log
[error]



=== TEST 17: plugin should work on the normalized url
--- 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,
[[{
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/*",
"plugins": {
"uri-blocker": {
"block_rules": ["/hello/world"]
}
}
}]]
)

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



=== TEST 18: hit
--- yaml_config eval: $::servlet_yaml_config
--- request
GET /hello;a=b/world;a/;
--- error_code: 403
--- no_error_log
[error]



=== TEST 19: reject bad uri
--- yaml_config eval: $::servlet_yaml_config
--- config
location /t {
content_by_lua_block {
local http = require "resty.http"
local uri = "http://127.0.0.1:" .. ngx.var.server_port
for _, path in ipairs({
"/;/a", "/%2e;", "/%2E%2E;", "/.;", "/..;",
"/%2E%2e;", "/b/;/c"
}) do
local httpc = http.new()
local res, err = httpc:request_uri(uri .. path)
if not res then
ngx.say(err)
return
end

if res.status ~= 400 then
ngx.say(path, " ", res.status)
end
end

ngx.say("ok")
}
}
--- request
GET /t
--- response_body
ok