From 3760e67cb234b940abddf5d348eb6e62411d62eb Mon Sep 17 00:00:00 2001 From: Peter Thompson Date: Mon, 5 Jan 2015 11:24:54 +0100 Subject: [PATCH 1/7] [Yaml] Improve YAML boolean escaping - Moves dumping single-quoting logic into Yaml\Escaper - Ensures that PHP values which would be interpreted as booleans in older versions of the YAML spec are escaped with single quotes when dumped by the Dumper. --- src/Symfony/Component/Yaml/Escaper.php | 26 ++++++++++++++++++- src/Symfony/Component/Yaml/Inline.php | 4 +-- .../Component/Yaml/Tests/InlineTest.php | 16 ++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Yaml/Escaper.php b/src/Symfony/Component/Yaml/Escaper.php index 6f9785886f..e4a7f07f34 100644 --- a/src/Symfony/Component/Yaml/Escaper.php +++ b/src/Symfony/Component/Yaml/Escaper.php @@ -72,7 +72,7 @@ class Escaper */ public static function requiresSingleQuoting($value) { - return preg_match('/[ \s \' " \: \{ \} \[ \] , & \* \# \?] | \A[ \- ? | < > = ! % @ ` ]/x', $value); + return self::containsCharRequiresSingleQuoting($value) || self::isValueRequiresSingleQuoting($value); } /** @@ -86,4 +86,28 @@ class Escaper { return sprintf("'%s'", str_replace('\'', '\'\'', $value)); } + + /** + * Determines if a PHP value contains any single characters that would cause + * the value to require single quoting in YAML. + * + * @param string $value A PHP value + * @return bool True if the value would require single quotes. + */ + private static function containsCharRequiresSingleQuoting($value) + { + return preg_match('/[ \s \' " \: \{ \} \[ \] , & \* \# \?] | \A[ \- ? | < > = ! % @ ` ]/x', $value); + } + + /** + * Determines if a PHP value is entirely composed of a value that would + * require require single quoting in YAML. + * + * @param string $value A PHP value + * @return bool True if the value would require single quotes. + */ + private static function isValueRequiresSingleQuoting($value) + { + return in_array(strtolower($value), array('null', '~', 'true', 'false', 'y', 'n', 'yes', 'no', 'on', 'off')); + } } diff --git a/src/Symfony/Component/Yaml/Inline.php b/src/Symfony/Component/Yaml/Inline.php index df873bf297..1bdcf87623 100644 --- a/src/Symfony/Component/Yaml/Inline.php +++ b/src/Symfony/Component/Yaml/Inline.php @@ -135,12 +135,10 @@ class Inline case Escaper::requiresDoubleQuoting($value): return Escaper::escapeWithDoubleQuotes($value); case Escaper::requiresSingleQuoting($value): + case preg_match(self::getTimestampRegex(), $value): return Escaper::escapeWithSingleQuotes($value); case '' == $value: return "''"; - case preg_match(self::getTimestampRegex(), $value): - case in_array(strtolower($value), array('null', '~', 'true', 'false')): - return "'$value'"; default: return $value; } diff --git a/src/Symfony/Component/Yaml/Tests/InlineTest.php b/src/Symfony/Component/Yaml/Tests/InlineTest.php index 31572481a5..0cbea85ded 100644 --- a/src/Symfony/Component/Yaml/Tests/InlineTest.php +++ b/src/Symfony/Component/Yaml/Tests/InlineTest.php @@ -190,6 +190,14 @@ class InlineTest extends \PHPUnit_Framework_TestCase "'#cfcfcf'" => '#cfcfcf', '::form_base.html.twig' => '::form_base.html.twig', + // Pre-YAML-1.2 booleans + "'y'" => 'y', + "'n'" => 'n', + "'yes'" => 'yes', + "'no'" => 'no', + "'on'" => 'on', + "'off'" => 'off', + '2007-10-30' => mktime(0, 0, 0, 10, 30, 2007), '2007-10-30T02:59:43Z' => gmmktime(2, 59, 43, 10, 30, 2007), '2007-10-30 02:59:43 Z' => gmmktime(2, 59, 43, 10, 30, 2007), @@ -257,6 +265,14 @@ class InlineTest extends \PHPUnit_Framework_TestCase "'-dash'" => '-dash', "'-'" => '-', + // Pre-YAML-1.2 booleans + "'y'" => 'y', + "'n'" => 'n', + "'yes'" => 'yes', + "'no'" => 'no', + "'on'" => 'on', + "'off'" => 'off', + // sequences '[foo, bar, false, null, 12]' => array('foo', 'bar', false, null, 12), '[\'foo,bar\', \'foo bar\']' => array('foo,bar', 'foo bar'), From 81a8090dde6c997a71d87604d4a421f58b514ca0 Mon Sep 17 00:00:00 2001 From: Lee Rowlands Date: Tue, 6 Jan 2015 14:18:13 +1000 Subject: [PATCH 2/7] Remove duplicate 'require' --- src/Symfony/Component/Yaml/Escaper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Yaml/Escaper.php b/src/Symfony/Component/Yaml/Escaper.php index e4a7f07f34..e3039f57e9 100644 --- a/src/Symfony/Component/Yaml/Escaper.php +++ b/src/Symfony/Component/Yaml/Escaper.php @@ -101,7 +101,7 @@ class Escaper /** * Determines if a PHP value is entirely composed of a value that would - * require require single quoting in YAML. + * require single quoting in YAML. * * @param string $value A PHP value * @return bool True if the value would require single quotes. From a0ec0fe40240f8737bae4692b618c534bb4cd2a4 Mon Sep 17 00:00:00 2001 From: Lee Rowlands Date: Tue, 6 Jan 2015 19:37:09 +1000 Subject: [PATCH 3/7] Add comment as requested --- src/Symfony/Component/Yaml/Escaper.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Symfony/Component/Yaml/Escaper.php b/src/Symfony/Component/Yaml/Escaper.php index e3039f57e9..be846e50c7 100644 --- a/src/Symfony/Component/Yaml/Escaper.php +++ b/src/Symfony/Component/Yaml/Escaper.php @@ -108,6 +108,8 @@ class Escaper */ private static function isValueRequiresSingleQuoting($value) { + // Note that whilst 'y' and 'n' are not supported as valid Booleans, + // they are escaped here for interoperability. return in_array(strtolower($value), array('null', '~', 'true', 'false', 'y', 'n', 'yes', 'no', 'on', 'off')); } } From cb347896b2b17958ece1086ad767d631b734c0c9 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 15 Jan 2015 10:49:01 +0100 Subject: [PATCH 4/7] [Debug] fix loading order for legacy classes --- .../Debug/Exception/FatalErrorException.php | 26 +++---- .../Debug/Exception/FlattenException.php | 72 ++++++++++--------- 2 files changed, 53 insertions(+), 45 deletions(-) diff --git a/src/Symfony/Component/Debug/Exception/FatalErrorException.php b/src/Symfony/Component/Debug/Exception/FatalErrorException.php index 26c8b9c0a1..52f14adaf8 100644 --- a/src/Symfony/Component/Debug/Exception/FatalErrorException.php +++ b/src/Symfony/Component/Debug/Exception/FatalErrorException.php @@ -9,19 +9,6 @@ * file that was distributed with this source code. */ -namespace Symfony\Component\Debug\Exception; - -use Symfony\Component\HttpKernel\Exception\FatalErrorException as LegacyFatalErrorException; - -/** - * Fatal Error Exception. - * - * @author Konstanton Myakshin - */ -class FatalErrorException extends LegacyFatalErrorException -{ -} - namespace Symfony\Component\HttpKernel\Exception; /** @@ -34,3 +21,16 @@ namespace Symfony\Component\HttpKernel\Exception; class FatalErrorException extends \ErrorException { } + +namespace Symfony\Component\Debug\Exception; + +use Symfony\Component\HttpKernel\Exception\FatalErrorException as LegacyFatalErrorException; + +/** + * Fatal Error Exception. + * + * @author Konstanton Myakshin + */ +class FatalErrorException extends LegacyFatalErrorException +{ +} diff --git a/src/Symfony/Component/Debug/Exception/FlattenException.php b/src/Symfony/Component/Debug/Exception/FlattenException.php index 5777ec1321..14f5d1ea1d 100644 --- a/src/Symfony/Component/Debug/Exception/FlattenException.php +++ b/src/Symfony/Component/Debug/Exception/FlattenException.php @@ -9,6 +9,46 @@ * file that was distributed with this source code. */ +namespace Symfony\Component\HttpKernel\Exception; + +use Symfony\Component\Debug\Exception\FlattenException as DebugFlattenException; + +/** + * FlattenException wraps a PHP Exception to be able to serialize it. + * + * Basically, this class removes all objects from the trace. + * + * @author Fabien Potencier + * + * @deprecated Deprecated in 2.3, to be removed in 3.0. Use the same class from the Debug component instead. + */ +class FlattenException +{ + private $handler; + + public static function __callStatic($method, $args) + { + if (!method_exists('Symfony\Component\Debug\Exception\FlattenException', $method)) { + throw new \BadMethodCallException(sprintf('Call to undefined method %s::%s()', get_called_class(), $method)); + } + + return call_user_func_array(array('Symfony\Component\Debug\Exception\FlattenException', $method), $args); + } + + public function __call($method, $args) + { + if (!isset($this->handler)) { + $this->handler = new DebugFlattenException(); + } + + if (!method_exists($this->handler, $method)) { + throw new \BadMethodCallException(sprintf('Call to undefined method %s::%s()', get_class($this), $method)); + } + + return call_user_func_array(array($this->handler, $method), $args); + } +} + namespace Symfony\Component\Debug\Exception; use Symfony\Component\HttpKernel\Exception\FlattenException as LegacyFlattenException; @@ -279,35 +319,3 @@ class FlattenException extends LegacyFlattenException return $array['__PHP_Incomplete_Class_Name']; } } - -namespace Symfony\Component\HttpKernel\Exception; - -use Symfony\Component\Debug\Exception\FlattenException as DebugFlattenException; - -/** - * FlattenException wraps a PHP Exception to be able to serialize it. - * - * Basically, this class removes all objects from the trace. - * - * @author Fabien Potencier - * - * @deprecated Deprecated in 2.3, to be removed in 3.0. Use the same class from the Debug component instead. - */ -class FlattenException -{ - private $handler; - - public static function __callStatic($method, $args) - { - return forward_static_call_array(array('Symfony\Component\Debug\Exception\FlattenException', $method), $args); - } - - public function __call($method, $args) - { - if (!isset($this->handler)) { - $this->handler = new DebugFlattenException(); - } - - return call_user_func_array(array($this->handler, $method), $args); - } -} From 8fa056bc95abe1c323be5460fda446329a402185 Mon Sep 17 00:00:00 2001 From: pthompson Date: Fri, 16 Jan 2015 14:12:56 +0100 Subject: [PATCH 5/7] Inline private 'is quoting required' methods in Escaper --- src/Symfony/Component/Yaml/Escaper.php | 36 +++++++------------------- 1 file changed, 9 insertions(+), 27 deletions(-) diff --git a/src/Symfony/Component/Yaml/Escaper.php b/src/Symfony/Component/Yaml/Escaper.php index be846e50c7..2820778fea 100644 --- a/src/Symfony/Component/Yaml/Escaper.php +++ b/src/Symfony/Component/Yaml/Escaper.php @@ -72,7 +72,15 @@ class Escaper */ public static function requiresSingleQuoting($value) { - return self::containsCharRequiresSingleQuoting($value) || self::isValueRequiresSingleQuoting($value); + // Determines if the PHP value contains any single characters that would + // cause it to require single quoting in YAML. + if (preg_match('/[ \s \' " \: \{ \} \[ \] , & \* \# \?] | \A[ \- ? | < > = ! % @ ` ]/x', $value)) { + return true; + } + + // Determines if a PHP value is entirely composed of a value that would + // require single quoting in YAML. + return in_array(strtolower($value), array('null', '~', 'true', 'false', 'y', 'n', 'yes', 'no', 'on', 'off')); } /** @@ -86,30 +94,4 @@ class Escaper { return sprintf("'%s'", str_replace('\'', '\'\'', $value)); } - - /** - * Determines if a PHP value contains any single characters that would cause - * the value to require single quoting in YAML. - * - * @param string $value A PHP value - * @return bool True if the value would require single quotes. - */ - private static function containsCharRequiresSingleQuoting($value) - { - return preg_match('/[ \s \' " \: \{ \} \[ \] , & \* \# \?] | \A[ \- ? | < > = ! % @ ` ]/x', $value); - } - - /** - * Determines if a PHP value is entirely composed of a value that would - * require single quoting in YAML. - * - * @param string $value A PHP value - * @return bool True if the value would require single quotes. - */ - private static function isValueRequiresSingleQuoting($value) - { - // Note that whilst 'y' and 'n' are not supported as valid Booleans, - // they are escaped here for interoperability. - return in_array(strtolower($value), array('null', '~', 'true', 'false', 'y', 'n', 'yes', 'no', 'on', 'off')); - } } From 0a50b63da1d77d064e7d0db01872596243ee6820 Mon Sep 17 00:00:00 2001 From: Lorenz Schori Date: Tue, 6 Jan 2015 20:06:57 +0100 Subject: [PATCH 6/7] [EventDispatcher] Add missing checks to RegisterListenersPass * Support services using a parameter for their class name. * Prevent abstract services as event subscribers. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | --- .../RegisterListenersPass.php | 6 ++- .../RegisterListenersPassTest.php | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterListenersPass.php b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterListenersPass.php index 6c56634216..a46342c418 100644 --- a/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterListenersPass.php +++ b/src/Symfony/Component/HttpKernel/DependencyInjection/RegisterListenersPass.php @@ -91,8 +91,12 @@ class RegisterListenersPass implements CompilerPassInterface throw new \InvalidArgumentException(sprintf('The service "%s" must be public as event subscribers are lazy-loaded.', $id)); } + if ($def->isAbstract()) { + throw new \InvalidArgumentException(sprintf('The service "%s" must not be abstract as event subscribers are lazy-loaded.', $id)); + } + // We must assume that the class value has been correctly filled, even if the service is created by a factory - $class = $def->getClass(); + $class = $container->getParameterBag()->resolveValue($def->getClass()); $refClass = new \ReflectionClass($class); $interface = 'Symfony\Component\EventDispatcher\EventSubscriberInterface'; diff --git a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php index 2851e55541..61fecc9a87 100644 --- a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php @@ -135,6 +135,58 @@ class RegisterListenersPassTest extends \PHPUnit_Framework_TestCase $registerListenersPass = new RegisterListenersPass(); $registerListenersPass->process($container); } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage The service "foo" must not be abstract as event subscribers are lazy-loaded. + */ + public function testAbstractEventSubscriber() + { + $container = new ContainerBuilder(); + $container->register('foo', 'stdClass')->setAbstract(true)->addTag('kernel.event_subscriber', array()); + $container->register('event_dispatcher', 'stdClass'); + + $registerListenersPass = new RegisterListenersPass(); + $registerListenersPass->process($container); + } + + public function testEventSubscriberResolvableClassName() + { + $container = new ContainerBuilder(); + + $container->setParameter('subscriber.class', 'Symfony\Component\EventDispatcher\Tests\DependencyInjection\SubscriberService'); + $container->register('foo', '%subscriber.class%')->addTag('kernel.event_subscriber', array()); + $container->register('event_dispatcher', 'stdClass'); + + $registerListenersPass = new RegisterListenersPass(); + $registerListenersPass->process($container); + + $definition = $container->getDefinition('event_dispatcher'); + $expected_calls = array( + array( + 'addSubscriberService', + array( + 'foo', + 'Symfony\Component\EventDispatcher\Tests\DependencyInjection\SubscriberService', + ), + ), + ); + $this->assertSame($expected_calls, $definition->getMethodCalls()); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage You have requested a non-existent parameter "subscriber.class" + */ + public function testEventSubscriberUnresolvableClassName() + { + $container = new ContainerBuilder(); + $container->register('foo', '%subscriber.class%')->addTag('kernel.event_subscriber', array()); + $container->register('event_dispatcher', 'stdClass'); + + $registerListenersPass = new RegisterListenersPass(); + $registerListenersPass->process($container); + } } class SubscriberService implements \Symfony\Component\EventDispatcher\EventSubscriberInterface From b08115bd63d2bf0258b0bb9eaed9d62d3ed65c85 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 16 Jan 2015 15:49:28 +0100 Subject: [PATCH 7/7] fixed tests --- .../Tests/DependencyInjection/RegisterListenersPassTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php index 61fecc9a87..4c86797917 100644 --- a/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/DependencyInjection/RegisterListenersPassTest.php @@ -154,7 +154,7 @@ class RegisterListenersPassTest extends \PHPUnit_Framework_TestCase { $container = new ContainerBuilder(); - $container->setParameter('subscriber.class', 'Symfony\Component\EventDispatcher\Tests\DependencyInjection\SubscriberService'); + $container->setParameter('subscriber.class', 'Symfony\Component\HttpKernel\Tests\DependencyInjection\SubscriberService'); $container->register('foo', '%subscriber.class%')->addTag('kernel.event_subscriber', array()); $container->register('event_dispatcher', 'stdClass'); @@ -167,7 +167,7 @@ class RegisterListenersPassTest extends \PHPUnit_Framework_TestCase 'addSubscriberService', array( 'foo', - 'Symfony\Component\EventDispatcher\Tests\DependencyInjection\SubscriberService', + 'Symfony\Component\HttpKernel\Tests\DependencyInjection\SubscriberService', ), ), );