Skip to content

Commit

Permalink
[RateLimiter] Fix DateInterval normalization
Browse files Browse the repository at this point in the history
  • Loading branch information
danydev committed Nov 5, 2024
1 parent b3e3efa commit c739355
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 2 deletions.
6 changes: 5 additions & 1 deletion RateLimiterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ protected static function configureOptions(OptionsResolver $options): void
{
$intervalNormalizer = static function (Options $options, string $interval): \DateInterval {
try {
return (new \DateTimeImmutable())->diff(new \DateTimeImmutable('+'.$interval));
// Create DateTimeImmutable from unix timesatmp, so the default timezone is ignored and we don't need to
// deal with quirks happening when modifying dates using a timezone with DST.
$now = \DateTimeImmutable::createFromFormat('U', time());

return $now->diff($now->modify('+'.$interval));
} catch (\Exception $e) {
if (!preg_match('/Failed to parse time string \(\+([^)]+)\)/', $e->getMessage(), $m)) {
throw $e;
Expand Down
34 changes: 34 additions & 0 deletions Tests/RateLimiterFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\RateLimiter\Tests;

use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\ClockMock;
use Symfony\Component\OptionsResolver\Exception\MissingOptionsException;
use Symfony\Component\RateLimiter\Policy\FixedWindowLimiter;
use Symfony\Component\RateLimiter\Policy\NoLimiter;
Expand Down Expand Up @@ -76,4 +77,37 @@ public static function invalidConfigProvider()
'policy' => 'token_bucket',
]];
}

/**
* @group time-sensitive
*/
public function testExpirationTimeCalculationWhenUsingDefaultTimezoneRomeWithIntervalAfterCETChange()
{
$originalTimezone = date_default_timezone_get();
try {
// Timestamp for 'Sun 27 Oct 2024 12:59:40 AM UTC' that's just 20 seconds before switch CEST->CET
ClockMock::withClockMock(1729990780);

// This is a prerequisite for the bug to happen
date_default_timezone_set('Europe/Rome');

$storage = new InMemoryStorage();
$factory = new RateLimiterFactory(
[
'id' => 'id_1',
'policy' => 'fixed_window',
'limit' => 30,
'interval' => '21 seconds',
],
$storage
);
$rateLimiter = $factory->create('key');
$rateLimiter->consume(1);
$limiterState = $storage->fetch('id_1-key');
// As expected the expiration is equal to the interval we defined
$this->assertSame(21, $limiterState->getExpirationTime());
} finally {
date_default_timezone_set($originalTimezone);
}
}
}
2 changes: 1 addition & 1 deletion Util/TimeUtil.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ final class TimeUtil
{
public static function dateIntervalToSeconds(\DateInterval $interval): int
{
$now = new \DateTimeImmutable();
$now = \DateTimeImmutable::createFromFormat('U', time());

return $now->add($interval)->getTimestamp() - $now->getTimestamp();
}
Expand Down

0 comments on commit c739355

Please sign in to comment.