Skip to content

Commit ab8bfd5

Browse files
soyukasoyuka
authored
fix(symfony): isolate api_platform.property_info tags (#8206)
Fixes #8201
1 parent 8c95af6 commit ab8bfd5

6 files changed

Lines changed: 167 additions & 12 deletions

File tree

‎src/Symfony/Bundle/ApiPlatformBundle.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\JsonStreamerTransformerPass;
2525
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\MetadataAwareNameConverterPass;
2626
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\MutatorPass;
27+
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\PropertyInfoTagPass;
2728
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\SerializerMappingLoaderPass;
2829
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\TestClientPass;
2930
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\TestMercureHubPass;
@@ -60,6 +61,7 @@ public function build(ContainerBuilder $container): void
6061
$container->addCompilerPass(new AuthenticatorManagerPass());
6162
$container->addCompilerPass(new SerializerMappingLoaderPass());
6263
$container->addCompilerPass(new MutatorPass());
64+
$container->addCompilerPass(new PropertyInfoTagPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -100);
6365
// Must run after Symfony's TransformerPass so we can rely on the value_object_transformer tag being processed.
6466
$container->addCompilerPass(new JsonStreamerTransformerPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -10);
6567
}

‎src/Symfony/Bundle/DependencyInjection/Compiler/PropertyInfoTagPass.php‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
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\Symfony\Bundle\DependencyInjection\Compiler;
15+
16+
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
17+
use Symfony\Component\DependencyInjection\ContainerBuilder;
18+
19+
/**
20+
* Bridges Symfony's public `property_info.*` tags to API Platform's private
21+
* `api_platform.property_info.*` tags so that `api_platform.property_info`
22+
* inherits framework- and third-party-registered extractors (e.g. Doctrine's
23+
* `DoctrineExtractor`) without API Platform's own extractors leaking back into
24+
* Symfony's `property_info` service.
25+
*
26+
* @internal
27+
*
28+
* @see https://github.com/api-platform/core/issues/8201
29+
*/
30+
final class PropertyInfoTagPass implements CompilerPassInterface
31+
{
32+
private const TAG_SUFFIXES = [
33+
'list_extractor',
34+
'type_extractor',
35+
'description_extractor',
36+
'access_extractor',
37+
'initializable_extractor',
38+
];
39+
40+
public function process(ContainerBuilder $container): void
41+
{
42+
foreach (self::TAG_SUFFIXES as $suffix) {
43+
$publicTag = 'property_info.'.$suffix;
44+
$privateTag = 'api_platform.property_info.'.$suffix;
45+
46+
foreach ($container->findTaggedServiceIds($publicTag) as $serviceId => $tags) {
47+
$definition = $container->getDefinition($serviceId);
48+
if ($definition->hasTag($privateTag)) {
49+
continue;
50+
}
51+
foreach ($tags as $attributes) {
52+
$definition->addTag($privateTag, $attributes);
53+
}
54+
}
55+
}
56+
}
57+
}

‎src/Symfony/Bundle/Resources/config/api.php‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,29 +77,29 @@
7777
$services->alias('api_platform.property_accessor', 'property_accessor');
7878

7979
$services->set('api_platform.property_info.reflection_extractor', ReflectionExtractor::class)
80-
->tag('property_info.list_extractor', ['priority' => -1000])
81-
->tag('property_info.type_extractor', ['priority' => -1002])
82-
->tag('property_info.access_extractor', ['priority' => -1000])
83-
->tag('property_info.initializable_extractor', ['priority' => -1000]);
80+
->tag('api_platform.property_info.list_extractor', ['priority' => -1000])
81+
->tag('api_platform.property_info.type_extractor', ['priority' => -1002])
82+
->tag('api_platform.property_info.access_extractor', ['priority' => -1000])
83+
->tag('api_platform.property_info.initializable_extractor', ['priority' => -1000]);
8484

8585
if (class_exists(DocBlockFactory::class)) {
8686
$services->set('api_platform.property_info.php_doc_extractor', PhpDocExtractor::class)
87-
->tag('property_info.description_extractor', ['priority' => -1000])
88-
->tag('property_info.type_extractor', ['priority' => -1001]);
87+
->tag('api_platform.property_info.description_extractor', ['priority' => -1000])
88+
->tag('api_platform.property_info.type_extractor', ['priority' => -1001]);
8989
}
9090

9191
if (class_exists(PhpDocParser::class) && class_exists(ContextFactory::class)) {
9292
$services->set('api_platform.property_info.phpstan_extractor', PhpStanExtractor::class)
93-
->tag('property_info.type_extractor', ['priority' => -1000]);
93+
->tag('api_platform.property_info.type_extractor', ['priority' => -1000]);
9494
}
9595

9696
$services->set('api_platform.property_info', PropertyInfoExtractor::class)
9797
->args([
98-
tagged_iterator('property_info.list_extractor'),
99-
tagged_iterator('property_info.type_extractor'),
100-
tagged_iterator('property_info.description_extractor'),
101-
tagged_iterator('property_info.access_extractor'),
102-
tagged_iterator('property_info.initializable_extractor'),
98+
tagged_iterator('api_platform.property_info.list_extractor'),
99+
tagged_iterator('api_platform.property_info.type_extractor'),
100+
tagged_iterator('api_platform.property_info.description_extractor'),
101+
tagged_iterator('api_platform.property_info.access_extractor'),
102+
tagged_iterator('api_platform.property_info.initializable_extractor'),
103103
]);
104104

105105
$services->set('api_platform.property_info.cache', PropertyInfoCacheExtractor::class)

