fix(router/atc): always re-calculate match_t.upstream_uri even in c…

…ase of cache hits (#12307)

`upstream_uri` is dependent on request path, and should not be automatically cached inside the route cache due to potential for path matching not being configured inside routes.

KAG-3505

kong/router/atc.lua

@@ -402,6 +402,19 @@ local add_debug_headers    = utils.add_debug_headers
 local get_upstream_uri_v0  = utils.get_upstream_uri_v0
 
 
+local function set_upstream_uri(req_uri, match_t)
+  local matched_route = match_t.route
+
+  local request_prefix = match_t.prefix or "/"
+  local request_postfix = sanitize_uri_postfix(req_uri:sub(#request_prefix + 1))
+
+  local upstream_base = match_t.upstream_url_t.path or "/"
+
+  match_t.upstream_uri = get_upstream_uri_v0(matched_route, request_postfix,
+                                             req_uri, upstream_base)
+end
+
+
 function _M:matching(params)
   local req_uri = params.uri
   local req_host = params.host
@@ -439,12 +452,6 @@ function _M:matching(params)
         service_hostname_type, service_path = get_service_info(service)
 
   local request_prefix = matched_route.strip_path and matched_path or nil
-  local request_postfix = request_prefix and req_uri:sub(#matched_path + 1) or req_uri:sub(2, -1)
-  request_postfix = sanitize_uri_postfix(request_postfix) or ""
-  local upstream_base = service_path or "/"
-
-  local upstream_uri = get_upstream_uri_v0(matched_route, request_postfix, req_uri,
-                                           upstream_base)
 
   return {
     route           = matched_route,
@@ -457,9 +464,9 @@ function _M:matching(params)
       type = service_hostname_type,
       host = service_host,
       port = service_port,
+      path = service_path,
     },
     upstream_scheme = service_protocol,
-    upstream_uri    = upstream_uri,
     upstream_host   = matched_route.preserve_host and req_host or nil,
   }
 end
@@ -546,6 +553,9 @@ function _M:exec(ctx)
 
   -- found a match
 
+  -- update upstream_uri in cache result
+  set_upstream_uri(req_uri, match_t)
+
   -- debug HTTP request header logic
   add_debug_headers(var, header, match_t)
 

spec/01-unit/08-router_spec.lua

@@ -4943,10 +4943,8 @@ do
   describe("Router (flavor = " .. flavor .. ")", function()
     reload_router(flavor)
 
-    local use_case, router
-
-    lazy_setup(function()
-      use_case = {
+    it("[cache hit should be case sensitive]", function()
+      local use_case = {
         {
           service = service,
           route   = {
@@ -4960,10 +4958,8 @@ do
           },
         },
       }
-    end)
 
-    it("[cache hit should be case sensitive]", function()
-      router = assert(new_router(use_case))
+      local router = assert(new_router(use_case))
 
       local ctx = {}
       local _ngx = mock_ngx("GET", "/foo", { test1 = "QUOTE", })
@@ -4993,6 +4989,46 @@ do
       -- cache miss, case sensitive
       assert.falsy(ctx.route_match_cached)
     end)
+
+    it("[cache hit should have correct match_t.upstream_uri]", function()
+      local host = "example.com"
+      local use_case = {
+        {
+          service = service,
+          route   = {
+            id = "e8fb37f1-102d-461e-9c51-6608a6bb8101",
+            hosts = { host },
+            preserve_host = true,
+          },
+        },
+      }
+
+      local router = assert(new_router(use_case))
+
+      local ctx = {}
+      local _ngx = mock_ngx("GET", "/foo", { host = host, })
+      router._set_ngx(_ngx)
+
+      -- first match
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+      assert.falsy(ctx.route_match_cached)
+      assert.equal(host, match_t.upstream_host)
+      assert.same("/foo", match_t.upstream_uri)
+
+      local ctx = {}
+      local _ngx = mock_ngx("GET", "/bar", { host = host, })
+      router._set_ngx(_ngx)
+
+      -- cache hit
+      local match_t = router:exec(ctx)
+      assert.truthy(match_t)
+      assert.same(use_case[1].route, match_t.route)
+      assert.same(ctx.route_match_cached, "pos")
+      assert.equal(host, match_t.upstream_host)
+      assert.same("/bar", match_t.upstream_uri)
+    end)
   end)
 end   -- local flavor = "traditional_compatible"
 

spec/02-integration/01-helpers/01-helpers_spec.lua

@@ -26,7 +26,6 @@ for _, strategy in helpers.each_strategy() do
       bp.routes:insert {
         hosts     = { "mock_upstream" },
         protocols = { "http" },
-        paths = { "/" },
         service   = service
       }
 

spec/02-integration/05-proxy/02-router_spec.lua

@@ -881,7 +881,6 @@ for _, strategy in helpers.each_strategy() do
         routes = insert_routes(bp, {
           {
             hosts = { "mock_upstream" },
-            paths = { "/" },
           },
         })
       end)
@@ -1302,7 +1301,6 @@ for _, strategy in helpers.each_strategy() do
         routes = insert_routes(bp, {
           {
             protocols = { "https" },
-            paths = { "/" },
             snis = { "www.example.org" },
             service = {
               name = "service_behind_www.example.org"

spec/02-integration/05-proxy/03-upstream_headers_spec.lua

@@ -278,7 +278,6 @@ for _, strategy in helpers.each_strategy() do
 
         assert(bp.routes:insert {
           hosts = { "headers-charset.com" },
-          paths = { "/" },
           service = service,
         })
 

spec/02-integration/05-proxy/14-server_tokens_spec.lua

@@ -291,7 +291,6 @@ describe("headers [#" .. strategy .. "]", function()
       return function()
         bp.routes:insert {
           hosts = { "headers-inspect.com" },
-          paths = { "/" },
         }
 
         local service = bp.services:insert({

spec/03-plugins/03-http-log/01-log_spec.lua

@@ -59,7 +59,6 @@ for _, strategy in helpers.each_strategy() do
 
       local route1 = bp.routes:insert {
         hosts   = { "http_logging.test" },
-        paths   = { "/" },
         service = service1
       }
 
@@ -627,7 +626,6 @@ for _, strategy in helpers.each_strategy() do
 
         local route = bp.routes:insert {
           hosts   = { "http_queue_logging.test" },
-          paths   = { "/" },
           service = service
         }
 

spec/03-plugins/07-loggly/01-log_spec.lua

@@ -19,7 +19,6 @@ for _, strategy in helpers.each_strategy() do
 
       local route1 = bp.routes:insert {
         hosts = { "logging.com" },
-        paths = { "/" },
       }
 
       local route2 = bp.routes:insert {

spec/03-plugins/13-cors/01-access_spec.lua

@@ -237,7 +237,6 @@ for _, strategy in helpers.each_strategy() do
 
       local route1 = bp.routes:insert({
         hosts = { "cors1.com" },
-        paths = { "/" },
       })
 
       local route2 = bp.routes:insert({

spec/03-plugins/25-oauth2/04-invalidations_spec.lua

@@ -43,7 +43,6 @@ for _, strategy in helpers.each_strategy() do
       route = assert(admin_api.routes:insert {
         hosts     = { "oauth2.com" },
         protocols = { "http", "https" },
-        paths = { "/" },
         service   = service,
       })
 

spec/03-plugins/31-proxy-cache/02-access_spec.lua

@@ -38,7 +38,6 @@ do
 
       local route1 = assert(bp.routes:insert {
         hosts = { "route-1.com" },
-        paths = { "/" },
       })
       local route2 = assert(bp.routes:insert {
         hosts = { "route-2.com" },