diff --git a/apisix/init.lua b/apisix/init.lua index 12fa94037316..4306809d05dc 100644 --- a/apisix/init.lua +++ b/apisix/init.lua @@ -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 @@ -262,6 +263,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 @@ -295,11 +334,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 diff --git a/conf/config-default.yaml b/conf/config-default.yaml index 0bda63548555..2d266b5b7e73 100644 --- a/conf/config-default.yaml +++ b/conf/config-default.yaml @@ -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) diff --git a/t/router/radixtree-uri-sanity.t b/t/router/radixtree-uri-sanity.t index e91834dec57f..d49285ff1c93 100644 --- a/t/router/radixtree-uri-sanity.t +++ b/t/router/radixtree-uri-sanity.t @@ -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__ @@ -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