From 5cd1d7b4cc03796c0f709776aa7e8e4c25f786ed Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 2 Jul 2018 13:54:43 +0200 Subject: [PATCH 1/7] [Security] add "anonymous: lazy" mode to firewalls --- .../Bundle/SecurityBundle/CHANGELOG.md | 1 + .../DataCollector/SecurityDataCollector.php | 3 +- .../Security/Factory/AnonymousFactory.php | 5 ++ .../DependencyInjection/SecurityExtension.php | 7 +- .../Resources/config/security.xml | 10 +++ .../Security/LazyFirewallContext.php | 73 ++++++++++++++++++ .../Controller/LocalizedController.php | 2 +- .../SecurityRoutingIntegrationTest.php | 10 +++ .../app/StandardFormLogin/config.yml | 3 +- .../Bundle/SecurityBundle/composer.json | 2 +- .../Token/Storage/TokenStorage.php | 12 +++ .../Core/Exception/LazyResponseException.php | 34 +++++++++ .../Security/Http/Event/LazyResponseEvent.php | 76 +++++++++++++++++++ .../Http/Firewall/ExceptionListener.php | 7 ++ 14 files changed, 239 insertions(+), 6 deletions(-) create mode 100644 src/Symfony/Bundle/SecurityBundle/Security/LazyFirewallContext.php create mode 100644 src/Symfony/Component/Security/Core/Exception/LazyResponseException.php create mode 100644 src/Symfony/Component/Security/Http/Event/LazyResponseEvent.php diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index 272cff93b0..5d00820859 100644 --- a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md @@ -8,6 +8,7 @@ CHANGELOG 4.3.0 ----- + * Added `anonymous: lazy` mode to firewalls to make them (not) start the session as late as possible * Added new encoder types: `auto` (recommended), `native` and `sodium` * The normalization of the cookie names configured in the `logout.delete_cookies` option is deprecated and will be disabled in Symfony 5.0. This affects to cookies diff --git a/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php b/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php index 29fa33b3f3..86805abf61 100644 --- a/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php +++ b/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php @@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\DataCollector\DataCollector; use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface; +use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; @@ -127,7 +128,7 @@ class SecurityDataCollector extends DataCollector implements LateDataCollectorIn $logoutUrl = null; try { - if (null !== $this->logoutUrlGenerator) { + if (null !== $this->logoutUrlGenerator && !$token instanceof AnonymousToken) { $logoutUrl = $this->logoutUrlGenerator->getLogoutPath(); } } catch (\Exception $e) { diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AnonymousFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AnonymousFactory.php index dc8e4a9ba3..a889edea00 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AnonymousFactory.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AnonymousFactory.php @@ -55,7 +55,12 @@ class AnonymousFactory implements SecurityFactoryInterface public function addConfiguration(NodeDefinition $builder) { $builder + ->beforeNormalization() + ->ifTrue(function ($v) { return 'lazy' === $v; }) + ->then(function ($v) { return ['lazy' => true]; }) + ->end() ->children() + ->booleanNode('lazy')->defaultFalse()->end() ->scalarNode('secret')->defaultNull()->end() ->end() ; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index b828dfd67a..4eca9ecef4 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -243,7 +243,8 @@ class SecurityExtension extends Extension implements PrependExtensionInterface list($matcher, $listeners, $exceptionListener, $logoutListener) = $this->createFirewall($container, $name, $firewall, $authenticationProviders, $providerIds, $configId); $contextId = 'security.firewall.map.context.'.$name; - $context = $container->setDefinition($contextId, new ChildDefinition('security.firewall.context')); + $context = new ChildDefinition($firewall['stateless'] || empty($firewall['anonymous']['lazy']) ? 'security.firewall.context' : 'security.firewall.lazy_context'); + $context = $container->setDefinition($contextId, $context); $context ->replaceArgument(0, new IteratorArgument($listeners)) ->replaceArgument(1, $exceptionListener) @@ -409,7 +410,9 @@ class SecurityExtension extends Extension implements PrependExtensionInterface } // Access listener - $listeners[] = new Reference('security.access_listener'); + if ($firewall['stateless'] || empty($firewall['anonymous']['lazy'])) { + $listeners[] = new Reference('security.access_listener'); + } // Exception listener $exceptionListener = new Reference($this->createExceptionListener($container, $firewall, $id, $configuredEntryPoint ?: $defaultEntryPoint, $firewall['stateless'])); diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml index 86bf11869b..bcb35370a5 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml @@ -151,6 +151,16 @@ + + + + + + + + + + diff --git a/src/Symfony/Bundle/SecurityBundle/Security/LazyFirewallContext.php b/src/Symfony/Bundle/SecurityBundle/Security/LazyFirewallContext.php new file mode 100644 index 0000000000..ef9b1e217c --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Security/LazyFirewallContext.php @@ -0,0 +1,73 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\SecurityBundle\Security; + +use Symfony\Component\HttpKernel\Event\RequestEvent; +use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; +use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; +use Symfony\Component\Security\Core\Exception\LazyResponseException; +use Symfony\Component\Security\Http\AccessMapInterface; +use Symfony\Component\Security\Http\Event\LazyResponseEvent; +use Symfony\Component\Security\Http\Firewall\AccessListener; +use Symfony\Component\Security\Http\Firewall\ExceptionListener; +use Symfony\Component\Security\Http\Firewall\LogoutListener; + +/** + * Lazily calls authentication listeners when actually required by the access listener. + * + * @author Nicolas Grekas + */ +class LazyFirewallContext extends FirewallContext +{ + private $accessListener; + private $tokenStorage; + private $map; + + public function __construct(iterable $listeners, ?ExceptionListener $exceptionListener, ?LogoutListener $logoutListener, ?FirewallConfig $config, AccessListener $accessListener, TokenStorage $tokenStorage, AccessMapInterface $map) + { + parent::__construct($listeners, $exceptionListener, $logoutListener, $config); + + $this->accessListener = $accessListener; + $this->tokenStorage = $tokenStorage; + $this->map = $map; + } + + public function getListeners(): iterable + { + return [$this]; + } + + public function __invoke(RequestEvent $event) + { + $this->tokenStorage->setInitializer(function () use ($event) { + $event = new LazyResponseEvent($event); + foreach (parent::getListeners() as $listener) { + if (\is_callable($listener)) { + $listener($event); + } else { + @trigger_error(sprintf('Calling the "%s::handle()" method from the firewall is deprecated since Symfony 4.3, implement "__invoke()" instead.', \get_class($listener)), E_USER_DEPRECATED); + $listener->handle($event); + } + } + }); + + try { + [$attributes] = $this->map->getPatterns($event->getRequest()); + + if ($attributes && [AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY] !== $attributes) { + ($this->accessListener)($event); + } + } catch (LazyResponseException $e) { + $event->setResponse($e->getResponse()); + } + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Controller/LocalizedController.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Controller/LocalizedController.php index 269827e2df..cf0e1150af 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Controller/LocalizedController.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/FormLoginBundle/Controller/LocalizedController.php @@ -59,6 +59,6 @@ class LocalizedController implements ContainerAwareInterface public function homepageAction() { - return new Response('Homepage'); + return (new Response('Homepage'))->setPublic(); } } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php index 06260c1bed..0303f1b4ee 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityRoutingIntegrationTest.php @@ -129,6 +129,16 @@ class SecurityRoutingIntegrationTest extends AbstractWebTestCase $client->request('GET', '/unprotected_resource'); } + public function testPublicHomepage() + { + $client = $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => 'config.yml']); + $client->request('GET', '/en/'); + + $this->assertEquals(200, $client->getResponse()->getStatusCode(), (string) $client->getResponse()); + $this->assertTrue($client->getResponse()->headers->getCacheControlDirective('public')); + $this->assertSame(0, self::$container->get('session')->getUsageIndex()); + } + private function assertAllowed($client, $path) { $client->request('GET', $path); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml index 4e2ac1e11b..ad8beee94c 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/config.yml @@ -27,7 +27,7 @@ security: check_path: /login_check default_target_path: /profile logout: ~ - anonymous: ~ + anonymous: lazy # This firewall is here just to check its the logout functionality second_area: @@ -38,6 +38,7 @@ security: path: /second/logout access_control: + - { path: ^/en/$, roles: IS_AUTHENTICATED_ANONYMOUSLY } - { path: ^/unprotected_resource$, roles: IS_AUTHENTICATED_ANONYMOUSLY } - { path: ^/secure-but-not-covered-by-access-control$, roles: IS_AUTHENTICATED_ANONYMOUSLY } - { path: ^/secured-by-one-ip$, ip: 10.10.10.10, roles: IS_AUTHENTICATED_ANONYMOUSLY } diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json index 4bd0816938..77bd4a0cfb 100644 --- a/src/Symfony/Bundle/SecurityBundle/composer.json +++ b/src/Symfony/Bundle/SecurityBundle/composer.json @@ -24,7 +24,7 @@ "symfony/security-core": "^4.4", "symfony/security-csrf": "^4.2|^5.0", "symfony/security-guard": "^4.2|^5.0", - "symfony/security-http": "^4.3" + "symfony/security-http": "^4.4" }, "require-dev": { "symfony/asset": "^3.4|^4.0|^5.0", diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/Storage/TokenStorage.php b/src/Symfony/Component/Security/Core/Authentication/Token/Storage/TokenStorage.php index 8a02802d9c..bf491797aa 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/Storage/TokenStorage.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/Storage/TokenStorage.php @@ -25,12 +25,18 @@ use Symfony\Contracts\Service\ResetInterface; class TokenStorage implements TokenStorageInterface, ResetInterface { private $token; + private $initializer; /** * {@inheritdoc} */ public function getToken() { + if ($initializer = $this->initializer) { + $this->initializer = null; + $initializer(); + } + return $this->token; } @@ -43,9 +49,15 @@ class TokenStorage implements TokenStorageInterface, ResetInterface @trigger_error(sprintf('Not implementing the "%s::getRoleNames()" method in "%s" is deprecated since Symfony 4.3.', TokenInterface::class, \get_class($token)), E_USER_DEPRECATED); } + $this->initializer = null; $this->token = $token; } + public function setInitializer(?callable $initializer): void + { + $this->initializer = $initializer; + } + public function reset() { $this->setToken(null); diff --git a/src/Symfony/Component/Security/Core/Exception/LazyResponseException.php b/src/Symfony/Component/Security/Core/Exception/LazyResponseException.php new file mode 100644 index 0000000000..8edc248a04 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Exception/LazyResponseException.php @@ -0,0 +1,34 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Exception; + +use Symfony\Component\HttpFoundation\Response; + +/** + * A signaling exception that wraps a lazily computed response. + * + * @author Nicolas Grekas + */ +class LazyResponseException extends \Exception implements ExceptionInterface +{ + private $response; + + public function __construct(Response $response) + { + $this->response = $response; + } + + public function getResponse(): Response + { + return $this->response; + } +} diff --git a/src/Symfony/Component/Security/Http/Event/LazyResponseEvent.php b/src/Symfony/Component/Security/Http/Event/LazyResponseEvent.php new file mode 100644 index 0000000000..aa473bc0aa --- /dev/null +++ b/src/Symfony/Component/Security/Http/Event/LazyResponseEvent.php @@ -0,0 +1,76 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Event; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Event\RequestEvent; +use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\Component\Security\Core\Exception\LazyResponseException; + +/** + * Wraps a lazily computed response in a signaling exception. + * + * @author Nicolas Grekas + */ +final class LazyResponseEvent extends RequestEvent +{ + private $event; + + public function __construct(parent $event) + { + $this->event = $event; + } + + /** + * {@inheritdoc} + */ + public function setResponse(Response $response) + { + $this->stopPropagation(); + $this->event->stopPropagation(); + + throw new LazyResponseException($response); + } + + /** + * {@inheritdoc} + */ + public function getKernel(): HttpKernelInterface + { + return $this->event->getKernel(); + } + + /** + * {@inheritdoc} + */ + public function getRequest(): Request + { + return $this->event->getRequest(); + } + + /** + * {@inheritdoc} + */ + public function getRequestType(): int + { + return $this->event->getRequestType(); + } + + /** + * {@inheritdoc} + */ + public function isMasterRequest(): bool + { + return $this->event->isMasterRequest(); + } +} diff --git a/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php b/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php index c97a051024..549543e3ef 100644 --- a/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php @@ -26,6 +26,7 @@ use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\AccountStatusException; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\InsufficientAuthenticationException; +use Symfony\Component\Security\Core\Exception\LazyResponseException; use Symfony\Component\Security\Core\Exception\LogoutException; use Symfony\Component\Security\Core\Security; use Symfony\Component\Security\Http\Authorization\AccessDeniedHandlerInterface; @@ -103,6 +104,12 @@ class ExceptionListener return; } + if ($exception instanceof LazyResponseException) { + $event->setResponse($exception->getResponse()); + + return; + } + if ($exception instanceof LogoutException) { $this->handleLogoutException($exception); From 152dec95bcef93962cd2e7adea76893d221c8681 Mon Sep 17 00:00:00 2001 From: k0d3r1s Date: Wed, 18 Sep 2019 16:21:11 +0300 Subject: [PATCH 2/7] [DependencyInjection] Fix wrong exception when service is synthetic --- .../Compiler/AbstractRecursivePass.php | 4 +++ .../Tests/Compiler/AutowirePassTest.php | 4 +-- .../Compiler/ResolveBindingsPassTest.php | 27 ++++++++++++++++--- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php index a89bf1647a..5ca2b2246b 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php @@ -89,6 +89,10 @@ abstract class AbstractRecursivePass implements CompilerPassInterface */ protected function getConstructor(Definition $definition, $required) { + if ($definition->isSynthetic()) { + return null; + } + if (\is_string($factory = $definition->getFactory())) { if (!\function_exists($factory)) { throw new RuntimeException(sprintf('Invalid service "%s": function "%s" does not exist.', $this->currentId, $factory)); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php index f729f72ba0..7eb7f60246 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php @@ -589,12 +589,10 @@ class AutowirePassTest extends TestCase ); } - /** - * @exceptedExceptionMessage Invalid service "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy": method "setLogger()" does not exist. - */ public function testWithNonExistingSetterAndAutowiring() { $this->expectException('Symfony\Component\DependencyInjection\Exception\RuntimeException'); + $this->expectExceptionMessage('Invalid service "Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass": method "setLogger()" does not exist.'); $container = new ContainerBuilder(); $definition = $container->register(CaseSensitiveClass::class, CaseSensitiveClass::class)->setAutowired(true); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php index a44fea4d60..fd526caa94 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveBindingsPassTest.php @@ -14,9 +14,11 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Argument\BoundArgument; use Symfony\Component\DependencyInjection\Compiler\AutowireRequiredMethodsPass; +use Symfony\Component\DependencyInjection\Compiler\DefinitionErrorExceptionPass; use Symfony\Component\DependencyInjection\Compiler\ResolveBindingsPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass; use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy; @@ -110,12 +112,10 @@ class ResolveBindingsPassTest extends TestCase $this->assertEquals([['setDefaultLocale', ['fr']]], $definition->getMethodCalls()); } - /** - * @exceptedExceptionMessage Invalid service "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy": method "setLogger()" does not exist. - */ public function testWithNonExistingSetterAndBinding() { $this->expectException('Symfony\Component\DependencyInjection\Exception\RuntimeException'); + $this->expectExceptionMessage('Invalid service "Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy": method "setLogger()" does not exist.'); $container = new ContainerBuilder(); $bindings = [ @@ -129,4 +129,25 @@ class ResolveBindingsPassTest extends TestCase $pass = new ResolveBindingsPass(); $pass->process($container); } + + public function testSyntheticServiceWithBind() + { + $container = new ContainerBuilder(); + $argument = new BoundArgument('bar'); + + $container->register('foo', 'stdClass') + ->addArgument(new Reference('synthetic.service')); + + $container->register('synthetic.service') + ->setSynthetic(true) + ->setBindings(['$apiKey' => $argument]); + + $container->register(NamedArgumentsDummy::class, NamedArgumentsDummy::class) + ->setBindings(['$apiKey' => $argument]); + + (new ResolveBindingsPass())->process($container); + (new DefinitionErrorExceptionPass())->process($container); + + $this->assertSame([1 => 'bar'], $container->getDefinition(NamedArgumentsDummy::class)->getArguments()); + } } From 79090928916350a1da48c540ec3e275855420fe7 Mon Sep 17 00:00:00 2001 From: Konstantin Myakshin Date: Thu, 30 May 2019 00:00:39 +0300 Subject: [PATCH 3/7] [Messenger] Improve error message when routing to an invalid transport (closes #31613) --- .../FrameworkExtension.php | 19 ++++++++++------ .../Fixtures/php/messenger.php | 5 +++++ .../Fixtures/php/messenger_routing.php | 3 ++- .../messenger_routing_invalid_transport.php | 16 ++++++++++++++ .../Fixtures/xml/messenger.xml | 3 +++ .../Fixtures/xml/messenger_routing.xml | 3 ++- .../messenger_routing_invalid_transport.xml | 22 +++++++++++++++++++ .../Fixtures/yml/messenger.yml | 4 ++++ .../Fixtures/yml/messenger_routing.yml | 3 ++- .../messenger_routing_invalid_transport.yml | 9 ++++++++ .../FrameworkExtensionTest.php | 20 ++++++++++------- 11 files changed, 89 insertions(+), 18 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing_invalid_transport.yml diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 38b552716d..1d83148ff7 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -1774,6 +1774,16 @@ class FrameworkExtension extends Extension } } + $senderReferences = []; + // alias => service_id + foreach ($senderAliases as $alias => $serviceId) { + $senderReferences[$alias] = new Reference($serviceId); + } + // service_id => service_id + foreach ($senderAliases as $serviceId) { + $senderReferences[$serviceId] = new Reference($serviceId); + } + $messageToSendersMapping = []; foreach ($config['routing'] as $message => $messageConfiguration) { if ('*' !== $message && !class_exists($message) && !interface_exists($message, false)) { @@ -1782,19 +1792,14 @@ class FrameworkExtension extends Extension // make sure senderAliases contains all senders foreach ($messageConfiguration['senders'] as $sender) { - if (!isset($senderAliases[$sender])) { - $senderAliases[$sender] = $sender; + if (!isset($senderReferences[$sender])) { + throw new LogicException(sprintf('Invalid Messenger routing configuration: the "%s" class is being routed to a sender called "%s". This is not a valid transport or service id.', $message, $sender)); } } $messageToSendersMapping[$message] = $messageConfiguration['senders']; } - $senderReferences = []; - foreach ($senderAliases as $alias => $serviceId) { - $senderReferences[$alias] = new Reference($serviceId); - } - $container->getDefinition('messenger.senders_locator') ->replaceArgument(0, $messageToSendersMapping) ->replaceArgument(1, ServiceLocatorTagPass::register($container, $senderReferences)) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger.php index 1160dfc573..adb8239d04 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger.php @@ -9,5 +9,10 @@ $container->loadFromExtension('framework', [ FooMessage::class => ['sender.bar', 'sender.biz'], BarMessage::class => 'sender.foo', ], + 'transports' => [ + 'sender.biz' => 'null://', + 'sender.bar' => 'null://', + 'sender.foo' => 'null://', + ], ], ]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php index 0cecbd71a5..eb45950901 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing.php @@ -7,7 +7,7 @@ $container->loadFromExtension('framework', [ 'default_serializer' => 'messenger.transport.symfony_serializer', ], 'routing' => [ - 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage' => ['amqp', 'audit'], + 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage' => ['amqp', 'messenger.transport.audit'], 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\SecondMessage' => [ 'senders' => ['amqp', 'audit'], ], @@ -15,6 +15,7 @@ $container->loadFromExtension('framework', [ ], 'transports' => [ 'amqp' => 'amqp://localhost/%2f/messages', + 'audit' => 'null://', ], ], ]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php new file mode 100644 index 0000000000..ee77e3a3f2 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/php/messenger_routing_invalid_transport.php @@ -0,0 +1,16 @@ +loadFromExtension('framework', [ + 'serializer' => true, + 'messenger' => [ + 'serializer' => [ + 'default_serializer' => 'messenger.transport.symfony_serializer', + ], + 'routing' => [ + 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage' => 'invalid', + ], + 'transports' => [ + 'amqp' => 'amqp://localhost/%2f/messages', + ], + ], +]); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger.xml index e0dc11360a..bacd772dcb 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger.xml @@ -14,6 +14,9 @@ + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml index 62028169ea..0b022e78a0 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing.xml @@ -11,7 +11,7 @@ - + @@ -21,6 +21,7 @@ + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml new file mode 100644 index 0000000000..98c487fbf8 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/messenger_routing_invalid_transport.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger.yml index 7f038af11f..82fea3b27a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger.yml @@ -3,3 +3,7 @@ framework: routing: 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\FooMessage': ['sender.bar', 'sender.biz'] 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\BarMessage': 'sender.foo' + transports: + sender.biz: 'null://' + sender.bar: 'null://' + sender.foo: 'null://' diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml index 540d0d8ad0..0e493eb882 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing.yml @@ -4,9 +4,10 @@ framework: serializer: default_serializer: messenger.transport.symfony_serializer routing: - 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage': [amqp, audit] + 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage': [amqp, messenger.transport.audit] 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\SecondMessage': senders: [amqp, audit] '*': amqp transports: amqp: 'amqp://localhost/%2f/messages' + audit: 'null://' diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing_invalid_transport.yml b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing_invalid_transport.yml new file mode 100644 index 0000000000..3bf0f2ddf9 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/yml/messenger_routing_invalid_transport.yml @@ -0,0 +1,9 @@ +framework: + serializer: true + messenger: + serializer: + default_serializer: messenger.transport.symfony_serializer + routing: + 'Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage': invalid + transports: + amqp: 'amqp://localhost/%2f/messages' diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index 8bea0514ac..60fefc659f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -668,8 +668,8 @@ abstract class FrameworkExtensionTest extends TestCase $this->assertTrue($container->getAlias('message_bus')->isPublic()); $this->assertTrue($container->hasAlias('messenger.default_bus')); $this->assertTrue($container->getAlias('messenger.default_bus')->isPublic()); - $this->assertFalse($container->hasDefinition('messenger.transport.amqp.factory')); - $this->assertFalse($container->hasDefinition('messenger.transport.redis.factory')); + $this->assertTrue($container->hasDefinition('messenger.transport.amqp.factory')); + $this->assertTrue($container->hasDefinition('messenger.transport.redis.factory')); $this->assertTrue($container->hasDefinition('messenger.transport_factory')); $this->assertSame(TransportFactory::class, $container->getDefinition('messenger.transport_factory')->getClass()); } @@ -712,14 +712,11 @@ abstract class FrameworkExtensionTest extends TestCase $senderLocatorDefinition = $container->getDefinition('messenger.senders_locator'); $sendersMapping = $senderLocatorDefinition->getArgument(0); - $this->assertEquals([ - 'amqp', - 'audit', - ], $sendersMapping[DummyMessage::class]); + $this->assertEquals(['amqp', 'messenger.transport.audit'], $sendersMapping[DummyMessage::class]); $sendersLocator = $container->getDefinition((string) $senderLocatorDefinition->getArgument(1)); - $this->assertSame(['amqp', 'audit'], array_keys($sendersLocator->getArgument(0))); + $this->assertSame(['amqp', 'audit', 'messenger.transport.amqp', 'messenger.transport.audit'], array_keys($sendersLocator->getArgument(0))); $this->assertEquals(new Reference('messenger.transport.amqp'), $sendersLocator->getArgument(0)['amqp']->getValues()[0]); - $this->assertEquals(new Reference('audit'), $sendersLocator->getArgument(0)['audit']->getValues()[0]); + $this->assertEquals(new Reference('messenger.transport.audit'), $sendersLocator->getArgument(0)['messenger.transport.audit']->getValues()[0]); } public function testMessengerTransportConfiguration() @@ -776,6 +773,13 @@ abstract class FrameworkExtensionTest extends TestCase $this->createContainerFromFile('messenger_middleware_factory_erroneous_format'); } + public function testMessengerInvalidTransportRouting() + { + $this->expectException('LogicException'); + $this->expectExceptionMessage('Invalid Messenger routing configuration: the "Symfony\Bundle\FrameworkBundle\Tests\Fixtures\Messenger\DummyMessage" class is being routed to a sender called "invalid". This is not a valid transport or service id.'); + $this->createContainerFromFile('messenger_routing_invalid_transport'); + } + public function testTranslator() { $container = $this->createContainerFromFile('full'); From 8a9d173c36a6f5fd9d379f750f35d81d49e004f7 Mon Sep 17 00:00:00 2001 From: "M. Vondano" Date: Sun, 1 Sep 2019 19:33:46 +0200 Subject: [PATCH 4/7] Do not include hidden commands in suggested alternatives --- src/Symfony/Component/Console/Application.php | 20 +++++++++++++++--- .../Console/Tests/ApplicationTest.php | 2 ++ .../Tests/Fixtures/FooHiddenCommand.php | 21 +++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 src/Symfony/Component/Console/Tests/Fixtures/FooHiddenCommand.php diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php index 8a9bdc39bf..d181e41e80 100644 --- a/src/Symfony/Component/Console/Application.php +++ b/src/Symfony/Component/Console/Application.php @@ -529,6 +529,10 @@ class Application { $namespaces = []; foreach ($this->all() as $command) { + if ($command->isHidden()) { + continue; + } + $namespaces = array_merge($namespaces, $this->extractAllNamespaces($command->getName())); foreach ($command->getAliases() as $alias) { @@ -622,6 +626,11 @@ class Application $message = sprintf('Command "%s" is not defined.', $name); if ($alternatives = $this->findAlternatives($name, $allCommands)) { + // remove hidden commands + $alternatives = array_filter($alternatives, function ($name) { + return !$this->get($name)->isHidden(); + }); + if (1 == \count($alternatives)) { $message .= "\n\nDid you mean this?\n "; } else { @@ -630,7 +639,7 @@ class Application $message .= implode("\n ", $alternatives); } - throw new CommandNotFoundException($message, $alternatives); + throw new CommandNotFoundException($message, array_values($alternatives)); } // filter out aliases for commands which are already on the list @@ -654,13 +663,18 @@ class Application } $abbrevs = array_map(function ($cmd) use ($commandList, $usableWidth, $maxLen) { if (!$commandList[$cmd] instanceof Command) { - return $cmd; + $commandList[$cmd] = $this->commandLoader->get($cmd); } + + if ($commandList[$cmd]->isHidden()) { + return false; + } + $abbrev = str_pad($cmd, $maxLen, ' ').' '.$commandList[$cmd]->getDescription(); return Helper::strlen($abbrev) > $usableWidth ? Helper::substr($abbrev, 0, $usableWidth - 3).'...' : $abbrev; }, array_values($commands)); - $suggestions = $this->getAbbreviationSuggestions($abbrevs); + $suggestions = $this->getAbbreviationSuggestions(array_filter($abbrevs)); throw new CommandNotFoundException(sprintf("Command \"%s\" is ambiguous.\nDid you mean one of these?\n%s", $name, $suggestions), array_values($commands)); } diff --git a/src/Symfony/Component/Console/Tests/ApplicationTest.php b/src/Symfony/Component/Console/Tests/ApplicationTest.php index 59ca5d9e15..c7e8495265 100644 --- a/src/Symfony/Component/Console/Tests/ApplicationTest.php +++ b/src/Symfony/Component/Console/Tests/ApplicationTest.php @@ -74,6 +74,7 @@ class ApplicationTest extends TestCase require_once self::$fixturesPath.'/FooSubnamespaced2Command.php'; require_once self::$fixturesPath.'/TestAmbiguousCommandRegistering.php'; require_once self::$fixturesPath.'/TestAmbiguousCommandRegistering2.php'; + require_once self::$fixturesPath.'/FooHiddenCommand.php'; } protected function normalizeLineBreaks($text) @@ -616,6 +617,7 @@ class ApplicationTest extends TestCase $application->add(new \Foo1Command()); $application->add(new \Foo2Command()); $application->add(new \Foo3Command()); + $application->add(new \FooHiddenCommand()); $expectedAlternatives = [ 'afoobar', diff --git a/src/Symfony/Component/Console/Tests/Fixtures/FooHiddenCommand.php b/src/Symfony/Component/Console/Tests/Fixtures/FooHiddenCommand.php new file mode 100644 index 0000000000..75fbf7804d --- /dev/null +++ b/src/Symfony/Component/Console/Tests/Fixtures/FooHiddenCommand.php @@ -0,0 +1,21 @@ +setName('foo:hidden') + ->setAliases(['afoohidden']) + ->setHidden(true) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output) + { + } +} From f3406338e65182e5c6d096e83593877163efad69 Mon Sep 17 00:00:00 2001 From: "M. Vondano" Date: Sun, 1 Sep 2019 19:33:46 +0200 Subject: [PATCH 5/7] [Console] Deprecate abbreviating hidden command names using Application->find() --- UPGRADE-4.4.md | 5 ++ UPGRADE-5.0.md | 1 + src/Symfony/Component/Console/Application.php | 19 +++++-- src/Symfony/Component/Console/CHANGELOG.md | 1 + .../Console/Tests/ApplicationTest.php | 54 +++++++++++++++++++ .../Tests/Fixtures/BarHiddenCommand.php | 21 ++++++++ 6 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 src/Symfony/Component/Console/Tests/Fixtures/BarHiddenCommand.php diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md index 584eb693e2..323bf96c18 100644 --- a/UPGRADE-4.4.md +++ b/UPGRADE-4.4.md @@ -6,6 +6,11 @@ Cache * Added argument `$prefix` to `AdapterInterface::clear()` +Console +------- + + * Deprecated finding hidden commands using an abbreviation, use the full name instead + Debug ----- diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index 1039e3a5fe..e0df969c4a 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -31,6 +31,7 @@ Config Console ------- + * Removed support for finding hidden commands using an abbreviation, use the full name instead * Removed the `setCrossingChar()` method in favor of the `setDefaultCrossingChar()` method in `TableStyle`. * Removed the `setHorizontalBorderChar()` method in favor of the `setDefaultCrossingChars()` method in `TableStyle`. * Removed the `getHorizontalBorderChar()` method in favor of the `getBorderChars()` method in `TableStyle`. diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php index af1af4b8ec..e66705ab37 100644 --- a/src/Symfony/Component/Console/Application.php +++ b/src/Symfony/Component/Console/Application.php @@ -692,12 +692,14 @@ class Application implements ResetInterface foreach ($abbrevs as $abbrev) { $maxLen = max(Helper::strlen($abbrev), $maxLen); } - $abbrevs = array_map(function ($cmd) use ($commandList, $usableWidth, $maxLen) { + $abbrevs = array_map(function ($cmd) use ($commandList, $usableWidth, $maxLen, &$commands) { if (!$commandList[$cmd] instanceof Command) { $commandList[$cmd] = $this->commandLoader->get($cmd); } if ($commandList[$cmd]->isHidden()) { + unset($commands[array_search($cmd, $commands)]); + return false; } @@ -705,12 +707,21 @@ class Application implements ResetInterface return Helper::strlen($abbrev) > $usableWidth ? Helper::substr($abbrev, 0, $usableWidth - 3).'...' : $abbrev; }, array_values($commands)); - $suggestions = $this->getAbbreviationSuggestions(array_filter($abbrevs)); - throw new CommandNotFoundException(sprintf("Command \"%s\" is ambiguous.\nDid you mean one of these?\n%s", $name, $suggestions), array_values($commands)); + if (\count($commands) > 1) { + $suggestions = $this->getAbbreviationSuggestions(array_filter($abbrevs)); + + throw new CommandNotFoundException(sprintf("Command \"%s\" is ambiguous.\nDid you mean one of these?\n%s", $name, $suggestions), array_values($commands)); + } } - return $this->get(reset($commands)); + $command = $this->get(reset($commands)); + + if ($command->isHidden()) { + @trigger_error(sprintf('Command "%s" is hidden, finding it using an abbreviation is deprecated since Symfony 4.4, use its full name instead.', $command->getName()), E_USER_DEPRECATED); + } + + return $command; } /** diff --git a/src/Symfony/Component/Console/CHANGELOG.md b/src/Symfony/Component/Console/CHANGELOG.md index b9e837a8d1..d1b8a324c6 100644 --- a/src/Symfony/Component/Console/CHANGELOG.md +++ b/src/Symfony/Component/Console/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 4.4.0 ----- + * deprecated finding hidden commands using an abbreviation, use the full name instead * added `Question::setTrimmable` default to true to allow the answer to be trimmed * added method `preventRedrawFasterThan()` and `forceRedrawSlowerThan()` on `ProgressBar` * `Application` implements `ResetInterface` diff --git a/src/Symfony/Component/Console/Tests/ApplicationTest.php b/src/Symfony/Component/Console/Tests/ApplicationTest.php index 9b444419b8..e750975d2d 100644 --- a/src/Symfony/Component/Console/Tests/ApplicationTest.php +++ b/src/Symfony/Component/Console/Tests/ApplicationTest.php @@ -76,6 +76,7 @@ class ApplicationTest extends TestCase require_once self::$fixturesPath.'/TestAmbiguousCommandRegistering.php'; require_once self::$fixturesPath.'/TestAmbiguousCommandRegistering2.php'; require_once self::$fixturesPath.'/FooHiddenCommand.php'; + require_once self::$fixturesPath.'/BarHiddenCommand.php'; } protected function normalizeLineBreaks($text) @@ -441,6 +442,16 @@ class ApplicationTest extends TestCase ]; } + public function testFindWithAmbiguousAbbreviationsFindsCommandIfAlternativesAreHidden() + { + $application = new Application(); + + $application->add(new \FooCommand()); + $application->add(new \FooHiddenCommand()); + + $this->assertInstanceOf('FooCommand', $application->find('foo:')); + } + public function testFindCommandEqualNamespace() { $application = new Application(); @@ -708,6 +719,49 @@ class ApplicationTest extends TestCase $application->find('foo::bar'); } + public function testFindHiddenWithExactName() + { + $application = new Application(); + $application->add(new \FooHiddenCommand()); + + $this->assertInstanceOf('FooHiddenCommand', $application->find('foo:hidden')); + $this->assertInstanceOf('FooHiddenCommand', $application->find('afoohidden')); + } + + /** + * @group legacy + * @expectedDeprecation Command "%s:hidden" is hidden, finding it using an abbreviation is deprecated since Symfony 4.4, use its full name instead. + * @dataProvider provideAbbreviationsForHiddenCommands + */ + public function testFindHiddenWithAbbreviatedName($name) + { + $application = new Application(); + + $application->add(new \FooHiddenCommand()); + $application->add(new \BarHiddenCommand()); + + $application->find($name); + } + + public function provideAbbreviationsForHiddenCommands() + { + return [ + ['foo:hidde'], + ['afoohidd'], + ['bar:hidde'], + ]; + } + + public function testFindAmbiguousCommandsIfAllAlternativesAreHidden() + { + $application = new Application(); + + $application->add(new \FooCommand()); + $application->add(new \FooHiddenCommand()); + + $this->assertInstanceOf('FooCommand', $application->find('foo:')); + } + public function testSetCatchExceptions() { $application = new Application(); diff --git a/src/Symfony/Component/Console/Tests/Fixtures/BarHiddenCommand.php b/src/Symfony/Component/Console/Tests/Fixtures/BarHiddenCommand.php new file mode 100644 index 0000000000..d500f8731f --- /dev/null +++ b/src/Symfony/Component/Console/Tests/Fixtures/BarHiddenCommand.php @@ -0,0 +1,21 @@ +setName('bar:hidden') + ->setAliases(['abarhidden']) + ->setHidden(true) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output) + { + } +} From 4db140277038f16eb4b6af9477a6b152966de7c2 Mon Sep 17 00:00:00 2001 From: marie <15118505+marie@users.noreply.github.com> Date: Fri, 20 Sep 2019 15:03:12 +0500 Subject: [PATCH 6/7] [HttpFoundation] allow additinal characters in not raw cookies --- .../Component/HttpFoundation/Cookie.php | 21 +++++++++++++---- .../Component/HttpFoundation/Response.php | 2 +- .../HttpFoundation/Tests/CookieTest.php | 23 +++++++++++++++---- .../cookie_urlencode.expected | 3 ++- .../response-functional/cookie_urlencode.php | 9 +++++--- .../invalid_cookie_name.php | 2 +- 6 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Cookie.php b/src/Symfony/Component/HttpFoundation/Cookie.php index 83a97087f1..98a5ef00a8 100644 --- a/src/Symfony/Component/HttpFoundation/Cookie.php +++ b/src/Symfony/Component/HttpFoundation/Cookie.php @@ -18,6 +18,10 @@ namespace Symfony\Component\HttpFoundation; */ class Cookie { + const SAMESITE_NONE = 'none'; + const SAMESITE_LAX = 'lax'; + const SAMESITE_STRICT = 'strict'; + protected $name; protected $value; protected $domain; @@ -25,12 +29,13 @@ class Cookie protected $path; protected $secure; protected $httpOnly; + private $raw; private $sameSite; - const SAMESITE_NONE = 'none'; - const SAMESITE_LAX = 'lax'; - const SAMESITE_STRICT = 'strict'; + private static $reservedCharsList = "=,; \t\r\n\v\f"; + private static $reservedCharsFrom = ['=', ',', ';', ' ', "\t", "\r", "\n", "\v", "\f"]; + private static $reservedCharsTo = ['%3D', '%2C', '%3B', '%20', '%09', '%0D', '%0A', '%0B', '%0C']; /** * Creates cookie from raw header string. @@ -97,7 +102,7 @@ class Cookie public function __construct($name, $value = null, $expire = 0, $path = '/', $domain = null, $secure = false, $httpOnly = true, $raw = false, $sameSite = null) { // from PHP source code - if (preg_match("/[=,; \t\r\n\013\014]/", $name)) { + if ($raw && false !== strpbrk($name, self::$reservedCharsList)) { throw new \InvalidArgumentException(sprintf('The cookie name "%s" contains invalid characters.', $name)); } @@ -143,7 +148,13 @@ class Cookie */ public function __toString() { - $str = ($this->isRaw() ? $this->getName() : urlencode($this->getName())).'='; + if ($this->isRaw()) { + $str = $this->getName(); + } else { + $str = str_replace(self::$reservedCharsFrom, self::$reservedCharsTo, $this->getName()); + } + + $str .= '='; if ('' === (string) $this->getValue()) { $str .= 'deleted; expires='.gmdate('D, d-M-Y H:i:s T', time() - 31536001).'; Max-Age=0'; diff --git a/src/Symfony/Component/HttpFoundation/Response.php b/src/Symfony/Component/HttpFoundation/Response.php index 8a17acdd5f..26e3a3378e 100644 --- a/src/Symfony/Component/HttpFoundation/Response.php +++ b/src/Symfony/Component/HttpFoundation/Response.php @@ -342,7 +342,7 @@ class Response // cookies foreach ($this->headers->getCookies() as $cookie) { - header('Set-Cookie: '.$cookie->getName().strstr($cookie, '='), false, $this->statusCode); + header('Set-Cookie: '.$cookie, false, $this->statusCode); } // status diff --git a/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php b/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php index 38c195df36..169f917875 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/CookieTest.php @@ -24,10 +24,9 @@ use Symfony\Component\HttpFoundation\Cookie; */ class CookieTest extends TestCase { - public function invalidNames() + public function namesWithSpecialCharacters() { return [ - [''], [',MyName'], [';MyName'], [' MyName'], @@ -40,12 +39,26 @@ class CookieTest extends TestCase } /** - * @dataProvider invalidNames + * @dataProvider namesWithSpecialCharacters */ - public function testInstantiationThrowsExceptionIfCookieNameContainsInvalidCharacters($name) + public function testInstantiationThrowsExceptionIfRawCookieNameContainsSpecialCharacters($name) { $this->expectException('InvalidArgumentException'); - new Cookie($name); + new Cookie($name, null, 0, null, null, null, false, true); + } + + /** + * @dataProvider namesWithSpecialCharacters + */ + public function testInstantiationSucceedNonRawCookieNameContainsSpecialCharacters($name) + { + $this->assertInstanceOf(Cookie::class, new Cookie($name)); + } + + public function testInstantiationThrowsExceptionIfCookieNameIsEmpty() + { + $this->expectException('InvalidArgumentException'); + new Cookie(''); } public function testInvalidExpiration() diff --git a/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/cookie_urlencode.expected b/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/cookie_urlencode.expected index 14e44a398a..17a9efc669 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/cookie_urlencode.expected +++ b/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/cookie_urlencode.expected @@ -4,7 +4,8 @@ Array [0] => Content-Type: text/plain; charset=utf-8 [1] => Cache-Control: no-cache, private [2] => Date: Sat, 12 Nov 1955 20:04:00 GMT - [3] => Set-Cookie: ?*():@&+$/%#[]=%3F%2A%28%29%3A%40%26%2B%24%2F%25%23%5B%5D; path=/ + [3] => Set-Cookie: %3D%2C%3B%20%09%0D%0A%0B%0C=%3D%2C%3B%20%09%0D%0A%0B%0C; path=/ [4] => Set-Cookie: ?*():@&+$/%#[]=%3F%2A%28%29%3A%40%26%2B%24%2F%25%23%5B%5D; path=/ + [5] => Set-Cookie: ?*():@&+$/%#[]=%3F%2A%28%29%3A%40%26%2B%24%2F%25%23%5B%5D; path=/ ) shutdown diff --git a/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/cookie_urlencode.php b/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/cookie_urlencode.php index 05b9af30d5..9ffb0dfec8 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/cookie_urlencode.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/cookie_urlencode.php @@ -4,9 +4,12 @@ use Symfony\Component\HttpFoundation\Cookie; $r = require __DIR__.'/common.inc'; -$str = '?*():@&+$/%#[]'; +$str1 = "=,; \t\r\n\v\f"; +$r->headers->setCookie(new Cookie($str1, $str1, 0, '', null, false, false, false, null)); -$r->headers->setCookie(new Cookie($str, $str, 0, '', null, false, false)); +$str2 = '?*():@&+$/%#[]'; + +$r->headers->setCookie(new Cookie($str2, $str2, 0, '', null, false, false, false, null)); $r->sendHeaders(); -setcookie($str, $str, 0, '/'); +setcookie($str2, $str2, 0, '/'); diff --git a/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/invalid_cookie_name.php b/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/invalid_cookie_name.php index 3fe1571845..3acf86039d 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/invalid_cookie_name.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Fixtures/response-functional/invalid_cookie_name.php @@ -5,7 +5,7 @@ use Symfony\Component\HttpFoundation\Cookie; $r = require __DIR__.'/common.inc'; try { - $r->headers->setCookie(new Cookie('Hello + world', 'hodor')); + $r->headers->setCookie(new Cookie('Hello + world', 'hodor', 0, null, null, null, false, true)); } catch (\InvalidArgumentException $e) { echo $e->getMessage(); } From fda5b20c3974bb6269d2ff9e29dd61a495af1cf4 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sat, 28 Sep 2019 18:12:11 +0200 Subject: [PATCH 7/7] sync phpunit script with master --- phpunit | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phpunit b/phpunit index d334dc0720..b07ca07d06 100755 --- a/phpunit +++ b/phpunit @@ -17,5 +17,8 @@ if (!getenv('SYMFONY_PHPUNIT_VERSION')) { putenv('SYMFONY_PHPUNIT_VERSION=6.5'); } } +if (!getenv('SYMFONY_PATCH_TYPE_DECLARATIONS')) { + putenv('SYMFONY_PATCH_TYPE_DECLARATIONS=deprecations=1'); +} putenv('SYMFONY_PHPUNIT_DIR='.__DIR__.'/.phpunit'); require __DIR__.'/vendor/symfony/phpunit-bridge/bin/simple-phpunit';