Skip to content

Commit

Permalink
[squashme]
Browse files Browse the repository at this point in the history
  • Loading branch information
thibaultcha committed Nov 11, 2019
1 parent 3fb3356 commit 0b58992
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 49 deletions.
48 changes: 27 additions & 21 deletions kong/router.lua
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,17 @@ end

local split_port
do
local ZERO, NINE, LEFTBRACKET, RIGHTBRACKET = ("09[]"):byte(1, -1)


local function safe_add_port(host, port)
if not port then
return host
end

return host .. ':' .. port
return host .. ":" .. port
end

local ZERO, NINE, LEFTBRACKET, RIGHTBRACKET = ("09[]"):byte(1, -1)

local function onlydigits(s, begin)
for i = begin or 1, #s do
Expand All @@ -76,14 +78,15 @@ do
return true
end

--- splits an optional ':port' section from a hostname

--- Splits an optional ':port' section from a hostname
-- the port section must be decimal digits only.
-- brackets ('[]') are peeled off the hostname if present.
-- if there's more than one colon and no brackets, no split is possible.
-- on non-parseable input, returns name unchanged,
-- every string input produces at least one string output.
-- @tparam name string the string to split.
-- @tparam default_port number default port number
-- @tparam string name the string to split.
-- @tparam number default_port default port number
-- @treturn string hostname without port
-- @treturn string hostname with port
-- @treturn boolean true if input had a port number
Expand All @@ -96,7 +99,7 @@ do
local splitpos = find(name, "]:", 2, true)
if splitpos then
if splitpos == #name - 1 then
return sub(name, 2, splitpos - 1), name .. (default_port or ''), false
return sub(name, 2, splitpos - 1), name .. (default_port or ""), false
end

if onlydigits(name, splitpos + 2) then
Expand Down Expand Up @@ -128,13 +131,15 @@ do
return sub(name, 1, firstcolon - 1), name, true
end


-- split_port is a pure function, so we can memoize it.
local memo_h = setmetatable({}, {__mode = 'k'})
local memo_hp = setmetatable({}, {__mode = 'k'})
local memo_p = setmetatable({}, {__mode = 'k'})
local memo_h = setmetatable({}, { __mode = "k" })
local memo_hp = setmetatable({}, { __mode = "k" })
local memo_p = setmetatable({}, { __mode = "k" })

function split_port(name, default_port)
local k = name .. '#' .. (default_port or '')

split_port = function(name, default_port)
local k = name .. "#" .. (default_port or "")
local h, hp, p = memo_h[k], memo_hp[k], memo_p[k]
if not h then
h, hp, p = l_split_port(name, default_port)
Expand All @@ -146,7 +151,6 @@ do
end



