concretecms / concrete5-legacy

Legacy repository for concrete5
http://www.concrete5.org
559 stars 323 forks source link

fix email validation #1977

Closed Remo closed 6 years ago

Remo commented 6 years ago

emails addresses with a TLD longer than 3 characters are considered invalid, let's fix this.

ConcreteOwl commented 6 years ago

A couple of year ago I posted a hack for this in the forums.. https://www.concrete5.org/community/forums/customizing_c5/patch-5.6-to-accept-new-top-level-domain-extensions Your solution look much more elegant @Remo

Remo commented 6 years ago

@weyboat if you happen to find such issues, please post them on github. There are too many things going on in the forum, can't monitor these things, but I'm happy to merge some code into this repo if something shows up.

ConcreteOwl commented 6 years ago

Okay @Remo I will in future, thanks for the heads up..

From: Remo Laubacher Sent: Tuesday, April 3, 2018 9:27 AM To: concrete5/concrete5-legacy Cc: David Gibson ; Mention Subject: Re: [concrete5/concrete5-legacy] fix email validation (#1977)

@weyboat if you happen to find such issues, please post them on github. There are too many things going on in the forum, can't monitor these things, but I'm happy to merge some code into this repo if something shows up.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

hissy commented 6 years ago

It allows long TLDs but disallows multibyte domain names.

Remo commented 6 years ago

@hissy how would you fix this? Call to idn_to_ascii() first? I think it would be nice if we could rely on PHP functions and not a custom regex.

Remo commented 6 years ago

Something like this (untested)?

    public function email($em, $testMXRecord = false) {
        $emailParts = explode('@', $em);
        if (count($emailParts) == 2 && filter_var($emailParts[0] . '@' . idn_to_ascii($emailParts[1]), FILTER_VALIDATE_EMAIL)) {
            if ($testMXRecord && function_exists('getmxrr')) {
                list($username, $domain) = explode( '@', $em );
                return getmxrr($domain, $mxrecords);
            } else {
                return true;
            }
        } else {
            return false;
        }
    }
hissy commented 6 years ago

How about adding a 3rd party library?

https://github.com/dominicsayers/isemail

This library is referenced from another library that used in version8

https://github.com/egulias/EmailValidator

Remo commented 6 years ago

Would be nice if we wouldn't have to add new dependencies, but we'd certainly have to make sure it works with the ancient PHP versions. The latter requires PHP 5.5 which I think would be a deal breaker.

hissy commented 6 years ago

"EmailValidator" requires php5.5 and it depends on composer, but isemail function is only one file and it seems works on old php (not yet tested)