I don't agree with this approach as it's an all or nothing change. You're disabling it for everyone even if they do have a valid configuration and things work correctly if TLS security isn't explicitly set up otherwise. It's on par with saying that since CentOS 5 servers won't work with the current update system CDN when they disable support for older protocols that nobody can have secure connections to the update server.

Maybe i misunderstood but this doesnt disable TLS if it is explicitly set in the config. It only disables the guesswork

I think disabling it is fine. It's kind of a strange feature to be honest. It creates issues which are very hard to track down. If I set up my system to not use TLS, I expect it to not use it and not magically use it neverthless.
We already have a setting to enable TLS or SSL. I think that's all we need.
If you really want, add an option "auto" there which enables opportunistic TLS if selected, but if it's set to "none", it should not use any security.

Right, it disables enhancing to TLS if the server reports that it is able to support it. PHPMailer probably shouldn't bump up though if it requires extra configuration that isn't set in the class that would be set otherwise if you explicitly enable the TLS security option.

We too often take a stance that it's Joomla's responsibility to mask server configuration issues or bugs in third party libraries. This is another PR in that direction. If a server broadcasts that it supports TLS then fails when you try to use it that's a server configuration error, not a Joomla error. If the auto-TLS upgrade fails because PHPMailer needs more info to use it that's a PHPMailer issue, not a Joomla issue.

But, go ahead and merge this. It sounds like the decision is made already.

i agree 100% that we should not do anything just because a server is badly
setup but reading phpmailer's own wiki and issue tracker this is a feature
even they are not too happy with

They should pull it or default it to off then if it's really THAT buggy.

Perhaps they should but we know how reluctant developers can be to remove a
cool and clever feature ;) ;)

Updated the PR to try with TLS, if it fails, it will try sending it without TLS. Zero configuration :)

In general, a better idea. Still tricky because they only throw one Exception type for all errors. I'd say check if $this->SMTPAutoTLS is already false before trying the second send (someone in an extension could customize this stuff, you never know) and it'd be fine.

As for other improvements, see #9530

@mbabker Good point on adding the check. I have added it in. Thanks.

Hi,
For what is worth...
This change is not working for me in WAMP 3 environment (Windows 10) but adding the following fixes the issue. Please note I am a newbie so please check it, I am not sure what the change implies but it does work and without it the mail will fail:

Line 120 $this->SMTPAutoTLS = false;
// Added lines
$this->SMTPOptions = array(
'ssl' => array(
'verify_peer' => false,
'verify_peer_name' => false,
'allow_self_signed' => true
)
);
// End of added lines

That change disables SSL verification in full. PHP 5.6+ by default
verifies certificates if you've entered a configuration that communicates
on a secure port or protocol.

On Thursday, April 14, 2016, cjsdfw notifications@github.com wrote:

Hi,
For what is worth...
This change is not working for me in WAMP 3 environment (Windows 10) but
adding the following fixes the issue. Please note I am a newbie so please
check it, I am not sure what the change implies but it does work and
without it the mail will fail:

Line 120 $this->SMTPAutoTLS = false;
// Added lines
$this->SMTPOptions = array(
'ssl' => array(
'verify_peer' => false,
'verify_peer_name' => false,
'allow_self_signed' => true
)
);
// End of added lines

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9528 (comment)