From b3f597b4ba4d05dc8123bc0c162918e4cd867af6 Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Thu, 7 Nov 2019 08:01:10 +0800 Subject: [PATCH 1/5] bugfix(radixtree_host_uri): match priority, host > uri. Fix #761. --- lua/apisix/http/router/radixtree_host_uri.lua | 139 ++++++------------ t/router/radixtree-host-uri2.t | 137 +++++++++++++++++ 2 files changed, 182 insertions(+), 94 deletions(-) create mode 100644 t/router/radixtree-host-uri2.t diff --git a/lua/apisix/http/router/radixtree_host_uri.lua b/lua/apisix/http/router/radixtree_host_uri.lua index a4ebc432721a..6a329cf500bb 100644 --- a/lua/apisix/http/router/radixtree_host_uri.lua +++ b/lua/apisix/http/router/radixtree_host_uri.lua @@ -21,127 +21,77 @@ local plugin = require("apisix.plugin") local ipairs = ipairs local type = type local error = error -local loadstring = loadstring +local tab_insert = table.insert local user_routes local cached_version -local only_uri_routes = {} +local host_router local only_uri_router -local host_uri_routes = {} -local host_uri_router local _M = {version = 0.1} -local function add_host_uri_routes(path, host, route) - core.log.info("add host+uri route, host: ", host, " path: ", path) - - local filter_fun, err - if route.value.filter_func then - filter_fun, err = loadstring( - "return " .. route.value.filter_func, - "router#" .. route.value.id) - if not filter_fun then - core.log.error("failed to load filter function: ", err, - " route id: ", route.value.id) - return - end - - filter_fun = filter_fun() +local function push_host_router(route, host_routes, only_uri_routes) + if type(route) ~= "table" then + return end - core.table.insert(host_uri_routes, { - paths = {host .. path}, + local hosts = route.value.hosts or {route.value.host} + + local radixtree_route = { + paths = route.value.uris or route.value.uri, methods = route.value.methods, - remote_addrs = route.value.remote_addrs or route.value.remote_addr, + remote_addrs = route.value.remote_addrs + or route.value.remote_addr, vars = route.value.vars, - filter_fun = filter_fun, + -- filter_fun = filter_fun, handler = function (api_ctx) api_ctx.matched_params = nil api_ctx.matched_route = route - end, - }) -end - - -local function push_radixtree_host_router(route) - if type(route) ~= "table" then - return - end - - local hosts = route.value.hosts or {route.value.host} - local hosts_wildcard = {} - local uris = route.value.uris or {route.value.uri} - - local added_count = 0 - for _, host in ipairs(hosts) do - if host:sub(1, 1) == "*" then - core.table.insert(hosts_wildcard, host) - else - for _, uri in ipairs(uris) do - add_host_uri_routes(uri, host, route) - end - added_count = added_count + 1 end - end + } - -- 4 cases: - -- hosts = {} - -- hosts = {"foo.com"} - -- hosts = {"*.foo.com", "bar.com"} - -- hosts = {"*.foo.com", "*.bar.com"} - if added_count > 0 and added_count == #hosts then + if #hosts == 0 then + core.table.insert(only_uri_routes, radixtree_route) return end - if #hosts_wildcard == 0 then - hosts_wildcard = nil - end - - local filter_fun, err - if route.value.filter_func then - filter_fun, err = loadstring( - "return " .. route.value.filter_func, - "router#" .. route.value.id) - if not filter_fun then - core.log.error("failed to load filter function: ", err, - " route id: ", route.value.id) - return + for i, host in ipairs(hosts) do + local host_rev = host:reverse() + if not host_routes[host_rev] then + host_routes[host_rev] = {radixtree_route} + else + tab_insert(host_routes[host_rev], radixtree_route) end - - filter_fun = filter_fun() end - - core.table.insert(only_uri_routes, { - paths = uris, - method = route.value.methods, - hosts = hosts_wildcard, - remote_addrs = route.value.remote_addrs or route.value.remote_addr, - vars = route.value.vars, - filter_fun = filter_fun, - handler = function (api_ctx) - api_ctx.matched_params = nil - api_ctx.matched_route = route - end, - }) - - return end local function create_radixtree_router(routes) - core.table.clear(host_uri_routes) - core.table.clear(only_uri_routes) - host_uri_router = nil - only_uri_router = nil + local host_routes = {} + local only_uri_routes = {} + host_router = nil for _, route in ipairs(routes or {}) do - push_radixtree_host_router(route) + push_host_router(route, host_routes, only_uri_routes) end - -- create router: host_uri_router - if #host_uri_routes > 0 then - host_uri_router = router.new(host_uri_routes) + -- create router: host_router + local host_router_routes = {} + for host_rev, routes in pairs(host_routes) do + local sub_router = router.new(routes) + + core.table.insert(host_router_routes, { + paths = host_rev, + filter_fun = function(vars, opts, api_ctx, ...) + return sub_router:dispatch(vars.uri, opts, api_ctx, ...) + end, + handler = function (api_ctx) + end + }) + end + if #host_router_routes > 0 then + host_router = router.new(host_router_routes) end -- create router: only_uri_router @@ -175,10 +125,11 @@ function _M.match(api_ctx) match_opts.remote_addr = api_ctx.var.remote_addr match_opts.vars = api_ctx.var match_opts.host = api_ctx.var.host + api_ctx.radixtree_opts = match_opts - if host_uri_router then - local host_uri = api_ctx.var.host .. api_ctx.var.uri - local ok = host_uri_router:dispatch(host_uri, match_opts, api_ctx) + if host_router then + local host_uri = api_ctx.var.host + local ok = host_router:dispatch(host_uri:reverse(), match_opts, api_ctx) if ok then return true end diff --git a/t/router/radixtree-host-uri2.t b/t/router/radixtree-host-uri2.t new file mode 100644 index 000000000000..1efafb6ffefd --- /dev/null +++ b/t/router/radixtree-host-uri2.t @@ -0,0 +1,137 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX 'no_plan'; + +repeat_each(1); +log_level('info'); +worker_connections(256); +no_root_location(); +no_shuffle(); + +sub read_file($) { + my $infile = shift; + open my $in, $infile + or die "cannot open $infile for reading: $!"; + my $cert = do { local $/; <$in> }; + close $in; + $cert; +} + +our $yaml_config = read_file("conf/config.yaml"); +$yaml_config =~ s/node_listen: 9080/node_listen: 1984/; +$yaml_config =~ s/enable_heartbeat: true/enable_heartbeat: false/; +$yaml_config =~ s/config_center: etcd/config_center: yaml/; +$yaml_config =~ s/enable_admin: true/enable_admin: false/; +$yaml_config =~ s/http: 'radixtree_uri'/http: 'radixtree_host_uri'/; + +run_tests(); + +__DATA__ + +=== TEST 1: test.com +--- yaml_config eval: $::yaml_config +--- apisix_yaml +routes: + - + uri: /server_port + host: test.com + upstream: + nodes: + "127.0.0.1:1981": 1 + type: roundrobin + - + uri: /server_port + upstream: + nodes: + "127.0.0.1:1980": 1 + type: roundrobin +#END + +--- request +GET /server_port +--- more_headers +Host: test.com +--- response_body eval +qr/1981/ +--- error_log +use config_center: yaml +--- no_error_log +[error] + + + +=== TEST 2: sanity +--- yaml_config eval: $::yaml_config +--- apisix_yaml +routes: + - + uri: /server_port + host: *.test.com + upstream: + nodes: + "127.0.0.1:1981": 1 + type: roundrobin + - + uri: /server_port + upstream: + nodes: + "127.0.0.1:1980": 1 + type: roundrobin +#END + +--- request +GET /server_port +--- more_headers +Host: www.test.com +--- response_body eval +qr/1981/ +--- error_log +use config_center: yaml +--- no_error_log +[error] + + + +=== TEST 3: sanity +--- yaml_config eval: $::yaml_config +--- apisix_yaml +routes: + - + uri: /* + host: *.test.com + upstream: + nodes: + "127.0.0.1:1981": 1 + type: roundrobin + - + uri: /server_port + upstream: + nodes: + "127.0.0.1:1980": 1 + type: roundrobin +#END + +--- request +GET /server_port +--- more_headers +Host: www.test.com +--- response_body eval +qr/1981/ +--- error_log +use config_center: yaml +--- no_error_log +[error] From 74cf45fb62365a4afa8e774a08bb701474b860e5 Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Thu, 7 Nov 2019 08:12:02 +0800 Subject: [PATCH 2/5] change: removed useless code. --- lua/apisix/http/router/radixtree_host_uri.lua | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lua/apisix/http/router/radixtree_host_uri.lua b/lua/apisix/http/router/radixtree_host_uri.lua index 6a329cf500bb..bf01c6d1c717 100644 --- a/lua/apisix/http/router/radixtree_host_uri.lua +++ b/lua/apisix/http/router/radixtree_host_uri.lua @@ -83,8 +83,8 @@ local function create_radixtree_router(routes) core.table.insert(host_router_routes, { paths = host_rev, - filter_fun = function(vars, opts, api_ctx, ...) - return sub_router:dispatch(vars.uri, opts, api_ctx, ...) + filter_fun = function(vars, opts, ...) + return sub_router:dispatch(vars.uri, opts, ...) end, handler = function (api_ctx) end @@ -125,7 +125,6 @@ function _M.match(api_ctx) match_opts.remote_addr = api_ctx.var.remote_addr match_opts.vars = api_ctx.var match_opts.host = api_ctx.var.host - api_ctx.radixtree_opts = match_opts if host_router then local host_uri = api_ctx.var.host From 0985084df5fd3ce4454847697733352304b99161 Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Thu, 7 Nov 2019 08:17:32 +0800 Subject: [PATCH 3/5] bugfix: loaded filter_fun . --- lua/apisix/http/router/radixtree_host_uri.lua | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/lua/apisix/http/router/radixtree_host_uri.lua b/lua/apisix/http/router/radixtree_host_uri.lua index bf01c6d1c717..f372c1efae9d 100644 --- a/lua/apisix/http/router/radixtree_host_uri.lua +++ b/lua/apisix/http/router/radixtree_host_uri.lua @@ -36,6 +36,20 @@ local function push_host_router(route, host_routes, only_uri_routes) return end + local filter_fun, err + if route.value.filter_func then + filter_fun, err = loadstring( + "return " .. route.value.filter_func, + "router#" .. route.value.id) + if not filter_fun then + core.log.error("failed to load filter function: ", err, + " route id: ", route.value.id) + return + end + + filter_fun = filter_fun() + end + local hosts = route.value.hosts or {route.value.host} local radixtree_route = { @@ -44,7 +58,7 @@ local function push_host_router(route, host_routes, only_uri_routes) remote_addrs = route.value.remote_addrs or route.value.remote_addr, vars = route.value.vars, - -- filter_fun = filter_fun, + filter_fun = filter_fun, handler = function (api_ctx) api_ctx.matched_params = nil api_ctx.matched_route = route From 52cfdac35fef973921527125478b2ccfb54e9058 Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Thu, 7 Nov 2019 08:21:31 +0800 Subject: [PATCH 4/5] test: added new test cases. --- t/router/radixtree-host-uri2.t | 70 +++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/t/router/radixtree-host-uri2.t b/t/router/radixtree-host-uri2.t index 1efafb6ffefd..1c7c3c3724ec 100644 --- a/t/router/radixtree-host-uri2.t +++ b/t/router/radixtree-host-uri2.t @@ -74,7 +74,7 @@ use config_center: yaml -=== TEST 2: sanity +=== TEST 2: *.test.com + uri --- yaml_config eval: $::yaml_config --- apisix_yaml routes: @@ -106,7 +106,7 @@ use config_center: yaml -=== TEST 3: sanity +=== TEST 3: *.test.com + /* --- yaml_config eval: $::yaml_config --- apisix_yaml routes: @@ -135,3 +135,69 @@ qr/1981/ use config_center: yaml --- no_error_log [error] + + + +=== TEST 4: filter_func(not match) +--- yaml_config eval: $::yaml_config +--- apisix_yaml +routes: + - + uri: /* + host: *.test.com + filter_func: "function(vars) return vars.arg_name == 'json' end" + upstream: + nodes: + "127.0.0.1:1981": 1 + type: roundrobin + - + uri: /server_port + upstream: + nodes: + "127.0.0.1:1980": 1 + type: roundrobin +#END + +--- request +GET /server_port?name=unknown +--- more_headers +Host: www.test.com +--- response_body eval +qr/1980/ +--- error_log +use config_center: yaml +--- no_error_log +[error] + + + +=== TEST 5: filter_func(match) +--- yaml_config eval: $::yaml_config +--- apisix_yaml +routes: + - + uri: /* + host: *.test.com + filter_func: "function(vars) return vars.arg_name == 'json' end" + upstream: + nodes: + "127.0.0.1:1981": 1 + type: roundrobin + - + uri: /server_port + upstream: + nodes: + "127.0.0.1:1980": 1 + type: roundrobin +#END + +--- request +GET /server_port?name=json +--- more_headers +Host: www.test.com +--- response_body eval +qr/1981/ +--- error_log +use config_center: yaml +--- no_error_log +[error] From 0fc5bbfe2561f0e5a9853c1cb02dd6506d87af7b Mon Sep 17 00:00:00 2001 From: Yuansheng Date: Thu, 7 Nov 2019 12:50:23 +0800 Subject: [PATCH 5/5] change: local cache. --- Makefile | 4 +++- lua/apisix/http/router/radixtree_host_uri.lua | 2 ++ lua/apisix/schema_def.lua | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index b5d17852fd36..06e8fef8ad45 100644 --- a/Makefile +++ b/Makefile @@ -65,10 +65,12 @@ endif check: .travis/openwhisk-utilities/scancode/scanCode.py --config .travis/ASF-Release.cfg ./ luacheck -q lua - ./utils/lj-releng lua/*.lua lua/apisix/*.lua \ + ./utils/lj-releng lua/*.lua \ + lua/apisix/*.lua \ lua/apisix/admin/*.lua \ lua/apisix/core/*.lua \ lua/apisix/http/*.lua \ + lua/apisix/http/router/*.lua \ lua/apisix/plugins/*.lua \ lua/apisix/plugins/grpc-transcode/*.lua \ lua/apisix/plugins/limit-count/*.lua > \ diff --git a/lua/apisix/http/router/radixtree_host_uri.lua b/lua/apisix/http/router/radixtree_host_uri.lua index f372c1efae9d..051d3b1d4e02 100644 --- a/lua/apisix/http/router/radixtree_host_uri.lua +++ b/lua/apisix/http/router/radixtree_host_uri.lua @@ -22,6 +22,8 @@ local ipairs = ipairs local type = type local error = error local tab_insert = table.insert +local loadstring = loadstring +local pairs = pairs local user_routes local cached_version local host_router diff --git a/lua/apisix/schema_def.lua b/lua/apisix/schema_def.lua index 7d9832fcd00a..ec8eca43c639 100644 --- a/lua/apisix/schema_def.lua +++ b/lua/apisix/schema_def.lua @@ -253,7 +253,8 @@ local upstream_schema = { description = "the key of chash for dynamic load balancing", type = "string", pattern = [[^((uri|server_name|server_addr|request_uri|remote_port]] - .. [[|remote_addr|query_string|host|hostname)|arg_[0-9a-zA-z_-]+)$]], + .. [[|remote_addr|query_string|host|hostname)]] + .. [[|arg_[0-9a-zA-z_-]+)$]], }, desc = {type = "string", maxLength = 256}, id = id_schema