Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/Dispatcher/CharCountBased.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

use function assert;
use function end;
use function preg_match;

/** @final */
class CharCountBased extends RegexBasedAbstract
Expand All @@ -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;
}

Expand Down
3 changes: 1 addition & 2 deletions src/Dispatcher/GroupCountBased.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use FastRoute\Dispatcher\Result\Matched;

use function count;
use function preg_match;

/** @final */
class GroupCountBased extends RegexBasedAbstract
Expand All @@ -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;
}

Expand Down
3 changes: 1 addition & 2 deletions src/Dispatcher/GroupPosBased.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use FastRoute\Dispatcher\Result\Matched;

use function assert;
use function preg_match;

/** @final */
class GroupPosBased extends RegexBasedAbstract
Expand All @@ -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;
}

Expand Down
4 changes: 1 addition & 3 deletions src/Dispatcher/MarkBased.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@

use FastRoute\Dispatcher\Result\Matched;

use function preg_match;

/** @final */
class MarkBased extends RegexBasedAbstract
{
/** @inheritDoc */
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;
}

Expand Down
25 changes: 25 additions & 0 deletions src/Dispatcher/RegexBasedAbstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<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])) {
Expand Down
33 changes: 33 additions & 0 deletions test/Dispatcher/DispatcherTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<string, array{0: string, 1: string, 2: Closure(ConfigureRoutes):void, 3: string, 4?: array<string, string>}> */
public static function provideFoundDispatchCases(): iterable
{
Expand Down