Skip to content
This repository has been archived by the owner on Jan 31, 2020. It is now read-only.

fixed a bug where empty $email was passed to idn_to_utf8 #94

Merged
merged 3 commits into from
May 17, 2017

Conversation

Exlord
Copy link
Contributor

@Exlord Exlord commented Jun 23, 2016

checked if $email is not empty before passing it to idn_to_utf8.
idn_to_utf8 throws a warning when value is empty

checked if $email is not empty before passing it to idn_to_utf8
@Exlord
Copy link
Contributor Author

Exlord commented Jun 23, 2016

Test case already exist for empty value
https://github.com/zendframework/zend-validator/blob/master/test/EmailAddressTest.php#L259
but this only happens when intl extension is active.

@@ -547,6 +547,9 @@ protected function idnToAscii($email)
*/
protected function idnToUtf8($email)
{
if(strlen($email) == 0)
return $email;
Copy link
Member

Choose a reason for hiding this comment

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

PSR-2, missing parentheses and space after if

http://www.php-fig.org/psr/psr-2/

@@ -547,6 +547,9 @@ protected function idnToAscii($email)
*/
protected function idnToUtf8($email)
{
if (strlen($email) == 0)
Copy link
Member

Choose a reason for hiding this comment

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

PSR-2 missing opening and closing parentheses: http://www.php-fig.org/psr/psr-2/#5-1-if-elseif-else

Copy link
Contributor Author

@Exlord Exlord Jun 23, 2016

Choose a reason for hiding this comment

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

@webimpress braces where missing not parentheses.

Copy link
Member

Choose a reason for hiding this comment

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

@Exlord sorry, you have right :) thanks for your fix

@michalbundyra
Copy link
Member

Could you also provide test for your change?

@Exlord
Copy link
Contributor Author

Exlord commented Jun 23, 2016

@webimpress Test case already exist for empty value
https://github.com/zendframework/zend-validator/blob/master/test/EmailAddressTest.php#L259
but this only happens when intl extension is active. how can i enable intl extension when writing test case?

@Maks3w
Copy link
Member

Maks3w commented Jun 23, 2016

  1. @Exlord Please review your PR until all checks are successful.
  2. INTL is enabled on CI so the bug should have been detected. So that test is not working as expected or don't cover this use case.

@weierophinney weierophinney merged commit dcbb971 into zendframework:master May 17, 2017
weierophinney added a commit that referenced this pull request May 17, 2017
fixed a bug where empty $email was passed to idn_to_utf8
weierophinney added a commit that referenced this pull request May 17, 2017
weierophinney added a commit that referenced this pull request May 17, 2017
@weierophinney
Copy link
Member

Thanks, @Exlord

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants