Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit bda4eb5

Browse filesBrowse files
xabbuhxabbuh
committed
do not validate non-submitted form fields in PATCH requests
1 parent 823ccf0 commit bda4eb5
Copy full SHA for bda4eb5

File tree

5 files changed

+113
-51
lines changed
Filter options

5 files changed

+113
-51
lines changed

‎src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public function validate($form, Constraint $formConstraint)
4141

4242
$validator = $this->context->getValidator()->inContext($this->context);
4343

44-
if ($form->isSynchronized()) {
44+
if ($form->isSubmitted() && $form->isSynchronized()) {
4545
// Validate the form data only if transformation succeeded
4646
$groups = self::getValidationGroups($form);
4747
$data = $form->getData();
@@ -90,7 +90,7 @@ public function validate($form, Constraint $formConstraint)
9090
}
9191
}
9292
}
93-
} else {
93+
} elseif (!$form->isSynchronized()) {
9494
$childrenSynchronized = true;
9595

9696
/** @var FormInterface $child */

‎src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php
+1-4Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -273,9 +273,6 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or
273273
*/
274274
private function acceptsErrors(FormInterface $form)
275275
{
276-
// Ignore non-submitted forms. This happens, for example, in PATCH
277-
// requests.
278-
// https://github.com/symfony/symfony/pull/10567
279-
return $form->isSubmitted() && ($this->allowNonSynchronized || $form->isSynchronized());
276+
return $this->allowNonSynchronized || $form->isSynchronized();
280277
}
281278
}

‎src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/Extension/Validator/Constraints/FormValidatorTest.php
+38-41Lines changed: 38 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,8 @@ public function testValidate()
6161
{
6262
$object = new \stdClass();
6363
$options = ['validation_groups' => ['group1', 'group2']];
64-
$form = $this->getBuilder('name', '\stdClass', $options)
65-
->setData($object)
66-
->getForm();
64+
$form = $this->getCompoundForm($object, $options);
65+
$form->submit([]);
6766

6867
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
6968

@@ -82,9 +81,8 @@ public function testValidateConstraints()
8281
'validation_groups' => ['group1', 'group2'],
8382
'constraints' => [$constraint1, $constraint2],
8483
];
85-
$form = $this->getBuilder('name', '\stdClass', $options)
86-
->setData($object)
87-
->getForm();
84+
$form = $this->getCompoundForm($object, $options);
85+
$form->submit([]);
8886

8987
// First default constraints
9088
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
@@ -110,10 +108,9 @@ public function testValidateChildIfValidConstraint()
110108
'validation_groups' => ['group1', 'group2'],
111109
'constraints' => [new Valid()],
112110
];
113-
$form = $this->getBuilder('name', '\stdClass', $options)->getForm();
111+
$form = $this->getCompoundForm($object, $options);
114112
$parent->add($form);
115-
116-
$form->setData($object);
113+
$parent->submit([]);
117114

118115
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
119116

@@ -146,8 +143,8 @@ public function testDontValidateIfParentWithoutValidConstraint()
146143
public function testMissingConstraintIndex()
147144
{
148145
$object = new \stdClass();
149-
$form = new FormBuilder('name', '\stdClass', $this->dispatcher, $this->factory);
150-
$form = $form->setData($object)->getForm();
146+
$form = $this->getCompoundForm($object);
147+
$form->submit([]);
151148

152149
$this->expectValidateAt(0, 'data', $object, ['Default']);
153150

@@ -170,10 +167,9 @@ public function testValidateConstraintsOptionEvenIfNoValidConstraint()
170167
'validation_groups' => ['group1', 'group2'],
171168
'constraints' => [$constraint1, $constraint2],
172169
];
173-
$form = $this->getBuilder('name', '\stdClass', $options)
174-
->setData($object)
175-
->getForm();
170+
$form = $this->getCompoundForm($object, $options);
176171
$parent->add($form);
172+
$parent->submit([]);
177173

