If you dont want a null value use the NotBlank validator in addition to the TypeValidator - this allows you to validate types but not require the value to be present, or require it independently.

Thank you, @merk. I'm aware of NotBlank. I use it all the time. In fact, it seems to be the only way to workaround the aforementioned issue. But, IMHO, that's not a proper solution.

The purpose of Validators are to enforce specific constraints. In this particular case, the TypeValidator is not enforcing the constraint it was configured to validate. I would consider this a bug... or at least a caveat that should be documented.

What if I have a property that can be a string (any string, blank is okay), but NULL is not okay. Conceivably, the combination of NotNull followed by Type (type: string) should validate this scenario... but it doesn't because the validator ignores NULLs.

At the very least, there should be an explanation (in the comments) as to why the "if (null === $value)" logic is needed. Especially since it was removed two years ago.

Forgive me-- my example didn't make sense. What I meant to say was that Type (type:string) by itself should validate the scenario. NotNull is redundant (but should work also).

I took a look at other validator classes from the Validator component (https://github.com/symfony/symfony/tree/master/src/Symfony/Component/Validator/Constraints) and it seems like most have similar logic -- namely, they immediately exit if the value is NULL or blank. Since the majority of the validators are using the same logic, I'm assuming there's a reason behind it.

If it's not a code bug, then at minimum I believe this to be a documentation bug. It should be spelled out clearly that validators only validate non-NULL values. I still don't think this makes sense conceptually nor is it what I believe most developer would expect, but a note in the documentation may help others like myself avoid hours of unnecessary debugging.

Hi, thank you for reporting this issue! The reason why most of the validators silently allow null values is that (a) whether a value is set or not and (b) which constraints this value has to satisfy, if it is set, are usually orthogonal concepts.

For example:

• a value may be null
• if the value is not null, it must be an integer within the range 10-99

For this reason, most validators accept null and empty strings (for string validators) as valid values. If you want to force a value to be set, combine them with a NotNull/NotBlank constraint.

Hope this clarifies your doubts. :)

@webmozart, Thanks so much for the explanation! At least I know the reasoning behind it now and it seems simple enough to workaround. However, I would still like to suggest that this behavior be documented (in the Validation section of the book) as I do not believe this to be ordinary behavior.

Though I completely agree with your opinion that a "value being set" vs. a "value satisfying some constraint" are generally orthogonal concepts, I do not agree that this is a reason for Validators to ignore null and empty strings.

As a user of validation frameworks, I expect the validation to do exactly what I tell it to do. If I ask it to validate that a value is an integer, than I expect it to do so -- null and empty string are not integers. If I ask it to validate that a value is an Email, than I expect it do so -- null and empty string are not valid emails. etc. etc. The validation framework should allow me to specify that a particular value may be null, in which case, it should not enforce any subsequent constrains... but it should not simply assume that null and empty string are okay. I've worked with many validation libraries (in PHP, Java, and JS) and I don't believe I've seen this behavior in any of them (although, to be fair, I can't say that I've looked at all of the code in those libraries).

Because a "value being set" vs. a "value satisfying some constraint" are somewhat orthogonal concepts, I think that's a strong argument for differentiating how these are handled. Instead of all Validators always ignoring null and empty strings, perhaps a better approach would be for each validator to accept a parameters which specifies if null is okay or not. This can be added to the base Constraint class so that all Validators can benefit. That also eliminates duplicate code in the Validators.

Perhaps this is something that can be considered for Symfony 2.5.

Thanks for your consideration and allowing me to express my opinion on the matter.

Bests

@natorojr Thank you for the feedback! As you've mentioned other validation libraries: The Validation component takes a lot of concepts from the JSR-303 Bean Validation specification for Java (followed up by JSR-349), one of the major validation specifications available for that language and implemented, for example, in the Hibernate validator. The concept of orthogonal NotNull/other constraints is taken directly from those specs.

You are, of course, completely right that this should be documented well. If you think that the docs are currently lacking in that regard, I would really appreciate a PR improving them.

You are also right that the given behavior is not quite straight-forward at first. Nevertheless, I think it is much simpler to understand than implementing a mechanism for conditional validation ("only validate constraint A and B if the value is not empty" etc.)

I don't like to introduce duplicate functionality, such as adding a configuration option for null acceptance, without a good reason. What we'd end up with is something like this:

/**
 * @Assert\Type("integer", acceptNull=false)
 * @Assert\Length(min=3, acceptNull=false)
 */
private $number;

I don't see how this would be clearer/easier/you name it than the current way:

/**
 * @Assert\NotNull
 * @Assert\Type("integer")
 * @Assert\Length(min=3)
 */
private $number;

@webmozart, I agree that my suggestion of adding additional parameters is not necessarily a better solution, or even a better workaround. Admittedly, it was just a brain dump and I hadn't thought it all the way through. Please disregard that suggestion.

