From b5716b1155044a79fceaa4b10a8349643c7a8d34 Mon Sep 17 00:00:00 2001 From: Joshua Schmid Date: Wed, 17 Jul 2024 14:17:11 +0200 Subject: [PATCH] fix(proxy-cache): correctly add age header on cache hit --- .../kong/proxy-cache-fix-age-header.yml | 4 + kong/plugins/proxy-cache/handler.lua | 2 +- .../31-proxy-cache/02-access_spec.lua | 82 +++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/kong/proxy-cache-fix-age-header.yml diff --git a/changelog/unreleased/kong/proxy-cache-fix-age-header.yml b/changelog/unreleased/kong/proxy-cache-fix-age-header.yml new file mode 100644 index 000000000000..b4d94e0c8ae4 --- /dev/null +++ b/changelog/unreleased/kong/proxy-cache-fix-age-header.yml @@ -0,0 +1,4 @@ +message: | + **proxy-cache**: Fixed an issue where the Age header was not being updated correctly when serving cached responses. +scope: Plugin +type: bugfix diff --git a/kong/plugins/proxy-cache/handler.lua b/kong/plugins/proxy-cache/handler.lua index 69e11ae081b3..4b2c04421951 100644 --- a/kong/plugins/proxy-cache/handler.lua +++ b/kong/plugins/proxy-cache/handler.lua @@ -401,7 +401,7 @@ function ProxyCacheHandler:access(conf) reset_res_header(res) - set_res_header(res, "Age", floor(time() - res.timestamp), conf) + set_res_header(res, "age", floor(time() - res.timestamp), conf) set_res_header(res, "X-Cache-Status", "Hit", conf) set_res_header(res, "X-Cache-Key", cache_key, conf) 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 1dc0c5bb9305..250b07a78302 100644 --- a/spec/03-plugins/31-proxy-cache/02-access_spec.lua +++ b/spec/03-plugins/31-proxy-cache/02-access_spec.lua @@ -102,6 +102,12 @@ do local route22 = assert(bp.routes:insert({ hosts = { "route-22.test" }, })) + local route23 = assert(bp.routes:insert({ + hosts = { "route-23.test" }, + })) + local route24 = assert(bp.routes:insert({ + hosts = { "route-24.test" }, + })) local consumer1 = assert(bp.consumers:insert { username = "bob", @@ -336,6 +342,32 @@ do }, }) + assert(bp.plugins:insert { + name = "proxy-cache", + route = { id = route23.id }, + config = { + strategy = policy, + content_type = { "text/plain", "application/json" }, + [policy] = policy_config, + response_headers = { + age = true, + ["X-Cache-Status"] = true, + ["X-Cache-Key"] = true + }, + }, + }) + + assert(bp.plugins:insert { + name = "proxy-cache", + route = { id = route24.id }, + config = { + strategy = policy, + content_type = { "text/plain", "application/json" }, + [policy] = policy_config, + -- leave reponse_header to default values + }, + }) + assert(helpers.start_kong({ plugins = "bundled", nginx_conf = "spec/fixtures/custom_nginx.template", @@ -416,6 +448,56 @@ do assert.is_not_nil(res.headers["X-Cache-Key"]) end) + it("response_headers headers on the response when configured", function() + -- Initial query to set cache + local res = assert(client:get("/get", { + headers = { + Host = "route-23.test", + }, + })) + -- Cache should be Miss + assert.res_status(200, res) + assert.is_same("Miss", res.headers["X-Cache-Status"]) + assert.is_not_nil(res.headers["X-Cache-Key"]) + assert.is_nil(res.headers["Age"]) + -- Cache should be HIT + res = assert(client:get("/get", { + headers = { + Host = "route-23.test", + }, + })) + assert.res_status(200, res) + assert.same("Hit", res.headers["X-Cache-Status"]) + -- response_headers are configured + assert.is_not_nil(res.headers["Age"]) + assert.is_not_nil(res.headers["X-Cache-Key"]) + end) + + it("response_headers headers on the response when set to default", function() + -- Initial query to set cache + local res = assert(client:get("/get", { + headers = { + Host = "route-24.test", + }, + })) + -- Cache should be Miss + assert.res_status(200, res) + assert.is_same("Miss", res.headers["X-Cache-Status"]) + assert.is_not_nil(res.headers["X-Cache-Key"]) + assert.is_nil(res.headers["Age"]) + res = assert(client:get("/get", { + headers = { + Host = "route-24.test", + }, + })) + -- Cache should be Hit + assert.res_status(200, res) + assert.same("Hit", res.headers["X-Cache-Status"]) + -- response_headers are on by default + assert.is_not_nil(res.headers["Age"]) + assert.is_not_nil(res.headers["X-Cache-Key"]) + end) + it("respects cache ttl", function() local res = assert(get(client, "route-6.test"))