Skip to content
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.

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

Wrong condition in /mail/EmailAddress.php #739

Closed
smrtab opened this issue Oct 15, 2018 · 1 comment · Fixed by #740
Closed

Wrong condition in /mail/EmailAddress.php #739

smrtab opened this issue Oct 15, 2018 · 1 comment · Fixed by #740
Labels
status: work in progress Twilio or the community is in the process of implementing

Comments

@smrtab
Copy link
Contributor

smrtab commented Oct 15, 2018

Good day,

The next condition on line 73 is wrong (file /mail/EmailAddress.php).
if (!is_string($emailAddress) && filter_var($emailAddress, FILTER_VALIDATE_EMAIL))

It turns out that email address shouldn't be a string and should be a valid address simultaneously in order to throw exception.

Probably the right condition is:
if (!(is_string($emailAddress) && filter_var($emailAddress, FILTER_VALIDATE_EMAIL)))

smrtab added a commit to smrtab/sendgrid-php that referenced this issue Oct 15, 2018
@devchas devchas added the status: work in progress Twilio or the community is in the process of implementing label Oct 17, 2018
thinkingserious added a commit that referenced this issue Oct 22, 2018
@adamjpeterson
Copy link

adamjpeterson commented Sep 25, 2019

FYI this logic seems to fail once every 10k times or so, such that the error is thrown when it shouldn't be. We are forcing the email to a string and legitimate emails are still throwing this exception. We've had to comment out this validation because it craps out too often.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: work in progress Twilio or the community is in the process of implementing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants