fix(mail): embed image logo only when using useTemplate

lib/private/Mail/EMailTemplate.php

@@ -42,6 +42,8 @@ class EMailTemplate implements IEMailTemplate {
 	protected ?string $senderName = null;
 	/** @var array<array{name: string, content: string, mimeType: string}> images to embed inline, referenced via cid: */
 	protected array $inlineImages = [];
+	/** absolute URL of the logo, if added to the body, will be replaced with cid: if logo is to be attached to the email */
+	private string $logoUrl = '';
 
 	protected string $head = <<<EOF
 <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
@@ -298,16 +300,36 @@ public function addHeader(): void {
 			$logoSizeDimensions = ' width="' . $this->logoWidth . '" height="' . $this->logoHeight . '"';
 		}
 
+		$this->logoUrl = $this->urlGenerator->getAbsoluteURL($this->themingDefaults->getLogo(false));
+		$this->htmlBody .= vsprintf($this->header, [$this->themingDefaults->getDefaultColorPrimary(), $this->logoUrl, $this->themingDefaults->getName(), $logoSizeDimensions]);
+	}
+
+	/**
+	 * Renders the HTML email with the logo embedded instead of linked, so mail
+	 * clients don't have to fetch it from the internet (some (e.g. gmail) block that).
+	 *
+	 * The caller must attach the images returned by getInlineImages() afterwards.
+	 */
+	public function renderHtmlWithInlineImages(): string {
+		$html = $this->renderHtml();
+		$this->inlineImages = [];
+		if ($this->logoUrl === '') {
+			return $html;
+		}
+
+		$logoSrc = 'src="' . $this->logoUrl . '"';
+		$position = strpos($html, $logoSrc);
+		if ($position === false) {
+			return $html;
+		}
+
 		$logoImage = $this->themingDefaults->getLogoImage();
-		if ($logoImage !== null) {
-			// Embed the logo directly in the message instead of linking to it, so mail
-			// clients don't have to fetch it from the internet (some (e.g. gmail) block that).
-			$logoSrc = 'cid:logo';
-			$this->inlineImages[] = ['name' => 'logo'] + $logoImage;
-		} else {
-			$logoSrc = $this->urlGenerator->getAbsoluteURL($this->themingDefaults->getLogo(false));
+		if ($logoImage === null) {
+			return $html;
 		}
-		$this->htmlBody .= vsprintf($this->header, [$this->themingDefaults->getDefaultColorPrimary(), $logoSrc, $this->themingDefaults->getName(), $logoSizeDimensions]);
+		$this->inlineImages[] = ['name' => 'logo'] + $logoImage;
+
+		return substr_replace($html, 'src="cid:logo"', $position, strlen($logoSrc));
 	}
 
 	#[\Override]
@@ -319,6 +341,7 @@ public function setLanguage(string $language): void {
 
 	/**
 	 * Images that must be embedded inline in the message, referenced via `cid:<name>` in the HTML body
+	 * returned by the last renderHtmlWithInlineImages() call
 	 *
 	 * @return array<array{name: string, content: string, mimeType: string}>
 	 */

lib/private/Mail/Message.php

@@ -291,11 +291,13 @@ public function useTemplate(IEMailTemplate $emailTemplate): IMessage {
 		$this->setSubject($emailTemplate->renderSubject());
 		$this->setPlainBody($emailTemplate->renderText());
 		if (!$this->plainTextOnly) {
-			$this->setHtmlBody($emailTemplate->renderHtml());
 			if ($emailTemplate instanceof EMailTemplate) {
+				$this->setHtmlBody($emailTemplate->renderHtmlWithInlineImages());
 				foreach ($emailTemplate->getInlineImages() as $image) {
 					$this->attachInline($image['content'], $image['name'], $image['mimeType']);
 				}
+			} else {
+				$this->setHtmlBody($emailTemplate->renderHtml());
 			}
 		}
 		return $this;

lib/public/Mail/IMailer.php

@@ -26,6 +26,16 @@
  *
  * This message can then be passed to send() of \OC\Mail\Mailer
  *
+ * To send an email built from a template, pass the template to the message with
+ * IMessage::useTemplate() instead of setting the rendered bodies manually, so
+ * that images embedded in the template (e.g. the logo) are attached as well:
+ *
+ * 	$template = $mailer->createEMailTemplate('app.MailId');
+ * 	$template->setSubject('Your Subject');
+ * 	$template->addHeader();
+ * 	$template->addBodyText('The message text');
+ * 	$message->useTemplate($template);
+ *
  * @since 8.1.0
  */
 interface IMailer {

tests/lib/Mail/EMailTemplateTest.php

@@ -225,11 +225,14 @@ public function testEMailTemplateEmbedsLogo(): void {
 		$this->defaults->method('getName')->willReturn('TestCloud');
 		$this->defaults->method('getLogoImage')
 			->willReturn(['content' => 'PNGDATA', 'mimeType' => 'image/png']);
-		$this->urlGenerator->expects($this->never())->method('getAbsoluteURL');
+		$this->urlGenerator->method('getAbsoluteURL')
+			->willReturn('https://example.org/img/logo-mail-header.png');
 
 		$this->emailTemplate->addHeader();
 
-		$this->assertStringContainsString('src="cid:logo"', $this->emailTemplate->renderHtml());
+		$html = $this->emailTemplate->renderHtmlWithInlineImages();
+		$this->assertStringContainsString('src="cid:logo"', $html);
+		$this->assertStringNotContainsString('https://example.org/img/logo-mail-header.png', $html);
 		$this->assertSame(
 			[['name' => 'logo', 'content' => 'PNGDATA', 'mimeType' => 'image/png']],
 			$this->emailTemplate->getInlineImages()
@@ -458,4 +461,81 @@ public function testEMailTemplateDefaultFooterUsesLanguage(): void {
 		$template->addFooter();
 		$template->renderHtml();
 	}
+
+	public function testEMailTemplateLinksLogoWhenRenderedWithoutInlineImages(): void {
+		$this->defaults->method('getDefaultColorPrimary')->willReturn('#0082c9');
+		$this->defaults->method('getName')->willReturn('TestCloud');
+		$this->defaults->expects($this->never())->method('getLogoImage');
+		$this->urlGenerator->method('getAbsoluteURL')
+			->willReturn('https://example.org/img/logo-mail-header.png');
+
+		$this->emailTemplate->addHeader();
+
+		$html = $this->emailTemplate->renderHtml();
+		$this->assertStringContainsString('src="https://example.org/img/logo-mail-header.png"', $html);
+		$this->assertStringNotContainsString('cid:logo', $html);
+		$this->assertSame([], $this->emailTemplate->getInlineImages());
+	}
+
+	public function testEMailTemplateLinksLogoWhenNoLogoImageAvailable(): void {
+		$this->defaults->method('getDefaultColorPrimary')->willReturn('#0082c9');
+		$this->defaults->method('getName')->willReturn('TestCloud');
+		$this->defaults->method('getLogoImage')->willReturn(null);
+		$this->urlGenerator->method('getAbsoluteURL')
+			->willReturn('https://example.org/img/logo-mail-header.png');
+
+		$this->emailTemplate->addHeader();
+
+		$html = $this->emailTemplate->renderHtmlWithInlineImages();
+		$this->assertStringContainsString('src="https://example.org/img/logo-mail-header.png"', $html);
+		$this->assertSame([], $this->emailTemplate->getInlineImages());
+	}
+	public function testEMailTemplateEmbedsOnlyTheHeaderLogo(): void {
+		$this->defaults->method('getDefaultColorPrimary')->willReturn('#0082c9');
+		$this->defaults->method('getName')->willReturn('TestCloud');
+		$this->defaults->method('getLogoImage')
+			->willReturn(['content' => 'PNGDATA', 'mimeType' => 'image/png']);
+		$this->urlGenerator->method('getAbsoluteURL')
+			->willReturn('https://example.org/img/logo-mail-header.png');
+
+		$this->emailTemplate->addHeader();
+		$this->emailTemplate->addBodyListItem('Item', '', 'https://example.org/img/logo-mail-header.png');
+
+		$html = $this->emailTemplate->renderHtmlWithInlineImages();
+		$this->assertSame(1, substr_count($html, 'src="cid:logo"'));
+		$this->assertSame(1, substr_count($html, 'src="https://example.org/img/logo-mail-header.png"'));
+	}
+
+	public function testEMailTemplateKeepsCustomHeaderWithoutLogo(): void {
+		$this->defaults->method('getDefaultColorPrimary')->willReturn('#0082c9');
+		$this->defaults->method('getName')->willReturn('TestCloud');
+		$this->defaults->expects($this->never())->method('getLogoImage');
+		$this->urlGenerator->method('getAbsoluteURL')
+			->willReturn('https://example.org/img/logo-mail-header.png');
+
+		$template = new class($this->defaults, $this->urlGenerator, $this->l10n, 252, 120, 'test.TestTemplate', []) extends EMailTemplate {
+			protected string $header = '<img class="logo" src="https://brand.example/logo.png">';
+		};
+		$template->addHeader();
+
+		$this->assertSame($template->renderHtml(), $template->renderHtmlWithInlineImages());
+		$this->assertSame([], $template->getInlineImages());
+	}
+
+	public function testEMailTemplateKeepsOwnHeaderImplementation(): void {
+		$this->defaults->expects($this->never())->method('getLogoImage');
+
+		$template = new class($this->defaults, $this->urlGenerator, $this->l10n, 252, 120, 'test.TestTemplate', []) extends EMailTemplate {
+			// a subclass may keep its own logo URL in a property of the same name
+			private string $logoUrl = 'https://brand.example/logo.png';
+
+			public function addHeader(): void {
+				$this->htmlBody .= '<img class="logo" src="' . $this->logoUrl . '">';
+			}
+		};
+		$template->addHeader();
+
+		$this->assertSame($template->renderHtml(), $template->renderHtmlWithInlineImages());
+		$this->assertSame([], $template->getInlineImages());
+	}
 }

tests/lib/Mail/MessageTest.php

@@ -8,6 +8,7 @@
 
 namespace Test\Mail;
 
+use OC\Mail\EMailTemplate;
 use OC\Mail\Message;
 use OCP\Mail\Headers\AutoSubmitted;
 use OCP\Mail\IEMailTemplate;
@@ -255,6 +256,28 @@ public function testBothRenderingOptions(): void {
 		$message->useTemplate($template);
 	}
 
+	public function testUseTemplateEmbedsInlineImages(): void {
+		$symfonyEmail = new Email();
+		$template = $this->createMock(EMailTemplate::class);
+		$template->method('renderSubject')->willReturn('Subject');
+		$template->method('renderText')->willReturn('Text');
+		$template->expects($this->never())->method('renderHtml');
+		$template->expects($this->once())
+			->method('renderHtmlWithInlineImages')
+			->willReturn('<img src="cid:logo">');
+		$template->method('getInlineImages')
+			->willReturn([['name' => 'logo', 'content' => 'PNGDATA', 'mimeType' => 'image/png']]);
+
+		$message = new Message($symfonyEmail, false);
+		$message->useTemplate($template);
+
+		$this->assertSame('<img src="cid:logo">', $symfonyEmail->getHtmlBody());
+		$attachments = $symfonyEmail->getAttachments();
+		$this->assertCount(1, $attachments);
+		$this->assertSame('logo', $attachments[0]->getFilename());
+		$this->assertSame('PNGDATA', $attachments[0]->getBody());
+	}
+
 	public function testSetAutoSubmitted1(): void {
 		$headers = new Headers($this->createMock(HeaderInterface::class));
 		$headers->addTextHeader(AutoSubmitted::HEADER, 'yes');