Skip to content

Commit 167b544

Browse files
michalsnmichalsn
authored
fix: nested transformer request scope leakage (#10281)
1 parent ac18654 commit 167b544

6 files changed

Lines changed: 306 additions & 14 deletions

File tree

‎system/API/BaseTransformer.php‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@
5454
*/
5555
abstract class BaseTransformer implements TransformerInterface
5656
{
57+
/**
58+
* Nesting depth of the transformation currently in progress (0 = root).
59+
*/
60+
private static int $depth = 0;
61+
5762
/**
5863
* @var list<string>|null
5964
*/
@@ -69,17 +74,24 @@ abstract class BaseTransformer implements TransformerInterface
6974
public function __construct(
7075
private ?IncomingRequest $request = null,
7176
) {
77+
// An explicitly provided request is always honored - its scope is
78+
// intentional. Only the implicit global fallback is suppressed for
79+
// nested transformers, which is the actual leak vector.
80+
$explicitRequest = $request instanceof IncomingRequest;
81+
7282
$this->request = $request ?? request();
7383

74-
$fields = $this->request->getGet('fields');
75-
$this->fields = is_string($fields)
76-
? array_map(trim(...), explode(',', $fields))
77-
: $fields;
84+
if ($explicitRequest || self::$depth === 0) {
85+
$fields = $this->request->getGet('fields');
86+
$this->fields = is_string($fields)
87+
? array_map(trim(...), explode(',', $fields))
88+
: $fields;
7889

79-
$includes = $this->request->getGet('include');
80-
$this->includes = is_string($includes)
81-
? array_map(trim(...), explode(',', $includes))
82-
: $includes;
90+
$includes = $this->request->getGet('include');
91+
$this->includes = is_string($includes)
92+
? array_map(trim(...), explode(',', $includes))
93+
: $includes;
94+
}
8395
}
8496

8597
/**
@@ -207,13 +219,19 @@ private function insertIncludes(array $data): array
207219
}
208220
}
209221

210-
foreach ($this->includes as $include) {
211-
$method = 'include' . ucfirst($include);
212-
if (method_exists($this, $method)) {
213-
$data[$include] = $this->{$method}();
214-
} else {
215-
throw ApiException::forMissingInclude($include);
222+
self::$depth++;
223+
224+
try {
225+
foreach ($this->includes as $include) {
226+
$method = 'include' . ucfirst($include);
227+
if (method_exists($this, $method)) {
228+
$data[$include] = $this->{$method}();
229+
} else {
230+
throw ApiException::forMissingInclude($include);
231+
}
216232
}
233+
} finally {
234+
self::$depth--;
217235
}
218236

219237
return $data;

‎tests/_support/API/ChildTransformer.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of CodeIgniter 4 framework.
7+
*
8+
* (c) CodeIgniter Foundation <admin@codeigniter.com>
9+
*
10+
* For the full copyright and license information, please view
11+
* the LICENSE file that was distributed with this source code.
12+
*/
13+
14+
namespace Tests\Support\API;
15+
16+
use CodeIgniter\API\BaseTransformer;
17+
18+
/**
19+
* Nested transformer used to verify that the root request's scope
20+
* (fields/includes) does not leak into related resources.
21+
*/
22+
class ChildTransformer extends BaseTransformer
23+
{
24+
public function toArray(mixed $resource): array
25+
{
26+
return [
27+
'child_id' => $resource['id'] ?? null,
28+
'status' => 'transformed',
29+
];
30+
}
31+
}

‎tests/_support/API/ParentTransformer.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of CodeIgniter 4 framework.
7+
*
8+
* (c) CodeIgniter Foundation <admin@codeigniter.com>
9+
*
10+
* For the full copyright and license information, please view
11+
* the LICENSE file that was distributed with this source code.
12+
*/
13+
14+
namespace Tests\Support\API;
15+
16+
use CodeIgniter\API\BaseTransformer;
17+
18+
/**
19+
* Root transformer used to verify that nested transformers do not inherit
20+
* the root request's `fields`/`include` query state.
21+
*/
22+
class ParentTransformer extends BaseTransformer
23+
{
24+
public function toArray(mixed $resource): array
25+
{
26+
return [
27+
'parent_id' => $resource['id'] ?? null,
28+
];
29+
}
30+
31+
/**
32+
* Includes a single related child resource.
33+
*
34+
* @return array<string, mixed>
35+
*/
36+
protected function includeChildren(): array
37+
{
38+
return (new ChildTransformer())->transform(['id' => 99]);
39+
}
40+
41+
/**
42+
* Includes a collection of related child resources.
43+
*
44+
* @return list<array<string, mixed>>
45+
*/
46+
protected function includeChildrenCollection(): array
47+
{
48+
return (new ChildTransformer())->transformMany([
49+
['id' => 77],
50+
['id' => 88],
51+
]);
52+
}
53+
54+
/**
55+
* Includes a single child while explicitly forwarding the request, opting
56+
* the child into the request-derived scope even though it is nested.
57+
*
58+
* @return array<string, mixed>
59+
*/
60+
protected function includeExplicitChild(): array
61+
{
62+
$childRequest = clone request();
63+
$childRequest->setGlobal('get', ['fields' => 'child_id']);
64+
65+
return (new ChildTransformer($childRequest))->transform(['id' => 99]);
66+
}
67+
}

‎tests/system/API/TransformerTest.php‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
use Config\Services;
2323
use PHPUnit\Framework\Attributes\Group;
2424
use stdClass;
25+
use Tests\Support\API\ChildTransformer;
26+
use Tests\Support\API\ParentTransformer;
2527

2628
/**
2729
* @internal
@@ -33,12 +35,14 @@ protected function setUp(): void
3335
{
3436
parent::setUp();
3537

38+
Services::resetSingle('request');
3639
Services::superglobals()->setGetArray([]);
3740
}
3841

3942
protected function tearDown(): void
4043
{
4144
Services::superglobals()->setGetArray([]);
45+
Services::resetSingle('request');
4246

4347
parent::tearDown();
4448
}
@@ -641,4 +645,160 @@ protected function includePosts(): array
641645
$this->assertArrayHasKey('posts', $result);
642646
$this->assertSame([['id' => 1, 'title' => 'Post 1']], $result['posts']);
643647
}
648+
649+
public function testNestedTransformerDoesNotInheritIncludeState(): void
650+
{
651+
// The child transformer has no includeChildren() method. If the root
652+
// request's `include=children` leaked into it, transforming the child
653+
// would raise an ApiException for the missing include method.
654+
$request = $this->createMockRequest('include=children');
655+
Services::injectMock('request', $request);
656+
657+
$transformer = new ParentTransformer($request);
658+
659+
$result = $transformer->transform(['id' => 1]);
660+
661+
$this->assertSame([
662+
'parent_id' => 1,
663+
'children' => ['child_id' => 99, 'status' => 'transformed'],
664+
], $result);
665+
}
666+
667+
public function testNestedTransformerDoesNotInheritFieldFilter(): void
668+
{
669+
// `fields=parent_id` applies to the root only. If it leaked into the
670+
// child, array_intersect_key would strip every child field, leaving [].
671+
$request = $this->createMockRequest('include=children&fields=parent_id');
672+
Services::injectMock('request', $request);
673+
674+
$transformer = new ParentTransformer($request);
675+
676+
$result = $transformer->transform(['id' => 1]);
677+
678+
$this->assertSame([
679+
'parent_id' => 1,
680+
'children' => ['child_id' => 99, 'status' => 'transformed'],
681+
], $result);
682+
}
683+
684+
public function testNestedCollectionTransformerDoesNotInheritState(): void
685+
{
686+
$request = $this->createMockRequest('include=childrenCollection&fields=parent_id');
687+
Services::injectMock('request', $request);
688+
689+
$transformer = new ParentTransformer($request);
690+
691+
$result = $transformer->transform(['id' => 1]);
692+
693+
$this->assertSame([
694+
'parent_id' => 1,
695+
'childrenCollection' => [
696+
['child_id' => 77, 'status' => 'transformed'],
697+
['child_id' => 88, 'status' => 'transformed'],
698+
],
699+
], $result);
700+
}
701+
702+
public function testBareNestedInstantiationDoesNotInheritState(): void
703+
{
704+
// Reproduces the exact leak vector: a child created with a bare
705+
// `new ChildTransformer()` (no request passed) inside an include
706+
// method must not pick up the root request's scope from request().
707+
$request = $this->createMockRequest('include=children&fields=parent_id');
708+
Services::injectMock('request', $request);
709+
710+
$root = new ParentTransformer();
711+
712+
$result = $root->transform(['id' => 5]);
713+
714+
$this->assertSame([
715+
'parent_id' => 5,
716+
'children' => ['child_id' => 99, 'status' => 'transformed'],
717+
], $result);
718+
}
719+
720+
public function testNestedTransformerHonorsExplicitRequest(): void
721+
{
722+
// A child created with an explicitly passed request must honor that
723+
// request's scope even while nested - the isolation only suppresses
724+
// the implicit global fallback, not deliberate developer intent.
725+
$request = $this->createMockRequest('include=explicitChild&fields=child_id,parent_id');
726+
Services::injectMock('request', $request);
727+
728+
$transformer = new ParentTransformer($request);
729+
730+
$result = $transformer->transform(['id' => 1]);
731+
732+
$this->assertSame([
733+
'parent_id' => 1,
734+
'explicitChild' => ['child_id' => 99],
735+
], $result);
736+
}
737+
738+
public function testRootScopeStillAppliesAfterNesting(): void
739+
{
740+
// Sanity check that the root transformer keeps applying its own scope
741+
// while nested children are isolated.
742+
$request = $this->createMockRequest('include=children&fields=parent_id');
743+
Services::injectMock('request', $request);
744+
745+
$transformer = new ParentTransformer($request);
746+
747+
$result = $transformer->transform(['id' => 1, 'secret' => 'hidden']);
748+
749+
// Root keeps only parent_id (plus the include key), the child is intact.
750+
$this->assertArrayHasKey('parent_id', $result);
751+
$this->assertArrayNotHasKey('secret', $result);
752+
$this->assertSame(['child_id' => 99, 'status' => 'transformed'], $result['children']);
753+
}
754+
755+
public function testDepthIsRestoredAfterIncludeThrows(): void
756+
{
757+
$request = $this->createMockRequest('include=nonexistent');
758+
Services::injectMock('request', $request);
759+
760+
$throwingRoot = new class ($request) extends BaseTransformer {
761+
public function toArray(mixed $resource): array
762+
{
763+
return $resource;
764+
}
765+
};
766+
767+
try {
768+
$throwingRoot->transform(['id' => 1, 'name' => 'Test']);
769+
$this->fail('Expected ApiException was not thrown.');
770+
} catch (ApiException) {
771+
// expected
772+
}
773+
774+
// The nesting depth must be balanced after the exception, so a fresh
775+
// root transformer still applies the request scope (depth back to 0).
776+
$fieldsRequest = $this->createMockRequest('fields=id');
777+
Services::injectMock('request', $fieldsRequest);
778+
779+
$nextRoot = new class ($fieldsRequest) extends BaseTransformer {
780+
public function toArray(mixed $resource): array
781+
{
782+
return $resource;
783+
}
784+
};
785+
786+
$result = $nextRoot->transform(['id' => 1, 'name' => 'Test']);
787+
788+
$this->assertSame(['id' => 1], $result);
789+
}
790+
791+
public function testBareNestedTransformerStillUsedByChildTransformerDirectly(): void
792+
{
793+
// When ChildTransformer is itself the root (no parent), it must apply
794+
// request scope as usual - the isolation only affects nesting.
795+
$request = $this->createMockRequest('fields=child_id');
796+
Services::injectMock('request', $request);
797+
798+
$transformer = new ChildTransformer($request);
799+
800+
$result = $transformer->transform(['id' => 42]);
801+
802+
$this->assertSame(['child_id' => 42], $result);
803+
}
644804
}

‎user_guide_src/source/changelogs/v4.7.4.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Deprecations
3030
Bugs Fixed
3131
**********
3232

33+
- **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.
3334
- **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.
3435
- **HTTP:** Fixed a bug where the User Agent library reported Safari's WebKit version instead of the browser version from the ``Version`` token.
3536

‎user_guide_src/source/outgoing/api_transformers.rst‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,14 @@ The response would include:
174174
]
175175
}
176176
177+
.. note:: The ``fields`` and ``include`` query parameters describe the **root** resource only.
178+
Transformers instantiated inside an ``include*()`` method (such as ``PostTransformer`` above)
179+
are treated as nested resources: when created **without an explicit request** they do **not**
180+
inherit the root request's ``fields``/``include`` state. This prevents the parent's query
181+
parameters from leaking into related resources, which could otherwise cause incorrect field
182+
filtering or unexpected/recursive includes. If you deliberately pass a request to a nested
183+
transformer, that request's scope is honored.
184+
177185
Restricting Available Includes
178186
===============================
179187

@@ -262,6 +270,13 @@ Class Reference
262270

263271
Initializes the transformer and extracts the ``fields`` and ``include`` query parameters from the request.
264272

273+
.. note:: When no request is explicitly passed, the global request's ``fields`` and ``include``
274+
are read **only for the root transformer**. A transformer constructed during a nested
275+
transformation (i.e. inside an ``include*()`` method) without an explicit request still uses
276+
the global request object, but does not read ``fields``/``include`` from it, to avoid leaking
277+
the root's scope. Passing an explicit request always applies that request's scope, regardless
278+
of nesting.
279+
265280
.. php:method:: toArray(mixed $resource)
266281
267282
:param mixed $resource: The resource being transformed (Entity, array, object, or null)

0 commit comments

Comments
 (0)