From 6cd0d63b7a9d1ca8a0b9d0d4c853c48adbd94f9b Mon Sep 17 00:00:00 2001 From: Molkobain Date: Thu, 30 Apr 2026 19:28:30 +0200 Subject: [PATCH 1/2] =?UTF-8?q?N=C2=B09584=20-=20Fix=20"Unable=20to=20conn?= =?UTF-8?q?ect=20with=20STARTTLS"=20error=20when=20sending=20emails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sources/Core/Email/EmailSymfony.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sources/Core/Email/EmailSymfony.php b/sources/Core/Email/EmailSymfony.php index 1d29aec200..273dc484e8 100644 --- a/sources/Core/Email/EmailSymfony.php +++ b/sources/Core/Email/EmailSymfony.php @@ -189,8 +189,14 @@ protected function SendSynchronous(&$aIssues, $oLog = null) // Handle peer verification $oStream = $oTransport->getStream(); $aOptions = $oStream->getStreamOptions(); - if (!$bVerifyPeer && array_key_exists('ssl', $aOptions)) { - // Disable verification + /* + * N°9584 Connection with the SMTP server always starts unencrypted, so there won't be any `ssl` option at first. + * But we have to force them if certificate should not be verified so they can be used when the connection is "upgraded" to TLS via STARTTLS + * @see https://datatracker.ietf.org/doc/html/rfc3207 + * @see https://www.php.net/manual/en/function.stream-socket-enable-crypto.php + */ + if (!$bVerifyPeer) { + // Disable verification - must set even when 'ssl' key is absent (e.g. STARTTLS starts plain and upgrades later) $aOptions['ssl']['verify_peer'] = false; $aOptions['ssl']['verify_peer_name'] = false; $aOptions['ssl']['allow_self_signed'] = true; From b8149b669941c5e035f04b539c8292bccd5dcbcc Mon Sep 17 00:00:00 2001 From: Molkobain Date: Thu, 30 Apr 2026 21:03:32 +0200 Subject: [PATCH 2/2] =?UTF-8?q?N=C2=B09584=20-=20Refactor=20EMailSymfony?= =?UTF-8?q?=20transport=20to=20add=20unit=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sources/Core/Email/EmailSymfony.php | 50 ++++++++++------ .../sources/core/Email/EmailSymfonyTest.php | 60 +++++++++++++++++++ 2 files changed, 92 insertions(+), 18 deletions(-) diff --git a/sources/Core/Email/EmailSymfony.php b/sources/Core/Email/EmailSymfony.php index 273dc484e8..fbb0120343 100644 --- a/sources/Core/Email/EmailSymfony.php +++ b/sources/Core/Email/EmailSymfony.php @@ -29,6 +29,7 @@ use Symfony\Component\Mailer\Exception\TransportExceptionInterface; use Symfony\Component\Mailer\Transport; use Symfony\Component\Mailer\Mailer; +use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport; use Symfony\Component\Mime\Email as SymfonyEmail; use Symfony\Component\Mime\HtmlToTextConverter\DefaultHtmlToTextConverter; use Symfony\Component\Mime\Part\DataPart; @@ -184,24 +185,7 @@ protected function SendSynchronous(&$aIssues, $oLog = null) $sDsn = sprintf('smtp://%s%s@%s%s', $sDsnUser, $sDsnPassword, $sDsnPort, $sEncQuery); } - $oTransport = Transport::fromDsn($sDsn); - - // Handle peer verification - $oStream = $oTransport->getStream(); - $aOptions = $oStream->getStreamOptions(); - /* - * N°9584 Connection with the SMTP server always starts unencrypted, so there won't be any `ssl` option at first. - * But we have to force them if certificate should not be verified so they can be used when the connection is "upgraded" to TLS via STARTTLS - * @see https://datatracker.ietf.org/doc/html/rfc3207 - * @see https://www.php.net/manual/en/function.stream-socket-enable-crypto.php - */ - if (!$bVerifyPeer) { - // Disable verification - must set even when 'ssl' key is absent (e.g. STARTTLS starts plain and upgrades later) - $aOptions['ssl']['verify_peer'] = false; - $aOptions['ssl']['verify_peer_name'] = false; - $aOptions['ssl']['allow_self_signed'] = true; - } - $oStream->setStreamOptions($aOptions); + $oTransport = $this->CreateSmtpTransport($sDsn, $bVerifyPeer); $oMailer = new Mailer($oTransport); break; @@ -267,6 +251,36 @@ protected function SendSynchronous(&$aIssues, $oLog = null) } } + /** + * Build and configure an SMTP transport from a DSN string. + * + * Extracted from {@see SendSynchronous} to make SSL option handling independently testable. + * When $bVerifyPeer is false, the ssl stream context options must be written unconditionally: + * with STARTTLS the connection starts unencrypted, so the 'ssl' key is absent from the stream + * options at construction time and only used later when stream_socket_enable_crypto() is called. + * + * @param string $sDsn Full Symfony Mailer DSN (smtp:// or smtps://) + * @param bool $bVerifyPeer Whether to verify the peer SSL certificate + * + * @return \Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport + */ + protected function CreateSmtpTransport(string $sDsn, bool $bVerifyPeer): EsmtpTransport + { + /** @var EsmtpTransport $oTransport */ + $oTransport = Transport::fromDsn($sDsn); + + $oStream = $oTransport->getStream(); + $aOptions = $oStream->getStreamOptions(); + if (!$bVerifyPeer) { + $aOptions['ssl']['verify_peer'] = false; + $aOptions['ssl']['verify_peer_name'] = false; + $aOptions['ssl']['allow_self_signed'] = true; + } + $oStream->setStreamOptions($aOptions); + + return $oTransport; + } + /** * Reprocess the body of the message (if it is an HTML message) * to replace the URL of images based on attachments by a link diff --git a/tests/php-unit-tests/unitary-tests/sources/core/Email/EmailSymfonyTest.php b/tests/php-unit-tests/unitary-tests/sources/core/Email/EmailSymfonyTest.php index 1a289798d5..2cefa18742 100644 --- a/tests/php-unit-tests/unitary-tests/sources/core/Email/EmailSymfonyTest.php +++ b/tests/php-unit-tests/unitary-tests/sources/core/Email/EmailSymfonyTest.php @@ -2,6 +2,7 @@ use Combodo\iTop\Core\Email\EMailSymfony; use Combodo\iTop\Test\UnitTest\ItopTestCase; +use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport; use Symfony\Component\Mime\Part\DataPart; use Symfony\Component\Mime\Part\Multipart\AlternativePart; use Symfony\Component\Mime\Part\Multipart\RelatedPart; @@ -298,4 +299,63 @@ public function provideSetBodyPlainTextDoesNotContainCss(): array ], ]; } + + /** + * @dataProvider provideCreateSmtpTransportSslOptions + */ + public function testCreateSmtpTransportSslOptions(string $sDsn, bool $bVerifyPeer, array $aExpectedSslOptions): void + { + $oEmail = new EMailSymfony(); + /** @var EsmtpTransport $oTransport */ + $oTransport = $this->InvokeNonPublicMethod(EMailSymfony::class, 'CreateSmtpTransport', $oEmail, [$sDsn, $bVerifyPeer]); + + $aActualSslOptions = $oTransport->getStream()->getStreamOptions()['ssl'] ?? []; + + $this->assertSame($aExpectedSslOptions, $aActualSslOptions); + } + + public function provideCreateSmtpTransportSslOptions(): array + { + $aDisabledVerification = [ + 'verify_peer' => false, + 'verify_peer_name' => false, + 'allow_self_signed' => true, + ]; + + return [ + // Regression scenario (N°9584): STARTTLS starts the connection unencrypted, so the 'ssl' key + // is absent from stream options at construction time. verify_peer=false must still be applied. + 'STARTTLS, verify_peer=false' => [ + 'smtp://localhost:587?encryption=starttls', + false, + $aDisabledVerification, + ], + 'implicit TLS (smtps), verify_peer=false' => [ + 'smtps://localhost:465', + false, + $aDisabledVerification, + ], + 'plain SMTP, verify_peer=false' => [ + 'smtp://localhost:25', + false, + $aDisabledVerification, + ], + // Default behavior: verify_peer=true must leave stream options untouched (empty). + 'STARTTLS, verify_peer=true (default)' => [ + 'smtp://localhost:587?encryption=starttls', + true, + [], + ], + 'implicit TLS (smtps), verify_peer=true (default)' => [ + 'smtps://localhost:465', + true, + [], + ], + 'plain SMTP, verify_peer=true (default)' => [ + 'smtp://localhost:25', + true, + [], + ], + ]; + } }