From b1ad9688dd83b672d18fdc4305850f43b02da574 Mon Sep 17 00:00:00 2001 From: janiussyafiq Date: Mon, 22 Jun 2026 09:45:30 +0800 Subject: [PATCH 1/2] fix(redis-schema): deepcopy base schema instead of mutating shared tables limit-conn's key_ttl schema variants were built by mutating the shared policy_to_additional_properties tables in place, leaking key_ttl (and its default of 3600) into every other consumer of apisix/utils/redis-schema.lua: limit-count and limit-req confs silently gained a key_ttl = 3600 field during validation. Build the limit-conn variants from core.table.deepcopy of the shared base so the extra property stays local to limit-conn. deepcopy matches the schema-derive idiom already used by limit-count and ai-cache, and yields a fully independent tree so future nested edits cannot leak back into the shared base. Add t/utils/redis-schema.t: one block loads limit-conn first and asserts limit-count/limit-req confs across redis and redis-cluster do not gain a key_ttl; a second block pins limit-conn's own behavior (default 3600 applied, explicit value honored). Signed-off-by: janiussyafiq --- apisix/utils/redis-schema.lua | 8 +- t/utils/redis-schema.t | 154 ++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 t/utils/redis-schema.t diff --git a/apisix/utils/redis-schema.lua b/apisix/utils/redis-schema.lua index 0a884ca81620..991919a2a9d5 100644 --- a/apisix/utils/redis-schema.lua +++ b/apisix/utils/redis-schema.lua @@ -15,6 +15,8 @@ -- limitations under the License. -- +local core = require("apisix.core") + local policy_to_additional_properties = { redis = { properties = { @@ -86,12 +88,14 @@ local policy_to_additional_properties = { }, } -local limit_conn_redis_cluster_schema = policy_to_additional_properties["redis-cluster"] +local limit_conn_redis_cluster_schema = + core.table.deepcopy(policy_to_additional_properties["redis-cluster"]) limit_conn_redis_cluster_schema.properties.key_ttl = { type = "integer", default = 3600, } -local limit_conn_redis_schema = policy_to_additional_properties["redis"] +local limit_conn_redis_schema = + core.table.deepcopy(policy_to_additional_properties["redis"]) limit_conn_redis_schema.properties.key_ttl = { type = "integer", default = 3600, } diff --git a/t/utils/redis-schema.t b/t/utils/redis-schema.t new file mode 100644 index 000000000000..493aedcb9446 --- /dev/null +++ b/t/utils/redis-schema.t @@ -0,0 +1,154 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); + +add_block_preprocessor(sub { + my ($block) = @_; + + if (!defined $block->request) { + $block->set_value("request", "GET /t"); + } + +}); + +run_tests(); + +__DATA__ + +=== TEST 1: limit-count/limit-req validation must not pick up limit-conn's key_ttl +--- config + location /t { + content_by_lua_block { + -- limit-conn extends the shared redis/redis-cluster schemas with + -- key_ttl. Load it first so that any leak of key_ttl into the + -- shared tables of apisix.utils.redis-schema would be visible + -- to the other rate limiting plugins below. + require("apisix.plugins.limit-conn") + + local cases = { + { + plugin = "limit-count", + conf = { + count = 2, + time_window = 60, + policy = "redis", + redis_host = "127.0.0.1", + }, + }, + { + plugin = "limit-count", + conf = { + count = 2, + time_window = 60, + policy = "redis-cluster", + redis_cluster_nodes = {"127.0.0.1:5000", "127.0.0.1:5001"}, + redis_cluster_name = "redis-cluster-1", + }, + }, + { + plugin = "limit-req", + conf = { + rate = 1, + burst = 0, + key = "remote_addr", + policy = "redis", + redis_host = "127.0.0.1", + }, + }, + { + plugin = "limit-req", + conf = { + rate = 1, + burst = 0, + key = "remote_addr", + policy = "redis-cluster", + redis_cluster_nodes = {"127.0.0.1:5000", "127.0.0.1:5001"}, + redis_cluster_name = "redis-cluster-1", + }, + }, + } + + for _, case in ipairs(cases) do + local plugin = require("apisix.plugins." .. case.plugin) + local ok, err = plugin.check_schema(case.conf) + if not ok then + ngx.say(err) + elseif case.conf.key_ttl ~= nil then + ngx.say(case.plugin, " ", case.conf.policy, + " got key_ttl: ", case.conf.key_ttl) + else + ngx.say("passed") + end + end + ngx.say("done") + } + } +--- response_body +passed +passed +passed +passed +done + + + +=== TEST 2: limit-conn still defaults key_ttl to 3600 and honors an explicit value +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.limit-conn") + + local conf = { + conn = 1, + burst = 0, + default_conn_delay = 0.1, + key = "remote_addr", + policy = "redis", + redis_host = "127.0.0.1", + } + local ok, err = plugin.check_schema(conf) + if not ok then + ngx.say(err) + return + end + ngx.say(conf.key_ttl) + + conf = { + conn = 1, + burst = 0, + default_conn_delay = 0.1, + key = "remote_addr", + policy = "redis-cluster", + redis_cluster_nodes = {"127.0.0.1:5000", "127.0.0.1:5001"}, + redis_cluster_name = "redis-cluster-1", + key_ttl = 60, + } + ok, err = plugin.check_schema(conf) + if not ok then + ngx.say(err) + return + end + ngx.say(conf.key_ttl) + } + } +--- response_body +3600 +60 From c7dfd2840b753756344464732a6f001ff4e26d12 Mon Sep 17 00:00:00 2001 From: janiussyafiq Date: Mon, 22 Jun 2026 09:45:30 +0800 Subject: [PATCH 2/2] fix(limit-req): deepcopy shared redis schema to avoid aliasing limit-req bound redis_schema.schema directly, holding a live reference to the shared module table. It only reads the table today, so nothing leaks, but the alias is a latent footgun: any future in-place mutation reachable through limit-req would poison every other consumer. deepcopy the schema at load time to match limit-count and ai-cache, so limit-req owns an independent copy. No behavior change. Signed-off-by: janiussyafiq --- apisix/plugins/limit-req.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apisix/plugins/limit-req.lua b/apisix/plugins/limit-req.lua index 6c5b856217cb..dd3c1dcf8fae 100644 --- a/apisix/plugins/limit-req.lua +++ b/apisix/plugins/limit-req.lua @@ -17,7 +17,7 @@ local limit_req_new = require("resty.limit.req").new local core = require("apisix.core") local redis_schema = require("apisix.utils.redis-schema") -local policy_to_additional_properties = redis_schema.schema +local policy_to_additional_properties = core.table.deepcopy(redis_schema.schema) local plugin_name = "limit-req" local sleep = core.sleep local apisix_plugin = require("apisix.plugin")