Revert "set schema to smtps if MAIL_ENCRYPTION === tls"

[crynobone]

Reverts #53749

It seems that many of our Laravel users mistakenly configured MAIL_ENCRYPTION=tls when symfony/mailer would disregard this value completely and doesn't have any impact to determining smtp vs smtps or tls configuration within symfony/mailer since v4.4.

Moving forward it might is best to add scheme to the configuration and remove MAIL_ENCRYPTION completely.

[danielrona]

symfony/mailer will always use encryption when the server advertises STARTTLScapabilities

MAIL_ENCRYPTION=null would be the correct configuration in 99% of the use cases.

When the server does not advertise STARTTLS capabilities but you want the connection to be encrypted and the server does support TLS you would set MAIL_ENCRYPTION=tls

It seems that many people have MAIL_ENCRYPTION=tls set, even though it is supposed to be null

Anyway this reverts the changes, but would also end up making everyone who has properly set up MAIL_ENCRYPTION=null having a broken config again.

So we would actually need an option to set the scheme, wich is not provided here.

[crynobone]

symfony/mailer doesn't use encryption/MAIL_ENCRYPTION value since 4.4: symfony/mailer@4e6a8b2

4.3

new Dsn('smtp', 'example.com', self::USER, self::PASSWORD, 99, ['encryption' => 'ssl', 'auth_mode' => 'login']),

4.4

new Dsn('smtps', 'example.com', self::USER, self::PASSWORD, 99, ['auth_mode' => 'login']),

So when Laravel send the value $config with encryption, symfony/mailer simply ignores it:

framework/src/Illuminate/Mail/MailManager.php

Lines 202 to 209 in b9df1f7

	$transport = $factory->create(new Dsn(
	$scheme,
	$config['host'],
	$config['username'] ?? null,
	$config['password'] ?? null,
	$config['port'] ?? null,
	$config
	));

[danielrona]

> * [BC BREAK] Removed the encryption DSN option (use smtps instead)

With your change you would set the scheme always to smtp unless it's Port 465 only then you would set it to smtps.

That's why I initially proposed the change to only include the scheme in the configuration.

[crynobone]

With your change you would set the scheme always to smtp unless it's Port 465 only then you would set it to smtps.

Yes, that how symfony/mailer been handling it in 7.0 and 7.1. https://github.com/symfony/mailer/blob/69c9948451fb3a6a4d47dc8261d1794734e76cdd/Transport/Smtp/EsmtpTransportFactory.php#L27

