From 5ae46289c2dc8df73038a621e022564373dfddf8 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 18 Aug 2021 10:44:19 +1200 Subject: [PATCH] dev/core#2769 use php email validation not hacked qf Per https://lab.civicrm.org/dev/core/-/issues/2769 we have had problems over the years with quickform's email validation and we now have a hacked version that is problematic from a maintenance pov & also doesn't work with the string I have just encountered: name.-o-.i.10@example.com (which I am told is valid and which passes the php filter). We already have an email rule which calls a php native function which is better maintained than our layers of hacks. This PR registers our email rule - which overrides the quickform one. If we merge this we can revert quickform back to unhacked which will improve debugging and maintenance (although it's actually bypassed now with this change) --- CRM/Core/Form.php | 1 + CRM/Utils/Rule.php | 38 ++++++++++++++++++++++++++-- tests/phpunit/CRM/Utils/RuleTest.php | 29 ++++++++++++++++++++- 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index 0f07bdd1496c..5478f87252a0 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -347,6 +347,7 @@ public function registerRules() { 'settingPath', 'autocomplete', 'validContact', + 'email', ]; foreach ($rules as $rule) { diff --git a/CRM/Utils/Rule.php b/CRM/Utils/Rule.php index 9c5707925cb1..862e4603f747 100644 --- a/CRM/Utils/Rule.php +++ b/CRM/Utils/Rule.php @@ -639,12 +639,46 @@ public static function boolean($value) { * * @return bool */ - public static function email($value) { + public static function email($value): bool { + if (function_exists('idn_to_ascii')) { + $parts = explode('@', $value); + foreach ($parts as &$part) { + // if the function returns FALSE then let filter_var have at it. + $part = self::idnToAsci($part) ?: $part; + if ($part === 'localhost') { + // if we are in a dev environment add .com to trick it into accepting localhost. + // this is a bit best-effort - ie we don't really care that it's in a bigger if. + $part .= '.com'; + } + } + $value = implode('@', $parts); + } return (bool) filter_var($value, FILTER_VALIDATE_EMAIL); } /** - * @param $list + * Convert domain string to ascii. + * + * See https://lab.civicrm.org/dev/core/-/issues/2769 + * and also discussion over in guzzle land + * https://github.com/guzzle/guzzle/pull/2454 + * + * @param string $string + * + * @return string|false + */ + private static function idnToAsci(string $string) { + if (!\extension_loaded('intl')) { + return $string; + } + if (defined('INTL_IDNA_VARIANT_UTS46')) { + return idn_to_ascii($string, 0, INTL_IDNA_VARIANT_UTS46); + } + return idn_to_ascii($string); + } + + /** + * @param string $list * * @return bool */ diff --git a/tests/phpunit/CRM/Utils/RuleTest.php b/tests/phpunit/CRM/Utils/RuleTest.php index 52c7da3faeb7..098a0fad5f36 100644 --- a/tests/phpunit/CRM/Utils/RuleTest.php +++ b/tests/phpunit/CRM/Utils/RuleTest.php @@ -306,7 +306,8 @@ public function testCreditCardValidation($number, $type): void { } /** - * Test cvvs + * Test cvvs. + * * @return array */ public static function cvvs(): array { @@ -343,4 +344,30 @@ public function testCvvRule($cvv, $type, $expected): void { $this->assertEquals($expected, CRM_Utils_Rule::cvv($cvv, $type)); } + /** + * Test CVV rule + * + * @param string $email + * @param bool $expected expected outcome of the rule validation + * + * @dataProvider emails + */ + public function testEmailRule(string $email, bool $expected): void { + $this->assertEquals($expected, CRM_Utils_Rule::email($email)); + } + + /** + * Test emails. + * + * @return array + */ + public static function emails(): array { + $cases = []; + $cases['name.-o-.i.10@example.com'] = ['name.-o-.i.10@example.com', TRUE]; + $cases['test@ēxāmplē.co.nz'] = ['test@ēxāmplē.co.nz', TRUE]; + $cases['test@localhost'] = ['test@localhost', TRUE]; + $cases['test@ēxāmplē.co'] = ['test@exāmple', FALSE]; + return $cases; + } + }