TYPO3.Flow

Bastian Waidelich wrote:

In my opinion, the validator should allow an email address if in doubt (e.g. containig "+" or "@localhost") so I agree to Karstens comment on https://review.typo3.org/#/c/2665/
But if you think this is required, we could add a validator option to use filter_var() instead.

These option is really a good idea.. so we can have both :)

So, if Karsten is right, I don't know why this should be discussed again. You cannot provide a validator for every case out there. So why not decide the behaviour and point that out clearly? If anyone has a different usecase he has to write his own validator then.

I just agree in using filter_var as long as it really supports the RFC. If it has bugs, it's not better then a preg_match. But IMHO introducing options for changing/choosing a behaviour is just sick.

Hi Bastian,

Bastian Waidelich wrote:

In my opinion, the validator should allow an email address if in doubt (e.g. containig "+" or "@localhost") so I agree to Karstens comment on https://review.typo3.org/#/c/2665/

Could you explain why you and Karsten think so?

For testing email sending and receiving this might be fine to use such an email address, but for an application used in production, I would expect that an address like "example@localhost" fails to validate.
I'm not alone with that opinion. Symfony explicitly expects "user@localhost" to be invalid: https://github.com/symfony/Validator/blob/master/Tests/Constraints/EmailValidatorTest.php

They even fixed their validator to have this behaviour, for PHP versions below 5.3.3 where such an email passed the filter_var() test. https://github.com/symfony/symfony/commit/d235570653fe5c79961e0ab61a3317a954e0463b

According to RFC 5321 only email addresses with a resolvable FQDNs are considered to be valid. "foo@localhost" is never resolvable thus always invalid.

However it seems that "fred@com" is valid, as com is a TLD and resolvable. (see last comment here: https://bugs.php.net/bug.php?id=49576#1282080887)

• Add a string length check (like in t3lib_div::validEmail) as a security measurement for PHP < 5.3.4
• Use filter_var() as the validation is correct as of PHP 5.3.3
• Add an additional check for PHP 5.3.3 like Symfony did
• Change the tests accordingly

• All of the above
• Transform
• Add an option to additionally do an MX check (see Symfony EmailValidator)

This would still fail to validate mail addresses like "fred@com", I have no solution for that but I still find that being too restrictive in an validator is better than too loose at least for the default case.

Alexander Schnitzler wrote:

I just agree in using filter_var as long as it really supports the RFC. If it has bugs, it's not better then a preg_match.

The only known bug is the one with not validating "foo@com".

But IMHO introducing options for changing/choosing a behaviour is just sick.

Having an option to somehow validate this to true (in combination with an MX check) would make sense to me.

After all, what seemed to be easy at first sight (just use filter_var), is not that easy if you have a closer look.

In the end, neither t3lib_div::validEmail is absolutely correct, nor the EmailValidator in FLOW3 or Extbase.
But if you follow my arguments above: 3lib_div::validEmail is more accurate, more consistent and more secure than the
current FLOW3 Validator and this is why I will approve the change in Extbase #6970 to use this method in favor of the regex.

I leave it to the FLOW3 team to decide whether they agree to this or if the code base diverges in this case.

[...] but I still find that being too restrictive in an validator is better than too loose at least for the default case.

You're absolutely right with it!

Alexander Schnitzler wrote:

I just agree in using filter_var as long as it really supports the RFC. If it has bugs, it's not better then a preg_match.

The only known bug is the one with not validating "foo@com".

Alright, didn't know this is the only bug but then we could discuss to fix this (next quote) or just clearly point out the new behaviour.

But IMHO introducing options for changing/choosing a behaviour is just sick.

Having an option to somehow validate this to true (in combination with an MX check) would make sense to me.

Full ack here too if it's the way you described it. I just wouldn't add an option to choose between pure filter_var and the current preg_match.

Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13608

at https://github.com/php/php-src/blob/master/ext/filter/logical_filters.c#L499 there are some interesting comments about the intention of failing particular occurrences (regarding that @com for example). Maybe that gives some insight.

Helmut Hummel wrote:

Hi Helmut,

Bastian Waidelich wrote:

In my opinion, the validator should allow an email address if in doubt (e.g. containig "+" or "@localhost") so I agree to Karstens comment on https://review.typo3.org/#/c/2665/

Could you explain why you and Karsten think so?

I can't speak for Karsten of course, but I'm in favour of less strict validators usually because they're mostly not about security but about preventing the user from mistyping - if a hacker can't use foo@localhost he will use foo@localhost.com *g
I currently work on a registration where username = email address. I use local email addresses to test this, otherwise I had to create real emails or disable the validator while testing.
Apart from that it would be quite annoying if someone could not register with my service because his email address is considered invalid.

But apparently "+" is allowed by filter_var() in fact and to work around the other issue I can use a "proper" local domain. So, I'm happy with the change :)

Bastian Waidelich wrote:

if a hacker can't use foo@localhost he will use foo@localhost.com

Ha, it seems redmine doesn't support @localhost emails either :-D

Patch set 3 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13608