minor symfony#52070 [HttpFoundation] Cache trusted values (Toflar)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[HttpFoundation] Cache trusted values

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

Found another potential performance bottleneck. I personally detected it in the context of the `RouterListener` that sets the current request context and resets it, after the request is finished:

1. `onKernelRequest`: https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/HttpKernel/EventListener/RouterListener.php#L100 (then calls https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/Routing/RequestContext.php#L70).
2. `onKernelFinishRequest`: https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/HttpKernel/EventListener/RouterListener.php#L93 (also calls `RequestContext::fromRequest()`).

The subsequent `RequestContext::fromRequest()` always calls  `$request->getPort()` and `$request->isSecure()` where the trusted values are re-evaluated over and over again although as long as you don't change the trusted headers, they will always be the same.

I noticed it in the context of the routing process because I use quite a few subrequests but I think the optimization can affect many parts of an application. It's true for all the request methods that rely on the trusted headers like `$request->getHost()`, `$request->getClientIps()`, `$request->getBaseUrl()` etc.

So I figured, it would be best to cache the internal `getTrustedValues()` so all of those methods are optimized at once. This method is a rather heavy one because [it splits headers](https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/HttpFoundation/Request.php#L2077) and [combines parts](https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/HttpFoundation/Request.php#L2081) again etc.

This only needs to be done once per trusted header set.

Commits
-------

73abd62 [HttpFoundation] Cache trusted values

Author: fabpot

src/Symfony/Component/HttpFoundation/Request.php

@@ -212,6 +212,8 @@ class Request
     private bool $isForwardedValid = true;
     private bool $isSafeContentPreferred;
 
+    private array $trustedValuesCache = [];
+
     private static int $trustedHeaderSet = -1;
 
     private const FORWARDED_PARAMS = [
@@ -1997,8 +1999,20 @@ public function isFromTrustedProxy(): bool
         return self::$trustedProxies && IpUtils::checkIp($this->server->get('REMOTE_ADDR', ''), self::$trustedProxies);
     }
 
+    /**
+     * This method is rather heavy because it splits and merges headers, and it's called by many other methods such as
+     * getPort(), isSecure(), getHost(), getClientIps(), getBaseUrl() etc. Thus, we try to cache the results for
+     * best performance.
+     */
     private function getTrustedValues(int $type, string $ip = null): array
     {
+        $cacheKey = $type."\0".((self::$trustedHeaderSet & $type) ? $this->headers->get(self::TRUSTED_HEADERS[$type]) : '');
+        $cacheKey .= "\0".$ip."\0".$this->headers->get(self::TRUSTED_HEADERS[self::HEADER_FORWARDED]);
+
+        if (isset($this->trustedValuesCache[$cacheKey])) {
+            return $this->trustedValuesCache[$cacheKey];
+        }
+
         $clientValues = [];
         $forwardedValues = [];
 
@@ -2011,7 +2025,6 @@ private function getTrustedValues(int $type, string $ip = null): array
         if ((self::$trustedHeaderSet & self::HEADER_FORWARDED) && (isset(self::FORWARDED_PARAMS[$type])) && $this->headers->has(self::TRUSTED_HEADERS[self::HEADER_FORWARDED])) {
             $forwarded = $this->headers->get(self::TRUSTED_HEADERS[self::HEADER_FORWARDED]);
             $parts = HeaderUtils::split($forwarded, ',;=');
-            $forwardedValues = [];
             $param = self::FORWARDED_PARAMS[$type];
             foreach ($parts as $subParts) {
                 if (null === $v = HeaderUtils::combine($subParts)[$param] ?? null) {
@@ -2033,15 +2046,15 @@ private function getTrustedValues(int $type, string $ip = null): array
         }
 
         if ($forwardedValues === $clientValues || !$clientValues) {
-            return $forwardedValues;
+            return $this->trustedValuesCache[$cacheKey] = $forwardedValues;
         }
 
         if (!$forwardedValues) {
-            return $clientValues;
+            return $this->trustedValuesCache[$cacheKey] = $clientValues;
         }
 
         if (!$this->isForwardedValid) {
-            return null !== $ip ? ['0.0.0.0', $ip] : [];
+            return $this->trustedValuesCache[$cacheKey] = null !== $ip ? ['0.0.0.0', $ip] : [];
         }
         $this->isForwardedValid = false;
 

src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

@@ -2550,6 +2550,23 @@ public function testTrustedProxiesRemoteAddr($serverRemoteAddr, $trustedProxies,
         $this->assertSame($result, Request::getTrustedProxies());
     }
 
+    public function testTrustedValuesCache()
+    {
+        $request = Request::create('http://example.com/');
+        $request->server->set('REMOTE_ADDR', '3.3.3.3');
+        $request->headers->set('X_FORWARDED_FOR', '1.1.1.1, 2.2.2.2');
+        $request->headers->set('X_FORWARDED_PROTO', 'https');
+
+        $this->assertFalse($request->isSecure());
+
+        Request::setTrustedProxies(['3.3.3.3', '2.2.2.2'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO);
+        $this->assertTrue($request->isSecure());
+
+        // Header is changed, cache must not be hit now
+        $request->headers->set('X_FORWARDED_PROTO', 'http');
+        $this->assertFalse($request->isSecure());
+    }
+
     public static function trustedProxiesRemoteAddr()
     {
         return [