‎src/Symfony/Tests/Bundle/DependencyInjection/ApiPlatformExtensionTest.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,4 +412,36 @@ public function testPaginationMaximumItemsPerPageWhenDefaultsKeyIsMissing(): voi
412412
$this->assertTrue($this->container->hasParameter('api_platform.collection.pagination.maximum_items_per_page'));
413413
$this->assertSame(30, $this->container->getParameter('api_platform.collection.pagination.maximum_items_per_page'));
414414
}
415+
416+
/**
417+
* @see https://github.com/api-platform/core/issues/8201
418+
*/
419+
public function testPropertyInfoExtractorsDoNotLeakIntoFrameworkPropertyInfo(): void
420+
{
421+
$config = self::DEFAULT_CONFIG;
422+
(new ApiPlatformExtension())->load($config, $this->container);
423+
424+
$services = ['api_platform.property_info.reflection_extractor'];
425+
if (class_exists(\phpDocumentor\Reflection\DocBlockFactory::class)) {
426+
$services[] = 'api_platform.property_info.php_doc_extractor';
427+
}
428+
if (class_exists(\PHPStan\PhpDocParser\Parser\PhpDocParser::class) && class_exists(\phpDocumentor\Reflection\Types\ContextFactory::class)) {
429+
$services[] = 'api_platform.property_info.phpstan_extractor';
430+
}
431+
432+
foreach ($services as $service) {
433+
$this->assertContainerHasService($service);
434+
$tags = $this->container->getDefinition($service)->getTags();
435+
foreach ($tags as $name => $_) {
436+
$this->assertStringStartsNotWith('property_info.', $name, \sprintf('Service "%s" must not use the global "property_info.*" tag namespace (leaks into Symfony\'s property_info and breaks the validator chain — issue #8201). Found tag "%s".', $service, $name));
437+
}
438+
}
439+
440+
$apiPlatformPropertyInfo = $this->container->getDefinition('api_platform.property_info');
441+
foreach ($apiPlatformPropertyInfo->getArguments() as $arg) {
442+
if ($arg instanceof \Symfony\Component\DependencyInjection\Argument\TaggedIteratorArgument) {
443+
$this->assertStringStartsWith('api_platform.property_info.', $arg->getTag(), \sprintf('api_platform.property_info must consume only "api_platform.property_info.*" private tags; found "%s".', $arg->getTag()));
444+
}
445+
}
446+
}
415447
}

‎tests/Symfony/Bundle/ApiPlatformBundleTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\JsonStreamerTransformerPass;
2626
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\MetadataAwareNameConverterPass;
2727
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\MutatorPass;
28+
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\PropertyInfoTagPass;
2829
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\SerializerMappingLoaderPass;
2930
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\TestClientPass;
3031
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\TestMercureHubPass;
@@ -59,6 +60,7 @@ public function testBuild(): void
5960
$this->assertContains(AuthenticatorManagerPass::class, $passClasses);
6061
$this->assertContains(SerializerMappingLoaderPass::class, $passClasses);
6162
$this->assertContains(MutatorPass::class, $passClasses);
63+
$this->assertContains(PropertyInfoTagPass::class, $passClasses);
6264
$this->assertContains(JsonStreamerTransformerPass::class, $passClasses);
6365
}
6466
}

‎tests/Symfony/Bundle/DependencyInjection/Compiler/PropertyInfoTagPassTest.php‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
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\Symfony\Bundle\DependencyInjection\Compiler;
15+
16+
use ApiPlatform\Symfony\Bundle\DependencyInjection\Compiler\PropertyInfoTagPass;
17+
use PHPUnit\Framework\TestCase;
18+
use Symfony\Component\DependencyInjection\ContainerBuilder;
19+
use Symfony\Component\DependencyInjection\Definition;
20+
21+
/**
22+
* @see https://github.com/api-platform/core/issues/8201
23+
*/
24+
final class PropertyInfoTagPassTest extends TestCase
25+
{
26+
public function testBridgesPublicTagsToPrivateNamespace(): void
27+
{
28+
$container = new ContainerBuilder();
29+
$container->setDefinition('framework.reflection_extractor', (new Definition(\stdClass::class))
30+
->addTag('property_info.type_extractor', ['priority' => -100])
31+
->addTag('property_info.list_extractor'));
32+
33+
(new PropertyInfoTagPass())->process($container);
34+
35+
$definition = $container->getDefinition('framework.reflection_extractor');
36+
$this->assertSame([['priority' => -100]], $definition->getTag('api_platform.property_info.type_extractor'));
37+
$this->assertSame([[]], $definition->getTag('api_platform.property_info.list_extractor'));
38+
}
39+
40+
public function testSkipsServicesAlreadyCarryingThePrivateTag(): void
41+
{
42+
$container = new ContainerBuilder();
43+
$container->setDefinition('api_platform.property_info.reflection_extractor', (new Definition(\stdClass::class))
44+
->addTag('property_info.type_extractor', ['priority' => -1002])
45+
->addTag('api_platform.property_info.type_extractor', ['priority' => -1002]));
46+
47+
(new PropertyInfoTagPass())->process($container);
48+
49+
$tags = $container->getDefinition('api_platform.property_info.reflection_extractor')->getTag('api_platform.property_info.type_extractor');
50+
$this->assertCount(1, $tags, 'Pass must not re-tag services that already carry the private tag.');
51+
}
52+
53+
public function testDoesNotTagServicesWithoutPublicTags(): void
54+
{
55+
$container = new ContainerBuilder();
56+
$container->setDefinition('unrelated_service', new Definition(\stdClass::class));
57+
58+
(new PropertyInfoTagPass())->process($container);
59+
60+
$this->assertSame([], $container->getDefinition('unrelated_service')->getTags());
61+
}
62+
}

0 commit comments

Comments
 (0)