Skip to content

Commit

Permalink
change: don't treat route segment with ':' as parameter by default
Browse files Browse the repository at this point in the history
Fix #3134.

You can still get the original behavior by using
radixtree_uri_with_parameter router.

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
  • Loading branch information
spacewander committed Dec 30, 2020
1 parent 45e909a commit adf3ff8
Show file tree
Hide file tree
Showing 17 changed files with 463 additions and 127 deletions.
2 changes: 1 addition & 1 deletion apisix/admin/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
--
local require = require
local core = require("apisix.core")
local route = require("resty.radixtree")
local route = require("apisix.utils.router")
local plugin = require("apisix.plugin")
local ngx = ngx
local get_method = ngx.req.get_method
Expand Down
2 changes: 1 addition & 1 deletion apisix/api_router.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
-- limitations under the License.
--
local require = require
local router = require("resty.radixtree")
local router = require("apisix.utils.router")
local plugin_mod = require("apisix.plugin")
local ip_restriction = require("apisix.plugins.ip-restriction")
local core = require("apisix.core")
Expand Down
2 changes: 1 addition & 1 deletion apisix/control/router.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
-- limitations under the License.
--
local require = require
local router = require("resty.radixtree")
local router = require("apisix.utils.router")
local builtin_v1_routes = require("apisix.control.v1")
local plugin_mod = require("apisix.plugin")
local core = require("apisix.core")
Expand Down
118 changes: 118 additions & 0 deletions apisix/http/router/base.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
--
-- 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.
--
local require = require
local radixtree = require("resty.radixtree")
local router = require("apisix.utils.router")
local core = require("apisix.core")
local http_route = require("apisix.http.route")
local ipairs = ipairs
local type = type
local error = error
local loadstring = loadstring


local _M = {}


function _M.create_radixtree_uri_router(routes, uri_routes, with_parameter)
routes = routes or {}

core.table.clear(uri_routes)

for _, route in ipairs(routes) do
if type(route) == "table" then
local status = core.table.try_read_attr(route, "value", "status")
-- check the status
if status and status == 0 then
goto CONTINUE
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)
goto CONTINUE
end

filter_fun = filter_fun()
end

core.log.info("insert uri route: ",
core.json.delay_encode(route.value))
core.table.insert(uri_routes, {
paths = route.value.uris or route.value.uri,
methods = route.value.methods,
priority = route.value.priority,
hosts = route.value.hosts or route.value.host,
remote_addrs = route.value.remote_addrs
or route.value.remote_addr,
vars = route.value.vars,
filter_fun = filter_fun,
handler = function (api_ctx, match_opts)
api_ctx.matched_params = nil
api_ctx.matched_route = route
api_ctx.curr_req_matched = match_opts.matched
end
})

::CONTINUE::
end
end

core.log.info("route items: ", core.json.delay_encode(uri_routes, true))

if with_parameter then
return radixtree.new(uri_routes)
else
return router.new(uri_routes)
end
end


function _M.match_uri(uri_router, match_opts, api_ctx)
core.table.clear(match_opts)
match_opts.method = api_ctx.var.request_method
match_opts.host = api_ctx.var.host
match_opts.remote_addr = api_ctx.var.remote_addr
match_opts.vars = api_ctx.var
match_opts.matched = core.tablepool.fetch("matched_route_record", 0, 4)

local ok = uri_router:dispatch(api_ctx.var.uri, match_opts, api_ctx, match_opts)
return ok
end


function _M.init_worker(filter)
local user_routes, err = core.config.new("/routes", {
automatic = true,
item_schema = core.schema.route,
checker = http_route.check_route,
filter = filter,
})
if not user_routes then
error("failed to create etcd instance for fetching /routes : " .. err)
end

return user_routes
end


return _M
29 changes: 2 additions & 27 deletions apisix/http/router/radixtree_host_uri.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,13 @@
-- limitations under the License.
--
local require = require
local router = require("resty.radixtree")
local router = require("apisix.utils.router")
local core = require("apisix.core")
local http_route = require("apisix.http.route")
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
local only_uri_router
Expand Down Expand Up @@ -126,6 +123,7 @@ end

local match_opts = {}
function _M.match(api_ctx)
local user_routes = _M.user_routes
if not cached_version or cached_version ~= user_routes.conf_version then
create_radixtree_router(user_routes.values)
cached_version = user_routes.conf_version
Expand All @@ -151,27 +149,4 @@ function _M.match(api_ctx)
end


