Skip to content

Commit 339f1ce

Browse files
loks0nclaude
andcommitted
Make routing coroutine-safe by removing Route mutations
Router::match and the wildcard branch in Http::runInternal both wrote to the shared Route singleton (setMatchedPath, path) on every request. Under Swoole coroutines the Route is shared across in-flight requests, so concurrent requests could observe each other's matched path. - Router::match now returns [Route, matchedPath] instead of mutating the Route. A new Router::setFallback slot replaces Http::$wildcardRoute, so the method-agnostic catch-all flows through the same matching path as any other route. - Route::matchedPath / setMatchedPath / getMatchedPath are removed. - Http::execute takes the matched path as a parameter; runInternal threads it through. Public Http::match keeps its ?Route shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3e3b431 commit 339f1ce

3 files changed

Lines changed: 71 additions & 47 deletions

File tree

src/Http/Http.php

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,11 @@ class Http
106106
protected ?Route $route = null;
107107

108108
/**
109-
* Wildcard route
110-
* If set, this get's executed if no other route is matched
109+
* Matched route key (template after placeholder substitution).
110+
*
111+
* Cached alongside $route for $fresh=false re-matches.
111112
*/
112-
protected static ?Route $wildcardRoute = null;
113+
protected string $matchedPath = '';
113114

114115
/**
115116
* Compression
@@ -242,9 +243,10 @@ public static function delete(string $url): Route
242243
*/
243244
public static function wildcard(): Route
244245
{
245-
self::$wildcardRoute = new Route('', '');
246+
$route = new Route('', '');
247+
Router::setFallback($route);
246248

247-
return self::$wildcardRoute;
249+
return $route;
248250
}
249251

250252
/**
@@ -545,30 +547,54 @@ public function start(): void
545547
* @param bool $fresh If true, will not match any cached route
546548
*/
547549
public function match(Request $request, bool $fresh = true): ?Route
550+
{
551+
return $this->matchInternal($request, $fresh)[0] ?? null;
552+
}
553+
554+
/**
555+
* Match a request and return both the matched Route and the route key it
556+
* matched against. Returning the matched key separately avoids mutating
557+
* the shared Route instance, which would race under coroutines.
558+
*
559+
* @return array{0: Route, 1: string}|null
560+
*/
561+
private function matchInternal(Request $request, bool $fresh = true): ?array
548562
{
549563
if (null !== $this->route && !$fresh) {
550-
return $this->route;
564+
return [$this->route, $this->matchedPath];
551565
}
552566

553567
$url = parse_url($request->getURI(), PHP_URL_PATH);
554568
$url = \is_string($url) ? ($url === '' ? '/' : $url) : '/';
555569
$method = $request->getMethod();
556570
$method = (self::REQUEST_METHOD_HEAD === $method) ? self::REQUEST_METHOD_GET : $method;
557571

558-
$this->route = Router::match($method, $url);
572+
$match = Router::match($method, $url);
573+
574+
if ($match === null) {
575+
$this->route = null;
576+
$this->matchedPath = '';
577+
return null;
578+
}
579+
580+
[$this->route, $this->matchedPath] = $match;
559581

560-
return $this->route;
582+
return $match;
561583
}
562584

563585
/**
564-
* Execute a given route with middlewares and error handling
586+
* Execute a given route with middlewares and error handling.
587+
*
588+
* $matchedPath is the route key this request matched against (the
589+
* registered template after placeholder substitution). Pass '' for the
590+
* fallback route or when path params aren't relevant.
565591
*/
566-
public function execute(Route $route, Request $request, Response $response): static
592+
public function execute(Route $route, Request $request, Response $response, string $matchedPath = ''): static
567593
{
568594
$arguments = [];
569595
$groups = $route->getGroups();
570596

571-
$preparedPath = Router::preparePath($route->getMatchedPath());
597+
$preparedPath = Router::preparePath($matchedPath);
572598
$pathValues = $route->getPathValues($request, $preparedPath[0]);
573599

574600
try {
@@ -790,7 +816,9 @@ private function runInternal(Request $request, Response $response): static
790816
}
791817

792818
$method = $request->getMethod();
793-
$route = $this->match($request);
819+
$match = $this->matchInternal($request);
820+
$route = $match[0] ?? null;
821+
$matchedPath = $match[1] ?? '';
794822
$groups = ($route instanceof Route) ? $route->getGroups() : [];
795823

796824
$this->context()->set('route', fn() => $route, []);
@@ -830,17 +858,8 @@ private function runInternal(Request $request, Response $response): static
830858
return $this;
831859
}
832860

