From 3956807b23349c29db85a61c50a09eae50090e8d Mon Sep 17 00:00:00 2001 From: Evgeny Ruban Date: Thu, 30 Nov 2023 21:53:12 +0400 Subject: [PATCH 1/7] [RateLimit] Allow to get RateLimit without consuming again. --- .../Component/RateLimiter/Policy/SlidingWindowLimiter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php index bf62b89ffc7f9..5a1d0b18f2825 100644 --- a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php +++ b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php @@ -65,7 +65,7 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation $now = microtime(true); $hitCount = $window->getHitCount(); $availableTokens = $this->getAvailableTokens($hitCount); - if ($availableTokens >= $tokens) { + if ($tokens !== 0 && $availableTokens >= $tokens) { $window->add($tokens); $reservation = new Reservation($now, new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit)); From 717b3822d47ace83cd3f5c9bc6c3d8ec492116fd Mon Sep 17 00:00:00 2001 From: Evgeny Ruban Date: Thu, 30 Nov 2023 22:03:39 +0400 Subject: [PATCH 2/7] [RateLimit] Allow to get RateLimit without consuming again. --- .../Component/RateLimiter/Policy/SlidingWindowLimiter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php index 5a1d0b18f2825..bb0f989f5a32f 100644 --- a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php +++ b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php @@ -65,7 +65,7 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation $now = microtime(true); $hitCount = $window->getHitCount(); $availableTokens = $this->getAvailableTokens($hitCount); - if ($tokens !== 0 && $availableTokens >= $tokens) { + if (0 !== $tokens && $availableTokens >= $tokens) { $window->add($tokens); $reservation = new Reservation($now, new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit)); From 788a44debed885de4d79914a1305bdbb46017a90 Mon Sep 17 00:00:00 2001 From: Evgeny Ruban Date: Thu, 30 Nov 2023 22:55:27 +0400 Subject: [PATCH 3/7] [RateLimiter] Allow to get RateLimit without consuming again. --- .../Policy/SlidingWindowLimiter.php | 2 +- .../Tests/Policy/SlidingWindowLimiterTest.php | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php index bb0f989f5a32f..18f1cdea3d5df 100644 --- a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php +++ b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php @@ -65,7 +65,7 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation $now = microtime(true); $hitCount = $window->getHitCount(); $availableTokens = $this->getAvailableTokens($hitCount); - if (0 !== $tokens && $availableTokens >= $tokens) { + if ((0 !== $tokens || 0 !== $availableTokens) && $availableTokens >= $tokens) { $window->add($tokens); $reservation = new Reservation($now, new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit)); diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php index 21deb69c3932b..b58943ac7211f 100644 --- a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php +++ b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php @@ -52,6 +52,26 @@ public function testConsume() $rateLimit = $limiter->consume(10); $this->assertTrue($rateLimit->isAccepted()); $this->assertSame(10, $rateLimit->getLimit()); + + // Test without consuming a try + $limiter->reset(); + + $rateLimit = $limiter->consume(0); + $this->assertTrue($rateLimit->isAccepted()); + $this->assertEquals( + \DateTimeImmutable::createFromFormat('U', (string)floor(microtime(true))), + $rateLimit->getRetryAfter() + ); + + $limiter->reset(); + $limiter->consume(10); + + $rateLimit = $limiter->consume(0); + $this->assertFalse($rateLimit->isAccepted()); + $this->assertEquals( + \DateTimeImmutable::createFromFormat('U', (string)floor(microtime(true) + 12 / 10)), + $rateLimit->getRetryAfter() + ); } public function testWaitIntervalOnConsumeOverLimit() From 59aedb8bacd039b4f0e392bd301a6dfc16b06627 Mon Sep 17 00:00:00 2001 From: Evgeny Ruban Date: Thu, 30 Nov 2023 22:59:46 +0400 Subject: [PATCH 4/7] [RateLimit] Allow to get RateLimit without consuming again. --- .../RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php index b58943ac7211f..cf9a202d08a1b 100644 --- a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php +++ b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php @@ -59,7 +59,7 @@ public function testConsume() $rateLimit = $limiter->consume(0); $this->assertTrue($rateLimit->isAccepted()); $this->assertEquals( - \DateTimeImmutable::createFromFormat('U', (string)floor(microtime(true))), + \DateTimeImmutable::createFromFormat('U', (string) floor(microtime(true))), $rateLimit->getRetryAfter() ); @@ -69,7 +69,7 @@ public function testConsume() $rateLimit = $limiter->consume(0); $this->assertFalse($rateLimit->isAccepted()); $this->assertEquals( - \DateTimeImmutable::createFromFormat('U', (string)floor(microtime(true) + 12 / 10)), + \DateTimeImmutable::createFromFormat('U', (string) floor(microtime(true) + 12 / 10)), $rateLimit->getRetryAfter() ); } From 6fe7205d08244b39537b9491c599a9b414cea272 Mon Sep 17 00:00:00 2001 From: Evgeny Ruban Date: Thu, 30 Nov 2023 23:40:15 +0400 Subject: [PATCH 5/7] [RateLimit] Allow to get RateLimit without consuming again. --- .../RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php index cf9a202d08a1b..5ac98c5ad5700 100644 --- a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php +++ b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php @@ -52,8 +52,11 @@ public function testConsume() $rateLimit = $limiter->consume(10); $this->assertTrue($rateLimit->isAccepted()); $this->assertSame(10, $rateLimit->getLimit()); + } - // Test without consuming a try + public function testConsumeZeroTokens() + { + $limiter = $this->createLimiter(); $limiter->reset(); $rateLimit = $limiter->consume(0); From 2d9fb06bf3fe97d3cefa7f4bb9621f31f66cb992 Mon Sep 17 00:00:00 2001 From: Evgeny Ruban Date: Wed, 6 Dec 2023 21:52:27 +0400 Subject: [PATCH 6/7] fix_rate_limit_bc_after_51676_pr - --- .../RateLimiter/Policy/FixedWindowLimiter.php | 20 +++++++++++++++---- .../Policy/SlidingWindowLimiter.php | 20 +++++++++++++++---- .../Tests/Policy/FixedWindowLimiterTest.php | 15 ++++++++++++++ .../Tests/Policy/SlidingWindowLimiterTest.php | 17 +++++++++++++++- 4 files changed, 63 insertions(+), 9 deletions(-) diff --git a/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php b/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php index 05fab322f03be..f8106dac61520 100644 --- a/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php +++ b/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php @@ -59,21 +59,33 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation $now = microtime(true); $availableTokens = $window->getAvailableTokens($now); - if ($availableTokens >= max(1, $tokens)) { + if (0 !== $tokens && $availableTokens > $tokens) { $window->add($tokens, $now); $reservation = new Reservation($now, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit)); } else { + if ($availableTokens === $tokens) { + $window->add($tokens, $now); + } + $waitDuration = $window->calculateTimeForTokens(max(1, $tokens), $now); - if (null !== $maxTime && $waitDuration > $maxTime) { + if ($availableTokens !== $tokens && 0 !== $tokens && null !== $maxTime && $waitDuration > $maxTime) { // process needs to wait longer than set interval throw new MaxWaitDurationExceededException(sprintf('The rate limiter wait time ("%d" seconds) is longer than the provided maximum time ("%d" seconds).', $waitDuration, $maxTime), new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), false, $this->limit)); } - $window->add($tokens, $now); + if ($availableTokens !== $tokens) { + $window->add($tokens, $now); + } + + if ($availableTokens === $tokens || 0 === $tokens) { + $accepted = true; + } else { + $accepted = false; + } - $reservation = new Reservation($now + $waitDuration, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), false, $this->limit)); + $reservation = new Reservation($now + $waitDuration, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), $accepted, $this->limit)); } if (0 < $tokens) { diff --git a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php index 18f1cdea3d5df..3cdeca120bdf0 100644 --- a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php +++ b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php @@ -65,21 +65,33 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation $now = microtime(true); $hitCount = $window->getHitCount(); $availableTokens = $this->getAvailableTokens($hitCount); - if ((0 !== $tokens || 0 !== $availableTokens) && $availableTokens >= $tokens) { + if (0 !== $tokens && $availableTokens > $tokens) { $window->add($tokens); $reservation = new Reservation($now, new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit)); } else { + if ($availableTokens === $tokens) { + $window->add($tokens); + } + $waitDuration = $window->calculateTimeForTokens($this->limit, max(1, $tokens)); - if (null !== $maxTime && $waitDuration > $maxTime) { + if ($availableTokens !== $tokens && 0 !== $tokens && null !== $maxTime && $waitDuration > $maxTime) { // process needs to wait longer than set interval throw new MaxWaitDurationExceededException(sprintf('The rate limiter wait time ("%d" seconds) is longer than the provided maximum time ("%d" seconds).', $waitDuration, $maxTime), new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), false, $this->limit)); } - $window->add($tokens); + if ($availableTokens !== $tokens) { + $window->add($tokens); + } + + if ($availableTokens === $tokens || 0 === $tokens) { + $accepted = true; + } else { + $accepted = false; + } - $reservation = new Reservation($now + $waitDuration, new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), false, $this->limit)); + $reservation = new Reservation($now + $waitDuration, new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), $accepted, $this->limit)); } if (0 < $tokens) { diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php index e4bb38edd96ba..3acdedb5466a6 100644 --- a/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php +++ b/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php @@ -57,6 +57,21 @@ public function testConsume() $this->assertEquals($retryAfter, $rateLimit->getRetryAfter()); } + public function testConsumeLastToken() + { + $now = time(); + $limiter = $this->createLimiter(); + $limiter->consume(9); + + $rateLimit = $limiter->consume(1); + $this->assertSame(0, $rateLimit->getRemainingTokens()); + $this->assertTrue($rateLimit->isAccepted()); + $this->assertEquals( + \DateTimeImmutable::createFromFormat('U', $now + 60), + $rateLimit->getRetryAfter() + ); + } + /** * @dataProvider provideConsumeOutsideInterval */ diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php index 5ac98c5ad5700..bafe0948cbb39 100644 --- a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php +++ b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php @@ -54,6 +54,21 @@ public function testConsume() $this->assertSame(10, $rateLimit->getLimit()); } + public function testConsumeLastToken() + { + $limiter = $this->createLimiter(); + $limiter->reset(); + $limiter->consume(9); + + $rateLimit = $limiter->consume(1); + $this->assertSame(0, $rateLimit->getRemainingTokens()); + $this->assertTrue($rateLimit->isAccepted()); + $this->assertEquals( + \DateTimeImmutable::createFromFormat('U', (string) floor(microtime(true) + 12 / 10)), + $rateLimit->getRetryAfter() + ); + } + public function testConsumeZeroTokens() { $limiter = $this->createLimiter(); @@ -70,7 +85,7 @@ public function testConsumeZeroTokens() $limiter->consume(10); $rateLimit = $limiter->consume(0); - $this->assertFalse($rateLimit->isAccepted()); + $this->assertTrue($rateLimit->isAccepted()); $this->assertEquals( \DateTimeImmutable::createFromFormat('U', (string) floor(microtime(true) + 12 / 10)), $rateLimit->getRetryAfter() From c15d76d41d08e2fb0c952464ff9dca5ec358be4c Mon Sep 17 00:00:00 2001 From: Evgeny Ruban Date: Wed, 13 Dec 2023 21:59:18 +0400 Subject: [PATCH 7/7] [RateLimiter] Allow to get RateLimit without consuming again. --- .../RateLimiter/Policy/FixedWindowLimiter.php | 20 ++++--------------- .../Policy/SlidingWindowLimiter.php | 4 +++- .../Tests/Policy/FixedWindowLimiterTest.php | 15 -------------- .../Tests/Policy/SlidingWindowLimiterTest.php | 3 +++ 4 files changed, 10 insertions(+), 32 deletions(-) diff --git a/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php b/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php index f8106dac61520..05fab322f03be 100644 --- a/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php +++ b/src/Symfony/Component/RateLimiter/Policy/FixedWindowLimiter.php @@ -59,33 +59,21 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation $now = microtime(true); $availableTokens = $window->getAvailableTokens($now); - if (0 !== $tokens && $availableTokens > $tokens) { + if ($availableTokens >= max(1, $tokens)) { $window->add($tokens, $now); $reservation = new Reservation($now, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now)), true, $this->limit)); } else { - if ($availableTokens === $tokens) { - $window->add($tokens, $now); - } - $waitDuration = $window->calculateTimeForTokens(max(1, $tokens), $now); - if ($availableTokens !== $tokens && 0 !== $tokens && null !== $maxTime && $waitDuration > $maxTime) { + if (null !== $maxTime && $waitDuration > $maxTime) { // process needs to wait longer than set interval throw new MaxWaitDurationExceededException(sprintf('The rate limiter wait time ("%d" seconds) is longer than the provided maximum time ("%d" seconds).', $waitDuration, $maxTime), new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), false, $this->limit)); } - if ($availableTokens !== $tokens) { - $window->add($tokens, $now); - } - - if ($availableTokens === $tokens || 0 === $tokens) { - $accepted = true; - } else { - $accepted = false; - } + $window->add($tokens, $now); - $reservation = new Reservation($now + $waitDuration, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), $accepted, $this->limit)); + $reservation = new Reservation($now + $waitDuration, new RateLimit($window->getAvailableTokens($now), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), false, $this->limit)); } if (0 < $tokens) { diff --git a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php index 3cdeca120bdf0..75715cce62eaf 100644 --- a/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php +++ b/src/Symfony/Component/RateLimiter/Policy/SlidingWindowLimiter.php @@ -87,11 +87,13 @@ public function reserve(int $tokens = 1, float $maxTime = null): Reservation if ($availableTokens === $tokens || 0 === $tokens) { $accepted = true; + $timeToAct = $now; } else { $accepted = false; + $timeToAct = $now + $waitDuration; } - $reservation = new Reservation($now + $waitDuration, new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), $accepted, $this->limit)); + $reservation = new Reservation($timeToAct, new RateLimit($this->getAvailableTokens($window->getHitCount()), \DateTimeImmutable::createFromFormat('U', floor($now + $waitDuration)), $accepted, $this->limit)); } if (0 < $tokens) { diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php index 3acdedb5466a6..e4bb38edd96ba 100644 --- a/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php +++ b/src/Symfony/Component/RateLimiter/Tests/Policy/FixedWindowLimiterTest.php @@ -57,21 +57,6 @@ public function testConsume() $this->assertEquals($retryAfter, $rateLimit->getRetryAfter()); } - public function testConsumeLastToken() - { - $now = time(); - $limiter = $this->createLimiter(); - $limiter->consume(9); - - $rateLimit = $limiter->consume(1); - $this->assertSame(0, $rateLimit->getRemainingTokens()); - $this->assertTrue($rateLimit->isAccepted()); - $this->assertEquals( - \DateTimeImmutable::createFromFormat('U', $now + 60), - $rateLimit->getRetryAfter() - ); - } - /** * @dataProvider provideConsumeOutsideInterval */ diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php index bafe0948cbb39..f6e15a10255b0 100644 --- a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php +++ b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowLimiterTest.php @@ -114,6 +114,9 @@ public function testReserve() // 2 over the limit, causing the WaitDuration to become 2/10th of the 12s interval $this->assertEqualsWithDelta(12 / 5, $limiter->reserve(4)->getWaitDuration(), 1); + + $limiter->reset(); + $this->assertEquals(0, $limiter->reserve(10)->getWaitDuration()); } private function createLimiter(): SlidingWindowLimiter