Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(router/atc): uri replace didn't works well if contains the optional group #13024

Merged
merged 26 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ea43c90
fix(request-transformer): uri replace didn't works well if contains t…
git-hulk May 13, 2024
55ccf53
Add changelog
git-hulk May 13, 2024
8c08691
Update kong/router/atc.lua
git-hulk May 14, 2024
af1d2e0
Update spec/01-unit/08-router_spec.lua
git-hulk May 14, 2024
dba8768
Update spec/03-plugins/36-request-transformer/02-access_spec.lua
git-hulk May 14, 2024
5e1c0b5
Update changelog/unreleased/kong/fix-request-transformer-uri-replace.yml
git-hulk May 14, 2024
b59486f
Update changelog/unreleased/kong/fix-request-transformer-uri-replace.yml
git-hulk May 14, 2024
b5631e0
Update spec/03-plugins/36-request-transformer/02-access_spec.lua
git-hulk May 14, 2024
95ba613
Update kong/router/atc.lua
git-hulk May 14, 2024
7fb4088
Update spec/03-plugins/36-request-transformer/02-access_spec.lua
git-hulk May 14, 2024
25f2c73
Update spec/03-plugins/36-request-transformer/02-access_spec.lua
git-hulk May 14, 2024
203092a
Update changelog/unreleased/kong/fix-request-transformer-uri-replace.yml
git-hulk May 14, 2024
8688da3
Update spec/01-unit/08-router_spec.lua
git-hulk May 14, 2024
dd0a045
Update spec/03-plugins/36-request-transformer/02-access_spec.lua
git-hulk May 14, 2024
04c7e6d
Use assert.is_nil
git-hulk May 14, 2024
0dd1512
Update spec/01-unit/08-router_spec.lua
git-hulk May 14, 2024
5eb3ba9
Update changelog/unreleased/kong/fix-request-transformer-uri-replace.yml
git-hulk May 14, 2024
8d04f74
Apply suggestions from code review
StarlightIbuki May 14, 2024
d076fb7
Fix review comments
git-hulk May 14, 2024
e3f3fa3
Fix review comments
git-hulk May 14, 2024
63ca732
Fix review comments
git-hulk May 14, 2024
96e3b95
applying qiqi's suggestion
StarlightIbuki May 23, 2024
0db461e
Update kong/router/atc.lua
StarlightIbuki May 23, 2024
30641db
Update kong/router/atc.lua
git-hulk May 23, 2024
1baa8e0
Apply suggestions from code review
StarlightIbuki May 23, 2024
e50a7cd
Localized the next function in module
git-hulk May 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
message: |
Fixed an issue where the URI captures are unavailable when the first capture group is absent.
type: bugfix
scope: Core
16 changes: 15 additions & 1 deletion kong/router/atc.lua
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ local assert = assert
local setmetatable = setmetatable
local pairs = pairs
local ipairs = ipairs
local next = next
local max = math.max


Expand Down Expand Up @@ -344,6 +345,19 @@ local function set_upstream_uri(req_uri, match_t)
end


-- 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
local function has_capture(captures)
chronolaw marked this conversation as resolved.
Show resolved Hide resolved
if not captures then
return false
end
local next_i = next(captures)
StarlightIbuki marked this conversation as resolved.
Show resolved Hide resolved
chronolaw marked this conversation as resolved.
Show resolved Hide resolved
return next_i and next(captures, next_i) ~= nil
git-hulk marked this conversation as resolved.
Show resolved Hide resolved
end


function _M:matching(params)
local req_uri = params.uri
local req_host = params.host
Expand Down Expand Up @@ -387,7 +401,7 @@ function _M:matching(params)
service = service,
prefix = request_prefix,
matches = {
uri_captures = (captures and captures[1]) and captures or nil,
uri_captures = has_capture(captures) and captures or nil,
},
upstream_url_t = {
type = service_hostname_type,
Expand Down
20 changes: 20 additions & 0 deletions spec/01-unit/08-router_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3484,6 +3484,26 @@ for _, flavor in ipairs({ "traditional", "traditional_compatible", "expressions"
assert.is_nil(match_t.matches.headers)
end)

it("uri_captures works well with the optional capture group. Fix #13014", function()
git-hulk marked this conversation as resolved.
Show resolved Hide resolved
local use_case = {
{
service = service,
route = {
id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
paths = { [[~/(users/)?1984/(?<subpath>profile)$]] },
},
},
}

local router = assert(new_router(use_case))
local _ngx = mock_ngx("GET", "/1984/profile", { host = "domain.org" })
router._set_ngx(_ngx)
local match_t = router:exec()
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)
git-hulk marked this conversation as resolved.
Show resolved Hide resolved

it("returns uri_captures from a [uri regex]", function()
local use_case = {
{
Expand Down
42 changes: 42 additions & 0 deletions spec/03-plugins/36-request-transformer/02-access_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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/(?<subpath>htest)$" },
strip_path = false,
})

bp.plugins:insert {
route = { id = route1.id },
name = "request-transformer",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1114,6 +1130,32 @@ describe("Plugin: request-transformer(access) [#" .. strategy .. "]", function()
assert.is_truthy(string.find(json.data, "\"emptyarray\":[]", 1, true))
end)

it("replaces request uri with optional capture 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("replaces request 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 {
method = "GET",
Expand Down
Loading