✅ Fix PHP code styles

* N°9584 - Fix "Unable to connect with STARTTLS" error when sending emails

* N°9584 - Refactor EMailSymfony transport to add unit tests

sources/Application/TwigBase/Twig/Extension.php

@@ -153,8 +153,12 @@ class Extension
 			return twig_array_filter($oTwigEnv, $array, $arrow);
 		}, ['needs_environment' => true]);
 
-		// @since 3.3.0 N°8579
-		// Filter to remove spaces between HTML tags, overwrite the deprecated core "spaceless" filter
+		/**
+		 * Filter to remove spaces between HTML tags, overwrite the deprecated core "spaceless" filter
+		 * Usage in twig: {% apply spaceless %}some html{% endapply %}
+		 *
+		 * @since 3.2.3 3.3.0 N°8579
+		 */
 		$aFilters[] = new TwigFilter('spaceless', function (?string $content) {
 			return trim(preg_replace('/>\s+</', '><', $content ?? ''));
 		}, ['is_safe' => ['html']]);

sources/Core/Email/EmailSymfony.php

@@ -29,7 +29,9 @@ use Symfony\Component\CssSelector\Exception\ParseException;
 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;
 use Symfony\Component\Mime\Part\Multipart\RelatedPart;
 use Symfony\Component\Mime\Part\Multipart\MixedPart;
@@ -183,18 +185,7 @@ class EMailSymfony extends Email
 					$sDsn = sprintf('smtp://%s%s@%s%s', $sDsnUser, $sDsnPassword, $sDsnPort, $sEncQuery);
 				}
 
-				$oTransport = Transport::fromDsn($sDsn);
-
-				// Handle peer verification
-				$oStream = $oTransport->getStream();
-				$aOptions = $oStream->getStreamOptions();
-				if (!$bVerifyPeer && array_key_exists('ssl', $aOptions)) {
-					// Disable verification
-					$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;
@@ -260,6 +251,36 @@ class EMailSymfony extends Email
 		}
 	}
 
