shreemaan-abhishek opened a new pull request, #13467:
URL: https://github.com/apache/apisix/pull/13467
### Description
This PR fixes two issues in the redis-backed rate limiting plugins.
**limit-req: non-atomic redis commit lets concurrent requests exceed the
configured rate**
The redis policy commit path in `limit-req/util.lua` ran `GET excess` / `GET
last` / limit check / `SET excess` / `SET last` as separate commands. Under
concurrent load, multiple requests can read the same stale `excess`, each
conclude they are within limits, and all be admitted before any `SET` lands, so
clients can exceed the configured rate.
The commit path now runs a single redis Lua script (`EVAL`) that performs
the read, leaky-bucket decay, limit check, and write atomically. The
`excess`/`last` keys now share a `{...}` hash tag so they always map to the
same Redis Cluster slot, which `EVAL` with multiple keys requires
(`limit-req/util.lua` is shared by the redis-cluster policy). The dry-run
(non-commit) path keeps two plain `GET`s since it writes nothing back.
First-request admission (no prior state, `excess = 0`) is preserved.
Note on upgrades: the redis key format changes from `limit_req:<key>excess`
to `{limit_req:<key>}excess`. Old keys are simply ignored and expire via their
existing TTL (a few seconds), so the rate-limit state resets once when
upgrading.
**limit-conn: variable-resolved conn/burst values are not validated**
`conn` and `burst` can be resolved from request variables (e.g. a header).
The resolved value was only passed through `tonumber()`: values like `0`, `-1`,
`1.5`, or values above 2^53-1 were accepted and fed into `floor((conn - 1) /
max)`, producing wrong delay calculations. Resolved values are now rejected
unless they are positive integers within the safe integer range, and error
messages report the offending input.
Also adds a warning log when the redis-backed limit-conn rejects a
connection, noting that connections outliving `key_ttl` expire from the
tracking ZSET and can lead to undercounting of long-lived connections (e.g.
WebSocket, gRPC streaming).
#### Which issue(s) this PR fixes:
N/A
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not,
please discuss on the [APISIX mailing
list](https://github.com/apache/apisix/tree/master#community) first)
--
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]
