From 8bb83fe8c293b8cda8e3362158a2fd3288fd4e59 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Fri, 4 Sep 2026 05:29:02 +0200 Subject: [PATCH] fix(dispatcher): surface preg_match() engine failures instead of treating them as no-match All dispatcher strategies checked preg_match() results with `!== 1`, which conflates a genuine no-match (0) with a PCRE engine failure (false), e.g. PREG_BACKTRACK_LIMIT_ERROR from a catastrophically backtracking route pattern. Because dynamic routes for the same method are combined into shared regex chunks, one such route could make the preg_match() call for the whole chunk fail, silently turning an unrelated, cheap route in that chunk into a spurious NOT_FOUND. Add a shared matchRoute() helper on RegexBasedAbstract that calls preg_match() and throws a RuntimeException (including preg_last_error_msg()) when it returns false, while preserving the existing 0/1 no-match/match behavior. Update GroupCountBased, GroupPosBased, CharCountBased and MarkBased to use it. Fixes #167 --- src/Dispatcher/CharCountBased.php | 3 +-- src/Dispatcher/GroupCountBased.php | 3 +-- src/Dispatcher/GroupPosBased.php | 3 +-- src/Dispatcher/MarkBased.php | 4 +--- src/Dispatcher/RegexBasedAbstract.php | 25 +++++++++++++++++++ test/Dispatcher/DispatcherTestCase.php | 33 ++++++++++++++++++++++++++ 6 files changed, 62 insertions(+), 9 deletions(-) diff --git a/src/Dispatcher/CharCountBased.php b/src/Dispatcher/CharCountBased.php index 131cca1..79a803a 100644 --- a/src/Dispatcher/CharCountBased.php +++ b/src/Dispatcher/CharCountBased.php @@ -7,7 +7,6 @@ use function assert; use function end; -use function preg_match; /** @final */ class CharCountBased extends RegexBasedAbstract @@ -18,7 +17,7 @@ protected function dispatchVariableRoute(array $routeData, string $uri): ?Matche foreach ($routeData as $data) { assert(isset($data['suffix'])); - if (preg_match($data['regex'], $uri . $data['suffix'], $matches) !== 1) { + if ($this->matchRoute($data['regex'], $uri . $data['suffix'], $matches) !== 1) { continue; } diff --git a/src/Dispatcher/GroupCountBased.php b/src/Dispatcher/GroupCountBased.php index 2448904..9dc3546 100644 --- a/src/Dispatcher/GroupCountBased.php +++ b/src/Dispatcher/GroupCountBased.php @@ -6,7 +6,6 @@ use FastRoute\Dispatcher\Result\Matched; use function count; -use function preg_match; /** @final */ class GroupCountBased extends RegexBasedAbstract @@ -15,7 +14,7 @@ class GroupCountBased extends RegexBasedAbstract protected function dispatchVariableRoute(array $routeData, string $uri): ?Matched { foreach ($routeData as $data) { - if (preg_match($data['regex'], $uri, $matches) !== 1) { + if ($this->matchRoute($data['regex'], $uri, $matches) !== 1) { continue; } diff --git a/src/Dispatcher/GroupPosBased.php b/src/Dispatcher/GroupPosBased.php index d2eebf5..b78be01 100644 --- a/src/Dispatcher/GroupPosBased.php +++ b/src/Dispatcher/GroupPosBased.php @@ -6,7 +6,6 @@ use FastRoute\Dispatcher\Result\Matched; use function assert; -use function preg_match; /** @final */ class GroupPosBased extends RegexBasedAbstract @@ -15,7 +14,7 @@ class GroupPosBased extends RegexBasedAbstract protected function dispatchVariableRoute(array $routeData, string $uri): ?Matched { foreach ($routeData as $data) { - if (preg_match($data['regex'], $uri, $matches) !== 1) { + if ($this->matchRoute($data['regex'], $uri, $matches) !== 1) { continue; } diff --git a/src/Dispatcher/MarkBased.php b/src/Dispatcher/MarkBased.php index bef8297..be30558 100644 --- a/src/Dispatcher/MarkBased.php +++ b/src/Dispatcher/MarkBased.php @@ -5,8 +5,6 @@ use FastRoute\Dispatcher\Result\Matched; -use function preg_match; - /** @final */ class MarkBased extends RegexBasedAbstract { @@ -14,7 +12,7 @@ class MarkBased extends RegexBasedAbstract protected function dispatchVariableRoute(array $routeData, string $uri): ?Matched { foreach ($routeData as $data) { - if (preg_match($data['regex'], $uri, $matches) !== 1) { + if ($this->matchRoute($data['regex'], $uri, $matches) !== 1) { continue; } diff --git a/src/Dispatcher/RegexBasedAbstract.php b/src/Dispatcher/RegexBasedAbstract.php index 9dce400..74de790 100644 --- a/src/Dispatcher/RegexBasedAbstract.php +++ b/src/Dispatcher/RegexBasedAbstract.php @@ -8,6 +8,11 @@ use FastRoute\Dispatcher\Result\Matched; use FastRoute\Dispatcher\Result\MethodNotAllowed; use FastRoute\Dispatcher\Result\NotMatched; +use RuntimeException; + +use function preg_last_error_msg; +use function preg_match; +use function sprintf; /** * @internal @@ -35,6 +40,26 @@ public function __construct(array $data) /** @param DynamicRouteChunks $routeData */ abstract protected function dispatchVariableRoute(array $routeData, string $uri): ?Matched; + /** + * Matches a route regex, distinguishing a genuine "no match" (preg_match() + * returns 0) from a PCRE engine failure such as PREG_BACKTRACK_LIMIT_ERROR + * (returns false), which would otherwise fail unrelated routes sharing the + * same combined regex chunk. + * + * @param array|null $matches + */ + protected function matchRoute(string $regex, string $subject, ?array &$matches = null): int + { + $result = preg_match($regex, $subject, $matches); + if ($result === false) { + throw new RuntimeException( + sprintf('Regex matching failed for "%s": %s', $regex, preg_last_error_msg()), + ); + } + + return $result; + } + public function dispatch(string $httpMethod, string $uri): Matched|NotMatched|MethodNotAllowed { if (isset($this->staticRouteMap[$httpMethod][$uri])) { diff --git a/test/Dispatcher/DispatcherTestCase.php b/test/Dispatcher/DispatcherTestCase.php index ba45688..2d8cfcf 100644 --- a/test/Dispatcher/DispatcherTestCase.php +++ b/test/Dispatcher/DispatcherTestCase.php @@ -13,8 +13,11 @@ use FastRoute\Dispatcher\Result\NotMatched; use PHPUnit\Framework\Attributes as PHPUnit; use PHPUnit\Framework\TestCase; +use RuntimeException; use function FastRoute\simpleDispatcher; +use function ini_set; +use function str_repeat; /** @phpstan-import-type ExtraParameters from DataGenerator */ abstract class DispatcherTestCase extends TestCase @@ -177,6 +180,36 @@ public function capturing(): void }, $this->generateDispatcherOptions()); } + /** + * A PCRE engine failure must surface instead of being reported as + * NOT_FOUND for an unrelated route in the same combined regex chunk. + * + * @see https://github.com/nikic/FastRoute/issues/167 + */ + #[PHPUnit\Test] + public function regexEngineFailureIsNotSilentlyTreatedAsNotFound(): void + { + $previousLimit = ini_set('pcre.backtrack_limit', '1000'); + self::assertNotFalse($previousLimit, 'Unable to lower pcre.backtrack_limit for this test'); + + try { + $this->expectException(RuntimeException::class); + $this->expectExceptionMessageMatches('/Backtrack limit exhausted/'); + + $dispatcher = simpleDispatcher(static function (ConfigureRoutes $r): void { + // Catastrophically backtracking pattern. + $r->addRoute('GET', '/{p:(?:a?a?)*}/complicated', 'complicated_pattern'); + // Cheap route in the same chunk: it would match on its own, but the + // chunk's preg_match() call fails. + $r->addRoute('GET', '/{p:a+}', 'cheap_route'); + }, $this->generateDispatcherOptions()); + + $dispatcher->dispatch('GET', '/' . str_repeat('a', 30)); + } finally { + ini_set('pcre.backtrack_limit', $previousLimit); + } + } + /** @return iterable}> */ public static function provideFoundDispatchCases(): iterable {