Copilot commented on code in PR #12756:
URL: https://github.com/apache/apisix/pull/12756#discussion_r2917093073
##########
apisix/plugins/limit-count/limit-count-redis.lua:
##########
@@ -40,12 +41,108 @@ local script = core.string.compress_script([=[
]=])
-function _M.new(plugin_name, limit, window, conf)
+local script_sliding = core.string.compress_script([=[
+ assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1")
+
+ local now = tonumber(ARGV[1])
+ local window = tonumber(ARGV[2])
+ local limit = tonumber(ARGV[3])
+ local cost = tonumber(ARGV[4])
+ local req_id = ARGV[5]
+
+ local window_start = now - window
+
+ -- remove events outside of the window
+ redis.call('ZREMRANGEBYSCORE', KEYS[1], 0, window_start)
+
+ local current = redis.call('ZCARD', KEYS[1])
+
+ if current + cost > limit then
+ local earliest = redis.call('ZRANGE', KEYS[1], 0, 0, 'WITHSCORES')
+ local reset = 0
+ if #earliest == 2 then
+ reset = earliest[2] + window - now
+ if reset < 0 then
+ reset = 0
+ end
+ end
+ return {-1, reset}
+ end
+
+ for i = 1, cost do
+ local member = req_id .. ':' .. i
+ redis.call('ZADD', KEYS[1], now, member)
+ end
+
+ redis.call('PEXPIRE', KEYS[1], window)
+
+ local remaining = limit - (current + cost)
+
+ local earliest = redis.call('ZRANGE', KEYS[1], 0, 0, 'WITHSCORES')
+ local reset = 0
+ if #earliest == 2 then
+ reset = earliest[2] + window - now
+ if reset < 0 then
+ reset = 0
+ end
+ end
+
+ return {remaining, reset}
+]=])
+
+
+local script_approximate_sliding = core.string.compress_script([=[
+ assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1")
+
+ local now = tonumber(ARGV[1])
+ local window = tonumber(ARGV[2])
+ local limit = tonumber(ARGV[3])
+ local cost = tonumber(ARGV[4])
Review Comment:
Same issue as the exact sliding script: this assert checks `ARGV[3]` (limit)
rather than `ARGV[4]` (cost). Validate `cost` explicitly to ensure
malformed/invalid costs do not silently pass through.
##########
apisix/plugins/limit-count/limit-count-redis.lua:
##########
@@ -40,12 +41,108 @@ local script = core.string.compress_script([=[
]=])
-function _M.new(plugin_name, limit, window, conf)
+local script_sliding = core.string.compress_script([=[
+ assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1")
+
+ local now = tonumber(ARGV[1])
+ local window = tonumber(ARGV[2])
+ local limit = tonumber(ARGV[3])
+ local cost = tonumber(ARGV[4])
Review Comment:
The Lua assert is validating `ARGV[3]` (which is `limit` in this script),
not `cost`. This makes the validation ineffective and allows invalid `cost`
values to proceed. Update the assertion to validate `ARGV[4]` (the parsed
`cost`) instead.
##########
apisix/plugins/limit-count/limit-count-redis.lua:
##########
@@ -74,14 +186,15 @@ function _M.incoming(self, key, cost)
local remaining = res[1]
ttl = res[2]
Review Comment:
For `fixed` mode, the script returns TTL in seconds (`TTL` + `EXPIRE`). For
`sliding` and `approximate_sliding`, the scripts compute `reset` in
milliseconds (due to `now/window` being ms and `PEXPIRE`). Returning mixed
units through the same `ttl` variable will produce incorrect
`X-RateLimit-Reset`/reset behavior unless the caller compensates. Standardize
the return unit across modes (preferably seconds to match existing behavior, or
rename/convert explicitly at the boundary).
```suggestion
ttl = res[2]
if self.window_type == "sliding" or self.window_type ==
"approximate_sliding" then
ttl = ttl / 1000
end
```
##########
apisix/plugins/limit-count/limit-count-redis-cluster.lua:
##########
@@ -39,7 +40,101 @@ local script = core.string.compress_script([=[
]=])
-function _M.new(plugin_name, limit, window, conf)
+local script_sliding = core.string.compress_script([=[
+ assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1")
+
+ local now = tonumber(ARGV[1])
+ local window = tonumber(ARGV[2])
+ local limit = tonumber(ARGV[3])
+ local cost = tonumber(ARGV[4])
+ local req_id = ARGV[5]
+
+ local window_start = now - window
+
+ redis.call('ZREMRANGEBYSCORE', KEYS[1], 0, window_start)
+
+ local current = redis.call('ZCARD', KEYS[1])
+
+ if current + cost > limit then
+ local earliest = redis.call('ZRANGE', KEYS[1], 0, 0, 'WITHSCORES')
+ local reset = 0
+ if #earliest == 2 then
+ reset = earliest[2] + window - now
+ if reset < 0 then
+ reset = 0
+ end
+ end
+ return {-1, reset}
+ end
+
+ for i = 1, cost do
+ local member = req_id .. ':' .. i
+ redis.call('ZADD', KEYS[1], now, member)
+ end
+
+ redis.call('PEXPIRE', KEYS[1], window)
+
+ local remaining = limit - (current + cost)
+
+ local earliest = redis.call('ZRANGE', KEYS[1], 0, 0, 'WITHSCORES')
+ local reset = 0
+ if #earliest == 2 then
+ reset = earliest[2] + window - now
+ if reset < 0 then
+ reset = 0
+ end
+ end
+
+ return {remaining, reset}
+]=])
+
+
+local script_approximate_sliding = core.string.compress_script([=[
+ assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1")
+
+ local now = tonumber(ARGV[1])
+ local window = tonumber(ARGV[2])
+ local limit = tonumber(ARGV[3])
+ local cost = tonumber(ARGV[4])
+
+ -- Calculate window IDs
+ local window_id = math.floor(now / window)
+ local prev_window_id = window_id - 1
+
+ -- Get counts from current and previous windows
+ local curr_key = KEYS[1] .. ':' .. window_id
+ local prev_key = KEYS[1] .. ':' .. prev_window_id
+
+ local curr_count = tonumber(redis.call('GET', curr_key) or 0)
+ local prev_count = tonumber(redis.call('GET', prev_key) or 0)
Review Comment:
In Redis Cluster, Lua scripts cannot access keys in different hash slots.
Deriving `curr_key/prev_key` by appending `:window_id` will generally produce
different slots, leading to `CROSSSLOT`/cluster script errors at runtime.
Ensure all keys share the same hash tag (e.g., wrap a stable portion of the
base key in `{...}` and append suffixes outside the braces), or pass both
derived keys via `KEYS[]` using a consistent hash tag strategy.
##########
docs/en/latest/plugins/limit-count.md:
##########
@@ -46,6 +51,7 @@ You may see the following rate limiting headers in the
response:
| ----------------------- | ------- |
----------------------------------------- | ------------- |
-------------------------------------- |
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|
| count | integer | True
| | > 0 | The maximum number of
requests allowed within a given time interval.
|
| time_window | integer | True
| | > 0 | The time interval corresponding
to the rate limiting `count` in seconds.
|
+| window_type | string | False
| fixed | ["fixed","sliding"] | The window behavior type.
`fixed` uses a fixed window algorithm. `sliding` uses a sliding window
algorithm to enforce an exact number of requests per rolling window. `sliding`
is only supported when `policy` is `redis` or `redis-cluster`.
|
Review Comment:
The schema supports `approximate_sliding`, but the docs only list
`["fixed","sliding"]` and do not describe the approximate mode. Update the
allowed values and add a brief description/behavior note for
`approximate_sliding` (including the Redis/Redis Cluster restriction).
##########
apisix/plugins/limit-count/limit-count-redis-cluster.lua:
##########
@@ -39,7 +40,101 @@ local script = core.string.compress_script([=[
]=])
-function _M.new(plugin_name, limit, window, conf)
+local script_sliding = core.string.compress_script([=[
+ assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1")
+
+ local now = tonumber(ARGV[1])
+ local window = tonumber(ARGV[2])
+ local limit = tonumber(ARGV[3])
+ local cost = tonumber(ARGV[4])
+ local req_id = ARGV[5]
+
+ local window_start = now - window
+
+ redis.call('ZREMRANGEBYSCORE', KEYS[1], 0, window_start)
+
+ local current = redis.call('ZCARD', KEYS[1])
+
+ if current + cost > limit then
+ local earliest = redis.call('ZRANGE', KEYS[1], 0, 0, 'WITHSCORES')
+ local reset = 0
+ if #earliest == 2 then
+ reset = earliest[2] + window - now
+ if reset < 0 then
+ reset = 0
+ end
+ end
+ return {-1, reset}
+ end
+
+ for i = 1, cost do
+ local member = req_id .. ':' .. i
+ redis.call('ZADD', KEYS[1], now, member)
+ end
+
+ redis.call('PEXPIRE', KEYS[1], window)
+
+ local remaining = limit - (current + cost)
+
+ local earliest = redis.call('ZRANGE', KEYS[1], 0, 0, 'WITHSCORES')
+ local reset = 0
+ if #earliest == 2 then
+ reset = earliest[2] + window - now
+ if reset < 0 then
+ reset = 0
+ end
+ end
+
+ return {remaining, reset}
+]=])
+
+
+local script_approximate_sliding = core.string.compress_script([=[
+ assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1")
+
+ local now = tonumber(ARGV[1])
+ local window = tonumber(ARGV[2])
+ local limit = tonumber(ARGV[3])
+ local cost = tonumber(ARGV[4])
Review Comment:
Same as above: the script asserts on `limit` instead of `cost`. This should
validate `ARGV[4]` to correctly enforce `cost >= 1`.
##########
apisix/plugins/limit-count/limit-count-redis.lua:
##########
@@ -59,13 +156,28 @@ function _M.incoming(self, key, cost)
return red, err, 0
end
- local limit = self.limit
- local window = self.window
- local res
key = self.plugin_name .. tostring(key)
local ttl = 0
- res, err = red:eval(script, 1, key, limit, window, cost or 1)
+ local limit = self.limit
+ local c = cost or 1
+ local res
+
+ if self.window_type == "sliding" then
+ local now = ngx.now() * 1000
+ local window = self.window * 1000
+ local req_id = ngx_var.request_id
+
+ res, err = red:eval(script_sliding, 1, key, now, window, limit, c,
req_id)
+ elseif self.window_type == "approximate_sliding" then
+ local now = ngx.now() * 1000
+ local window = self.window * 1000
+
+ res, err = red:eval(script_approximate_sliding, 1, key, now, window,
limit, c)
+ else
Review Comment:
`ngx.var.request_id` is not guaranteed to be present in all Nginx/APISIX
builds/configurations; passing `nil` into `red:eval(...)` can cause an argument
error and break rate limiting. Provide a deterministic fallback request
identifier when `ngx.var.request_id` is unavailable (for example, derive from
connection attributes or generate a UUID).
##########
t/plugin/limit-count-redis-sliding.t:
##########
@@ -0,0 +1,501 @@
+#
+# 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_shuffle();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->error_log && !$block->no_error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: redis policy with sliding window - basic N per window
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 2,
+ "window_type": "sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+=== TEST 2: redis policy with sliding window - enforce N per window
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- error_code eval
+[200, 200, 503]
+
+
+=== TEST 3: redis policy with sliding window - remaining header on reject
+--- config
+ location /t {
+ content_by_lua_block {
+ local json = require "t.toolkit.json"
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/hello"
+ local ress = {}
+
+ -- ensure previous windows are expired before starting this test
+ ngx.sleep(2.2)
+
+ -- first request: allowed, remaining should be 1
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(ress, {res.status,
res.headers["X-RateLimit-Remaining"]})
+ end
+
+ -- second request: allowed, remaining should be 0
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(ress, {res.status,
res.headers["X-RateLimit-Remaining"]})
+ end
+
+ -- third request: rejected, remaining header should stay at 0
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(ress, {res.status,
res.headers["X-RateLimit-Remaining"]})
+ end
+
+ ngx.say(json.encode(ress))
+ }
+ }
+--- response_body
+[[200,"1"],[200,"0"],[503,"0"]]
+
+
+=== TEST 4: redis policy with sliding window - allow after window passes
+--- config
+ location /t {
+ content_by_lua_block {
+ local json = require "t.toolkit.json"
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/hello"
+ local codes = {}
+
+ -- ensure previous windows are expired before starting this test
+ ngx.sleep(2.2)
+
+ -- consume full quota
+ for i = 1, 2 do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(codes, res.status)
+ end
+
+ -- wait longer than the sliding window (2s)
+ ngx.sleep(2.2)
+
+ -- should be allowed again after window has passed
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(codes, res.status)
+ end
+
+ ngx.say(json.encode(codes))
+ }
+ }
+--- response_body
+[200,200,200]
+
+
+=== TEST 5: setup route with fixed window for boundary burst comparison
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/2',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/fixed",
+ "plugins": {
+ "limit-count": {
+ "count": 4,
+ "time_window": 4,
+ "window_type": "fixed",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 6: setup route with sliding window for boundary burst comparison
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/3',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/sliding",
+ "plugins": {
+ "limit-count": {
+ "count": 4,
+ "time_window": 4,
+ "window_type": "sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 7: sliding window - cost parameter support
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/4',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/sliding-cost",
+ "plugins": {
+ "limit-count": {
+ "count": 10,
+ "time_window": 3,
+ "window_type": "sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 8: sliding window - verify X-RateLimit headers accuracy
+--- config
+ location /t {
+ content_by_lua_block {
+ local json = require "t.toolkit.json"
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/sliding-cost"
+ local results = {}
+
+ -- ensure previous windows are expired
+ ngx.sleep(3.5)
+
+ -- Send requests and check headers
+ for i = 1, 5 do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say("error: " .. err)
+ return
+ end
+
+ local limit = res.headers["X-RateLimit-Limit"]
+ local remaining = res.headers["X-RateLimit-Remaining"]
+ local reset = res.headers["X-RateLimit-Reset"]
+
+ table.insert(results, {
+ req = i,
+ status = res.status,
+ limit = limit,
+ remaining = remaining,
+ has_reset = reset ~= nil
+ })
+ end
+
+ for _, r in ipairs(results) do
+ ngx.say(string.format("req %d: status=%d, limit=%s,
remaining=%s, has_reset=%s",
+ r.req, r.status, r.limit or "nil", r.remaining or "nil",
tostring(r.has_reset)))
+ end
+ }
+ }
+--- response_body_like
+req 1: status=404, limit=10, remaining=9, has_reset=true
+req 2: status=404, limit=10, remaining=8, has_reset=true
+req 3: status=404, limit=10, remaining=7, has_reset=true
+req 4: status=404, limit=10, remaining=6, has_reset=true
+req 5: status=404, limit=10, remaining=5, has_reset=true
+
+
+
+=== TEST 9: verify local policy rejects sliding window
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/5',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/local-sliding",
+ "plugins": {
+ "limit-count": {
+ "count": 10,
+ "time_window": 60,
+ "window_type": "sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "local"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ else
+ ngx.say("ERROR: should have been rejected")
+ end
+ }
+ }
+--- error_code: 400
+
+
+
+=== TEST 10: sliding window with redis-cluster policy
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ -- Note: This test requires redis-cluster to be available
+ -- It validates schema but may fail at runtime if cluster
unavailable
+ local code, body = t('/apisix/admin/routes/6',
Review Comment:
This test explicitly notes it may fail if Redis Cluster isn’t available,
which makes the suite non-deterministic. Consider moving this case to the
cluster-specific test file (or gating it with a skip condition based on
environment/CI capability) so local Redis-only runs stay reliable.
##########
apisix/plugins/limit-count/limit-count-redis-cluster.lua:
##########
@@ -39,7 +40,101 @@ local script = core.string.compress_script([=[
]=])
-function _M.new(plugin_name, limit, window, conf)
+local script_sliding = core.string.compress_script([=[
+ assert(tonumber(ARGV[3]) >= 1, "cost must be at least 1")
+
+ local now = tonumber(ARGV[1])
+ local window = tonumber(ARGV[2])
+ local limit = tonumber(ARGV[3])
+ local cost = tonumber(ARGV[4])
+ local req_id = ARGV[5]
Review Comment:
The assert validates `ARGV[3]` (limit) rather than `ARGV[4]` (cost), so
invalid `cost` values are not rejected. Update the assertion to validate `cost`.
##########
t/plugin/limit-count-redis-approximate-sliding.t:
##########
@@ -0,0 +1,501 @@
+#
+# 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_shuffle();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->error_log && !$block->no_error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: redis policy with approximate sliding window - basic N per window
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 2,
+ "window_type": "approximate_sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+=== TEST 2: redis policy with approximate sliding window - enforce N per window
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- error_code eval
+[200, 200, 503]
+
+
+=== TEST 3: redis policy with approximate sliding window - remaining header on
reject
+--- config
+ location /t {
+ content_by_lua_block {
+ local json = require "t.toolkit.json"
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/hello"
+ local ress = {}
+
+ -- ensure previous windows are expired before starting this test
+ ngx.sleep(2.2)
+
+ -- first request: allowed, remaining should be 1
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(ress, {res.status,
res.headers["X-RateLimit-Remaining"]})
+ end
+
+ -- second request: allowed, remaining should be 0
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(ress, {res.status,
res.headers["X-RateLimit-Remaining"]})
+ end
+
+ -- third request: rejected, remaining header should stay at 0
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(ress, {res.status,
res.headers["X-RateLimit-Remaining"]})
+ end
+
+ ngx.say(json.encode(ress))
+ }
+ }
+--- response_body
+[[200,"1"],[200,"0"],[503,"0"]]
+
+
+=== TEST 4: redis policy with approximate sliding window - allow after window
passes
+--- config
+ location /t {
+ content_by_lua_block {
+ local json = require "t.toolkit.json"
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/hello"
+ local codes = {}
+
+ -- ensure previous windows are expired before starting this test
+ ngx.sleep(2.2)
+
+ -- consume full quota
+ for i = 1, 2 do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(codes, res.status)
+ end
+
+ -- wait longer than the sliding window (2s)
+ ngx.sleep(2.2)
+
+ -- should be allowed again after window has passed
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(codes, res.status)
+ end
+
+ ngx.say(json.encode(codes))
+ }
+ }
+--- response_body
+[200,200,200]
+
+
+=== TEST 5: setup route with fixed window for boundary burst comparison
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/2',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/fixed",
+ "plugins": {
+ "limit-count": {
+ "count": 4,
+ "time_window": 4,
+ "window_type": "fixed",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 6: setup route with sliding window for boundary burst comparison
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/3',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/sliding",
+ "plugins": {
+ "limit-count": {
+ "count": 4,
+ "time_window": 4,
+ "window_type": "approximate_sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 7: sliding window - cost parameter support
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/4',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/sliding-cost",
+ "plugins": {
+ "limit-count": {
+ "count": 10,
+ "time_window": 3,
+ "window_type": "approximate_sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 8: sliding window - verify X-RateLimit headers accuracy
+--- config
+ location /t {
+ content_by_lua_block {
+ local json = require "t.toolkit.json"
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/sliding-cost"
+ local results = {}
+
+ -- ensure previous windows are expired
+ ngx.sleep(3.5)
+
+ -- Send requests and check headers
+ for i = 1, 5 do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say("error: " .. err)
+ return
+ end
+
+ local limit = res.headers["X-RateLimit-Limit"]
+ local remaining = res.headers["X-RateLimit-Remaining"]
+ local reset = res.headers["X-RateLimit-Reset"]
+
+ table.insert(results, {
+ req = i,
+ status = res.status,
+ limit = limit,
+ remaining = remaining,
+ has_reset = reset ~= nil
+ })
+ end
+
+ for _, r in ipairs(results) do
+ ngx.say(string.format("req %d: status=%d, limit=%s,
remaining=%s, has_reset=%s",
+ r.req, r.status, r.limit or "nil", r.remaining or "nil",
tostring(r.has_reset)))
+ end
+ }
+ }
+--- response_body_like
+req 1: status=404, limit=10, remaining=9, has_reset=true
+req 2: status=404, limit=10, remaining=8, has_reset=true
+req 3: status=404, limit=10, remaining=7, has_reset=true
+req 4: status=404, limit=10, remaining=6, has_reset=true
+req 5: status=404, limit=10, remaining=5, has_reset=true
+
+
+
+=== TEST 9: verify local policy rejects sliding window
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/5',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/local-sliding",
+ "plugins": {
+ "limit-count": {
+ "count": 10,
+ "time_window": 60,
+ "window_type": "approximate_sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "local"
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ else
+ ngx.say("ERROR: should have been rejected")
+ end
+ }
+ }
+--- error_code: 400
+
+
+
+=== TEST 10: sliding window with redis-cluster policy
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ -- Note: This test requires redis-cluster to be available
+ -- It validates schema but may fail at runtime if cluster
unavailable
+ local code, body = t('/apisix/admin/routes/6',
Review Comment:
Same non-determinism issue as in the sliding test file: this Redis Cluster
scenario should be in the cluster-specific suite or explicitly skipped when
Redis Cluster isn’t available, rather than potentially failing “by design”.
##########
t/plugin/limit-count-redis-sliding.t:
##########
@@ -0,0 +1,501 @@
+#
+# 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_shuffle();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!$block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->error_log && !$block->no_error_log) {
+ $block->set_value("no_error_log", "[error]\n[alert]");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: redis policy with sliding window - basic N per window
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "limit-count": {
+ "count": 2,
+ "time_window": 2,
+ "window_type": "sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+=== TEST 2: redis policy with sliding window - enforce N per window
+--- pipelined_requests eval
+["GET /hello", "GET /hello", "GET /hello"]
+--- error_code eval
+[200, 200, 503]
+
+
+=== TEST 3: redis policy with sliding window - remaining header on reject
+--- config
+ location /t {
+ content_by_lua_block {
+ local json = require "t.toolkit.json"
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/hello"
+ local ress = {}
+
+ -- ensure previous windows are expired before starting this test
+ ngx.sleep(2.2)
+
+ -- first request: allowed, remaining should be 1
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(ress, {res.status,
res.headers["X-RateLimit-Remaining"]})
+ end
+
+ -- second request: allowed, remaining should be 0
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(ress, {res.status,
res.headers["X-RateLimit-Remaining"]})
+ end
+
+ -- third request: rejected, remaining header should stay at 0
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(ress, {res.status,
res.headers["X-RateLimit-Remaining"]})
+ end
+
+ ngx.say(json.encode(ress))
+ }
+ }
+--- response_body
+[[200,"1"],[200,"0"],[503,"0"]]
+
+
+=== TEST 4: redis policy with sliding window - allow after window passes
+--- config
+ location /t {
+ content_by_lua_block {
+ local json = require "t.toolkit.json"
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/hello"
+ local codes = {}
+
+ -- ensure previous windows are expired before starting this test
+ ngx.sleep(2.2)
+
+ -- consume full quota
+ for i = 1, 2 do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(codes, res.status)
+ end
+
+ -- wait longer than the sliding window (2s)
+ ngx.sleep(2.2)
+
+ -- should be allowed again after window has passed
+ do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say(err)
+ return
+ end
+ table.insert(codes, res.status)
+ end
+
+ ngx.say(json.encode(codes))
+ }
+ }
+--- response_body
+[200,200,200]
+
+
+=== TEST 5: setup route with fixed window for boundary burst comparison
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/2',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/fixed",
+ "plugins": {
+ "limit-count": {
+ "count": 4,
+ "time_window": 4,
+ "window_type": "fixed",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 6: setup route with sliding window for boundary burst comparison
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/3',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/sliding",
+ "plugins": {
+ "limit-count": {
+ "count": 4,
+ "time_window": 4,
+ "window_type": "sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 7: sliding window - cost parameter support
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/4',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/sliding-cost",
+ "plugins": {
+ "limit-count": {
+ "count": 10,
+ "time_window": 3,
+ "window_type": "sliding",
+ "rejected_code": 503,
+ "key": "remote_addr",
+ "policy": "redis",
+ "redis_host": "127.0.0.1",
+ "redis_port": 6379,
+ "redis_timeout": 1000
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 8: sliding window - verify X-RateLimit headers accuracy
+--- config
+ location /t {
+ content_by_lua_block {
+ local json = require "t.toolkit.json"
+ local http = require "resty.http"
+ local uri = "http://127.0.0.1:"; .. ngx.var.server_port
+ .. "/sliding-cost"
+ local results = {}
+
+ -- ensure previous windows are expired
+ ngx.sleep(3.5)
+
+ -- Send requests and check headers
+ for i = 1, 5 do
+ local httpc = http.new()
+ local res, err = httpc:request_uri(uri)
+ if not res then
+ ngx.say("error: " .. err)
+ return
+ end
+
+ local limit = res.headers["X-RateLimit-Limit"]
+ local remaining = res.headers["X-RateLimit-Remaining"]
+ local reset = res.headers["X-RateLimit-Reset"]
+
+ table.insert(results, {
+ req = i,
+ status = res.status,
+ limit = limit,
+ remaining = remaining,
+ has_reset = reset ~= nil
+ })
+ end
+
+ for _, r in ipairs(results) do
+ ngx.say(string.format("req %d: status=%d, limit=%s,
remaining=%s, has_reset=%s",
+ r.req, r.status, r.limit or "nil", r.remaining or "nil",
tostring(r.has_reset)))
+ end
+ }
+ }
+--- response_body_like
+req 1: status=404, limit=10, remaining=9, has_reset=true
+req 2: status=404, limit=10, remaining=8, has_reset=true
+req 3: status=404, limit=10, remaining=7, has_reset=true
+req 4: status=404, limit=10, remaining=6, has_reset=true
+req 5: status=404, limit=10, remaining=5, has_reset=true
Review Comment:
The assertion hard-codes upstream status `404`, which is unrelated to rate
limiting and can become brittle if the upstream test server behavior changes.
Consider asserting only the `limit/remaining/reset` parts (or use an endpoint
known to return 200) so the test focuses on rate limit headers rather than
upstream routing behavior.
```suggestion
req 1: status=\d+, limit=10, remaining=9, has_reset=true
req 2: status=\d+, limit=10, remaining=8, has_reset=true
req 3: status=\d+, limit=10, remaining=7, has_reset=true
req 4: status=\d+, limit=10, remaining=6, has_reset=true
req 5: status=\d+, limit=10, remaining=5, has_reset=true
```
##########
docs/zh/latest/plugins/limit-count.md:
##########
@@ -47,6 +52,7 @@ description: limit-count 插件使用固定窗口算法，通过给定时间间
| ------------------- | ------- | ---------- | ------------- |
--------------------------------------- |
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|
| count | integer | 是 | | > 0 | 给定时间间隔内允许的最大请求数。 |
| time_window | integer | 是 | | > 0 | 速率限制 `count` 对应的时间间隔（以秒为单位）。 |
+| window_type | string | 否 | fixed | ["fixed","sliding"] | 窗口行为类型。`fixed`
使用固定窗口算法；`sliding` 使用滑动窗口算法，在滚动时间窗口内精确限制请求次数。仅当 `policy` 为 `redis` 或
`redis-cluster` 时才支持 `sliding`。 |
Review Comment:
文档里 `window_type` 的枚举值缺少 `approximate_sliding`，且未描述近似滑动窗口模式，但代码 schema
已支持它。建议补充 `approximate_sliding` 的可选值与说明（包括仅在 `redis`/`redis-cluster` 下支持）。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