Yes, I'm familiar with JSR-303 Bean Validation (I've done quite a bit of enterprise Java development). In fact, that is one of the main reasons I have gravitated towards Symfony's Validation component over other PHP validation libraries (for not just my Symfony projects, but non-Symfony projects as well). Nevertheless, I do not recall ever running into this problem or behavior in the Java world, which leads me to believe there is something different in Symfony's implementation of the JSR.

It's been a while since I've played with Java's bean validation, so I skimmed over the JSR-349 (Bean Validation 1.1) specification. Coincidentally, I found this note:

While not mandatory, it is considered a good practice to split the core constraint validation from the not null constraint validation (for example, an Email constraint will return true on a null object, i.e. will not take care of the NotNull validation).
null can have multiple meanings but is commonly used to express that a value does not make sense, is not available or is simply unknown. Those constraints on the value are orthogonal in most cases to other constraints. For example a String, if present, must be an email but can be null. Separating both concerns is a good practice.

So, Symfony's Validator implementation does coincide with the specification with respect to it's handling of null values. However, the last sentence says a "string, if present, must be an email but can be null". Which, to me, means that an empty string should not be considered valid. As I mentioned in my previous comment, I do not believe an empty string should be considered a valid email address. Putting the handling of nulls aside, I still don't think it makes sense for any validation framework to allow an empty string to be considered a valid email.

It looks like in Symfony's EmailValidator, both null and empty strings are ignored.

https://github.com/symfony/symfony/blob/2.4/src/Symfony/Component/Validator/Constraints/EmailValidator.php#L30

if (null === $value || '' === $value) {
    return;
}

Comparing that to the corresponding Hibernate Validator code, which also ignores nulls as well as empy strings, I can see how Symfony inherited this behavior.

https://github.com/hibernate/hibernate-validator/blob/5.0/engine/src/main/java/org/hibernate/validator/internal/constraintvalidators/EmailValidator.java#L66

if ( value == null || value.length() == 0 ) {
    return true;
}

I guess, ultimately, the specification leaves this choice up to the implementation.

Anyhow, I've gone on quite a tangent from the original topic of this issue.

I'm going to try to whip up a self-contained code example which better conveys why I don't think this behavior (handling of nulls and empty strings) is what most people would logically expect. Please bare with me.

I understand that this behavior is not what people initially expect. When they do:

/** @Email */
private $email;

Then a new user will probably expect that the value is always an email address, and not null and not an empty string. However, some users prefer to separate null and empty value handling from the actual validation. How do we serve those users?

As I've hinted before, we could introduce conditional validation:

/** @UnlessEquals(null, constraints=@Email) */
private $email;

This is very flexible, but this flexibility is also a drawback. Checking whether a value may be null/empty or not becomes a lot more complicated (both for humans and machines. This is done, for example, by the Form component to guess whether a field should be required or not). Also, adding the respective constraint automatically based on other metadata becomes more complicated. Right now, it is possible (while not implemented) to automatically add the NotNull constraint if corresponding Doctrine metadata is available:

/**
 * @Column(type="integer", nullable=false)
 * @Type("integer")
 * @Range(min=3, max=10)
 */
private $number;

Using just this metadata, one could easily check that the NotNull constraint is not present and add it by default. How would you do the same for conditional validation? You'd need to wrap the existing constraints, add the condition, respect possible existing conditions and probably take care of a pile of edge cases.

To sum up, I think that the current way, while not completely intuitive at first, is the most flexible and also the easiest solution at the moment and falls in line with JSR-303/349, which both have been crafted by experts in that field. Ultimately, this seems to be a matter of documentation.

If, however, you're still convinced that you have a better solution in mind, let's continue this discussion and change things for Symfony 3.0.

Hi @webmozart,

Thanks again for entertaining this thread.

Just to be clear... in case my ramblings have led you to believe otherwise... I'm very pleased that the Symfony Validation component is based off of JSR-303/349 and I would not want to see it stray very much (if at all) from the specification. I completely agree that using this JSR as a foundation, and leveraging the work of experts that have spent countless hours of thinking about validation, was the best thing to do for Symfony.

From a user perspective, I would just want to see the behavior of the library be a bit more consistent, intuitive, and give expected results. I think this can be achieved without having to add any new features or parameters that are not already supported by the library or JSR specification. I think it's just a matter of (1) improving the documentation and (2) changing the logic in a few places. It seems like we both agree on the former... my goal now is to convince you that the later is also needed.

I spent some time comparing the Symfony validator code to the corresponding Hibernate ones. For the most part, they are analogous. However, I think I've identified a few differences which could explain some of the issues and unexpected behavior I'm seeing in Symfony that I do not recall ever seeing in Hibernate.

As it turns out, ignoring NULL values isn't the underlying problem as I had originally thought. On the other hand, ignoring empty strings does cause some issues and unexpected behavior in a few places.

The easiest way to summarize the underlying problem is as follows: Because you are ignoring/bypassing empty strings in most of the string-based validators, I believe it's impossible (at this time) to validate an optional non-empty string value.

In other words, suppose you want to implement the following validation rule:

Rule 1: The value is optional, meaning it can be null. However, if a value is provided, it must be a non-empty string.
Rule 2: The value is optional, meaning it can be null. However, if a value is provided, it must be a valid email address.

Hopefully you agree that this is a very common validation rule. For example, suppose you're updating a user account record in a database. The password field in the user table doesn't need to be changed every time you update a user record. Only when the user wants to change or reset his password does the field get updated. As such, if a password is supplied, it has to be a non-empty string (most likely a hash of some sort). There are many other such scenarios where this validation rule is needed.

So, how would you accomplish this with Symfony?

Here's some sample code I whipped up. NOTE: For ease of testing, I decided to use plain-old PHP instead of using annotations or YAML (as I normally would in an application). The benefit however is that you should be able to copy and paste this as-is and run it... assuming you fetch the needed dependencies with Composer.

<?php

require __DIR__ . '/vendor/autoload.php';

use \Symfony\Component\Validator\Constraints\DateTime;
use \Symfony\Component\Validator\Constraints\Email;
use \Symfony\Component\Validator\Constraints\Length;
use \Symfony\Component\Validator\Constraints\NotBlank;
use \Symfony\Component\Validator\Constraints\NotNull;
use \Symfony\Component\Validator\Constraints\Type;
use \Symfony\Component\Validator\ValidatorBuilder;

class Test
{

    private $validator;

    public function __construct()
    {
        $validatorBuilder = new ValidatorBuilder();
        $this->validator = $validatorBuilder->getValidator();
    }

    public function run()
    {
        echo "Rule 1:\n";
        echo "Value is optional, so it can be null. However, if value is provided, it must be a non-empty string.\n";

        $values = array(NULL, '', 'foobar');
        $expectedResults = array(TRUE, FALSE, TRUE);

        echo "First attempt...\n";
        $this->test($values, $expectedResults, array(
            new Type(array('type' => 'string')), // Must be a string
            new NotBlank() // Can't be empty
        ));
        echo "Second attempt...\n";
        $this->test($values, $expectedResults, array(
            new NotBlank(), // Can't be empty
            new Type(array('type' => 'string')) // Must be a string
        ));
        echo "Third attempt...\n";
        $this->test($values, $expectedResults, array(
            new Type(array('type' => 'string')), // Must be a string
            new Length(array('min' => 1)) // Can't be empty
        ));
        echo "Fourth attempt...\n";
        $this->test($values, $expectedResults, array(
            new Type(array('type' => 'string')), // Must be a string
            new Length(array('min' => 1)), // Can't be empty
            new NotBlank() // No, really... it can't be empty
        ));

        echo "Rule 2:\n";
        echo "Value is optional, so it can be null. However, if value is provided, it must be a valid email address.\n";

        $values = array(NULL, '', 'foobar', 'foobar@foobar.com');
        $expectedResults = array(TRUE, FALSE, FALSE, TRUE);

        echo "First attempt...\n";
        $this->test($values, $expectedResults, array(
            new Email() // Must be a valid email, presumably all emails are strings
        ));
        echo "Second attempt...\n";
        $this->test($values, $expectedResults, array(
            new Type(array('type' => 'string')), // Must be a string (let's be explicity this time)
            new Email() // Must be a valid email
        ));
        echo "Third attempt...\n";
        $this->test($values, $expectedResults, array(
            new NotBlank(), // Can't be empty
            new Type(array('type' => 'string')), // Must be a string
            new Email() // Must be a valid email
        ));
        echo "Fourth attempt...\n";
        $this->test($values, $expectedResults, array(
            new Type(array('type' => 'string')), // Must be a string
            new Email(), // Must be a valid email
            new NotBlank() // Can't be empty
        ));
    }

    private function test($values, $expectedResults, $validators)
    {
        for ($i = 0, $n = count($values); $i < $n; $i++) {
            $value = $values[$i];
            $expectedResult = $expectedResults[$i];
            $violations = $this->validator->validateValue($value, $validators);
            $result = (count($violations) === 0);
            $valueExport = var_export($value, TRUE);
            echo "  The value {$valueExport} is considered " . ($result ? 'VALID' : 'INVALID') . " <== " . ($result === $expectedResult ? 'EXPECTED' : 'NOT EXPECTED') . "\n";
        }
    }

}

$test = new Test();
$test->run();

I've tested a the set of validators which I believe most people using the library would expect to work. But, after much, experimentation and frustration, they'll realize this cannot be accomplished with the existing Symfony validator logic.

The output of this program is:

Rule 1:
Value is optional, so it can be null. However, if value is provided, it must be a non-empty string.
First attempt...
  The value NULL is considered INVALID <== UNEXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Second attempt...
  The value NULL is considered INVALID <== UNEXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Third attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered VALID <== UNEXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Fourth attempt...
  The value NULL is considered INVALID <== UNEXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Rule 2:
