From a08ddf76361899ebe6f171611a1832da3e01a16a Mon Sep 17 00:00:00 2001 From: Maxime Steinhausser Date: Fri, 21 Feb 2020 15:30:21 +0100 Subject: [PATCH 1/4] [Validator] Add target guards for Composite nested constraints --- .../Validator/Constraints/Composite.php | 11 +++++ .../Validator/Mapping/ClassMetadata.php | 18 +++++-- .../Validator/Mapping/MemberMetadata.php | 18 +++++-- .../Tests/Mapping/ClassMetadataTest.php | 37 ++++++++++++++ .../Tests/Mapping/MemberMetadataTest.php | 48 +++++++++++++++++++ 5 files changed, 126 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/Composite.php b/src/Symfony/Component/Validator/Constraints/Composite.php index b14276c3bb..d8c6079ee2 100644 --- a/src/Symfony/Component/Validator/Constraints/Composite.php +++ b/src/Symfony/Component/Validator/Constraints/Composite.php @@ -136,6 +136,17 @@ abstract class Composite extends Constraint */ abstract protected function getCompositeOption(); + /** + * @internal Used by metadata + * + * @return Constraint[] + */ + public function getNestedContraints() + { + /* @var Constraint[] $nestedConstraints */ + return $this->{$this->getCompositeOption()}; + } + /** * Initializes the nested constraints. * diff --git a/src/Symfony/Component/Validator/Mapping/ClassMetadata.php b/src/Symfony/Component/Validator/Mapping/ClassMetadata.php index 572bbc6d55..cf8782c085 100644 --- a/src/Symfony/Component/Validator/Mapping/ClassMetadata.php +++ b/src/Symfony/Component/Validator/Mapping/ClassMetadata.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Validator\Mapping; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Constraints\Composite; use Symfony\Component\Validator\Constraints\GroupSequence; use Symfony\Component\Validator\Constraints\Traverse; use Symfony\Component\Validator\Exception\ConstraintDefinitionException; @@ -178,9 +179,7 @@ class ClassMetadata extends GenericMetadata implements ClassMetadataInterface */ public function addConstraint(Constraint $constraint) { - if (!\in_array(Constraint::CLASS_CONSTRAINT, (array) $constraint->getTargets())) { - throw new ConstraintDefinitionException(sprintf('The constraint "%s" cannot be put on classes.', \get_class($constraint))); - } + $this->checkConstraint($constraint); if ($constraint instanceof Traverse) { if ($constraint->traverse) { @@ -495,4 +494,17 @@ class ClassMetadata extends GenericMetadata implements ClassMetadataInterface $this->members[$property][] = $metadata; } + + private function checkConstraint(Constraint $constraint) + { + if (!\in_array(Constraint::CLASS_CONSTRAINT, (array) $constraint->getTargets(), true)) { + throw new ConstraintDefinitionException(sprintf('The constraint "%s" cannot be put on classes.', \get_class($constraint))); + } + + if ($constraint instanceof Composite) { + foreach ($constraint->getNestedContraints() as $nestedContraint) { + $this->checkConstraint($nestedContraint); + } + } + } } diff --git a/src/Symfony/Component/Validator/Mapping/MemberMetadata.php b/src/Symfony/Component/Validator/Mapping/MemberMetadata.php index fb2fcb439f..124e75a0ee 100644 --- a/src/Symfony/Component/Validator/Mapping/MemberMetadata.php +++ b/src/Symfony/Component/Validator/Mapping/MemberMetadata.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Validator\Mapping; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Constraints\Composite; use Symfony\Component\Validator\Exception\ConstraintDefinitionException; /** @@ -71,9 +72,7 @@ abstract class MemberMetadata extends GenericMetadata implements PropertyMetadat */ public function addConstraint(Constraint $constraint) { - if (!\in_array(Constraint::PROPERTY_CONSTRAINT, (array) $constraint->getTargets())) { - throw new ConstraintDefinitionException(sprintf('The constraint "%s" cannot be put on properties or getters.', \get_class($constraint))); - } + $this->checkConstraint($constraint); parent::addConstraint($constraint); @@ -181,4 +180,17 @@ abstract class MemberMetadata extends GenericMetadata implements PropertyMetadat * @return \ReflectionMethod|\ReflectionProperty The reflection instance */ abstract protected function newReflectionMember($objectOrClassName); + + private function checkConstraint(Constraint $constraint) + { + if (!\in_array(Constraint::PROPERTY_CONSTRAINT, (array) $constraint->getTargets(), true)) { + throw new ConstraintDefinitionException(sprintf('The constraint "%s" cannot be put on properties or getters.', \get_class($constraint))); + } + + if ($constraint instanceof Composite) { + foreach ($constraint->getNestedContraints() as $nestedContraint) { + $this->checkConstraint($nestedContraint); + } + } + } } diff --git a/src/Symfony/Component/Validator/Tests/Mapping/ClassMetadataTest.php b/src/Symfony/Component/Validator/Tests/Mapping/ClassMetadataTest.php index 73af5c1894..02fe484525 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/ClassMetadataTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/ClassMetadataTest.php @@ -13,8 +13,11 @@ namespace Symfony\Component\Validator\Tests\Mapping; use PHPUnit\Framework\TestCase; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Constraints\Composite; use Symfony\Component\Validator\Constraints\Valid; +use Symfony\Component\Validator\Exception\ConstraintDefinitionException; use Symfony\Component\Validator\Mapping\ClassMetadata; +use Symfony\Component\Validator\Tests\Fixtures\ClassConstraint; use Symfony\Component\Validator\Tests\Fixtures\ConstraintA; use Symfony\Component\Validator\Tests\Fixtures\ConstraintB; use Symfony\Component\Validator\Tests\Fixtures\PropertyConstraint; @@ -52,6 +55,20 @@ class ClassMetadataTest extends TestCase $this->metadata->addConstraint(new PropertyConstraint()); } + public function testAddCompositeConstraintRejectsNestedPropertyConstraints() + { + $this->expectException(ConstraintDefinitionException::class); + $this->expectExceptionMessage('The constraint "Symfony\Component\Validator\Tests\Fixtures\PropertyConstraint" cannot be put on classes.'); + + $this->metadata->addConstraint(new ClassCompositeConstraint([new PropertyConstraint()])); + } + + public function testAddCompositeConstraintAcceptsNestedClassConstraints() + { + $this->metadata->addConstraint($constraint = new ClassCompositeConstraint([new ClassConstraint()])); + $this->assertSame($this->metadata->getConstraints(), [$constraint]); + } + public function testAddPropertyConstraints() { $this->metadata->addPropertyConstraint('firstName', new ConstraintA()); @@ -311,3 +328,23 @@ class ClassMetadataTest extends TestCase $this->assertCount(0, $this->metadata->getPropertyMetadata('foo'), '->getPropertyMetadata() returns an empty collection if no metadata is configured for the given property'); } } + +class ClassCompositeConstraint extends Composite +{ + public $nested; + + public function getDefaultOption() + { + return $this->getCompositeOption(); + } + + protected function getCompositeOption() + { + return 'nested'; + } + + public function getTargets() + { + return [self::CLASS_CONSTRAINT]; + } +} diff --git a/src/Symfony/Component/Validator/Tests/Mapping/MemberMetadataTest.php b/src/Symfony/Component/Validator/Tests/Mapping/MemberMetadataTest.php index 651ba95642..c387e39797 100644 --- a/src/Symfony/Component/Validator/Tests/Mapping/MemberMetadataTest.php +++ b/src/Symfony/Component/Validator/Tests/Mapping/MemberMetadataTest.php @@ -12,11 +12,16 @@ namespace Symfony\Component\Validator\Tests\Mapping; use PHPUnit\Framework\TestCase; +use Symfony\Component\Validator\Constraints\Collection; +use Symfony\Component\Validator\Constraints\Composite; +use Symfony\Component\Validator\Constraints\Required; use Symfony\Component\Validator\Constraints\Valid; +use Symfony\Component\Validator\Exception\ConstraintDefinitionException; use Symfony\Component\Validator\Mapping\MemberMetadata; use Symfony\Component\Validator\Tests\Fixtures\ClassConstraint; use Symfony\Component\Validator\Tests\Fixtures\ConstraintA; use Symfony\Component\Validator\Tests\Fixtures\ConstraintB; +use Symfony\Component\Validator\Tests\Fixtures\PropertyConstraint; class MemberMetadataTest extends TestCase { @@ -43,6 +48,34 @@ class MemberMetadataTest extends TestCase $this->metadata->addConstraint(new ClassConstraint()); } + public function testAddCompositeConstraintRejectsNestedClassConstraints() + { + $this->expectException(ConstraintDefinitionException::class); + $this->expectExceptionMessage('The constraint "Symfony\Component\Validator\Tests\Fixtures\ClassConstraint" cannot be put on properties or getters.'); + + $this->metadata->addConstraint(new PropertyCompositeConstraint([new ClassConstraint()])); + } + + public function testAddCompositeConstraintRejectsDeepNestedClassConstraints() + { + $this->expectException(ConstraintDefinitionException::class); + $this->expectExceptionMessage('The constraint "Symfony\Component\Validator\Tests\Fixtures\ClassConstraint" cannot be put on properties or getters.'); + + $this->metadata->addConstraint(new Collection(['field1' => new Required([new ClassConstraint()])])); + } + + public function testAddCompositeConstraintAcceptsNestedPropertyConstraints() + { + $this->metadata->addConstraint($constraint = new PropertyCompositeConstraint([new PropertyConstraint()])); + $this->assertSame($this->metadata->getConstraints(), [$constraint]); + } + + public function testAddCompositeConstraintAcceptsDeepNestedPropertyConstraints() + { + $this->metadata->addConstraint($constraint = new Collection(['field1' => new Required([new PropertyConstraint()])])); + $this->assertSame($this->metadata->getConstraints(), [$constraint]); + } + public function testSerialize() { $this->metadata->addConstraint(new ConstraintA(['property1' => 'A'])); @@ -82,3 +115,18 @@ class TestMemberMetadata extends MemberMetadata { } } + +class PropertyCompositeConstraint extends Composite +{ + public $nested; + + public function getDefaultOption() + { + return $this->getCompositeOption(); + } + + protected function getCompositeOption() + { + return 'nested'; + } +} From 235920a3882dcf7147b6f641c6707105c9d7089a Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Mon, 6 Jul 2020 20:36:32 +0200 Subject: [PATCH 2/4] fix mapping errors from unmapped forms --- .../ViolationMapper/ViolationMapper.php | 7 ---- .../ViolationMapper/Fixtures/Issue.php | 16 ++++++++ .../ViolationMapper/ViolationMapperTest.php | 38 ++++++++++++++++--- 3 files changed, 49 insertions(+), 12 deletions(-) create mode 100644 src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/Fixtures/Issue.php diff --git a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php index 659c266ce6..2a024ee4b9 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php +++ b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php @@ -241,13 +241,6 @@ class ViolationMapper implements ViolationMapperInterface // Form inherits its parent data // Cut the piece out of the property path and proceed $propertyPathBuilder->remove($i); - } elseif (!$scope->getConfig()->getMapped()) { - // Form is not mapped - // Set the form as new origin and strip everything - // we have so far in the path - $origin = $scope; - $propertyPathBuilder->remove(0, $i + 1); - $i = 0; } else { /* @var \Symfony\Component\PropertyAccess\PropertyPathInterface $propertyPath */ $propertyPath = $scope->getPropertyPath(); diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/Fixtures/Issue.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/Fixtures/Issue.php new file mode 100644 index 0000000000..62b6a4e77d --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/Fixtures/Issue.php @@ -0,0 +1,16 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Tests\Extension\Validator\ViolationMapper\Fixtures; + +class Issue +{ +} diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php index 2fa3e92892..6e3fffc217 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php @@ -22,6 +22,7 @@ use Symfony\Component\Form\Form; use Symfony\Component\Form\FormConfigBuilder; use Symfony\Component\Form\FormError; use Symfony\Component\Form\FormInterface; +use Symfony\Component\Form\Tests\Extension\Validator\ViolationMapper\Fixtures\Issue; use Symfony\Component\PropertyAccess\PropertyPath; use Symfony\Component\Validator\ConstraintViolation; use Symfony\Component\Validator\ConstraintViolationInterface; @@ -70,12 +71,12 @@ class ViolationMapperTest extends TestCase $this->params = ['foo' => 'bar']; } - protected function getForm($name = 'name', $propertyPath = null, $dataClass = null, $errorMapping = [], $inheritData = false, $synchronized = true) + protected function getForm($name = 'name', $propertyPath = null, $dataClass = null, $errorMapping = [], $inheritData = false, $synchronized = true, array $options = []) { $config = new FormConfigBuilder($name, $dataClass, $this->dispatcher, [ 'error_mapping' => $errorMapping, - ]); - $config->setMapped(true); + ] + $options); + $config->setMapped(isset($options['mapped']) ? $options['mapped'] : true); $config->setInheritData($inheritData); $config->setPropertyPath($propertyPath); $config->setCompound(true); @@ -96,9 +97,9 @@ class ViolationMapperTest extends TestCase * * @return ConstraintViolation */ - protected function getConstraintViolation($propertyPath) + protected function getConstraintViolation($propertyPath, $root = null) { - return new ConstraintViolation($this->message, $this->messageTemplate, $this->params, null, $propertyPath, null); + return new ConstraintViolation($this->message, $this->messageTemplate, $this->params, $root, $propertyPath, null); } /** @@ -112,6 +113,33 @@ class ViolationMapperTest extends TestCase return $error; } + public function testMappingErrorsWhenFormIsNotMapped() + { + $form = $this->getForm('name', null, Issue::class, [ + 'child1' => 'child2', + ]); + + $violation = $this->getConstraintViolation('children[child1].data', $form); + + $child1Form = $this->getForm('child1', null, null, [], false, true, [ + 'mapped' => false, + ]); + $child2Form = $this->getForm('child2', null, null, [], false, true, [ + 'mapped' => false, + ]); + + $form->add($child1Form); + $form->add($child2Form); + + $form->submit([]); + + $this->mapper->mapViolation($violation, $form); + + $this->assertCount(0, $form->getErrors()); + $this->assertCount(0, $form->get('child1')->getErrors()); + $this->assertCount(1, $form->get('child2')->getErrors()); + } + public function testMapToFormInheritingParentDataIfDataDoesNotMatch() { $violation = $this->getConstraintViolation('children[address].data.foo'); From d642d85cd3f2d438d0731aef0304225ea556c652 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Sat, 8 Aug 2020 16:09:36 +0200 Subject: [PATCH 3/4] Use PHPUnit 9.3 on php 8. --- phpunit | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/phpunit b/phpunit index 713594fc19..2b4412dc4d 100755 --- a/phpunit +++ b/phpunit @@ -12,7 +12,11 @@ if (!getenv('SYMFONY_PHPUNIT_VERSION')) { if (false === getenv('SYMFONY_PHPUNIT_REMOVE_RETURN_TYPEHINT') && false !== strpos(@file_get_contents(__DIR__.'/src/Symfony/Component/HttpKernel/Kernel.php'), 'const MAJOR_VERSION = 3;')) { putenv('SYMFONY_PHPUNIT_REMOVE_RETURN_TYPEHINT=1'); } - putenv('SYMFONY_PHPUNIT_VERSION=8.3'); + if (\PHP_VERSION_ID >= 80000) { + putenv('SYMFONY_PHPUNIT_VERSION=9.3'); + } else { + putenv('SYMFONY_PHPUNIT_VERSION=8.3'); + } } elseif (\PHP_VERSION_ID >= 70000) { putenv('SYMFONY_PHPUNIT_VERSION=6.5'); } From 46f635c492e46bfcd004c7f01f8a86e8c52856df Mon Sep 17 00:00:00 2001 From: Joshua Nye Date: Wed, 5 Aug 2020 16:33:59 -0400 Subject: [PATCH 4/4] [Yaml] Fix for #36624; Allow PHP constant as first key in block --- src/Symfony/Component/Yaml/Parser.php | 8 +++-- .../Component/Yaml/Tests/ParserTest.php | 30 ++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Yaml/Parser.php b/src/Symfony/Component/Yaml/Parser.php index e859766f97..6d8e79b271 100644 --- a/src/Symfony/Component/Yaml/Parser.php +++ b/src/Symfony/Component/Yaml/Parser.php @@ -230,8 +230,12 @@ class Parser $this->parseBlock($this->getRealCurrentLineNb() + 1, $this->getNextEmbedBlock(null, true), $flags) ); } else { - if (isset($values['leadspaces']) - && self::preg_match('#^(?P'.Inline::REGEX_QUOTED_STRING.'|[^ \'"\{\[].*?) *\:(\s+(?P.+?))?\s*$#u', $this->trimTag($values['value']), $matches) + if ( + isset($values['leadspaces']) + && ( + '!' === $values['value'][0] + || self::preg_match('#^(?P'.Inline::REGEX_QUOTED_STRING.'|[^ \'"\{\[].*?) *\:(\s+(?P.+?))?\s*$#u', $this->trimTag($values['value']), $matches) + ) ) { // this is a compact notation element, add to next block and parse $block = $values['value']; diff --git a/src/Symfony/Component/Yaml/Tests/ParserTest.php b/src/Symfony/Component/Yaml/Tests/ParserTest.php index b6cdb6a9aa..1627389bf8 100644 --- a/src/Symfony/Component/Yaml/Tests/ParserTest.php +++ b/src/Symfony/Component/Yaml/Tests/ParserTest.php @@ -2025,6 +2025,34 @@ YAML; $this->assertSame($expected, $this->parser->parse($yaml, Yaml::PARSE_CONSTANT | Yaml::PARSE_KEYS_AS_STRINGS)); } + public function testPhpConstantTagMappingAsScalarKey() + { + $yaml = <<assertSame([ + 'map1' => [['foo' => 'value_0', 'bar' => 'value_1']], + 'map2' => [['foo' => 'value_0', 'bar' => 'value_1']], + ], $this->parser->parse($yaml, Yaml::PARSE_CONSTANT)); + } + + public function testTagMappingAsScalarKey() + { + $yaml = <<assertSame([ + 'map1' => [['0' => 'value_0', '1' => 'value_1']], + ], $this->parser->parse($yaml)); + } + public function testMergeKeysWhenMappingsAreParsedAsObjects() { $yaml = <<assertSame(['parameters' => 'abc'], $this->parser->parse($yaml));