From ea43c9088cae6dc4af69f55aedaf379dd8001424 Mon Sep 17 00:00:00 2001 From: git-hulk Date: Mon, 13 May 2024 20:51:05 +0800 Subject: [PATCH 01/26] fix(request-transformer): uri replace didn't works well if contains the optional group --- kong/router/atc.lua | 7 +++- spec/01-unit/08-router_spec.lua | 20 ++++++++++ .../36-request-transformer/02-access_spec.lua | 40 +++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index c41af0da0bde..f7af95290aa5 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -2,6 +2,7 @@ local _M = {} local _MT = { __index = _M, } +local pl_tablex = require("pl.tablex") local lrucache = require("resty.lrucache") local tb_new = require("table.new") local utils = require("kong.router.utils") @@ -382,12 +383,16 @@ function _M:matching(params) local request_prefix = matched_route.strip_path and matched_path or nil + local uri_captures = nil + if matched_path and captures and pl_tablex.size(captures) > 1 then + uri_captures = captures + end return { route = matched_route, service = service, prefix = request_prefix, matches = { - uri_captures = (captures and captures[1]) and captures or nil, + uri_captures = uri_captures }, upstream_url_t = { type = service_hostname_type, diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index 046cb25bd8b0..2d3ff6a27fa2 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -3484,6 +3484,26 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" assert.is_nil(match_t.matches.headers) end) + it("uri_captreus works well with the optional capture group. Fix #13014", function() + local use_case = { + { + service = service, + route = { + id = "e8fb37f1-102d-461e-9c51-6608a6bb8101", + paths = { [[~/(users/)?1984/(?profile)$]] }, + }, + }, + } + + local router = assert(new_router(use_case)) + local _ngx = mock_ngx("GET", "/users/1984/profile", + { host = "domain.org" }) + router._set_ngx(_ngx) + local match_t = router:exec() + assert.equal("users/", match_t.matches.uri_captures[1]) + assert.equal("profile", match_t.matches.uri_captures.subpath) + assert.same(nil, match_t.matches.uri_captures.scope) + end) it("returns uri_captures from a [uri regex]", function() local use_case = { { diff --git a/spec/03-plugins/36-request-transformer/02-access_spec.lua b/spec/03-plugins/36-request-transformer/02-access_spec.lua index bf3de4192826..9498d5d639ef 100644 --- a/spec/03-plugins/36-request-transformer/02-access_spec.lua +++ b/spec/03-plugins/36-request-transformer/02-access_spec.lua @@ -131,6 +131,12 @@ describe("Plugin: request-transformer(access) [#" .. strategy .. "]", function() hosts = { "test28.test" } }) + local route29 = bp.routes:insert({ + hosts = { "test29.test" }, + paths = { "~/(gw/)?api/(?htest)$" }, + strip_path = false + }) + bp.plugins:insert { route = { id = route1.id }, name = "request-transformer", @@ -471,6 +477,16 @@ describe("Plugin: request-transformer(access) [#" .. strategy .. "]", function() } } + bp.plugins:insert { + route = { id = route29.id }, + name = "request-transformer", + config = { + replace = { + uri = "/api/v2/$(uri_captures[\"subpath\"])", + } + } + } + assert(helpers.start_kong({ database = strategy, plugins = "bundled, request-transformer", @@ -1113,6 +1129,30 @@ describe("Plugin: request-transformer(access) [#" .. strategy .. "]", function() local json = assert.request(r).has.jsonbody() assert.is_truthy(string.find(json.data, "\"emptyarray\":[]", 1, true)) end) + it("replace request uri with optional capature prefix", function() + local r = assert(client:send { + method = "GET", + path = "/api/htest", + headers = { + host = "test29.test" + } + }) + assert.response(r).has.status(404) + local body = assert(assert.response(r).has.jsonbody()) + assert.equals("/api/v2/htest", body.vars.request_uri) + end) + it("replace reqest uri with the capature prefix", function() + local r = assert(client:send { + method = "GET", + path = "/gw/api/htest", + headers = { + host = "test29.test" + } + }) + assert.response(r).has.status(404) + local body = assert(assert.response(r).has.jsonbody()) + assert.equals("/api/v2/htest", body.vars.request_uri) + end) pending("escape UTF-8 characters when replacing upstream path - enable after Kong 2.4", function() local r = assert(client:send { From 55ccf53362af496f8104094015d0a6ce0ae0de2d Mon Sep 17 00:00:00 2001 From: git-hulk Date: Mon, 13 May 2024 20:59:26 +0800 Subject: [PATCH 02/26] Add changelog --- .../unreleased/kong/fix-request-transformer-uri-replace.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/unreleased/kong/fix-request-transformer-uri-replace.yml diff --git a/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml new file mode 100644 index 000000000000..f0b142cefe2d --- /dev/null +++ b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml @@ -0,0 +1,4 @@ +message: | + **reqeust-transformer**: Fixed the uri replace didn't works well if contains the optional group. +type: bugfix +scope: Plugin From 8c0869137a422b294cb6b601d8e390b0da7965f7 Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:34:34 +0800 Subject: [PATCH 03/26] Update kong/router/atc.lua Co-authored-by: Xumin <100666470+StarlightIbuki@users.noreply.github.com> --- kong/router/atc.lua | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index f7af95290aa5..2ed641f850cb 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -383,10 +383,11 @@ function _M:matching(params) local request_prefix = matched_route.strip_path and matched_path or nil - local uri_captures = nil - if matched_path and captures and pl_tablex.size(captures) > 1 then - uri_captures = captures - end + local uri_captures = nil + if matched_path and captures and pl_tablex.size(captures) > 1 then + uri_captures = captures + end + return { route = matched_route, service = service, From af1d2e09db6df67b5d5c9c60de136e8d75b9351b Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:34:42 +0800 Subject: [PATCH 04/26] Update spec/01-unit/08-router_spec.lua Co-authored-by: Xumin <100666470+StarlightIbuki@users.noreply.github.com> --- spec/01-unit/08-router_spec.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index 2d3ff6a27fa2..9f4a8739d465 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -3504,6 +3504,7 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" assert.equal("profile", match_t.matches.uri_captures.subpath) assert.same(nil, match_t.matches.uri_captures.scope) end) + it("returns uri_captures from a [uri regex]", function() local use_case = { { From dba8768fbbe23b00abde46496c0c701a3f5db844 Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:34:55 +0800 Subject: [PATCH 05/26] Update spec/03-plugins/36-request-transformer/02-access_spec.lua Co-authored-by: Xumin <100666470+StarlightIbuki@users.noreply.github.com> --- spec/03-plugins/36-request-transformer/02-access_spec.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/03-plugins/36-request-transformer/02-access_spec.lua b/spec/03-plugins/36-request-transformer/02-access_spec.lua index 9498d5d639ef..421a81201140 100644 --- a/spec/03-plugins/36-request-transformer/02-access_spec.lua +++ b/spec/03-plugins/36-request-transformer/02-access_spec.lua @@ -1129,7 +1129,8 @@ describe("Plugin: request-transformer(access) [#" .. strategy .. "]", function() local json = assert.request(r).has.jsonbody() assert.is_truthy(string.find(json.data, "\"emptyarray\":[]", 1, true)) end) - it("replace request uri with optional capature prefix", function() + + it("replace request uri with optional capture prefix", function() local r = assert(client:send { method = "GET", path = "/api/htest", From 5e1c0b5c20348ae2b88e965be5ffa4644916f9ed Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:35:04 +0800 Subject: [PATCH 06/26] Update changelog/unreleased/kong/fix-request-transformer-uri-replace.yml Co-authored-by: Xumin <100666470+StarlightIbuki@users.noreply.github.com> --- .../unreleased/kong/fix-request-transformer-uri-replace.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml index f0b142cefe2d..1583f7e1c785 100644 --- a/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml +++ b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml @@ -1,4 +1,4 @@ message: | - **reqeust-transformer**: Fixed the uri replace didn't works well if contains the optional group. + **reqeust-transformer**: Fixed an issue where the URI captures are unavailable when an optional group is absent. type: bugfix scope: Plugin From b59486f1a8f9b6ebcfd2a5eecb0af635e2d011fc Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:35:10 +0800 Subject: [PATCH 07/26] Update changelog/unreleased/kong/fix-request-transformer-uri-replace.yml Co-authored-by: Xumin <100666470+StarlightIbuki@users.noreply.github.com> --- .../unreleased/kong/fix-request-transformer-uri-replace.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml index 1583f7e1c785..c09f4a772a1f 100644 --- a/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml +++ b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml @@ -1,4 +1,4 @@ message: | **reqeust-transformer**: Fixed an issue where the URI captures are unavailable when an optional group is absent. type: bugfix -scope: Plugin +scope: Core From b5631e0df02de68b9f0c159b311475269dcda48f Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:35:35 +0800 Subject: [PATCH 08/26] Update spec/03-plugins/36-request-transformer/02-access_spec.lua Co-authored-by: Xumin <100666470+StarlightIbuki@users.noreply.github.com> --- spec/03-plugins/36-request-transformer/02-access_spec.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/03-plugins/36-request-transformer/02-access_spec.lua b/spec/03-plugins/36-request-transformer/02-access_spec.lua index 421a81201140..d96e24854faf 100644 --- a/spec/03-plugins/36-request-transformer/02-access_spec.lua +++ b/spec/03-plugins/36-request-transformer/02-access_spec.lua @@ -1142,7 +1142,8 @@ describe("Plugin: request-transformer(access) [#" .. strategy .. "]", function() local body = assert(assert.response(r).has.jsonbody()) assert.equals("/api/v2/htest", body.vars.request_uri) end) - it("replace reqest uri with the capature prefix", function() + + it("replace request uri with the capature prefix", function() local r = assert(client:send { method = "GET", path = "/gw/api/htest", From 95ba6137c96966d6740b749732b5f022394804e1 Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:51:47 +0800 Subject: [PATCH 09/26] Update kong/router/atc.lua Co-authored-by: Chrono --- kong/router/atc.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 2ed641f850cb..4ce58ed755e9 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -393,7 +393,7 @@ function _M:matching(params) service = service, prefix = request_prefix, matches = { - uri_captures = uri_captures + uri_captures = uri_captures, }, upstream_url_t = { type = service_hostname_type, From 7fb4088a9fbe108cfd0e70597e43d4e9a3409bce Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:51:54 +0800 Subject: [PATCH 10/26] Update spec/03-plugins/36-request-transformer/02-access_spec.lua Co-authored-by: Chrono --- spec/03-plugins/36-request-transformer/02-access_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/03-plugins/36-request-transformer/02-access_spec.lua b/spec/03-plugins/36-request-transformer/02-access_spec.lua index d96e24854faf..6e89098f406f 100644 --- a/spec/03-plugins/36-request-transformer/02-access_spec.lua +++ b/spec/03-plugins/36-request-transformer/02-access_spec.lua @@ -1143,7 +1143,7 @@ describe("Plugin: request-transformer(access) [#" .. strategy .. "]", function() assert.equals("/api/v2/htest", body.vars.request_uri) end) - it("replace request uri with the capature prefix", function() + it("replaces request uri with the capature prefix", function() local r = assert(client:send { method = "GET", path = "/gw/api/htest", From 25f2c73b82fe3611fefc5735f065dad8d891adfb Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:52:01 +0800 Subject: [PATCH 11/26] Update spec/03-plugins/36-request-transformer/02-access_spec.lua Co-authored-by: Chrono --- spec/03-plugins/36-request-transformer/02-access_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/03-plugins/36-request-transformer/02-access_spec.lua b/spec/03-plugins/36-request-transformer/02-access_spec.lua index 6e89098f406f..8bcbbf6c1332 100644 --- a/spec/03-plugins/36-request-transformer/02-access_spec.lua +++ b/spec/03-plugins/36-request-transformer/02-access_spec.lua @@ -1130,7 +1130,7 @@ describe("Plugin: request-transformer(access) [#" .. strategy .. "]", function() assert.is_truthy(string.find(json.data, "\"emptyarray\":[]", 1, true)) end) - it("replace request uri with optional capture prefix", function() + it("replaces request uri with optional capture prefix", function() local r = assert(client:send { method = "GET", path = "/api/htest", From 203092aaba454b7048784eb166023ea7bc33bcd5 Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 15:52:11 +0800 Subject: [PATCH 12/26] Update changelog/unreleased/kong/fix-request-transformer-uri-replace.yml Co-authored-by: Chrono --- .../unreleased/kong/fix-request-transformer-uri-replace.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml index c09f4a772a1f..9e8b4feaedc1 100644 --- a/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml +++ b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml @@ -1,4 +1,4 @@ message: | - **reqeust-transformer**: Fixed an issue where the URI captures are unavailable when an optional group is absent. + Fixed an issue where the URI captures are unavailable when an optional group is absent. type: bugfix scope: Core From 8688da307cee5895fb4947ce1d578cc8b39a8a11 Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 16:03:28 +0800 Subject: [PATCH 13/26] Update spec/01-unit/08-router_spec.lua Co-authored-by: Chrono --- spec/01-unit/08-router_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index 9f4a8739d465..279be6f157b3 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -3497,7 +3497,7 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" local router = assert(new_router(use_case)) local _ngx = mock_ngx("GET", "/users/1984/profile", - { host = "domain.org" }) + { host = "domain.org" }) router._set_ngx(_ngx) local match_t = router:exec() assert.equal("users/", match_t.matches.uri_captures[1]) From dd0a04583c845d4cf209adbbe238f516577b9cae Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 16:03:53 +0800 Subject: [PATCH 14/26] Update spec/03-plugins/36-request-transformer/02-access_spec.lua Co-authored-by: Chrono --- spec/03-plugins/36-request-transformer/02-access_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/03-plugins/36-request-transformer/02-access_spec.lua b/spec/03-plugins/36-request-transformer/02-access_spec.lua index 8bcbbf6c1332..aeffa72c7f45 100644 --- a/spec/03-plugins/36-request-transformer/02-access_spec.lua +++ b/spec/03-plugins/36-request-transformer/02-access_spec.lua @@ -134,7 +134,7 @@ describe("Plugin: request-transformer(access) [#" .. strategy .. "]", function() local route29 = bp.routes:insert({ hosts = { "test29.test" }, paths = { "~/(gw/)?api/(?htest)$" }, - strip_path = false + strip_path = false, }) bp.plugins:insert { From 04c7e6dec6e7d05d71e61c3108b72487af1ec483 Mon Sep 17 00:00:00 2001 From: git-hulk Date: Tue, 14 May 2024 16:06:31 +0800 Subject: [PATCH 15/26] Use assert.is_nil --- spec/01-unit/08-router_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index 279be6f157b3..f623d30e2a86 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -3502,7 +3502,7 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" local match_t = router:exec() assert.equal("users/", match_t.matches.uri_captures[1]) assert.equal("profile", match_t.matches.uri_captures.subpath) - assert.same(nil, match_t.matches.uri_captures.scope) + assert.is_nil(match_t.matches.uri_captures.scope) end) it("returns uri_captures from a [uri regex]", function() From 0dd1512b46b1385f69ff1ee83f192bc33948bbda Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 16:19:16 +0800 Subject: [PATCH 16/26] Update spec/01-unit/08-router_spec.lua MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: MikoĊ‚aj Nowak --- spec/01-unit/08-router_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index f623d30e2a86..bddeed33c79e 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -3484,7 +3484,7 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" assert.is_nil(match_t.matches.headers) end) - it("uri_captreus works well with the optional capture group. Fix #13014", function() + it("uri_captures works well with the optional capture group. Fix #13014", function() local use_case = { { service = service, From 5eb3ba9e7ec50e8af8dd8d332c37cd3510976d14 Mon Sep 17 00:00:00 2001 From: hulk Date: Tue, 14 May 2024 17:03:45 +0800 Subject: [PATCH 17/26] Update changelog/unreleased/kong/fix-request-transformer-uri-replace.yml Co-authored-by: Xumin <100666470+StarlightIbuki@users.noreply.github.com> --- .../unreleased/kong/fix-request-transformer-uri-replace.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml index 9e8b4feaedc1..02c55a15f70b 100644 --- a/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml +++ b/changelog/unreleased/kong/fix-request-transformer-uri-replace.yml @@ -1,4 +1,4 @@ message: | - Fixed an issue where the URI captures are unavailable when an optional group is absent. + Fixed an issue where the URI captures are unavailable when the first capture group is absent. type: bugfix scope: Core From 8d04f748a127dc4c58821622a291c979191d0239 Mon Sep 17 00:00:00 2001 From: Xumin <100666470+StarlightIbuki@users.noreply.github.com> Date: Tue, 14 May 2024 17:11:35 +0800 Subject: [PATCH 18/26] Apply suggestions from code review --- kong/router/atc.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 4ce58ed755e9..3833e00f6d12 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -2,7 +2,7 @@ local _M = {} local _MT = { __index = _M, } -local pl_tablex = require("pl.tablex") +local isempty= require("table.isempty") local lrucache = require("resty.lrucache") local tb_new = require("table.new") local utils = require("kong.router.utils") @@ -384,7 +384,7 @@ function _M:matching(params) local request_prefix = matched_route.strip_path and matched_path or nil local uri_captures = nil - if matched_path and captures and pl_tablex.size(captures) > 1 then + if matched_path and captures and not isempty(captures) then uri_captures = captures end From d076fb73f8b4c3392458a609b1c76ce2b601c788 Mon Sep 17 00:00:00 2001 From: git-hulk Date: Tue, 14 May 2024 17:44:53 +0800 Subject: [PATCH 19/26] Fix review comments --- kong/router/atc.lua | 4 ++-- spec/01-unit/08-router_spec.lua | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 3833e00f6d12..b010bf05a868 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -2,7 +2,7 @@ local _M = {} local _MT = { __index = _M, } -local isempty= require("table.isempty") +local tb_nkeys = require("table.nkeys") local lrucache = require("resty.lrucache") local tb_new = require("table.new") local utils = require("kong.router.utils") @@ -384,7 +384,7 @@ function _M:matching(params) local request_prefix = matched_route.strip_path and matched_path or nil local uri_captures = nil - if matched_path and captures and not isempty(captures) then + if matched_path and captures and tb_nkeys(captures) > 1 then uri_captures = captures end diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index bddeed33c79e..c9832dade3b0 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -3496,11 +3496,13 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" } local router = assert(new_router(use_case)) - local _ngx = mock_ngx("GET", "/users/1984/profile", + local _ngx = mock_ngx("GET", "/1984/profile", { host = "domain.org" }) router._set_ngx(_ngx) local match_t = router:exec() - assert.equal("users/", match_t.matches.uri_captures[1]) + if flavor ~= "traditional" then + assert.is_nil(match_t.matches.uri_captures[1]) + end assert.equal("profile", match_t.matches.uri_captures.subpath) assert.is_nil(match_t.matches.uri_captures.scope) end) From e3f3fa37454460b1a8a7354c48fce959db83bded Mon Sep 17 00:00:00 2001 From: git-hulk Date: Tue, 14 May 2024 18:39:01 +0800 Subject: [PATCH 20/26] Fix review comments --- kong/router/atc.lua | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index b010bf05a868..63faf2c136f2 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -2,9 +2,9 @@ local _M = {} local _MT = { __index = _M, } -local tb_nkeys = require("table.nkeys") local lrucache = require("resty.lrucache") local tb_new = require("table.new") +local tb_nkeys = require("table.nkeys") local utils = require("kong.router.utils") local transform = require("kong.router.transform") local rat = require("kong.tools.request_aware_table") @@ -384,7 +384,9 @@ function _M:matching(params) local request_prefix = matched_route.strip_path and matched_path or nil local uri_captures = nil - if matched_path and captures and tb_nkeys(captures) > 1 then + -- The whole matched uri will be put in captures[0], so we need to check + -- the number of table keys >= 2 to determine if there has any uri captured. + if matched_path and captures and tb_nkeys(captures) >= 2 then uri_captures = captures end From 63ca7326c0fb476bf750ee604a12ba5fc76363ca Mon Sep 17 00:00:00 2001 From: git-hulk Date: Tue, 14 May 2024 21:26:11 +0800 Subject: [PATCH 21/26] Fix review comments --- kong/router/atc.lua | 4 ++-- spec/01-unit/08-router_spec.lua | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 63faf2c136f2..18e541d451ee 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -385,8 +385,8 @@ function _M:matching(params) local uri_captures = nil -- The whole matched uri will be put in captures[0], so we need to check - -- the number of table keys >= 2 to determine if there has any uri captured. - if matched_path and captures and tb_nkeys(captures) >= 2 then + -- the number of table keys >= 2 to determine if there is any uri captured. + if captures and tb_nkeys(captures) >= 2 then uri_captures = captures end diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index c9832dade3b0..025120392ecf 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -3496,13 +3496,10 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions" } local router = assert(new_router(use_case)) - local _ngx = mock_ngx("GET", "/1984/profile", - { host = "domain.org" }) + local _ngx = mock_ngx("GET", "/1984/profile", { host = "domain.org" }) router._set_ngx(_ngx) local match_t = router:exec() - if flavor ~= "traditional" then - assert.is_nil(match_t.matches.uri_captures[1]) - end + assert.falsy(match_t.matches.uri_captures[1]) assert.equal("profile", match_t.matches.uri_captures.subpath) assert.is_nil(match_t.matches.uri_captures.scope) end) From 96e3b95c08c7b0920fe10aecb11248e24084bb17 Mon Sep 17 00:00:00 2001 From: xumin Date: Thu, 23 May 2024 10:59:57 +0800 Subject: [PATCH 22/26] applying qiqi's suggestion --- kong/router/atc.lua | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 18e541d451ee..27729220693e 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -4,7 +4,6 @@ local _MT = { __index = _M, } local lrucache = require("resty.lrucache") local tb_new = require("table.new") -local tb_nkeys = require("table.nkeys") local utils = require("kong.router.utils") local transform = require("kong.router.transform") local rat = require("kong.tools.request_aware_table") @@ -345,6 +344,16 @@ local function set_upstream_uri(req_uri, match_t) end +-- captures has the form { [0] = full_path, [1] = capture1, [2] = capture2, ... } +-- and captures[0] will be the full matched path +-- this function tests if there are captures other than the full path +-- by checking if there are 2 or more than 2 keys +local function has_capture(captures) + local next_i = next(captures) + return next_i and next(captures, next_i) ~= nil +end + + function _M:matching(params) local req_uri = params.uri local req_host = params.host @@ -383,19 +392,12 @@ function _M:matching(params) local request_prefix = matched_route.strip_path and matched_path or nil - local uri_captures = nil - -- The whole matched uri will be put in captures[0], so we need to check - -- the number of table keys >= 2 to determine if there is any uri captured. - if captures and tb_nkeys(captures) >= 2 then - uri_captures = captures - end - return { route = matched_route, service = service, prefix = request_prefix, matches = { - uri_captures = uri_captures, + uri_captures = has_capture(captures) and captures or nil, }, upstream_url_t = { type = service_hostname_type, From 0db461ee0028b2a95680929e0e109edb86855b76 Mon Sep 17 00:00:00 2001 From: Xumin <100666470+StarlightIbuki@users.noreply.github.com> Date: Thu, 23 May 2024 14:36:32 +0800 Subject: [PATCH 23/26] Update kong/router/atc.lua Co-authored-by: hulk --- kong/router/atc.lua | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 27729220693e..44acd4280f2a 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -349,6 +349,9 @@ end -- this function tests if there are captures other than the full path -- by checking if there are 2 or more than 2 keys local function has_capture(captures) + if not captures then + return nil + end local next_i = next(captures) return next_i and next(captures, next_i) ~= nil end From 30641dbbe89bf0f500e2a72c382b93470d61f791 Mon Sep 17 00:00:00 2001 From: hulk Date: Thu, 23 May 2024 14:47:33 +0800 Subject: [PATCH 24/26] Update kong/router/atc.lua --- kong/router/atc.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 44acd4280f2a..fee52e0e1f91 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -350,7 +350,7 @@ end -- by checking if there are 2 or more than 2 keys local function has_capture(captures) if not captures then - return nil + return false end local next_i = next(captures) return next_i and next(captures, next_i) ~= nil From 1baa8e0ed23ba279db358d40cac90085ee6997f4 Mon Sep 17 00:00:00 2001 From: Xumin <100666470+StarlightIbuki@users.noreply.github.com> Date: Thu, 23 May 2024 14:49:01 +0800 Subject: [PATCH 25/26] Apply suggestions from code review --- kong/router/atc.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index fee52e0e1f91..d05f20f24e98 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -344,7 +344,7 @@ local function set_upstream_uri(req_uri, match_t) end --- captures has the form { [0] = full_path, [1] = capture1, [2] = capture2, ... } +-- captures has the form { [0] = full_path, [1] = capture1, [2] = capture2, ..., ["named1"] = named1, ... } -- and captures[0] will be the full matched path -- this function tests if there are captures other than the full path -- by checking if there are 2 or more than 2 keys From e50a7cd9be853e1c33fae167502c2efddc9276fc Mon Sep 17 00:00:00 2001 From: git-hulk Date: Mon, 27 May 2024 14:15:09 +0800 Subject: [PATCH 26/26] Localized the next function in module --- kong/router/atc.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index d05f20f24e98..31aaf6c5a4ed 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -15,6 +15,7 @@ local assert = assert local setmetatable = setmetatable local pairs = pairs local ipairs = ipairs +local next = next local max = math.max