From 48338fcf10685f339ecfec8eb06d62d999a93e57 Mon Sep 17 00:00:00 2001 From: Emmanuel Vella Date: Thu, 25 Jul 2013 14:05:03 +0200 Subject: [PATCH 01/20] Ignore null value in comparison validators --- .../Validator/Constraints/AbstractComparisonValidator.php | 4 ++++ .../Validator/Tests/Constraints/EqualToValidatorTest.php | 3 ++- .../Tests/Constraints/GreaterThanOrEqualValidatorTest.php | 1 + .../Validator/Tests/Constraints/GreaterThanValidatorTest.php | 3 ++- .../Validator/Tests/Constraints/IdenticalToValidatorTest.php | 3 ++- .../Tests/Constraints/LessThanOrEqualValidatorTest.php | 1 + .../Validator/Tests/Constraints/LessThanValidatorTest.php | 3 ++- .../Validator/Tests/Constraints/NotEqualToValidatorTest.php | 3 ++- .../Tests/Constraints/NotIdenticalToValidatorTest.php | 3 ++- 9 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php b/src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php index 929283eaeb..6b76fc80b2 100644 --- a/src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php +++ b/src/Symfony/Component/Validator/Constraints/AbstractComparisonValidator.php @@ -26,6 +26,10 @@ abstract class AbstractComparisonValidator extends ConstraintValidator */ public function validate($value, Constraint $constraint) { + if (null === $value) { + return; + } + if (!$this->compareValues($value, $constraint->value, $constraint)) { $this->context->addViolation($constraint->message, array( '{{ value }}' => $this->valueToString($constraint->value), diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EqualToValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EqualToValidatorTest.php index 61189ed780..b47d9e6750 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/EqualToValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/EqualToValidatorTest.php @@ -39,7 +39,8 @@ class EqualToValidatorTest extends AbstractComparisonValidatorTestCase array(3, 3), array(3, '3'), array('a', 'a'), - array(new \DateTime('2000-01-01'), new \DateTime('2000-01-01')) + array(new \DateTime('2000-01-01'), new \DateTime('2000-01-01')), + array(null, 1), ); } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanOrEqualValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanOrEqualValidatorTest.php index 71bcd71954..2f97b03eb4 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanOrEqualValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanOrEqualValidatorTest.php @@ -41,6 +41,7 @@ class GreaterThanOrEqualValidatorTest extends AbstractComparisonValidatorTestCas array(new \DateTime('2000/01/01'), new \DateTime('2000/01/01')), array('a', 'a'), array('z', 'a'), + array(null, 1), ); } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanValidatorTest.php index a838c58b7c..72087fab83 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanValidatorTest.php @@ -37,7 +37,8 @@ class GreaterThanValidatorTest extends AbstractComparisonValidatorTestCase return array( array(2, 1), array(new \DateTime('2005/01/01'), new \DateTime('2001/01/01')), - array('333', '22') + array('333', '22'), + array(null, 1), ); } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/IdenticalToValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/IdenticalToValidatorTest.php index 58fdb7e589..a1f6a69f4a 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/IdenticalToValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/IdenticalToValidatorTest.php @@ -40,7 +40,8 @@ class IdenticalToValidatorTest extends AbstractComparisonValidatorTestCase return array( array(3, 3), array('a', 'a'), - array($date, $date) + array($date, $date), + array(null, 1), ); } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/LessThanOrEqualValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/LessThanOrEqualValidatorTest.php index 2614905a88..406e222119 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/LessThanOrEqualValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/LessThanOrEqualValidatorTest.php @@ -41,6 +41,7 @@ class LessThanOrEqualValidatorTest extends AbstractComparisonValidatorTestCase array(new \DateTime('2000-01-01'), new \DateTime('2020-01-01')), array('a', 'a'), array('a', 'z'), + array(null, 1), ); } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/LessThanValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/LessThanValidatorTest.php index 61af9c6a2e..f26269b3c2 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/LessThanValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/LessThanValidatorTest.php @@ -37,7 +37,8 @@ class LessThanValidatorTest extends AbstractComparisonValidatorTestCase return array( array(1, 2), array(new \DateTime('2000-01-01'), new \DateTime('2010-01-01')), - array('22', '333') + array('22', '333'), + array(null, 1), ); } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/NotEqualToValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/NotEqualToValidatorTest.php index d7c446a1e2..0122643a7e 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/NotEqualToValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/NotEqualToValidatorTest.php @@ -38,7 +38,8 @@ class NotEqualToValidatorTest extends AbstractComparisonValidatorTestCase return array( array(1, 2), array('22', '333'), - array(new \DateTime('2001-01-01'), new \DateTime('2000-01-01')) + array(new \DateTime('2001-01-01'), new \DateTime('2000-01-01')), + array(null, 1), ); } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/NotIdenticalToValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/NotIdenticalToValidatorTest.php index 48b1931d9f..462202dd5a 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/NotIdenticalToValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/NotIdenticalToValidatorTest.php @@ -40,7 +40,8 @@ class NotIdenticalToValidatorTest extends AbstractComparisonValidatorTestCase array('2', 2), array('22', '333'), array(new \DateTime('2001-01-01'), new \DateTime('2000-01-01')), - array(new \DateTime('2000-01-01'), new \DateTime('2000-01-01')) + array(new \DateTime('2000-01-01'), new \DateTime('2000-01-01')), + array(null, 1), ); } From 98f6969e9c2ebbb00d81bf9362f072cb200da38a Mon Sep 17 00:00:00 2001 From: Helmer Aaviksoo Date: Thu, 8 Aug 2013 01:27:53 +0300 Subject: [PATCH 02/20] Fix empty process argument escaping on Windows --- src/Symfony/Component/Process/ProcessUtils.php | 8 ++++++-- src/Symfony/Component/Process/Tests/ProcessUtilsTest.php | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Process/ProcessUtils.php b/src/Symfony/Component/Process/ProcessUtils.php index 42ed342afd..4a5b7d6073 100644 --- a/src/Symfony/Component/Process/ProcessUtils.php +++ b/src/Symfony/Component/Process/ProcessUtils.php @@ -41,11 +41,15 @@ class ProcessUtils //@see https://bugs.php.net/bug.php?id=43784 //@see https://bugs.php.net/bug.php?id=49446 if (defined('PHP_WINDOWS_VERSION_BUILD')) { + if ('' === $argument) { + return escapeshellarg($argument); + } + $escapedArgument = ''; foreach (preg_split('/([%"])/i', $argument, -1, PREG_SPLIT_NO_EMPTY | PREG_SPLIT_DELIM_CAPTURE) as $part) { - if ('"' == $part) { + if ('"' === $part) { $escapedArgument .= '\\"'; - } elseif ('%' == $part) { + } elseif ('%' === $part) { $escapedArgument .= '^%'; } else { $escapedArgument .= escapeshellarg($part); diff --git a/src/Symfony/Component/Process/Tests/ProcessUtilsTest.php b/src/Symfony/Component/Process/Tests/ProcessUtilsTest.php index e51da5a5e2..603fac53e7 100644 --- a/src/Symfony/Component/Process/Tests/ProcessUtilsTest.php +++ b/src/Symfony/Component/Process/Tests/ProcessUtilsTest.php @@ -30,6 +30,7 @@ class ProcessUtilsTest extends \PHPUnit_Framework_TestCase array('"foo bar"', 'foo bar'), array('^%"path"^%', '%path%'), array('"<|>"\\"" "\\""\'f"', '<|>" "\'f'), + array('""', ''), ); } @@ -37,6 +38,7 @@ class ProcessUtilsTest extends \PHPUnit_Framework_TestCase array("'foo bar'", 'foo bar'), array("'%path%'", '%path%'), array("'<|>\" \"'\\''f'", '<|>" "\'f'), + array("''", ''), ); } } From e47657dba1c5d929435d11f66aa31ed37c58cb5a Mon Sep 17 00:00:00 2001 From: ShiraNai7 Date: Wed, 7 Aug 2013 15:15:14 +0200 Subject: [PATCH 03/20] Make sure ContextErrorException is loaded during compile time errors --- src/Symfony/Component/Debug/ErrorHandler.php | 5 ++++ .../Debug/Tests/ErrorHandlerTest.php | 25 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/Symfony/Component/Debug/ErrorHandler.php b/src/Symfony/Component/Debug/ErrorHandler.php index dfb7dad003..52fd7a50f7 100644 --- a/src/Symfony/Component/Debug/ErrorHandler.php +++ b/src/Symfony/Component/Debug/ErrorHandler.php @@ -120,6 +120,11 @@ class ErrorHandler } if ($this->displayErrors && error_reporting() & $level && $this->level & $level) { + // make sure the ContextErrorException class is loaded (https://bugs.php.net/bug.php?id=65322) + if (!class_exists('Symfony\Component\Debug\Exception\ContextErrorException')) { + require __DIR__.'/Exception/ContextErrorException.php'; + } + throw new ContextErrorException(sprintf('%s: %s in %s line %d', isset($this->levels[$level]) ? $this->levels[$level] : $level, $message, $file, $line), 0, $level, $file, $line, $context); } diff --git a/src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php b/src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php index db06f6107e..24c422fe54 100644 --- a/src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php +++ b/src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php @@ -20,6 +20,31 @@ use Symfony\Component\Debug\ErrorHandler; */ class ErrorHandlerTest extends \PHPUnit_Framework_TestCase { + public function testCompileTimeError() + { + // the ContextErrorException must not be loaded for this test to work + if (class_exists('Symfony\Component\Debug\Exception\ContextErrorException', false)) { + $this->markTestSkipped('The ContextErrorException class is already loaded.'); + } + + $handler = ErrorHandler::register(E_ALL | E_STRICT); + $displayErrors = ini_get('display_errors'); + ini_set('display_errors', '1'); + + try { + // trigger compile time error + eval(<<<'PHP' +class _BaseCompileTimeError { function foo() {} } +class _CompileTimeError extends _BaseCompileTimeError { function foo($invalid) {} } +PHP + ); + } catch(\Exception $e) { + // if an exception is thrown, the test passed + } + + ini_set('display_errors', $displayErrors); + restore_error_handler(); + } public function testConstruct() { From 2769c9dbb147a4c05cb3c7d3c338dbc8200b5a2c Mon Sep 17 00:00:00 2001 From: Lee Rowlands Date: Sat, 18 May 2013 07:58:23 +1000 Subject: [PATCH 04/20] Use strstr instead of strpos --- src/Symfony/Component/ClassLoader/ClassLoader.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/ClassLoader/ClassLoader.php b/src/Symfony/Component/ClassLoader/ClassLoader.php index a2038a0d9f..9d3b72e3ee 100644 --- a/src/Symfony/Component/ClassLoader/ClassLoader.php +++ b/src/Symfony/Component/ClassLoader/ClassLoader.php @@ -181,7 +181,7 @@ class ClassLoader $classPath .= str_replace('_', DIRECTORY_SEPARATOR, $className) . '.php'; foreach ($this->prefixes as $prefix => $dirs) { - if (0 === strpos($class, $prefix)) { + if ($class === strstr($class, $prefix)) { foreach ($dirs as $dir) { if (file_exists($dir . DIRECTORY_SEPARATOR . $classPath)) { return $dir . DIRECTORY_SEPARATOR . $classPath; From 6ed0fdfda41c995726254ed9a9c8d69712438a1f Mon Sep 17 00:00:00 2001 From: Jakub Zalas Date: Mon, 24 Jun 2013 15:42:05 +0100 Subject: [PATCH 05/20] [Form] Moved auto_initialize option to the BaseType --- .../Component/Form/Extension/Core/Type/BaseType.php | 2 ++ .../Form/Extension/Core/Type/ButtonType.php | 13 +++++++++++++ .../Component/Form/Extension/Core/Type/FormType.php | 2 -- .../Tests/Extension/Core/Type/SubmitTypeTest.php | 9 +++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php b/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php index 79333a6799..a6f8c430e1 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php @@ -33,6 +33,7 @@ abstract class BaseType extends AbstractType public function buildForm(FormBuilderInterface $builder, array $options) { $builder->setDisabled($options['disabled']); + $builder->setAutoInitialize($options['auto_initialize']); } /** @@ -112,6 +113,7 @@ abstract class BaseType extends AbstractType 'label' => null, 'attr' => array(), 'translation_domain' => null, + 'auto_initialize' => true, )); $resolver->setAllowedTypes(array( diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ButtonType.php b/src/Symfony/Component/Form/Extension/Core/Type/ButtonType.php index 3569963bc4..75e95ab41e 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ButtonType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ButtonType.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Form\Extension\Core\Type; use Symfony\Component\Form\ButtonTypeInterface; +use Symfony\Component\OptionsResolver\OptionsResolverInterface; /** * A form button. @@ -35,4 +36,16 @@ class ButtonType extends BaseType implements ButtonTypeInterface { return 'button'; } + + /** + * {@inheritdoc} + */ + public function setDefaultOptions(OptionsResolverInterface $resolver) + { + parent::setDefaultOptions($resolver); + + $resolver->setDefaults(array( + 'auto_initialize' => false, + )); + } } diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php index eb1897cee4..190bb52cec 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php @@ -55,7 +55,6 @@ class FormType extends BaseType ->setDataMapper($options['compound'] ? new PropertyPathMapper($this->propertyAccessor) : null) ->setMethod($options['method']) ->setAction($options['action']) - ->setAutoInitialize($options['auto_initialize']) ; if ($options['trim']) { @@ -189,7 +188,6 @@ class FormType extends BaseType // According to RFC 2396 (http://www.ietf.org/rfc/rfc2396.txt) // section 4.2., empty URIs are considered same-document references 'action' => '', - 'auto_initialize' => true, )); $resolver->setAllowedTypes(array( diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/SubmitTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/SubmitTypeTest.php index 8cc72281b2..2319a4fe75 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/SubmitTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/SubmitTypeTest.php @@ -51,4 +51,13 @@ class SubmitTypeTest extends TypeTestCase $this->assertTrue($button->isClicked()); } + + public function testSubmitCanBeAddedToForm() + { + $form = $this->factory + ->createBuilder('form') + ->getForm(); + + $this->assertSame($form, $form->add('send', 'submit')); + } } From 7eaaec14684643074a701c7e7799fcad03996d96 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 9 Aug 2013 14:53:54 +0200 Subject: [PATCH 06/20] [FrameworkBundle] made code more generic --- src/Symfony/Bundle/FrameworkBundle/Console/Application.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Console/Application.php b/src/Symfony/Bundle/FrameworkBundle/Console/Application.php index aac3928e5d..9d7e4ff1a9 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Console/Application.php +++ b/src/Symfony/Bundle/FrameworkBundle/Console/Application.php @@ -11,7 +11,7 @@ namespace Symfony\Bundle\FrameworkBundle\Console; -use Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand; +use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\Console\Application as BaseApplication; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -78,7 +78,7 @@ class Application extends BaseApplication $container = $this->kernel->getContainer(); foreach ($this->all() as $command) { - if ($command instanceof ContainerAwareCommand) { + if ($command instanceof ContainerAwareInterface) { $command->setContainer($container); } } From 96aec0f49cbb29592e387d1845c8ccb1693b6c94 Mon Sep 17 00:00:00 2001 From: Dmitrii Chekaliuk Date: Wed, 1 May 2013 15:57:33 +0300 Subject: [PATCH 07/20] Fix internal sub-request creation Fixes the creation of internal sub-request in case of disabled trusted client-ip header --- .../Fragment/InlineFragmentRenderer.php | 7 ++-- .../Fragment/InlineFragmentRendererTest.php | 37 ++++++++++++++++++- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php b/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php index 2c8d58f956..25007bd115 100644 --- a/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php +++ b/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php @@ -111,10 +111,11 @@ class InlineFragmentRenderer extends RoutableFragmentRenderer // Sub-request object will point to localhost as client ip and real client ip // will be included into trusted header for client ip try { - $trustedHeaderName = Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP); - $currentXForwardedFor = $request->headers->get($trustedHeaderName, ''); + if ($trustedHeaderName = Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)) { + $currentXForwardedFor = $request->headers->get($trustedHeaderName, ''); - $server['HTTP_'.$trustedHeaderName] = ($currentXForwardedFor ? $currentXForwardedFor.', ' : '').$request->getClientIp(); + $server['HTTP_'.$trustedHeaderName] = ($currentXForwardedFor ? $currentXForwardedFor.', ' : '').$request->getClientIp(); + } } catch (\InvalidArgumentException $e) { // Do nothing } diff --git a/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php b/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php index f405dae09c..ded1071f9f 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fragment/InlineFragmentRendererTest.php @@ -67,6 +67,26 @@ class InlineFragmentRendererTest extends \PHPUnit_Framework_TestCase $strategy->render(new ControllerReference('main_controller', array('object' => $object), array()), Request::create('/')); } + public function testRenderWithTrustedHeaderDisabled() + { + $trustedHeaderName = Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP); + + Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, ''); + + $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface'); + $kernel + ->expects($this->any()) + ->method('handle') + ->with(Request::create('/')) + ; + + $strategy = new InlineFragmentRenderer($kernel); + + $strategy->render('/', Request::create('/')); + + Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, $trustedHeaderName); + } + /** * @expectedException \RuntimeException */ @@ -147,8 +167,11 @@ class InlineFragmentRendererTest extends \PHPUnit_Framework_TestCase { $expectedSubRequest = Request::create('/'); $expectedSubRequest->headers->set('Surrogate-Capability', 'abc="ESI/1.0"'); - $expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1')); - $expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1'); + + if (Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP)) { + $expectedSubRequest->headers->set('x-forwarded-for', array('127.0.0.1')); + $expectedSubRequest->server->set('HTTP_X_FORWARDED_FOR', '127.0.0.1'); + } $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface'); $kernel @@ -163,4 +186,14 @@ class InlineFragmentRendererTest extends \PHPUnit_Framework_TestCase $request->headers->set('Surrogate-Capability', 'abc="ESI/1.0"'); $strategy->render('/', $request); } + + public function testESIHeaderIsKeptInSubrequestWithTrustedHeaderDisabled() + { + $trustedHeaderName = Request::getTrustedHeaderName(Request::HEADER_CLIENT_IP); + Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, ''); + + $this->testESIHeaderIsKeptInSubrequest(); + + Request::setTrustedHeaderName(Request::HEADER_CLIENT_IP, $trustedHeaderName); + } } From e8e76ece5aeed8c3ca7da0741da9d93bc0ec5757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= Date: Sat, 10 Aug 2013 11:31:15 +0200 Subject: [PATCH 08/20] [TwigBridge] Prevent code extension to display warning --- src/Symfony/Bridge/Twig/Extension/CodeExtension.php | 4 +++- .../Bundle/FrameworkBundle/Templating/Helper/CodeHelper.php | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Extension/CodeExtension.php b/src/Symfony/Bridge/Twig/Extension/CodeExtension.php index 40809fb7b4..852bd3af84 100644 --- a/src/Symfony/Bridge/Twig/Extension/CodeExtension.php +++ b/src/Symfony/Bridge/Twig/Extension/CodeExtension.php @@ -137,7 +137,9 @@ class CodeExtension extends \Twig_Extension public function fileExcerpt($file, $line) { if (is_readable($file)) { - $code = highlight_file($file, true); + // highlight_file could throw warnings + // see https://bugs.php.net/bug.php?id=25725 + $code = @highlight_file($file, true); // remove main code/span tags $code = preg_replace('#^\s*(.*)\s*#s', '\\1', $code); $content = preg_split('#
#', $code); diff --git a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/CodeHelper.php b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/CodeHelper.php index 3c031d2c5b..edbf7a878a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/CodeHelper.php +++ b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/CodeHelper.php @@ -131,7 +131,9 @@ class CodeHelper extends Helper } } - $code = highlight_file($file, true); + // highlight_file could throw warnings + // see https://bugs.php.net/bug.php?id=25725 + $code = @highlight_file($file, true); // remove main code/span tags $code = preg_replace('#^\s*(.*)\s*#s', '\\1', $code); $content = preg_split('#
#', $code); From 18896d5a9eaf3ea1d2a6acc4bfff74a96c63295e Mon Sep 17 00:00:00 2001 From: marcj Date: Mon, 12 Aug 2013 23:30:49 +0200 Subject: [PATCH 09/20] [Validator] fixed the wrong isAbstract() check against the class (fixed #8589) --- .../Mapping/Loader/StaticMethodLoader.php | 6 +++++- .../Loader/AbstractMethodStaticLoader.php | 10 ++++++++++ .../Mapping/Loader/StaticMethodLoaderTest.php | 18 +++++++++++++++++- 3 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 src/Symfony/Component/Validator/Tests/Mapping/Loader/AbstractMethodStaticLoader.php diff --git a/src/Symfony/Component/Validator/Mapping/Loader/StaticMethodLoader.php b/src/Symfony/Component/Validator/Mapping/Loader/StaticMethodLoader.php index 673786752e..d8086b2b9c 100644 --- a/src/Symfony/Component/Validator/Mapping/Loader/StaticMethodLoader.php +++ b/src/Symfony/Component/Validator/Mapping/Loader/StaticMethodLoader.php @@ -31,9 +31,13 @@ class StaticMethodLoader implements LoaderInterface /** @var \ReflectionClass $reflClass */ $reflClass = $metadata->getReflectionClass(); - if (!$reflClass->isInterface() && !$reflClass->isAbstract() && $reflClass->hasMethod($this->methodName)) { + if (!$reflClass->isInterface() && $reflClass->hasMethod($this->methodName)) { $reflMethod = $reflClass->getMethod($this->methodName); + if ($reflMethod->isAbstract()) { + return false; + } + if (!$reflMethod->isStatic()) { throw new MappingException(sprintf('The method %s::%s should be static', $reflClass->name, $this->methodName)); } diff --git a/src/Symfony/Component/Validator/Tests/Mapping/Loader/AbstractMethodStaticLoader.php b/src/Symfony/Component/Validator/Tests/Mapping/Loader/AbstractMethodStaticLoader.php new file mode 100644 index 0000000000..3a1416cfc5 --- /dev/null +++ b/src/Symfony/Component/Validator/Tests/Mapping/Loader/AbstractMethodStaticLoader.php @@ -0,0 +1,10 @@ +assertCount(0, $metadata->getConstraints()); } - public function testLoadClassMetadataIgnoresAbstractClasses() + public function testLoadClassMetadataInAbstractClasses() { $loader = new StaticMethodLoader('loadMetadata'); $metadata = new ClassMetadata(__NAMESPACE__.'\AbstractStaticLoader'); $loader->loadClassMetadata($metadata); + $this->assertCount(1, $metadata->getConstraints()); + } + + public function testLoadClassMetadataIgnoresAbstractMethods() + { + $loader = new StaticMethodLoader('loadMetadata'); + try { + include __DIR__ . '/AbstractMethodStaticLoader.php'; + $this->fail('AbstractMethodStaticLoader should produce a strict standard error.'); + } catch (\Exception $e) { + } + + $metadata = new ClassMetadata(__NAMESPACE__.'\AbstractMethodStaticLoader'); + $loader->loadClassMetadata($metadata); + $this->assertCount(0, $metadata->getConstraints()); } } @@ -86,6 +101,7 @@ abstract class AbstractStaticLoader { public static function loadMetadata(ClassMetadata $metadata) { + $metadata->addConstraint(new ConstraintA()); } } From 9a29e5bade46c40377e3710cac1ec141bafc176d Mon Sep 17 00:00:00 2001 From: Jaik Dean Date: Mon, 12 Aug 2013 11:51:52 +0100 Subject: [PATCH 10/20] Fixed documentation grammar for AuthenticationManagerInterface::authenticate() --- .../Core/Authentication/AuthenticationManagerInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Core/Authentication/AuthenticationManagerInterface.php b/src/Symfony/Component/Security/Core/Authentication/AuthenticationManagerInterface.php index d8f4716002..c97d747d42 100644 --- a/src/Symfony/Component/Security/Core/Authentication/AuthenticationManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authentication/AuthenticationManagerInterface.php @@ -23,7 +23,7 @@ use Symfony\Component\Security\Core\Exception\AuthenticationException; interface AuthenticationManagerInterface { /** - * Attempts to authenticates a TokenInterface object. + * Attempts to authenticate a TokenInterface object. * * @param TokenInterface $token The TokenInterface instance to authenticate * From 572ba68338c6fd06fc522ef399f36a3156009a0b Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 13 Aug 2013 11:52:49 +0200 Subject: [PATCH 11/20] [TwigBridge] removed superflous ; when rendering form_enctype() (closes #8660) --- src/Symfony/Bridge/Twig/Node/FormEnctypeNode.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Node/FormEnctypeNode.php b/src/Symfony/Bridge/Twig/Node/FormEnctypeNode.php index 93bce1b9e6..0114b354d8 100644 --- a/src/Symfony/Bridge/Twig/Node/FormEnctypeNode.php +++ b/src/Symfony/Bridge/Twig/Node/FormEnctypeNode.php @@ -23,8 +23,6 @@ class FormEnctypeNode extends SearchAndRenderBlockNode { parent::compile($compiler); - $compiler->raw(";\n"); - // Uncomment this as soon as the deprecation note should be shown // $compiler->write('trigger_error(\'The helper form_enctype(form) is deprecated since version 2.3 and will be removed in 3.0. Use form_start(form) instead.\', E_USER_DEPRECATED)'); } From bff6f3c4a6639e151a0c5199661e3ed2c1ec0927 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 13 Aug 2013 15:36:51 +0200 Subject: [PATCH 12/20] [Process] Fix CS --- src/Symfony/Component/Process/Process.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index deabdacfe6..9f09407323 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -1122,8 +1122,8 @@ class Process /** * Handles the windows file handles fallbacks * - * @param callable $callback A valid PHP callback - * @param Boolean $closeEmptyHandles if true, handles that are empty will be assumed closed + * @param callable $callback A valid PHP callback + * @param Boolean $closeEmptyHandles if true, handles that are empty will be assumed closed */ private function processFileHandles($callback, $closeEmptyHandles = false) { From 7716fb25ab585b910222f3a6354da2e9a80129a8 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 13 Aug 2013 17:56:09 +0200 Subject: [PATCH 13/20] [Process] Add failing test for #8739 --- .../Process/Tests/AbstractProcessTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php index 6078b441da..5348f57a9c 100644 --- a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php +++ b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php @@ -70,6 +70,23 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase $this->assertLessThan(1.3, $duration); } + public function testCallbacksAreExecutedWithStart() + { + $data = ''; + + $process = $this->getProcess('echo "foo";sleep 1;echo "foo"'); + $process->start(function ($type, $buffer) use (&$data) { + $data .= $buffer; + }); + + $start = microtime(true); + while ($process->isRunning()) { + usleep(10000); + } + + $this->assertEquals("foo\nfoo\n", $data); + } + /** * tests results from sub processes * From 3ef517b356e5e39cba01f37887c8b5588ce2752d Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 13 Aug 2013 18:00:31 +0200 Subject: [PATCH 14/20] [Process] Fix #8739 --- src/Symfony/Component/Process/Process.php | 286 ++++++++++------------ 1 file changed, 136 insertions(+), 150 deletions(-) diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index 9f09407323..94ce991200 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -38,6 +38,7 @@ class Process // Timeout Precision in seconds. const TIMEOUT_PRECISION = 0.2; + private $callback; private $commandline; private $cwd; private $env; @@ -164,6 +165,7 @@ class Process public function __clone() { + $this->callback = null; $this->exitcode = null; $this->fallbackExitcode = null; $this->processInformation = null; @@ -199,7 +201,7 @@ class Process { $this->start($callback); - return $this->wait($callback); + return $this->wait(); } /** @@ -234,7 +236,7 @@ class Process $this->stderr = ''; $this->incrementalOutputOffset = 0; $this->incrementalErrorOutputOffset = 0; - $callback = $this->buildCallback($callback); + $this->callback = $this->buildCallback($callback); //Fix for PHP bug #51800: reading from STDOUT pipe hangs forever on Windows if the output is too big. //Workaround for this problem is to use temporary files instead of pipes on Windows platform. @@ -285,67 +287,9 @@ class Process stream_set_blocking($pipe, false); } - if (null === $this->stdin) { - fclose($this->pipes[0]); - unset($this->pipes[0]); - - return; - } - - $writePipes = array($this->pipes[0]); - unset($this->pipes[0]); - $stdinLen = strlen($this->stdin); - $stdinOffset = 0; - - while ($writePipes) { - if (defined('PHP_WINDOWS_VERSION_BUILD')) { - $this->processFileHandles($callback); - } - - $r = $this->pipes; - $w = $writePipes; - $e = null; - - if (false === $n = @stream_select($r, $w, $e, 0, ceil(static::TIMEOUT_PRECISION * 1E6))) { - // if a system call has been interrupted, forget about it, let's try again - if ($this->hasSystemCallBeenInterrupted()) { - continue; - } - break; - } - - // nothing has changed, let's wait until the process is ready - if (0 === $n) { - continue; - } - - if ($w) { - $written = fwrite($writePipes[0], (binary) substr($this->stdin, $stdinOffset), 8192); - if (false !== $written) { - $stdinOffset += $written; - } - if ($stdinOffset >= $stdinLen) { - fclose($writePipes[0]); - $writePipes = null; - } - } - - foreach ($r as $pipe) { - $type = array_search($pipe, $this->pipes); - $data = fread($pipe, 8192); - if (strlen($data) > 0) { - call_user_func($callback, $type == 1 ? self::OUT : self::ERR, $data); - } - if (false === $data || feof($pipe)) { - fclose($pipe); - unset($this->pipes[$type]); - } - } - - $this->checkTimeout(); - } - - $this->updateStatus(); + $this->writePipes(false); + $this->updateStatus(false); + $this->checkTimeout(); } /** @@ -391,55 +335,15 @@ class Process */ public function wait($callback = null) { - $this->updateStatus(); - $callback = $this->buildCallback($callback); - while ($this->pipes || (defined('PHP_WINDOWS_VERSION_BUILD') && $this->fileHandles)) { - if (defined('PHP_WINDOWS_VERSION_BUILD') && $this->fileHandles) { - $this->processFileHandles($callback, !$this->pipes); - } - $this->checkTimeout(); - - if ($this->pipes) { - $r = $this->pipes; - $w = null; - $e = null; - - // let's have a look if something changed in streams - if (false === $n = @stream_select($r, $w, $e, 0, ceil(static::TIMEOUT_PRECISION * 1E6))) { - // if a system call has been interrupted, forget about it, let's try again - // otherwise, an error occured, let's reset pipes - if (!$this->hasSystemCallBeenInterrupted()) { - $this->pipes = array(); - } - - continue; - } - - // nothing has changed - if (0 === $n) { - continue; - } - - foreach ($r as $pipe) { - $type = array_search($pipe, $this->pipes); - $data = fread($pipe, 8192); - - if (strlen($data) > 0) { - // last exit code is output and caught to work around --enable-sigchild - if (3 == $type) { - $this->fallbackExitcode = (int) $data; - } else { - call_user_func($callback, $type == 1 ? self::OUT : self::ERR, $data); - } - } - if (false === $data || feof($pipe)) { - fclose($pipe); - unset($this->pipes[$type]); - } - } - } + $this->updateStatus(false); + if (null !== $callback) { + $this->callback = $this->buildCallback($callback); } - $this->updateStatus(); + while ($this->processInformation['running']) { + $this->updateStatus(true); + $this->checkTimeout(); + } + $this->updateStatus(false); if ($this->processInformation['signaled']) { if ($this->isSigchildEnabled()) { throw new RuntimeException('The process has been signaled.'); @@ -482,7 +386,7 @@ class Process */ public function getOutput() { - $this->updateOutput(); + $this->readPipes(false); return $this->stdout; } @@ -514,7 +418,7 @@ class Process */ public function getErrorOutput() { - $this->updateErrorOutput(); + $this->readPipes(false); return $this->stderr; } @@ -553,7 +457,7 @@ class Process throw new RuntimeException('This PHP has been compiled with --enable-sigchild. You must use setEnhanceSigchildCompatibility() to use this method'); } - $this->updateStatus(); + $this->updateStatus(false); return $this->exitcode; } @@ -605,7 +509,7 @@ class Process throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved'); } - $this->updateStatus(); + $this->updateStatus(false); return $this->processInformation['signaled']; } @@ -627,7 +531,7 @@ class Process throw new RuntimeException('This PHP has been compiled with --enable-sigchild. Term signal can not be retrieved'); } - $this->updateStatus(); + $this->updateStatus(false); return $this->processInformation['termsig']; } @@ -643,7 +547,7 @@ class Process */ public function hasBeenStopped() { - $this->updateStatus(); + $this->updateStatus(false); return $this->processInformation['stopped']; } @@ -659,7 +563,7 @@ class Process */ public function getStopSignal() { - $this->updateStatus(); + $this->updateStatus(false); return $this->processInformation['stopsig']; } @@ -675,7 +579,7 @@ class Process return false; } - $this->updateStatus(); + $this->updateStatus(false); return $this->processInformation['running']; } @@ -697,7 +601,7 @@ class Process */ public function isTerminated() { - $this->updateStatus(); + $this->updateStatus(false); return $this->status == self::STATUS_TERMINATED; } @@ -711,7 +615,7 @@ class Process */ public function getStatus() { - $this->updateStatus(); + $this->updateStatus(false); return $this->status; } @@ -1062,14 +966,18 @@ class Process } /** - * Updates the status of the process. + * Updates the status of the process, reads pipes. + * + * @param Boolean $blocking Whether to use a clocking read call. */ - protected function updateStatus() + protected function updateStatus($blocking) { if (self::STATUS_STARTED !== $this->status) { return; } + $this->readPipes($blocking); + $this->processInformation = proc_get_status($this->process); if (!$this->processInformation['running']) { $this->status = self::STATUS_TERMINATED; @@ -1079,29 +987,6 @@ class Process } } - /** - * Updates the current error output of the process (STDERR). - */ - protected function updateErrorOutput() - { - if (isset($this->pipes[self::STDERR]) && is_resource($this->pipes[self::STDERR])) { - $this->addErrorOutput(stream_get_contents($this->pipes[self::STDERR])); - } - } - - /** - * Updates the current output of the process (STDOUT). - */ - protected function updateOutput() - { - if (defined('PHP_WINDOWS_VERSION_BUILD') && isset($this->fileHandles[self::STDOUT]) && is_resource($this->fileHandles[self::STDOUT])) { - fseek($this->fileHandles[self::STDOUT], $this->readBytes[self::STDOUT]); - $this->addOutput(stream_get_contents($this->fileHandles[self::STDOUT])); - } elseif (isset($this->pipes[self::STDOUT]) && is_resource($this->pipes[self::STDOUT])) { - $this->addOutput(stream_get_contents($this->pipes[self::STDOUT])); - } - } - /** * Return whether PHP has been compiled with the '--enable-sigchild' option or not * @@ -1122,10 +1007,9 @@ class Process /** * Handles the windows file handles fallbacks * - * @param callable $callback A valid PHP callback - * @param Boolean $closeEmptyHandles if true, handles that are empty will be assumed closed + * @param Boolean $closeEmptyHandles if true, handles that are empty will be assumed closed */ - private function processFileHandles($callback, $closeEmptyHandles = false) + private function processFileHandles($closeEmptyHandles = false) { $fh = $this->fileHandles; foreach ($fh as $type => $fileHandle) { @@ -1133,7 +1017,7 @@ class Process $data = fread($fileHandle, 8192); if (strlen($data) > 0) { $this->readBytes[$type] += strlen($data); - call_user_func($callback, $type == 1 ? self::OUT : self::ERR, $data); + call_user_func($this->callback, $type == 1 ? self::OUT : self::ERR, $data); } if (false === $data || ($closeEmptyHandles && '' === $data && feof($fileHandle))) { fclose($fileHandle); @@ -1154,4 +1038,106 @@ class Process // stream_select returns false when the `select` system call is interrupted by an incoming signal return isset($lastError['message']) && false !== stripos($lastError['message'], 'interrupted system call'); } + + /** + * Reads pipes, executes callback. + * + * @param Boolean $blocking Whether to use blocking calls or not. + */ + private function readPipes($blocking) + { + if (defined('PHP_WINDOWS_VERSION_BUILD') && $this->fileHandles) { + $this->processFileHandles(!$this->pipes); + } + + if ($this->pipes) { + $r = $this->pipes; + $w = null; + $e = null; + + // let's have a look if something changed in streams + if (false === $n = @stream_select($r, $w, $e, 0, $blocking ? ceil(self::TIMEOUT_PRECISION * 1E6) : 0)) { + // if a system call has been interrupted, forget about it, let's try again + // otherwise, an error occured, let's reset pipes + if (!$this->hasSystemCallBeenInterrupted()) { + $this->pipes = array(); + } + + return; + } + + // nothing has changed + if (0 === $n) { + return; + } + + foreach ($r as $pipe) { + $type = array_search($pipe, $this->pipes); + $data = fread($pipe, 8192); + + if (strlen($data) > 0) { + // last exit code is output and caught to work around --enable-sigchild + if (3 == $type) { + $this->fallbackExitcode = (int) $data; + } else { + call_user_func($this->callback, $type == 1 ? self::OUT : self::ERR, $data); + } + } + if (false === $data || feof($pipe)) { + fclose($pipe); + unset($this->pipes[$type]); + } + } + } + } + + /** + * Writes data to pipes. + * + * @param Boolean $blocking Whether to use blocking calls or not. + */ + private function writePipes($blocking) + { + if (null === $this->stdin) { + fclose($this->pipes[0]); + unset($this->pipes[0]); + + return; + } + + $writePipes = array($this->pipes[0]); + unset($this->pipes[0]); + $stdinLen = strlen($this->stdin); + $stdinOffset = 0; + + while ($writePipes) { + $r = array(); + $w = $writePipes; + $e = null; + + if (false === $n = @stream_select($r, $w, $e, 0, $blocking ? ceil(static::TIMEOUT_PRECISION * 1E6) : 0)) { + // if a system call has been interrupted, forget about it, let's try again + if ($this->hasSystemCallBeenInterrupted()) { + continue; + } + break; + } + + // nothing has changed, let's wait until the process is ready + if (0 === $n) { + continue; + } + + if ($w) { + $written = fwrite($writePipes[0], (binary) substr($this->stdin, $stdinOffset), 8192); + if (false !== $written) { + $stdinOffset += $written; + } + if ($stdinOffset >= $stdinLen) { + fclose($writePipes[0]); + $writePipes = null; + } + } + } + } } From 57d41591338ea3b1d8e6564c457fe5bb5662ca27 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 13 Aug 2013 18:01:05 +0200 Subject: [PATCH 15/20] [Process] Avoid zombie process in case of unit tests failure --- .../Component/Process/Tests/AbstractProcessTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php index 5348f57a9c..76002c1639 100644 --- a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php +++ b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php @@ -263,7 +263,7 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase public function testStop() { - $process = $this->getProcess('php -r "while (true) {}"'); + $process = $this->getProcess('php -r "sleep(4);"'); $process->start(); $this->assertTrue($process->isRunning()); $process->stop(); @@ -279,7 +279,7 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase public function testIsNotSuccessful() { - $process = $this->getProcess('php -r "while (true) {}"'); + $process = $this->getProcess('php -r "sleep(4);"'); $process->start(); $this->assertTrue($process->isRunning()); $process->stop(); @@ -314,7 +314,7 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase $this->markTestSkipped('Windows does not support POSIX signals'); } - $process = $this->getProcess('php -r "while (true) {}"'); + $process = $this->getProcess('php -r "sleep(4);"'); $process->start(); $process->stop(); $this->assertTrue($process->hasBeenSignaled()); @@ -329,7 +329,7 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase // SIGTERM is only defined if pcntl extension is present $termSignal = defined('SIGTERM') ? SIGTERM : 15; - $process = $this->getProcess('php -r "while (true) {}"'); + $process = $this->getProcess('php -r "sleep(4);"'); $process->start(); $process->stop(); @@ -360,7 +360,7 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase // Sleep doesn't work as it will allow the process to handle signals and close // file handles from the other end. - $process = $this->getProcess('php -r "while (true) {}"'); + $process = $this->getProcess('php -r "sleep 4"'); $process->start(); // PHP will deadlock when it tries to cleanup $process From fa769a2c2102fe4e831f2c90e5a8c0eb37c117bd Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 13 Aug 2013 19:23:51 +0200 Subject: [PATCH 16/20] [Process] Add more precision to Process::stop timeout --- src/Symfony/Component/Process/Process.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index 94ce991200..f769b8b129 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -631,12 +631,10 @@ class Process */ public function stop($timeout = 10) { - $timeoutMicro = (int) $timeout*1E6; + $timeoutMicro = microtime(true) + $timeout; if ($this->isRunning()) { proc_terminate($this->process); - $time = 0; - while (1 == $this->isRunning() && $time < $timeoutMicro) { - $time += 1000; + while ($this->isRunning() && microtime(true) < $timeoutMicro) { usleep(1000); } From d74eaf9603385a165ebb58b0d7d57d5f97943bea Mon Sep 17 00:00:00 2001 From: Douglas Greenshields Date: Tue, 13 Aug 2013 20:56:24 +0100 Subject: [PATCH 17/20] corrected English grammar (s/does not exists/does not exist) --- src/Symfony/Component/Finder/Shell/Command.php | 2 +- .../Component/HttpKernel/Profiler/ProfilerStorageInterface.php | 2 +- .../Component/Routing/Exception/RouteNotFoundException.php | 2 +- .../Component/Templating/Tests/Loader/ChainLoaderTest.php | 2 +- .../Component/Templating/Tests/Loader/FilesystemLoaderTest.php | 2 +- .../Component/Validator/Tests/Mapping/GetterMetadataTest.php | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/Finder/Shell/Command.php b/src/Symfony/Component/Finder/Shell/Command.php index 1a060c8cae..85e98043f0 100644 --- a/src/Symfony/Component/Finder/Shell/Command.php +++ b/src/Symfony/Component/Finder/Shell/Command.php @@ -182,7 +182,7 @@ class Command public function get($label) { if (!isset($this->labels[$label])) { - throw new \RuntimeException('Label "'.$label.'" does not exists.'); + throw new \RuntimeException('Label "'.$label.'" does not exist.'); } return $this->bits[$this->labels[$label]]; diff --git a/src/Symfony/Component/HttpKernel/Profiler/ProfilerStorageInterface.php b/src/Symfony/Component/HttpKernel/Profiler/ProfilerStorageInterface.php index 3bb0d68ffd..f4b9e5e212 100644 --- a/src/Symfony/Component/HttpKernel/Profiler/ProfilerStorageInterface.php +++ b/src/Symfony/Component/HttpKernel/Profiler/ProfilerStorageInterface.php @@ -35,7 +35,7 @@ interface ProfilerStorageInterface /** * Reads data associated with the given token. * - * The method returns false if the token does not exists in the storage. + * The method returns false if the token does not exist in the storage. * * @param string $token A token * diff --git a/src/Symfony/Component/Routing/Exception/RouteNotFoundException.php b/src/Symfony/Component/Routing/Exception/RouteNotFoundException.php index 4d5f288e7e..fd275bebe9 100644 --- a/src/Symfony/Component/Routing/Exception/RouteNotFoundException.php +++ b/src/Symfony/Component/Routing/Exception/RouteNotFoundException.php @@ -12,7 +12,7 @@ namespace Symfony\Component\Routing\Exception; /** - * Exception thrown when a route does not exists + * Exception thrown when a route does not exist * * @author Alexandre Salomé * diff --git a/src/Symfony/Component/Templating/Tests/Loader/ChainLoaderTest.php b/src/Symfony/Component/Templating/Tests/Loader/ChainLoaderTest.php index 3f3fc00df7..62a3dc2521 100644 --- a/src/Symfony/Component/Templating/Tests/Loader/ChainLoaderTest.php +++ b/src/Symfony/Component/Templating/Tests/Loader/ChainLoaderTest.php @@ -45,7 +45,7 @@ class ChainLoaderTest extends \PHPUnit_Framework_TestCase { $loader = new ProjectTemplateLoader1(array($this->loader1, $this->loader2)); $this->assertFalse($loader->load(new TemplateReference('bar', 'php')), '->load() returns false if the template is not found'); - $this->assertFalse($loader->load(new TemplateReference('foo', 'php')), '->load() returns false if the template does not exists for the given renderer'); + $this->assertFalse($loader->load(new TemplateReference('foo', 'php')), '->load() returns false if the template does not exist for the given renderer'); $this->assertInstanceOf( 'Symfony\Component\Templating\Storage\FileStorage', $loader->load(new TemplateReference('foo.php', 'php')), diff --git a/src/Symfony/Component/Templating/Tests/Loader/FilesystemLoaderTest.php b/src/Symfony/Component/Templating/Tests/Loader/FilesystemLoaderTest.php index 3168a62289..8eb663637d 100644 --- a/src/Symfony/Component/Templating/Tests/Loader/FilesystemLoaderTest.php +++ b/src/Symfony/Component/Templating/Tests/Loader/FilesystemLoaderTest.php @@ -61,7 +61,7 @@ class FilesystemLoaderTest extends \PHPUnit_Framework_TestCase $loader = new ProjectTemplateLoader2($pathPattern); $loader->setDebugger($debugger = new \Symfony\Component\Templating\Tests\Fixtures\ProjectTemplateDebugger()); - $this->assertFalse($loader->load(new TemplateReference('foo.xml', 'php')), '->load() returns false if the template does not exists for the given engine'); + $this->assertFalse($loader->load(new TemplateReference('foo.xml', 'php')), '->load() returns false if the template does not exist for the given engine'); $this->assertTrue($debugger->hasMessage('Failed loading template'), '->load() logs a "Failed loading template" message if the template is not found'); $loader = new ProjectTemplateLoader2(array(self::$fixturesPath.'/null/%name%', $pathPattern)); diff --git a/src/Symfony/Component/Validator/Tests/Mapping/GetterMetadataTest.php b/src/Symfony/Component/Validator/Tests/Mapping/GetterMetadataTest.php index d38225aa2e..8682ab050e 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/GetterMetadataTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/GetterMetadataTest.php @@ -28,7 +28,7 @@ class GetterMetadataTest extends \PHPUnit_Framework_TestCase public function testGetPropertyValueFromPublicGetter() { // private getters don't work yet because ReflectionMethod::setAccessible() - // does not exists yet in a stable PHP release + // does not exist yet in a stable PHP release $entity = new Entity('foobar'); $metadata = new GetterMetadata(self::CLASSNAME, 'internal'); From 909fab6979c8a7880897fc6a8b9d5bc57983ab6b Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 13 Aug 2013 20:21:22 +0200 Subject: [PATCH 18/20] [Process] Fix #8742 : Signal-terminated processes are not successful --- src/Symfony/Component/Process/Process.php | 77 +++++++++++++------ .../Process/Tests/AbstractProcessTest.php | 18 +++++ .../Tests/SigchildDisabledProcessTest.php | 5 ++ .../Tests/SigchildEnabledProcessTest.php | 5 ++ 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index 6f345bc57f..c8772d9ae7 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -332,8 +332,6 @@ class Process usleep(1000); } - $exitcode = proc_close($this->process); - if ($this->processInformation['signaled']) { if ($this->isSigchildEnabled()) { throw new RuntimeException('The process has been signaled.'); @@ -342,12 +340,6 @@ class Process throw new RuntimeException(sprintf('The process has been signaled with signal "%s".', $this->processInformation['termsig'])); } - $this->exitcode = $this->processInformation['running'] ? $exitcode : $this->processInformation['exitcode']; - - if (-1 == $this->exitcode && null !== $this->fallbackExitcode) { - $this->exitcode = $this->fallbackExitcode; - } - return $this->exitcode; } @@ -664,20 +656,7 @@ class Process } } - foreach ($this->pipes as $pipe) { - fclose($pipe); - } - $this->pipes = array(); - - $exitcode = proc_close($this->process); - $this->exitcode = -1 === $this->processInformation['exitcode'] ? $exitcode : $this->processInformation['exitcode']; - - if (defined('PHP_WINDOWS_VERSION_BUILD')) { - foreach ($this->fileHandles as $fileHandle) { - fclose($fileHandle); - } - $this->fileHandles = array(); - } + $this->updateStatus(false); } $this->status = self::STATUS_TERMINATED; @@ -1070,11 +1049,10 @@ class Process $this->readPipes($blocking); $this->processInformation = proc_get_status($this->process); + $this->captureExitCode(); if (!$this->processInformation['running']) { + $this->close(); $this->status = self::STATUS_TERMINATED; - if (-1 !== $this->processInformation['exitcode']) { - $this->exitcode = $this->processInformation['exitcode']; - } } } @@ -1237,4 +1215,53 @@ class Process } } } + + /** + * Captures the exitcode if mentioned in the process informations. + */ + private function captureExitCode() + { + if (isset($this->processInformation['exitcode']) && -1 != $this->processInformation['exitcode']) { + $this->exitcode = $this->processInformation['exitcode']; + } + } + + + /** + * Closes process resource, closes file handles, sets the exitcode. + * + * @return Integer The exitcode + */ + private function close() + { + foreach ($this->pipes as $pipe) { + fclose($pipe); + } + + $this->pipes = null; + $exitcode = -1; + + if (is_resource($this->process)) { + $exitcode = proc_close($this->process); + } + + $this->exitcode = $this->exitcode !== null ? $this->exitcode : -1; + $this->exitcode = -1 != $exitcode ? $exitcode : $this->exitcode; + + if (-1 == $this->exitcode && null !== $this->fallbackExitcode) { + $this->exitcode = $this->fallbackExitcode; + } elseif (-1 === $this->exitcode && $this->processInformation['signaled'] && 0 < $this->processInformation['termsig']) { + // if process has been signaled, no exitcode but a valid termsig, apply unix convention + $this->exitcode = 128 + $this->processInformation['termsig']; + } + + if (defined('PHP_WINDOWS_VERSION_BUILD')) { + foreach ($this->fileHandles as $fileHandle) { + fclose($fileHandle); + } + $this->fileHandles = array(); + } + + return $this->exitcode; + } } diff --git a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php index 53f79c5b55..a3b8c2a937 100644 --- a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php +++ b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php @@ -483,6 +483,24 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase $this->assertEquals('Caught SIGUSR1', $process->getOutput()); } + public function testExitCodeIsAvailableAfterSignal() + { + $this->verifyPosixIsEnabled(); + + $process = $this->getProcess('sleep 4'); + $process->start(); + $process->signal(SIGKILL); + + while ($process->isRunning()) { + usleep(10000); + } + + $this->assertFalse($process->isRunning()); + $this->assertTrue($process->hasBeenSignaled()); + $this->assertFalse($process->isSuccessful()); + $this->assertEquals(137, $process->getExitCode()); + } + /** * @expectedException Symfony\Component\Process\Exception\LogicException */ diff --git a/src/Symfony/Component/Process/Tests/SigchildDisabledProcessTest.php b/src/Symfony/Component/Process/Tests/SigchildDisabledProcessTest.php index 2e364d6392..8779263736 100644 --- a/src/Symfony/Component/Process/Tests/SigchildDisabledProcessTest.php +++ b/src/Symfony/Component/Process/Tests/SigchildDisabledProcessTest.php @@ -138,6 +138,11 @@ class SigchildDisabledProcessTest extends AbstractProcessTest $this->markTestSkipped('Retrieving Pid is not supported in sigchild environment'); } + public function testExitCodeIsAvailableAfterSignal() + { + $this->markTestSkipped('Signal is not supported in sigchild environment'); + } + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Process/Tests/SigchildEnabledProcessTest.php b/src/Symfony/Component/Process/Tests/SigchildEnabledProcessTest.php index 40ffb772b7..296b00dfcb 100644 --- a/src/Symfony/Component/Process/Tests/SigchildEnabledProcessTest.php +++ b/src/Symfony/Component/Process/Tests/SigchildEnabledProcessTest.php @@ -98,6 +98,11 @@ class SigchildEnabledProcessTest extends AbstractProcessTest $this->markTestSkipped('Retrieving Pid is not supported in sigchild environment'); } + public function testExitCodeIsAvailableAfterSignal() + { + $this->markTestSkipped('Signal is not supported in sigchild environment'); + } + /** * {@inheritdoc} */ From 8d9c7c6faed34deab2fed9a1018c1beb0ee1da81 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Wed, 14 Aug 2013 12:09:40 +0200 Subject: [PATCH 19/20] [Process] Fix #8746 : slowness added in unit tests since #8741 --- src/Symfony/Component/Process/Process.php | 60 ++++++++++++++--------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index f769b8b129..91634297d8 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -287,7 +287,7 @@ class Process stream_set_blocking($pipe, false); } - $this->writePipes(false); + $this->writePipes(); $this->updateStatus(false); $this->checkTimeout(); } @@ -339,9 +339,9 @@ class Process if (null !== $callback) { $this->callback = $this->buildCallback($callback); } - while ($this->processInformation['running']) { - $this->updateStatus(true); + while ($this->pipes || (defined('PHP_WINDOWS_VERSION_BUILD') && $this->fileHandles)) { $this->checkTimeout(); + $this->readPipes(true); } $this->updateStatus(false); if ($this->processInformation['signaled']) { @@ -1069,23 +1069,7 @@ class Process return; } - foreach ($r as $pipe) { - $type = array_search($pipe, $this->pipes); - $data = fread($pipe, 8192); - - if (strlen($data) > 0) { - // last exit code is output and caught to work around --enable-sigchild - if (3 == $type) { - $this->fallbackExitcode = (int) $data; - } else { - call_user_func($this->callback, $type == 1 ? self::OUT : self::ERR, $data); - } - } - if (false === $data || feof($pipe)) { - fclose($pipe); - unset($this->pipes[$type]); - } - } + $this->processReadPipes($r); } } @@ -1094,7 +1078,7 @@ class Process * * @param Boolean $blocking Whether to use blocking calls or not. */ - private function writePipes($blocking) + private function writePipes() { if (null === $this->stdin) { fclose($this->pipes[0]); @@ -1109,7 +1093,11 @@ class Process $stdinOffset = 0; while ($writePipes) { - $r = array(); + if (defined('PHP_WINDOWS_VERSION_BUILD')) { + $this->processFileHandles(); + } + + $r = $this->pipes; $w = $writePipes; $e = null; @@ -1136,6 +1124,34 @@ class Process $writePipes = null; } } + + $this->processReadPipes($r); + } + } + + /** + * Processes read pipes, executes callback on it. + * + * @param array $pipes + */ + private function processReadPipes(array $pipes) + { + foreach ($pipes as $pipe) { + $type = array_search($pipe, $this->pipes); + $data = fread($pipe, 8192); + + if (strlen($data) > 0) { + // last exit code is output and caught to work around --enable-sigchild + if (3 == $type) { + $this->fallbackExitcode = (int) $data; + } else { + call_user_func($this->callback, $type == 1 ? self::OUT : self::ERR, $data); + } + } + if (false === $data || feof($pipe)) { + fclose($pipe); + unset($this->pipes[$type]); + } } } } From 8c4bae35925c2baae7e121b86ee0a08c2c95a912 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Wed, 14 Aug 2013 13:29:25 +0200 Subject: [PATCH 20/20] [Process] Revert change --- src/Symfony/Component/Process/Tests/AbstractProcessTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php index 76002c1639..1021746e76 100644 --- a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php +++ b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php @@ -360,7 +360,7 @@ abstract class AbstractProcessTest extends \PHPUnit_Framework_TestCase // Sleep doesn't work as it will allow the process to handle signals and close // file handles from the other end. - $process = $this->getProcess('php -r "sleep 4"'); + $process = $this->getProcess('php -r "while (true) {}"'); $process->start(); // PHP will deadlock when it tries to cleanup $process