8000 bug #47283 [HttpFoundation] Prevent accepted rate limits with no rema… · symfony/symfony@5294748 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5294748

Browse files
fabpotfabpot
committed
bug #47283 [HttpFoundation] Prevent accepted rate limits with no remaining token to be preferred over denied ones (MatTheCat)
This PR was squashed before being merged into the 5.4 branch. Discussion ---------- [HttpFoundation] Prevent accepted rate limits with no remaining token to be preferred over denied ones | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #47282 | License | MIT | Doc PR | N/A When a rate limit is first accepted with no remaining token, following rejected rate limits will be ignored because they also will have no remaining token. The request would then be processed. Commits ------- 8188f1c [HttpFoundation] Prevent accepted rate limits with no remaining token to be preferred over denied ones
2 parents cb3684e + 8188f1c commit 5294748

File tree

4 files changed

+117
-4
lines changed

4 files changed

+117
-4
lines changed

‎src/Symfony/Component/HttpFoundation/RateLimiter/AbstractRequestRateLimiter.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ public function consume(Request $request): RateLimit
3535
foreach ($limiters as $limiter) {
3636
$rateLimit = $limiter->consume(1);
3737

38-
if (null === $minimalRateLimit || $rateLimit->getRemainingTokens() < $minimalRateLimit->getRemainingTokens()) {
39-
$minimalRateLimit = $rateLimit;
40-
}
38+
$minimalRateLimit = $minimalRateLimit ? self::getMinimalRateLimit($minimalRateLimit, $rateLimit) : $rateLimit;
4139
}
4240

4341
return $minimalRateLimit;
@@ -54,4 +52,20 @@ public function reset(Request $request): void
5452
* @return LimiterInterface[] a set of limiters using keys extracted from the request
5553
*/
5654
abstract protected function getLimiters(Request $request): array;
55+
56+
private static function getMinimalRateLimit(RateLimit $first, RateLimit $second): RateLimit
57+
{
58+
if ($first->isAccepted() !== $second->isAccepted()) {
59+
return $first->isAccepted() ? $second : $first;
60+
}
61+
62+
$firstRemainingTokens = $first->getRemainingTokens();
63+
$secondRemainingTokens = $second->getRemainingTokens();
64+
65+
if ($firstRemainingTokens === $secondRemainingTokens) {
66+
return $first->getRetryAfter() < $second->getRetryAfter() ? $second : $first;
67+
}
68+
69+
return $firstRemainingTokens > $secondRemainingTokens ? $second : $first;
70+
}
5771
}

‎src/Symfony/Component/HttpFoundation/Tests/RateLimiter/AbstractRequestRateLimiterTest.php

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\Tests\RateLimiter;
13+
14+
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\HttpFoundation\Request;
16+
use Symfony\Component\RateLimiter\LimiterInterface;
17+
use Symfony\Component\RateLimiter\RateLimit;
18+
19+
class AbstractRequestRateLimiterTest extends TestCase
20+
{
21+
/**
22+
* @dataProvider provideRateLimits
23+
*/
24+
public function testConsume(array $rateLimits, ?RateLimit $expected)
25+
{
26+
$rateLimiter = new MockAbstractRequestRateLimiter(array_map(function (RateLimit $rateLimit) {
27+
$limiter = $this->createStub(LimiterInterface::class);
28+
$limiter->method('consume')->willReturn($rateLimit);
29+
30+
return $limiter;
31+
}, $rateLimits));
32+
33+
$this->assertSame($expected, $rateLimiter->consume(new Request()));
34+
}
35+
36+
public function provideRateLimits()
37+
{
38+
$now = new \DateTimeImmutable();
39+
40+
yield 'Both accepted with different count of remaining tokens' => [
41+
[
42+
$expected = new RateLimit(0, $now, true, 1), // less remaining tokens
43+
new RateLimit(1, $now, true, 1),
44+
],
45+
$expected,
46+
];
47+
48+
yield 'Both accepted with same count of remaining tokens' => [
49+
[
50+
$expected = new RateLimit(0, $now->add(new \DateInterval('P1D')), true, 1), // longest wait time
51+
new RateLimit(0, $now, true, 1),
52+
],
53+
$expected,
54+
];
55+
56+
yield 'Accepted and denied' => [
57+
[
58+
new RateLimit(0, $now, true, 1),
59+
$expected = new RateLimit(0, $now, false, 1), // denied
60+
],
61+
$expected,
62+
];
63+
}
64+
}

‎src/Symfony/Component/HttpFoundation/Tests/RateLimiter/MockAbstractRequestRateLimiter.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\Tests\RateLimiter;
13+
14+
use Symfony\Component\HttpFoundation\RateLimiter\AbstractRequestRateLimiter;
15+
use Symfony\Component\HttpFoundation\Request;
16+
use Symfony\Component\RateLimiter\LimiterInterface;
17+
18+
class MockAbstractRequestRateLimiter extends AbstractRequestRateLimiter
19+
{
20+
/**
21+
* @var LimiterInterface[]
22+
*/
23+
private $limiters;
24+
25+
public function __construct(array $limiters)
26+
{
27+
$this->limiters = $limiters;
28+
}
29+
30+
protected function getLimiters(Request $request): array
31+
{
32+
return $this->limiters;
33+
}
34+
}

‎src/Symfony/Component/HttpFoundation/composer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@
2727
"symfony/dependency-injection": "^5.4|^6.0",
2828
"symfony/http-kernel": "^5.4.12|^6.0.12|^6.1.4",
2929
"symfony/mime": "^4.4|^5.0|^6.0",
30-
"symfony/expression-language": "^4.4|^5.0|^6.0"
30+
"symfony/expression-language": "^4.4|^5.0|^6.0",
31+
"symfony/rate-limiter": "^5.2|^6.0"
3132
},
3233
"suggest" : {
3334
"symfony/mime": "To use the file extension guesser"

0 commit comments

Comments
 (0)
0