8000 [HttpFoundation] Extract request matchers for better reusability · symfony/symfony@6cfd3b7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6cfd3b7

Browse files
fabpotfabpot
committed
[HttpFoundation] Extract request matchers for better reusability
1 parent 58117d7 commit 6cfd3b7

29 files changed

+931
-44
lines changed

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,13 @@
3333
use Symfony\Component\EventDispatcher\EventDispatcher;
3434
use Symfony\Component\ExpressionLanguage\Expression;
3535
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
36-
use Symfony\Component\HttpFoundation\RequestMatcher;
36+
use Symfony\Component\HttpFoundation\ChainRequestMatcher;
37+
use Symfony\Component\HttpFoundation\RequestMatcher\AttributesRequestMatcher;
38+
use Symfony\Component\HttpFoundation\RequestMatcher\HostRequestMatcher;
39+
use Symfony\Component\HttpFoundation\RequestMatcher\IpsRequestMatcher;
40+
use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher;
41+
use Symfony\Component\HttpFou F438 ndation\RequestMatcher\PathRequestMatcher;
42+
use Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher;
3743
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
3844
use Symfony\Component\HttpKernel\KernelEvents;
3945
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
@@ -893,7 +899,7 @@ private function createRequestMatcher(ContainerBuilder $container, string $path
893899
$methods = array_map('strtoupper', $methods);
894900
}
895901

896-
if (null !== $ips) {
902+
if ($ips) {
897903
foreach ($ips as $ip) {
898904
$container->resolveEnvPlaceholders($ip, null, $usedEnvs);
899905

@@ -905,22 +911,58 @@ private function createRequestMatcher(ContainerBuilder $container, string $path
905911
}
906912
}
907913

908-
$id = '.security.request_matcher.'.ContainerBuilder::hash([$path, $host, $port, $methods, $ips, $attributes]);
914+
$id = '.security.request_matcher.'.ContainerBuilder::hash([ChainRequestMatcher::class, $path, $host, $port, $methods, $ips, $attributes]);
909915

910916
if (isset($this->requestMatchers[$id])) {
911917
return $this->requestMatchers[$id];
912918
}
913919

914-
// only add arguments that are necessary
915-
$arguments = [$path, $host, $methods, $ips, $attributes, null, $port];
916-
while (\count($arguments) > 0 && !end($arguments)) {
917-
array_pop($arguments);
920+
$arguments = [];
921+
if ($methods) {
922+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([MethodRequestMatcher::class, $methods]))) {
923+
$container->register($lid, MethodRequestMatcher::class)->setArguments([$methods]);
924+
}
925+
$arguments[] = new Reference($lid);
926+
}
927+
928+
if ($path) {
929+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([PathRequestMatcher::class, $path]))) {
930+
$container->register($lid, PathRequestMatcher::class)->setArguments([$path]);
931+
}
932+
$arguments[] = new Reference($lid);
933+
}
934+
935+
if ($host) {
936+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([HostRequestMatcher::class, $host]))) {
937+
$container->register($lid, HostRequestMatcher::class)->setArguments([$host]);
938+
}
939+
$arguments[] = new Reference($lid);
940+
}
941+
942+
if ($ips) {
943+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([IpsRequestMatcher::class, $ips]))) {
944+
$container->register($lid, IpsRequestMatcher::class)->setArguments([$ips]);
945+
}
946+
$arguments[] = new Reference($lid);
947+
}
948+
949+
if ($attributes) {
950+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([AttributesRequestMatcher::class, $attributes]))) {
951+
$container->register($lid, AttributesRequestMatcher::class)->setArguments([$attributes]);
952+
}
953+
$arguments[] = new Reference($lid);
954+
}
955+
956+
if ($port) {
957+
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([PortRequestMatcher::class, $port]))) {
958+
$container->register($lid, PortRequestMatcher::class)->setArguments([$port]);
959+
}
960+
$arguments[] = new Reference($lid);
918961
}
919962

920963
$container
921-
->register($id, RequestMatcher::class)
922-
->setPublic(false)
923-
->setArguments($arguments)
964+
->register($id, ChainRequestMatcher::class)
965+
->setArguments([$arguments])
924966
;
925967

