Skip to content

[RateLimiter] bug #42194 fix: sliding window policy to use microtime #43677

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 24, 2021
Merged

[RateLimiter] bug #42194 fix: sliding window policy to use microtime #43677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Symfony/Component/RateLimiter/Policy/NoLimiter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions src/Symfony/Component/RateLimiter/Policy/SlidingWindow.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}

Expand All @@ -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;
}
Expand Down Expand Up @@ -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)
Expand All @@ -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);
}
}
4 changes: 2 additions & 2 deletions src/Symfony/Component/RateLimiter/RateLimit.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
30 changes: 30 additions & 0 deletions src/Symfony/Component/RateLimiter/Tests/Policy/SlidingWindowTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,34 @@ public function testLongIntervalCreate()
$new = SlidingWindow::createFromPreviousWindow($window, 60);
$this->assertFalse($new->isExpired());
}

public function testCreateFromPreviousWindowUsesMicrotime()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These tests consistently pass after the changes. The old code would inconsistently fail, though (depend on the microtome(true) result at the moment of running the test.

{
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);
}
}
14 changes: 14 additions & 0 deletions src/Symfony/Component/RateLimiter/Tests/RateLimitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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);
}
}