shreemaan-abhishek commented on code in PR #13467:
URL: https://github.com/apache/apisix/pull/13467#discussion_r3370555070
##########
apisix/plugins/limit-conn/init.lua:
##########
@@ -43,14 +44,29 @@ local _M = {}
local function resolve_var(ctx, value)
if type(value) == "string" then
+ local original = value
local err, _
value, err, _ = core.utils.resolve_var(value, ctx.var)
if err then
- return nil, "could not resolve var for value: " .. value .. ",
err: " .. err
+ return nil, "could not resolve var for value: " .. original .. ",
err: " .. err
end
+ local resolved = value
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: " ..
tostring(resolved)
+ end
+ if value <= 0 then
Review Comment:
Good catch, you are right. `burst` allows `minimum = 0` in the schema, so a
dynamic `burst` resolving to `0` should be admitted, while only `conn` needs to
stay strictly positive (`exclusiveMinimum = 0`). I split the validation:
`resolve_var` now takes an `allow_zero` flag, passed `true` for both
`conf.burst` and `rule.burst`, so `burst = 0` is accepted while negative,
fractional and unsafe-integer values are still rejected. Added tests for
dynamic `burst: 0` (admitted) and `burst: -1` (rejected). Pushed in b9365968b.
--
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]