833-
if (null === $route && null !== self::$wildcardRoute) {
834-
$route = self::$wildcardRoute;
835-
$this->route = $route;
836-
$path = parse_url($request->getURI(), PHP_URL_PATH);
837-
$path = \is_string($path) ? ($path === '' ? '/' : $path) : '/';
838-
$route->path($path);
839-
840-
$this->context()->set('route', fn() => $route, []);
841-
}
842861
if (null !== $route) {
843-
return $this->execute($route, $request, $response);
862+
return $this->execute($route, $request, $response, $matchedPath);
844863
}
845864

846865
if (self::REQUEST_METHOD_OPTIONS === $method) {
@@ -923,6 +942,5 @@ public static function reset(): void
923942
self::$options = [];
924943
self::$startHooks = [];
925944
self::$requestHooks = [];
926-
self::$wildcardRoute = null;
927945
}
928946
}

src/Http/Route.php

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ class Route extends Hook
3838
*/
3939
protected int $order;
4040

41-
protected string $matchedPath = '';
42-
4341
public function __construct(string $method, string $path)
4442
{
4543
parent::__construct();
@@ -48,17 +46,6 @@ public function __construct(string $method, string $path)
4846
$this->order = ++self::$counter;
4947
}
5048

51-
public function setMatchedPath(string $path): self
52-
{
53-
$this->matchedPath = $path;
54-
return $this;
55-
}
56-
57-
public function getMatchedPath(): string
58-
{
59-
return $this->matchedPath;
60-
}
61-
6249
/**
6350
* Get Route Order ID
6451
*/

src/Http/Router.php

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ class Router
1414

1515
protected static bool $allowOverride = false;
1616

17+
/**
18+
* Fallback route used when no method-specific route matches. Method-agnostic.
19+
*/
20+
protected static ?Route $fallback = null;
21+
1722
/**
1823
* @var array<string,Route[]>
1924
*/
@@ -105,13 +110,28 @@ public static function addRouteAlias(string $path, Route $route): void
105110
self::$routes[$route->getMethod()][$alias] = $route;
106111
}
107112

113+
/**
114+
* Set the method-agnostic fallback route used when nothing else matches.
115+
*/
116+
public static function setFallback(?Route $route): void
117+
{
118+
self::$fallback = $route;
119+
}
120+
108121
/**
109122
* Match route against the method and path.
123+
*
124+
* Returns the matched Route together with the route key it matched against
125+
* (the registered template after placeholder substitution, or '*' for a
126+
* wildcard, or '' for the fallback). Returning the matched key avoids
127+
* mutating the shared Route instance, which would race under coroutines.
128+
*
129+
* @return array{0: Route, 1: string}|null
110130
*/
111-
public static function match(string $method, string $path): ?Route
131+
public static function match(string $method, string $path): ?array
112132
{
113133
if (!\array_key_exists($method, self::$routes)) {
114-
return null;
134+
return self::$fallback !== null ? [self::$fallback, ''] : null;
115135
}
116136

117137
$parts = array_values(array_filter(explode('/', $path), fn($segment) => $segment !== ''));
@@ -129,9 +149,7 @@ public static function match(string $method, string $path): ?Route
129149
);
130150

131151
if (\array_key_exists($match, self::$routes[$method])) {
132-
$route = self::$routes[$method][$match];
133-
$route->setMatchedPath($match);
134-
return $route;
152+
return [self::$routes[$method][$match], $match];
135153
}
136154
}
137155

@@ -140,9 +158,7 @@ public static function match(string $method, string $path): ?Route
140158
*/
141159
$match = self::WILDCARD_TOKEN;
142160
if (\array_key_exists($match, self::$routes[$method])) {
143-
$route = self::$routes[$method][$match];
144-
$route->setMatchedPath($match);
145-
return $route;
161+
return [self::$routes[$method][$match], $match];
146162
}
147163

148164
/**
@@ -152,12 +168,14 @@ public static function match(string $method, string $path): ?Route
152168
$current = ($current ?? '') . "{$part}/";
153169
$match = $current . self::WILDCARD_TOKEN;
154170
if (\array_key_exists($match, self::$routes[$method])) {
155-
$route = self::$routes[$method][$match];
156-
$route->setMatchedPath($match);
157-
return $route;
171+
return [self::$routes[$method][$match], $match];
158172
}
159173
}
160174

175+
if (self::$fallback !== null) {
176+
return [self::$fallback, ''];
177+
}
178+
161179
return null;
162180
}
163181

@@ -219,6 +237,7 @@ public static function preparePath(string $path): array
219237
public static function reset(): void
220238
{
221239
self::$params = [];
240+
self::$fallback = null;
222241
self::$routes = [
223242
Http::REQUEST_METHOD_GET => [],
224243
Http::REQUEST_METHOD_POST => [],

0 commit comments

Comments
 (0)