From b6bfc48c370e9278f58cddb279c143c33674bdb9 Mon Sep 17 00:00:00 2001 From: chronolaw Date: Tue, 9 Jan 2024 10:51:27 +0800 Subject: [PATCH 1/8] get_upstream_uri --- kong/router/atc.lua | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index c28bad7f8ea..d2f9494c062 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -402,6 +402,24 @@ local add_debug_headers = utils.add_debug_headers local get_upstream_uri_v0 = utils.get_upstream_uri_v0 +local function get_upstream_uri(matched_route, matched_path, req_uri, service_path) + --local matched_route = match_t.route + --local matched_path = match_t.matches.path + + 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 upstream_uri +end + + function _M:matching(params) local req_uri = params.uri local req_host = params.host @@ -438,13 +456,8 @@ function _M:matching(params) service_host, service_port, 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) + local upstream_uri = get_upstream_uri(matched_route, matched_path, req_uri, + service_path) return { route = matched_route, @@ -452,6 +465,7 @@ function _M:matching(params) prefix = request_prefix, matches = { uri_captures = (captures and captures[1]) and captures or nil, + path = matched_path, }, upstream_url_t = { type = service_hostname_type, From 1fc0a37fff5d7da4d92ed076214426abe1c0effa Mon Sep 17 00:00:00 2001 From: chronolaw Date: Tue, 9 Jan 2024 11:06:50 +0800 Subject: [PATCH 2/8] fix get_upstream_uri --- kong/router/atc.lua | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index d2f9494c062..0148b968035 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -402,12 +402,11 @@ local add_debug_headers = utils.add_debug_headers local get_upstream_uri_v0 = utils.get_upstream_uri_v0 -local function get_upstream_uri(matched_route, matched_path, req_uri, service_path) +local function get_upstream_uri(matched_route, matched_path, + request_prefix, req_uri, service_path) --local matched_route = match_t.route --local matched_path = match_t.matches.path - 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 "" @@ -456,8 +455,10 @@ function _M:matching(params) service_host, service_port, service_hostname_type, service_path = get_service_info(service) - local upstream_uri = get_upstream_uri(matched_route, matched_path, req_uri, - service_path) + local request_prefix = matched_route.strip_path and matched_path or nil + + local upstream_uri = get_upstream_uri(matched_route, matched_path, + request_prefix, req_uri, service_path) return { route = matched_route, @@ -465,7 +466,6 @@ function _M:matching(params) prefix = request_prefix, matches = { uri_captures = (captures and captures[1]) and captures or nil, - path = matched_path, }, upstream_url_t = { type = service_hostname_type, From f018bc1f5d1aae713054b99933eebe1bb850a057 Mon Sep 17 00:00:00 2001 From: chronolaw Date: Tue, 9 Jan 2024 11:27:31 +0800 Subject: [PATCH 3/8] test get_upstream_uri --- kong/router/atc.lua | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 0148b968035..48e9acd7542 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -404,8 +404,6 @@ local get_upstream_uri_v0 = utils.get_upstream_uri_v0 local function get_upstream_uri(matched_route, matched_path, request_prefix, req_uri, service_path) - --local matched_route = match_t.route - --local matched_path = match_t.matches.path 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 "" @@ -466,11 +464,13 @@ function _M:matching(params) prefix = request_prefix, matches = { uri_captures = (captures and captures[1]) and captures or nil, + path = matched_path, }, upstream_url_t = { type = service_hostname_type, host = service_host, port = service_port, + path = service_path, }, upstream_scheme = service_protocol, upstream_uri = upstream_uri, @@ -556,6 +556,13 @@ function _M:exec(ctx) if match_t.route.preserve_host then match_t.upstream_host = req_host end + + -- update upstream_uri in cache result + --[[ + match_t.upstream_uri = get_upstream_uri(match_t.route, match_t.matches.path, + match_t.request_prefix, req_uri, + match_t.upstream_url_t.path) + --]] end -- found a match From fe70ba5c057ddb04c96f22da304287354e82b81a Mon Sep 17 00:00:00 2001 From: chronolaw Date: Tue, 9 Jan 2024 11:38:11 +0800 Subject: [PATCH 4/8] calc match_t.upstream_uri --- kong/router/atc.lua | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 48e9acd7542..1a3aab5fe6b 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -402,8 +402,12 @@ local add_debug_headers = utils.add_debug_headers local get_upstream_uri_v0 = utils.get_upstream_uri_v0 -local function get_upstream_uri(matched_route, matched_path, - request_prefix, req_uri, service_path) +local function get_upstream_uri(req_uri, match_t) + local matched_route = match_t.route + local matched_path = match_t.matches.path + + local request_prefix = match_t.prefix + local service_path = match_t.upstream_url_t.path 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 "" @@ -455,16 +459,13 @@ function _M:matching(params) local request_prefix = matched_route.strip_path and matched_path or nil - local upstream_uri = get_upstream_uri(matched_route, matched_path, - request_prefix, req_uri, service_path) - return { route = matched_route, service = service, prefix = request_prefix, matches = { - uri_captures = (captures and captures[1]) and captures or nil, path = matched_path, + uri_captures = (captures and captures[1]) and captures or nil, }, upstream_url_t = { type = service_hostname_type, @@ -473,7 +474,6 @@ function _M:matching(params) path = service_path, }, upstream_scheme = service_protocol, - upstream_uri = upstream_uri, upstream_host = matched_route.preserve_host and req_host or nil, } end @@ -556,17 +556,13 @@ function _M:exec(ctx) if match_t.route.preserve_host then match_t.upstream_host = req_host end - - -- update upstream_uri in cache result - --[[ - match_t.upstream_uri = get_upstream_uri(match_t.route, match_t.matches.path, - match_t.request_prefix, req_uri, - match_t.upstream_url_t.path) - --]] end -- found a match + -- update upstream_uri in cache result + match_t.upstream_uri = get_upstream_uri(req_uri, match_t) + -- debug HTTP request header logic add_debug_headers(var, header, match_t) From a49995e3b9aae9d05567ee0ef144f00ed3d6b15d Mon Sep 17 00:00:00 2001 From: chronolaw Date: Tue, 9 Jan 2024 12:13:17 +0800 Subject: [PATCH 5/8] tests --- spec/01-unit/08-router_spec.lua | 50 ++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/spec/01-unit/08-router_spec.lua b/spec/01-unit/08-router_spec.lua index c7b4c42eded..e08f8b8d279 100644 --- a/spec/01-unit/08-router_spec.lua +++ b/spec/01-unit/08-router_spec.lua @@ -4945,10 +4945,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 = { @@ -4962,10 +4960,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", }) @@ -4995,6 +4991,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" From 18fd7250b5cb39dd4647dac8741056e04421df6f Mon Sep 17 00:00:00 2001 From: chronolaw Date: Tue, 9 Jan 2024 14:07:20 +0800 Subject: [PATCH 6/8] clean --- kong/router/atc.lua | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 1a3aab5fe6b..33fc3a4b816 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -404,12 +404,11 @@ local get_upstream_uri_v0 = utils.get_upstream_uri_v0 local function get_upstream_uri(req_uri, match_t) local matched_route = match_t.route - local matched_path = match_t.matches.path local request_prefix = match_t.prefix local service_path = match_t.upstream_url_t.path - local request_postfix = request_prefix and req_uri:sub(#matched_path + 1) or req_uri:sub(2, -1) + local request_postfix = request_prefix and req_uri:sub(#request_prefix + 1) or req_uri:sub(2, -1) request_postfix = sanitize_uri_postfix(request_postfix) or "" local upstream_base = service_path or "/" @@ -464,7 +463,6 @@ function _M:matching(params) service = service, prefix = request_prefix, matches = { - path = matched_path, uri_captures = (captures and captures[1]) and captures or nil, }, upstream_url_t = { From 7ec7a0a3b0d311012970ea90c7cb0023eb7ae12b Mon Sep 17 00:00:00 2001 From: chronolaw Date: Tue, 9 Jan 2024 14:25:00 +0800 Subject: [PATCH 7/8] set_upstream_uri --- kong/router/atc.lua | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/kong/router/atc.lua b/kong/router/atc.lua index 33fc3a4b816..8b3c03ad1b1 100644 --- a/kong/router/atc.lua +++ b/kong/router/atc.lua @@ -402,21 +402,16 @@ local add_debug_headers = utils.add_debug_headers local get_upstream_uri_v0 = utils.get_upstream_uri_v0 -local function get_upstream_uri(req_uri, match_t) +local function set_upstream_uri(req_uri, match_t) local matched_route = match_t.route - local request_prefix = match_t.prefix - local service_path = match_t.upstream_url_t.path + local request_prefix = match_t.prefix or "/" + local request_postfix = sanitize_uri_postfix(req_uri:sub(#request_prefix + 1)) - local request_postfix = request_prefix and req_uri:sub(#request_prefix + 1) or req_uri:sub(2, -1) - request_postfix = sanitize_uri_postfix(request_postfix) or "" + local upstream_base = match_t.upstream_url_t.path or "/" - local upstream_base = service_path or "/" - - local upstream_uri = get_upstream_uri_v0(matched_route, request_postfix, req_uri, - upstream_base) - - return upstream_uri + match_t.upstream_uri = get_upstream_uri_v0(matched_route, request_postfix, + req_uri, upstream_base) end @@ -559,7 +554,7 @@ function _M:exec(ctx) -- found a match -- update upstream_uri in cache result - match_t.upstream_uri = get_upstream_uri(req_uri, match_t) + set_upstream_uri(req_uri, match_t) -- debug HTTP request header logic add_debug_headers(var, header, match_t) From 70c2fa9c74dbfc473dbe52689af54c303dba2772 Mon Sep 17 00:00:00 2001 From: chronolaw Date: Thu, 11 Jan 2024 18:24:37 +0800 Subject: [PATCH 8/8] remove paths added in #12261 --- spec/02-integration/01-helpers/01-helpers_spec.lua | 1 - spec/02-integration/05-proxy/02-router_spec.lua | 2 -- spec/02-integration/05-proxy/03-upstream_headers_spec.lua | 1 - spec/02-integration/05-proxy/14-server_tokens_spec.lua | 1 - spec/03-plugins/03-http-log/01-log_spec.lua | 2 -- spec/03-plugins/07-loggly/01-log_spec.lua | 1 - spec/03-plugins/13-cors/01-access_spec.lua | 1 - spec/03-plugins/25-oauth2/04-invalidations_spec.lua | 1 - spec/03-plugins/31-proxy-cache/02-access_spec.lua | 1 - 9 files changed, 11 deletions(-) diff --git a/spec/02-integration/01-helpers/01-helpers_spec.lua b/spec/02-integration/01-helpers/01-helpers_spec.lua index ccc0232afce..e267935e066 100644 --- a/spec/02-integration/01-helpers/01-helpers_spec.lua +++ b/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 } diff --git a/spec/02-integration/05-proxy/02-router_spec.lua b/spec/02-integration/05-proxy/02-router_spec.lua index 26ba41a4617..74d4f491bee 100644 --- a/spec/02-integration/05-proxy/02-router_spec.lua +++ b/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" diff --git a/spec/02-integration/05-proxy/03-upstream_headers_spec.lua b/spec/02-integration/05-proxy/03-upstream_headers_spec.lua index c78203d3b5f..3132d0a6bfd 100644 --- a/spec/02-integration/05-proxy/03-upstream_headers_spec.lua +++ b/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.test" }, - paths = { "/" }, service = service, }) diff --git a/spec/02-integration/05-proxy/14-server_tokens_spec.lua b/spec/02-integration/05-proxy/14-server_tokens_spec.lua index 3de5077db9d..6cee745a135 100644 --- a/spec/02-integration/05-proxy/14-server_tokens_spec.lua +++ b/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.test" }, - paths = { "/" }, } local service = bp.services:insert({ diff --git a/spec/03-plugins/03-http-log/01-log_spec.lua b/spec/03-plugins/03-http-log/01-log_spec.lua index 4a69c9b221d..55591eb85dd 100644 --- a/spec/03-plugins/03-http-log/01-log_spec.lua +++ b/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 } @@ -628,7 +627,6 @@ for _, strategy in helpers.each_strategy() do local route = bp.routes:insert { hosts = { "http_queue_logging.test" }, - paths = { "/" }, service = service } diff --git a/spec/03-plugins/07-loggly/01-log_spec.lua b/spec/03-plugins/07-loggly/01-log_spec.lua index 4987cbb1d9a..dd5e35a0199 100644 --- a/spec/03-plugins/07-loggly/01-log_spec.lua +++ b/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.test" }, - paths = { "/" }, } local route2 = bp.routes:insert { diff --git a/spec/03-plugins/13-cors/01-access_spec.lua b/spec/03-plugins/13-cors/01-access_spec.lua index 42692a43089..7bba3a82ce8 100644 --- a/spec/03-plugins/13-cors/01-access_spec.lua +++ b/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.test" }, - paths = { "/" }, }) local route2 = bp.routes:insert({ diff --git a/spec/03-plugins/25-oauth2/04-invalidations_spec.lua b/spec/03-plugins/25-oauth2/04-invalidations_spec.lua index 18218b6cfdb..90f7b25bf85 100644 --- a/spec/03-plugins/25-oauth2/04-invalidations_spec.lua +++ b/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, }) diff --git a/spec/03-plugins/31-proxy-cache/02-access_spec.lua b/spec/03-plugins/31-proxy-cache/02-access_spec.lua index 67e026d9e32..aa8b350773d 100644 --- a/spec/03-plugins/31-proxy-cache/02-access_spec.lua +++ b/spec/03-plugins/31-proxy-cache/02-access_spec.lua @@ -38,7 +38,6 @@ do local route1 = assert(bp.routes:insert { hosts = { "route-1.test" }, - paths = { "/" }, }) local route2 = assert(bp.routes:insert { hosts = { "route-2.test" },