NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[Mailer] Fixed 'verify_peer' option in mailer DSN being ignored#37961

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:5.1fromsnake-4:patch-1

Aug 31, 2020

Merged

[Mailer] Fixed 'verify_peer' option in mailer DSN being ignored#37961

fabpot merged 1 commit intosymfony:5.1fromsnake-4:patch-1

Aug 31, 2020

Conversation

Copy link

Contributor

snake-4 commentedAug 27, 2020

Q	A
Branch?	5.1
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	N/A
License	MIT
Doc PR	N/A

The mailer DSN option 'verify_peer' was being ignored because$dsn->getOption('verify_peer', true) was returning a string and thus NOT operator on it was always resulting in false. I propose changing the line where it is used with afilter_var call with theFILTER_VALIDATE_BOOLEAN as the filter parameter to overcome this issue.

carsonbot added Status: Needs Review Mailer Bug labels

Aug 27, 2020

fabpot requested changes

Aug 27, 2020

View reviewed changes

Copy link

Member

fabpot left a comment

Choose a reason for hiding this comment

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

Can you add a test case to cover this fix?

src/Symfony/Component/Mailer/Transport/Smtp/EsmtpTransportFactory.php OutdatedShow resolvedHide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels

Aug 27, 2020

Copy link

ContributorAuthor

snake-4 commentedAug 27, 2020

Can you add a test case to cover this fix?

I've added some test cases to cover this fix in my latest commit but I've also added a check to avoid disabling the verification for a DSN like thissmtps://:@example.com?verify_peer= because it could be misleading to disable it when it clearly doesn't have keywords like no, off or false and is just an empty string.