Value is optional, so it can be null. However, if value is provided, it must be a valid email address.
First attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered VALID <== UNEXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED
Second attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered VALID <== UNEXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED
Third attempt...
  The value NULL is considered INVALID <== UNEXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED
Fourth attempt...
  The value NULL is considered INVALID <== UNEXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED

So, what is the underlying problem here?
In the case of Rule 1: The problem is mainly with the NotBlankValidator. The NotBlankValidator is implemented as follows:

public function validate($value, Constraint $constraint)
{
    if (false === $value || (empty($value) && '0' != $value)) {
        $this->context->addViolation($constraint->message);
    }
}

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Constraints/NotBlankValidator.php#L27

This means, using NotBlank, makes it impossible for a string value to be null (optional) and also not blank.

If we modify the logic of NotBlankValidator as follows:

public function validate($value, Constraint $constraint)
{
    if ($value === null) {
        return;
    }

    if (false === $value || (empty($value) && '0' != $value)) {
        $this->context->addViolation($constraint->message);
    }
}

Rerunning the tests gives the following results:

Rule 1:
Value is optional, so it can be null. However, if value is provided, it must be a non-empty string.
First attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Second attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Third attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered VALID <== UNEXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Fourth attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Rule 2:
Value is optional, so it can be null. However, if value is provided, it must be a valid email address.
First attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered VALID <== UNEXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED
Second attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered VALID <== UNEXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED
Third attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED
Fourth attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED

As you can see, this change has corrected many of the previously failed tests.

What about the remaining unexpected results? These are due to the fact that you are ignoring empty strings inside of most string validators.

Let's look at Email and Length (the other two validators used in the code).

        if (null === $value || '' === $value) {
            return;
        }

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Constraints/EmailValidator.php#L30

If I change that line to:

        if (null === $value) {
            return;
        }

Make a similar change in LengthValidator https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Constraints/LengthValidator.php#L28

Now rerun the tests and you'll notice that all of them give the expected result:

Rule 1:
Value is optional, so it can be null. However, if value is provided, it must be a non-empty string.
First attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Second attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Third attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Fourth attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered VALID <== EXPECTED
Rule 2:
Value is optional, so it can be null. However, if value is provided, it must be a valid email address.
First attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED
Second attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED
Third attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED
Fourth attempt...
  The value NULL is considered VALID <== EXPECTED
  The value '' is considered INVALID <== EXPECTED
  The value 'foobar' is considered INVALID <== EXPECTED
  The value 'foobar@foobar.com' is considered VALID <== EXPECTED

So, as you can see... in string based validators, ignoring empty strings leads to unusual and unexpected behavior. Moreover, considering NULL invalid in NotBlank also leads to issues.

Here's a list of validators that all exhibit this issue:

CardSchemeValidator.php: if (null === $value || '' === $value) {
CountryValidator.php: if (null === $value || '' === $value) {
CurrencyValidator.php: if (null === $value || '' === $value) {
DateValidator.php: if (null === $value || '' === $value || $value instanceof \DateTime) {
EmailValidator.php: if (null === $value || '' === $value) {
ExpressionValidator.php: if (null === $value || '' === $value) {
FileValidator.php: if (null === $value || '' === $value) {
IbanValidator.php: if (null === $value || '' === $value) {
ImageValidator.php: if ($failed || null === $value || '' === $value) {
IpValidator.php: if (null === $value || '' === $value) {
IsbnValidator.php: if (null === $value || '' === $value) {
IssnValidator.php: if (null === $value || '' === $value) {
LanguageValidator.php: if (null === $value || '' === $value) {
LengthValidator.php: if (null === $value || '' === $value) {
LocaleValidator.php: if (null === $value || '' === $value) {
LuhnValidator.php: if (null === $value || '' === $value) {
RegexValidator.php: if (null === $value || '' === $value) {
TimeValidator.php: if (null === $value || '' === $value || $value instanceof \DateTime) {
UrlValidator.php: if (null === $value || '' === $value) {

I think if we were to fix those issues, most, if not all, of the validators would work as users would expect them to.

Thank you for the detailed research!

I agree very much about the confusing and inconsistent handling of empty strings, and, to be honest, I can't remember why I did it this way in the first place. I just checked the commit history on GitHub, which shows that the handling of empty strings has always been that way since I committed the first prototype of the Validator component in 6e310bd. I think this seemed like a good idea back then (from a web form perspective) and nobody ever questioned it since.

Even though the changes you propose make sense, they are a very heavy BC break that we can't make before Symfony 3.0.

What may be possible is to provide a parallel set of validators for the constraints you outlined above (NotBlank, CardScheme, Country etc.). Then we'd need another ConstraintValidatorFactory which creates these other validators and which can be used on demand, either by passing it manually when building the Validator, or by setting a configuration option in config.yml.

However, I don't know how other users feel about it. So I suggest that you create either an issue with a very detailed description or even a PR which implements this change (or a prototype). Then we can gather feedback and see if this can be included in the core framework.

Hi @natorojr, thanks for investigating this so deeply.

I agree with just that the use-cases are good and common and that there seems to be a problem.

The solution for use-case 1 is the third attemt because you don't need the NotBlank for optional fields (whereas the other attempts falsely use it). The only problem seems to be the handling of '' in the validators. It makes really no sense that the LengthValidator accepts an empty string. It is exactly the purpose of the Lengh(min=3) validator that the string must be at least 3 characters long, so why should it accept 0 characters...

I think fixing this would be sufficient and the NotBlank validator is ok. It should not allow null because that is what it is for. If you need optional fields, just don't use a NotBlank constraint and you are fine.

Also one more point. The problem with '' in the validators has probably not really come up until now because the form component translates empty fields ('') to null automatically. So there are generally no cases where '' is validated using forms. Only when using the validator standalone, you realize the problem actually and the meaning it has (impossible use-cases as given above).

Hi @webmozart & @Tobion,

I'm glad I was finally able to convey the inconsistencies and unexpected behaviors I was witnessing which initially prompted me to open the bug. I struggled describing the problem at first, but I think the examples helped clarify the underlying issues.

@webmozart,

I completely understand the concern around potentially breaking BC should these changes be made in the 2.x series. The effort involved in testing all possible combinations of Validators that these changes could potentially effect would be a considerable effort in itself. I think it's worth an hour of my time to analyze what the ripple affect would be; at minimum, I could make the changes locally and see how badly the unit tests break. Symfony 3.0 could be any time between 3 and 5 years away, so waiting until them seems unreasonable (for me).

I like your idea of creating a separate set of Validators which exhibits a more consistent and expected behavior and loading those validators using a different ConstraintValidatorFactory (via framework configuration or dependency injection). The only downside to that approach would be having to maintain a lot of duplicate code (we're essentially just changing a single line in a subset of files -- the rest of the code remains unchanged).

Just some random alternative options (off the top of my head):

• We could branch or fork the component and maintain a separate line of development, merge changes as neeeded, and geenrate tags/releases separately for Composer/Packagist distribution purposes.
• Another option is to have some sort of global parameter which can easily be switched at the configuration level to change the behavior of the Validator E.g., "treatEmptyStringSameAsNull", or something like that, which would be TRUE by default (for BC) but could easily be switched off.

framework:
    validation:
        treat_empty_string_same_as_null: true

Like I said, just some ideas.

@Tobion,

I agree that leaving the NotBlankValidator as-is and only fixing the other validators I mentioned is probably sufficient and will fix the majority of the inconsitencies. Having said that, there are two main reason why I think it might be worth also fixing NotBlank:

(1) The Hibernate NotBlankValidator considers null values valid (or rather bypasses them)

https://github.com/hibernate/hibernate-validator/blob/master/engine/src/main/java/org/hibernate/validator/internal/constraintvalidators/NotBlankValidator.java#L43

(2) Doing so would reach ultimate consistency across all validators and validator sequences. In other words, we'd make it such that NotNull becomes the only way to specifiy that a value is required. Without NotNull, all validators (or validator sequences) become optional --- meaning they are only validated if a value is provided. Another way to think of it would be: all validation is optional (only runs if a non-null value is specified) unless NotNull is specified as the first validator (which makes the value required).

IMHO, I think that gives the user the most flexible and intuitive results.

Re: forms and validation. Thanks for pointing out that the Form component converts empty fields to nulls automatically. I did not know that. But it certainly explains why this problem has not been reported previously.

As much as I love and appreciate the power of the Form component, I find myself using it less these days. This is probably due to the shift in frontend web development towards RIAs and SPAs vs traditional web forms. Nevertheless, a good Validation framework/library is essential to any application regardless of how the data/objects to be validated are being populated (be it web form, or AJAX, or API). For that reason, I think it's important to make Symfony's Validation component truely independent and reusable with or without Symfony Forms (as well as inside and outside of the Symfony framework itself).

I having been using the Valdiator component without forms for sometime now, which might be the reason why I've noticed these issues more often than most.

Well, I'd like to help out in any way I can. Let's discuss what the best plan of action is to improve the consistency and behavior of the Validation component moving forward and I will do my best to contribute towards the efforts.

@webmozart,

I think I've devised a way to work this into the framework sooner than later, while avoiding duplicate code (maintenance headaches) and without breaking BC.

Why don't we just use basic inheritance and overridden methods? Allow me to explain...

Note: I'm going to use EmailValidator as an example, but I'm sure you'll be able to see how this is easily generalized/repeatable across all Validators.

Removing comments and extra whitespace from all code for brevity

Suppose we changed EmailValidator to the following:

<?php

namespace Symfony\Component\Validator\Constraints;

use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
use Symfony\Component\Validator\Exception\UnexpectedTypeException;

class EmailValidator extends ConstraintValidator
{

    public function validate($value, Constraint $constraint)
    {
        if ($this->skipValidation($value)) {
            return;
        }

        if (!is_scalar($value) && !(is_object($value) && method_exists($value, '__toString'))) {
            throw new UnexpectedTypeException($value, 'string');
        }
        $value = (string) $value;
        $valid = filter_var($value, FILTER_VALIDATE_EMAIL);
        if ($valid) {
            $host = substr($value, strpos($value, '@') + 1);
            if ($valid && $constraint->checkMX) {
                $valid = $this->checkMX($host);
            } elseif ($valid && $constraint->checkHost) {
                $valid = $this->checkHost($host);
            }
        }
        if (!$valid) {
            $this->context->addViolation($constraint->message, array('{{ value }}' => $value));
        }
    }

    private function checkMX($host)
    {
        return checkdnsrr($host, 'MX');
    }

    private function checkHost($host)
    {
        return $this->checkMX($host) || (checkdnsrr($host, "A") || checkdnsrr($host, "AAAA"));
    }

    protected function skipValidation($value)
    {
        return (null === $value || '' === $value);
    }

}

Then, suppose we introduced a new constraint and corresponding validator called StrictEmail which simply extended the default Email, like so:

<?php

namespace Symfony\Component\Validator\Constraints;

class StrictEmailValidator extends EmailValidator
{

    protected function skipValidation($value)
    {
        return (null === $value);
    }

}

As you can see, I'm simply encapsulating the conditional expression (null/empty check) into an inheritable/overridable method. This allows subclasses of the validator to determine if and when it should exit early or allow validation to proceed based on the specified value.

Since most of the validators only bypass validation when $value is null, we could even add to the base ConstraitValidator, as follows:

<?php
namespace Symfony\Component\Validator;

abstract class ConstraintValidator implements ConstraintValidatorInterface
{
    protected $context;

    public function initialize(ExecutionContextInterface $context)
    {
        $this->context = $context;
    }

    protected function skipValidation($value)
    {
        return (null === $value);
    }
}

By default, users will get the old behavior and, hence, it wouldn't break backwards compatibility. If a user, like myself, wants to use a stricter definition of what values can be ignored, they simply need to use StrictEmail instead of the default Email constraints.

Using these basic OO principles gives us the added benefit of not having to maintain duplicate code (or entire copies of classes) just to change the behavior of one line. The actual validation logic is reused in either case.

This seems like the overall best approach: straightforward/clean solution, easy to implement, no BC breakage, easy to maintain.

We can keep the "Strict" version of each validator in place until Symfony 3.0, at which point the "Strict" version could become the default.

What do you guys think? I can work on a PR during my spare time over the next couple of weeks if you think this solution is likely to get approved by the powers that be.

Hibernates NotBlank validator only works with strings whereas in Symfony it also forbids false and 0. So the handling of null does not seem the only difference. In this regard NotBlank seems to do the same as Legth(min=1) in hibernate.

I like your idea of creating a separate set of Validators which exhibits a more consistent and expected behavior and loading those validators using a different ConstraintValidatorFactory (via framework configuration or dependency injection). The only downside to that approach would be having to maintain a lot of duplicate code

I don't think that's true. We can just extract most of the code into a new base class, and keep the current classes as subclass with an additional a priori check for empty strings, e.g.

class FixedLengthValidator
{
    public function isValid($value, $constraint)
    {
        // most of the current code
    }
} 

class LengthValidator extends FixedLengthValidator
{
    public function validate($value, $constraint)
    {
        if ('' === $value) {
            return;
        }

        parent::validate($value, $constraint);
    }
}

(The name FixedLengthValidator is of course horrible, let's search for something better there.)

• P.S.: I like the proposal StrictLengthValidator

If we go that route and fix the current validators, we should also take care of another issue, namely the dependency of some validators on type validators (#6799). For example, consider:

/**
 * @Type("integer")
 * @Range(min=3)
 */
private $number;

If $number contains anything else but an integer, validating it will generate two errors, one from Type and one from Range. What's worse, some validators even fail with an UnexpectedTypeException if the input is invalid, which doesn't make sense if I combine the constraint with a Type constraint anyway.

I'm not sure how to fix this.

(i) The easiest solution is to let validators silently ignore

• null values
• values of an unexpected input type

For example:

class FixedRangeValidator
{
    public function validate($value, $constraint)
    {
        if (null === $value || !is_numeric($value)) {
            return;
        }

        parent::validate($value, $constraint);
    }
}

With this solution we split the constraints into three orthogonal vectors:

• NotNull for validating that a value is present
• Type for validating the value's type
• everything else for validating the content, given that the value has a specific type and is not null

The (very big) downside is that you then have to declare the Type constraint on every property, except if you prevent invalid values already in your setters (which leads using a validation library ad absurdum..).

(ii) A more complicated solution is to do the same as in (i) and automatically deduce the needed Type constraint from the other constraints that are already set. For example:

class Range extends Constraint
{
    public function getAcceptedTypes()
    {
        return array('numeric');
    }
}

Then doing:

$validator->validateValue($value, array(
    new Range(array('min' => 3)),
));

is implicitly the same as

$validator->validateValue($value, array(
    new Type('numeric'),
    new Range(array('min' => 3)),
));

unless a Type constraint was defined explicitly.

What do you think?

I already went pretty far astray here, but I think that these two issues are very closely related.

(@natorojr for the record, I saw your latest comment just now)

@webmozart Yup, I just saw yours as well. Looks like our messages crossed paths. I'm digesting your last comment now, but, from what I've read so far, looks like we're on the same wavelength.

@webmozart

Seems like we're on the same page regarding how to enhance the existing Validator component to support a more consistent/expected behavior while avoiding duplicate code and maintaining BC. This at least gives me hope of potentially seeing this rolled into the 2.x line in a reasonable time frame.

You bring up a good and interesting point about validator type expectation/dependency. Of course, Hibernate doesn't have this problem because Java is a strongly typed language and, hence, types are checked and enforced at compile time. That's obviously not the case for PHP. To make matters worse, PHP has some funky type juggling/coercion that probably needs to be considered in the context of validation.

Let me read over the referenced bug and think about this particular issues some more before commenting/sharing my thoughts.

The idea to let validators ignore invalid types seems quite bad to me because it opens so much room for security flaws and unexpected behavior. The point of the validator is to validate a value against a while list. So whenever users forget to validate the type for each property, the validation is basically useless because wrong user input like array instead of string (which is very easy to accomplish) will still pass.

Another question is, whether the Range validator will accept int/float or numeric. Allowing numeric will let numeric strings pass as well, which could also lead to security flaws.
Or do we allow Object->__toString() in string validators like Length? This can also have big impact.

Btw https://github.com/symfony/symfony/blob/2.4/src/Symfony/Component/Validator/ConstraintValidatorInterface.php#L36 doesnt mention an UnexpectedTypeException could be thrown currently.

Hello,

Here are my thoughts. Forgive me if I'm repeating some points that have already been mentioned. I just want to layout my entire thought process.

I see three possible ways for a validator to handle a value of unexpected type:
(1) Ignore it (same as @webmozart's Option-i)
(2) Throw an exception (as suggested in #6799)
(3) Consider it invalid (effectively the same as @webmozart's Option-ii)

Regarding (1): I completely agree with @Tobion's sentiments. I think this is a bad idea. In my experience, the primary purpose of a Validation library, and why most people use them, is to ensure that objects (or other such data structures) are populated with exactly the values they expect (and/or business domain requires). In many ways, a validation library is a security measure; it safeguards and protects other parts of your system (particularly your data abstraction layer) from processing invalid data. For this reason, I strongly beleive that validation libraries need to lean on the side of being overly strict; a lenient validation library is a potential vulnerability. So, I agree that this approach defeats the purpose of a validation library.

Regarding (2): This seems like a sensible solution at first. After all, throwing exceptions is generally the OO way of dealing with unexpected situations at runtime. Having said that, I also agree with some of the other comments posted in #6799. Mainly that having to wrap every call to your validation service within a try-catch statement seems inconvenient and redundant (from the user's perspective). If validators were to throw exceptions on unexpected values, users would be forced to handle validation errors in two completely different ways: catching exceptions and inspecting the ConstrainViolation(List). Again, this seems a bit counter productive. The ConstraintViolation(List) API already provides a nice OO abstraction for dealing with the set of validation errors affecting your input.

Regarding (3): Of these three options, this may be the best (in my opinion). I think most would agree that any value of an incorrect type qualifies as a validation error. So, why not just add a ConstraintViolation whenever an unexpected type (other than null) is encountered? This approach leads to consistent results and ensures that there is only one way for the validation service to report validation errors back to the user --- via the ConstraintViolation(List) abstraction.

Of course, as @webmozart alluded to, this comes with some implications... Namely that every Validator would need to document and enforce the type(s) of values it was intended to validate. Personally, I don't think this is a big deal, as the benifits outweigh the alternatives.

So, let's consider some of the examples previously used.

$validator->validateValue($value, array(
    new StrictRange(array('min' => 3))
));

Result: This would only consider NULL and numeric values >= 3 as valid. All other values are invalid.

In most cases, this approach eliminates the need for having to explicitly specify a Type constraint because types would be enforced by each individual validator.

However, @Tobion brought up a good point. What about the difference between integers and floats? This distinction often matters and needs to be safeguarded.

In lieu of creating two separate classes StrictIntRange and StrictFloatRange, I propose that the user must enforce a specific type by explicitly configuring the Type constraint. As follows...

$validator->validateValue($value, array(
    new Type(array('type'=>'integer')),
    new StrictRange(array('min' => 3))
));

Result: This would only consider NULL and integer values >= 3 as valid. All other values are invalid.

In other words, in order to avoid duplicate logic, I think it's okay for validators to expect one or more types. The user can always specify a strict type by explicitly using the Type constraint.

One potential caveat with this approach is how to deal with violation messages. If a validator encounters an unexpected type and adds a constraint violation, should the user be allowed to customize that message like any other violation message? I don't see why not. By default, the message could be "This value should be of type {{ type }}" (borrowed from the Type constraint). But what if the user wants to change that? It seems like every constraint would have to support a "typeMessage" parameter which would allow them to override the default error message.

Well, the good news is that this approach could be easily implemented as part of the "Strict" validator class extensions we had already proposed earlier in the conversation... which means no BC breaks.

In most cases, this approach eliminates the need for having to explicitly specify a Type constraint because types would be enforced by each individual validator.

That doesn't work, because - by default - all constraints specified on a value are validated at once. That means that you get duplicate errors if you specify multiple constraints with the same type requirements.

One potential caveat with this approach is how to deal with violation messages. If a validator encounters an unexpected type and adds a constraint violation, should the user be allowed to customize that message like any other violation message? I don't see why not. By default, the message could be "This value should be of type {{ type }}" (borrowed from the Type constraint). But what if the user wants to change that? It seems like every constraint would have to support a "typeMessage" parameter

My previous point, and this point, are the reasons why the type requirements should be packed into a Type constraint - unless there is an existing one present. If you want to customize the types, or the message, you just have to provide a Type constraint manually.

It's not so simple though. As I said before, by default, all constraints belonging to the currently validated group are executed at once by default, regardless of whether one of them throws an exception or not. Now we want to create a dependency between the Type constraint and other constraints. This is effectively the same as group sequences: When the Type constraint fails, the other constraints should not be validated. But what happens if a manual group sequence is being executed?

The easiest solution to our problem seems to (I'm partially repeating myself for the sake of completeness):

• ignore null in all validators except NotNull
• specify expected types in all validators except NotNull and Type
• ignore unexpected types in those validators
• if no Type constraint is present, but other constraints are:
  • add a Type constraint with the intersection of all types specified by the given constraints
  • set the groups of the Type constraint to the union of all groups specified by the constraints

That means that:

/**
 * @Date(groups="Foo") <- expects string or DateTime
 * @GreaterThan("2014-01-01", groups="Bar") <- expects string, numeric or DateTime
 */
private $date;

implicitly results in

/**
 * @Type({"string", "\DateTime"}, groups={"Foo", "Bar"})
 * @Date(groups="Foo") <- expects string or DateTime
 * @GreaterThan("2014-01-01", groups="Bar") <- expects string, numeric or DateTime
 */
private $date;

This way, the Type constraint is validated whenever a group containing any of the other constraints is validated. Also, since the other constraints ignore invalid types, only one violation is generated if $date contains an invalid type.

To conclude: I think this solution is reasonable. I'm currently refactoring the internals of the Validator component to solve some of the currently existing issues, after that we can look into this.

@webmozart,

You're absolutely correct! Silly me. I completely overlooked the fact that all specified constraints in the group are tested (regardless of whether a previous one failed). My suggested approach would have correctly validated values that satisfy the constraints, but it would also have resulted in duplicate violations for values that do not. Obviously, that's not what we want. Apologies for that.

Some validation frameworks stop validation on first error, by default. But that's not the case here. Otherwise we wouldn't be having many of these conversations to begin with. The fact that all constraints in a group are tested (no matter what) makes this a tricky problem to solve (especially with PHP being loosely typed). I can't remember off the top of my head if Hibernate (or JSR-303/349) has such a concept as "stopping on first violation". I'll take a quick look, but I'm assuming you would have implemented it by now if there were.

Regarding your proposed solution: At this point, seems like it might be the only way to workaround this issue. Your fourth bullet point ("ignored unexpected types") makes me feel a bit uneasy, as it doesn't seem like something a validation engine should do... but I understand now why it would be necessary to solve the problem.

If that is the approach taken, then my only suggestion would be that it be accompanied by clear and thorough documentation.

Let me know if I can provide any other insight or help. Thanks for your time and consideration regarding this issue.

I can't remember off the top of my head if Hibernate (or JSR-303/349) has such a concept as "stopping on first violation".

Yes, that is called "group sequences". But as I've mentioned, they usually need to be specified manually, like so:

/**
 * @GroupSequence({"User", "Slow"})
 */
class User
{
    // ... constraints in group "Default" (="User") and others in group "Slow" ...
}

If a violation is thrown by any of the constraints in group "Default", the constraints in the group "Slow" will be skipped.

However, combining these user-defined group sequences with implicit group sequences for the Type constraints sounds unnecessarily complicated to me.

Replaced by #12312.