function _M.routes()
if not user_routes then
return nil, nil
end

return user_routes.values, user_routes.conf_version
end


function _M.init_worker(filter)
local err
user_routes, err = core.config.new("/routes", {
automatic = true,
item_schema = core.schema.route,
checker = http_route.check_route,
filter = filter,
})
if not user_routes then
error("failed to create etcd instance for fetching /routes : " .. err)
end
end


return _M
99 changes: 5 additions & 94 deletions apisix/http/router/radixtree_uri.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,8 @@
-- limitations under the License.
--
local require = require
local router = require("resty.radixtree")
local core = require("apisix.core")
local http_route = require("apisix.http.route")
local ipairs = ipairs
local type = type
local error = error
local loadstring = loadstring
local user_routes
local base_router = require("apisix.http.router.base")
local cached_version


Expand All @@ -31,64 +25,12 @@ local _M = {version = 0.2}

local uri_routes = {}
local uri_router
local function create_radixtree_router(routes)
routes = routes or {}

core.table.clear(uri_routes)

for _, route in ipairs(routes) do
if type(route) == "table" then
local status = core.table.try_read_attr(route, "value", "status")
-- check the status
if status and status == 0 then
goto CONTINUE
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)
goto CONTINUE
end

filter_fun = filter_fun()
end

core.log.info("insert uri route: ",
core.json.delay_encode(route.value))
core.table.insert(uri_routes, {
paths = route.value.uris or route.value.uri,
methods = route.value.methods,
priority = route.value.priority,
hosts = route.value.hosts or route.value.host,
remote_addrs = route.value.remote_addrs
or route.value.remote_addr,
vars = route.value.vars,
filter_fun = filter_fun,
handler = function (api_ctx, match_opts)
api_ctx.matched_params = nil
api_ctx.matched_route = route
api_ctx.curr_req_matched = match_opts.matched
end
})

::CONTINUE::
end
end

core.log.info("route items: ", core.json.delay_encode(uri_routes, true))
uri_router = router.new(uri_routes)
end


local match_opts = {}
function _M.match(api_ctx)
local user_routes = _M.user_routes
if not cached_version or cached_version ~= user_routes.conf_version then
create_radixtree_router(user_routes.values)
uri_router = base_router.create_radixtree_uri_router(user_routes.values,
uri_routes, false)
cached_version = user_routes.conf_version
end

Expand All @@ -97,38 +39,7 @@ function _M.match(api_ctx)
return true
end

core.table.clear(match_opts)
match_opts.method = api_ctx.var.request_method
match_opts.host = api_ctx.var.host
match_opts.remote_addr = api_ctx.var.remote_addr
match_opts.vars = api_ctx.var
match_opts.matched = core.tablepool.fetch("matched_route_record", 0, 4)

local ok = uri_router:dispatch(api_ctx.var.uri, match_opts, api_ctx, match_opts)
return ok
end


function _M.routes()
if not user_routes then
return nil, nil
end

return user_routes.values, user_routes.conf_version
end


function _M.init_worker(filter)
local err
user_routes, err = core.config.new("/routes", {
automatic = true,
item_schema = core.schema.route,
checker = http_route.check_route,
filter = filter,
})
if not user_routes then
error("failed to create etcd instance for fetching /routes : " .. err)
end
return base_router.match_uri(uri_router, match_opts, api_ctx)
end


Expand Down
46 changes: 46 additions & 0 deletions apisix/http/router/radixtree_uri_with_parameter.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
--
-- 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.
--
local require = require
local core = require("apisix.core")
local base_router = require("apisix.http.router.base")
local cached_version


local _M = {}


local uri_routes = {}
local uri_router
local match_opts = {}
function _M.match(api_ctx)
local user_routes = _M.user_routes
if not cached_version or cached_version ~= user_routes.conf_version then
uri_router = base_router.create_radixtree_uri_router(user_routes.values,
uri_routes, true)
cached_version = user_routes.conf_version
end

if not uri_router then
core.log.error("failed to fetch valid `uri_with_parameter` router: ")
return true
end

return base_router.match_uri(uri_router, match_opts, api_ctx)
end


return _M
Loading

0 comments on commit adf3ff8

Please sign in to comment.