From 0e32f57b2c644cb8879a905da199d04fb5a5dc1d Mon Sep 17 00:00:00 2001 From: samugi Date: Tue, 6 Jun 2023 18:23:07 +0200 Subject: [PATCH 1/9] feat(request-aware-table) add request aware table add a request-aware table able to detect accesses from different requests. --- kong-3.5.0-0.rockspec | 1 + kong/templates/nginx_kong.lua | 1 + kong/tools/request_aware_table.lua | 89 +++++++++++ spec/01-unit/28-request-aware-table_spec.lua | 156 +++++++++++++++++++ 4 files changed, 247 insertions(+) create mode 100644 kong/tools/request_aware_table.lua create mode 100644 spec/01-unit/28-request-aware-table_spec.lua diff --git a/kong-3.5.0-0.rockspec b/kong-3.5.0-0.rockspec index cf357401cf6b..c02b73864a35 100644 --- a/kong-3.5.0-0.rockspec +++ b/kong-3.5.0-0.rockspec @@ -163,6 +163,7 @@ build = { ["kong.tools.kong-lua-sandbox"] = "kong/tools/kong-lua-sandbox.lua", ["kong.tools.protobuf"] = "kong/tools/protobuf.lua", ["kong.tools.mime_type"] = "kong/tools/mime_type.lua", + ["kong.tools.request_aware_table"] = "kong/tools/request_aware_table.lua", ["kong.runloop.handler"] = "kong/runloop/handler.lua", ["kong.runloop.events"] = "kong/runloop/events.lua", diff --git a/kong/templates/nginx_kong.lua b/kong/templates/nginx_kong.lua index 2da98f9213d6..efa41d2b8239 100644 --- a/kong/templates/nginx_kong.lua +++ b/kong/templates/nginx_kong.lua @@ -58,6 +58,7 @@ log_format kong_log_format '$remote_addr - $remote_user [$time_local] ' # Load variable indexes lua_kong_load_var_index default; +lua_kong_load_var_index $request_id; upstream kong_upstream { server 0.0.0.1; diff --git a/kong/tools/request_aware_table.lua b/kong/tools/request_aware_table.lua new file mode 100644 index 000000000000..bc7ef1828025 --- /dev/null +++ b/kong/tools/request_aware_table.lua @@ -0,0 +1,89 @@ +local table_clear = require "table.clear" + +local get_phase = ngx.get_phase +local fmt = string.format +local ngx_var = ngx.var + +local NGX_VAR_PHASES = { + set = true, + rewrite = true, + balancer = true, + access = true, + content = true, + header_filter = true, + body_filter = true, + log = true, +} + + +--- Request aware table constructor +-- Wraps an existing table (or creates a new one) with request-aware access +-- logic to protect the underlying data from race conditions. +-- @param data_table (optional) The target table to use as underlying data +-- @param request_awareness_mode (optional) The mode for request awareness +-- - "off": No request awareness mode +-- - any other value, or nil: Control access for every r/w operation +-- @return The newly created table with request-aware access +local function new(data_table, request_awareness_mode) + if data_table and type(data_table) ~= "table" then + error("data_table must be a table", 2) + end + + local allowed_request_id + local proxy = {} + local data = data_table or {} + + -- Check if access is allowed based on the request ID + local function enforce_sequential_access() + local curr_phase = get_phase() + if not NGX_VAR_PHASES[curr_phase] then + error(fmt("cannot enforce sequential access in %s phase", curr_phase), 2) + end + + local curr_request_id = ngx_var.request_id + + allowed_request_id = allowed_request_id or curr_request_id + + if curr_request_id ~= allowed_request_id then + error("race condition detected; access to table forbidden", 2) + end + end + + --- Clear data table + -- @tparam function fn (optional) An optional function to use instead + -- of `table.clear` to clear the data table + function proxy.clear(fn) + if fn then + fn(data) + + else + table_clear(data) + end + + allowed_request_id = nil + end + + local _proxy_mt = { + __index = function(_, k) + if request_awareness_mode ~= "off" then + enforce_sequential_access() + end + + return data[k] + end, + + __newindex = function(_, k, v) + if request_awareness_mode ~= "off" then + enforce_sequential_access() + end + + data[k] = v + end + } + + return setmetatable(proxy, _proxy_mt) +end + +return { + new = new, +} diff --git a/spec/01-unit/28-request-aware-table_spec.lua b/spec/01-unit/28-request-aware-table_spec.lua new file mode 100644 index 000000000000..08ebc183c455 --- /dev/null +++ b/spec/01-unit/28-request-aware-table_spec.lua @@ -0,0 +1,156 @@ +local utils = require "kong.tools.utils" +local tablex = require "pl.tablex" + +local rat + +local function assert_rw_allowed(tab, orig_t) + for k, v in pairs(orig_t or {}) do + -- reads from orig_t succeed + local val = assert.has_no.errors(function() return tab[k] end) + assert.equal(v, val) + end + + local k = utils.random_string() + local v = utils.random_string() + -- writing new values succeeds + assert.has_no.errors(function() tab[k] = v end) + -- reading new values succeeds + local val = assert.has_no.errors(function() return tab[k] end) + assert.equal(v, val) +end + +local function assert_rw_denied(tab, orig_t) + local err_str = "race condition detected" + for k, v in pairs(orig_t or {}) do + -- reads from orig_t error out + assert.error_matches(function() return nil, tab[k] == v end, err_str) + end + + local k = utils.random_string() + local v = utils.random_string() + -- writing new values errors out + assert.error_matches(function() tab[k] = v end, err_str) + -- reading new values errors out + assert.error_matches(function() return tab[k] == v end, err_str) +end + +describe("Request aware table", function() + local old_ngx + local tab + + lazy_setup(function() + old_ngx = ngx + _G.ngx = { + get_phase = function() return "access" end, + var = {}, + } + rat = require "kong.tools.request_aware_table" + end) + + lazy_teardown(function() + _G.ngx = old_ngx + end) + + describe("with concurrency check enabled", function() + local orig_t + + before_each(function() + orig_t = { + k1 = utils.random_string(), + k2 = utils.random_string(), + } + tab = rat.new(orig_t, "on") + end) + + it("allows access when there are no race conditions", function() + -- create a new RAT with request_id = 1 (clear after use) + _G.ngx.var.request_id = "1" + assert_rw_allowed(tab, orig_t) + tab.clear() + + -- reuse RAT with different request_id (allowed) + _G.ngx.var.request_id = "2" + assert_rw_allowed(tab) + end) + + it("denies access when there are race conditions", function() + -- create a new RAT with request_id = 1 (no clear) + _G.ngx.var.request_id = "1" + assert_rw_allowed(tab, orig_t) + + -- reuse RAT with different request_id (not allowed) + _G.ngx.var.request_id = "2" + assert_rw_denied(tab) + end) + + it("clears the table successfully", function() + -- create a new RAT with request_id = 1 (clear after use) + _G.ngx.var.request_id = "1" + assert_rw_allowed(tab, orig_t) + tab.clear() + + assert.same(0, tablex.size(orig_t)) + end) + + it("allows defining a custom clear function", function() + -- create a new RAT with request_id = 1 (clear after use) + _G.ngx.var.request_id = "1" + orig_t.persist = "persistent_value" + assert_rw_allowed(tab, orig_t) + + -- custom clear function that keeps persistent_value + tab.clear(function(t) + for k in pairs(t) do + if k ~= "persist" then + t[k] = nil + end + end + end) + + -- confirm persistent_value is the only key left + assert.same(1, tablex.size(orig_t)) + assert.equal("persistent_value", tab.persist) + + -- clear the whole table and confirm it's empty + tab.clear() + assert.same(0, tablex.size(orig_t)) + end) + end) + + describe("with concurrency check disabled", function() + local orig_t + + before_each(function() + orig_t = { + k1 = utils.random_string(), + k2 = utils.random_string(), + } + tab = rat.new(orig_t, "off") + end) + + before_each(function() + tab.clear() + end) + + it("allows access when there are no race conditions", function() + -- create a new RAT with request_id = 1 (clear after use) + _G.ngx.var.request_id = "1" + assert_rw_allowed(tab, orig_t) + tab.clear() + + -- reuse RAT with different request_id (allowed) + _G.ngx.var.request_id = "2" + assert_rw_allowed(tab, orig_t) + end) + + it("allows access when there are race conditions", function() + -- create a new RAT with request_id = 1, (no clear) + _G.ngx.var.request_id = "1" + assert_rw_allowed(tab, orig_t) + + -- reuse RAT with different request_id (allowed with check disabled) + _G.ngx.var.request_id = "2" + assert_rw_allowed(tab, orig_t) + end) + end) +end) From 4e172da014922386fcfd522d945d4c8f38b191bb Mon Sep 17 00:00:00 2001 From: samugi Date: Wed, 30 Aug 2023 18:16:17 +0200 Subject: [PATCH 2/9] tests(request-aware-table): integration --- spec/01-unit/28-request-aware-table_spec.lua | 110 +--------------- .../05-proxy/31-request-aware-table_spec.lua | 121 ++++++++++++++++++ .../plugins/request-aware-table/handler.lua | 52 ++++++++ .../plugins/request-aware-table/schema.lua | 11 ++ 4 files changed, 187 insertions(+), 107 deletions(-) create mode 100644 spec/02-integration/05-proxy/31-request-aware-table_spec.lua create mode 100644 spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua create mode 100644 spec/fixtures/custom_plugins/kong/plugins/request-aware-table/schema.lua diff --git a/spec/01-unit/28-request-aware-table_spec.lua b/spec/01-unit/28-request-aware-table_spec.lua index 08ebc183c455..b82456b65a1f 100644 --- a/spec/01-unit/28-request-aware-table_spec.lua +++ b/spec/01-unit/28-request-aware-table_spec.lua @@ -1,39 +1,7 @@ -local utils = require "kong.tools.utils" local tablex = require "pl.tablex" local rat -local function assert_rw_allowed(tab, orig_t) - for k, v in pairs(orig_t or {}) do - -- reads from orig_t succeed - local val = assert.has_no.errors(function() return tab[k] end) - assert.equal(v, val) - end - - local k = utils.random_string() - local v = utils.random_string() - -- writing new values succeeds - assert.has_no.errors(function() tab[k] = v end) - -- reading new values succeeds - local val = assert.has_no.errors(function() return tab[k] end) - assert.equal(v, val) -end - -local function assert_rw_denied(tab, orig_t) - local err_str = "race condition detected" - for k, v in pairs(orig_t or {}) do - -- reads from orig_t error out - assert.error_matches(function() return nil, tab[k] == v end, err_str) - end - - local k = utils.random_string() - local v = utils.random_string() - -- writing new values errors out - assert.error_matches(function() tab[k] = v end, err_str) - -- reading new values errors out - assert.error_matches(function() return tab[k] == v end, err_str) -end - describe("Request aware table", function() local old_ngx local tab @@ -52,51 +20,16 @@ describe("Request aware table", function() end) describe("with concurrency check enabled", function() - local orig_t + local orig_t = {} before_each(function() - orig_t = { - k1 = utils.random_string(), - k2 = utils.random_string(), - } tab = rat.new(orig_t, "on") end) - it("allows access when there are no race conditions", function() - -- create a new RAT with request_id = 1 (clear after use) - _G.ngx.var.request_id = "1" - assert_rw_allowed(tab, orig_t) - tab.clear() - - -- reuse RAT with different request_id (allowed) - _G.ngx.var.request_id = "2" - assert_rw_allowed(tab) - end) - - it("denies access when there are race conditions", function() - -- create a new RAT with request_id = 1 (no clear) - _G.ngx.var.request_id = "1" - assert_rw_allowed(tab, orig_t) - - -- reuse RAT with different request_id (not allowed) - _G.ngx.var.request_id = "2" - assert_rw_denied(tab) - end) - - it("clears the table successfully", function() - -- create a new RAT with request_id = 1 (clear after use) - _G.ngx.var.request_id = "1" - assert_rw_allowed(tab, orig_t) - tab.clear() - - assert.same(0, tablex.size(orig_t)) - end) - it("allows defining a custom clear function", function() - -- create a new RAT with request_id = 1 (clear after use) - _G.ngx.var.request_id = "1" orig_t.persist = "persistent_value" - assert_rw_allowed(tab, orig_t) + orig_t.foo = "bar" + orig_t.baz = "qux" -- custom clear function that keeps persistent_value tab.clear(function(t) @@ -116,41 +49,4 @@ describe("Request aware table", function() assert.same(0, tablex.size(orig_t)) end) end) - - describe("with concurrency check disabled", function() - local orig_t - - before_each(function() - orig_t = { - k1 = utils.random_string(), - k2 = utils.random_string(), - } - tab = rat.new(orig_t, "off") - end) - - before_each(function() - tab.clear() - end) - - it("allows access when there are no race conditions", function() - -- create a new RAT with request_id = 1 (clear after use) - _G.ngx.var.request_id = "1" - assert_rw_allowed(tab, orig_t) - tab.clear() - - -- reuse RAT with different request_id (allowed) - _G.ngx.var.request_id = "2" - assert_rw_allowed(tab, orig_t) - end) - - it("allows access when there are race conditions", function() - -- create a new RAT with request_id = 1, (no clear) - _G.ngx.var.request_id = "1" - assert_rw_allowed(tab, orig_t) - - -- reuse RAT with different request_id (allowed with check disabled) - _G.ngx.var.request_id = "2" - assert_rw_allowed(tab, orig_t) - end) - end) end) diff --git a/spec/02-integration/05-proxy/31-request-aware-table_spec.lua b/spec/02-integration/05-proxy/31-request-aware-table_spec.lua new file mode 100644 index 000000000000..ca34bf9eedf8 --- /dev/null +++ b/spec/02-integration/05-proxy/31-request-aware-table_spec.lua @@ -0,0 +1,121 @@ +local helpers = require "spec.helpers" + + +local function clear_table(client, checks) + local res = client:get("/", { + query = { + checks = checks, + clear = true, + } + }) + assert.response(res).has.status(200) + assert.logfile().has.no.line("[error]", true) +end + +for _, checks in ipairs({ true, false }) do +for _, strategy in helpers.each_strategy() do + describe("request aware table tests [#" .. strategy .. "] .. checks=" .. tostring(checks), function() + local client + lazy_setup(function() + local bp = helpers.get_db_utils(strategy, { + "plugins", + "routes", + "services", + }, { + "request-aware-table" + }) + + local service = assert(bp.services:insert({ + url = helpers.mock_upstream_url + })) + + local route = bp.routes:insert({ + service = service, + paths = { "/" } + }) + + bp.plugins:insert({ + name = "request-aware-table", + route = { id = route.id }, + }) + + helpers.start_kong({ + database = strategy, + plugins = "bundled, request-aware-table", + nginx_conf = "spec/fixtures/custom_nginx.template", + }) + end) + + lazy_teardown(function() + helpers.stop_kong() + end) + + before_each(function() + helpers.clean_logfile() + client = helpers.proxy_client() + clear_table(client, checks) + end) + + after_each(function() + if client then + client:close() + end + end) + + describe("with concurrency check enabled", function() + it("allows access when there are no race conditions", function() + local res = client:get("/", { + query = { + checks = checks, + } + }) + assert.response(res).has.status(200) + assert.logfile().has.no.line("[error]", true) + end) + it("denies access when there are race conditions and checks are enabled", function() + -- access from request 1 (don't clear) + local ok, r = pcall(client.get, client, "/", { + query = { + checks = checks, + }, + }) + assert(ok) + assert.response(r).has.status(200) + + -- access from request 2 + ok, r = pcall(client.get, client, "/", { + query = { + checks = checks, + }, + }) + if checks then + assert(not ok) + assert.logfile().has.line("race condition detected", true) + else + assert(ok) + assert.response(r).has.status(200) + end + end) + it("allows access when table is cleared between requests", function() + -- access from request 1 (clear) + local r = client:get("/", { + query = { + checks = checks, + clear = true, + }, + }) + assert.response(r).has.status(200) + + -- access from request 2 + r = client:get("/", { + query = { + checks = checks, + }, + }) + assert.response(r).has.status(200) + assert.logfile().has.no.line("[error]", true) + end) + end) + end) +end +end \ No newline at end of file diff --git a/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua new file mode 100644 index 000000000000..c51c6aa29b6c --- /dev/null +++ b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua @@ -0,0 +1,52 @@ +local RAT = require "kong.tools.request_aware_table" + +local get_phase = ngx.get_phase +local ngx = ngx +local kong = kong + + +local _M = { + PRIORITY = 1001, + VERSION = "1.0", +} + +local checks_tab = RAT.new({}, "on") +local no_checks_tab = RAT.new({}, "off") + +local function access_tables() + local query = kong.request.get_query() + local tab + + if query.checks ~= "false" then + tab = checks_tab + else + tab = no_checks_tab + end + + if query.clear == "true" and get_phase() == "access" then + tab.clear() + end + + -- write access + tab.foo = "bar" + -- read access + ngx.log(ngx.DEBUG, "accessing to tab.foo" .. tab.foo) + + if query.clear == "true" and get_phase() == "body_filter" then + tab.clear() + end +end + +function _M:access(conf) + access_tables() +end + +function _M:header_filter(conf) + access_tables() +end + +function _M:body_filter(conf) + access_tables() +end + +return _M diff --git a/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/schema.lua b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/schema.lua new file mode 100644 index 000000000000..2881307755a5 --- /dev/null +++ b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/schema.lua @@ -0,0 +1,11 @@ +return { + name = "request-aware-table", + fields = { + { + config = { + type = "record", + fields = { } + } + } + } +} From 391173d7c89afb16323644dc81a046cc2089de9b Mon Sep 17 00:00:00 2001 From: samugi Date: Thu, 31 Aug 2023 12:04:20 +0200 Subject: [PATCH 3/9] feat(request-aware-table): enable checks when log_level is debug --- changelog/unreleased/kong/11017.yaml | 5 + kong/tools/request_aware_table.lua | 18 ++- spec/01-unit/28-request-aware-table_spec.lua | 55 ++++---- .../05-proxy/31-request-aware-table_spec.lua | 127 ++++++++---------- .../plugins/request-aware-table/handler.lua | 18 +-- 5 files changed, 108 insertions(+), 115 deletions(-) create mode 100644 changelog/unreleased/kong/11017.yaml diff --git a/changelog/unreleased/kong/11017.yaml b/changelog/unreleased/kong/11017.yaml new file mode 100644 index 000000000000..6ce700d75fab --- /dev/null +++ b/changelog/unreleased/kong/11017.yaml @@ -0,0 +1,5 @@ +message: Add a request-aware table able to detect accesses from different requests. +type: feature +scope: Core +prs: + - 11017 diff --git a/kong/tools/request_aware_table.lua b/kong/tools/request_aware_table.lua index bc7ef1828025..8226fe3cf9f8 100644 --- a/kong/tools/request_aware_table.lua +++ b/kong/tools/request_aware_table.lua @@ -1,5 +1,8 @@ local table_clear = require "table.clear" +local LOG_LEVELS = require "kong.constants".LOG_LEVELS +local get_log_level = require("resty.kong.log").get_log_level + local get_phase = ngx.get_phase local fmt = string.format local ngx_var = ngx.var @@ -16,15 +19,18 @@ local NGX_VAR_PHASES = { } +local function is_debug_mode() + local log_level = get_log_level(LOG_LEVELS[kong.configuration.log_level]) + return log_level == LOG_LEVELS.debug +end + + --- Request aware table constructor -- Wraps an existing table (or creates a new one) with request-aware access -- logic to protect the underlying data from race conditions. -- @param data_table (optional) The target table to use as underlying data --- @param request_awareness_mode (optional) The mode for request awareness --- - "off": No request awareness mode --- - any other value, or nil: Control access for every r/w operation -- @return The newly created table with request-aware access -local function new(data_table, request_awareness_mode) +local function new(data_table) if data_table and type(data_table) ~= "table" then error("data_table must be a table", 2) end @@ -65,7 +71,7 @@ local function new(data_table, request_awareness_mode) local _proxy_mt = { __index = function(_, k) - if request_awareness_mode ~= "off" then + if is_debug_mode() then enforce_sequential_access() end @@ -73,7 +79,7 @@ local function new(data_table, request_awareness_mode) end, __newindex = function(_, k, v) - if request_awareness_mode ~= "off" then + if is_debug_mode() then enforce_sequential_access() end diff --git a/spec/01-unit/28-request-aware-table_spec.lua b/spec/01-unit/28-request-aware-table_spec.lua index b82456b65a1f..89c96fd3e554 100644 --- a/spec/01-unit/28-request-aware-table_spec.lua +++ b/spec/01-unit/28-request-aware-table_spec.lua @@ -3,14 +3,18 @@ local tablex = require "pl.tablex" local rat describe("Request aware table", function() + local orig_t = {} local old_ngx local tab lazy_setup(function() old_ngx = ngx - _G.ngx = { - get_phase = function() return "access" end, - var = {}, + _G.ngx.get_phase = function() return "access" end + _G.ngx.var = {} + _G.kong = { + configuration = { + log_level = "debug", + }, } rat = require "kong.tools.request_aware_table" end) @@ -19,34 +23,29 @@ describe("Request aware table", function() _G.ngx = old_ngx end) - describe("with concurrency check enabled", function() - local orig_t = {} + before_each(function() + tab = rat.new(orig_t) + end) - before_each(function() - tab = rat.new(orig_t, "on") - end) + it("allows defining a custom clear function", function() + orig_t.persist = "persistent_value" + orig_t.foo = "bar" + orig_t.baz = "qux" - it("allows defining a custom clear function", function() - orig_t.persist = "persistent_value" - orig_t.foo = "bar" - orig_t.baz = "qux" - - -- custom clear function that keeps persistent_value - tab.clear(function(t) - for k in pairs(t) do - if k ~= "persist" then - t[k] = nil - end + -- custom clear function that keeps persistent_value + tab.clear(function(t) + for k in pairs(t) do + if k ~= "persist" then + t[k] = nil end - end) - - -- confirm persistent_value is the only key left - assert.same(1, tablex.size(orig_t)) - assert.equal("persistent_value", tab.persist) - - -- clear the whole table and confirm it's empty - tab.clear() - assert.same(0, tablex.size(orig_t)) + end end) + + -- confirm persistent_value is the only key left + assert.same(1, tablex.size(orig_t)) + assert.equal("persistent_value", tab.persist) + -- clear the whole table and confirm it's empty + tab.clear() + assert.same(0, tablex.size(orig_t)) end) end) diff --git a/spec/02-integration/05-proxy/31-request-aware-table_spec.lua b/spec/02-integration/05-proxy/31-request-aware-table_spec.lua index ca34bf9eedf8..42d4a80b88cf 100644 --- a/spec/02-integration/05-proxy/31-request-aware-table_spec.lua +++ b/spec/02-integration/05-proxy/31-request-aware-table_spec.lua @@ -1,10 +1,15 @@ local helpers = require "spec.helpers" +local LOG_LEVELS = { + "debug", + "info", + -- any other log level behaves the same as "info" +} -local function clear_table(client, checks) + +local function clear_table(client) local res = client:get("/", { query = { - checks = checks, clear = true, } }) @@ -12,83 +17,73 @@ local function clear_table(client, checks) assert.logfile().has.no.line("[error]", true) end -for _, checks in ipairs({ true, false }) do -for _, strategy in helpers.each_strategy() do - describe("request aware table tests [#" .. strategy .. "] .. checks=" .. tostring(checks), function() - local client - lazy_setup(function() - local bp = helpers.get_db_utils(strategy, { - "plugins", - "routes", - "services", - }, { - "request-aware-table" - }) +for _, log_level in ipairs(LOG_LEVELS) do + local concurrency_checks = log_level == "debug" - local service = assert(bp.services:insert({ - url = helpers.mock_upstream_url - })) + for _, strategy in helpers.each_strategy() do + describe("request aware table tests [#" .. strategy .. "] concurrency checks: " .. tostring(concurrency_checks), function() + local client + lazy_setup(function() + local bp = helpers.get_db_utils(strategy, { + "plugins", + "routes", + "services", + }, { + "request-aware-table" + }) - local route = bp.routes:insert({ - service = service, - paths = { "/" } - }) + local service = assert(bp.services:insert({ + url = helpers.mock_upstream_url + })) - bp.plugins:insert({ - name = "request-aware-table", - route = { id = route.id }, - }) + local route = bp.routes:insert({ + service = service, + paths = { "/" } + }) - helpers.start_kong({ - database = strategy, - plugins = "bundled, request-aware-table", - nginx_conf = "spec/fixtures/custom_nginx.template", - }) - end) + bp.plugins:insert({ + name = "request-aware-table", + route = { id = route.id }, + }) - lazy_teardown(function() - helpers.stop_kong() - end) + helpers.start_kong({ + database = strategy, + plugins = "bundled, request-aware-table", + nginx_conf = "spec/fixtures/custom_nginx.template", + log_level = log_level, + }) + end) - before_each(function() - helpers.clean_logfile() - client = helpers.proxy_client() - clear_table(client, checks) - end) + lazy_teardown(function() + helpers.stop_kong() + end) - after_each(function() - if client then - client:close() - end - end) + before_each(function() + helpers.clean_logfile() + client = helpers.proxy_client() + clear_table(client) + end) + + after_each(function() + if client then + client:close() + end + end) - describe("with concurrency check enabled", function() it("allows access when there are no race conditions", function() - local res = client:get("/", { - query = { - checks = checks, - } - }) + local res = client:get("/") assert.response(res).has.status(200) assert.logfile().has.no.line("[error]", true) end) it("denies access when there are race conditions and checks are enabled", function() -- access from request 1 (don't clear) - local ok, r = pcall(client.get, client, "/", { - query = { - checks = checks, - }, - }) + local ok, r = pcall(client.get, client, "/") assert(ok) assert.response(r).has.status(200) -- access from request 2 - ok, r = pcall(client.get, client, "/", { - query = { - checks = checks, - }, - }) - if checks then + ok, r = pcall(client.get, client, "/") + if concurrency_checks then assert(not ok) assert.logfile().has.line("race condition detected", true) else @@ -100,22 +95,16 @@ for _, strategy in helpers.each_strategy() do -- access from request 1 (clear) local r = client:get("/", { query = { - checks = checks, clear = true, }, }) assert.response(r).has.status(200) -- access from request 2 - r = client:get("/", { - query = { - checks = checks, - }, - }) + r = client:get("/") assert.response(r).has.status(200) assert.logfile().has.no.line("[error]", true) end) end) - end) -end + end end \ No newline at end of file diff --git a/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua index c51c6aa29b6c..e64c7cabe9f1 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua @@ -1,7 +1,6 @@ local RAT = require "kong.tools.request_aware_table" local get_phase = ngx.get_phase -local ngx = ngx local kong = kong @@ -10,29 +9,24 @@ local _M = { VERSION = "1.0", } -local checks_tab = RAT.new({}, "on") -local no_checks_tab = RAT.new({}, "off") +local tab = RAT.new({}) local function access_tables() local query = kong.request.get_query() - local tab - - if query.checks ~= "false" then - tab = checks_tab - else - tab = no_checks_tab - end if query.clear == "true" and get_phase() == "access" then + -- clear before access tab.clear() end -- write access tab.foo = "bar" - -- read access - ngx.log(ngx.DEBUG, "accessing to tab.foo" .. tab.foo) + tab.bar = "baz" + -- read/write access + tab.baz = tab.foo .. tab.bar if query.clear == "true" and get_phase() == "body_filter" then + -- clear after access tab.clear() end end From 000eb115afb66761fb12957166df0e5c1f7bf0c5 Mon Sep 17 00:00:00 2001 From: samugi Date: Thu, 7 Sep 2023 15:24:48 +0200 Subject: [PATCH 4/9] feat(tools): request-aware-table pr feedback * refactor * only use static log level to turn concurrency checks on or off * use single metatable, keep data and request_id in the instance * use table.new -style initialization * reintroduce :clear() method Co-authored-by: Datong Sun --- .../{11017.yaml => request-aware-table.yml} | 2 - kong/tools/request_aware_table.lua | 151 ++++++++++-------- spec/01-unit/28-request-aware-table_spec.lua | 51 ------ ...ec.lua => 33-request-aware-table_spec.lua} | 31 ++-- .../plugins/request-aware-table/handler.lua | 41 ++--- 5 files changed, 126 insertions(+), 150 deletions(-) rename changelog/unreleased/kong/{11017.yaml => request-aware-table.yml} (88%) delete mode 100644 spec/01-unit/28-request-aware-table_spec.lua rename spec/02-integration/05-proxy/{31-request-aware-table_spec.lua => 33-request-aware-table_spec.lua} (83%) diff --git a/changelog/unreleased/kong/11017.yaml b/changelog/unreleased/kong/request-aware-table.yml similarity index 88% rename from changelog/unreleased/kong/11017.yaml rename to changelog/unreleased/kong/request-aware-table.yml index 6ce700d75fab..96cf070b5d0d 100644 --- a/changelog/unreleased/kong/11017.yaml +++ b/changelog/unreleased/kong/request-aware-table.yml @@ -1,5 +1,3 @@ message: Add a request-aware table able to detect accesses from different requests. type: feature scope: Core -prs: - - 11017 diff --git a/kong/tools/request_aware_table.lua b/kong/tools/request_aware_table.lua index 8226fe3cf9f8..c03e6554d25a 100644 --- a/kong/tools/request_aware_table.lua +++ b/kong/tools/request_aware_table.lua @@ -1,93 +1,118 @@ -local table_clear = require "table.clear" +--- NOTE: tool is designed to assist with **detecting** request contamination +-- issues on CI, during test runs. It does not offer security safeguards. -local LOG_LEVELS = require "kong.constants".LOG_LEVELS -local get_log_level = require("resty.kong.log").get_log_level +local table_new = require "table.new" +local table_clear = require "table.clear" -local get_phase = ngx.get_phase -local fmt = string.format -local ngx_var = ngx.var +local debug_mode = kong.configuration.log_level == "debug" +local get_phase = ngx.get_phase +local var = ngx.var +local log = ngx.log local NGX_VAR_PHASES = { - set = true, - rewrite = true, - balancer = true, - access = true, - content = true, + set = true, + rewrite = true, + access = true, + content = true, header_filter = true, - body_filter = true, - log = true, + body_filter = true, + log = true, + balancer = true, } +-- Check if access is allowed for table, based on the request ID +local function enforce_sequential_access(table) + if not NGX_VAR_PHASES[get_phase()] then + -- not in a request context: allow access and reset allowed request ID + rawset(table, "__allowed_request_id", nil) + return + end + + local curr_request_id = var.request_id + local allowed_request_id = rawget(table, "__allowed_request_id") + if not allowed_request_id then + -- first access. Set allowed request ID and allow access + rawset(table, "__allowed_request_id", curr_request_id) + return + end -local function is_debug_mode() - local log_level = get_log_level(LOG_LEVELS[kong.configuration.log_level]) - return log_level == LOG_LEVELS.debug + if curr_request_id ~= table.__allowed_request_id then + error("race condition detected; access to table forbidden", 2) + end end ---- Request aware table constructor --- Wraps an existing table (or creates a new one) with request-aware access --- logic to protect the underlying data from race conditions. --- @param data_table (optional) The target table to use as underlying data --- @return The newly created table with request-aware access -local function new(data_table) - if data_table and type(data_table) ~= "table" then - error("data_table must be a table", 2) +local function clear_table(self) + if debug_mode == false then + table_clear(self) + return end - local allowed_request_id - local proxy = {} - local data = data_table or {} + table_clear(self.__data) + rawset(self, "__allowed_request_id", nil) +end - -- Check if access is allowed based on the request ID - local function enforce_sequential_access() - local curr_phase = get_phase() - if not NGX_VAR_PHASES[curr_phase] then - error(fmt("cannot enforce sequential access in %s phase", curr_phase), 2) - end - local curr_request_id = ngx_var.request_id +local __proxy_mt = { + __index = function(t, k) + if k == "clear" then + return clear_table + end - allowed_request_id = allowed_request_id or curr_request_id + enforce_sequential_access(t) + return t.__data[k] + end, - if curr_request_id ~= allowed_request_id then - error("race condition detected; access to table forbidden", 2) + __newindex = function(t, k, v) + if k == "clear" then + log(ngx.WARN, "cannot overwrite 'clear' method") + return end - end - --- Clear data table - -- @tparam function fn (optional) An optional function to use instead - -- of `table.clear` to clear the data table - function proxy.clear(fn) - if fn then - fn(data) + enforce_sequential_access(t) + t.__data[k] = v + end, +} + - else - table_clear(data) +local __direct_mt = { + __index = { clear = clear_table }, + + __newindex = function(t, k, v) + if k == "clear" then + log(ngx.WARN, "cannot overwrite 'clear' method") + return end - allowed_request_id = nil - end + rawset(t, k, v) + end, +} - local _proxy_mt = { - __index = function(_, k) - if is_debug_mode() then - enforce_sequential_access() - end - return data[k] - end, +-- Request aware table constructor +-- +-- Creates a new table with request-aware access logic to protect the +-- underlying data from race conditions. +-- The table includes a :clear() method to delete all elements. +-- +-- The request-aware access logic is turned off when `debug_mode` is disabled. +-- +-- @param narr (optional) pre allocated array elements +-- @param nrec (optional) pre allocated hash elements +-- @return The newly created table with request-aware access +local function new(narr, nrec) + if not narr then narr = 0 end + if not nrec then nrec = 0 end - __newindex = function(_, k, v) - if is_debug_mode() then - enforce_sequential_access() - end + local data = table_new(narr, nrec) - data[k] = v - end - } + -- return table without proxy when debug_mode is disabled + if debug_mode == false then + return setmetatable(data, __direct_mt) + end - return setmetatable(proxy, _proxy_mt) + -- wrap table in proxy (for access checks) when debug_mode is enabled + return setmetatable({ __data = data }, __proxy_mt) end return { diff --git a/spec/01-unit/28-request-aware-table_spec.lua b/spec/01-unit/28-request-aware-table_spec.lua deleted file mode 100644 index 89c96fd3e554..000000000000 --- a/spec/01-unit/28-request-aware-table_spec.lua +++ /dev/null @@ -1,51 +0,0 @@ -local tablex = require "pl.tablex" - -local rat - -describe("Request aware table", function() - local orig_t = {} - local old_ngx - local tab - - lazy_setup(function() - old_ngx = ngx - _G.ngx.get_phase = function() return "access" end - _G.ngx.var = {} - _G.kong = { - configuration = { - log_level = "debug", - }, - } - rat = require "kong.tools.request_aware_table" - end) - - lazy_teardown(function() - _G.ngx = old_ngx - end) - - before_each(function() - tab = rat.new(orig_t) - end) - - it("allows defining a custom clear function", function() - orig_t.persist = "persistent_value" - orig_t.foo = "bar" - orig_t.baz = "qux" - - -- custom clear function that keeps persistent_value - tab.clear(function(t) - for k in pairs(t) do - if k ~= "persist" then - t[k] = nil - end - end - end) - - -- confirm persistent_value is the only key left - assert.same(1, tablex.size(orig_t)) - assert.equal("persistent_value", tab.persist) - -- clear the whole table and confirm it's empty - tab.clear() - assert.same(0, tablex.size(orig_t)) - end) -end) diff --git a/spec/02-integration/05-proxy/31-request-aware-table_spec.lua b/spec/02-integration/05-proxy/33-request-aware-table_spec.lua similarity index 83% rename from spec/02-integration/05-proxy/31-request-aware-table_spec.lua rename to spec/02-integration/05-proxy/33-request-aware-table_spec.lua index 42d4a80b88cf..fbe25539355c 100644 --- a/spec/02-integration/05-proxy/31-request-aware-table_spec.lua +++ b/spec/02-integration/05-proxy/33-request-aware-table_spec.lua @@ -7,14 +7,17 @@ local LOG_LEVELS = { } -local function clear_table(client) +local function new_table() + local client = helpers.proxy_client() local res = client:get("/", { query = { + new_tab = true, clear = true, } }) assert.response(res).has.status(200) assert.logfile().has.no.line("[error]", true) + client:close() end for _, log_level in ipairs(LOG_LEVELS) do @@ -60,8 +63,8 @@ for _, log_level in ipairs(LOG_LEVELS) do before_each(function() helpers.clean_logfile() + new_table() client = helpers.proxy_client() - clear_table(client) end) after_each(function() @@ -75,32 +78,40 @@ for _, log_level in ipairs(LOG_LEVELS) do assert.response(res).has.status(200) assert.logfile().has.no.line("[error]", true) end) - it("denies access when there are race conditions and checks are enabled", function() + + it("denies access when there are race conditions and checks are enabled (else allows)", function() -- access from request 1 (don't clear) - local ok, r = pcall(client.get, client, "/") - assert(ok) + local r = client:get("/") assert.response(r).has.status(200) -- access from request 2 - ok, r = pcall(client.get, client, "/") + r = client:get("/") if concurrency_checks then - assert(not ok) assert.logfile().has.line("race condition detected", true) else - assert(ok) assert.response(r).has.status(200) end end) + it("allows access when table is cleared between requests", function() -- access from request 1 (clear) local r = client:get("/", { query = { clear = true, - }, + } }) assert.response(r).has.status(200) - -- access from request 2 + -- access from request 2 (clear) + r = client:get("/", { + query = { + clear = true, + } + }) + assert.response(r).has.status(200) + assert.logfile().has.no.line("[error]", true) + + -- access from request 3 r = client:get("/") assert.response(r).has.status(200) assert.logfile().has.no.line("[error]", true) diff --git a/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua index e64c7cabe9f1..25966373fc70 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua @@ -1,46 +1,39 @@ local RAT = require "kong.tools.request_aware_table" -local get_phase = ngx.get_phase local kong = kong - +local tab local _M = { PRIORITY = 1001, VERSION = "1.0", } -local tab = RAT.new({}) - -local function access_tables() - local query = kong.request.get_query() - - if query.clear == "true" and get_phase() == "access" then - -- clear before access - tab.clear() - end - +local function access_table() -- write access tab.foo = "bar" tab.bar = "baz" -- read/write access tab.baz = tab.foo .. tab.bar - - if query.clear == "true" and get_phase() == "body_filter" then - -- clear after access - tab.clear() - end end + function _M:access(conf) - access_tables() -end + local query = kong.request.get_query() + if query.new_tab == "true" then + -- new table + tab = RAT.new() + end -function _M:header_filter(conf) - access_tables() -end + -- access multiple times during same request + access_table() + access_table() + access_table() -function _M:body_filter(conf) - access_tables() + if query.clear == "true" then + -- clear table + tab:clear() + end end + return _M From 3604e3d270af9583ae5a62beb6f99e9851615e2c Mon Sep 17 00:00:00 2001 From: samugi Date: Wed, 27 Sep 2023 12:55:08 +0200 Subject: [PATCH 5/9] feat(request-aware-table): add to rate-limiting --- kong/plugins/rate-limiting/policies/init.lua | 41 +++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/kong/plugins/rate-limiting/policies/init.lua b/kong/plugins/rate-limiting/policies/init.lua index 12f9f32983e8..80731cb0648b 100644 --- a/kong/plugins/rate-limiting/policies/init.lua +++ b/kong/plugins/rate-limiting/policies/init.lua @@ -2,7 +2,7 @@ local policy_cluster = require "kong.plugins.rate-limiting.policies.cluster" local timestamp = require "kong.tools.timestamp" local reports = require "kong.reports" local redis = require "resty.redis" -local table_clear = require "table.clear" +local request_aware_table = require "kong.tools.request_aware_table" local kong = kong local pairs = pairs @@ -18,23 +18,26 @@ local EMPTY_UUID = "00000000-0000-0000-0000-000000000000" -- for `conf.sync_rate > 0` local auto_sync_timer -local cur_usage = { - --[[ - [cache_key] = - --]] -} +local cur_usage = request_aware_table.new() +-- { +-- [[ +-- [cache_key] = +-- ]] +-- } -local cur_usage_expire_at = { - --[[ - [cache_key] = - --]] -} +local cur_usage_expire_at = request_aware_table.new() +-- { +-- [[ +-- [cache_key] = +-- ]] +-- } -local cur_delta = { - --[[ - [cache_key] = - --]] -} +local cur_delta = request_aware_table.new() +-- { +-- [[ +-- [cache_key] = +-- ]] +-- } local function is_present(str) @@ -138,9 +141,9 @@ local function get_redis_connection(conf) end local function clear_local_counter() - table_clear(cur_usage) - table_clear(cur_usage_expire_at) - table_clear(cur_delta) + cur_usage:clear() + cur_usage_expire_at:clear() + cur_delta:clear() end local function sync_to_redis(premature, conf) From bb1c7eb90176dd85d5d977eea9be9f607d623eca Mon Sep 17 00:00:00 2001 From: Datong Sun Date: Fri, 29 Sep 2023 07:17:05 +0800 Subject: [PATCH 6/9] Update request_aware_table.lua --- kong/tools/request_aware_table.lua | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/kong/tools/request_aware_table.lua b/kong/tools/request_aware_table.lua index c03e6554d25a..cd6bff4e4b10 100644 --- a/kong/tools/request_aware_table.lua +++ b/kong/tools/request_aware_table.lua @@ -5,10 +5,12 @@ local table_new = require "table.new" local table_clear = require "table.clear" local debug_mode = kong.configuration.log_level == "debug" +local error = error local get_phase = ngx.get_phase local var = ngx.var local log = ngx.log + local NGX_VAR_PHASES = { set = true, rewrite = true, @@ -65,8 +67,7 @@ local __proxy_mt = { __newindex = function(t, k, v) if k == "clear" then - log(ngx.WARN, "cannot overwrite 'clear' method") - return + error("cannot set the 'clear' method of request aware table", 2) end enforce_sequential_access(t) @@ -80,8 +81,7 @@ local __direct_mt = { __newindex = function(t, k, v) if k == "clear" then - log(ngx.WARN, "cannot overwrite 'clear' method") - return + error("cannot set the 'clear' method of request aware table", 2) end rawset(t, k, v) From cc9b6d830451500c93ed15b4c5718a501c8e24b6 Mon Sep 17 00:00:00 2001 From: Datong Sun Date: Fri, 29 Sep 2023 07:19:13 +0800 Subject: [PATCH 7/9] Update request_aware_table.lua --- kong/tools/request_aware_table.lua | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/kong/tools/request_aware_table.lua b/kong/tools/request_aware_table.lua index cd6bff4e4b10..bb13f17d2359 100644 --- a/kong/tools/request_aware_table.lua +++ b/kong/tools/request_aware_table.lua @@ -4,11 +4,12 @@ local table_new = require "table.new" local table_clear = require "table.clear" -local debug_mode = kong.configuration.log_level == "debug" -local error = error -local get_phase = ngx.get_phase -local var = ngx.var -local log = ngx.log +local debug_mode = kong.configuration.log_level == "debug" +local error = error +local rawset = rawset +local setmetatable = setmetatable +local get_phase = ngx.get_phase +local var = ngx.var local NGX_VAR_PHASES = { From b98fa423bb21efd02102ceac9e974514a608e385 Mon Sep 17 00:00:00 2001 From: samugi Date: Wed, 4 Oct 2023 17:35:17 +0200 Subject: [PATCH 8/9] feat(request-aware-table): PR feedback * small refactors here and there --- kong/tools/request_aware_table.lua | 14 ++++---- .../05-proxy/33-request-aware-table_spec.lua | 32 +++++++++++-------- .../plugins/request-aware-table/handler.lua | 12 ++++--- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/kong/tools/request_aware_table.lua b/kong/tools/request_aware_table.lua index bb13f17d2359..104723010ad5 100644 --- a/kong/tools/request_aware_table.lua +++ b/kong/tools/request_aware_table.lua @@ -22,24 +22,26 @@ local NGX_VAR_PHASES = { log = true, balancer = true, } +local ALLOWED_REQUEST_ID_K = "__allowed_request_id" + -- Check if access is allowed for table, based on the request ID local function enforce_sequential_access(table) if not NGX_VAR_PHASES[get_phase()] then - -- not in a request context: allow access and reset allowed request ID - rawset(table, "__allowed_request_id", nil) + -- allow access and reset allowed request ID + rawset(table, ALLOWED_REQUEST_ID_K, nil) return end local curr_request_id = var.request_id - local allowed_request_id = rawget(table, "__allowed_request_id") + local allowed_request_id = rawget(table, ALLOWED_REQUEST_ID_K) if not allowed_request_id then -- first access. Set allowed request ID and allow access - rawset(table, "__allowed_request_id", curr_request_id) + rawset(table, ALLOWED_REQUEST_ID_K, curr_request_id) return end - if curr_request_id ~= table.__allowed_request_id then + if curr_request_id ~= table[ALLOWED_REQUEST_ID_K] then error("race condition detected; access to table forbidden", 2) end end @@ -52,7 +54,7 @@ local function clear_table(self) end table_clear(self.__data) - rawset(self, "__allowed_request_id", nil) + rawset(self, ALLOWED_REQUEST_ID_K, nil) end diff --git a/spec/02-integration/05-proxy/33-request-aware-table_spec.lua b/spec/02-integration/05-proxy/33-request-aware-table_spec.lua index fbe25539355c..604fae57fb65 100644 --- a/spec/02-integration/05-proxy/33-request-aware-table_spec.lua +++ b/spec/02-integration/05-proxy/33-request-aware-table_spec.lua @@ -7,11 +7,22 @@ local LOG_LEVELS = { } -local function new_table() +local function trigger_plugin_new_table() local client = helpers.proxy_client() local res = client:get("/", { query = { new_tab = true, + } + }) + assert.response(res).has.status(200) + assert.logfile().has.no.line("[error]", true) + client:close() +end + +local function trigger_plugin_clear_table() + local client = helpers.proxy_client() + local res = client:get("/", { + query = { clear = true, } }) @@ -63,7 +74,7 @@ for _, log_level in ipairs(LOG_LEVELS) do before_each(function() helpers.clean_logfile() - new_table() + trigger_plugin_new_table() client = helpers.proxy_client() end) @@ -95,21 +106,14 @@ for _, log_level in ipairs(LOG_LEVELS) do it("allows access when table is cleared between requests", function() -- access from request 1 (clear) - local r = client:get("/", { - query = { - clear = true, - } - }) + local r = client:get("/") assert.response(r).has.status(200) + trigger_plugin_clear_table() -- access from request 2 (clear) - r = client:get("/", { - query = { - clear = true, - } - }) + r = client:get("/") assert.response(r).has.status(200) - assert.logfile().has.no.line("[error]", true) + trigger_plugin_clear_table() -- access from request 3 r = client:get("/") @@ -118,4 +122,4 @@ for _, log_level in ipairs(LOG_LEVELS) do end) end) end -end \ No newline at end of file +end diff --git a/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua index 25966373fc70..a3c640cfeca8 100644 --- a/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua +++ b/spec/fixtures/custom_plugins/kong/plugins/request-aware-table/handler.lua @@ -22,16 +22,18 @@ function _M:access(conf) if query.new_tab == "true" then -- new table tab = RAT.new() + ngx.exit(200) end - -- access multiple times during same request - access_table() - access_table() - access_table() - if query.clear == "true" then -- clear table tab:clear() + ngx.exit(200) + end + + -- access multiple times during same request + for _ = 1, 3 do + access_table() end end From e17992f24194be831f30c4808aa7ee8c3ccfe303 Mon Sep 17 00:00:00 2001 From: Datong Sun Date: Wed, 11 Oct 2023 20:42:52 -0700 Subject: [PATCH 9/9] fix styles --- kong/tools/request_aware_table.lua | 8 ++++---- .../05-proxy/33-request-aware-table_spec.lua | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/kong/tools/request_aware_table.lua b/kong/tools/request_aware_table.lua index 104723010ad5..fd9f2ad90356 100644 --- a/kong/tools/request_aware_table.lua +++ b/kong/tools/request_aware_table.lua @@ -4,7 +4,7 @@ local table_new = require "table.new" local table_clear = require "table.clear" -local debug_mode = kong.configuration.log_level == "debug" +local debug_mode = (kong.configuration.log_level == "debug") local error = error local rawset = rawset local setmetatable = setmetatable @@ -42,13 +42,13 @@ local function enforce_sequential_access(table) end if curr_request_id ~= table[ALLOWED_REQUEST_ID_K] then - error("race condition detected; access to table forbidden", 2) + error("concurrent access from different request to shared table detected", 2) end end local function clear_table(self) - if debug_mode == false then + if not debug_mode then table_clear(self) return end @@ -110,7 +110,7 @@ local function new(narr, nrec) local data = table_new(narr, nrec) -- return table without proxy when debug_mode is disabled - if debug_mode == false then + if not debug_mode then return setmetatable(data, __direct_mt) end diff --git a/spec/02-integration/05-proxy/33-request-aware-table_spec.lua b/spec/02-integration/05-proxy/33-request-aware-table_spec.lua index 604fae57fb65..bbce6e3079ca 100644 --- a/spec/02-integration/05-proxy/33-request-aware-table_spec.lua +++ b/spec/02-integration/05-proxy/33-request-aware-table_spec.lua @@ -98,7 +98,7 @@ for _, log_level in ipairs(LOG_LEVELS) do -- access from request 2 r = client:get("/") if concurrency_checks then - assert.logfile().has.line("race condition detected", true) + assert.logfile().has.line("concurrent access from different request to shared table detected", true) else assert.response(r).has.status(200) end