From 7691b2d96e2fd5d86f191ea79333c42395c1273d Mon Sep 17 00:00:00 2001 From: Jerzy Lekowski Date: Sat, 23 Oct 2021 13:42:42 +0100 Subject: [PATCH] bug #42194 [RateLimiter] fix: sliding window policy to use microtime --- .../RateLimiter/Policy/NoLimiter.php | 2 +- .../RateLimiter/Policy/SlidingWindow.php | 12 ++++---- .../Component/RateLimiter/RateLimit.php | 4 +-- .../Tests/Policy/SlidingWindowTest.php | 30 +++++++++++++++++++ .../RateLimiter/Tests/RateLimitTest.php | 14 +++++++++ 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/Symfony/Component/RateLimiter/Policy/NoLimiter.php b/src/Symfony/Component/RateLimiter/Policy/NoLimiter.php index da66028eccdd4..4878e4abde7a8 100644 --- a/src/Symfony/Component/RateLimiter/Policy/NoLimiter.php +++ b/src/Symfony/Component/RateLimiter/Policy/NoLimiter.php @@ -27,7 +27,7 @@ final class NoLimiter implements LimiterInterface { public function reserve(int $tokens = 1, float $maxTime = null): Reservation { - return new Reservation(time(), new RateLimit(\PHP_INT_MAX, new \DateTimeImmutable(), true, \PHP_INT_MAX)); + return new Reservation(microtime(true), new RateLimit(\PHP_INT_MAX, new \DateTimeImmutable(), true, \PHP_INT_MAX)); } public function consume(int $tokens = 1): RateLimit diff --git a/src/Symfony/Component/RateLimiter/Policy/SlidingWindow.php b/src/Symfony/Component/RateLimiter/Policy/SlidingWindow.php index 7dd297b2cbe0e..cb27197f9ee87 100644 --- a/src/Symfony/Component/RateLimiter/Policy/SlidingWindow.php +++ b/src/Symfony/Component/RateLimiter/Policy/SlidingWindow.php @@ -39,7 +39,7 @@ final class SlidingWindow implements LimiterStateInterface private $intervalInSeconds; /** - * @var int the unix timestamp when the current window ends + * @var float the unix timestamp when the current window ends */ private $windowEndAt; @@ -55,7 +55,7 @@ public function __construct(string $id, int $intervalInSeconds) } $this->id = $id; $this->intervalInSeconds = $intervalInSeconds; - $this->windowEndAt = time() + $intervalInSeconds; + $this->windowEndAt = microtime(true) + $intervalInSeconds; $this->cached = false; } @@ -64,7 +64,7 @@ public static function createFromPreviousWindow(self $window, int $intervalInSec $new = new self($window->id, $intervalInSeconds); $windowEndAt = $window->windowEndAt + $intervalInSeconds; - if (time() < $windowEndAt) { + if (microtime(true) < $windowEndAt) { $new->hitCountForLastWindow = $window->hitCount; $new->windowEndAt = $windowEndAt; } @@ -101,7 +101,7 @@ public function getExpirationTime(): ?int public function isExpired(): bool { - return time() > $this->windowEndAt; + return microtime(true) > $this->windowEndAt; } public function add(int $hits = 1) @@ -115,13 +115,13 @@ public function add(int $hits = 1) public function getHitCount(): int { $startOfWindow = $this->windowEndAt - $this->intervalInSeconds; - $percentOfCurrentTimeFrame = min((time() - $startOfWindow) / $this->intervalInSeconds, 1); + $percentOfCurrentTimeFrame = min((microtime(true) - $startOfWindow) / $this->intervalInSeconds, 1); return (int) floor($this->hitCountForLastWindow * (1 - $percentOfCurrentTimeFrame) + $this->hitCount); } public function getRetryAfter(): \DateTimeImmutable { - return \DateTimeImmutable::createFromFormat('U', $this->windowEndAt); + return \DateTimeImmutable::createFromFormat('U.u', $this->windowEndAt); } } diff --git a/src/Symfony/Component/RateLimiter/RateLimit.php b/src/Symfony/Component/RateLimiter/RateLimit.php index ac1b4e0804bf9..ccc6a79a91baa 100644 --- a/src/Symfony/Component/RateLimiter/RateLimit.php +++ b/src/Symfony/Component/RateLimiter/RateLimit.php @@ -67,11 +67,11 @@ public function getLimit(): int public function wait(): void { - $delta = $this->retryAfter->getTimestamp() - time(); + $delta = $this->retryAfter->format('U.u') - microtime(true); if ($delta <= 0) { return; } - sleep($delta); + usleep($delta * 1e6); } } diff --git a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowTest.php b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowTest.php index 4a9ace9ec287b..f81784706c185 100644 --- a/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowTest.php +++ b/src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowTest.php @@ -72,4 +72,34 @@ public function testLongIntervalCreate() $new = SlidingWindow::createFromPreviousWindow($window, 60); $this->assertFalse($new->isExpired()); } + + public function testCreateFromPreviousWindowUsesMicrotime() + { + ClockMock::register(SlidingWindow::class); + $window = new SlidingWindow('foo', 8); + + usleep(11.6 * 1e6); // wait just under 12s (8+4) + $new = SlidingWindow::createFromPreviousWindow($window, 4); + + // should be 400ms left (12 - 11.6) + $this->assertEqualsWithDelta(0.4, $new->getRetryAfter()->format('U.u') - microtime(true), 0.2); + } + + public function testIsExpiredUsesMicrotime() + { + ClockMock::register(SlidingWindow::class); + $window = new SlidingWindow('foo', 10); + + usleep(10.1 * 1e6); + $this->assertTrue($window->isExpired()); + } + + public function testGetRetryAfterUsesMicrotime() + { + $window = new SlidingWindow('foo', 10); + + usleep(9.5 * 1e6); + // should be 500ms left (10 - 9.5) + $this->assertEqualsWithDelta(0.5, $window->getRetryAfter()->format('U.u') - microtime(true), 0.2); + } } diff --git a/src/Symfony/Component/RateLimiter/Tests/RateLimitTest.php b/src/Symfony/Component/RateLimiter/Tests/RateLimitTest.php index c62eda36f4fd2..612c028cc9035 100644 --- a/src/Symfony/Component/RateLimiter/Tests/RateLimitTest.php +++ b/src/Symfony/Component/RateLimiter/Tests/RateLimitTest.php @@ -12,9 +12,13 @@ namespace Symfony\Component\RateLimiter\Tests; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ClockMock; use Symfony\Component\RateLimiter\Exception\RateLimitExceededException; use Symfony\Component\RateLimiter\RateLimit; +/** + * @group time-sensitive + */ class RateLimitTest extends TestCase { public function testEnsureAcceptedDoesNotThrowExceptionIfAccepted() @@ -40,4 +44,14 @@ public function testEnsureAcceptedThrowsRateLimitExceptionIfNotAccepted() $this->fail('RateLimitExceededException not thrown.'); } + + public function testWaitUsesMicrotime() + { + ClockMock::register(RateLimit::class); + $rateLimit = new RateLimit(10, new \DateTimeImmutable('+2500 ms'), true, 10); + + $start = microtime(true); + $rateLimit->wait(); // wait 2.5 seconds + $this->assertEqualsWithDelta($start + 2.5, microtime(true), 0.1); + } }