janiussyafiq commented on code in PR #13019: URL: https://github.com/apache/apisix/pull/13019#discussion_r2844797368
########## t/plugin/limit-req.t: ########## @@ -452,7 +452,7 @@ passed --- more_headers apikey: auth-jack --- error_code eval -[403, 403, 403, 403] +[200, 403, 403, 403] Review Comment: https://github.com/openresty/lua-resty-limit-traffic/blob/2a834b193ccf1cd94af6c7f2856eb2dce9fc5a39/lib/resty/limit/req.lua#L105 Since the first request key is always new, hence the request will fall onto the else condition where `excess = 0` hence making the first request always passing. This condition is verified by adding a test case manually into the repo within this file (https://github.com/openresty/lua-resty-limit-traffic/blob/master/t/req.t). ``` === TEST 8: very low rate and burst (0.1) - first request always accepted --- http_config eval " $::HttpConfig lua_shared_dict store 1m; " --- config location = /t { content_by_lua ' local limit_req = require "resty.limit.req" ngx.shared.store:flush_all() local lim = limit_req.new("store", 0.1, 0.1) local key = "test_key" -- First request should always be accepted with delay=0 local delay1, err1 = lim:incoming(key, true) if not delay1 then ngx.say("1st request failed: ", err1) else ngx.say("1st request delay: ", delay1) end -- Second request local delay2, err2 = lim:incoming(key, true) if not delay2 then ngx.say("2nd request failed: ", err2) else ngx.say("2nd request delay: ", delay2) end -- Third request local delay3, err3 = lim:incoming(key, true) if not delay3 then ngx.say("3rd request failed: ", err3) else ngx.say("3rd request delay: ", delay3) end '; } --- request GET /t --- response_body 1st request delay: 0 2nd request failed: rejected 3rd request failed: rejected --- no_error_log [error] [lua] ``` Response I get is: ``` root@bf0f4dba9671:/workspace# FLUSH_ETCD=1 prove -I../test-nginx/lib -I./ -r -s lua-resty-limit-traffic/t/req.t lua-resty-limit-traffic/t/req.t .. ok All tests successful. Files=1, Tests=64, 7 wallclock secs ( 0.01 usr 0.01 sys + 0.12 cusr 0.09 csys = 0.23 CPU) Result: PASS ``` Hence confirming this behavior is expected. This also reflected here: https://github.com/apache/apisix/blob/c8248a69381c02c0f1e6743b65a80cc02f22c155/t/plugin/limit-req.t#L166-L170 -- 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]
