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
14 changes: 10 additions & 4 deletions system/HTTP/IncomingRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,10 @@ public function getDefaultLocale(): string
}

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

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

return $this->fetchFromArray($data, $index, $filter, $flags);
Comment thread
rahul05ranjan marked this conversation as resolved.
}

/**
Expand Down
26 changes: 21 additions & 5 deletions system/HTTP/RequestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,22 @@ public function fetchGlobal(string $name, $index = null, ?int $filter = null, $f
$this->populateGlobals($name);
}

return $this->fetchFromArray($this->globals[$name], $index, $filter, $flags);
}

/**
* Fetches one or more items from an array, applying the same filtering
* and index resolution as fetchGlobal().
*
* @param array<string, mixed> $data
* @param int|list<string>|string|null $index
* @param int|null $filter Filter constant
* @param array<string, mixed>|int|null $flags Options
*
* @return mixed
*/
protected function fetchFromArray(array $data, $index = null, ?int $filter = null, $flags = null)
{
// Null filters cause null values to return.
$filter ??= FILTER_UNSAFE_RAW;
$flags = is_array($flags) ? $flags : (is_numeric($flags) ? (int) $flags : 0);
Expand All @@ -299,9 +315,9 @@ public function fetchGlobal(string $name, $index = null, ?int $filter = null, $f
if ($index === null) {
$values = [];

foreach ($this->globals[$name] as $key => $value) {
foreach ($data as $key => $value) {
$values[$key] = is_array($value)
? $this->fetchGlobal($name, $key, $filter, $flags)
? $this->fetchFromArray($data, $key, $filter, $flags)
: filter_var($value, $filter, $flags);
}

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

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

return $output;
}

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

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

$value ??= $this->globals[$name][$index] ?? null;
$value ??= $data[$index] ?? null;

if (is_array($value)
&& (
Expand Down
45 changes: 45 additions & 0 deletions system/Superglobals.php
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,51 @@ public function setRequestArray(array $array): self
return $this;
}

/**
* Returns the merged $_GET, $_POST, and $_COOKIE data according to the
* `request_order` (or `variables_order`) ini setting, without mutating
* $_REQUEST.
*
* PHP populates $_REQUEST only once at the start of the request. When
* $_GET is modified later (e.g. by SiteURIFactory), $_REQUEST becomes
* stale. This method returns the current merged values so callers can
* read up-to-date request data without relying on the stale $_REQUEST.
*
* @param string|null $requestOrder Overrides the `request_order` ini
* setting. Useful for testing, since the
* ini setting cannot be changed at runtime.
*
* @return array<string, request_items>
*/
public function getRequestData(?string $requestOrder = null): array
{
$requestOrder ??= (string) ini_get('request_order');

if ($requestOrder === '') {
$requestOrder = (string) ini_get('variables_order');
}

if ($requestOrder === '') {
$requestOrder = 'GP';
}

$request = [];

foreach (str_split($requestOrder) as $type) {
match ($type) {
// array_replace_recursive() matches PHP's own $_REQUEST merge
// (php_autoglobal_merge): numeric keys are preserved and
// array values are merged recursively.
'G' => $request = array_replace_recursive($request, $this->get),
'P' => $request = array_replace_recursive($request, $this->post),
'C' => $request = array_replace_recursive($request, $this->cookie),
default => null,
};
}

return $request;
}

/**
* Get all $_FILES values.
*
Expand Down
17 changes: 14 additions & 3 deletions tests/system/HTTP/IncomingRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,23 @@ private function createRequest(?App $config = null, false|string|null $body = nu

public function testCanGrabRequestVars(): void
{
service('superglobals')->setRequest('TEST', '5');
service('superglobals')->setGet('TEST', '5');

$this->assertSame('5', $this->request->getVar('TEST'));
$this->assertNull($this->request->getVar('TESTY'));
}

public function testGetVarReflectsGetChangesWhenRequestIsStale(): void
{
// Simulate the state after SiteURIFactory updates $_GET: $_REQUEST
// still holds the original value while $_GET has been refreshed.
service('superglobals')
->setGetArray(['code' => 'good'])
->setRequestArray(['code' => 'stale']);

$this->assertSame('good', $this->request->getVar('code'));
}

public function testCanGrabGetVars(): void
{
service('superglobals')->setGet('TEST', '5');
Expand Down Expand Up @@ -525,8 +536,8 @@ public function testGetVarWorksWithJsonAndGetParams(): void
$config->baseURL = 'http://example.com/';

// GET method
service('superglobals')->setRequest('foo', 'bar');
service('superglobals')->setRequest('fizz', 'buzz');
service('superglobals')->setGet('foo', 'bar');
service('superglobals')->setGet('fizz', 'buzz');

$request = $this->createRequest($config);
$request = $request->withMethod('GET');
Expand Down
76 changes: 76 additions & 0 deletions tests/system/SuperglobalsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,82 @@ public function testRequestSetArray(): void
$this->assertSame($data, $_REQUEST);
}

public function testGetRequestDataMergesGetAndPost(): void
{
$this->superglobals->setGetArray(['get_key' => 'get_value']);
$this->superglobals->setPostArray(['post_key' => 'post_value']);

$data = $this->superglobals->getRequestData();

$this->assertSame('get_value', $data['get_key']);
$this->assertSame('post_value', $data['post_key']);
}

public function testGetRequestDataReflectsGetChanges(): void
{
$this->superglobals->setGetArray(['key' => 'old']);

$this->assertSame('old', $this->superglobals->getRequestData()['key']);

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

$this->assertSame('new', $this->superglobals->getRequestData()['key']);
}

public function testGetRequestDataMergesCookie(): void
{
$this->superglobals->setGetArray(['get_key' => 'get_value']);
$this->superglobals->setPostArray(['post_key' => 'post_value']);
$this->superglobals->setCookieArray(['cookie_key' => 'cookie_value']);

$data = $this->superglobals->getRequestData('GPC');

$this->assertSame('get_value', $data['get_key']);
$this->assertSame('post_value', $data['post_key']);
$this->assertSame('cookie_value', $data['cookie_key']);
}

public function testGetRequestDataRespectsOrder(): void
{
$this->superglobals->setGetArray(['shared' => 'get']);
$this->superglobals->setPostArray(['shared' => 'post']);
$this->superglobals->setCookieArray(['shared' => 'cookie']);

// Later sources overwrite earlier ones, matching PHP's request_order.
$this->assertSame('post', $this->superglobals->getRequestData('GP')['shared']);
$this->assertSame('cookie', $this->superglobals->getRequestData('GPC')['shared']);
$this->assertSame('get', $this->superglobals->getRequestData('PG')['shared']);
}

public function testGetRequestDataIgnoresUnknownOrderTypes(): void
{
$this->superglobals->setGetArray(['get_key' => 'get_value']);

$data = $this->superglobals->getRequestData('GX');

$this->assertSame(['get_key' => 'get_value'], $data);
}

public function testGetRequestDataPreservesNumericKeys(): void
{
$this->superglobals->setGetArray([100 => 'foo']); // @phpstan-ignore argument.type (numeric keys are valid in superglobals, e.g. ?100=foo)

$data = $this->superglobals->getRequestData('G');

$this->assertSame([100 => 'foo'], $data);
}

public function testGetRequestDataMergesRecursively(): void
{
$this->superglobals->setGetArray(['a' => ['x' => 'get']]);
$this->superglobals->setPostArray(['a' => ['y' => 'post']]);

$data = $this->superglobals->getRequestData('GP');

$this->assertSame(['a' => ['x' => 'get', 'y' => 'post']], $data);
}

// $_FILES tests
public function testFilesGetArray(): void
{
Expand Down
6 changes: 3 additions & 3 deletions tests/system/Validation/ValidationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1289,7 +1289,7 @@ public function testRulesForSingleRuleWithAsteriskWillReturnNoError(): void
$config = new App();
$config->baseURL = 'http://example.com/';

service('superglobals')->setRequestArray([
service('superglobals')->setPostArray([
'id_user' => [
1,
3,
Expand All @@ -1316,7 +1316,7 @@ public function testRulesForSingleRuleWithAsteriskWillReturnError(): void
$config = new App();
$config->baseURL = 'http://example.com/';

service('superglobals')->setRequestArray([
service('superglobals')->setPostArray([
'id_user' => [
'1dfd',
3,
Expand Down Expand Up @@ -1366,7 +1366,7 @@ public function testRulesForSingleRuleWithSingleValue(): void
$config = new App();
$config->baseURL = 'http://example.com/';

service('superglobals')->setRequestArray([
service('superglobals')->setPostArray([
'id_user' => 'gh',
]);

Expand Down
1 change: 1 addition & 0 deletions user_guide_src/source/changelogs/v4.7.5.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ Bugs Fixed
- **Files:** Fixed a bug where ``File::move()`` and ``UploadedFile::move()`` set executable and overly permissive file permissions (``0777 & ~umask()`` instead of ``0666 & ~umask()``), and ``UploadedFile::move()`` targeted the parent directory instead of the destination file for ``chmod()``.
- **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them.
- **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden).
- **HTTP:** Fixed a bug where ``IncomingRequest::getVar()`` returned stale data after ``$_GET`` was updated during URI parsing. It now returns a merged view of ``$_GET``, ``$_POST``, and ``$_COOKIE`` (respecting the ``request_order`` ini setting) instead of reading the stale ``$_REQUEST``.
- **I18n:** Fixed a bug where ``Time::today()``, ``Time::yesterday()``, and ``Time::tomorrow()`` ignored the specified ``$timezone`` and ``setTestNow()`` when calculating the day.
- **Logger:** Fixed a bug where interpolating a log message with array or non-stringable context values could raise PHP warnings or errors.
- **Validation:** Fixed a bug where ``valid_cc_number`` accepted non-digit characters (e.g., a decimal point) in the card number. Such values could pass the Luhn check and triggered an ``Undefined array key`` warning inside it; the number is now checked with ``ctype_digit()``.
Expand Down
8 changes: 5 additions & 3 deletions user_guide_src/source/incoming/incomingrequest.rst
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ getVar()
in new projects. Even if you are already using it, we recommend that you use
another, more appropriate method.

The ``getVar()`` method will pull from ``$_REQUEST``, so will return any data from ``$_GET``, ``$_POST``, or ``$_COOKIE`` (depending on php.ini `request-order <https://www.php.net/manual/en/ini.core.php#ini.request-order>`_).
The ``getVar()`` method returns a merged view of ``$_GET``, ``$_POST``, and ``$_COOKIE`` (depending on php.ini `request-order <https://www.php.net/manual/en/ini.core.php#ini.request-order>`_). It does not read or modify ``$_REQUEST``.

.. warning:: If you want to validate POST data only, don't use ``getVar()``.
Newer values override older values. POST values may be overridden by the
Expand Down Expand Up @@ -373,14 +373,16 @@ The methods provided by the parent classes that are available are:
`Types of filters <https://www.php.net/manual/en/filters.php>`__.
:param int $flags: Flags to apply. A list of flags can be found in
`Filter flags <https://www.php.net/manual/en/filter.constants.php#filter.constants.flags.generic>`__.
:returns: ``$_REQUEST`` if no parameters supplied, otherwise the REQUEST value if found, or null if not
:returns: The merged ``$_GET``, ``$_POST``, and ``$_COOKIE`` data if no parameters supplied, otherwise the value if found, or null if not
:rtype: array|bool|float|int|object|string|null

.. important:: This method exists only for backward compatibility. Do not use it
in new projects. Even if you are already using it, we recommend that you use
another, more appropriate method.

This method is identical to ``getGet()``, only it fetches REQUEST data.
This method is identical to ``getGet()``, only it fetches the merged
``$_GET``, ``$_POST``, and ``$_COOKIE`` data (respecting the
``request_order`` ini setting) instead of ``$_REQUEST``.

.. php:method:: getGet([$index = null[, $filter = null[, $flags = null]]])

Expand Down
Loading