Skip to content

Commit 07087af

Browse files
loks0nclaude
andcommitted
Fix: Re-read request params at each hook/action call site
Dispatcher::execute hoisted \$this->request->getParams() into a local before any init hooks fired, then passed that stale array to every subsequent getArguments() call. That broke the init-hook-mutates-params contract: if an init hook applied filter/rewrite via \$request->setQueryString(), the route action and shutdown hooks would still see the pre-mutation view. The old Http::execute called \$request->getParams() inline at each call site specifically to preserve this ordering. Restore that behaviour. Adds two regression tests: init-hook mutation visible to the route action, and init-hook mutation visible to a shutdown hook's params. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 47f2349 commit 07087af

2 files changed

Lines changed: 67 additions & 8 deletions

File tree

src/Http/Dispatcher.php

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,41 +143,45 @@ public function execute(RouteMatch $match): void
143143
$route = $match->route;
144144
$groups = $route->getGroups();
145145
$pathValues = $route->getPathValues($this->request, $match->preparedPath);
146-
$requestParams = $this->request->getParams();
146+
147+
// Request params are re-read at each call site: init/request hooks
148+
// may mutate the request (e.g. applying filters), and later hooks +
149+
// the route action must see the updated view. Hoisting this into a
150+
// local would cache stale params across the lifecycle.
147151

148152
try {
149153
if ($route->getHook()) {
150154
foreach (Hooks::$init as $hook) {
151155
if (\in_array('*', $hook->getGroups())) {
152-
\call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams));
156+
\call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $this->request->getParams()));
153157
}
154158
}
155159
}
156160

157161
foreach ($groups as $group) {
158162
foreach (Hooks::$init as $hook) {
159163
if (\in_array($group, $hook->getGroups())) {
160-
\call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams));
164+
\call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $this->request->getParams()));
161165
}
162166
}
163167
}
164168

165169
if (!$this->response->isSent()) {
166-
\call_user_func_array($route->getAction(), $this->http->getArguments($route, $pathValues, $requestParams));
170+
\call_user_func_array($route->getAction(), $this->http->getArguments($route, $pathValues, $this->request->getParams()));
167171
}
168172

169173
foreach ($groups as $group) {
170174
foreach (Hooks::$shutdown as $hook) {
171175
if (\in_array($group, $hook->getGroups())) {
172-
\call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams));
176+
\call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $this->request->getParams()));
173177
}
174178
}
175179
}
176180

177181
if ($route->getHook()) {
178182
foreach (Hooks::$shutdown as $hook) {
179183
if (\in_array('*', $hook->getGroups())) {
180-
\call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $requestParams));
184+
\call_user_func_array($hook->getAction(), $this->http->getArguments($hook, $pathValues, $this->request->getParams()));
181185
}
182186
}
183187
}
@@ -188,7 +192,7 @@ public function execute(RouteMatch $match): void
188192
foreach (Hooks::$errors as $error) {
189193
if (\in_array($group, $error->getGroups())) {
190194
try {
191-
\call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $requestParams));
195+
\call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $this->request->getParams()));
192196
} catch (\Throwable $e) {
193197
throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e);
194198
}
@@ -199,7 +203,7 @@ public function execute(RouteMatch $match): void
199203
foreach (Hooks::$errors as $error) {
200204
if (\in_array('*', $error->getGroups())) {
201205
try {
202-
\call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $requestParams));
206+
\call_user_func_array($error->getAction(), $this->http->getArguments($error, $pathValues, $this->request->getParams()));
203207
} catch (\Throwable $e) {
204208
throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e);
205209
}

tests/DispatcherTest.php

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Utopia\Http\Adapter\FPM\Request;
1010
use Utopia\Http\Adapter\FPM\Response;
1111
use Utopia\Http\Adapter\FPM\Server;
12+
use Utopia\Validator\Text;
1213

1314
/**
1415
* End-to-end coverage for {@see Dispatcher} via Http::run().
@@ -220,6 +221,60 @@ public function testOptionsHookFiresForOptionsMethod(): void
220221
$this->assertStringNotContainsString('GET-HANDLER', $output);
221222
}
222223

224+
public function testInitHookMutationsToRequestParamsAreVisibleToRouteAction(): void
225+
{
226+
// Regression: Dispatcher::execute must re-read $request->getParams()
227+
// at each hook/action call site. Hoisting the array into a local
228+
// before init hooks fire would cache a pre-hook snapshot, so the
229+
// route action would see stale params despite an init hook having
230+
// mutated them (e.g. to apply auth/filter rewrites).
231+
Http::init()
232+
->inject('request')
233+
->action(function (Request $request) {
234+
$request->setQueryString(['x' => 'from-init-hook']);
235+
});
236+
237+
Http::get('/filter-me')
238+
->param('x', 'original', new Text(64), 'x param', true)
239+
->inject('response')
240+
->action(function (string $x, Response $response) {
241+
$response->send($x);
242+
});
243+
244+
$output = $this->runRequest('GET', '/filter-me');
245+
246+
$this->assertSame('from-init-hook', $output);
247+
}
248+
249+
public function testShutdownHookSeesMutationsFromInitHook(): void
250+
{
251+
// The same guarantee for the init → shutdown path: shutdown hooks
252+
// read getArguments() fresh, so an init-time mutation is visible.
253+
Http::init()
254+
->inject('request')
255+
->action(function (Request $request) {
256+
$request->setQueryString(['token' => 'init-token']);
257+
});
258+
259+
Http::shutdown()
260+
->param('token', '', new Text(64), 'token param', true)
261+
->inject('response')
262+
->action(function (string $token, Response $response) {
263+
echo '|shutdown:' . $token;
264+
});
265+
266+
Http::get('/lifecycle-params')
267+
->inject('response')
268+
->action(function (Response $response) {
269+
$response->send('ok');
270+
});
271+
272+
$output = $this->runRequest('GET', '/lifecycle-params');
273+
274+
$this->assertStringContainsString('ok', $output);
275+
$this->assertStringContainsString('|shutdown:init-token', $output);
276+
}
277+
223278
public function testWildcardRouteMatchCarriesWildcardToken(): void
224279
{
225280
Http::wildcard()

0 commit comments

Comments
 (0)