From 831c3df79ce8215765dd34fd771d06f083ccaa6f Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Tue, 27 Jun 2023 16:18:14 +0200
Subject: [PATCH] [TASK] Add shared cache for VariableProcessor
VariableProcessor::addHash() is called to convert URI path
variables that don't comply with certain rules to a hash,
which does.
First of all, variables must not exceed a certain length
of 32, second of all, variables must only contain word
characters aka "\w" or "[A-Za-z0-9_]".
So whenever a variable is too long or contains invalid
characters, it is converted to an md5 hash which complies
with both rules.
It has been reported that the checks, whether to generate a
hash and the hashing would have a very noticable performance
impact.
This change introduces as specific runtime cache for
VariableProcessor that is shared with any other new instance.
This way, the necessity whether a hash is required and the hash
representation of a value can be resolved from this cache.
For instance, this would speed up scenarios that are enerating
large product lists and generate, when every invocation with a
new UID would have triggered a new hash check and assignment.
Resolves: #100974
Releases: main, 12.4
Change-Id: I19e1d138a79792cc05a6a908f1ee5f87236a65bd
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/79710
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: core-ci <typo3@b13.com>
---
.../Routing/Enhancer/AbstractEnhancer.php | 3 +-
.../Routing/Enhancer/VariableProcessor.php | 50 +++++++++++++++----
.../Enhancer/VariableProcessorCache.php | 36 +++++++++++++
typo3/sysext/core/Configuration/Services.yaml | 7 +++
.../Enhancer/VariableProcessorTest.php | 11 ++--
5 files changed, 90 insertions(+), 17 deletions(-)
create mode 100644 typo3/sysext/core/Classes/Routing/Enhancer/VariableProcessorCache.php
diff --git a/typo3/sysext/core/Classes/Routing/Enhancer/AbstractEnhancer.php b/typo3/sysext/core/Classes/Routing/Enhancer/AbstractEnhancer.php
index e314a1ad1d81..a3c5e649fe2f 100644
--- a/typo3/sysext/core/Classes/Routing/Enhancer/AbstractEnhancer.php
+++ b/typo3/sysext/core/Classes/Routing/Enhancer/AbstractEnhancer.php
@@ -20,6 +20,7 @@ namespace TYPO3\CMS\Core\Routing\Enhancer;
use TYPO3\CMS\Core\Routing\Aspect\AspectInterface;
use TYPO3\CMS\Core\Routing\Aspect\ModifiableAspectInterface;
use TYPO3\CMS\Core\Routing\Route;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
/**
* Abstract Enhancer, useful for custom enhancers
@@ -181,7 +182,7 @@ abstract class AbstractEnhancer implements EnhancerInterface
if (isset($this->variableProcessor)) {
return $this->variableProcessor;
}
- return $this->variableProcessor = new VariableProcessor();
+ return $this->variableProcessor = GeneralUtility::makeInstance(VariableProcessor::class);
}
/**
diff --git a/typo3/sysext/core/Classes/Routing/Enhancer/VariableProcessor.php b/typo3/sysext/core/Classes/Routing/Enhancer/VariableProcessor.php
index ff59e9b0b921..3b5330e3fafb 100644
--- a/typo3/sysext/core/Classes/Routing/Enhancer/VariableProcessor.php
+++ b/typo3/sysext/core/Classes/Routing/Enhancer/VariableProcessor.php
@@ -36,25 +36,53 @@ class VariableProcessor
*/
protected $nestedValues = [];
+ public function __construct(private readonly VariableProcessorCache $cache)
+ {
+ }
+
protected function addHash(string $value): string
{
- if (strlen($value) < 31 && !preg_match('#[^\w]#', $value)) {
+ if (!$this->requiresHashing($value)) {
return $value;
}
- // removing one bit, e.g. for enforced route prefix `{!value}`
- $hash = substr(md5($value), 0, -1);
- // Symfony Route Compiler requires first literal to be non-integer
- if ($hash[0] === (string)(int)$hash[0]) {
- $hash[0] = str_replace(
- range('0', '9'),
- range('o', 'x'),
- $hash[0]
- );
- }
+ // generate hash (fetch from cache, if available)
+ $hash = $this->generateHash($value);
+ // store hash locally (indicator, that this value was processed)
$this->hashes[$hash] = $value;
return $hash;
}
+ /**
+ * Determines whether a parameter value requires hashing.
+ * This is the case if the value has 31+ chars (Symfony has a limitation of 32 chars),
+ * or if the value contains any non-word characters besides `[A-Za-z0-9_]`, such as `@`.
+ */
+ protected function requiresHashing(string $value): bool
+ {
+ if (!isset($this->cache->requiresHashing[$value])) {
+ $this->cache->requiresHashing[$value] = strlen($value) >= 31 || preg_match('#[^\w]#', $value) > 0;
+ }
+ return $this->cache->requiresHashing[$value];
+ }
+
+ protected function generateHash(string $value): string
+ {
+ if (!isset($this->cache->hashes[$value])) {
+ // remove one char, which might be used as enforced route prefix `{!value}`
+ $hash = substr(md5($value), 0, -1);
+ // Symfony Route Compiler requires the first literal to be non-integer
+ if ($hash[0] === (string)(int)$hash[0]) {
+ $hash[0] = str_replace(
+ range('0', '9'),
+ range('o', 'x'),
+ $hash[0]
+ );
+ }
+ $this->cache->hashes[$value] = $hash;
+ }
+ return $this->cache->hashes[$value];
+ }
+
/**
* @throws \OutOfRangeException
*/
diff --git a/typo3/sysext/core/Classes/Routing/Enhancer/VariableProcessorCache.php b/typo3/sysext/core/Classes/Routing/Enhancer/VariableProcessorCache.php
new file mode 100644
index 000000000000..883cde58207d
--- /dev/null
+++ b/typo3/sysext/core/Classes/Routing/Enhancer/VariableProcessorCache.php
@@ -0,0 +1,36 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Routing\Enhancer;
+
+/**
+ * Shared cache among multiple `VariableProcessor` instances
+ *
+ * @internal
+ */
+class VariableProcessorCache
+{
+ /**
+ * @var array<string, bool>
+ */
+ public array $requiresHashing = [];
+
+ /**
+ * @var array<string, string>
+ */
+ public array $hashes = [];
+}
diff --git a/typo3/sysext/core/Configuration/Services.yaml b/typo3/sysext/core/Configuration/Services.yaml
index 6399f0ad83a3..3d0ea395b6bf 100644
--- a/typo3/sysext/core/Configuration/Services.yaml
+++ b/typo3/sysext/core/Configuration/Services.yaml
@@ -147,6 +147,13 @@ services:
TYPO3\CMS\Core\Routing\RequestContextFactory:
public: true
+ TYPO3\CMS\Core\Routing\Enhancer\VariableProcessor:
+ public: true
+ shared: false
+
+ TYPO3\CMS\Core\Routing\Enhancer\VariableProcessorCache:
+ shared: true
+
# FAL security checks for backend users
TYPO3\CMS\Core\Resource\Security\StoragePermissionsAspect:
tags:
diff --git a/typo3/sysext/core/Tests/Unit/Routing/Enhancer/VariableProcessorTest.php b/typo3/sysext/core/Tests/Unit/Routing/Enhancer/VariableProcessorTest.php
index e731e16489cc..3c17b33eab85 100644
--- a/typo3/sysext/core/Tests/Unit/Routing/Enhancer/VariableProcessorTest.php
+++ b/typo3/sysext/core/Tests/Unit/Routing/Enhancer/VariableProcessorTest.php
@@ -18,6 +18,7 @@ declare(strict_types=1);
namespace TYPO3\CMS\Core\Tests\Unit\Routing\Enhancer;
use TYPO3\CMS\Core\Routing\Enhancer\VariableProcessor;
+use TYPO3\CMS\Core\Routing\Enhancer\VariableProcessorCache;
use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
final class VariableProcessorTest extends UnitTestCase
@@ -127,7 +128,7 @@ final class VariableProcessorTest extends UnitTestCase
*/
public function isRoutePathProcessed(?string $namespace, array $arguments, string $inflatedRoutePath, string $deflatedRoutePath): void
{
- $subject = new VariableProcessor();
+ $subject = new VariableProcessor(new VariableProcessorCache());
self::assertSame(
$deflatedRoutePath,
$subject->deflateRoutePath($inflatedRoutePath, $namespace, $arguments)
@@ -166,7 +167,7 @@ final class VariableProcessorTest extends UnitTestCase
*/
public function parametersAreProcessed(array $arguments, array $deflatedParameters): void
{
- $subject = new VariableProcessor();
+ $subject = new VariableProcessor(new VariableProcessorCache());
$inflatedParameters = ['a' => 'a', 'first' => ['aa' => 'aa', 'second' => ['aaa' => 'aaa', '@any' => '@any']]];
self::assertEquals(
$deflatedParameters,
@@ -242,7 +243,7 @@ final class VariableProcessorTest extends UnitTestCase
*/
public function namespaceParametersAreProcessed(string $namespace, array $arguments, array $deflatedParameters): void
{
- $subject = new VariableProcessor();
+ $subject = new VariableProcessor(new VariableProcessorCache());
$inflatedParameters = ['a' => 'a', 'first' => ['aa' => 'aa', 'second' => ['aaa' => 'aaa', '@any' => '@any']]];
self::assertEquals(
$deflatedParameters,
@@ -330,7 +331,7 @@ final class VariableProcessorTest extends UnitTestCase
*/
public function keysAreDeflated(?string $namespace, array $arguments, array $deflatedKeys): void
{
- $subject = new VariableProcessor();
+ $subject = new VariableProcessor(new VariableProcessorCache());
$inflatedKeys = ['a' => 'a', 'b' => 'b', 'c' => ['d' => 'd', 'e' => 'e']];
self::assertEquals(
$deflatedKeys,
@@ -350,6 +351,6 @@ final class VariableProcessorTest extends UnitTestCase
{
$this->expectException(\OutOfRangeException::class);
$this->expectExceptionCode(1537633463);
- (new VariableProcessor())->inflateKeys($deflatedKeys, $namespace, $arguments);
+ (new VariableProcessor(new VariableProcessorCache()))->inflateKeys($deflatedKeys, $namespace, $arguments);
}
}
--
GitLab