-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Description
Symfony version(s) affected: 4.4.7
Description
allowEmptyString option of Symfony\Component\Validator\Constraints\LengthValidator allows empty string
How to reproduce
- Create a form extending Symfony\Component\Form\AbstractType
- Inside buildForm() method, add a field
$builder
...
->add('title', TextType::class, [
'constraints' => [
new Length([
'allowEmptyString' => false,
'max' => 255,
])
]
])
...
- According to documentation
https://symfony.com/doc/4.4/reference/constraints/Length.html#allowemptystring
Set it to false to consider empty strings not valid.
- Create a controller, submit the form described above with data
['title' => '']
Expectation:
Form is invalid and has a proper error.
Reality:
The form is valid.
The Reason
Currently, the code in LengthValidator only has the following lines mentioning allowEmptyString:
if (null !== $constraint->min && null === $constraint->allowEmptyString) {
@trigger_error(sprintf('Using the "%s" constraint with the "min" option without setting the "allowEmptyString" one is deprecated and defaults to true. In 5.0, it will become optional and default to false.', Length::class), E_USER_DEPRECATED);
}
if (null === $value || ('' === $value && ($constraint->allowEmptyString ?? true))) {
return;
}
The first block only triggers error if 'min' option is provided. In my example above 'min' is not provided.
The second block inner part (return) is not reached as well, because
null === $value
evaluates to false
('' === $value && ($constraint->allowEmptyString ?? true)))
also evaluates to false
:
'' === $value
evaluates totrue
$constraint->allowEmptyString ?? true
evaluates tofalse
true && false
evaluates tofalse
Further down the code, allowEmptyString is never checked. Only charset, min and max validations are checked.
Possible Solution
Write code which validates the the given string is not empty if allowEmptyString is false.
Also, write tests.