fix(router/atc): URI captures are unavailable when the first capture …

…group is absent (#13024)

Fix #13014, https://konghq.atlassian.net/browse/KAG-4474

Co-authored-by: Xumin <100666470+StarlightIbuki@users.noreply.github.com>
Co-authored-by: Chrono <chronolaw@gmail.com>
Co-authored-by: Mikołaj Nowak <mikolaj.nowak93@gmail.com>
Co-authored-by: xumin <xumin.zhou@konghq.com>

changelog/unreleased/kong/fix-request-transformer-uri-replace.yml

@@ -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

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
 
 
@@ -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)
+  if not captures then
+    return false
+  end
+  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
@@ -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,

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_captures 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/(?<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)
+
       it("returns uri_captures from a [uri regex]", function()
         local use_case = {
           {

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/(?<subpath>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",
@@ -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",