Skip to content

Commit 9ec81ae

Browse files
rahul05ranjanrahul05ranjan
committed
fix: make getVar() read merged superglobals instead of stale $_REQUEST
$_REQUEST is populated only once at the start of the request, so it becomes stale when SiteURIFactory updates $_GET during URI parsing. getVar() previously read the stale $_REQUEST, breaking withRequest() for GET parameters. Instead of mutating $_REQUEST (the approach rejected in #10205), this change makes getVar() return a merged view of $_GET, $_POST, and $_COOKIE according to request_order, leaving $_REQUEST untouched. - Add Superglobals::getRequestData() returning the merged data. - Extract RequestTrait::fetchFromArray() to reuse the filtering logic. - Update getVar() to use getRequestData() + fetchFromArray(). - Revert the SiteURIFactory syncRequest() calls. - Update tests. Fixes #9872
1 parent 908744f commit 9ec81ae

8 files changed

Lines changed: 55 additions & 35 deletions

File tree

‎system/HTTP/IncomingRequest.php‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,10 @@ public function getDefaultLocale(): string
368368
}
369369

370370
/**
371-
* Fetch an item from JSON input stream with fallback to $_REQUEST object. This is the simplest way
372-
* to grab data from the request object and can be used in lieu of the
373-
* other get* methods in most cases.
371+
* Fetch an item from JSON input stream with fallback to the merged
372+
* $_GET, $_POST, and $_COOKIE data. This is the simplest way to grab data
373+
* from the request object and can be used in lieu of the other get*
374+
* methods in most cases.
374375
*
375376
* @param list<string>|string|null $index
376377
* @param int|null $filter Filter constant
@@ -387,7 +388,12 @@ public function getVar($index = null, $filter = null, $flags = null)
387388
return $this->getJsonVar($index, false, $filter, $flags);
388389
}
389390

390-
return $this->fetchGlobal('request', $index, $filter, $flags);
391+
// $_REQUEST is populated only once at the start of the request, so it
392+
// can become stale when $_GET is modified later (e.g. by SiteURIFactory).
393+
// Merge the current superglobals instead of reading the stale $_REQUEST.
394+
$data = service('superglobals')->getRequestData();
395+
396+
return $this->fetchFromArray($data, $index, $filter, $flags);
391397
}
392398

393399
/**

‎system/HTTP/RequestTrait.php‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,22 @@ public function fetchGlobal(string $name, $index = null, ?int $filter = null, $f
291291
$this->populateGlobals($name);
292292
}
293293

294+
return $this->fetchFromArray($this->globals[$name], $index, $filter, $flags);
295+
}
296+
297+
/**
298+
* Fetches one or more items from an array, applying the same filtering
299+
* and index resolution as fetchGlobal().
300+
*
301+
* @param array<string, mixed> $data
302+
* @param int|list<string>|string|null $index
303+
* @param int|null $filter Filter constant
304+
* @param array<string, mixed>|int|null $flags Options
305+
*
306+
* @return mixed
307+
*/
308+
private function fetchFromArray(array $data, $index = null, ?int $filter = null, $flags = null)
309+
{
294310
// Null filters cause null values to return.
295311
$filter ??= FILTER_UNSAFE_RAW;
296312
$flags = is_array($flags) ? $flags : (is_numeric($flags) ? (int) $flags : 0);
@@ -299,9 +315,9 @@ public function fetchGlobal(string $name, $index = null, ?int $filter = null, $f
299315
if ($index === null) {
300316
$values = [];
301317

302-
foreach ($this->globals[$name] as $key => $value) {
318+
foreach ($data as $key => $value) {
303319
$values[$key] = is_array($value)
304-
? $this->fetchGlobal($name, $key, $filter, $flags)
320+
? $this->fetchFromArray($data, $key, $filter, $flags)
305321
: filter_var($value, $filter, $flags);
306322
}
307323

@@ -313,15 +329,15 @@ public function fetchGlobal(string $name, $index = null, ?int $filter = null, $f
313329
$output = [];
314330

315331
foreach ($index as $key) {
316-
$output[$key] = $this->fetchGlobal($name, $key, $filter, $flags);
332+
$output[$key] = $this->fetchFromArray($data, $key, $filter, $flags);
317333
}
318334

319335
return $output;
320336
}
321337

322338
// Does the index contain array notation?
323339
if (is_string($index) && ($count = preg_match_all('/(?:^[^\[]+)|\[[^]]*\]/', $index, $matches)) > 1) {
324-
$value = $this->globals[$name];
340+
$value = $data;
325341

326342
for ($i = 0; $i < $count; $i++) {
327343
$key = trim($matches[0][$i], '[]');
@@ -338,7 +354,7 @@ public function fetchGlobal(string $name, $index = null, ?int $filter = null, $f
338354
}
339355
}
340356

341-
$value ??= $this->globals[$name][$index] ?? null;
357+
$value ??= $data[$index] ?? null;
342358

343359
if (is_array($value)
344360
&& (

‎system/HTTP/SiteURIFactory.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ private function parseRequestURI(): string
171171

172172
// Update our global GET for values likely to have been changed
173173
parse_str($this->superglobals->server('QUERY_STRING'), $get);
174-
$this->superglobals->setGetArray($get)->syncRequest();
174+
$this->superglobals->setGetArray($get);
175175

176176
return URI::removeDotSegments($path);
177177
}
@@ -203,7 +203,7 @@ private function parseQueryString(): string
203203

204204
// Update our global GET for values likely to have been changed
205205
parse_str($this->superglobals->server('QUERY_STRING'), $get);
206-
$this->superglobals->setGetArray($get)->syncRequest();
206+
$this->superglobals->setGetArray($get);
207207

208208
return URI::removeDotSegments($path);
209209
}

‎system/Superglobals.php‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -388,16 +388,18 @@ public function setRequestArray(array $array): self
388388
}
389389

390390
/**
391-
* Rebuilds $_REQUEST from $_GET, $_POST, and $_COOKIE according to the
392-
* `request_order` (or `variables_order`) ini setting.
391+
* Returns the merged $_GET, $_POST, and $_COOKIE data according to the
392+
* `request_order` (or `variables_order`) ini setting, without mutating
393+
* $_REQUEST.
393394
*
394395
* PHP populates $_REQUEST only once at the start of the request. When
395396
* $_GET is modified later (e.g. by SiteURIFactory), $_REQUEST becomes
396-
* stale. This method re-synchronizes $_REQUEST with the current values.
397+
* stale. This method returns the current merged values so callers can
398+
* read up-to-date request data without relying on the stale $_REQUEST.
397399
*
398-
* @return self
400+
* @return array<string, request_items>
399401
*/
400-
public function syncRequest(): self
402+
public function getRequestData(): array
401403
{
402404
$requestOrder = ini_get('request_order') ?: ini_get('variables_order') ?: 'GP';
403405

@@ -412,7 +414,7 @@ public function syncRequest(): self
412414
};
413415
}
414416

415-
return $this->setRequestArray($request);
417+
return $request;
416418
}
417419

418420
/**

‎tests/system/HTTP/IncomingRequestTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ private function createRequest(?App $config = null, false|string|null $body = nu
7070

7171
public function testCanGrabRequestVars(): void
7272
{
73-
service('superglobals')->setRequest('TEST', '5');
73+
service('superglobals')->setGet('TEST', '5');
7474

7575
$this->assertSame('5', $this->request->getVar('TEST'));
7676
$this->assertNull($this->request->getVar('TESTY'));
@@ -525,8 +525,8 @@ public function testGetVarWorksWithJsonAndGetParams(): void
525525
$config->baseURL = 'http://example.com/';
526526

527527
// GET method
528-
service('superglobals')->setRequest('foo', 'bar');
529-
service('superglobals')->setRequest('fizz', 'buzz');
528+
service('superglobals')->setGet('foo', 'bar');
529+
service('superglobals')->setGet('fizz', 'buzz');
530530

531531
$request = $this->createRequest($config);
532532
$request = $request->withMethod('GET');

‎tests/system/HTTP/SiteURIFactoryDetectRoutePathTest.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,6 @@ public function testQueryStringWithQueryString(): void
256256
$this->assertSame($expected, $factory->detectRoutePath('QUERY_STRING'));
257257
$this->assertSame('code=good', $_SERVER['QUERY_STRING']); // @phpstan-ignore codeigniter.superglobalsOffsetAccess (checks the live superglobal, not the snapshot service)
258258
$this->assertSame(['code' => 'good'], $_GET);
259-
$this->assertSame(['code' => 'good'], $_REQUEST); // @phpstan-ignore codeigniter.superglobalsOffsetAccess (checks the live superglobal, not the snapshot service)
260259
}
261260

262261
public function testQueryStringEmpty(): void

‎tests/system/SuperglobalsTest.php‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -302,32 +302,29 @@ public function testRequestSetArray(): void
302302
$this->assertSame($data, $_REQUEST);
303303
}
304304

305-
public function testSyncRequestRebuildsRequestFromGetPostCookie(): void
305+
public function testGetRequestDataMergesGetPostCookie(): void
306306
{
307307
$this->superglobals->setGetArray(['get_key' => 'get_value']);
308308
$this->superglobals->setPostArray(['post_key' => 'post_value']);
309309
$this->superglobals->setCookieArray(['cookie_key' => 'cookie_value']);
310310

311-
$this->superglobals->syncRequest();
311+
$data = $this->superglobals->getRequestData();
312312

313-
$this->assertSame('get_value', $this->superglobals->request('get_key'));
314-
$this->assertSame('post_value', $this->superglobals->request('post_key'));
315-
$this->assertSame('cookie_value', $this->superglobals->request('cookie_key'));
316-
$this->assertSame('get_value', $_REQUEST['get_key']); // @phpstan-ignore codeigniter.superglobalsOffsetAccess (checks the live superglobal, not the snapshot service)
313+
$this->assertSame('get_value', $data['get_key']);
314+
$this->assertSame('post_value', $data['post_key']);
315+
$this->assertSame('cookie_value', $data['cookie_key']);
317316
}
318317

319-
public function testSyncRequestReflectsGetChanges(): void
318+
public function testGetRequestDataReflectsGetChanges(): void
320319
{
321320
$this->superglobals->setGetArray(['key' => 'old']);
322-
$this->superglobals->syncRequest();
323321

324-
$this->assertSame('old', $this->superglobals->request('key'));
322+
$this->assertSame('old', $this->superglobals->getRequestData()['key']);
325323

326324
// Simulate SiteURIFactory updating $_GET after the request started.
327325
$this->superglobals->setGetArray(['key' => 'new']);
328-
$this->superglobals->syncRequest();
329326

330-
$this->assertSame('new', $this->superglobals->request('key'));
327+
$this->assertSame('new', $this->superglobals->getRequestData()['key']);
331328
}
332329

333330
// $_FILES tests

‎tests/system/Validation/ValidationTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,7 +1289,7 @@ public function testRulesForSingleRuleWithAsteriskWillReturnNoError(): void
12891289
$config = new App();
12901290
$config->baseURL = 'http://example.com/';
12911291

1292-
service('superglobals')->setRequestArray([
1292+
service('superglobals')->setPostArray([
12931293
'id_user' => [
12941294
1,
12951295
3,
@@ -1316,7 +1316,7 @@ public function testRulesForSingleRuleWithAsteriskWillReturnError(): void
13161316
$config = new App();
13171317
$config->baseURL = 'http://example.com/';
13181318

1319-
service('superglobals')->setRequestArray([
1319+
service('superglobals')->setPostArray([
13201320
'id_user' => [
13211321
'1dfd',
13221322
3,
@@ -1366,7 +1366,7 @@ public function testRulesForSingleRuleWithSingleValue(): void
13661366
$config = new App();
13671367
$config->baseURL = 'http://example.com/';
13681368

1369-
service('superglobals')->setRequestArray([
1369+
service('superglobals')->setPostArray([
13701370
'id_user' => 'gh',
13711371
]);
13721372

0 commit comments

Comments
 (0)