178174
$this->expectValidateValueAt(0, 'data', $object, $constraint1, 'group1');
179175
$this->expectValidateValueAt(1, 'data', $object, $constraint2, 'group2');
@@ -361,9 +357,8 @@ public function testHandleGroupSequenceValidationGroups()
361357
{
362358
$object = new \stdClass();
363359
$options = ['validation_groups' => new GroupSequence(['group1', 'group2'])];
364-
$form = $this->getBuilder('name', '\stdClass', $options)
365-
->setData($object)
366-
->getForm();
360+
$form = $this->getCompoundForm($object, $options);
361+
$form->submit([]);
367362

368363
$this->expectValidateAt(0, 'data', $object, new GroupSequence(['group1', 'group2']));
369364
$this->expectValidateAt(1, 'data', $object, new GroupSequence(['group1', 'group2']));
@@ -377,9 +372,8 @@ public function testHandleCallbackValidationGroups()
377372
{
378373
$object = new \stdClass();
379374
$options = ['validation_groups' => [$this, 'getValidationGroups']];
380-
$form = $this->getBuilder('name', '\stdClass', $options)
381-
->setData($object)
382-
->getForm();
375+
$form = $this->getCompoundForm($object, $options);
376+
$form->submit([]);
383377

384378
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
385379

@@ -392,9 +386,8 @@ public function testDontExecuteFunctionNames()
392386
{
393387
$object = new \stdClass();
394388
$options = ['validation_groups' => 'header'];
395-
$form = $this->getBuilder('name', '\stdClass', $options)
396-
->setData($object)
397-
->getForm();
389+
$form = $this->getCompoundForm($object, $options);
390+
$form->submit([]);
398391

399392
$this->expectValidateAt(0, 'data', $object, ['header']);
400393

@@ -409,9 +402,8 @@ public function testHandleClosureValidationGroups()
409402
$options = ['validation_groups' => function (FormInterface $form) {
410403
return ['group1', 'group2'];
411404
}];
412-
$form = $this->getBuilder('name', '\stdClass', $options)
413-
->setData($object)
414-
->getForm();
405+
$form = $this->getCompoundForm($object, $options);
406+
$form->submit([]);
415407

416408
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
417409

@@ -455,7 +447,7 @@ public function testDontUseValidationGroupOfUnclickedButton()
455447
->setCompound(true)
456448
->setDataMapper(new PropertyPathMapper())
457449
->getForm();
458-
$form = $this->getForm('name', '\stdClass', [
450+
$form = $this->getCompoundForm($object, [
459451
'validation_groups' => 'form_group',
460452
'constraints' => [new Valid()],
461453
]);
@@ -465,7 +457,7 @@ public function testDontUseValidationGroupOfUnclickedButton()
465457
'validation_groups' => 'button_group',
466458
]));
467459

468-
$form->setData($object);
460+
$parent->submit([]);
469461

470462
$this->expectValidateAt(0, 'data', $object, ['form_group']);
471463

@@ -484,10 +476,9 @@ public function testUseInheritedValidationGroup()
484476
->setDataMapper(new PropertyPathMapper())
485477
->getForm();
486478
$formOptions = ['constraints' => [new Valid()]];
487-
$form = $this->getBuilder('name', '\stdClass', $formOptions)->getForm();
479+
$form = $this->getCompoundForm($object, $formOptions);
488480
$parent->add($form);
489-
490-
$form->setData($object);
481+
$parent->submit([]);
491482

492483
$this->expectValidateAt(0, 'data', $object, ['group']);
493484

@@ -506,10 +497,9 @@ public function testUseInheritedCallbackValidationGroup()
506497
->setDataMapper(new PropertyPathMapper())
507498
->getForm();
508499
$formOptions = ['constraints' => [new Valid()]];
509-
$form = $this->getBuilder('name', '\stdClass', $formOptions)->getForm();
500+
$form = $this->getCompoundForm($object, $formOptions);
510501
$parent->add($form);
511-
512-
$form->setData($object);
502+
$parent->submit([]);
513503

514504
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
515505

@@ -523,7 +513,7 @@ public function testUseInheritedClosureValidationGroup()
523513
$object = new \stdClass();
524514

525515
$parentOptions = [
526-
'validation_groups' => function (FormInterface $form) {
516+
'validation_groups' => function () {
527517
return ['group1', 'group2'];
528518
},
529519
];
@@ -532,10 +522,9 @@ public function testUseInheritedClosureValidationGroup()
532522
->setDataMapper(new PropertyPathMapper())
533523
->getForm();
534524
$formOptions = ['constraints' => [new Valid()]];
535-
$form = $this->getBuilder('name', '\stdClass', $formOptions)->getForm();
525+
$form = $this->getCompoundForm($object, $formOptions);
536526
$parent->add($form);
537-
538-
$form->setData($object);
527+
$parent->submit([]);
539528

540529
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
541530

@@ -547,9 +536,8 @@ public function testUseInheritedClosureValidationGroup()
547536
public function testAppendPropertyPath()
548537
{
549538
$object = new \stdClass();
550-
$form = $this->getBuilder('name', '\stdClass')
551-
->setData($object)
552-
->getForm();
539+
$form = $this->getCompoundForm($object);
540+
$form->submit([]);
553541

554542
$this->expectValidateAt(0, 'data', $object, ['Default']);
555543

@@ -690,6 +678,15 @@ private function getForm($name = 'name', $dataClass = null, array $options = [])
690678
return $this->getBuilder($name, $dataClass, $options)->getForm();
691679
}
692680

681+
private function getCompoundForm($data, array $options = [])
682+
{
683+
return $this->getBuilder('name', \get_class($data), $options)
684+
->setData($data)
685+
->setCompound(true)
686+
->setDataMapper(new PropertyPathMapper())
687+
->getForm();
688+
}
689+
693690
private function getSubmitButton($name = 'name', array $options = [])
694691
{
695692
$builder = new SubmitButtonBuilder($name, $options);

‎src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/Extension/Validator/ValidatorExtensionTest.php
+68Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,19 @@
1212
namespace Symfony\Component\Form\Tests\Extension\Validator;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Form\AbstractType;
1516
use Symfony\Component\Form\Extension\Validator\Constraints\Form as FormConstraint;
1617
use Symfony\Component\Form\Extension\Validator\ValidatorExtension;
1718
use Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser;
1819
use Symfony\Component\Form\Form;
20+
use Symfony\Component\Form\FormBuilderInterface;
21+
use Symfony\Component\Form\FormFactoryBuilder;
22+
use Symfony\Component\OptionsResolver\OptionsResolver;
23+
use Symfony\Component\Validator\Constraints\NotBlank;
1924
use Symfony\Component\Validator\Mapping\CascadingStrategy;
2025
use Symfony\Component\Validator\Mapping\ClassMetadata;
26+
use Symfony\Component\Validator\Mapping\Factory\LazyLoadingMetadataFactory;
27+
use Symfony\Component\Validator\Mapping\Loader\StaticMethodLoader;
2128
use Symfony\Component\Validator\Mapping\TraversalStrategy;
2229
use Symfony\Component\Validator\Tests\Fixtures\FakeMetadataFactory;
2330
use Symfony\Component\Validator\Validation;
@@ -45,4 +52,65 @@ public function test2Dot5ValidationApi()
4552
$this->assertSame(CascadingStrategy::CASCADE, $metadata->getPropertyMetadata('children')[0]->cascadingStrategy);
4653
$this->assertSame(TraversalStrategy::IMPLICIT, $metadata->getPropertyMetadata('children')[0]->traversalStrategy);
4754
}
55+
56+
public function testDataConstraintsInvalidateFormEvenIfFieldIsNotSubmitted()
57+
{
58+
$form = $this->createForm(FooType::class);
59+
$form->submit(['baz' => 'foobar'], false);
60+
61+
$this->assertTrue($form->isSubmitted());
62+
$this->assertFalse($form->isValid());
63+
$this->assertFalse($form->get('bar')->isSubmitted());
64+
$this->assertCount(1, $form->get('bar')->getErrors());
65+
}
66+
67+
public function testFieldConstraintsDoNotInvalidFormIfFieldIsNotSubmitted()
68+
{
69+
$form = $this->createForm(FooType::class);
70+
$form->submit(['bar' => 'foobar'], false);
71+
72+
$this->assertTrue($form->isSubmitted());
73+
$this->assertTrue($form->isValid());
74+
}
75+
76+
private function createForm($type)
77+
{
78+
$validator = Validation::createValidatorBuilder()
79+
->setMetadataFactory(new LazyLoadingMetadataFactory(new StaticMethodLoader()))
80+
->getValidator();
81+
$formFactoryBuilder = new FormFactoryBuilder();
82+
$formFactoryBuilder->addExtension(new ValidatorExtension($validator));
83+
$formFactory = $formFactoryBuilder->getFormFactory();
84+
85+
return $formFactory->create($type);
86+
}
87+
}
88+
89+
class Foo
90+
{
91+
public $bar;
92+
public $baz;
93+
94+
public static function loadValidatorMetadata(ClassMetadata $metadata)
95+
{
96+
$metadata->addPropertyConstraint('bar', new NotBlank());
97+
}
98+
}
99+
100+
class FooType extends AbstractType
101+
{
102+
public function buildForm(FormBuilderInterface $builder, array $options)
103+
{
104+
$builder
105+
->add('bar')
106+
->add('baz', null, [
107+
'constraints' => [new NotBlank()],
108+
])
109+
;
110+
}
111+
112+
public function configureOptions(OptionsResolver $resolver)
113+
{
114+
$resolver->setDefault('data_class', Foo::class);
115+
}
48116
}

‎src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ public function testAbortDotRuleMappingIfNotSynchronized()
205205
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
206206
}
207207

208-
public function testAbortMappingIfNotSubmitted()
208+
public function testMappingIfNotSubmitted()
209209
{
210210
$violation = $this->getConstraintViolation('children[address].data.street');
211211
$parent = $this->getForm('parent');
@@ -225,10 +225,10 @@ public function testAbortMappingIfNotSubmitted()
225225

226226
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
227227
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
228-
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
228+
$this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error');
229229
}
230230

231-
public function testAbortDotRuleMappingIfNotSubmitted()
231+
public function testDotRuleMappingIfNotSubmitted()
232232
{
233233
$violation = $this->getConstraintViolation('data.address');
234234
$parent = $this->getForm('parent');
@@ -250,7 +250,7 @@ public function testAbortDotRuleMappingIfNotSubmitted()
250250

251251
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
252252
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
253-
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
253+
$this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error');
254254
}
255255

256256
public function provideDefaultTests()

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.