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
This commit is contained in:
dualfroz committed 2026-09-06 01:25:06 +02:00
1 parent c0905f9089
commit 7ac8857878
6 files changed
+62 -9

No files matched your search

+1 -2
View File
@@ -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;
}
+1 -2
View File
@@ -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;
}
+1 -2
View File
@@ -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;
}
+1 -3
View File
@@ -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;
}
+25
View File
@@ -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,26 @@ abstract class RegexBasedAbstract implements Dispatcher
/** @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<int|string, mixed>|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])) {
+33
View File
@@ -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,36 @@ abstract class DispatcherTestCase extends TestCase
}, $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<string, array{0: string, 1: string, 2: Closure(ConfigureRoutes):void, 3: string, 4?: array<string, string>}> */
public static function provideFoundDispatchCases(): iterable
{