Skip to content

Commit

Permalink
fix(router) ensure rebuilds when Routes have 'regex_priority = nil'
Browse files Browse the repository at this point in the history
If Routes have `regex_priority` properties mixed between numbers and nil
(e.g. one Route was created with a default `regex_priority`, the other
was created with a purposefully NULL `regex_priority`, the comparison in
the router rebuild sorting of Routes would error out, because we are
comparing one Route's `nil` value against another Route's `number`.

Fix #4254
  • Loading branch information
thibaultcha authored and kikito committed Jan 29, 2019
1 parent f87a82f commit f541c5c
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
9 changes: 7 additions & 2 deletions kong/runloop/handler.lua
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,15 @@ do

sort(routes, function(r1, r2)
r1, r2 = r1.route, r2.route
if r1.regex_priority == r2.regex_priority then

local rp1 = r1.regex_priority or 0
local rp2 = r2.regex_priority or 0

if rp1 == rp2 then
return r1.created_at < r2.created_at
end
return r1.regex_priority > r2.regex_priority

return rp1 > rp2
end)

local err
Expand Down
31 changes: 31 additions & 0 deletions spec/02-integration/05-proxy/02-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ local function insert_routes(routes)
end

local function remove_routes(routes)
if not routes then
return
end

local services = {}

for _, route in ipairs(routes) do
Expand Down Expand Up @@ -1093,7 +1097,34 @@ for _, strategy in helpers.each_strategy() do
end
end)

describe("router rebuilds", function()
local routes

lazy_teardown(function()
remove_routes(routes)
end)

it("when Routes have 'regex_priority = nil'", function()
-- Regression test for issue:
-- https://github.com/Kong/kong/issues/4254
routes = insert_routes {
{
methods = { "GET" },
regex_priority = 1,
},
{
methods = { "POST", "PUT" },
regex_priority = ngx.null,
},
}

local res = assert(proxy_client:send {
method = "GET",
})

assert.response(res).has_status(200)
end)
end)
end)
end)
end

0 comments on commit f541c5c

Please sign in to comment.