8000 [Validator] #18156 In "strict" mode, email validator inaccurately cla… by natechicago · Pull Request #18428 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Validator] #18156 In "strict" mode, email validator inaccurately cla… #18428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

[Validator] #18156 In "strict" mode, email validator inaccurately cla… #18428

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f69769d
[Validator] #18156 In "strict" mode, email validator inaccurately cla…
natechicago Apr 4, 2016
5a7f775
[Validator] #18156 In "strict" mode, email validator inaccurately cla…
natechicago Apr 5, 2016
cf5d97f
[Validator] #18156 In "strict" mode, email validator inaccurately cla…
natechicago Apr 10, 2016
8b448d7
[Validator] #18156 In "strict" mode, email validator inaccurately cla…
natechicago Apr 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[Validator] #18156 In "strict" mode, email validator inaccurately cla… 
…ims certain valid emails are invalid

Changes from code review.
  • Loading branch information
@natechicago
natechicago committed Apr 5, 2016
commit 5a7f7754f49f6234c38123e8a8f09736537d850c
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ private function addValidationSection(ArrayNodeDefinition $rootNode)
->end()
->scalarNode('translation_domain')->defaultValue('validators')->end()
->booleanNode('strict_email')->defaultFalse()->end() // deprecated
->scalarNode('email_profile')->defaultValue('basic')->end()
->scalarNode('email_profile')->defaultNull()->end()
->end()
->end()
->end()
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ protected static function getBundleDefaultConfig()
'static_method' => array('loadValidatorMetadata'),
'translation_domain' => 'validators',
'strict_email' => false, // deprecated
'email_profile' => 'basic',
'email_profile' => null,
),
'annotations' => array(
'cache' => 'file',
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Validator/Constraints/Email.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
*/
class Email extends Constraint
{
const PROFILE_BASIC_REGX = 'basic';
const PROFILE_HTML5_REGX = 'html5';
const PROFILE_BASIC_REGEX = 'basic';
const PROFILE_HTML5_REGEX = 'html5';
const PROFILE_RFC_ALLOW_WARNINGS = 'rfc';
const PROFILE_RFC_DISALLOW_WARNINGS = 'rfc-no-warn';

Expand Down
29 changes: 16 additions & 13 deletions src/Symfony/Component/Validator/Constraints/EmailValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,19 @@ class EmailValidator extends ConstraintValidator
private $defaultProfile;

/**
* @param bool $strict Deprecated. If the constraint does not define a
* validation profile, this will determine if
* rfc-no-warn should be used as the default profile.
* @param string $profile If the constraint does not define a validation
* profile, this will specify which profile to use.
* @param bool $strict Deprecated. If the constraint does not define
* a validation profile, this will determine if
* 'rfc-no-warn' or 'basic' should be used as
* the default profile.
* @param string|null $profile If the constraint does not define a validation
* profile, this will specify which profile to use.
*/
public function __construct($strict = false, $profile = Email::PROFILE_BASIC_REGX)
public function __construct($strict = false, $profile = null)
{
$this->defaultProfile = $strict
? Email::PROFILE_RFC_DISALLOW_WARNINGS
$this->defaultProfile = (null === $profile)
? $strict
? Email::PROFILE_RFC_DISALLOW_WARNINGS
: Email::PROFILE_BASIC_REGEX
: $profile;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using if instead of nesting the ternary operator would increase readability.

}

Expand All @@ -62,7 +65,7 @@ public function validate($value, Constraint $constraint)
if (isset($constraint->strict)) {
$constraint->profile = $constraint->strict
? Email::PROFILE_RFC_DISALLOW_WARNINGS
: Email::PROFILE_BASIC_REGX;
: Email::PROFILE_BASIC_REGEX;
}

if (null === $constraint->profile) {
Expand All @@ -71,9 +74,9 @@ public function validate($value, Constraint $constraint)

// Determine if the email address is valid
switch ($constraint->profile) {
case Email::PROFILE_BASIC_REGX:
case Email::PROFILE_HTML5_REGX:
$regex = ($constraint->profile === Email::PROFILE_BASIC_REGX)
case Email::PROFILE_BASIC_REGEX:
case Email::PROFILE_HTML5_REGEX:
$regex = (Email::PROFILE_BASIC_REGEX === $constraint->profile)
? '/^.+\@\S+\.\S+$/'
: '/^[a-zA-Z0-9.!#$%&’*+\/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$/';
$emailAddressIsValid = (bool) preg_match($regex, $value);
Expand All @@ -89,7 +92,7 @@ public function validate($value, Constraint $constraint)
$emailAddressIsValid = $rfcValidator->isValid(
$value,
false,
$constraint->profile === Email::PROFILE_RFC_DISALLOW_WARNINGS
Email::PROFILE_RFC_DISALLOW_WARNINGS === $constraint->profile
);
break;
default:
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Component/Validator/Tests/Constraints/EmailValidatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function testExpectsStringCompatibleType()
*/
public function testBasicValidationProfile($emailData)
{
$this->runValidationProfileTest(Email::PROFILE_BASIC_REGX, $emailData);
$this->runValidationProfileTest(Email::PROFILE_BASIC_REGEX, $emailData);
}

/**
Expand All @@ -64,7 +64,7 @@ public function testBasicValidationProfile($emailData)
*/
public function testHtml5ValidationProfile($emailData)
{
$this->runValidationProfileTest(Email::PROFILE_HTML5_REGX, $emailData);
$this->runValidationProfileTest(Email::PROFILE_HTML5_REGEX, $emailData);
}

/**
Expand Down Expand Up @@ -189,8 +189,8 @@ protected function buildEmailData($email, $basic, $html5, $rfc, $rfcNoWarn)
array(
$email,
array(
Email::PROFILE_BASIC_REGX => $basic,
Email::PROFILE_HTML5_REGX => $html5,
Email::PROFILE_BASIC_REGEX => $basic,
Email::PROFILE_HTML5_REGEX => $html5,
Email::PROFILE_RFC_ALLOW_WARNINGS => $rfc,
Email::PROFILE_RFC_DISALLOW_WARNINGS => $rfcNoWarn,
),
Expand Down
0