Skip to content

Commit

Permalink
feat(router) implement groups captures from regex URIs
Browse files Browse the repository at this point in the history
We now perform efficient matching for URIs based on its property and the
regex being evaluated. We favorise running `ngx.re.find` when the more
expensive `ngx.re.match` is not necessary.

When a regex URI has capture groups, we return them from the router
`exec()` for further processing inside the core, which will be
responsible for passing them along to plugins.

This also has the advantage of being more performant on router cache
hits, since URI stripping is done only once, at matching type, and
cached in the LRU structure. The same way, groups matching is cached as
well.

Implements: #677 (partially)
  • Loading branch information
thibaultcha committed Jul 8, 2017
1 parent be88fb9 commit c9e3aad
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 28 deletions.
107 changes: 79 additions & 28 deletions kong/core/router.lua
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,24 @@ local function marshall_api(api)
-- regex URI
local strip_regex = uri .. [[/?(?P<stripped_uri>.*)]]

local from, _, err = re_find(uri, [[\(.*\)]], "jo")
if err then
log(ERR, "could not determine if regex URI has capture groups: ",
err)
end

local has_captures = from ~= nil

api_t.uris[uri] = {
has_captures = has_captures,
strip_regex = strip_regex,
}

insert(api_t.uris, {
value = uri,
regex = uri,
strip_regex = strip_regex,
value = uri,
regex = uri,
has_captures = has_captures,
strip_regex = strip_regex,
})
end
end
Expand Down Expand Up @@ -309,31 +319,77 @@ do
end,

[MATCH_RULES.URI] = function(api_t, ctx)
local uri = ctx.uri_prefix or ctx.uri_regex or ctx.uri
do
local uri = ctx.uri_prefix or ctx.uri_regex or ctx.uri
local regex_t = api_t.uris[uri]

if regex_t then
if api_t.strip_uri or regex_t.has_captures then
local m, err = re_match(ctx.uri, regex_t.strip_regex, "ajo")
if err then
log(ERR, "could not evaluate URI prefix/regex: ", err)
return
end

if api_t.uris[uri] then
if api_t.strip_uri then
ctx.strip_uri_regex = api_t.uris[uri].strip_regex
end
if m then
if m.stripped_uri then
ctx.stripped_uri = "/" .. m.stripped_uri
-- remove the stripped_uri group
m[#m] = nil
m.stripped_uri = nil
end

return true
if regex_t.has_captures then
m[0] = nil
ctx.uri_captures = m
end

return true
end
end

-- plain or prefix match from the index without strip_uri
return true
end
end

for i = 1, #api_t.uris do
local regex_t = api_t.uris[i]

local from, _, err = re_find(ctx.uri, regex_t.regex, "ajo")
if err then
log(ERR, "could not evaluate URI prefix/regex: ", err)
return
end
if api_t.strip_uri or regex_t.has_captures then
local m, err = re_match(ctx.uri, regex_t.strip_regex, "ajo")
if err then
log(ERR, "could not evaluate URI prefix/regex: ", err)
return
end

if from then
if api_t.strip_uri then
ctx.strip_uri_regex = regex_t.strip_regex
if m then
if m.stripped_uri then
ctx.stripped_uri = "/" .. m.stripped_uri
-- remove the stripped_uri group
m[#m] = nil
m.stripped_uri = nil
end

if regex_t.has_captures then
m[0] = nil
ctx.uri_captures = m
end

return true
end

return true
else
-- prefix match without strip_uri
local from, _, err = re_find(ctx.uri, regex_t.regex, "ajo")
if err then
log(ERR, "could not evaluate URI prefix/regex: ", err)
return
end

if from then
return true
end
end
end
end,
Expand Down Expand Up @@ -659,7 +715,8 @@ function _M.new(apis)
cache:set(cache_key, {
api_t = matched_api,
ctx = {
strip_uri_regex = ctx.strip_uri_regex,
stripped_uri = ctx.stripped_uri,
uri_captures = ctx.uri_captures,
}
})

Expand Down Expand Up @@ -709,14 +766,8 @@ function _M.new(apis)

local uri_root = request_uri == "/"

if not uri_root and ctx.strip_uri_regex then
local m, err = re_match(uri, ctx.strip_uri_regex, "ajo")
if not m then
log(ERR, "could not strip URI: ", err)
return
end

uri = "/" .. m.stripped_uri
if not uri_root and api_t.strip_uri and ctx.stripped_uri then
uri = ctx.stripped_uri
end


Expand Down Expand Up @@ -757,7 +808,7 @@ function _M.new(apis)
ngx.header["Kong-Api-Name"] = api_t.api.name
end

return api_t.api, api_t.upstream, host_header, uri
return api_t.api, api_t.upstream, host_header, uri, ctx.uri_captures
end


Expand Down
79 changes: 79 additions & 0 deletions spec/01-unit/11-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,85 @@ describe("Router", function()
local _, _, _, uri = router.exec(_ngx)
assert.equal("/hello/world", uri)
end)

it("returns groups captures from a [uri regex]", function()
local use_case = {
{
name = "api-1",
uris = { [[/users/(?P<user_id>\d+)/profile/?(?P<scope>[a-z]*)]] },
},
}

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

local _ngx = mock_ngx("GET", "/users/1984/profile", {})
local _, _, _, _, uri_captures = router.exec(_ngx)
assert.equal("1984", uri_captures[1])
assert.equal("1984", uri_captures.user_id)
assert.equal("", uri_captures[2])
assert.equal("", uri_captures.scope)
-- no full match
assert.is_nil(uri_captures[0])
-- no stripped_uri capture
assert.is_nil(uri_captures.stripped_uri)
assert.equal(2, #uri_captures)

-- again, this time from the LRU cache
local _, _, _, _, uri_captures = router.exec(_ngx)
assert.equal("1984", uri_captures[1])
assert.equal("1984", uri_captures.user_id)
assert.equal("", uri_captures[2])
assert.equal("", uri_captures.scope)
-- no full match
assert.is_nil(uri_captures[0])
-- no stripped_uri capture
assert.is_nil(uri_captures.stripped_uri)
assert.equal(2, #uri_captures)

local _ngx = mock_ngx("GET", "/users/1984/profile/email", {})
local _, _, _, _, uri_captures = router.exec(_ngx)
assert.equal("1984", uri_captures[1])
assert.equal("1984", uri_captures.user_id)
assert.equal("email", uri_captures[2])
assert.equal("email", uri_captures.scope)
-- no full match
assert.is_nil(uri_captures[0])
-- no stripped_uri capture
assert.is_nil(uri_captures.stripped_uri)
assert.equal(2, #uri_captures)
end)

it("returns no group capture from a [uri prefix] match", function()
local use_case = {
{
name = "api-1",
uris = { "/hello" },
strip_uri = true,
},
}

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

local _ngx = mock_ngx("GET", "/hello/world", {})
local _, _, _, uri, uri_captures = router.exec(_ngx)
assert.equal("/world", uri)
assert.is_nil(uri_captures)
end)

it("returns no group capture from a [uri regex] match without groups", function()
local use_case = {
{
name = "api-1",
uris = { [[/users/\d+/profile]] },
},
}

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

local _ngx = mock_ngx("GET", "/users/1984/profile", {})
local _, _, _, _, uri_captures = router.exec(_ngx)
assert.is_nil(uri_captures)
end)
end)

describe("preserve Host header", function()
Expand Down

0 comments on commit c9e3aad

Please sign in to comment.