+	/**
+	 * 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
@@ -416,13 +437,13 @@ class EMailSymfony extends Email
 
 		$this->m_aData['body'] = ['body' => $sBody, 'mimeType' => $sMimeType];
 
-		$oTextPart = new TextPart(strip_tags($sBody), 'utf-8', 'plain', 'base64');
-
 		// Embed inline images and store them in attachments (so BuildSymfonyMessageFromInternal can pick them)
 		if ($sPrimaryMimeType === 'text/html') {
 			$aAdditionalParts = $this->EmbedInlineImages($sBody);
+			$oTextPart = new TextPart((new DefaultHtmlToTextConverter())->convert($sBody, 'utf-8'), 'utf-8', 'plain', 'base64');
 			$oHtmlPart = new TextPart($sBody, 'utf-8', 'html', 'base64');
-			$oAlternativePart = new AlternativePart($oHtmlPart, $oTextPart);
+			// It's important de order parts from least prefered to most prefered as per RFC 2046 {@see https://www.rfc-editor.org/rfc/rfc2046.html#section-5.1.4}
+			$oAlternativePart = new AlternativePart($oTextPart, $oHtmlPart);
 			// Default root part is the HTML body
 			$oRootPart = $oAlternativePart;
 

tests/php-unit-tests/unitary-tests/sources/core/Email/EmailSymfonyTest.php

@@ -1,6 +1,12 @@
 <?php
 
+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;
+use Symfony\Component\Mime\Part\TextPart;
 
 class EmailSymfonyTest extends ItopTestCase
 {
@@ -135,4 +141,221 @@ HTML;
 
 		$this->assertSame($sExpectedBody, $sActualBody);
 	}
+
+	/**
+	 * Returns the parts of the AlternativePart produced by SetBody() for an HTML email.
+	 *
+	 * Handles both the simple case (AlternativePart at root) and the inline-images case
+	 * where the root is a RelatedPart whose first child is the AlternativePart.
+	 *
+	 * @return AbstractPart[]
+	 */
+	private function GetAlternativePartsFromHtmlEmail(EMailSymfony $oEmail): array
+	{
+		$oSymfonyMessage = $this->GetNonPublicProperty($oEmail, 'm_oMessage');
+		$oBody = $oSymfonyMessage->getBody();
+
+		// With inline images the root is a RelatedPart; the AlternativePart is its first child.
+		if ($oBody instanceof RelatedPart) {
+			$oBody = $oBody->getParts()[0];
+		}
+
+		$this->assertInstanceOf(AlternativePart::class, $oBody, 'Body should be a multipart/alternative for HTML emails');
+
+		return $oBody->getParts();
+	}
+
+	/**
+	 * RFC 2046 §5.1.4: parts in multipart/alternative must be ordered from least to most preferred.
+	 * Email clients display the last part they support, so text/plain must come first and text/html last.
+	 *
+	 * @see https://www.rfc-editor.org/rfc/rfc2046.html#section-5.1.4
+	 * @covers \Combodo\iTop\Core\Email\EmailSymfony::SetBody()
+	 * @since N°9574
+	 */
+	public function testSetBodyAlternativePartOrderForHtmlEmailIsPlainThenHtml(): void
+	{
+		$oEmail = new EMailSymfony();
+		$oEmail->SetBody('<p>Hello there!</p>', 'text/html');
+
+		[$oFirstPart, $oSecondPart] = $this->GetAlternativePartsFromHtmlEmail($oEmail);
+
+		$this->assertSame('plain', $oFirstPart->getMediaSubtype(), 'First part must be text/plain (least preferred per RFC 2046)');
+		$this->assertSame('html', $oSecondPart->getMediaSubtype(), 'Last part must be text/html (most preferred per RFC 2046)');
+	}
+
+	/**
+	 * @dataProvider provideSetBodyPlainTextDoesNotContainCss
+	 *
+	 * @covers \Combodo\iTop\Core\Email\EmailSymfony::SetBody()
+	 * @since N°9574
+	 */
+	public function testSetBodyPlainTextDoesNotContainCss(string $sHtml, ?string $sCustomStyles): void
+	{
+		$oEmail = new EMailSymfony();
+		$oEmail->SetBody($sHtml, 'text/html', $sCustomStyles);
+
+		// We locate the plain text part by subtype to be order-agnostic and isolate this assertion from the order bug.
+		$aParts = $this->GetAlternativePartsFromHtmlEmail($oEmail);
+		$oPlainPart = null;
+		foreach ($aParts as $oPart) {
+			if ($oPart instanceof TextPart && $oPart->getMediaSubtype() === 'plain') {
+				$oPlainPart = $oPart;
+				break;
+			}
+		}
+		$this->assertNotNull($oPlainPart, 'No text/plain part found in the message');
+
+		$sPlainText = $oPlainPart->getBody();
+
+		$this->assertStringNotContainsString('<style>', $sPlainText, 'Style tag must not appear in plain text');
+		$this->assertStringNotContainsString('color:', $sPlainText, 'CSS color rule must not appear in plain text');
+		$this->assertStringNotContainsString('font-size:', $sPlainText, 'CSS font-size rule must not appear in plain text');
+		$this->assertStringNotContainsString('@media', $sPlainText, 'CSS @media rule must not appear in plain text');
+		$this->assertStringContainsString('Hello there!', $sPlainText, 'Actual content must be preserved in plain text');
+	}
+
+	/**
+	 * The HTML part must contain the body content and the CSS inlined by Emogrifier.
+	 * This guards against regressions where the wrong body (e.g. the plain-text version)
+	 * would end up in the HTML part.
+	 *
+	 * @covers \Combodo\iTop\Core\Email\EmailSymfony::SetBody()
+	 * @since N°9574
+	 */
+	public function testSetBodyHtmlPartContainsBodyAndInlinedCss(): void
+	{
+		$oEmail = new EMailSymfony();
+		$oEmail->SetBody('<html><body><p>Hello there!</p></body></html>', 'text/html', 'p { color: red; }');
+
+		$aParts = $this->GetAlternativePartsFromHtmlEmail($oEmail);
+
+		$oHtmlPart = null;
+		foreach ($aParts as $oPart) {
+			if ($oPart instanceof TextPart && $oPart->getMediaSubtype() === 'html') {
+				$oHtmlPart = $oPart;
+				break;
+			}
+		}
+		$this->assertNotNull($oHtmlPart, 'No text/html part found in the message');
+
+		$sHtmlContent = $oHtmlPart->getBody();
+		$this->assertStringContainsString('Hello there!', $sHtmlContent, 'HTML part must preserve the original text content');
+		$this->assertStringContainsString('color: red', $sHtmlContent, 'HTML part must contain the CSS inlined by Emogrifier');
+	}
+
+	/**
+	 * With inline images, SetBody() wraps the AlternativePart in a RelatedPart.
+	 * The AlternativePart must still be correctly ordered (plain first, HTML last)
+	 * and the plain-text part must not contain CSS.
+	 *
+	 * @see https://www.rfc-editor.org/rfc/rfc2046.html#section-5.1.4
+	 * @covers \Combodo\iTop\Core\Email\EmailSymfony::SetBody()
+	 * @since N°9574
+	 */
+	public function testSetBodyWithInlineImagesHasCorrectPartStructure(): void
+	{
+		// Anonymous subclass so we can inject a fake inline image part without a real inline image in DB
+		$oEmail = new class () extends EMailSymfony {
+			protected function EmbedInlineImages(string &$sBody): array
+			{
+				return [new DataPart('fake-image-data', 'image.png', 'image/png')];
+			}
+		};
+		$oEmail->SetBody('<html><head><style>p { color: red; }</style></head><body><p>Hello there!</p></body></html>', 'text/html');
+
+		$oSymfonyMessage = $this->GetNonPublicProperty($oEmail, 'm_oMessage');
+		$oBody = $oSymfonyMessage->getBody();
+
+		// Root must be a RelatedPart when inline images are present
+		$this->assertInstanceOf(RelatedPart::class, $oBody, 'Root part must be multipart/related when inline images are present');
+
+		// The AlternativePart must be the first child of the RelatedPart
+		$aRelatedParts = $oBody->getParts();
+		$this->assertInstanceOf(AlternativePart::class, $aRelatedParts[0], 'First child of RelatedPart must be the AlternativePart');
+
+		// Order and CSS checks are delegated to the shared helper, which now handles RelatedPart
+		[$oFirstPart, $oSecondPart] = $this->GetAlternativePartsFromHtmlEmail($oEmail);
+		$this->assertSame('plain', $oFirstPart->getMediaSubtype(), 'First part must be text/plain (least preferred per RFC 2046)');
+		$this->assertSame('html', $oSecondPart->getMediaSubtype(), 'Last part must be text/html (most preferred per RFC 2046)');
+	}
+
+	public function provideSetBodyPlainTextDoesNotContainCss(): array
+	{
+		$sCustomStyles = 'p { color: blue; font-size: 14px; }';
+
+		return [
+			'<style> tag in HTML, no custom styles' => [
+				'<html><head><style>body { color: red; font-size: 12px; } @media print { p { color: black; } }</style></head><body><p>Hello there!</p></body></html>',
+				null,
+			],
+			'<style> tag in HTML with custom styles' => [
+				'<html><head><style>body { color: red; font-size: 12px; } @media print { p { color: black; } }</style></head><body><p>Hello there!</p></body></html>',
+				$sCustomStyles,
+			],
+			'custom styles only, no <style> tag' => [
+				'<html><body><p>Hello there!</p></body></html>',
+				$sCustomStyles,
+			],
+		];
+	}
+
+	/**
+	 * @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,
+				[],
+			],
+		];
+	}
 }