926968
return $this->requestMatchers[$id] = new Reference($id);

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
use Symfony\Component\DependencyInjection\ContainerBuilder;
2020
use Symfony\Component\DependencyInjection\Definition;
2121
use Symfony\Component\DependencyInjection\Reference;
22+
use Symfony\Component\HttpFoundation\RequestMatcher\HostRequestMatcher;
23+
use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher;
24+
use Symfony\Component\HttpFoundation\RequestMatcher\PathRequestMatcher;
25+
use Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher;
2226
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
2327
use Symfony\Component\PasswordHasher\Hasher\Pbkdf2PasswordHasher;
2428
use Symfony\Component\PasswordHasher\Hasher\PlaintextPasswordHasher;
@@ -131,7 +135,7 @@ public function testFirewalls()
131135
[
132136
'simple',
133137
'security.user_checker',
134-
'.security.request_matcher.xmi9dcw',
138+
'.security.request_matcher.h5ibf38',
135139
false,
136140
false,
137141
'',
@@ -180,7 +184,7 @@ public function testFirewalls()
180184
[
181185
'host',
182186
'security.user_checker',
183-
'.security.request_matcher.iw4hyjb',
187+
'.security.request_matcher.bcmu4fb',
184188
true,
185189
false,
186190
'security.user.provider.concrete.default',
@@ -248,20 +252,27 @@ public function testFirewallRequestMatchers()
248252
foreach ($arguments[1]->getValues() as $reference) {
249253
if ($reference instanceof Reference) {
250254
$definition = $container->getDefinition((string) $reference);
251-
$matchers[] = $definition->getArguments();
255+
$matchers[] = $definition->getArgument(0);
252256
}
253257
}
254258

255-
$this->assertEquals([
256-
[
257-
'/login',
258-
],
259-
[
260-
'/test',
261-
'foo\\.example\\.org',
262-
['GET', 'POST'],
263-
],
264-
], $matchers);
259+
$this->assertCount(2, $matchers);
260+
261+
$this->assertCount(1, $matchers[0]);
262+
$def = $container->getDefinition((string) $matchers[0][0]);
263+
$this->assertSame(PathRequestMatcher::class, $def->getClass());
264+
$this->assertSame('/login', $def->getArgument(0));
265+
266+
$this->assertCount(3, $matchers[1]);
267+
$def = $container->getDefinition((string) $matchers[1][0]);
268+
$this->assertSame(MethodRequestMatcher::class, $def->getClass());
269+
$this->assertSame(['GET', 'POST'], $def->getArgument(0));
270+
$def = $container->getDefinition((string) $matchers[1][1]);
271+
$this->assertSame(PathRequestMatcher::class, $def->getClass());
272+
$this->assertSame('/test', $def->getArgument(0));
273+
$def = $container->getDefinition((string) $matchers[1][2]);
274+
$this->assertSame(HostRequestMatcher::class, $def->getClass());
275+
$this->assertSame('foo\\.example\\.org', $def->getArgument(0));
265276
}
266277

267278
public function testUserCheckerAliasIsRegistered()
@@ -294,17 +305,23 @@ public function testAccess()
294305
if (1 === $i) {
295306
$this->assertEquals(['ROLE_USER'], $attributes);
296307
$this->assertEquals('https', $channel);
297-
$this->assertEquals(
298-
['/blog/524', null, ['GET', 'POST'], [], [], null, 8000],
299-
$requestMatcher->getArguments()
300-
);
308+
$this->assertCount(3, $requestMatcher->getArgument(0));
309+
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[0]);
310+
$this->assertSame(MethodRequestMatcher::class, $def->getClass());
311+
$this->assertSame(['GET', 'POST'], $def->getArgument(0));
312+
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[1]);
313+
$this->assertSame(PathRequestMatcher::class, $def->getClass());
314+
$this->assertSame('/blog/524', $def->getArgument(0));
315+
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[2]);
316+
$this->assertSame(PortRequestMatcher::class, $def->getClass());
317+
$this->assertSame(8000, $def->getArgument(0));
301318
} elseif (2 === $i) {
302319
$this->assertEquals(['IS_AUTHENTICATED_ANONYMOUSLY'], $attributes);
303320
$this->assertNull($channel);
304-
$this->assertEquals(
305-
['/blog/.*'],
306-
$requestMatcher->getArguments()
307-
);
321+
$this->assertCount(1, $requestMatcher->getArgument(0));
322+
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[0]);
323+
$this->assertSame(PathRequestMatcher::class, $def->getClass());
324+
$this->assertSame('/blog/.*', $def->getArgument(0));
308325
} elseif (3 === $i) {
309326
$this->assertEquals('IS_AUTHENTICATED_ANONYMOUSLY', $attributes[0]);
310327
$expression = $container->getDefinition((string) $attributes[1])->getArgument(0);

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
use Symfony\Component\DependencyInjection\Reference;
2727
use Symfony\Component\ExpressionLanguage\Expression;
2828
use Symfony\Component\HttpFoundation\Request;
29-
use Symfony\Component\HttpFoundation\RequestMatcher;
29+
use Symfony\Component\HttpFoundation\RequestMatcher\PathRequestMatcher;
3030
use Symfony\Component\HttpFoundation\Response;
3131
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
3232
use Symfony\Component\Security\Core\Exception\AuthenticationException;
@@ -256,7 +256,7 @@ public function testRegisterAccessControlWithSpecifiedRequestMatcherService()
256256
$container = $this->getRawContainer();
257257

258258
$requestMatcherId = 'My\Test\RequestMatcher';
259-
$requestMatcher = new RequestMatcher('/');
259+
$requestMatcher = new PathRequestMatcher('/');
260260
$container->set($requestMatcherId, $requestMatcher);
261261

262262
$container->loadFromExtension('security', [
@@ -291,7 +291,7 @@ public function testRegisterAccessControlWithRequestMatcherAndAdditionalOptionsT
291291
$container = $this->getRawContainer();
292292

293293
$requestMatcherId = 'My\Test\RequestMatcher';
294-
$requestMatcher = new RequestMatcher('/');
294+
$requestMatcher = new PathRequestMatcher('/');
295295
$container->set($requestMatcherId, $requestMatcher);
296296

297297
$container->loadFromExtension('security', [

‎src/Symfony/Bundle/SecurityBundle/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
"symfony/dependency-injection": "^6.2",
2424
"symfony/event-dispatcher": "^5.4|^6.0",
2525
"symfony/http-kernel": "^6.2",
26-
"symfony/http-foundation": "^5.4|^6.0",
26+
"symfony/http-foundation": "^6.2",
2727
"symfony/password-hasher": "^5.4|^6.0",
2828
"symfony/security-core": "^6.2",
2929
"symfony/security-csrf": "^5.4|^6.0",

‎src/Symfony/Component/HttpFoundation/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ CHANGELOG
66

77
* The HTTP cache store uses the `xxh128` algorithm
88
* Deprecate calling `JsonResponse::setCallback()`, `Response::setExpires/setLastModified/setEtag()`, `MockArraySessionStorage/NativeSessionStorage::setMetadataBag()`, `NativeSessionStorage::setSaveHandler()` without arguments
9+
* Add request matchers under the `Symfony\Component\HttpFoundation\RequestMatcher` namespace
10+
* Deprecate `RequestMatcher` in favor of `ChainRequestMatcher`
11+
* Deprecate `Symfony\Component\HttpFoundation\ExpressionRequestMatcher` in favor of `Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher`
912

1013
6.1
1114
---

‎src/Symfony/Component/HttpFoundation/ChainRequestMatcher.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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;
13+
14+
/**
15+
* ChainRequestMatcher verifies that all checks match against a Request instance.
16+
*
17+
* @author Fabien Potencier <fabien@symfony.com>
18+
*/
19+
class ChainRequestMatcher implements RequestMatcherInterface
20+
{
21+
/**
22+
* @param iterable<RequestMatcherInterface> $matchers
23+
*/
24+
public function __construct(private iterable $matchers)
25+
{
26+
}
27+
28+
public function matches(Request $request): bool
29+
{
30+
foreach ($this->matchers as $matcher) {
31+
if (!$matcher->matches($request)) {
32+
return false;
33+
}
34+
}
35+
36+
return true;
37+
}
38+
}

‎src/Symfony/Component/HttpFoundation/ExpressionRequestMatcher.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,16 @@
1313

1414
use Symfony\Component\ExpressionLanguage\Expression;
1515
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
16+
use Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher as NewExpressionRequestMatcher;
17+
18+
trigger_deprecation('symfony/http-foundation', '6.2', 'The "%s" class is deprecated, use "%s" instead.', ExpressionRequestMatcher::class, NewExpressionRequestMatcher::class);
1619

1720
/**
1821
* ExpressionRequestMatcher uses an expression to match a Request.
1922
*
2023
* @author Fabien Potencier <fabien@symfony.com>
24+
*
25+
* @deprecated since Symfony 6.2, use "Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher" instead
2126
*/
2227
class ExpressionRequestMatcher extends RequestMatcher
2328
{

‎src/Symfony/Component/HttpFoundation/RequestMatcher.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,14 @@
1111

1212
namespace Symfony\Component\HttpFoundation;
1313

14+
trigger_deprecation('symfony/http-foundation', '6.2', 'The "%s" class is deprecated, use "%s" instead.', RequestMatcher::class, ChainRequestMatcher::class);
15+
1416
/**
1517
* RequestMatcher compares a pre-defined set of checks against a Request instance.
1618
*
1719
* @author Fabien Potencier <fabien@symfony.com>
20+
*
21+
* @deprecated since Symfony 6.2, use ChainRequestMatcher instead
1822
*/
1923
class RequestMatcher implements RequestMatcherInterface
2024
{

‎src/Symfony/Component/HttpFoundation/RequestMatcher/AttributesRequestMatcher.php

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
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\RequestMatcher;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\HttpFoundation\RequestMatcherInterface;
16+
17+
/**
18+
* Checks the Request attributes matches all regular expressions.
19+
*
20+
* @author Fabien Potencier <fabien@symfony.com>
21+
*/
22+
class AttributesRequestMatcher implements RequestMatcherInterface
23+
{
24+
/**
25+
* @param array<string, string> $regexps
26+
*/
27+
public function __construct(private array $regexps)
28+
{
29+
}
30+
31+
public function matches(Request $request): bool
32+
{
33+
foreach ($this->regexps as $key => $regexp) {
34+
$attribute = $request->attributes->get($key);
35+
if (!\is_string($attribute)) {
36+
return false;
37+
}
38+
if (!preg_match('{'.$regexp.'}', $attribute)) {
39+
return false;
40+
}
41+
}
42+
43+
return true;
44+
}
45+
}

‎src/Symfony/Component/HttpFoundation/RequestMatcher/ExpressionRequestMatcher.php

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
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\RequestMatcher;
13+
14+
use Symfony\Component\ExpressionLanguage\Expression;
15+
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
16+
use Symfony\Component\HttpFoundation\Request;
17+
use Symfony\Component\HttpFoundation\RequestMatcherInterface;
18+
19+
/**
20+
* ExpressionRequestMatcher uses an expression to match a Request.
21+
*
22+
* @author Fabien Potencier <fabien@symfony.com>
23+
*/
24+
class ExpressionRequestMatcher implements RequestMatcherInterface
25+
{
26+
public function __construct(
27+
private ExpressionLanguage $language,
28+
private Expression|string $expression,
29+
) {
30+
}
31+
32+
public function matches(Request $request): bool
33+
{
34+
return $this->language->evaluate($this->expression, [
35+
'request' => $request,
36+
'method' => $request->getMethod(),
37+
'path' => rawurldecode($request->getPathInfo()),
38+
'host' => $request->getHost(),
39+
'ip' => $request->getClientIp(),
40+
'attributes' => $request->attributes->all(),
41+
]);
42+
}
43+
}

0 commit comments

Comments
 (0)
0