Skip to content

Commit 9c53b3d

Browse files
nicolas-grekasnicolas-grekas
committed
[DI] Restrict autowired registration to "same-vendor" namespaces
1 parent ab93fea commit 9c53b3d

File tree

13 files changed

+130
-107
lines changed

13 files changed

+130
-107
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
use Symfony\Component\DependencyInjection\Definition;
1717
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
1818
use Symfony\Component\DependencyInjection\LazyProxy\ProxyHelper;
19-
use Symfony\Component\DependencyInjection\Reference;
2019
use Symfony\Component\DependencyInjection\TypedReference;
2120

2221
/**
@@ -76,9 +75,9 @@ public static function createResourceForClass(\ReflectionClass $reflectionClass)
7675
*/
7776
protected function processValue($value, $isRoot = false)
7877
{
79-
if ($value instanceof TypedReference && !$this->container->has((string) $value)) {
80-
if ($ref = $this->getAutowiredReference($value->getType(), $value->canBeAutoregistered())) {
81-
$value = new TypedReference((string) $ref, $value->getType(), $value->getInvalidBehavior(), $value->canBeAutoregistered());
78+
if ($value instanceof TypedReference) {
79+
if ($ref = $this->getAutowiredReference($value)) {
80+
$value = $ref;
8281
} else {
8382
$this->container->log($this, $this->createTypeNotFoundMessage($value->getType(), 'typed reference'));
8483
}
@@ -242,7 +241,7 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
242241
continue;
243242
}
244243

245-
if (!$value = $this->getAutowiredReference($type, !$parameter->isOptional())) {
244+
if (!$value = $this->getAutowiredReference(new TypedReference($type, $type, !$parameter->isOptional() ? $class : ''))) {
246245
$failureMessage = $this->createTypeNotFoundMessage($type, sprintf('argument "$%s" of method "%s()"', $parameter->name, $class !== $this->currentId ? $class.'::'.$method : $method));
247246

248247
if ($parameter->isDefaultValueAvailable()) {
@@ -276,29 +275,33 @@ private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, a
276275
}
277276

278277
/**
279-
* @return Reference|null A reference to the service matching the given type, if any
278+
* @return TypedReference|null A reference to the service matching the given type, if any
280279
*/
281-
private function getAutowiredReference($type, $autoRegister)
280+
private function getAutowiredReference(TypedReference $reference)
282281
{
283-
if ($this->container->has($type) && !$this->container->findDefinition($type)->isAbstract()) {
284-
return new Reference($type);
285-
}
282+
$type = $reference->getType();
286283

287-
if (isset($this->autowired[$type])) {
288-
return $this->autowired[$type] ? new Reference($this->autowired[$type]) : null;
284+
if ($type !== (string) $reference || ($this->container->has($type) && !$this->container->findDefinition($type)->isAbstract())) {
285+
return $reference;
289286
}
290287

291288
if (null === $this->types) {
292289
$this->populateAvailableTypes();
293290
}
294291

295292
if (isset($this->definedTypes[$type])) {
296-
return new Reference($this->types[$type]);
293+
return new TypedReference($this->types[$type], $type);
297294
}
298295

299-
if ($autoRegister && !isset($this->types[$type]) && !isset($this->ambiguousServiceTypes[$type])) {
300-
return $this->createAutowiredDefinition($type);
296+
if (!$reference->canBeAutoregistered() || isset($this->types[$type]) || isset($this->ambiguousServiceTypes[$type])) {
297+
return;
301298
}
299+
300+
if (isset($this->autowired[$type])) {
301+
return $this->autowired[$type] ? new TypedReference($this->autowired[$type], $type) : null;
302+
}
303+
304+
return $this->createAutowiredDefinition($type);
302305
}
303306

304307
/**
@@ -384,7 +387,7 @@ private function set($type, $id)
384387
*
385388
* @param string $type
386389
*
387-
* @return Reference|null A reference to the registered definition
390+
* @return TypedReference|null A reference to the registered definition
388391
*/
389392
private function createAutowiredDefinition($type)
390393
{
@@ -412,7 +415,7 @@ private function createAutowiredDefinition($type)
412415

413416
$this->container->log($this, sprintf('Type "%s" has been auto-registered for service "%s".', $type, $this->currentId));
414417

415-
return new Reference($argumentId);
418+
return new TypedReference($argumentId, $type);
416419
}
417420

418421
private function createTypeNotFoundMessage($type, $label)

‎src/Symfony/Component/DependencyInjection/Compiler/RegisterServiceSubscribersPass.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,11 @@ protected function processValue($value, $isRoot = false)
7171
}
7272
$this->container->addObjectResource($class);
7373
$subscriberMap = array();
74+
$declaringClass = (new \ReflectionMethod($class, 'getSubscribedServices'))->class;
7475

7576
foreach ($class::getSubscribedServices() as $key => $type) {
7677
if (!is_string($type) || !preg_match('/^\??[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)*+$/', $type)) {
77-
throw new InvalidArgumentException(sprintf('%s::getSubscribedServices() must return valid PHP types for service "%s" key "%s", "%s" returned.', $class, $this->currentId, $key, is_string($type) ? $type : gettype($type)));
78+
throw new InvalidArgumentException(sprintf('"%s::getSubscribedServices()" must return valid PHP types for service "%s" key "%s", "%s" returned.', $class, $this->currentId, $key, is_string($type) ? $type : gettype($type)));
7879
}
7980
if ($optionalBehavior = '?' === $type[0]) {
8081
$type = substr($type, 1);
@@ -85,18 +86,18 @@ protected function processValue($value, $isRoot = false)
8586
}
8687
if (!isset($serviceMap[$key])) {
8788
if (!$autowire) {
88-
throw new InvalidArgumentException(sprintf('Service "%s" misses a "container.service_subscriber" tag with "key"/"id" attributes corresponding to entry "%s" as returned by %s::getSubscribedServices().', $this->currentId, $key, $class));
89+
throw new InvalidArgumentException(sprintf('Service "%s" misses a "container.service_subscriber" tag with "key"/"id" attributes corresponding to entry "%s" as returned by "%s::getSubscribedServices()".', $this->currentId, $key, $class));
8990
}
9091
$serviceMap[$key] = new Reference($type);
9192
}
9293

93-
$subscriberMap[$key] = new TypedReference((string) $serviceMap[$key], $type, $optionalBehavior ?: ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE);
94+
$subscriberMap[$key] = new TypedReference((string) $serviceMap[$key], $type, $declaringClass, $optionalBehavior ?: ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE);
9495
unset($serviceMap[$key]);
9596
}
9697

9798
if ($serviceMap = array_keys($serviceMap)) {
9899
$message = sprintf(1 < count($serviceMap) ? 'keys "%s" do' : 'key "%s" does', str_replace('%', '%%', implode('", "', $serviceMap)));
99-
throw new InvalidArgumentException(sprintf('Service %s not exist in the map returned by %s::getSubscribedServices() for service "%s".', $message, $class, $this->currentId));
100+
throw new InvalidArgumentException(sprintf('Service %s not exist in the map returned by "%s::getSubscribedServices()" for service "%s".', $message, $class, $this->currentId));
100101
}
101102

102103
$serviceLocator = $this->serviceLocator;

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,9 @@ public function testSomeSpecificArgumentsAreSet()
368368
$definition = $container->getDefinition('multiple');
369369
$this->assertEquals(
370370
array(
371-
new Reference(A::class),
371+
new TypedReference(A::class, A::class, MultipleArguments::class),
372372
new Reference('foo'),
373-
new Reference(Dunglas::class),
373+
new TypedReference(Dunglas::class, Dunglas::class, MultipleArguments::class),
374374
),
375375
$definition->getArguments()
376376
);
@@ -423,10 +423,10 @@ public function testOptionalScalarArgsDontMessUpOrder()
423423
$definition = $container->getDefinition('with_optional_scalar');
424424
$this->assertEquals(
425425
array(
426-
new Reference(A::class),
426+
new TypedReference(A::class, A::class, MultipleArgumentsOptionalScalar::class),
427427
// use the default value
428428
'default_val',
429-
new Reference(Lille::class),
429+
new TypedReference(Lille::class, Lille::class),
430430
),
431431
$definition->getArguments()
432432
);
@@ -447,8 +447,8 @@ public function testOptionalScalarArgsNotPassedIfLast()
447447
$definition = $container->getDefinition('with_optional_scalar_last');
448448
$this->assertEquals(
449449
array(
450-
new Reference(A::class),
451-
new Reference(Lille::class),
450+
new TypedReference(A::class, A::class, MultipleArgumentsOptionalScalarLast::class),
451+
new TypedReference(Lille::class, Lille::class, MultipleArgumentsOptionalScalarLast::class),
452452
),
453453
$definition->getArguments()
454454
);
@@ -486,7 +486,7 @@ public function testSetterInjection()
486486
);
487487
// test setFoo args
488488
$this->assertEquals(
489-
array(new Reference(Foo::class)),
489+
array(new TypedReference(Foo::class, Foo::class, SetterInjection::class)),
490490
$methodCalls[1][1]
491491
);
492492
}
@@ -515,7 +515,7 @@ public function testExplicitMethodInjection()
515515
array_column($methodCalls, 0)
516516
);
517517
$this->assertEquals(
518-
array(new Reference(A::class)),
518+
array(new TypedReference(A::class, A::class, SetterInjection::class)),
519519
$methodCalls[0][1]
520520
);
521521
}
@@ -526,7 +526,7 @@ public function testTypedReference()
526526

527527
$container
528528
->register('bar', Bar::class)
529-
->setProperty('a', array(new TypedReference(A::class, A::class)))
529+
->setProperty('a', array(new TypedReference(A::class, A::class, Bar::class)))
530530
;
531531

532532
$pass = new AutowirePass();
@@ -629,7 +629,7 @@ public function testEmptyStringIsKept()
629629
(new ResolveClassPass())->process($container);
630630
(new AutowirePass())->process($container);
631631

632-
$this->assertEquals(array(new Reference(A::class), '', new Reference(Lille::class)), $container->getDefinition('foo')->getArguments());
632+
$this->assertEquals(array(new TypedReference(A::class, A::class, MultipleArgumentsOptionalScalar::class), '', new TypedReference(Lille::class, Lille::class)), $container->getDefinition('foo')->getArguments());
633633
}
634634

635635
public function testWithFactory()
@@ -644,7 +644,7 @@ public function testWithFactory()
644644
(new ResolveClassPass())->process($container);
645645
(new AutowirePass())->process($container);
646646

647-
$this->assertEquals(array(new Reference(Foo::class)), $definition->getArguments());
647+
$this->assertEquals(array(new TypedReference(Foo::class, Foo::class, A::class)), $definition->getArguments());
648648
}
649649

650650
/**

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/RegisterServiceSubscribersPassTest.php‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
use Symfony\Component\DependencyInjection\ContainerInterface;
1919
use Symfony\Component\DependencyInjection\Reference;
2020
use Symfony\Component\DependencyInjection\ServiceLocator;
21+
use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition;
22+
use Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber;
2123
use Symfony\Component\DependencyInjection\TypedReference;
2224

2325
require_once __DIR__.'/../Fixtures/includes/classes.php';
@@ -32,7 +34,7 @@ public function testInvalidClass()
3234
{
3335
$container = new ContainerBuilder();
3436

35-
$container->register('foo', 'stdClass')
37+
$container->register('foo', CustomDefinition::class)
3638
->addTag('container.service_subscriber')
3739
;
3840

@@ -48,7 +50,7 @@ public function testInvalidAttributes()
4850
{
4951
$container = new ContainerBuilder();
5052

51-
$container->register('foo', 'TestServiceSubscriber')
53+
$container->register('foo', TestServiceSubscriber::class)
5254
->addTag('container.service_subscriber', array('bar' => '123'))
5355
;
5456

@@ -60,7 +62,7 @@ public function testNoAttributes()
6062
{
6163
$container = new ContainerBuilder();
6264

63-
$container->register('foo', 'TestServiceSubscriber')
65+
$container->register('foo', TestServiceSubscriber::class)
6466
->addArgument(new Reference('container'))
6567
->addTag('container.service_subscriber')
6668
;
@@ -75,10 +77,10 @@ public function testNoAttributes()
7577
$this->assertSame(ServiceLocator::class, $locator->getClass());
7678

7779
$expected = array(
78-
'TestServiceSubscriber' => new ServiceClosureArgument(new TypedReference('TestServiceSubscriber', 'TestServiceSubscriber')),
79-
'stdClass' => new ServiceClosureArgument(new TypedReference('stdClass', 'stdClass', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
80-
'bar' => new ServiceClosureArgument(new TypedReference('stdClass', 'stdClass')),
81-
'baz' => new ServiceClosureArgument(new TypedReference('stdClass', 'stdClass', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
80+
TestServiceSubscriber::class => new ServiceClosureArgument(new TypedReference(TestServiceSubscriber::class, TestServiceSubscriber::class, TestServiceSubscriber::class)),
81+
CustomDefinition::class => new ServiceClosureArgument(new TypedReference(CustomDefinition::class, CustomDefinition::class, TestServiceSubscriber::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
82+
'bar' => new ServiceClosureArgument(new TypedReference(CustomDefinition::class, CustomDefinition::class, TestServiceSubscriber::class)),
83+
'baz' => new ServiceClosureArgument(new TypedReference(CustomDefinition::class, CustomDefinition::class, TestServiceSubscriber::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
8284
);
8385

8486
$this->assertEquals($expected, $locator->getArgument(0));
@@ -88,7 +90,7 @@ public function testWithAttributes()
8890
{
8991
$container = new ContainerBuilder();
9092

91-
$container->register('foo', 'TestServiceSubscriber')
93+
$container->register('foo', TestServiceSubscriber::class)
9294
->setAutowired(true)
9395
->addArgument(new Reference('container'))
9496
->addTag('container.service_subscriber', array('key' => 'bar', 'id' => 'bar'))
@@ -105,31 +107,31 @@ public function testWithAttributes()
105107
$this->assertSame(ServiceLocator::class, $locator->getClass());
106108

107109
$expected = array(
108-
'TestServiceSubscriber' => new ServiceClosureArgument(new TypedReference('TestServiceSubscriber', 'TestServiceSubscriber')),
109-
'stdClass' => new ServiceClosureArgument(new TypedReference('stdClass', 'stdClass', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
110-
'bar' => new ServiceClosureArgument(new TypedReference('bar', 'stdClass')),
111-
'baz' => new ServiceClosureArgument(new TypedReference('stdClass', 'stdClass', ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
110+
TestServiceSubscriber::class => new ServiceClosureArgument(new TypedReference(TestServiceSubscriber::class, TestServiceSubscriber::class, TestServiceSubscriber::class)),
111+
CustomDefinition::class => new ServiceClosureArgument(new TypedReference(CustomDefinition::class, CustomDefinition::class, TestServiceSubscriber::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
112+
'bar' => new ServiceClosureArgument(new TypedReference('bar', CustomDefinition::class, TestServiceSubscriber::class)),
113+
'baz' => new ServiceClosureArgument(new TypedReference(CustomDefinition::class, CustomDefinition::class, TestServiceSubscriber::class, ContainerInterface::IGNORE_ON_INVALID_REFERENCE)),
112114
);
113115

114116
$this->assertEquals($expected, $locator->getArgument(0));
115117
}
116118

117119
/**
118120
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
119-
* @expectedExceptionMessage Service key "test" does not exist in the map returned by TestServiceSubscriber::getSubscribedServices() for service "foo_service".
121+
* @expectedExceptionMessage Service key "test" does not exist in the map returned by "Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber::getSubscribedServices()" for service "foo_service".
120122
*/
121123
public function testExtraServiceSubscriber()
122124
{
123125
$container = new ContainerBuilder();
124-
$container->register('foo_service', 'TestServiceSubscriber')
126+
$container->register('foo_service', TestServiceSubscriber::class)
125127
->setAutowired(true)
126128
->addArgument(new Reference('container'))
127129
->addTag('container.service_subscriber', array(
128130
'key' => 'test',
129-
'id' => 'TestServiceSubscriber',
131+
'id' => TestServiceSubscriber::class,
130132
))
131133
;
132-
$container->register('TestServiceSubscriber', 'TestServiceSubscriber');
134+
$container->register(TestServiceSubscriber::class, TestServiceSubscriber::class);
133135
$container->compile();
134136
}
135137
}

‎src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use Symfony\Component\DependencyInjection\Definition;
2626
use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
2727
use Symfony\Component\DependencyInjection\ServiceLocator;
28+
use Symfony\Component\DependencyInjection\Tests\Fixtures\TestServiceSubscriber;
2829
use Symfony\Component\DependencyInjection\Variable;
2930
use Symfony\Component\ExpressionLanguage\Expression;
3031

@@ -557,15 +558,15 @@ public function testServiceLocator()
557558
public function testServiceSubscriber()
558559
{
559560
$container = new ContainerBuilder();
560-
$container->register('foo_service', 'TestServiceSubscriber')
561+
$container->register('foo_service', TestServiceSubscriber::class)
561562
->setAutowired(true)
562563
->addArgument(new Reference('container'))
563564
->addTag('container.service_subscriber', array(
564565
'key' => 'bar',
565-
'id' => 'TestServiceSubscriber',
566+
'id' => TestServiceSubscriber::class,
566567
))
567568
;
568-
$container->register('TestServiceSubscriber', 'TestServiceSubscriber');
569+
$container->register(TestServiceSubscriber::class, TestServiceSubscriber::class);
569570
$container->compile();
570571

571572
$dumper = new PhpDumper($container);

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/TestServiceSubscriber.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
4+
5+
use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;
6+
7+
class TestServiceSubscriber implements ServiceSubscriberInterface
8+
{
9+
public function __construct($container)
10+
{
11+
}
12+
13+
public static function getSubscribedServices()
14+
{
15+
return array(
16+
__CLASS__,
17+
'?'.CustomDefinition::class,
18+
'bar' => CustomDefinition::class,
19+
'baz' => '?'.CustomDefinition::class,
20+
);
21+
}
22+
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/includes/classes.php‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -108,20 +108,3 @@ public function __construct($lazyValues)
108108
$this->lazyValues = $lazyValues;
109109
}
110110
}
111-
112-
class TestServiceSubscriber implements ServiceSubscriberInterface
113-
{
114-
public function __construct($container)
115-
{
116-
}
117-
118-
public static function getSubscribedServices()
119-
{
120-
return array(
121-
__CLASS__,
122-
'?stdClass',
123-
'bar' => 'stdClass',
124-
'baz' => '?stdClass',
125-
);
126-
}
127-
}

0 commit comments

Comments
 (0)