Skip to content

Commit 4271cf8

Browse files
author
Thibaut Cholley
committed
fix: resolve union in types
1 parent 2abda53 commit 4271cf8

2 files changed

Lines changed: 142 additions & 0 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\TypeConfusion;
15+
16+
use ApiPlatform\Metadata\Post;
17+
18+
#[Post(uriTemplate: '/type-confusion/union-collection-targets{._format}')]
19+
class UnionCollectionTarget
20+
{
21+
public ?string $name = null;
22+
23+
/**
24+
* PHPDoc-only union array: each element may be either Foo or Bar.
25+
* Mirrors the real-world "@var PharmacyRevenueFile[]|MerchFile[]" pattern
26+
* that regressed under the GHSA-9rjg-x2p2-h68h security fix.
27+
*
28+
* @var Foo[]|Bar[]
29+
*/
30+
public array $attachments = [];
31+
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Functional\Security;
15+
16+
use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
17+
use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\TypeConfusion\Bar;
18+
use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\TypeConfusion\Foo;
19+
use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\TypeConfusion\UnionCollectionTarget;
20+
use ApiPlatform\Tests\SetupClassResourcesTrait;
21+
22+
/**
23+
* Regression test for the side-effect of the GHSA-9rjg-x2p2-h68h security fix.
24+
*
25+
* The fix added an is_a guard in AbstractItemNormalizer::getResourceFromIri() to prevent
26+
* CWE-843 type confusion. However, for a property typed as a union of collections
27+
* (e.g. "@var Foo[]|Bar[]"), the denormalization loop iterates over each possible type
28+
* one at a time (first tries Foo[], then Bar[]). The is_a check rejects any item that
29+
* does not match the single type being currently tried, so a mixed [Foo, Bar] collection
30+
* always fails: Foo[] fails on the Bar item, Bar[] fails on the Foo item.
31+
*
32+
* Before the fix, both iterations returned successfully (no type guard), so mixed
33+
* collections were accepted and Symfony's validator would catch genuinely wrong types
34+
* at the individual item path (e.g. "attachments[2]: must be Foo|Bar").
35+
*
36+
* After the fix, the per-item NotNormalizableValueException bubbles through the union
37+
* fallback logic and is re-wrapped at the collection level, producing a less informative
38+
* error ("attachments: must be array<Foo>|array<Bar>") and — worse — rejecting
39+
* previously valid mixed-type collections entirely.
40+
*/
41+
final class TypeConfusionUnionCollectionTest extends ApiTestCase
42+
{
43+
use SetupClassResourcesTrait;
44+
45+
protected static ?bool $alwaysBootKernel = false;
46+
47+
/**
48+
* @return class-string[]
49+
*/
50+
public static function getResources(): array
51+
{
52+
return [Foo::class, Bar::class, UnionCollectionTarget::class];
53+
}
54+
55+
public function testHomogeneousFooCollectionIsAccepted(): void
56+
{
57+
$response = self::createClient()->request('POST', '/type-confusion/union-collection-targets', [
58+
'headers' => ['Content-Type' => 'application/ld+json', 'Accept' => 'application/ld+json'],
59+
'json' => [
60+
'name' => 'all-foos',
61+
'attachments' => ['/type-confusion/foos/1', '/type-confusion/foos/2'],
62+
],
63+
]);
64+
65+
$this->assertResponseStatusCodeSame(201);
66+
}
67+
68+
public function testHomogeneousBarCollectionIsAccepted(): void
69+
{
70+
$response = self::createClient()->request('POST', '/type-confusion/union-collection-targets', [
71+
'headers' => ['Content-Type' => 'application/ld+json', 'Accept' => 'application/ld+json'],
72+
'json' => [
73+
'name' => 'all-bars',
74+
'attachments' => ['/type-confusion/bars/1', '/type-confusion/bars/2'],
75+
],
76+
]);
77+
78+
$this->assertResponseStatusCodeSame(201);
79+
}
80+
81+
/**
82+
* A @var Foo[]|Bar[] collection must accept a mix of Foo and Bar IRIs.
83+
*
84+
* This test demonstrates the regression introduced by GHSA-9rjg-x2p2-h68h:
85+
* the is_a guard in getResourceFromIri() causes AbstractItemNormalizer to reject
86+
* any mixed [Foo, Bar] payload because neither single-type iteration can satisfy
87+
* all items, even though each individual item is a valid union member.
88+
*/
89+
public function testMixedUnionCollectionIsAccepted(): void
90+
{
91+
$response = self::createClient()->request('POST', '/type-confusion/union-collection-targets', [
92+
'headers' => ['Content-Type' => 'application/ld+json', 'Accept' => 'application/ld+json'],
93+
'json' => [
94+
'name' => 'mixed',
95+
'attachments' => [
96+
'/type-confusion/foos/1',
97+
'/type-confusion/foos/2',
98+
'/type-confusion/bars/1',
99+
],
100+
],
101+
]);
102+
103+
$this->assertResponseStatusCodeSame(
104+
201,
105+
\sprintf(
106+
'Regression from GHSA-9rjg-x2p2-h68h: mixed Foo[]|Bar[] collection was rejected. Body: %s',
107+
$response->getContent(false)
108+
)
109+
);
110+
}
111+
}

0 commit comments

Comments
 (0)