From 0b58992d8132af798ab781fcb708c0c04384fe6a Mon Sep 17 00:00:00 2001 From: Thibault Charbonnier Date: Mon, 11 Nov 2019 12:12:54 -0800 Subject: [PATCH] [squashme] --- kong/router.lua | 48 ++++++---- spec/01-unit/08-router_spec.lua | 95 ++++++++++++++----- .../05-proxy/02-router_spec.lua | 4 +- 3 files changed, 98 insertions(+), 49 deletions(-) diff --git a/kong/router.lua b/kong/router.lua index 71ed51db6177..a8305c0ba922 100644 --- a/kong/router.lua +++ b/kong/router.lua @@ -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 @@ -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 @@ -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 @@ -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) @@ -146,7 +151,6 @@ do end - --[[ Hypothesis ---------- @@ -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 = {} @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index 7a828947b0ce..75424555ca79 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -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 @@ -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 { @@ -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) diff --git a/spec/02-integration/05-proxy/02-router_spec.lua b/spec/02-integration/05-proxy/02-router_spec.lua index 10b7df1a2eec..6ca95c59be73 100644 --- a/spec/02-integration/05-proxy/02-router_spec.lua +++ b/spec/02-integration/05-proxy/02-router_spec.lua @@ -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, }, @@ -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, },