From 317da3bdf83c5834dde2fc850509d39da8435472 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Fri, 26 Jan 2018 17:21:59 +0100 Subject: [PATCH 1/5] keep the context when validating forms --- .../Form/Extension/Validator/Constraints/FormValidator.php | 2 ++ .../Tests/Extension/Validator/Constraints/FormValidatorTest.php | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php index bddb8a09e8..4183e1a726 100644 --- a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php +++ b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php @@ -124,6 +124,7 @@ class FormValidator extends ConstraintValidator : gettype($form->getViewData()); if ($this->context instanceof ExecutionContextInterface) { + $this->context->setConstraint($constraint); $this->context->buildViolation($config->getOption('invalid_message')) ->setParameters(array_replace(array('{{ value }}' => $clientDataAsString), $config->getOption('invalid_message_parameters'))) ->setInvalidValue($form->getViewData()) @@ -144,6 +145,7 @@ class FormValidator extends ConstraintValidator // Mark the form with an error if it contains extra fields if (!$config->getOption('allow_extra_fields') && count($form->getExtraData()) > 0) { if ($this->context instanceof ExecutionContextInterface) { + $this->context->setConstraint($constraint); $this->context->buildViolation($config->getOption('extra_fields_message')) ->setParameter('{{ extra_fields }}', implode('", "', array_keys($form->getExtraData()))) ->setInvalidValue($form->getExtraData()) diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php index 78015b5049..4bb360a99f 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php @@ -51,6 +51,8 @@ class FormValidatorTest extends AbstractConstraintValidatorTest $this->serverParams = $this->getMockBuilder('Symfony\Component\Form\Extension\Validator\Util\ServerParams')->setMethods(array('getNormalizedIniPostMaxSize', 'getContentLength'))->getMock(); parent::setUp(); + + $this->constraint = new Form(); } protected function getApiVersion() From 35f98e208962d2d40f91ada5c3c994d2edc79b83 Mon Sep 17 00:00:00 2001 From: Greg Anderson Date: Fri, 19 Jan 2018 15:19:59 -0800 Subject: [PATCH 2/5] Follow-on to #25825: Fix edge case in getParameterOption. Add one test to demonstrate getParameterOption fix. Add several tests to demonstrate current limitations in hasParameterOption. --- .../Component/Console/Input/ArgvInput.php | 19 +++++++---- .../Console/Input/InputInterface.php | 4 +++ .../Console/Tests/Input/ArgvInputTest.php | 32 +++++++++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/Console/Input/ArgvInput.php b/src/Symfony/Component/Console/Input/ArgvInput.php index c2f533eb36..da064598d0 100644 --- a/src/Symfony/Component/Console/Input/ArgvInput.php +++ b/src/Symfony/Component/Console/Input/ArgvInput.php @@ -279,7 +279,11 @@ class ArgvInput extends Input foreach ($this->tokens as $token) { foreach ($values as $value) { - if ($token === $value || 0 === strpos($token, $value.'=')) { + // Options with values: + // For long options, test for '--option=' at beginning + // For short options, test for '-o' at beginning + $leading = 0 === strpos($value, '--') ? $value.'=' : $value; + if ($token === $value || 0 === strpos($token, $leading)) { return true; } } @@ -300,13 +304,16 @@ class ArgvInput extends Input $token = array_shift($tokens); foreach ($values as $value) { - if ($token === $value || 0 === strpos($token, $value.'=')) { - if (false !== $pos = strpos($token, '=')) { - return substr($token, $pos + 1); - } - + if ($token === $value) { return array_shift($tokens); } + // Options with values: + // For long options, test for '--option=' at beginning + // For short options, test for '-o' at beginning + $leading = 0 === strpos($value, '--') ? $value.'=' : $value; + if (0 === strpos($token, $leading)) { + return substr($token, strlen($leading)); + } } } diff --git a/src/Symfony/Component/Console/Input/InputInterface.php b/src/Symfony/Component/Console/Input/InputInterface.php index 9de6e3a409..ac14de7d6f 100644 --- a/src/Symfony/Component/Console/Input/InputInterface.php +++ b/src/Symfony/Component/Console/Input/InputInterface.php @@ -30,6 +30,8 @@ interface InputInterface * * This method is to be used to introspect the input parameters * before they have been validated. It must be used carefully. + * Does not necessarily return the correct result for short options + * when multiple flags are combined in the same option. * * @param string|array $values The values to look for in the raw parameters (can be an array) * @@ -42,6 +44,8 @@ interface InputInterface * * This method is to be used to introspect the input parameters * before they have been validated. It must be used carefully. + * Does not necessarily return the correct result for short options + * when multiple flags are combined in the same option. * * @param string|array $values The value(s) to look for in the raw parameters (can be an array) * @param mixed $default The default value to return if no result is found diff --git a/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php b/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php index 1fe21d0f6c..04f5ce3157 100644 --- a/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php +++ b/src/Symfony/Component/Console/Tests/Input/ArgvInputTest.php @@ -296,6 +296,10 @@ class ArgvInputTest extends TestCase $input = new ArgvInput(array('cli.php', '-f', 'foo')); $this->assertTrue($input->hasParameterOption('-f'), '->hasParameterOption() returns true if the given short option is in the raw input'); + $input = new ArgvInput(array('cli.php', '-etest')); + $this->assertTrue($input->hasParameterOption('-e'), '->hasParameterOption() returns true if the given short option is in the raw input'); + $this->assertFalse($input->hasParameterOption('-s'), '->hasParameterOption() returns true if the given short option is in the raw input'); + $input = new ArgvInput(array('cli.php', '--foo', 'foo')); $this->assertTrue($input->hasParameterOption('--foo'), '->hasParameterOption() returns true if the given short option is in the raw input'); @@ -306,6 +310,33 @@ class ArgvInputTest extends TestCase $this->assertTrue($input->hasParameterOption('--foo'), '->hasParameterOption() returns true if the given option with provided value is in the raw input'); } + public function testHasParameterOptionEdgeCasesAndLimitations() + { + $input = new ArgvInput(array('cli.php', '-fh')); + // hasParameterOption does not know if the previous short option, -f, + // takes a value or not. If -f takes a value, then -fh does NOT include + // -h; Otherwise it does. Since we do not know which short options take + // values, hasParameterOption does not support this use-case. + $this->assertFalse($input->hasParameterOption('-h'), '->hasParameterOption() returns true if the given short option is in the raw input'); + // hasParameterOption does detect that `-fh` contains `-f`, since + // `-f` is the first short option in the set. + $this->assertTrue($input->hasParameterOption('-f'), '->hasParameterOption() returns true if the given short option is in the raw input'); + // The test below happens to pass, although it might make more sense + // to disallow it, and require the use of + // $input->hasParameterOption('-f') && $input->hasParameterOption('-h') + // instead. + $this->assertTrue($input->hasParameterOption('-fh'), '->hasParameterOption() returns true if the given short option is in the raw input'); + // In theory, if -fh is supported, then -hf should also work. + // However, this is not supported. + $this->assertFalse($input->hasParameterOption('-hf'), '->hasParameterOption() returns true if the given short option is in the raw input'); + + $input = new ArgvInput(array('cli.php', '-f', '-h')); + // If hasParameterOption('-fh') is supported for 'cli.php -fh', then + // one might also expect that it should also be supported for + // 'cli.php -f -h'. However, this is not supported. + $this->assertFalse($input->hasParameterOption('-fh'), '->hasParameterOption() returns true if the given short option is in the raw input'); + } + public function testToString() { $input = new ArgvInput(array('cli.php', '-f', 'foo')); @@ -327,6 +358,7 @@ class ArgvInputTest extends TestCase public function provideGetParameterOptionValues() { return array( + array(array('app/console', 'foo:bar', '-edev'), '-e', 'dev'), array(array('app/console', 'foo:bar', '-e', 'dev'), '-e', 'dev'), array(array('app/console', 'foo:bar', '--env=dev'), '--env', 'dev'), array(array('app/console', 'foo:bar', '-e', 'dev'), array('-e', '--env'), 'dev'), From f167b505c427625fc283a1720be76a83262ae9e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 5 Feb 2018 08:51:00 +0100 Subject: [PATCH 3/5] [DI] minor: use a strict comparision in setDecoratedService --- src/Symfony/Component/DependencyInjection/Definition.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index 6f2a3c7e82..8286e9d1e7 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -144,7 +144,7 @@ class Definition */ public function setDecoratedService($id, $renamedId = null) { - if ($renamedId && $id == $renamedId) { + if ($renamedId && $id === $renamedId) { throw new \InvalidArgumentException(sprintf('The decorated service inner name for "%s" must be different than the service name itself.', $id)); } From 899ead2b66639c8fdc49c2ce1e1e80424a27943d Mon Sep 17 00:00:00 2001 From: Gabriel Caruso Date: Tue, 6 Feb 2018 07:47:25 -0200 Subject: [PATCH 4/5] Fix misspelling variable --- src/Symfony/Component/HttpFoundation/Tests/RequestTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php index 5dad405761..96f87958ed 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/RequestTest.php @@ -1982,11 +1982,11 @@ class RequestTest extends TestCase /** * @dataProvider methodCacheableProvider */ - public function testMethodCacheable($method, $chacheable) + public function testMethodCacheable($method, $cacheable) { $request = new Request(); $request->setMethod($method); - $this->assertEquals($chacheable, $request->isMethodCacheable()); + $this->assertEquals($cacheable, $request->isMethodCacheable()); } public function methodCacheableProvider() From d195a6f0600fdfb0665f4a7844dc32646f73b1ab Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Fri, 9 Feb 2018 13:55:03 +0100 Subject: [PATCH 5/5] [SecurityBundle] Backport test --- ...amePasswordFormAuthenticationListenerTest.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php index 2e99f70e7e..8634eca017 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/UsernamePasswordFormAuthenticationListenerTest.php @@ -77,14 +77,14 @@ class UsernamePasswordFormAuthenticationListenerTest extends TestCase } /** + * @dataProvider postOnlyDataProvider * @expectedException \Symfony\Component\HttpKernel\Exception\BadRequestHttpException * @expectedExceptionMessage The key "_username" must be a string, "array" given. */ - public function testHandleNonStringUsername() + public function testHandleNonStringUsername($postOnly) { $request = Request::create('/login_check', 'POST', array('_username' => array())); $request->setSession($this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock()); - $listener = new UsernamePasswordFormAuthenticationListener( new TokenStorage(), $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface')->getMock(), @@ -93,14 +93,20 @@ class UsernamePasswordFormAuthenticationListenerTest extends TestCase 'foo', new DefaultAuthenticationSuccessHandler($httpUtils), new DefaultAuthenticationFailureHandler($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $httpUtils), - array('require_previous_session' => false) + array('require_previous_session' => false, 'post_only' => $postOnly) ); - $event = new GetResponseEvent($this->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')->getMock(), $request, HttpKernelInterface::MASTER_REQUEST); - $listener->handle($event); } + public function postOnlyDataProvider() + { + return array( + array(true), + array(false), + ); + } + public function getUsernameForLength() { return array(