[Bug]: Multiple Issues in Advanced Ratelimit Policy
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 71
- Forks
- 111
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 110
Description
Please select the area the issue is related to
Gateway
Please select the aspect the issue is related to
Aspect/API (API backends, definitions, contracts, interfaces, OpenAPI), Aspect/Other (Anything else which does not match above categories)
Description
Issue 1:
Rejected requests consume quota on the other limits; enforcement depends on array order
algorithms/fixedwindow/multi.go:48-82 (and identically algorithms/gcra/multi.go:48)
MultiLimiter.AllowN iterates limiters in config order, consuming each one as it goes, and fail-fasts on the first denial. Limiters ahead of the denier have already been charged for a request that is never served. Compounding it, fixedwindow/redis.go:108 and the memory equivalent INCRBY unconditionally — a denied request still counts against its own window.
Net effect: a client that keeps retrying against a 429 destroys its own long-window quota — but only if the long limit happens to be listed first.
Run A — buggy order
{"quotas":[{"name":"t","limits":[{"limit":4,"duration":"1h"},{"limit":2,"duration":"1m"}]}]}
Send 6 requests inside one minute → 2 served (2 LLM calls), 4 rejected. The hourly counter is now at 6, past its limit of 4, even though only 2 requests were ever served. Wait for the next minute and send one more → 429. That's the bug: you're locked out for the rest of the clock hour having consumed 2 of your 4 hourly allowance.
Run B — control, safe order
{"quotas":[{"name":"t","limits":[{"limit":2,"duration":"1m"},{"limit":4,"duration":"1h"}]}]}
Flush Redis first (see below), then repeat identically. Same 6 requests → same 2 served. Wait for the next minute, send one more → 200. The hourly counter only reached 2, because the per-minute limit denied before the hourly one was touched.
Issue 2:
Quota mixing request-phase and response-phase cost sources is charged twice; the fallback default is applied even when a source succeeded
A quota whose costExtraction.sources span both phases is consumed in two independent limiter operations instead of one. HasRequestHeaderOnlyCostSources() inspects only request-phase sources, so a quota with request_header + response_body is classified as header-only and consumed in OnRequestHeaders via AllowN; OnResponseBody then sees HasResponsePhaseSources() == true and consumes again via ConsumeN. Because ExtractResponseCost evaluates response-phase sources in isolation, a failure there returns the quota's default — even though a request-phase source already produced a value — so the default is charged on top of the real cost. The second consumption also uses ConsumeN, which bypasses the limit check entirely, so the quota can be driven negative rather than the request being rejected.
Steps to reproduce
- Attach advanced-ratelimit to an LLMProvider resource with:
{"quotas":[{"name":"dbl","limits":[{"limit":100,"duration":"1m"}],
"costExtraction":{"enabled":true,"default":50,"sources":[
{"type":"request_header","key":"X-Cost","multiplier":1},
{"type":"response_body","jsonPath":"$.usage.no_such_field","multiplier":1}]}}]} - Send one request with header X-Cost: 1.
Actual: x-ratelimit-remaining: 49 — 51 consumed (1 + the 50 default).
Expected: x-ratelimit-remaining: 99 — 1 consumed.
Controls (same limit, one request, X-Cost: 1 each):
sources = [request_header] -> remaining 99 (1 charged)
sources = [response_body, bad jsonPath] -> remaining 50 (50 charged)
sources = both -> remaining 49 (51 charged — strictly additive)
Issue 3:
memory.maxEntries is dead config; the memory backend is unbounded
maxEntries references in Go source: 0
maxEntries occurrences in policy-definition.yaml: 1
The schema documents memory.maxEntries (default 10000) as "Maximum number of rate limit entries to store in memory. Oldest entries are evicted when limit is reached." Nothing in the module reads it. The only reclamation is removeExpired() (algorithms/fixedwindow/memory.go:358), which deletes expired entries on a ticker — and only if cleanupInterval > 0.
- Attach at resource level on GET /v1/agents, same policy:
{"quotas":[{"name":"cap","limits":[{"limit":1,"duration":"1h"}],
"keyExtraction":[{"type":"header","key":"X-User-Id"}]}]}
Leaving the existing policy on /v1/chat/completions is harmless — the key extraction here is header-only, so the cache key is route-scoped and the two get independent limiters. No cross-talk.
- Verify it deployed before reading anything into the results:
❯ API="https://localhost:8443/mistraltestprovider/v1/agents"
❯ KEY=<API_KEY_FROM_WORKSPACE>
curl -sD - -o /dev/null -k "https://localhost:8443/mistraltestprovider/v1/agents"
-H "X-API-Key: $KEY" -H "X-User-Id: probe" | grep -i "^x-ratelimit-limit:"
You need x-ratelimit-limit: 1 back. If that header is missing, the policy isn't on the route and everything after it is noise — that's exactly what happened last time.
Then the same loop, minus the body:
for i in $(seq 1 10); do
printf "u%d -> " "$i"
curl -s -o /dev/null -w "%{http_code}\n" -k "$API"
-H "X-API-Key: $KEY" -H "X-User-Id: u$i"
done
printf "u1 again -> "
curl -s -o /dev/null -w "%{http_code}\n" -k "$API" -H "X-API-Key: $KEY" -H "X-User-Id: u1"
u1..u10 -> 200, then u1 again -> 429. With max_entries = 5.0 honoured, u1 would have been evicted at the 6th entry and returned 200.
Issue 4:
API-level attachment does not scope the counter to the API
ratelimit.go:185 — defaultKeyExtraction := []KeyComponent{{Type: "routename"}}, unconditionally. metadata.AttachedTo is used only for limiter-cache reconciliation (ratelimit.go:338), never for key selection.
So an advanced-ratelimit policy attached at API level with no explicit keyExtraction still counts per resource — 5/min on an API with 4 resources yields 20/min. basic-ratelimit switches to apiname at API level (basic_ratelimit.go:92-95), so the two policies behave differently from the same UI action.
Repro: attach {"quotas":[{"name":"q","limits":[{"limit":5,"duration":"1m"}]}]} at API level on an API with two resources; send 5 to each. All 10 succeed. Adding "keyExtraction":[{"type":"apiname"}] fixes it.
Note: The expected functionality can be taken by setting "keyExtraction":[{"type":"apiname"}] to the quota, restart the runtime.
Steps to Reproduce
Mentioned above
Severity Level of the Issue
Severity/Major (Important functionality is broken. Should be prioritized. Doesn't need immediate attention)
Environment Details (with versions)
No response
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reproducing the four reported behaviors, then inspect algorithms/fixedwindow/multi.go, algorithms/gcra/multi.go, algorithms/fixedwindow/redis.go, algorithms/fixedwindow/memory.go, and ratelimit.go. Compare the advanced-ratelimit behavior with the examples and controls in the issue; done means rejected requests do not consume quota, mixed cost sources are charged correctly, memory.maxEntries is honored, and API-level policies are API-scoped.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, redis
- Domain
- api, backend-api-design, databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100