--[[
Hypothesis
----------
Expand Down Expand Up @@ -181,9 +185,9 @@ sort(SORTED_MATCH_RULES, function(a, b)
end)

local MATCH_SUBRULES = {
HAS_REGEX_URI = 0x01,
PLAIN_HOSTS_ONLY = 0x02,
HAS_HOST_PORT = 0x04,
HAS_REGEX_URI = 0x01,
PLAIN_HOSTS_ONLY = 0x02,
HAS_WILDCARD_HOST_PORT = 0x04,
}

local EMPTY_T = {}
Expand Down Expand Up @@ -361,7 +365,7 @@ local function marshall_route(r)

if has_port then
route_t.submatch_weight = bor(route_t.submatch_weight,
MATCH_SUBRULES.HAS_HOST_PORT)
MATCH_SUBRULES.HAS_WILDCARD_HOST_PORT)
end
end

Expand Down Expand Up @@ -770,8 +774,7 @@ do
local matchers = {
[MATCH_RULES.HOST] = function(route_t, ctx)
local req_host = ctx.hits.host or ctx.req_host
local host = route_t.hosts[req_host]
or route_t.hosts[split_port(req_host)]
local host = route_t.hosts[req_host] or route_t.hosts[ctx.host_no_port]
if host then
ctx.matches.host = host
return true
Expand Down Expand Up @@ -1353,10 +1356,12 @@ function _M.new(routes)
-- req_host might have port or maybe not, host_no_port definitely doesn't
-- if there wasn't a port, req_port is assumed to be the default port
-- according the protocol scheme
local host_no_port, host_with_port = split_port(
req_host, req_scheme == 'https' and 443 or 80)
local host_no_port, host_with_port = split_port(req_host,
req_scheme == "https"
and 443 or 80)

ctx.host_with_port = host_with_port
ctx.host_no_port = host_no_port

local hits = ctx.hits
local req_category = 0x00
Expand All @@ -1376,7 +1381,8 @@ function _M.new(routes)

elseif ctx.req_host then
for i = 1, #wildcard_hosts do
local from, _, err = re_find(host_with_port, wildcard_hosts[i].regex, "ajo")
local from, _, err = re_find(host_with_port, wildcard_hosts[i].regex,
"ajo")
if err then
log(ERR, "could not match wildcard host: ", err)
return
Expand Down
95 changes: 69 additions & 26 deletions spec/01-unit/08-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -210,31 +210,31 @@ describe("Router", function()
describe("split_port()", function()
it("splits port number", function()
for _, case in ipairs({
{ {''}, { '', '', false } },
{ {'localhost'}, { 'localhost', 'localhost', false } },
{ {'localhost:'}, { 'localhost', 'localhost', false } },
{ {'localhost:80'}, { 'localhost', 'localhost:80', true } },
{ {'localhost:23h'}, { 'localhost:23h', 'localhost:23h', false } },
{ {'localhost/24'}, { 'localhost/24', 'localhost/24', false } },
{ {'::1'}, { '::1', '::1', false } },
{ {'[::1]'}, { '::1', '[::1]', false } },
{ {'[::1]:'}, { '::1', '[::1]:', false } },
{ {'[::1]:80'}, { '::1', '[::1]:80', true } },
{ {'[::1]:80b'}, { '[::1]:80b', '[::1]:80b', false } },
{ {'[::1]/96'}, { '[::1]/96', '[::1]/96', false } },

{ {'', 88}, { '', ':88', false } },
{ {'localhost', 88}, { 'localhost', 'localhost:88', false } },
{ {'localhost:', 88}, { 'localhost', 'localhost:88', false } },
{ {'localhost:80', 88}, { 'localhost', 'localhost:80', true } },
{ {'localhost:23h', 88}, { 'localhost:23h', '[localhost:23h]:88', false } },
{ {'localhost/24', 88}, { 'localhost/24', 'localhost/24:88', false } },
{ {'::1', 88}, { '::1', '[::1]:88', false } },
{ {'[::1]', 88}, { '::1', '[::1]:88', false } },
{ {'[::1]:', 88}, { '::1', '[::1]:88', false } },
{ {'[::1]:80', 88}, { '::1', '[::1]:80', true } },
{ {'[::1]:80b', 88}, { '[::1]:80b', '[::1]:80b:88', false } },
{ {'[::1]/96', 88}, { '[::1]/96', '[::1]/96:88', false } },
{ { "" }, { "", "", false } },
{ { "localhost" }, { "localhost", "localhost", false } },
{ { "localhost:" }, { "localhost", "localhost", false } },
{ { "localhost:80" }, { "localhost", "localhost:80", true } },
{ { "localhost:23h" }, { "localhost:23h", "localhost:23h", false } },
{ { "localhost/24" }, { "localhost/24", "localhost/24", false } },
{ { "::1" }, { "::1", "::1", false } },
{ { "[::1]" }, { "::1", "[::1]", false } },
{ { "[::1]:" }, { "::1", "[::1]:", false } },
{ { "[::1]:80" }, { "::1", "[::1]:80", true } },
{ { "[::1]:80b" }, { "[::1]:80b", "[::1]:80b", false } },
{ { "[::1]/96" }, { "[::1]/96", "[::1]/96", false } },

{ { "", 88 }, { "", ":88", false } },
{ { "localhost", 88 }, { "localhost", "localhost:88", false } },
{ { "localhost:", 88 }, { "localhost", "localhost:88", false } },
{ { "localhost:80", 88 }, { "localhost", "localhost:80", true } },
{ { "localhost:23h", 88 }, { "localhost:23h", "[localhost:23h]:88", false } },
{ { "localhost/24", 88 }, { "localhost/24", "localhost/24:88", false } },
{ { "::1", 88 }, { "::1", "[::1]:88", false } },
{ { "[::1]", 88 }, { "::1", "[::1]:88", false } },
{ { "[::1]:", 88 }, { "::1", "[::1]:88", false } },
{ { "[::1]:80", 88 }, { "::1", "[::1]:80", true } },
{ { "[::1]:80b", 88 }, { "[::1]:80b", "[::1]:80b:88", false } },
{ { "[::1]/96", 88 }, { "[::1]/96", "[::1]/96:88", false } },
}) do
assert.same(case[2], { Router.split_port(unpack(case[1])) })
end
Expand Down Expand Up @@ -540,7 +540,7 @@ describe("Router", function()
assert.same(use_case[8].route.paths[1], match_t.matches.uri)
end)

describe("[IPv6 #literal host]", function()
describe("[IPv6 literal host]", function()
local use_case = {
-- 1: no port, with and without brackets, unique IPs
{
Expand Down Expand Up @@ -1156,6 +1156,49 @@ describe("Router", function()
assert.same(nil, match_t.matches.uri_captures)
end)

it("matches a [wildcard host + port] even if a [wildcard host] matched", function()
local use_case = {
{
service = service,
route = {
hosts = { "route.*" },
},
},
{
service = service,
route = {
hosts = { "route.*:123" },
},
},
{
service = service,
route = {
hosts = { "route.*:80" },
},
},
}

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

-- explicit port
local match_t = router.select("GET", "/", "route.org:123")
assert.truthy(match_t)
assert.equal(use_case[2].route, match_t.route)
assert.same("route.*:123", match_t.matches.host)
assert.same(nil, match_t.matches.method)
assert.same(nil, match_t.matches.uri)
assert.same(nil, match_t.matches.uri_captures)

-- implicit port
local match_t = router.select("GET", "/", "route.org")
assert.truthy(match_t)
assert.equal(use_case[3].route, match_t.route)
assert.same("route.*:80", match_t.matches.host)
assert.same(nil, match_t.matches.method)
assert.same(nil, match_t.matches.uri)
assert.same(nil, match_t.matches.uri_captures)
end)

it("matches [wildcard/plain + uri + method]", function()
finally(function()
table.remove(use_case)
Expand Down
4 changes: 2 additions & 2 deletions spec/02-integration/05-proxy/02-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ for _, strategy in helpers.each_strategy() do
hosts = {
"grpc1",
"grpc1:" .. helpers.get_proxy_port(false, true),
"grpc1:"..helpers.get_proxy_port(true, true),
"grpc1:" .. helpers.get_proxy_port(true, true),
},
service = service,
},
Expand All @@ -364,7 +364,7 @@ for _, strategy in helpers.each_strategy() do
hosts = {
"grpc2",
"grpc2:" .. helpers.get_proxy_port(false, true),
"grpc2:"..helpers.get_proxy_port(true, true),
"grpc2:" .. helpers.get_proxy_port(true, true),
},
service = service,
},
Expand Down

0 comments on commit 0b58992

Please sign in to comment.