SMTP: refactor and accept starttls :always and :auto

[eval]

This PR is a refactored version of #1515 (which is a master-rebased version of #1435) and adds :always and :auto as acceptable values for enable_starttls.

While the original PR 1515 fixes the disabling of starttls, it also yields unexpected/faulty 'shortcuts' in the if-chain given some inputs, e.g.:

1. ssl: false ensures anyenable_starttls-flag is ignored.
2. enable_starttls: false would ignore the enable_starttls_auto-flag.

I refactored the logic to come to the right starttls value for smtp (i.e. :always, :auto or false). It also adds the option to directly pass :always or :auto as value for enable_starttls (separate commit).

The implementation is based on this decision table:

| tls/ssl   | enable_starttls | enable_starttls_auto | result                                     |
|-----------+-----------------+----------------------+--------------------------------------------|
| true      | *               | *                    | {smtp_tls?: true, smtp_starttls: false}^1  |
| - / false | -               | -                    | {smtp_tls?: false, smtp_starttls: :auto}   |
| - / false | - / false       | true                 | {smtp_tls?: false, smtp_starttls: :auto}   |
| - / false | true            | *                    | {smtp_tls?: false, smtp_starttls: :always} |
| - / false | - / false       | false                | {smtp_tls?: false, smtp_starttls: false}   |
| - / false | false           | -                    | {smtp_tls?: false, smtp_starttls: false}   |
| - / false | :always         | *                    | {smtp_tls?: false, smtp_starttls: :always} |
| - / false | :auto           | *                    | {smtp_tls?: false, smtp_starttls: :auto}   |

* = any value
- = not provided (i.e. key absent or nil-value) 

^1: raise when truthy value for any enable_starttls* flag is provided (similar to IMAP).

[mikel]

Hey there @ahorek and @eval does this also replace #1526 ?

[ahorek]

with old rubies (net/smtp), both options were disabled by default, so historically it used to make sense to enable them separately

enable_starttls: true
enable_starttls_auto: true

but since starttls is now :auto by default due to the upstream change in net/smtp, it's problematic to do the opposite and disable them with existing options

what about introducing a new setting, that accepts the same values?

starttls: [false, :auto, :always]

if enable_starttls or enable_starttls_auto are passed - raise an error with a hint of what the new setting is

this is already a breaking change, so I suggest simplifying it if we have a chance. @mikel @eval What do you think?

Hey there @ahorek and @eval does this also replace #1526 ?

yes, but the related PR should still be merged for 2.8.x

[ron-shinall]

@mikel @eval @ahorek Do you have an ETA for a release that includes this? Thank you!

[ahorek]

@ron-shinall I'm not a maintainer.

only @mikel or @jeremy can push a new version, but none of them are active lately

[stanhu]

@jeremy Would you mind releasing a new version with this fix? We're seeing a number of users not able to disable STARTTLS without this change.

[ahorek]

@stanhu fyi it has already been released in 2.8.1

[stanhu]

@ahorek I thought that at first, but https://github.com/mikel/mail/blob/2.8.1/lib/mail/network/delivery_methods/smtp.rb#L114 shows that the refactoring in this pull request is not present. #1526 is present, however.

We have users that have configured settings[:tls] to be false, but settings[:tls] is nil. false || nil is actually nil, so tls in results in nil.

[ahorek]

the main problem was starttls cannot be disabled with any settings, this should already be fixed.

this refactor also fixes a combination of
tls: false + enable_starttls: true (should ignore starttls)
also enable_starttls: true is now enabled by default (in net/smtp) and a simple tls: false doesn't disable it

just pass tls: false + enable_starttls: false that should do what you want. I agree there's still inconsistency, but a workaround is pretty simple.

[stanhu]

a workaround is pretty simple.

Yes, the workaround is simple in principle, but we have potentially thousands of users that have set settings[:tls] and settings[:ssl] to false. We ship mail v2.8.1 now, but having to comment out existing settings that worked is not a good user experience. We will backport this patch for now.

[ahorek]

well, this PR is merged for 2.9, but it isn't released. If that's important for you, you should open a new issue and ask maintainers if they're willing to backport it.