From de050894b6bb34c83a5a5a0f61219a385eedc1fb Mon Sep 17 00:00:00 2001 From: Qi Date: Wed, 2 Mar 2022 14:57:04 +0800 Subject: [PATCH 1/9] fix(plugin) properly handle the `ngx.null` for `limits.{limit_name}.*` --- kong/plugins/response-ratelimiting/access.lua | 30 ++++++++++--------- .../01-schema_spec.lua | 7 +++++ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/kong/plugins/response-ratelimiting/access.lua b/kong/plugins/response-ratelimiting/access.lua index 00078502c936..a5aff356e5f6 100644 --- a/kong/plugins/response-ratelimiting/access.lua +++ b/kong/plugins/response-ratelimiting/access.lua @@ -36,21 +36,23 @@ local function get_usage(conf, identifier, limits, current_timestamp) for k, v in pairs(limits) do -- Iterate over limit names for lk, lv in pairs(v) do -- Iterare over periods - local current_usage, err = policies[conf.policy].usage(conf, identifier, k, lk, current_timestamp) - if err then - return nil, err + if lv ~= ngx.null then + local current_usage, err = policies[conf.policy].usage(conf, identifier, k, lk, current_timestamp) + if err then + return nil, err + end + + if not usage[k] then + usage[k] = {} + end + + if not usage[k][lk] then + usage[k][lk] = {} + end + + usage[k][lk].limit = lv + usage[k][lk].remaining = lv - current_usage end - - if not usage[k] then - usage[k] = {} - end - - if not usage[k][lk] then - usage[k][lk] = {} - end - - usage[k][lk].limit = lv - usage[k][lk].remaining = lv - current_usage end end diff --git a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua index 9b07aa41bd41..1785f5af6714 100644 --- a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua +++ b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua @@ -1,5 +1,6 @@ local schema_def = require "kong.plugins.response-ratelimiting.schema" local v = require("spec.helpers").validate_plugin_config_schema +local null = ngx.null describe("Plugin: response-rate-limiting (schema)", function() @@ -32,6 +33,12 @@ describe("Plugin: response-rate-limiting (schema)", function() assert.falsy(ok) assert.equal("unknown field", err.config.limits.seco) end) + it("limits: \'null\' value", function() + -- https://github.com/Kong/kong/issues/8314 + local config = {limits = {video = {second = null, minute = 1}}} + local ok, err = v(config, schema_def) + assert.truthy(ok) + end) it("limits: smaller unit is less than bigger unit", function() local config = {limits = {video = {second = 2, minute = 1}}} local ok, err = v(config, schema_def) From 42c33fe454ea4ff118f7f89c9dd17233ee610186 Mon Sep 17 00:00:00 2001 From: Qi Date: Wed, 2 Mar 2022 15:13:04 +0800 Subject: [PATCH 2/9] lint fix --- spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua index 1785f5af6714..df863a30c5d3 100644 --- a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua +++ b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua @@ -38,6 +38,7 @@ describe("Plugin: response-rate-limiting (schema)", function() local config = {limits = {video = {second = null, minute = 1}}} local ok, err = v(config, schema_def) assert.truthy(ok) + assert.falsy(err) end) it("limits: smaller unit is less than bigger unit", function() local config = {limits = {video = {second = 2, minute = 1}}} From a9d145b582642d86009723dcecd48bb15723583d Mon Sep 17 00:00:00 2001 From: Qi Date: Wed, 2 Mar 2022 16:21:07 +0800 Subject: [PATCH 3/9] Update spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua Co-authored-by: Datong Sun --- spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua index df863a30c5d3..465db9f4c771 100644 --- a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua +++ b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua @@ -33,7 +33,7 @@ describe("Plugin: response-rate-limiting (schema)", function() assert.falsy(ok) assert.equal("unknown field", err.config.limits.seco) end) - it("limits: \'null\' value", function() + it("limits: \'null\' value does not cause 500, issue #8314", function() -- https://github.com/Kong/kong/issues/8314 local config = {limits = {video = {second = null, minute = 1}}} local ok, err = v(config, schema_def) From 04679b7f343ad3e7c1305a8283e42cc522fcd727 Mon Sep 17 00:00:00 2001 From: Qi Date: Wed, 2 Mar 2022 16:21:15 +0800 Subject: [PATCH 4/9] Update spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua Co-authored-by: Datong Sun --- spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua index 465db9f4c771..9455b197035e 100644 --- a/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua +++ b/spec/03-plugins/24-response-rate-limiting/01-schema_spec.lua @@ -34,7 +34,6 @@ describe("Plugin: response-rate-limiting (schema)", function() assert.equal("unknown field", err.config.limits.seco) end) it("limits: \'null\' value does not cause 500, issue #8314", function() - -- https://github.com/Kong/kong/issues/8314 local config = {limits = {video = {second = null, minute = 1}}} local ok, err = v(config, schema_def) assert.truthy(ok) From da413f1c79dcb705e002c69e6c04da040aa13eb6 Mon Sep 17 00:00:00 2001 From: Qi Date: Wed, 2 Mar 2022 16:23:15 +0800 Subject: [PATCH 5/9] cache ngx.null at module level. --- kong/plugins/response-ratelimiting/access.lua | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/kong/plugins/response-ratelimiting/access.lua b/kong/plugins/response-ratelimiting/access.lua index a5aff356e5f6..2e8631d8d4d2 100644 --- a/kong/plugins/response-ratelimiting/access.lua +++ b/kong/plugins/response-ratelimiting/access.lua @@ -9,6 +9,9 @@ local error = error local tostring = tostring +local null = ngx.null + + local EMPTY = {} local HTTP_TOO_MANY_REQUESTS = 429 local RATELIMIT_REMAINING = "X-RateLimit-Remaining" @@ -36,7 +39,7 @@ local function get_usage(conf, identifier, limits, current_timestamp) for k, v in pairs(limits) do -- Iterate over limit names for lk, lv in pairs(v) do -- Iterare over periods - if lv ~= ngx.null then + if lv ~= null then local current_usage, err = policies[conf.policy].usage(conf, identifier, k, lk, current_timestamp) if err then return nil, err From 495ce48765d2786c95ef0a09cf467660bf623f79 Mon Sep 17 00:00:00 2001 From: Qi Date: Mon, 7 Mar 2022 09:49:37 +0800 Subject: [PATCH 6/9] fix function `adjust_field_for_context` --- kong/db/schema/init.lua | 12 ++++++-- kong/plugins/response-ratelimiting/access.lua | 30 +++++++++---------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/kong/db/schema/init.lua b/kong/db/schema/init.lua index 7b642ab21030..0c9705f3939d 100644 --- a/kong/db/schema/init.lua +++ b/kong/db/schema/init.lua @@ -26,7 +26,6 @@ local update_time = ngx.update_time local ngx_time = ngx.time local ngx_now = ngx.now local find = string.find -local null = ngx.null local max = math.max local sub = string.sub @@ -1578,8 +1577,15 @@ local function adjust_field_for_context(field, value, context, nulls, opts) end if subfield then - for i = 1, #value do - value[i] = adjust_field_for_context(subfield, value[i], context, nulls, opts) + if field.type ~= "map" then + for i = 1, #value do + value[i] = adjust_field_for_context(subfield, value[i], context, nulls, opts) + end + + else + for k, _ in pairs(value) do + value[k] = adjust_field_for_context(subfield, value[k], context, nulls, opts) + end end end end diff --git a/kong/plugins/response-ratelimiting/access.lua b/kong/plugins/response-ratelimiting/access.lua index 2e8631d8d4d2..e57794e67913 100644 --- a/kong/plugins/response-ratelimiting/access.lua +++ b/kong/plugins/response-ratelimiting/access.lua @@ -39,23 +39,21 @@ local function get_usage(conf, identifier, limits, current_timestamp) for k, v in pairs(limits) do -- Iterate over limit names for lk, lv in pairs(v) do -- Iterare over periods - if lv ~= null then - local current_usage, err = policies[conf.policy].usage(conf, identifier, k, lk, current_timestamp) - if err then - return nil, err - end - - if not usage[k] then - usage[k] = {} - end - - if not usage[k][lk] then - usage[k][lk] = {} - end - - usage[k][lk].limit = lv - usage[k][lk].remaining = lv - current_usage + local current_usage, err = policies[conf.policy].usage(conf, identifier, k, lk, current_timestamp) + if err then + return nil, err end + + if not usage[k] then + usage[k] = {} + end + + if not usage[k][lk] then + usage[k][lk] = {} + end + + usage[k][lk].limit = lv + usage[k][lk].remaining = lv - current_usage end end From 777d6823717543c1b3ca132f2d79973fd3faf11f Mon Sep 17 00:00:00 2001 From: Qi Date: Mon, 7 Mar 2022 09:53:45 +0800 Subject: [PATCH 7/9] roll back one line --- kong/db/schema/init.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/kong/db/schema/init.lua b/kong/db/schema/init.lua index 0c9705f3939d..9f96b66146ba 100644 --- a/kong/db/schema/init.lua +++ b/kong/db/schema/init.lua @@ -26,6 +26,7 @@ local update_time = ngx.update_time local ngx_time = ngx.time local ngx_now = ngx.now local find = string.find +local null = ngx.null local max = math.max local sub = string.sub From 00b95653d607fae9cdec9bfbc25f346b5c608585 Mon Sep 17 00:00:00 2001 From: Qi Date: Mon, 7 Mar 2022 10:00:25 +0800 Subject: [PATCH 8/9] fix lint --- kong/plugins/response-ratelimiting/access.lua | 3 --- 1 file changed, 3 deletions(-) diff --git a/kong/plugins/response-ratelimiting/access.lua b/kong/plugins/response-ratelimiting/access.lua index e57794e67913..00078502c936 100644 --- a/kong/plugins/response-ratelimiting/access.lua +++ b/kong/plugins/response-ratelimiting/access.lua @@ -9,9 +9,6 @@ local error = error local tostring = tostring -local null = ngx.null - - local EMPTY = {} local HTTP_TOO_MANY_REQUESTS = 429 local RATELIMIT_REMAINING = "X-RateLimit-Remaining" From b8b0b5afc590d40cba74199a9aa577dabbea7d5d Mon Sep 17 00:00:00 2001 From: Qi Date: Wed, 9 Mar 2022 20:29:44 +0800 Subject: [PATCH 9/9] Update kong/db/schema/init.lua Co-authored-by: Aapo Talvensaari --- kong/db/schema/init.lua | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kong/db/schema/init.lua b/kong/db/schema/init.lua index 9f96b66146ba..23368a817b64 100644 --- a/kong/db/schema/init.lua +++ b/kong/db/schema/init.lua @@ -1584,8 +1584,8 @@ local function adjust_field_for_context(field, value, context, nulls, opts) end else - for k, _ in pairs(value) do - value[k] = adjust_field_for_context(subfield, value[k], context, nulls, opts) + for k, v in pairs(value) do + value[k] = adjust_field_for_context(subfield, v, context, nulls, opts) end end end