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 FastRoute\Dispatcher\Result\Matched; use function assert; use function end; -use function preg_match; /** @final */ class CharCountBased extends RegexBasedAbstract @@ -18,7 +17,7 @@ class CharCountBased extends RegexBasedAbstract 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 @@ namespace FastRoute\Dispatcher; 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 @@ namespace FastRoute\Dispatcher; 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 @@ namespace FastRoute\Dispatcher; 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..b808c71 100644 --- a/src/Dispatcher/RegexBasedAbstract.php +++ b/src/Dispatcher/RegexBasedAbstract.php @@ -8,6 +8,11 @@ use FastRoute\Dispatcher; 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,33 @@ abstract class RegexBasedAbstract implements Dispatcher /** @param DynamicRouteChunks $routeData */ abstract protected function dispatchVariableRoute(array $routeData, string $uri): ?Matched; + /** + * Matches a compiled route regex against the subject, distinguishing a + * genuine "no match" (preg_match() returning 0) from a PCRE engine + * failure (preg_match() returning false). + * + * A regex engine failure - for example PREG_BACKTRACK_LIMIT_ERROR caused + * by a catastrophically backtracking route pattern - must not be treated + * as "no match". Silently swallowing it can make an unrelated, cheap + * route that happens to share the same combined regex chunk spuriously + * fail to match. + * + * @see https://github.com/nikic/FastRoute/issues/167 + * + * @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..d83d94a 100644 --- a/test/Dispatcher/DispatcherTestCase.php +++ b/test/Dispatcher/DispatcherTestCase.php @@ -13,8 +13,11 @@ use FastRoute\Dispatcher\Result\MethodNotAllowed; 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,40 @@ abstract class DispatcherTestCase extends TestCase }, $this->generateDispatcherOptions()); } + /** + * A PCRE engine failure (e.g. PREG_BACKTRACK_LIMIT_ERROR triggered by a + * catastrophically backtracking route pattern) must not be silently + * treated as "no match". Otherwise, an unrelated, cheap route that + * happens to share the same combined regex chunk would spuriously + * receive a NOT_FOUND result instead of surfacing the engine error. + * + * @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, unrelated route combined into the same regex chunk + // as the pattern above. It would match the request URI on + // its own, but the shared 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 {