From 3f42b153175380a35fd6a775a8f04a1afd26eb7a Mon Sep 17 00:00:00 2001 From: kaii-k Date: Wed, 26 Aug 2026 14:53:29 +0530 Subject: [PATCH 1/3] Bound FastRouteDispatcher::getAllowedMethods() cache size FastRouteDispatcher::getAllowedMethods() memoizes its result per requested URI in an instance property with no cap, TTL, or eviction. On dispatchers that outlive a single request (persistent-worker runtimes such as FrankenPHP worker mode, RoadRunner, Swoole, or any long-lived process reusing one App/Dispatcher instance, e.g. a queue worker or CLI daemon), an attacker can grow this cache without bound by requesting distinct, unmatched URIs, exhausting process memory. Cap the cache at 1000 entries with simple oldest-first eviction. This is a no-op for the traditional php-fpm-per-request model, where the dispatcher never survives past one request. --- Slim/Routing/FastRouteDispatcher.php | 16 ++++++++++++++++ tests/Routing/FastRouteDispatcherTest.php | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/Slim/Routing/FastRouteDispatcher.php b/Slim/Routing/FastRouteDispatcher.php index 797746bbb..f0b9c94b1 100644 --- a/Slim/Routing/FastRouteDispatcher.php +++ b/Slim/Routing/FastRouteDispatcher.php @@ -14,6 +14,17 @@ class FastRouteDispatcher extends GroupCountBased { + /** + * Maximum number of URIs to memoize in $allowedMethods. + * + * Dispatcher instances can outlive a single request (e.g. persistent-worker + * runtimes such as FrankenPHP worker mode, RoadRunner, Swoole, or long-running + * CLI/queue processes that reuse one App/Dispatcher). Without a cap, requesting + * distinct URIs indefinitely grows this cache with no eviction, allowing + * unbounded memory consumption from client-controlled input. + */ + private const MAX_ALLOWED_METHODS_CACHE_SIZE = 1000; + /** * @var string[][] */ @@ -104,6 +115,11 @@ public function getAllowedMethods(string $uri): array } } + if (count($this->allowedMethods) >= self::MAX_ALLOWED_METHODS_CACHE_SIZE) { + // Evict the oldest entry to keep the cache bounded. + unset($this->allowedMethods[array_key_first($this->allowedMethods)]); + } + return $this->allowedMethods[$uri] = array_keys($allowedMethods); } } diff --git a/tests/Routing/FastRouteDispatcherTest.php b/tests/Routing/FastRouteDispatcherTest.php index 48e575ac4..8068f79a4 100644 --- a/tests/Routing/FastRouteDispatcherTest.php +++ b/tests/Routing/FastRouteDispatcherTest.php @@ -107,6 +107,28 @@ public function testGetAllowedMethods($method, $uri, $callback, $allowedMethods) $this->assertSame($results, $allowedMethods); } + public function testGetAllowedMethodsCacheIsBounded() + { + /** @var FastRouteDispatcher $dispatcher */ + $dispatcher = simpleDispatcher(function (RouteCollector $r) { + $r->addRoute('GET', '/user', 'handler0'); + }, $this->generateDispatcherOptions()); + + $reflectionClass = new \ReflectionClass($dispatcher); + $maxCacheSize = $reflectionClass->getConstant('MAX_ALLOWED_METHODS_CACHE_SIZE'); + $cacheProperty = $reflectionClass->getProperty('allowedMethods'); + $cacheProperty->setAccessible(true); + + // Simulate a long-lived dispatcher instance (e.g. a persistent-worker + // runtime) being hit with far more distinct, attacker-controlled URIs + // than the cache is allowed to hold. + for ($i = 0; $i < $maxCacheSize + 500; $i++) { + $dispatcher->getAllowedMethods('/not-found-' . $i); + } + + $this->assertLessThanOrEqual($maxCacheSize, count($cacheProperty->getValue($dispatcher))); + } + public function testDuplicateVariableNameError() { $this->expectException(BadRouteException::class); From b5a38742ce490a268fe103bf5f132f66fe0aad24 Mon Sep 17 00:00:00 2001 From: kaii-k Date: Thu, 27 Aug 2026 01:48:48 +0530 Subject: [PATCH 2/3] Simplify to single-entry memo per akrabat review feedback getAllowedMethods() only ever needs its most recent result: dispatch() calls it once per invocation for the current URI, and across calls the cache only pays off when the same URI repeats back-to-back. Replace the capped/evicting map with a single (uri, methods) pair, which is bounded by construction and removes the cap/eviction logic entirely. --- Slim/Routing/FastRouteDispatcher.php | 32 ++++++++++++----------- tests/Routing/FastRouteDispatcherTest.php | 28 ++++++++++++++++---- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/Slim/Routing/FastRouteDispatcher.php b/Slim/Routing/FastRouteDispatcher.php index f0b9c94b1..fe3b72dfc 100644 --- a/Slim/Routing/FastRouteDispatcher.php +++ b/Slim/Routing/FastRouteDispatcher.php @@ -15,18 +15,23 @@ class FastRouteDispatcher extends GroupCountBased { /** - * Maximum number of URIs to memoize in $allowedMethods. + * The URI that $allowedMethods was computed for. * - * Dispatcher instances can outlive a single request (e.g. persistent-worker - * runtimes such as FrankenPHP worker mode, RoadRunner, Swoole, or long-running - * CLI/queue processes that reuse one App/Dispatcher). Without a cap, requesting - * distinct URIs indefinitely grows this cache with no eviction, allowing - * unbounded memory consumption from client-controlled input. + * getAllowedMethods() only ever needs to remember its most recent result: + * within a single dispatch() call it's invoked once for the current URI, + * and across calls it only saves recomputation when the same URI repeats + * back-to-back. Memoizing every distinct URI ever seen (unbounded) let a + * long-lived Dispatcher instance (e.g. a persistent-worker runtime, or any + * process that reuses one App/Dispatcher across many requests) be driven + * to unbounded memory growth by client-controlled URIs. A single-entry + * memo is bounded by construction. + * + * @var string|null */ - private const MAX_ALLOWED_METHODS_CACHE_SIZE = 1000; + private ?string $allowedMethodsUri = null; /** - * @var string[][] + * @var string[] */ private array $allowedMethods = []; @@ -97,8 +102,8 @@ private function routingResults(string $httpMethod, string $uri): array */ public function getAllowedMethods(string $uri): array { - if (isset($this->allowedMethods[$uri])) { - return $this->allowedMethods[$uri]; + if ($this->allowedMethodsUri === $uri) { + return $this->allowedMethods; } $allowedMethods = []; @@ -115,11 +120,8 @@ public function getAllowedMethods(string $uri): array } } - if (count($this->allowedMethods) >= self::MAX_ALLOWED_METHODS_CACHE_SIZE) { - // Evict the oldest entry to keep the cache bounded. - unset($this->allowedMethods[array_key_first($this->allowedMethods)]); - } + $this->allowedMethodsUri = $uri; - return $this->allowedMethods[$uri] = array_keys($allowedMethods); + return $this->allowedMethods = array_keys($allowedMethods); } } diff --git a/tests/Routing/FastRouteDispatcherTest.php b/tests/Routing/FastRouteDispatcherTest.php index 8068f79a4..1a1b305eb 100644 --- a/tests/Routing/FastRouteDispatcherTest.php +++ b/tests/Routing/FastRouteDispatcherTest.php @@ -115,18 +115,36 @@ public function testGetAllowedMethodsCacheIsBounded() }, $this->generateDispatcherOptions()); $reflectionClass = new \ReflectionClass($dispatcher); - $maxCacheSize = $reflectionClass->getConstant('MAX_ALLOWED_METHODS_CACHE_SIZE'); + $uriProperty = $reflectionClass->getProperty('allowedMethodsUri'); + $uriProperty->setAccessible(true); $cacheProperty = $reflectionClass->getProperty('allowedMethods'); $cacheProperty->setAccessible(true); // Simulate a long-lived dispatcher instance (e.g. a persistent-worker - // runtime) being hit with far more distinct, attacker-controlled URIs - // than the cache is allowed to hold. - for ($i = 0; $i < $maxCacheSize + 500; $i++) { + // runtime) being hit with many distinct, attacker-controlled URIs. + // The memo must stay a single entry regardless of how many distinct + // URIs are requested. + for ($i = 0; $i < 1500; $i++) { $dispatcher->getAllowedMethods('/not-found-' . $i); } - $this->assertLessThanOrEqual($maxCacheSize, count($cacheProperty->getValue($dispatcher))); + $this->assertSame('/not-found-1499', $uriProperty->getValue($dispatcher)); + $this->assertIsArray($cacheProperty->getValue($dispatcher)); + } + + public function testGetAllowedMethodsMemoizesLastUriOnly() + { + /** @var FastRouteDispatcher $dispatcher */ + $dispatcher = simpleDispatcher(function (RouteCollector $r) { + $r->addRoute('GET', '/user', 'handler0'); + $r->addRoute('POST', '/post', 'handler1'); + }, $this->generateDispatcherOptions()); + + $this->assertSame(['GET'], $dispatcher->getAllowedMethods('/user')); + $this->assertSame(['POST'], $dispatcher->getAllowedMethods('/post')); + // Re-requesting the first URI recomputes rather than returning a + // stale hit from the second lookup. + $this->assertSame(['GET'], $dispatcher->getAllowedMethods('/user')); } public function testDuplicateVariableNameError() From 7f4931f90e525638800358baf399ad85623692c0 Mon Sep 17 00:00:00 2001 From: Rob Allen Date: Sat, 29 Aug 2026 19:24:50 +0100 Subject: [PATCH 3/3] Remove unnecessary comments and test --- Slim/Routing/FastRouteDispatcher.php | 11 -------- tests/Routing/FastRouteDispatcherTest.php | 31 +++-------------------- 2 files changed, 3 insertions(+), 39 deletions(-) diff --git a/Slim/Routing/FastRouteDispatcher.php b/Slim/Routing/FastRouteDispatcher.php index fe3b72dfc..61abaeb2b 100644 --- a/Slim/Routing/FastRouteDispatcher.php +++ b/Slim/Routing/FastRouteDispatcher.php @@ -15,17 +15,6 @@ class FastRouteDispatcher extends GroupCountBased { /** - * The URI that $allowedMethods was computed for. - * - * getAllowedMethods() only ever needs to remember its most recent result: - * within a single dispatch() call it's invoked once for the current URI, - * and across calls it only saves recomputation when the same URI repeats - * back-to-back. Memoizing every distinct URI ever seen (unbounded) let a - * long-lived Dispatcher instance (e.g. a persistent-worker runtime, or any - * process that reuses one App/Dispatcher across many requests) be driven - * to unbounded memory growth by client-controlled URIs. A single-entry - * memo is bounded by construction. - * * @var string|null */ private ?string $allowedMethodsUri = null; diff --git a/tests/Routing/FastRouteDispatcherTest.php b/tests/Routing/FastRouteDispatcherTest.php index 1a1b305eb..11823aad0 100644 --- a/tests/Routing/FastRouteDispatcherTest.php +++ b/tests/Routing/FastRouteDispatcherTest.php @@ -107,32 +107,7 @@ public function testGetAllowedMethods($method, $uri, $callback, $allowedMethods) $this->assertSame($results, $allowedMethods); } - public function testGetAllowedMethodsCacheIsBounded() - { - /** @var FastRouteDispatcher $dispatcher */ - $dispatcher = simpleDispatcher(function (RouteCollector $r) { - $r->addRoute('GET', '/user', 'handler0'); - }, $this->generateDispatcherOptions()); - - $reflectionClass = new \ReflectionClass($dispatcher); - $uriProperty = $reflectionClass->getProperty('allowedMethodsUri'); - $uriProperty->setAccessible(true); - $cacheProperty = $reflectionClass->getProperty('allowedMethods'); - $cacheProperty->setAccessible(true); - - // Simulate a long-lived dispatcher instance (e.g. a persistent-worker - // runtime) being hit with many distinct, attacker-controlled URIs. - // The memo must stay a single entry regardless of how many distinct - // URIs are requested. - for ($i = 0; $i < 1500; $i++) { - $dispatcher->getAllowedMethods('/not-found-' . $i); - } - - $this->assertSame('/not-found-1499', $uriProperty->getValue($dispatcher)); - $this->assertIsArray($cacheProperty->getValue($dispatcher)); - } - - public function testGetAllowedMethodsMemoizesLastUriOnly() + public function testGetAllowedMethodsReusesLastUriOnly() { /** @var FastRouteDispatcher $dispatcher */ $dispatcher = simpleDispatcher(function (RouteCollector $r) { @@ -142,8 +117,8 @@ public function testGetAllowedMethodsMemoizesLastUriOnly() $this->assertSame(['GET'], $dispatcher->getAllowedMethods('/user')); $this->assertSame(['POST'], $dispatcher->getAllowedMethods('/post')); - // Re-requesting the first URI recomputes rather than returning a - // stale hit from the second lookup. + + // Repeating the first URI recomputes, as only the last result is kept. $this->assertSame(['GET'], $dispatcher->getAllowedMethods('/user')); }