Skip to content

Commit

Permalink
fix(router) do not match another API with a longer URI
Browse files Browse the repository at this point in the history
When matching URIs, we iterate over a list of APIs, and not a list of
URIs (for various, performance reasons, although a refactor of the
router is scheduled with some performance improvements later on).
This list of APIs is sorted per URI length. But an API can have more
than one URI. As such, the router can potentially evaluate an API with a
very long URI, and a much shorter one that matches the request URI,
instead of a following API which would be an exact match for that API.

To fix this, we sort our deserialized list of URIs to iterate on in the
select method, and if we do have a match, we conserve the URI that was
matched, to check if it belongs to the right API at API match time.

Fix #2662
  • Loading branch information
thibaultcha committed Jul 20, 2017
1 parent 71a0280 commit 49493d0
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 0 deletions.
5 changes: 5 additions & 0 deletions kong/core/router.lua
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,9 @@ function _M.new(apis)
return max_uri_a > max_uri_b
end

table.sort(uris_prefixes, function(a, b)
return #a > #b
end)

for category_bit, category in pairs(categories) do
table.sort(category.apis, function(a, b)
Expand Down Expand Up @@ -572,6 +575,8 @@ function _M.new(apis)
end

if from then
-- strip \Q...\E tokens
uri = sub(uris_prefixes[i], 3, -3)
req_category = bor(req_category, MATCH_RULES.URI)
break
end
Expand Down
19 changes: 19 additions & 0 deletions spec/01-unit/010-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,25 @@ describe("Router", function()
assert.same(use_case[#use_case], api_t.api)
end)
end)

it("does not incorrectly match another API which has a longer [uri]", function()
local use_case = {
{
name = "api-1",
uris = { "/a", "/bbbbbbb" }
},
{
name = "api-2",
uris = { "/a/bb" }
},
}

local router = assert(Router.new(use_case))

local api_t = router.select("GET", "/a/bb/foobar")
assert.truthy(api_t)
assert.same(use_case[2], api_t.api)
end)
end)

describe("misses", function()
Expand Down

0 comments on commit 49493d0

Please sign in to comment.