Skip to content

Commit

Permalink
feature #47595 [HttpFoundation] Extract request matchers for better r…
Browse files Browse the repository at this point in the history
…eusability (fabpot)

This PR was merged into the 6.2 branch.

Discussion
----------

[HttpFoundation] Extract request matchers for better reusability

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | n/a <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        |

The `RequestMatcher` class hardcodes its matchers. This PR extracts those into their own classes so that we can compose a `RequestMatcher` and allow better reusability.

Commits
-------

6cfd3b7 [HttpFoundation] Extract request matchers for better reusability
  • Loading branch information
fabpot committed Sep 18, 2022
2 parents 2c6b67b + 6cfd3b7 commit 77538c8
Show file tree
Hide file tree
Showing 29 changed files with 931 additions and 44 deletions.
Expand Up @@ -33,7 +33,13 @@
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\ExpressionLanguage\Expression;
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
use Symfony\Component\HttpFoundation\RequestMatcher;
use Symfony\Component\HttpFoundation\ChainRequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\AttributesRequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\HostRequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\IpsRequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\PathRequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
Expand Down Expand Up @@ -893,7 +899,7 @@ private function createRequestMatcher(ContainerBuilder $container, string $path
$methods = array_map('strtoupper', $methods);
}

if (null !== $ips) {
if ($ips) {
foreach ($ips as $ip) {
$container->resolveEnvPlaceholders($ip, null, $usedEnvs);

Expand All @@ -905,22 +911,58 @@ private function createRequestMatcher(ContainerBuilder $container, string $path
}
}

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

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

// only add arguments that are necessary
$arguments = [$path, $host, $methods, $ips, $attributes, null, $port];
while (\count($arguments) > 0 && !end($arguments)) {
array_pop($arguments);
$arguments = [];
if ($methods) {
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([MethodRequestMatcher::class, $methods]))) {
$container->register($lid, MethodRequestMatcher::class)->setArguments([$methods]);
}
$arguments[] = new Reference($lid);
}

if ($path) {
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([PathRequestMatcher::class, $path]))) {
$container->register($lid, PathRequestMatcher::class)->setArguments([$path]);
}
$arguments[] = new Reference($lid);
}

if ($host) {
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([HostRequestMatcher::class, $host]))) {
$container->register($lid, HostRequestMatcher::class)->setArguments([$host]);
}
$arguments[] = new Reference($lid);
}

if ($ips) {
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([IpsRequestMatcher::class, $ips]))) {
$container->register($lid, IpsRequestMatcher::class)->setArguments([$ips]);
}
$arguments[] = new Reference($lid);
}

if ($attributes) {
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([AttributesRequestMatcher::class, $attributes]))) {
$container->register($lid, AttributesRequestMatcher::class)->setArguments([$attributes]);
}
$arguments[] = new Reference($lid);
}

if ($port) {
if (!$container->hasDefinition($lid = '.security.request_matcher.'.ContainerBuilder::hash([PortRequestMatcher::class, $port]))) {
$container->register($lid, PortRequestMatcher::class)->setArguments([$port]);
}
$arguments[] = new Reference($lid);
}

$container
->register($id, RequestMatcher::class)
->setPublic(false)
->setArguments($arguments)
->register($id, ChainRequestMatcher::class)
->setArguments([$arguments])
;

