minor #51986 [Security] Do not match request twice in HttpUtils (To…

…flar)

This PR was merged into the 5.4 branch.

Discussion
----------

[Security] Do not match request twice in `HttpUtils`

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT

Found a pretty heavy performance issue in [Contao](https://contao.org) (thank you, Blackfire for sponsoring my Open Source license ❤️). It's probably not very apparent if you only use Symfony core components (due to fast regex matching) but as soon as you use dynamic router matching using e.g. [Symfony CMF Routing](https://github.com/symfony-cmf/Routing) with database hits like we do, you may encounter this issue.

The problem is that currently, `HttpUtils::checkRequestPath()` always matches a given `Request`, even if that has already been matched before. This is the **default** case in any Symfony application because

* In the `kernel.request` event stack, the `RouterListener` is called first. Route matching happens here.
* Later, the `FirewallListener` is called. In its stack, it calls `LogoutListener::supports()` which **always** calls `requiresLogout()` and thus triggers an additional match via `HttpUtils::checkRequestPath()` for every single request (https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/Security/Http/Firewall/LogoutListener.php#L142).

So if route matching has already happened before, we should not match again.

Commits
-------

ccacdf8 Do not match request twice in HttpUtils

src/Symfony/Component/Security/Http/HttpUtils.php

@@ -118,6 +118,11 @@ public function createRequest(Request $request, string $path)
     public function checkRequestPath(Request $request, string $path)
     {
         if ('/' !== $path[0]) {
+            // Shortcut if request has already been matched before
+            if ($request->attributes->has('_route')) {
+                return $path === $request->attributes->get('_route');
+            }
+
             try {
                 // matching a request is more powerful than matching a URL path + context, so try that first
                 if ($this->urlMatcher instanceof RequestMatcherInterface) {

src/Symfony/Component/Security/Http/Tests/HttpUtilsTest.php

@@ -318,6 +318,22 @@ public function testCheckRequestPathWithUrlMatcherLoadingException()
         $utils->checkRequestPath($this->getRequest(), 'foobar');
     }
 
+    public function testCheckRequestPathWithRequestAlreadyMatchedBefore()
+    {
+        $urlMatcher = $this->createMock(RequestMatcherInterface::class);
+        $urlMatcher
+            ->expects($this->never())
+            ->method('matchRequest')
+        ;
+
+        $request = $this->getRequest();
+        $request->attributes->set('_route', 'route_name');
+
+        $utils = new HttpUtils(null, $urlMatcher);
+        $this->assertTrue($utils->checkRequestPath($request, 'route_name'));
+        $this->assertFalse($utils->checkRequestPath($request, 'foobar'));
+    }
+
     public function testCheckPathWithoutRouteParam()
     {
         $urlMatcher = $this->createMock(UrlMatcherInterface::class);