From fe6a2dd64fc7f703d7bcd6e3b9448dc881debf60 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Thu, 19 Nov 2020 19:56:09 +0100 Subject: [PATCH] prevent duplicated error message for file upload limits --- .../Form/Extension/Core/Type/FileType.php | 4 +- .../ViolationMapper/ViolationMapper.php | 19 ++++ .../Component/Form/FileUploadError.php | 19 ++++ .../ViolationMapper/ViolationMapperTest.php | 92 +++++++++++++++++++ 4 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 src/Symfony/Component/Form/FileUploadError.php diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FileType.php b/src/Symfony/Component/Form/Extension/Core/Type/FileType.php index 207944bf5ad6..d8b93f6fc77b 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/FileType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/FileType.php @@ -12,8 +12,8 @@ namespace Symfony\Component\Form\Extension\Core\Type; use Symfony\Component\Form\AbstractType; +use Symfony\Component\Form\FileUploadError; use Symfony\Component\Form\FormBuilderInterface; -use Symfony\Component\Form\FormError; use Symfony\Component\Form\FormEvent; use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\FormInterface; @@ -171,7 +171,7 @@ private function getFileUploadError(int $errorCode) $message = strtr($messageTemplate, $messageParameters); } - return new FormError($message, $messageTemplate, $messageParameters); + return new FileUploadError($message, $messageTemplate, $messageParameters); } /** diff --git a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php index 8799fc196cc9..4c19e9850a44 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php +++ b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php @@ -11,12 +11,14 @@ namespace Symfony\Component\Form\Extension\Validator\ViolationMapper; +use Symfony\Component\Form\FileUploadError; use Symfony\Component\Form\FormError; use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\Util\InheritDataAwareIterator; use Symfony\Component\PropertyAccess\PropertyPathBuilder; use Symfony\Component\PropertyAccess\PropertyPathIterator; use Symfony\Component\PropertyAccess\PropertyPathIteratorInterface; +use Symfony\Component\Validator\Constraints\File; use Symfony\Component\Validator\ConstraintViolation; /** @@ -124,6 +126,23 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form // Only add the error if the form is synchronized if ($this->acceptsErrors($scope)) { + if ($violation->getConstraint() instanceof File && (string) \UPLOAD_ERR_INI_SIZE === $violation->getCode()) { + $errorsTarget = $scope; + + while (null !== $errorsTarget->getParent() && $errorsTarget->getConfig()->getErrorBubbling()) { + $errorsTarget = $errorsTarget->getParent(); + } + + $errors = $errorsTarget->getErrors(); + $errorsTarget->clearErrors(); + + foreach ($errors as $error) { + if (!$error instanceof FileUploadError) { + $errorsTarget->addError($error); + } + } + } + $scope->addError(new FormError( $violation->getMessage(), $violation->getMessageTemplate(), diff --git a/src/Symfony/Component/Form/FileUploadError.php b/src/Symfony/Component/Form/FileUploadError.php new file mode 100644 index 000000000000..20142b20337e --- /dev/null +++ b/src/Symfony/Component/Form/FileUploadError.php @@ -0,0 +1,19 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form; + +/** + * @internal + */ +class FileUploadError extends FormError +{ +} 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 822e36e6a1dc..8c71b7bfacef 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php @@ -18,12 +18,14 @@ use Symfony\Component\Form\Exception\TransformationFailedException; use Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper; use Symfony\Component\Form\Extension\Validator\ViolationMapper\ViolationMapper; +use Symfony\Component\Form\FileUploadError; 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\Constraints\File; use Symfony\Component\Validator\ConstraintViolation; use Symfony\Component\Validator\ConstraintViolationInterface; @@ -81,6 +83,7 @@ protected function getForm($name = 'name', $propertyPath = null, $dataClass = nu $config->setPropertyPath($propertyPath); $config->setCompound(true); $config->setDataMapper(new PropertyPathMapper()); + $config->setErrorBubbling($options['error_bubbling'] ?? false); if (!$synchronized) { $config->addViewTransformer(new CallbackTransformer( @@ -1590,4 +1593,93 @@ public function testBacktrackIfSeveralSubFormsWithSamePropertyPath() $this->assertEquals([$this->getFormError($violation2, $grandChild2)], iterator_to_array($grandChild2->getErrors()), $grandChild2->getName().' should have an error, but has none'); $this->assertEquals([$this->getFormError($violation3, $grandChild3)], iterator_to_array($grandChild3->getErrors()), $grandChild3->getName().' should have an error, but has none'); } + + public function testFileUploadErrorIsNotRemovedIfNoFileSizeConstraintViolationWasRaised() + { + $form = $this->getForm('form'); + $form->addError(new FileUploadError( + 'The file is too large. Allowed maximum size is 2 MB.', + 'The file is too large. Allowed maximum size is {{ limit }} {{ suffix }}.', + [ + '{{ limit }}' => '2', + '{{ suffix }}' => 'MB', + ] + )); + + $this->mapper->mapViolation($this->getConstraintViolation('data'), $form); + + $this->assertCount(2, $form->getErrors()); + } + + public function testFileUploadErrorIsRemovedIfFileSizeConstraintViolationWasRaised() + { + $form = $this->getForm('form'); + $form->addError(new FileUploadError( + 'The file is too large. Allowed maximum size is 2 MB.', + 'The file is too large. Allowed maximum size is {{ limit }} {{ suffix }}.', + [ + '{{ limit }}' => '2', + '{{ suffix }}' => 'MB', + ] + )); + + $violation = new ConstraintViolation( + 'The file is too large (3 MB). Allowed maximum size is 2 MB.', + 'The file is too large ({{ size }} {{ suffix }}). Allowed maximum size is {{ limit }} {{ suffix }}.', + [ + '{{ limit }}' => '2', + '{{ size }}' => '3', + '{{ suffix }}' => 'MB', + ], + '', + 'data', + null, + null, + (string) \UPLOAD_ERR_INI_SIZE, + new File() + ); + $this->mapper->mapViolation($this->getConstraintViolation('data'), $form); + $this->mapper->mapViolation($violation, $form); + + $this->assertCount(2, $form->getErrors()); + } + + public function testFileUploadErrorIsRemovedIfFileSizeConstraintViolationWasRaisedOnFieldWithErrorBubbling() + { + $parent = $this->getForm('parent'); + $child = $this->getForm('child', 'file', null, [], false, true, [ + 'error_bubbling' => true, + ]); + $parent->add($child); + $child->addError(new FileUploadError( + 'The file is too large. Allowed maximum size is 2 MB.', + 'The file is too large. Allowed maximum size is {{ limit }} {{ suffix }}.', + [ + '{{ limit }}' => '2', + '{{ suffix }}' => 'MB', + ] + )); + + $violation = new ConstraintViolation( + 'The file is too large (3 MB). Allowed maximum size is 2 MB.', + 'The file is too large ({{ size }} {{ suffix }}). Allowed maximum size is {{ limit }} {{ suffix }}.', + [ + '{{ limit }}' => '2', + '{{ size }}' => '3', + '{{ suffix }}' => 'MB', + ], + null, + 'data.file', + null, + null, + (string) \UPLOAD_ERR_INI_SIZE, + new File() + ); + $this->mapper->mapViolation($this->getConstraintViolation('data'), $parent); + $this->mapper->mapViolation($this->getConstraintViolation('data.file'), $parent); + $this->mapper->mapViolation($violation, $parent); + + $this->assertCount(3, $parent->getErrors()); + $this->assertCount(0, $child->getErrors()); + } }