diff --git a/.github/workflows/deploy-apidocs.yml b/.github/workflows/deploy-apidocs.yml index 7272ec7bec60..01cd0c83d28a 100644 --- a/.github/workflows/deploy-apidocs.yml +++ b/.github/workflows/deploy-apidocs.yml @@ -44,7 +44,7 @@ jobs: persist-credentials: false - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: '8.2' tools: phive diff --git a/.github/workflows/deploy-userguide-latest.yml b/.github/workflows/deploy-userguide-latest.yml index 0cba414e591f..92c7706eaa6a 100644 --- a/.github/workflows/deploy-userguide-latest.yml +++ b/.github/workflows/deploy-userguide-latest.yml @@ -30,7 +30,7 @@ jobs: persist-credentials: false - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: '8.2' coverage: none diff --git a/.github/workflows/reusable-coveralls.yml b/.github/workflows/reusable-coveralls.yml index ee00cf9e0ee5..4786f6ee433c 100644 --- a/.github/workflows/reusable-coveralls.yml +++ b/.github/workflows/reusable-coveralls.yml @@ -22,7 +22,7 @@ jobs: persist-credentials: false - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ inputs.php-version }} tools: composer diff --git a/.github/workflows/reusable-phpunit-test.yml b/.github/workflows/reusable-phpunit-test.yml index 62c94e334f1a..0fa024b71c61 100644 --- a/.github/workflows/reusable-phpunit-test.yml +++ b/.github/workflows/reusable-phpunit-test.yml @@ -175,7 +175,7 @@ jobs: persist-credentials: false - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ inputs.php-version }} tools: composer diff --git a/.github/workflows/reusable-serviceless-phpunit-test.yml b/.github/workflows/reusable-serviceless-phpunit-test.yml index 3df45b47822b..04333f91ef6f 100644 --- a/.github/workflows/reusable-serviceless-phpunit-test.yml +++ b/.github/workflows/reusable-serviceless-phpunit-test.yml @@ -72,7 +72,7 @@ jobs: fetch-depth: 0 - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ inputs.php-version }} tools: composer diff --git a/.github/workflows/test-autoreview.yml b/.github/workflows/test-autoreview.yml index 32759a6f348f..ff816aaeb519 100644 --- a/.github/workflows/test-autoreview.yml +++ b/.github/workflows/test-autoreview.yml @@ -39,7 +39,7 @@ jobs: uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: '8.2' diff --git a/.github/workflows/test-coding-standards.yml b/.github/workflows/test-coding-standards.yml index d19fffce17d9..7bbe338c37c7 100644 --- a/.github/workflows/test-coding-standards.yml +++ b/.github/workflows/test-coding-standards.yml @@ -38,7 +38,7 @@ jobs: uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php-version }} extensions: tokenizer diff --git a/.github/workflows/test-phpstan.yml b/.github/workflows/test-phpstan.yml index 8f3b6689883f..6c1461cb3318 100644 --- a/.github/workflows/test-phpstan.yml +++ b/.github/workflows/test-phpstan.yml @@ -49,7 +49,7 @@ jobs: uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: '8.2' extensions: intl diff --git a/.github/workflows/test-psalm.yml b/.github/workflows/test-psalm.yml index 19bbce540964..e417d3f99746 100644 --- a/.github/workflows/test-psalm.yml +++ b/.github/workflows/test-psalm.yml @@ -41,7 +41,7 @@ jobs: persist-credentials: false - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php-version }} extensions: intl, json, mbstring, xml, mysqli, oci8, pgsql, sqlsrv, sqlite3 diff --git a/.github/workflows/test-random-execution.yml b/.github/workflows/test-random-execution.yml index adbb28fa57d6..9b8d64160e47 100644 --- a/.github/workflows/test-random-execution.yml +++ b/.github/workflows/test-random-execution.yml @@ -174,7 +174,7 @@ jobs: fetch-depth: 0 - name: Setup PHP ${{ matrix.php-version }} - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php-version }} extensions: gd, curl, iconv, json, mbstring, openssl, sodium diff --git a/.github/workflows/test-rector.yml b/.github/workflows/test-rector.yml index c6c7efc8c8ea..25079edbd2b2 100644 --- a/.github/workflows/test-rector.yml +++ b/.github/workflows/test-rector.yml @@ -56,7 +56,7 @@ jobs: uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php-version }} extensions: intl diff --git a/.github/workflows/test-structarmed.yml b/.github/workflows/test-structarmed.yml index bdbf8c5e9247..905e26ab4f31 100644 --- a/.github/workflows/test-structarmed.yml +++ b/.github/workflows/test-structarmed.yml @@ -54,7 +54,7 @@ jobs: uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - name: Setup PHP - uses: shivammathur/setup-php@7c071dfe9dc99bdf297fa79cb49ea005b9fcadbc # 2.37.1 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php-version }} extensions: intl diff --git a/composer.json b/composer.json index 3e594e090a89..6fc90fbb3340 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ "psr/log": "^3.0" }, "require-dev": { - "boundwize/structarmed": "0.12.0", + "boundwize/structarmed": "0.12.6", "codeigniter/phpstan-codeigniter": "^1.5", "fakerphp/faker": "^1.24", "kint-php/kint": "^6.1", diff --git a/system/API/BaseTransformer.php b/system/API/BaseTransformer.php index 540d443d1cdc..4badb8267139 100644 --- a/system/API/BaseTransformer.php +++ b/system/API/BaseTransformer.php @@ -54,6 +54,11 @@ */ abstract class BaseTransformer implements TransformerInterface { + /** + * Nesting depth of the transformation currently in progress (0 = root). + */ + private static int $depth = 0; + /** * @var list|null */ @@ -69,17 +74,24 @@ abstract class BaseTransformer implements TransformerInterface public function __construct( private ?IncomingRequest $request = null, ) { + // An explicitly provided request is always honored - its scope is + // intentional. Only the implicit global fallback is suppressed for + // nested transformers, which is the actual leak vector. + $explicitRequest = $request instanceof IncomingRequest; + $this->request = $request ?? request(); - $fields = $this->request->getGet('fields'); - $this->fields = is_string($fields) - ? array_map(trim(...), explode(',', $fields)) - : $fields; + if ($explicitRequest || self::$depth === 0) { + $fields = $this->request->getGet('fields'); + $this->fields = is_string($fields) + ? array_map(trim(...), explode(',', $fields)) + : $fields; - $includes = $this->request->getGet('include'); - $this->includes = is_string($includes) - ? array_map(trim(...), explode(',', $includes)) - : $includes; + $includes = $this->request->getGet('include'); + $this->includes = is_string($includes) + ? array_map(trim(...), explode(',', $includes)) + : $includes; + } } /** @@ -207,13 +219,19 @@ private function insertIncludes(array $data): array } } - foreach ($this->includes as $include) { - $method = 'include' . ucfirst($include); - if (method_exists($this, $method)) { - $data[$include] = $this->{$method}(); - } else { - throw ApiException::forMissingInclude($include); + self::$depth++; + + try { + foreach ($this->includes as $include) { + $method = 'include' . ucfirst($include); + if (method_exists($this, $method)) { + $data[$include] = $this->{$method}(); + } else { + throw ApiException::forMissingInclude($include); + } } + } finally { + self::$depth--; } return $data; diff --git a/system/Database/BaseResult.php b/system/Database/BaseResult.php index c0bdc2aa1025..5bede241a59d 100644 --- a/system/Database/BaseResult.php +++ b/system/Database/BaseResult.php @@ -312,7 +312,7 @@ public function getCustomRowObject(int $n, string $className) $this->currentRow = $n; } - return $this->customResultObject[$className][$this->currentRow]; + return $this->customResultObject[$className][$this->currentRow] ?? null; } /** @@ -333,7 +333,7 @@ public function getRowArray(int $n = 0) $this->currentRow = $n; } - return $result[$this->currentRow]; + return $result[$this->currentRow] ?? null; } /** @@ -350,11 +350,11 @@ public function getRowObject(int $n = 0) return null; } - if ($n !== $this->customResultObject && isset($result[$n])) { + if ($n !== $this->currentRow && isset($result[$n])) { $this->currentRow = $n; } - return $result[$this->currentRow]; + return $result[$this->currentRow] ?? null; } /** @@ -440,7 +440,7 @@ public function getPreviousRow(string $type = 'object') $this->currentRow--; } - return $result[$this->currentRow]; + return $result[$this->currentRow] ?? null; } /** diff --git a/system/Session/Handlers/FileHandler.php b/system/Session/Handlers/FileHandler.php index f5e9261af0f4..62587e71b83e 100644 --- a/system/Session/Handlers/FileHandler.php +++ b/system/Session/Handlers/FileHandler.php @@ -240,17 +240,19 @@ public function close(): bool */ public function destroy($id): bool { + $filePath = $this->filePath . $id; + if ($this->close()) { - return is_file($this->filePath . $id) - ? (unlink($this->filePath . $id) && $this->destroyCookie()) + return (! is_link($filePath) && is_file($filePath)) + ? (@unlink($filePath) && $this->destroyCookie()) : true; } if ($this->filePath !== null) { clearstatcache(); - return is_file($this->filePath . $id) - ? (unlink($this->filePath . $id) && $this->destroyCookie()) + return (! is_link($filePath) && is_file($filePath)) + ? (@unlink($filePath) && $this->destroyCookie()) : true; } @@ -284,15 +286,19 @@ public function gc($max_lifetime): false|int while (($file = readdir($directory)) !== false) { // If the filename doesn't match this pattern, it's either not a session file or is not ours - if (preg_match($pattern, $file) !== 1 - || ! is_file($this->savePath . DIRECTORY_SEPARATOR . $file) - || ($mtime = filemtime($this->savePath . DIRECTORY_SEPARATOR . $file)) === false - || $mtime > $ts - ) { + if (preg_match($pattern, $file) !== 1) { + continue; + } + + $filePath = $this->savePath . DIRECTORY_SEPARATOR . $file; + + $stat = @lstat($filePath); + + if ($stat === false || is_link($filePath) || ! is_file($filePath) || $stat['mtime'] > $ts) { continue; } - unlink($this->savePath . DIRECTORY_SEPARATOR . $file); + @unlink($filePath); $collected++; } diff --git a/tests/_support/API/ChildTransformer.php b/tests/_support/API/ChildTransformer.php new file mode 100644 index 000000000000..a226dad97608 --- /dev/null +++ b/tests/_support/API/ChildTransformer.php @@ -0,0 +1,31 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support\API; + +use CodeIgniter\API\BaseTransformer; + +/** + * Nested transformer used to verify that the root request's scope + * (fields/includes) does not leak into related resources. + */ +class ChildTransformer extends BaseTransformer +{ + public function toArray(mixed $resource): array + { + return [ + 'child_id' => $resource['id'] ?? null, + 'status' => 'transformed', + ]; + } +} diff --git a/tests/_support/API/ParentTransformer.php b/tests/_support/API/ParentTransformer.php new file mode 100644 index 000000000000..803b4dbb353f --- /dev/null +++ b/tests/_support/API/ParentTransformer.php @@ -0,0 +1,67 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace Tests\Support\API; + +use CodeIgniter\API\BaseTransformer; + +/** + * Root transformer used to verify that nested transformers do not inherit + * the root request's `fields`/`include` query state. + */ +class ParentTransformer extends BaseTransformer +{ + public function toArray(mixed $resource): array + { + return [ + 'parent_id' => $resource['id'] ?? null, + ]; + } + + /** + * Includes a single related child resource. + * + * @return array + */ + protected function includeChildren(): array + { + return (new ChildTransformer())->transform(['id' => 99]); + } + + /** + * Includes a collection of related child resources. + * + * @return list> + */ + protected function includeChildrenCollection(): array + { + return (new ChildTransformer())->transformMany([ + ['id' => 77], + ['id' => 88], + ]); + } + + /** + * Includes a single child while explicitly forwarding the request, opting + * the child into the request-derived scope even though it is nested. + * + * @return array + */ + protected function includeExplicitChild(): array + { + $childRequest = clone request(); + $childRequest->setGlobal('get', ['fields' => 'child_id']); + + return (new ChildTransformer($childRequest))->transform(['id' => 99]); + } +} diff --git a/tests/system/API/TransformerTest.php b/tests/system/API/TransformerTest.php index 8c6f50857d23..11f37f0fd859 100644 --- a/tests/system/API/TransformerTest.php +++ b/tests/system/API/TransformerTest.php @@ -22,6 +22,8 @@ use Config\Services; use PHPUnit\Framework\Attributes\Group; use stdClass; +use Tests\Support\API\ChildTransformer; +use Tests\Support\API\ParentTransformer; /** * @internal @@ -33,12 +35,14 @@ protected function setUp(): void { parent::setUp(); + Services::resetSingle('request'); Services::superglobals()->setGetArray([]); } protected function tearDown(): void { Services::superglobals()->setGetArray([]); + Services::resetSingle('request'); parent::tearDown(); } @@ -641,4 +645,160 @@ protected function includePosts(): array $this->assertArrayHasKey('posts', $result); $this->assertSame([['id' => 1, 'title' => 'Post 1']], $result['posts']); } + + public function testNestedTransformerDoesNotInheritIncludeState(): void + { + // The child transformer has no includeChildren() method. If the root + // request's `include=children` leaked into it, transforming the child + // would raise an ApiException for the missing include method. + $request = $this->createMockRequest('include=children'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1]); + + $this->assertSame([ + 'parent_id' => 1, + 'children' => ['child_id' => 99, 'status' => 'transformed'], + ], $result); + } + + public function testNestedTransformerDoesNotInheritFieldFilter(): void + { + // `fields=parent_id` applies to the root only. If it leaked into the + // child, array_intersect_key would strip every child field, leaving []. + $request = $this->createMockRequest('include=children&fields=parent_id'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1]); + + $this->assertSame([ + 'parent_id' => 1, + 'children' => ['child_id' => 99, 'status' => 'transformed'], + ], $result); + } + + public function testNestedCollectionTransformerDoesNotInheritState(): void + { + $request = $this->createMockRequest('include=childrenCollection&fields=parent_id'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1]); + + $this->assertSame([ + 'parent_id' => 1, + 'childrenCollection' => [ + ['child_id' => 77, 'status' => 'transformed'], + ['child_id' => 88, 'status' => 'transformed'], + ], + ], $result); + } + + public function testBareNestedInstantiationDoesNotInheritState(): void + { + // Reproduces the exact leak vector: a child created with a bare + // `new ChildTransformer()` (no request passed) inside an include + // method must not pick up the root request's scope from request(). + $request = $this->createMockRequest('include=children&fields=parent_id'); + Services::injectMock('request', $request); + + $root = new ParentTransformer(); + + $result = $root->transform(['id' => 5]); + + $this->assertSame([ + 'parent_id' => 5, + 'children' => ['child_id' => 99, 'status' => 'transformed'], + ], $result); + } + + public function testNestedTransformerHonorsExplicitRequest(): void + { + // A child created with an explicitly passed request must honor that + // request's scope even while nested - the isolation only suppresses + // the implicit global fallback, not deliberate developer intent. + $request = $this->createMockRequest('include=explicitChild&fields=child_id,parent_id'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1]); + + $this->assertSame([ + 'parent_id' => 1, + 'explicitChild' => ['child_id' => 99], + ], $result); + } + + public function testRootScopeStillAppliesAfterNesting(): void + { + // Sanity check that the root transformer keeps applying its own scope + // while nested children are isolated. + $request = $this->createMockRequest('include=children&fields=parent_id'); + Services::injectMock('request', $request); + + $transformer = new ParentTransformer($request); + + $result = $transformer->transform(['id' => 1, 'secret' => 'hidden']); + + // Root keeps only parent_id (plus the include key), the child is intact. + $this->assertArrayHasKey('parent_id', $result); + $this->assertArrayNotHasKey('secret', $result); + $this->assertSame(['child_id' => 99, 'status' => 'transformed'], $result['children']); + } + + public function testDepthIsRestoredAfterIncludeThrows(): void + { + $request = $this->createMockRequest('include=nonexistent'); + Services::injectMock('request', $request); + + $throwingRoot = new class ($request) extends BaseTransformer { + public function toArray(mixed $resource): array + { + return $resource; + } + }; + + try { + $throwingRoot->transform(['id' => 1, 'name' => 'Test']); + $this->fail('Expected ApiException was not thrown.'); + } catch (ApiException) { + // expected + } + + // The nesting depth must be balanced after the exception, so a fresh + // root transformer still applies the request scope (depth back to 0). + $fieldsRequest = $this->createMockRequest('fields=id'); + Services::injectMock('request', $fieldsRequest); + + $nextRoot = new class ($fieldsRequest) extends BaseTransformer { + public function toArray(mixed $resource): array + { + return $resource; + } + }; + + $result = $nextRoot->transform(['id' => 1, 'name' => 'Test']); + + $this->assertSame(['id' => 1], $result); + } + + public function testBareNestedTransformerStillUsedByChildTransformerDirectly(): void + { + // When ChildTransformer is itself the root (no parent), it must apply + // request scope as usual - the isolation only affects nesting. + $request = $this->createMockRequest('fields=child_id'); + Services::injectMock('request', $request); + + $transformer = new ChildTransformer($request); + + $result = $transformer->transform(['id' => 42]); + + $this->assertSame(['child_id' => 42], $result); + } } diff --git a/tests/system/Database/BaseResultTest.php b/tests/system/Database/BaseResultTest.php new file mode 100644 index 000000000000..901faee6a565 --- /dev/null +++ b/tests/system/Database/BaseResultTest.php @@ -0,0 +1,332 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Database; + +use CodeIgniter\Test\CIUnitTestCase; +use PHPUnit\Framework\Attributes\Group; +use stdClass; + +/** + * @internal + */ +#[Group('Others')] +final class BaseResultTest extends CIUnitTestCase +{ + /** + * Create a minimal concrete implementation of BaseResult for testing. + * + * @param list> $resultArray Result set as arrays. + * @param list $resultObject Result set as objects. + * + * @return BaseResult + */ + private function createResultDouble(array $resultArray, array $resultObject): BaseResult + { + return new + /** + * @extends BaseResult + */ + class ($resultArray, $resultObject) extends BaseResult { + /** + * @param list> $resultArray Result set as arrays. + * @param list $resultObject Result set as objects. + */ + public function __construct(array $resultArray, array $resultObject) + { + $this->resultArray = $resultArray; + $this->resultObject = $resultObject; + $this->currentRow = 0; + + $connId = null; + $resultId = null; + parent::__construct($connId, $resultId); + } + + public function getFieldCount(): int + { + return 0; + } + + /** + * @return list + */ + public function getFieldNames(): array + { + return []; + } + + /** + * @return list + */ + public function getFieldData(): array + { + return []; + } + + public function freeResult(): void + { + } + + public function dataSeek(int $n = 0): bool + { + return true; + } + + /** + * @return false|list>|null + */ + protected function fetchAssoc(): array|bool|null + { + return false; + } + + protected function fetchObject(string $className = stdClass::class) + { + return false; + } + }; + } + + // -------------------------------------------------------------------- + // getRowArray() + // -------------------------------------------------------------------- + + public function testGetRowArrayReturnsRow(): void + { + $result = $this->createResultDouble( + [ + ['id' => 1, 'name' => 'John'], + ['id' => 2, 'name' => 'Jane'], + ], + [], + ); + + $this->assertSame(['id' => 1, 'name' => 'John'], $result->getRowArray(0)); + $this->assertSame(['id' => 2, 'name' => 'Jane'], $result->getRowArray(1)); + } + + public function testGetRowArrayReturnsNullForEmptyResult(): void + { + $result = $this->createResultDouble([], []); + + $this->assertNull($result->getRowArray(0)); + } + + public function testGetRowArrayReturnsFirstRowByDefault(): void + { + $result = $this->createResultDouble( + [ + ['id' => 1, 'name' => 'John'], + ['id' => 2, 'name' => 'Jane'], + ], + [], + ); + + $this->assertSame(['id' => 1, 'name' => 'John'], $result->getRowArray()); + } + + // -------------------------------------------------------------------- + // getRowObject() + // -------------------------------------------------------------------- + + public function testGetRowObjectReturnsObject(): void + { + $row1 = new stdClass(); + $row1->id = 1; + $row1->name = 'John'; + $row2 = new stdClass(); + $row2->id = 2; + $row2->name = 'Jane'; + + $result = $this->createResultDouble([], [$row1, $row2]); + + $this->assertSame($row1, $result->getRowObject(0)); + $this->assertSame($row2, $result->getRowObject(1)); + } + + public function testGetRowObjectReturnsNullForEmptyResult(): void + { + $result = $this->createResultDouble([], []); + + $this->assertNull($result->getRowObject(0)); + } + + public function testGetRowObjectReturnsFirstRowByDefault(): void + { + $row1 = new stdClass(); + $row1->id = 1; + $row1->name = 'John'; + + $result = $this->createResultDouble([], [$row1]); + + $this->assertSame($row1, $result->getRowObject()); + } + + public function testGetRowObjectAndGetRowArrayShareCurrentRow(): void + { + $row1 = new stdClass(); + $row1->id = 1; + $row1->name = 'John'; + $row2 = new stdClass(); + $row2->id = 2; + $row2->name = 'Jane'; + + $result = $this->createResultDouble( + [ + ['id' => 1, 'name' => 'John'], + ['id' => 2, 'name' => 'Jane'], + ], + [$row1, $row2], + ); + + // getRowObject(1) should advance currentRow to 1 (same as getRowArray would) + $result->getRowObject(1); + $this->assertSame(['id' => 2, 'name' => 'Jane'], $result->getRowArray(1)); + } + + public function testGetRowObjectUsesCurrentRowLikeGetRowArray(): void + { + $row1 = new stdClass(); + $row1->id = 1; + $row1->name = 'John'; + $row2 = new stdClass(); + $row2->id = 2; + $row2->name = 'Jane'; + + $result = $this->createResultDouble( + [ + ['id' => 1, 'name' => 'John'], + ['id' => 2, 'name' => 'Jane'], + ], + [$row1, $row2], + ); + + // Both methods should advance currentRow consistently + $result->getRowObject(1); + $result->getRowArray(); + $this->assertSame($row1, $result->getRowObject()); + } + + // -------------------------------------------------------------------- + // getRow() — convenience wrapper + // -------------------------------------------------------------------- + + public function testGetRowWithInvalidIndexReturnsFirstRow(): void + { + $result = $this->createResultDouble( + [['id' => 1, 'name' => 'John']], + [], + ); + + $this->assertSame(['id' => 1, 'name' => 'John'], $result->getRow(999, 'array')); + } + + public function testGetRowObjectWithInvalidIndexReturnsFirstRow(): void + { + $row1 = new stdClass(); + $row1->id = 1; + $row1->name = 'John'; + + $result = $this->createResultDouble([], [$row1]); + + $this->assertSame($row1, $result->getRow(999, 'object')); + } + + public function testGetRowNullForColumnNameNotFound(): void + { + $result = $this->createResultDouble( + [['id' => 1, 'name' => 'John']], + [], + ); + + $this->assertNull($result->getRow('nonexistent', 'array')); + } + + // -------------------------------------------------------------------- + // Custom Result Object + // -------------------------------------------------------------------- + + public function testGetCustomRowObjectWithInvalidIndexReturnsFirstRow(): void + { + $row = new stdClass(); + $row->id = 1; + $row->name = 'John'; + + $result = $this->createResultDouble([], []); + $result->customResultObject[stdClass::class] = [$row]; + + $this->assertSame($row, $result->getCustomRowObject(999, stdClass::class)); + } + + // -------------------------------------------------------------------- + // Fallback Tests (Null return on invalid currentRow) + // -------------------------------------------------------------------- + + public function testGetRowArrayReturnsNullWhenCurrentRowIsInvalid(): void + { + $result = $this->createResultDouble( + [['id' => 1, 'name' => 'John']], + [], + ); + + $result->currentRow = 999; + + $this->assertNull($result->getRowArray(999)); + } + + public function testGetRowObjectReturnsNullWhenCurrentRowIsInvalid(): void + { + $row1 = new stdClass(); + $row1->id = 1; + $row1->name = 'John'; + + $result = $this->createResultDouble( + [], + [$row1], + ); + + $result->currentRow = 999; + + $this->assertNull($result->getRowObject(999)); + } + + public function testGetCustomRowObjectReturnsNullWhenCurrentRowIsInvalid(): void + { + $row1 = new stdClass(); + $row1->id = 1; + $row1->name = 'John'; + + $result = $this->createResultDouble([], []); + $result->customResultObject[stdClass::class] = [$row1]; + + $result->currentRow = 999; + + $this->assertNotInstanceOf(stdClass::class, $result->getCustomRowObject(999, stdClass::class)); + } + + public function testGetPreviousRowReturnsNullWhenCurrentRowIsInvalid(): void + { + $result = $this->createResultDouble( + [ + ['id' => 1], + ['id' => 2], + ], + [], + ); + + $result->currentRow = -1; + + $this->assertNull($result->getPreviousRow('array')); + } +} diff --git a/tests/system/Session/Handlers/FileHandlerTest.php b/tests/system/Session/Handlers/FileHandlerTest.php new file mode 100644 index 000000000000..e7e532a631c4 --- /dev/null +++ b/tests/system/Session/Handlers/FileHandlerTest.php @@ -0,0 +1,105 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Session\Handlers; + +use CodeIgniter\Test\CIUnitTestCase; +use Config\Session as SessionConfig; +use PHPUnit\Framework\Attributes\Group; + +/** + * @internal + */ +#[Group('Others')] +final class FileHandlerTest extends CIUnitTestCase +{ + private string $savePath; + + protected function setUp(): void + { + parent::setUp(); + + $this->savePath = WRITEPATH . 'session_test'; + if (! is_dir($this->savePath)) { + mkdir($this->savePath, 0700, true); + } + } + + protected function tearDown(): void + { + parent::tearDown(); + + // clean up + if (is_dir($this->savePath)) { + $files = array_diff(scandir($this->savePath), ['.', '..']); + + foreach ($files as $file) { + $path = $this->savePath . DIRECTORY_SEPARATOR . $file; + if (is_link($path) || is_file($path)) { + @unlink($path); + } + } + rmdir($this->savePath); + } + } + + public function testGcIgnoresSymlinks(): void + { + $config = new SessionConfig(); + $config->savePath = $this->savePath; + $config->cookieName = 'ci_session'; + + $handler = new FileHandler($config, '127.0.0.1'); + + $sessionId = '1234567890abcdef1234567890abcdef'; + $sessionFile = $this->savePath . DIRECTORY_SEPARATOR . 'ci_session' . $sessionId; + + $targetFile = $this->savePath . DIRECTORY_SEPARATOR . 'target.txt'; + file_put_contents($targetFile, 'target'); + touch($targetFile, time() - 10000); + + symlink($targetFile, $sessionFile); + + $this->assertTrue(is_link($sessionFile)); + + $collected = $handler->gc(5000); + + $this->assertTrue(is_link($sessionFile)); + $this->assertSame(0, $collected); + } + + public function testDestroyIgnoresSymlinks(): void + { + $config = new SessionConfig(); + $config->savePath = $this->savePath; + $config->cookieName = 'ci_session'; + + $handler = new FileHandler($config, '127.0.0.1'); + + $sessionId = '1234567890abcdef1234567890abcdef'; + $handler->open($this->savePath, 'ci_session'); + + $sessionFile = $this->savePath . DIRECTORY_SEPARATOR . 'ci_session' . $sessionId; + + $targetFile = $this->savePath . DIRECTORY_SEPARATOR . 'target.txt'; + file_put_contents($targetFile, 'target'); + + symlink($targetFile, $sessionFile); + + $this->assertTrue(is_link($sessionFile)); + + $handler->destroy($sessionId); + + $this->assertTrue(is_link($sessionFile)); + } +} diff --git a/user_guide_src/source/changelogs/v4.7.4.rst b/user_guide_src/source/changelogs/v4.7.4.rst index daf64bbda631..7a38176717c0 100644 --- a/user_guide_src/source/changelogs/v4.7.4.rst +++ b/user_guide_src/source/changelogs/v4.7.4.rst @@ -30,6 +30,7 @@ Deprecations Bugs Fixed ********** +- **API:** Fixed a bug in Transformers where the root request's ``fields`` and ``include`` query parameters leaked into nested transformers created inside ``include*()`` methods, causing incorrect field filtering, unexpected includes, or infinite recursion. - **Database:** Fixed a bug where ``updateBatch()`` could be called after Query Builder ``where()`` conditions, even though it's not supported. In this situation, now the ``DatabaseException`` is thrown. - **HTTP:** Fixed a bug where the User Agent library reported Safari's WebKit version instead of the browser version from the ``Version`` token. diff --git a/user_guide_src/source/outgoing/api_transformers.rst b/user_guide_src/source/outgoing/api_transformers.rst index 5e2ab90ca5cb..38ce1de1fc14 100644 --- a/user_guide_src/source/outgoing/api_transformers.rst +++ b/user_guide_src/source/outgoing/api_transformers.rst @@ -174,6 +174,14 @@ The response would include: ] } +.. note:: The ``fields`` and ``include`` query parameters describe the **root** resource only. + Transformers instantiated inside an ``include*()`` method (such as ``PostTransformer`` above) + are treated as nested resources: when created **without an explicit request** they do **not** + inherit the root request's ``fields``/``include`` state. This prevents the parent's query + parameters from leaking into related resources, which could otherwise cause incorrect field + filtering or unexpected/recursive includes. If you deliberately pass a request to a nested + transformer, that request's scope is honored. + Restricting Available Includes =============================== @@ -262,6 +270,13 @@ Class Reference Initializes the transformer and extracts the ``fields`` and ``include`` query parameters from the request. + .. note:: When no request is explicitly passed, the global request's ``fields`` and ``include`` + are read **only for the root transformer**. A transformer constructed during a nested + transformation (i.e. inside an ``include*()`` method) without an explicit request still uses + the global request object, but does not read ``fields``/``include`` from it, to avoid leaking + the root's scope. Passing an explicit request always applies that request's scope, regardless + of nesting. + .. php:method:: toArray(mixed $resource) :param mixed $resource: The resource being transformed (Entity, array, object, or null)