Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[RateLimiter] CompoundLimiter was accepting requests even when some limiters already consumed all tokens#51666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:6.3from10n:6.3.4-fix-compound-limiter
Nov 10, 2023

Conversation

10n
Copy link
Contributor

@10n10n commentedSep 15, 2023
edited by nicolas-grekas
Loading

QA
Branch?6.3
Bug fix?yes
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

CompoundLimiter is accepting requests when the limit was reached previously.

When processing the limiters and the first one consumes exactly all the remaining tokens (remaining=0, accepted=true) and the next one already reached the limit previously (remaining=0, accepted=0) the $minimalRateLimit is considered the first one that will accept the request (even if it's not the most restrictive).

For example:
CompoundLimiter includes 2 limiters:

  • limiter 1 - remaining 2 tokens
  • limiter 2 - remaining 0 tokens

After consuming 2 tokens each each limiter generates to limits:

  • limiter1->consume(2), generates a limit indicating0 remaining tokens,accepts the request (it was last permitted)
  • limiter2->consume(2), generates a limit indicating0 remaining tokens,did not accept the request (it did not have 2 tokens to satisfy the request)

Because both of them have at this moment0 remaining tokens, the minimum limit that is returned will be the limit from thelimiter1 . This means that the CompundLimiter will accept the request, even if thelimiter2 should be more restrictive.

If we switch the order in the constructor, the request will be denied. The order should not matter.

@10n10n changed the title[RateLimier] - CompoundLimiter was accepting requests even when some limiters already consumed all tokens[RateLimtier] - CompoundLimiter was accepting requests even when some limiters already consumed all tokensSep 15, 2023
@10n10n changed the title[RateLimtier] - CompoundLimiter was accepting requests even when some limiters already consumed all tokens[RateLimiter] - CompoundLimiter was accepting requests even when some limiters already consumed all tokensSep 15, 2023
@nicolas-grekasnicolas-grekas added this to the6.3 milestoneSep 25, 2023
@nicolas-grekasnicolas-grekas changed the title[RateLimiter] - CompoundLimiter was accepting requests even when some limiters already consumed all tokens[RateLimiter] CompoundLimiter was accepting requests even when some limiters already consumed all tokensSep 29, 2023
Copy link
Member

@kbondkbond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I've confirmed this bug and that this PR fixes it.

@fabpotfabpot merged commit82b811d intosymfony:6.3Nov 10, 2023
@fabpotfabpot mentioned this pull requestNov 10, 2023
@nicolas-grekas
Copy link
Member

Thank you@10n

10n reacted with thumbs up emoji

This was referencedNov 10, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@kbondkbondkbond approved these changes

@wouterjwouterjAwaiting requested review from wouterj

Assignees
No one assigned
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.

5 participants
@10n@nicolas-grekas@kbond@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp