From 8d7fa32d15fb07de6f9d05c0718c34fbe12b44fd Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Tue, 7 Jan 2020 11:39:48 +0100 Subject: [PATCH 1/5] fix processing chain adapter based cache pool --- .../DependencyInjection/CachePoolPass.php | 7 +++- .../DependencyInjection/CachePoolPassTest.php | 40 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Cache/DependencyInjection/CachePoolPass.php b/src/Symfony/Component/Cache/DependencyInjection/CachePoolPass.php index e082df1c4559c..f52d0271e4117 100644 --- a/src/Symfony/Component/Cache/DependencyInjection/CachePoolPass.php +++ b/src/Symfony/Component/Cache/DependencyInjection/CachePoolPass.php @@ -103,7 +103,12 @@ public function process(ContainerBuilder $container) if (ChainAdapter::class === $class) { $adapters = []; foreach ($adapter->getArgument(0) as $provider => $adapter) { - $chainedPool = $adapter = new ChildDefinition($adapter); + if ($adapter instanceof ChildDefinition) { + $chainedPool = $adapter; + } else { + $chainedPool = $adapter = new ChildDefinition($adapter); + } + $chainedTags = [\is_int($provider) ? [] : ['provider' => $provider]]; $chainedClass = ''; diff --git a/src/Symfony/Component/Cache/Tests/DependencyInjection/CachePoolPassTest.php b/src/Symfony/Component/Cache/Tests/DependencyInjection/CachePoolPassTest.php index e763dabe48cfa..20701adcb4507 100644 --- a/src/Symfony/Component/Cache/Tests/DependencyInjection/CachePoolPassTest.php +++ b/src/Symfony/Component/Cache/Tests/DependencyInjection/CachePoolPassTest.php @@ -12,7 +12,9 @@ namespace Symfony\Component\Cache\Tests\DependencyInjection; use PHPUnit\Framework\TestCase; +use Symfony\Component\Cache\Adapter\ApcuAdapter; use Symfony\Component\Cache\Adapter\ArrayAdapter; +use Symfony\Component\Cache\Adapter\ChainAdapter; use Symfony\Component\Cache\Adapter\RedisAdapter; use Symfony\Component\Cache\DependencyInjection\CachePoolPass; use Symfony\Component\DependencyInjection\ChildDefinition; @@ -174,4 +176,42 @@ public function testThrowsExceptionWhenCachePoolTagHasUnknownAttributes() $this->cachePoolPass->process($container); } + + public function testChainAdapterPool() + { + $container = new ContainerBuilder(); + $container->setParameter('kernel.container_class', 'app'); + $container->setParameter('kernel.project_dir', 'foo'); + + $container->register('cache.adapter.array', ArrayAdapter::class) + ->addTag('cache.pool'); + $container->register('cache.adapter.apcu', ApcuAdapter::class) + ->setArguments([null, 0, null]) + ->addTag('cache.pool'); + $container->register('cache.chain', ChainAdapter::class) + ->addArgument(['cache.adapter.array', 'cache.adapter.apcu']) + ->addTag('cache.pool'); + $container->setDefinition('cache.app', new ChildDefinition('cache.chain')) + ->addTag('cache.pool'); + $container->setDefinition('doctrine.result_cache_pool', new ChildDefinition('cache.app')) + ->addTag('cache.pool'); + + $this->cachePoolPass->process($container); + + $appCachePool = $container->getDefinition('cache.app'); + $this->assertInstanceOf(ChildDefinition::class, $appCachePool); + $this->assertSame('cache.chain', $appCachePool->getParent()); + + $chainCachePool = $container->getDefinition('cache.chain'); + $this->assertNotInstanceOf(ChildDefinition::class, $chainCachePool); + $this->assertCount(2, $chainCachePool->getArgument(0)); + $this->assertInstanceOf(ChildDefinition::class, $chainCachePool->getArgument(0)[0]); + $this->assertSame('cache.adapter.array', $chainCachePool->getArgument(0)[0]->getParent()); + $this->assertInstanceOf(ChildDefinition::class, $chainCachePool->getArgument(0)[1]); + $this->assertSame('cache.adapter.apcu', $chainCachePool->getArgument(0)[1]->getParent()); + + $doctrineCachePool = $container->getDefinition('doctrine.result_cache_pool'); + $this->assertInstanceOf(ChildDefinition::class, $doctrineCachePool); + $this->assertSame('cache.app', $doctrineCachePool->getParent()); + } } From e48829e9b66dd80cd80d625cf9a9f91710133cce Mon Sep 17 00:00:00 2001 From: Douglas Greenshields Date: Mon, 6 Jan 2020 11:29:29 +0000 Subject: [PATCH 2/5] [DependencyInjection] Handle ServiceClosureArgument for callable in container linting --- .../Compiler/CheckTypeDeclarationsPass.php | 5 ++++ .../CheckTypeDeclarationsPassTest.php | 25 +++++++++++++++++++ .../BarMethodCall.php | 4 +++ 3 files changed, 34 insertions(+) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php index 4bbcedbf124a2..7c414258b79f2 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckTypeDeclarationsPass.php @@ -12,6 +12,7 @@ namespace Symfony\Component\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Argument\IteratorArgument; +use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException; @@ -219,6 +220,10 @@ private function checkType(Definition $checkedDefinition, $value, \ReflectionPar return; } + if (\in_array($type, ['callable', 'Closure'], true) && $value instanceof ServiceClosureArgument) { + return; + } + if ('iterable' === $type && (\is_array($value) || $value instanceof \Traversable || $value instanceof IteratorArgument)) { return; } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php index 22a29fa4d6dc0..350f85296a09c 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckTypeDeclarationsPassTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Argument\IteratorArgument; +use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument; use Symfony\Component\DependencyInjection\Compiler\CheckTypeDeclarationsPass; use Symfony\Component\DependencyInjection\Compiler\ResolveParameterPlaceHoldersPass; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -697,4 +698,28 @@ public function testProcessHandleClosureForCallable() $this->addToAssertionCount(1); } + + public function testProcessSuccessWhenPassingServiceClosureArgumentToCallable() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setCallable', [new ServiceClosureArgument(new Reference('foo'))]); + + (new CheckTypeDeclarationsPass(true))->process($container); + + $this->addToAssertionCount(1); + } + + public function testProcessSuccessWhenPassingServiceClosureArgumentToClosure() + { + $container = new ContainerBuilder(); + + $container->register('bar', BarMethodCall::class) + ->addMethodCall('setClosure', [new ServiceClosureArgument(new Reference('foo'))]); + + (new CheckTypeDeclarationsPass(true))->process($container); + + $this->addToAssertionCount(1); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/BarMethodCall.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/BarMethodCall.php index b7056016094a1..69f1a693a4c5b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/BarMethodCall.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/CheckTypeDeclarationsPass/BarMethodCall.php @@ -40,4 +40,8 @@ public function setIterable(iterable $iterable) public function setCallable(callable $callable): void { } + + public function setClosure(\Closure $closure): void + { + } } From 96e70a40801ba1292cf340683e1a46dc797367a3 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 7 Jan 2020 13:55:38 +0100 Subject: [PATCH 3/5] [HttpClient] fix exception in case of PSR17 discovery failure --- src/Symfony/Component/HttpClient/HttplugClient.php | 11 ++++++++--- src/Symfony/Component/HttpClient/Psr18Client.php | 11 ++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/HttpClient/HttplugClient.php b/src/Symfony/Component/HttpClient/HttplugClient.php index e756ea5d3f434..08023b9c42718 100644 --- a/src/Symfony/Component/HttpClient/HttplugClient.php +++ b/src/Symfony/Component/HttpClient/HttplugClient.php @@ -16,6 +16,7 @@ use Http\Client\Exception\RequestException; use Http\Client\HttpAsyncClient; use Http\Client\HttpClient as HttplugInterface; +use Http\Discovery\Exception\NotFoundException; use Http\Discovery\Psr17FactoryDiscovery; use Http\Message\RequestFactory; use Http\Message\StreamFactory; @@ -75,9 +76,13 @@ public function __construct(HttpClientInterface $client = null, ResponseFactoryI throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\HttplugClient" as no PSR-17 factories have been provided. Try running "composer require nyholm/psr7".'); } - $psr17Factory = class_exists(Psr17Factory::class, false) ? new Psr17Factory() : null; - $this->responseFactory = $this->responseFactory ?? $psr17Factory ?? Psr17FactoryDiscovery::findResponseFactory(); - $this->streamFactory = $this->streamFactory ?? $psr17Factory ?? Psr17FactoryDiscovery::findStreamFactory(); + try { + $psr17Factory = class_exists(Psr17Factory::class, false) ? new Psr17Factory() : null; + $this->responseFactory = $this->responseFactory ?? $psr17Factory ?? Psr17FactoryDiscovery::findResponseFactory(); + $this->streamFactory = $this->streamFactory ?? $psr17Factory ?? Psr17FactoryDiscovery::findStreamFactory(); + } catch (NotFoundException $e) { + throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\HttplugClient" as no PSR-17 factories have been found. Try running "composer require nyholm/psr7".', 0, $e); + } } $this->waitLoop = new HttplugWaitLoop($this->client, $this->promisePool, $this->responseFactory, $this->streamFactory); diff --git a/src/Symfony/Component/HttpClient/Psr18Client.php b/src/Symfony/Component/HttpClient/Psr18Client.php index ba59dc11f7ecc..67c2fdb8f07bc 100644 --- a/src/Symfony/Component/HttpClient/Psr18Client.php +++ b/src/Symfony/Component/HttpClient/Psr18Client.php @@ -11,6 +11,7 @@ namespace Symfony\Component\HttpClient; +use Http\Discovery\Exception\NotFoundException; use Http\Discovery\Psr17FactoryDiscovery; use Nyholm\Psr7\Factory\Psr17Factory; use Nyholm\Psr7\Request; @@ -68,9 +69,13 @@ public function __construct(HttpClientInterface $client = null, ResponseFactoryI throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\Psr18Client" as no PSR-17 factories have been provided. Try running "composer require nyholm/psr7".'); } - $psr17Factory = class_exists(Psr17Factory::class, false) ? new Psr17Factory() : null; - $this->responseFactory = $this->responseFactory ?? $psr17Factory ?? Psr17FactoryDiscovery::findResponseFactory(); - $this->streamFactory = $this->streamFactory ?? $psr17Factory ?? Psr17FactoryDiscovery::findStreamFactory(); + try { + $psr17Factory = class_exists(Psr17Factory::class, false) ? new Psr17Factory() : null; + $this->responseFactory = $this->responseFactory ?? $psr17Factory ?? Psr17FactoryDiscovery::findResponseFactory(); + $this->streamFactory = $this->streamFactory ?? $psr17Factory ?? Psr17FactoryDiscovery::findStreamFactory(); + } catch (NotFoundException $e) { + throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\HttplugClient" as no PSR-17 factories have been found. Try running "composer require nyholm/psr7".', 0, $e); + } } /** From d38cdc9dce8ba0c1ef263e2fd1948db186fe0324 Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Tue, 7 Jan 2020 17:40:07 +0100 Subject: [PATCH 4/5] [FrameworkBundle][ContainerLintCommand] Only skip .errored. services --- .../FrameworkBundle/Command/ContainerLintCommand.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php index 5e6277567eff3..290f1da5e3af7 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php +++ b/src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php @@ -102,11 +102,12 @@ private function getContainerBuilder(): ContainerBuilder $refl->setAccessible(true); $refl->setValue($parameterBag, true); - $passConfig = $container->getCompilerPassConfig(); - $passConfig->setRemovingPasses([]); - $passConfig->setAfterRemovingPasses([]); - - $skippedIds = $kernelContainer->getRemovedIds(); + $skippedIds = []; + foreach ($container->getServiceIds() as $serviceId) { + if (0 === strpos($serviceId, '.errored.')) { + $skippedIds[$serviceId] = true; + } + } } $container->setParameter('container.build_hash', 'lint_container'); From a16a574ca8465a1b472ec7264ff74b53113743bb Mon Sep 17 00:00:00 2001 From: Toni Rudolf Date: Sun, 29 Dec 2019 10:43:19 +0100 Subject: [PATCH 5/5] [Messenger] Added check if json_encode succeeded --- .../Tests/Transport/RedisExt/ConnectionTest.php | 14 ++++++++++++++ .../Messenger/Transport/RedisExt/Connection.php | 8 ++++++++ 2 files changed, 22 insertions(+) diff --git a/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php b/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php index 57eafa2c1db77..6dd7a0287b5e5 100644 --- a/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php +++ b/src/Symfony/Component/Messenger/Tests/Transport/RedisExt/ConnectionTest.php @@ -186,6 +186,20 @@ public function testGetNonBlocking() $redis->del('messenger-getnonblocking'); } + public function testJsonError() + { + $redis = new \Redis(); + + $connection = Connection::fromDsn('redis://localhost/json-error', [], $redis); + + try { + $connection->add("\xB1\x31", []); + } catch (TransportException $e) { + } + + $this->assertSame('Malformed UTF-8 characters, possibly incorrectly encoded', $e->getMessage()); + } + public function testMaxEntries() { $redis = $this->getMockBuilder(\Redis::class)->disableOriginalConstructor()->getMock(); diff --git a/src/Symfony/Component/Messenger/Transport/RedisExt/Connection.php b/src/Symfony/Component/Messenger/Transport/RedisExt/Connection.php index 33d6057f67004..e1980221625e3 100644 --- a/src/Symfony/Component/Messenger/Transport/RedisExt/Connection.php +++ b/src/Symfony/Component/Messenger/Transport/RedisExt/Connection.php @@ -248,6 +248,10 @@ public function add(string $body, array $headers, int $delayInMs = 0): void 'uniqid' => uniqid('', true), ]); + if (false === $message) { + throw new TransportException(json_last_error_msg()); + } + $score = (int) ($this->getCurrentTimeInMilliseconds() + $delayInMs); $added = $this->connection->zadd($this->queue, ['NX'], $score, $message); } else { @@ -256,6 +260,10 @@ public function add(string $body, array $headers, int $delayInMs = 0): void 'headers' => $headers, ]); + if (false === $message) { + throw new TransportException(json_last_error_msg()); + } + if ($this->maxEntries) { $added = $this->connection->xadd($this->stream, '*', ['message' => $message], $this->maxEntries, true); } else {