From 6dd023f255ae2a6924e6187193267f03ca231c73 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Wed, 22 Mar 2017 15:02:58 +0100 Subject: [PATCH 01/16] [Validator] Allow checkMX() to return false when $host is empty | Q | A | | --- | --- | | Branch? | master | | Bug fix? | yes | | New feature? | no | | BC breaks? | no | | Deprecations? | no | | Tests pass? | yes | | Fixed tickets | #22106 | | License | MIT | | Doc PR | n/a | --- .../Component/Validator/Constraints/EmailValidator.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Symfony/Component/Validator/Constraints/EmailValidator.php b/src/Symfony/Component/Validator/Constraints/EmailValidator.php index 6ae25e016d..d6b6a9021c 100644 --- a/src/Symfony/Component/Validator/Constraints/EmailValidator.php +++ b/src/Symfony/Component/Validator/Constraints/EmailValidator.php @@ -138,6 +138,10 @@ class EmailValidator extends ConstraintValidator */ private function checkMX($host) { + if (null === $host || '' === $host) { + return false; + } + return checkdnsrr($host, 'MX'); } From 6b0702e52a730062735c3f915f22caffbf25f677 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Thu, 23 Mar 2017 11:01:22 +0100 Subject: [PATCH 02/16] Add empty check on host in other methods + add unit tests Add empty on host in other methods where checkdnsrr is called and add unit tests to prevent future regressions --- .../Component/Validator/Constraints/EmailValidator.php | 6 +++++- .../Component/Validator/Constraints/UrlValidator.php | 2 +- .../Validator/Tests/Constraints/EmailValidatorTest.php | 6 ++++++ .../Validator/Tests/Constraints/UrlValidatorTest.php | 1 + 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/EmailValidator.php b/src/Symfony/Component/Validator/Constraints/EmailValidator.php index d6b6a9021c..cf334e5e51 100644 --- a/src/Symfony/Component/Validator/Constraints/EmailValidator.php +++ b/src/Symfony/Component/Validator/Constraints/EmailValidator.php @@ -138,7 +138,7 @@ class EmailValidator extends ConstraintValidator */ private function checkMX($host) { - if (null === $host || '' === $host) { + if ('' === $host) { return false; } @@ -154,6 +154,10 @@ class EmailValidator extends ConstraintValidator */ private function checkHost($host) { + if ('' === $host) { + return false; + } + return $this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA')); } } diff --git a/src/Symfony/Component/Validator/Constraints/UrlValidator.php b/src/Symfony/Component/Validator/Constraints/UrlValidator.php index db7244f40c..7e3a074e6c 100644 --- a/src/Symfony/Component/Validator/Constraints/UrlValidator.php +++ b/src/Symfony/Component/Validator/Constraints/UrlValidator.php @@ -80,7 +80,7 @@ class UrlValidator extends ConstraintValidator if ($constraint->checkDNS) { $host = parse_url($value, PHP_URL_HOST); - if (!checkdnsrr($host, 'ANY')) { + if ('' === $host || !checkdnsrr($host, 'ANY')) { if ($this->context instanceof ExecutionContextInterface) { $this->context->buildViolation($constraint->dnsMessage) ->setParameter('{{ value }}', $this->formatValue($host)) diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php index cfa0e99c92..f623129867 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php @@ -159,4 +159,10 @@ class EmailValidatorTest extends AbstractConstraintValidatorTest $this->assertNoViolation(); } + + public function testEmptyHostReturnsFalse() + { + $this->assertFalse($this->validator->checkMX('')); + $this->assertFalse($this->validator->checkHost('')); + } } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/UrlValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/UrlValidatorTest.php index 39f1708cf1..34ef99e018 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/UrlValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/UrlValidatorTest.php @@ -171,6 +171,7 @@ class UrlValidatorTest extends AbstractConstraintValidatorTest array('http://example.com/exploit.html?'), array('http://example.com/exploit.html?hel lo'), array('http://example.com/exploit.html?not_a%hex'), + array('http:/.com'), ); } From 197d19f34cc700a4a890bbfd565d387d4bc570d4 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Thu, 23 Mar 2017 11:43:53 +0100 Subject: [PATCH 03/16] Remove malformed EmailValidatorTest + Update UrlValidator test Remove malformed EmailValidatorTest + Update UrlValidator test --- .../Validator/Tests/Constraints/EmailValidatorTest.php | 6 ------ .../Validator/Tests/Constraints/UrlValidatorTest.php | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php index f623129867..cfa0e99c92 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php @@ -159,10 +159,4 @@ class EmailValidatorTest extends AbstractConstraintValidatorTest $this->assertNoViolation(); } - - public function testEmptyHostReturnsFalse() - { - $this->assertFalse($this->validator->checkMX('')); - $this->assertFalse($this->validator->checkHost('')); - } } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/UrlValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/UrlValidatorTest.php index 34ef99e018..a581c9e082 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/UrlValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/UrlValidatorTest.php @@ -171,7 +171,7 @@ class UrlValidatorTest extends AbstractConstraintValidatorTest array('http://example.com/exploit.html?'), array('http://example.com/exploit.html?hel lo'), array('http://example.com/exploit.html?not_a%hex'), - array('http:/.com'), + array('http://'), ); } From 60392fdd4357275fc6cf9cb229a903a8750ffd97 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Thu, 23 Mar 2017 12:37:06 +0100 Subject: [PATCH 04/16] fix coding standard to comply with fabbot --- .../Component/Validator/Constraints/EmailValidator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/EmailValidator.php b/src/Symfony/Component/Validator/Constraints/EmailValidator.php index cf334e5e51..a6caa4b4a6 100644 --- a/src/Symfony/Component/Validator/Constraints/EmailValidator.php +++ b/src/Symfony/Component/Validator/Constraints/EmailValidator.php @@ -141,7 +141,7 @@ class EmailValidator extends ConstraintValidator if ('' === $host) { return false; } - + return checkdnsrr($host, 'MX'); } @@ -157,7 +157,7 @@ class EmailValidator extends ConstraintValidator if ('' === $host) { return false; } - + return $this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA')); } } From a090ef855a2ef4d0badff7e13a1a9b2af729f53d Mon Sep 17 00:00:00 2001 From: apetitpa Date: Thu, 23 Mar 2017 12:49:26 +0100 Subject: [PATCH 05/16] Use LF line separator --- src/Symfony/Component/Validator/Constraints/EmailValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Validator/Constraints/EmailValidator.php b/src/Symfony/Component/Validator/Constraints/EmailValidator.php index a6caa4b4a6..3eee66fcd8 100644 --- a/src/Symfony/Component/Validator/Constraints/EmailValidator.php +++ b/src/Symfony/Component/Validator/Constraints/EmailValidator.php @@ -141,7 +141,7 @@ class EmailValidator extends ConstraintValidator if ('' === $host) { return false; } - + return checkdnsrr($host, 'MX'); } From 4fcb24bacb8caa1cfa4fe74a7c25a72b9a6e5a71 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Thu, 23 Mar 2017 13:36:20 +0100 Subject: [PATCH 06/16] Move empty condition in return statement --- .../Validator/Constraints/EmailValidator.php | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/EmailValidator.php b/src/Symfony/Component/Validator/Constraints/EmailValidator.php index 3eee66fcd8..cd635eb136 100644 --- a/src/Symfony/Component/Validator/Constraints/EmailValidator.php +++ b/src/Symfony/Component/Validator/Constraints/EmailValidator.php @@ -138,11 +138,7 @@ class EmailValidator extends ConstraintValidator */ private function checkMX($host) { - if ('' === $host) { - return false; - } - - return checkdnsrr($host, 'MX'); + return ('' !== $host) && checkdnsrr($host, 'MX'); } /** @@ -154,10 +150,6 @@ class EmailValidator extends ConstraintValidator */ private function checkHost($host) { - if ('' === $host) { - return false; - } - - return $this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA')); + return ('' !== $host) && ($this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA'))); } } From 9753a2773e9b8b00c180b0cc7aeb04609ab11d2f Mon Sep 17 00:00:00 2001 From: apetitpa Date: Thu, 23 Mar 2017 14:11:06 +0100 Subject: [PATCH 07/16] Add a test case to prevent future regressions --- .../Component/Validator/Tests/Constraints/EmailValidatorTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php index cfa0e99c92..cadd2b9704 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php @@ -96,6 +96,7 @@ class EmailValidatorTest extends AbstractConstraintValidatorTest array('example@'), array('example@localhost'), array('foo@example.com bar'), + array('foo@bar.fr@'), ); } From 6071ff6c8fbca770312fc934dfc092656ac47212 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Thu, 23 Mar 2017 14:51:52 +0100 Subject: [PATCH 08/16] Remove unnecessary parentheses --- .../Component/Validator/Constraints/EmailValidator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/EmailValidator.php b/src/Symfony/Component/Validator/Constraints/EmailValidator.php index cd635eb136..ce42219e26 100644 --- a/src/Symfony/Component/Validator/Constraints/EmailValidator.php +++ b/src/Symfony/Component/Validator/Constraints/EmailValidator.php @@ -138,7 +138,7 @@ class EmailValidator extends ConstraintValidator */ private function checkMX($host) { - return ('' !== $host) && checkdnsrr($host, 'MX'); + return '' !== $host && checkdnsrr($host, 'MX'); } /** @@ -150,6 +150,6 @@ class EmailValidator extends ConstraintValidator */ private function checkHost($host) { - return ('' !== $host) && ($this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA'))); + return '' !== $host && ($this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA'))); } } From 91b665c12ae6e00de8b74cf666eff24fc26d54cf Mon Sep 17 00:00:00 2001 From: apetitpa Date: Fri, 24 Mar 2017 16:26:16 +0100 Subject: [PATCH 09/16] Switch to `is_string` native method --- src/Symfony/Component/Validator/Constraints/UrlValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Validator/Constraints/UrlValidator.php b/src/Symfony/Component/Validator/Constraints/UrlValidator.php index 7e3a074e6c..a12dc14979 100644 --- a/src/Symfony/Component/Validator/Constraints/UrlValidator.php +++ b/src/Symfony/Component/Validator/Constraints/UrlValidator.php @@ -80,7 +80,7 @@ class UrlValidator extends ConstraintValidator if ($constraint->checkDNS) { $host = parse_url($value, PHP_URL_HOST); - if ('' === $host || !checkdnsrr($host, 'ANY')) { + if (!is_string($host) || !checkdnsrr($host, 'ANY')) { if ($this->context instanceof ExecutionContextInterface) { $this->context->buildViolation($constraint->dnsMessage) ->setParameter('{{ value }}', $this->formatValue($host)) From f390f29173dbbca90a62b61083039d5a03a6bb66 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Fri, 24 Mar 2017 16:26:34 +0100 Subject: [PATCH 10/16] remove non relevant test case --- .../Component/Validator/Tests/Constraints/EmailValidatorTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php index cadd2b9704..cfa0e99c92 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php @@ -96,7 +96,6 @@ class EmailValidatorTest extends AbstractConstraintValidatorTest array('example@'), array('example@localhost'), array('foo@example.com bar'), - array('foo@bar.fr@'), ); } From 6f24b0546723d8764d0c4753fbcbe8f567b72588 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Fri, 24 Mar 2017 17:16:55 +0100 Subject: [PATCH 11/16] Switch to `empty` native function to check emptiness Switch to `empty` because depending on the PHP version, substr returns '' or false --- .../Component/Validator/Constraints/EmailValidator.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/EmailValidator.php b/src/Symfony/Component/Validator/Constraints/EmailValidator.php index ce42219e26..1a771debf9 100644 --- a/src/Symfony/Component/Validator/Constraints/EmailValidator.php +++ b/src/Symfony/Component/Validator/Constraints/EmailValidator.php @@ -138,7 +138,7 @@ class EmailValidator extends ConstraintValidator */ private function checkMX($host) { - return '' !== $host && checkdnsrr($host, 'MX'); + return !empty($host) && checkdnsrr($host, 'MX'); } /** @@ -150,6 +150,6 @@ class EmailValidator extends ConstraintValidator */ private function checkHost($host) { - return '' !== $host && ($this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA'))); + return !empty($host) && ($this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA'))); } } From d973eb1f7d37113fb824ad933d466b227c675e0e Mon Sep 17 00:00:00 2001 From: apetitpa Date: Fri, 24 Mar 2017 17:17:11 +0100 Subject: [PATCH 12/16] Add a test to prevent future regressions --- .../Tests/Constraints/EmailValidatorTest.php | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php index cfa0e99c92..07d748fe08 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php @@ -159,4 +159,32 @@ class EmailValidatorTest extends AbstractConstraintValidatorTest $this->assertNoViolation(); } + + public function getCheckTypes() + { + return array( + array('checkMX', Email::MX_CHECK_FAILED_ERROR), + array('checkHost', Email::HOST_CHECK_FAILED_ERROR), + ); + } + + /** + * @dataProvider getCheckTypes + */ + public function testEmptyHostIsNotValid($checkType, $violation) + { + $this->validator->validate( + 'foo@bar.fr@', + new Email(array( + 'message' => 'myMessage', + $checkType => true, + )) + ); + + $this + ->buildViolation('myMessage') + ->setParameter('{{ value }}', '"foo@bar.fr@"') + ->setCode($violation) + ->assertRaised(); + } } From 894127bf09245c648fea8896ca2a0a8c5d0f628a Mon Sep 17 00:00:00 2001 From: apetitpa Date: Fri, 24 Mar 2017 23:08:25 +0100 Subject: [PATCH 13/16] rename dataset provider --- .../Validator/Tests/Constraints/EmailValidatorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php index 07d748fe08..18d29f2403 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php @@ -160,7 +160,7 @@ class EmailValidatorTest extends AbstractConstraintValidatorTest $this->assertNoViolation(); } - public function getCheckTypes() + public function provideCheckTypes() { return array( array('checkMX', Email::MX_CHECK_FAILED_ERROR), From 1825d0f2aefd9b36f84f7b86871e76429ac7a0f9 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Fri, 24 Mar 2017 23:09:52 +0100 Subject: [PATCH 14/16] cast substr result to string and remove empty function use --- .../Component/Validator/Constraints/EmailValidator.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/EmailValidator.php b/src/Symfony/Component/Validator/Constraints/EmailValidator.php index 1a771debf9..cdd162d816 100644 --- a/src/Symfony/Component/Validator/Constraints/EmailValidator.php +++ b/src/Symfony/Component/Validator/Constraints/EmailValidator.php @@ -93,7 +93,7 @@ class EmailValidator extends ConstraintValidator return; } - $host = substr($value, strrpos($value, '@') + 1); + $host = (string) substr($value, strrpos($value, '@') + 1); // Check for host DNS resource records if ($constraint->checkMX) { @@ -138,7 +138,7 @@ class EmailValidator extends ConstraintValidator */ private function checkMX($host) { - return !empty($host) && checkdnsrr($host, 'MX'); + return '' !== $host && checkdnsrr($host, 'MX'); } /** @@ -150,6 +150,6 @@ class EmailValidator extends ConstraintValidator */ private function checkHost($host) { - return !empty($host) && ($this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA'))); + return '' !== $host && ($this->checkMX($host) || (checkdnsrr($host, 'A') || checkdnsrr($host, 'AAAA'))); } } From c07fb416ff4c87aed6d628a3fa88712a285e76d0 Mon Sep 17 00:00:00 2001 From: apetitpa Date: Fri, 24 Mar 2017 23:17:19 +0100 Subject: [PATCH 15/16] update dataProvider function name --- .../Validator/Tests/Constraints/EmailValidatorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php index 18d29f2403..86f33b0adf 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php @@ -169,7 +169,7 @@ class EmailValidatorTest extends AbstractConstraintValidatorTest } /** - * @dataProvider getCheckTypes + * @dataProvider provideCheckTypes */ public function testEmptyHostIsNotValid($checkType, $violation) { From 7104c4e849c632c3f31b3cc188498c8cd6d2f64f Mon Sep 17 00:00:00 2001 From: apetitpa Date: Mon, 27 Mar 2017 16:23:37 +0200 Subject: [PATCH 16/16] move provider after test --- .../Tests/Constraints/EmailValidatorTest.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php index 86f33b0adf..5933bd5ed6 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php @@ -160,14 +160,6 @@ class EmailValidatorTest extends AbstractConstraintValidatorTest $this->assertNoViolation(); } - public function provideCheckTypes() - { - return array( - array('checkMX', Email::MX_CHECK_FAILED_ERROR), - array('checkHost', Email::HOST_CHECK_FAILED_ERROR), - ); - } - /** * @dataProvider provideCheckTypes */ @@ -187,4 +179,12 @@ class EmailValidatorTest extends AbstractConstraintValidatorTest ->setCode($violation) ->assertRaised(); } + + public function provideCheckTypes() + { + return array( + array('checkMX', Email::MX_CHECK_FAILED_ERROR), + array('checkHost', Email::HOST_CHECK_FAILED_ERROR), + ); + } }