diff --git a/apisix/plugins/limit-count/init.lua b/apisix/plugins/limit-count/init.lua index ed4db64a8cad..838bf27fdb56 100644 --- a/apisix/plugins/limit-count/init.lua +++ b/apisix/plugins/limit-count/init.lua @@ -391,7 +391,19 @@ local function resolve_var(ctx, value) end value = tonumber(value) if not value then - return nil, "resolved value is not a number: " .. tostring(value) + return nil, "resolved value is not a number" + end + -- count/time_window must be positive integers, matching the schema + if value <= 0 then + return nil, "resolved value must be a positive number, got: " .. tostring(value) + end + if value ~= math_floor(value) then + return nil, "resolved value must be an integer, got: " .. tostring(value) + end + -- LuaJIT doubles lose integer precision above 2^53 + if value > 9007199254740991 then + return nil, "resolved value exceeds safe integer range (2^53-1), got: " + .. tostring(value) end end return value @@ -420,17 +432,20 @@ local function get_rules(ctx, conf) local rules = {} for index, rule in ipairs(conf.rules) do + -- a rule keyed on a var absent for this request just doesn't apply + local key, _, n_resolved = core.utils.resolve_var(rule.key, ctx.var) + if n_resolved == 0 then + goto CONTINUE + end + -- the rule applies, so an invalid count/time_window must reject, not + -- silently skip it, else a client-controlled var could disable limiting local count, err = resolve_var(ctx, rule.count) if err then - goto CONTINUE + return nil, err end local time_window, err2 = resolve_var(ctx, rule.time_window) if err2 then - goto CONTINUE - end - local key, _, n_resolved = core.utils.resolve_var(rule.key, ctx.var) - if n_resolved == 0 then - goto CONTINUE + return nil, err2 end core.table.insert(rules, { count = count, diff --git a/t/plugin/limit-count-variable.t b/t/plugin/limit-count-variable.t index a4359c63a87b..e879a871c8b5 100644 --- a/t/plugin/limit-count-variable.t +++ b/t/plugin/limit-count-variable.t @@ -284,3 +284,171 @@ nginx_config: --- error_code: 200 --- access_log eval qr/\{\\x22rate_limiting_key\\x22:\\x22\/apisix\/routes\/1:\d+:test\.com\\x22,\\x22rate_limiting_limit\\x22:2,\\x22rate_limiting_remaining\\x22:1,\\x22rate_limiting_reset\\x22:10}/ + + + +=== TEST 9: set up route with count/time_window from request variables +--- 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, + [[{ + "methods": ["GET"], + "plugins": { + "limit-count": { + "count": "${http_count ?? 2}", + "time_window": "${http_time_window ?? 5}", + "rejected_code": 503, + "key_type": "var", + "key": "remote_addr", + "policy": "local" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 10: a client-supplied 0/negative/fractional count is rejected, not bypassed +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + local httpc = http.new() + for _, count in ipairs({"0", "-1", "1.5", "9999999999999999"}) do + local res = httpc:request_uri(uri, {method = "GET", + headers = {["count"] = count}}) + if res.status ~= 500 then + ngx.say("count=", count, " should be rejected with 500, got ", res.status) + return + end + end + ngx.say("passed") + } + } +--- request +GET /t +--- response_body +passed +--- error_log +resolved value must be a positive number +resolved value must be an integer +resolved value exceeds safe integer range + + + +=== TEST 11: a client-supplied 0/negative/fractional time_window is rejected, not bypassed +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + local httpc = http.new() + for _, time_window in ipairs({"0", "-1", "1.5", "9999999999999999"}) do + local res = httpc:request_uri(uri, {method = "GET", + headers = {["time_window"] = time_window}}) + if res.status ~= 500 then + ngx.say("time_window=", time_window, " should be rejected with 500, got ", + res.status) + return + end + end + ngx.say("passed") + } + } +--- request +GET /t +--- response_body +passed +--- error_log +resolved value must be a positive number +resolved value must be an integer +resolved value exceeds safe integer range + + + +=== TEST 12: set up rules-mode route with count from a request variable +--- 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, + [[{ + "methods": ["GET"], + "plugins": { + "limit-count": { + "rejected_code": 503, + "rules": [ + { + "key": "${http_user}", + "count": "${http_count ?? 2}", + "time_window": 60 + } + ] + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- response_body +passed + + + +=== TEST 13: rules-mode invalid count rejects, not silently skips the rule +--- config + location /t { + content_by_lua_block { + local http = require("resty.http") + local uri = "http://127.0.0.1:" .. ngx.var.server_port .. "/hello" + local httpc = http.new() + -- the rule's key resolves (user header present), so without bounds + -- validation a count of 0 would drop the rule and let the request pass + local res = httpc:request_uri(uri, {method = "GET", + headers = {["user"] = "jack", ["count"] = "0"}}) + if res.status ~= 500 then + ngx.say("invalid rule count should be rejected with 500, got ", res.status) + return + end + ngx.say("passed") + } + } +--- request +GET /t +--- response_body +passed +--- error_log +resolved value must be a positive number