return $this->requestMatchers[$id] = new Reference($id);
Expand Down
Expand Up @@ -19,6 +19,10 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpFoundation\RequestMatcher\HostRequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\MethodRequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\PathRequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\PortRequestMatcher;
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
use Symfony\Component\PasswordHasher\Hasher\Pbkdf2PasswordHasher;
use Symfony\Component\PasswordHasher\Hasher\PlaintextPasswordHasher;
Expand Down Expand Up @@ -131,7 +135,7 @@ public function testFirewalls()
[
'simple',
'security.user_checker',
'.security.request_matcher.xmi9dcw',
'.security.request_matcher.h5ibf38',
false,
false,
'',
Expand Down Expand Up @@ -180,7 +184,7 @@ public function testFirewalls()
[
'host',
'security.user_checker',
'.security.request_matcher.iw4hyjb',
'.security.request_matcher.bcmu4fb',
true,
false,
'security.user.provider.concrete.default',
Expand Down Expand Up @@ -248,20 +252,27 @@ public function testFirewallRequestMatchers()
foreach ($arguments[1]->getValues() as $reference) {
if ($reference instanceof Reference) {
$definition = $container->getDefinition((string) $reference);
$matchers[] = $definition->getArguments();
$matchers[] = $definition->getArgument(0);
}
}

$this->assertEquals([
[
'/login',
],
[
'/test',
'foo\\.example\\.org',
['GET', 'POST'],
],
], $matchers);
$this->assertCount(2, $matchers);

$this->assertCount(1, $matchers[0]);
$def = $container->getDefinition((string) $matchers[0][0]);
$this->assertSame(PathRequestMatcher::class, $def->getClass());
$this->assertSame('/login', $def->getArgument(0));

$this->assertCount(3, $matchers[1]);
$def = $container->getDefinition((string) $matchers[1][0]);
$this->assertSame(MethodRequestMatcher::class, $def->getClass());
$this->assertSame(['GET', 'POST'], $def->getArgument(0));
$def = $container->getDefinition((string) $matchers[1][1]);
$this->assertSame(PathRequestMatcher::class, $def->getClass());
$this->assertSame('/test', $def->getArgument(0));
$def = $container->getDefinition((string) $matchers[1][2]);
$this->assertSame(HostRequestMatcher::class, $def->getClass());
$this->assertSame('foo\\.example\\.org', $def->getArgument(0));
}

public function testUserCheckerAliasIsRegistered()
Expand Down Expand Up @@ -294,17 +305,23 @@ public function testAccess()
if (1 === $i) {
$this->assertEquals(['ROLE_USER'], $attributes);
$this->assertEquals('https', $channel);
$this->assertEquals(
['/blog/524', null, ['GET', 'POST'], [], [], null, 8000],
$requestMatcher->getArguments()
);
$this->assertCount(3, $requestMatcher->getArgument(0));
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[0]);
$this->assertSame(MethodRequestMatcher::class, $def->getClass());
$this->assertSame(['GET', 'POST'], $def->getArgument(0));
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[1]);
$this->assertSame(PathRequestMatcher::class, $def->getClass());
$this->assertSame('/blog/524', $def->getArgument(0));
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[2]);
$this->assertSame(PortRequestMatcher::class, $def->getClass());
$this->assertSame(8000, $def->getArgument(0));
} elseif (2 === $i) {
$this->assertEquals(['IS_AUTHENTICATED_ANONYMOUSLY'], $attributes);
$this->assertNull($channel);
$this->assertEquals(
['/blog/.*'],
$requestMatcher->getArguments()
);
$this->assertCount(1, $requestMatcher->getArgument(0));
$def = $container->getDefinition((string) $requestMatcher->getArgument(0)[0]);
$this->assertSame(PathRequestMatcher::class, $def->getClass());
$this->assertSame('/blog/.*', $def->getArgument(0));
} elseif (3 === $i) {
$this->assertEquals('IS_AUTHENTICATED_ANONYMOUSLY', $attributes[0]);
$expression = $container->getDefinition((string) $attributes[1])->getArgument(0);
Expand Down
Expand Up @@ -26,7 +26,7 @@
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\ExpressionLanguage\Expression;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestMatcher;
use Symfony\Component\HttpFoundation\RequestMatcher\PathRequestMatcher;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
Expand Down Expand Up @@ -256,7 +256,7 @@ public function testRegisterAccessControlWithSpecifiedRequestMatcherService()
$container = $this->getRawContainer();

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

$container->loadFromExtension('security', [
Expand Down Expand Up @@ -291,7 +291,7 @@ public function testRegisterAccessControlWithRequestMatcherAndAdditionalOptionsT
$container = $this->getRawContainer();

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

$container->loadFromExtension('security', [
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/SecurityBundle/composer.json
Expand Up @@ -23,7 +23,7 @@
"symfony/dependency-injection": "^6.2",
"symfony/event-dispatcher": "^5.4|^6.0",
"symfony/http-kernel": "^6.2",
"symfony/http-foundation": "^5.4|^6.0",
"symfony/http-foundation": "^6.2",
"symfony/password-hasher": "^5.4|^6.0",
"symfony/security-core": "^6.2",
"symfony/security-csrf": "^5.4|^6.0",
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/HttpFoundation/CHANGELOG.md
Expand Up @@ -6,6 +6,9 @@ CHANGELOG

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

6.1
---
Expand Down
38 changes: 38 additions & 0 deletions src/Symfony/Component/HttpFoundation/ChainRequestMatcher.php
@@ -0,0 +1,38 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpFoundation;

/**
* ChainRequestMatcher verifies that all checks match against a Request instance.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class ChainRequestMatcher implements RequestMatcherInterface
{
/**
* @param iterable<RequestMatcherInterface> $matchers
*/
public function __construct(private iterable $matchers)
{
}

public function matches(Request $request): bool
{
foreach ($this->matchers as $matcher) {
if (!$matcher->matches($request)) {
return false;
}
}

return true;
}
}
Expand Up @@ -13,11 +13,16 @@

use Symfony\Component\ExpressionLanguage\Expression;
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
use Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher as NewExpressionRequestMatcher;

trigger_deprecation('symfony/http-foundation', '6.2', 'The "%s" class is deprecated, use "%s" instead.', ExpressionRequestMatcher::class, NewExpressionRequestMatcher::class);

/**
* ExpressionRequestMatcher uses an expression to match a Request.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @deprecated since Symfony 6.2, use "Symfony\Component\HttpFoundation\RequestMatcher\ExpressionRequestMatcher" instead
*/
class ExpressionRequestMatcher extends RequestMatcher
{
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/HttpFoundation/RequestMatcher.php
Expand Up @@ -11,10 +11,14 @@

namespace Symfony\Component\HttpFoundation;

trigger_deprecation('symfony/http-foundation', '6.2', 'The "%s" class is deprecated, use "%s" instead.', RequestMatcher::class, ChainRequestMatcher::class);

/**
* RequestMatcher compares a pre-defined set of checks against a Request instance.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @deprecated since Symfony 6.2, use ChainRequestMatcher instead
*/
class RequestMatcher implements RequestMatcherInterface
{
Expand Down
@@ -0,0 +1,45 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpFoundation\RequestMatcher;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestMatcherInterface;

/**
* Checks the Request attributes matches all regular expressions.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class AttributesRequestMatcher implements RequestMatcherInterface
{
/**
* @param array<string, string> $regexps
*/
public function __construct(private array $regexps)
{
}

public function matches(Request $request): bool
{
foreach ($this->regexps as $key => $regexp) {
$attribute = $request->attributes->get($key);
if (!\is_string($attribute)) {
return false;
}
if (!preg_match('{'.$regexp.'}', $attribute)) {
return false;
}
}

return true;
}
}
@@ -0,0 +1,43 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\HttpFoundation\RequestMatcher;

use Symfony\Component\ExpressionLanguage\Expression;
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestMatcherInterface;

/**
* ExpressionRequestMatcher uses an expression to match a Request.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class ExpressionRequestMatcher implements RequestMatcherInterface
{
public function __construct(
private ExpressionLanguage $language,
private Expression|string $expression,
) {
}

public function matches(Request $request): bool
{
return $this->language->evaluate($this->expression, [
'request' => $request,
'method' => $request->getMethod(),
'path' => rawurldecode($request->getPathInfo()),
'host' => $request->getHost(),
'ip' => $request->getClientIp(),
'attributes' => $request->attributes->all(),
]);
}
}

0 comments on commit 77538c8

Please sign in to comment.