Before this, we are sending smtps or smtp when encryption is set to tls, and '' otherwise (and since symfony/mailer only check if 'smtps' === $dns->getScheme() to set tls to true.

With this PR the behavior remains the same. We only had to swap '' with smtp as Dsn class in 7.2 throws an exception as '' is not a valid value.

That's why I initially proposed the change to only include the scheme in the configuration.

I don't disagree with that, just that the use of MAIL_MAILER collides with existing usage.

[danielrona]

Yes but your proposed changes.

$scheme = $config['scheme'] ?? null;

if (! $scheme) {
    $scheme = ($config['port'] == 465) ? 'smtps' : 'smtp';
}

would always yield smtp unless it's port 465 than it would give smtps because $scheme will always be null, but for auto_tls in symfony/mailer to work you would need smtps as scheme

because

$autoTls = '' === $dsn->getOption('auto_tls') || filter_var($dsn->getOption('auto_tls', true), \FILTER_VALIDATE_BOOL);
$tls = 'smtps' === $dsn->getScheme() ? true : ($autoTls ? null : false);

will always result in being $tls = false unless port 465 is used

This would correct the problem for everyone who configured MAIL_ENCRYPTION=tls even though it should have been MAIL_ENCRYPTION=null but will set everyone up for failure who has MAIL_ENCRYPTION=null wich would be the proper configuration.

Test for Port 465 and scheme=smtps

<?php

use Symfony\Component\Mailer\Transport\Dsn;

test('testing that port 465 can send mails', function () {


    $dsn = new Dsn('smtps', '', '', '', 465, ['auth_mode' => 'login']);

    $autoTls = '' === $dsn->getOption('auto_tls') || filter_var($dsn->getOption('auto_tls', true), FILTER_VALIDATE_BOOL);
    $tls = 'smtps' === $dsn->getScheme() ? true : ($autoTls ? null : false);

    expect($autoTls)
        ->toBeTrue()
        ->and($tls)
        ->toBeTrue()
    ;
});

Test for port 587 and scheme=smtp because it's not port 465

<?php

use Symfony\Component\Mailer\Transport\Dsn;

test('testing that port 587 can send mails', function () {


    $dsn = new Dsn('smtp', '', '', '', 587, ['auth_mode' => 'login']);

    $autoTls = '' === $dsn->getOption('auto_tls') || filter_var($dsn->getOption('auto_tls', true), FILTER_VALIDATE_BOOL);
    $tls = 'smtps' === $dsn->getScheme() ? true : ($autoTls ? null : false);

    expect($autoTls)
        ->toBeTrue()
        ->and($tls)
        ->toBeTrue()
    ;
});

[dsone]

It seems that many people have MAIL_ENCRYPTION=tls set, even though it is supposed to be null

The default value inside laravel/framework/config/mail.php is tls which might be the cause of many such issues combined with the previous change.
Having that as default makes sense, though. Rather be strict about encryption than too lax.

If you do not have a MAIL_ENCRYPTION in your .env before 11.35 (you used the default value null anyway mostly and removed the variable perhaps at some point, or it's empty) it still works as long as you use another port than 465 to not use tls (local dev environments or staging for example via smtp:localhost:1025).

With 11.35 you now need to explicitely set MAIL_ENCRYPTION variable to null in your .env to not use tls. No matter the used port due to the default configuration in environments where tls is not available or available but not on the configured port, breaking mail transport.

Unfortunate but I think the previous pull request changes are necessary as they fix longstanding and accidental misconfiguration in a lot of Laravel installations. MAIL_PORT and MAIL_ENCRYPTION discrepancies specifically.

[crynobone]

$autoTls = '' === $dsn->getOption('auto_tls') || filter_var($dsn->getOption('auto_tls', true), \FILTER_VALIDATE_BOOL);

To configure auto_tls you need to add additional options:

        'smtp' => [
            'transport' => 'smtp',
            'url' => env('MAIL_URL'),
            'host' => env('MAIL_HOST', '127.0.0.1'),
            'port' => env('MAIL_PORT', 2525),
            'encryption' => env('MAIL_ENCRYPTION', 'tls'),
            'username' => env('MAIL_USERNAME'),
            'password' => env('MAIL_PASSWORD'),
            'timeout' => null,
            'local_domain' => env('MAIL_EHLO_DOMAIN', parse_url(env('APP_URL', 'http://localhost'), PHP_URL_HOST)),
+           'auto_tls' => true,
        ],

Or, use MAIL_URL such as:

MAIL_URL="smtp://usr:[email protected]:2525?auto_tls=true"

Check the latest commits:

• auto_tls is only available in symfony/mailer 7.1
• auto_tls is enabled by default (see additional tests

[crynobone]

Unfortunate but I think the previous pull request changes are necessary as they fix longstanding and accidental misconfiguration in a lot of Laravel installations. MAIL_PORT and MAIL_ENCRYPTION discrepancies specifically.

The previous PR (#53749) caused breaking change and that's not acceptable.

[GrahamCampbell]

I agree that the change should be reverted.

[dsone]

Just for cross reference as it pertains to this issue and solves it for good now:
v11.36.0: #53874

ℹ️ MAIL_SCHEME has been added as environment variable to set the scheme and MAIL_ENCRYPTION has been removed.