Forward inline images by kesselb · Pull Request #12651 · nextcloud/mail

kesselb · 2026-03-24T21:50:43Z

AI-assisted: Claude Sonnet 4.6 Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

The charset item is always there Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

kesselb · 2026-03-24T21:53:54Z

lib/IMAP/MessageMapper.php

 			/** @var Horde_Mime_Part $part */

-			if (!$part->isAttachment()) {
+			if (count($attachmentIds) === 0 && !$part->isAttachment()) {


Todo: Double-Check / Revert

kesselb · 2026-03-24T21:59:29Z

src/service/MessageService.js

 	}
 }

+export async function fetchMessageHtmlBodyForCompose(id) {


Todo: Remove

first iteration had a own endpoint and therefore the new method

Copilot

Pull request overview

Enables forwarding of inline images by preserving cid: ↔ attachment URL ↔ data-cid metadata across message rendering and compose, and by persisting attachment Content-ID / disposition so forwarded attachments can be reconstructed correctly.

Changes:

Add support for inline attachments (Content-ID + disposition) end-to-end: IMAP parsing, attachment forwarding, MIME building, and compose UI handling.
Extend HTML purification to rewrite cid: to attachment URLs and add a data-cid marker so forwarding can restore cid: references.
Add/adjust unit tests and test fixtures to cover encrypted/inline attachments, MIME building behavior, and HTMLPurifier transforms.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/data/decrypted-message-body-with-attachment.txt	Adds encrypted MIME fixture with both inline image and regular attachment parts.
tests/Unit/Service/TransmissionServiceTest.php	Updates tests for new LocalAttachment disposition handling.
tests/Unit/Service/MimeMessageTest.php	Adds coverage for inline attachments going into `multipart/related` and `data-cid` rewrite behavior.
tests/Unit/Service/MailManagerTest.php	Updates Attachment constructor usage with new fields.
tests/Unit/Service/HtmlPurify/TransformURLSchemeTest.php	Adjusts tests for new inline-attachments-based CID rewrite logic.
tests/Unit/Service/HtmlPurify/TransformCidDataAttrTest.php	New tests for adding `data-cid` based on inline attachment URLs.
tests/Unit/Service/Attachment/AttachmentServiceTest.php	Updates attachment creation/forwarding tests for contentId/disposition changes.
tests/Unit/IMAP/MessageMapperTest.php	Extends tests to validate content type and disposition for encrypted attachments and inline images.
tests/Unit/Db/LocalAttachmentTest.php	New unit tests for LocalAttachment disposition helper method.
tests/Unit/Controller/MessagesControllerTest.php	Updates Attachment constructor usage with new fields.
src/store/mainStore/actions.js	Prepares both normal and inline attachments when forwarding; adds compose HTML body fetch path.
src/service/MessageService.js	Adds a compose-specific HTML body fetch helper.
src/components/TextEditor.vue	Allows `img[data-cid]` through the editor’s HTML support config.
src/components/ComposerAttachments.vue	Preserves attachment `type`/`databaseId` for forwarding inline previews.
src/components/ComposerAttachment.vue	Computes preview source including forwarded inline image download URLs.
lib/Service/TransmissionService.php	Applies LocalAttachment disposition/contentId to MIME parts, supporting omitted Content-Disposition for iMIP.
lib/Service/MimeMessage.php	Separates inline attachments into `multipart/related` and rewrites `data-cid` back to `cid:`.
lib/Service/MailTransmission.php	Updates build() call signature for MIME message construction.
lib/Service/HtmlPurify/TransformURLScheme.php	Refactors CID URL rewriting to use inline attachments list.
lib/Service/HtmlPurify/TransformCidDataAttr.php	New HTMLPurifier transform to add `data-cid` based on inline attachment URLs.
lib/Service/Html.php	Updates sanitizer to use new transforms and inline attachment URL enrichment.
lib/Service/DataUri/DataUri.php	Documents data URI parameter shape for charset access.
lib/Service/Attachment/AttachmentService.php	Persists contentId/disposition on forwarded attachments; supports forwarding inline message attachments.
lib/Service/AntiSpamService.php	Updates build() call signature for MIME message construction.
lib/Provider/Command/MessageSend.php	Infers and persists attachment disposition when provider API lacks it.
lib/Model/IMAPMessage.php	Exposes inlineAttachments in full message payload; updates HTML sanitization call signature.
lib/Migration/Version5008Date20260320125737.php	Adds DB columns for `content_id` and `disposition` on `mail_attachments`.
lib/IMAP/MessageMapper.php	Adjusts attachment enumeration and attachment metadata extraction (type/disposition/content-id).
lib/Db/LocalAttachment.php	Adds contentId/disposition fields and helper for disposition checks.
lib/Controller/MessagesController.php	Minor formatting adjustment in HTML body retrieval path.
lib/Contracts/IAttachmentService.php	Improves return type doc for getAttachment() to include ISimpleFile.
lib/Attachment.php	Extends Attachment model to carry contentId/disposition; improves name/disposition normalization.
appinfo/info.xml	Bumps app version.
REUSE.toml	Adds REUSE annotation for the new test fixture.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T16:28:10Z

lib/IMAP/MessageMapper.php

 			}

-			if (!empty($attachmentIds) && !in_array($part->getMimeId(), $attachmentIds, true)) {
+			if (!in_array($part->getMimeId(), $attachmentIds, true)) {


In getAttachments(), when $attachmentIds is empty (the default), the in_array($part->getMimeId(), $attachmentIds, true) check will always be false and the loop will continue for every part. This makes getAttachments() return an empty list in the common “fetch all attachments” call path. The ID filter should only run when $attachmentIds is non-empty (or invert the logic to explicitly handle the empty case).

Suggested change

if (!in_array($part->getMimeId(), $attachmentIds, true)) {

if (count($attachmentIds) > 0 && !in_array($part->getMimeId(), $attachmentIds, true)) {

Copilot · 2026-03-25T16:28:10Z

lib/Service/HtmlPurify/TransformURLScheme.php

+		$inlineAttachment = array_find($this->inlineAttachments, static function ($attachment) use ($uri) {
+			return $attachment['cid'] === $uri->path;
+		});


array_find() is used here, but it isn’t available in the PHP versions Nextcloud Mail typically supports and there’s no polyfill in this repository. This will cause a fatal error when rewriting cid: URLs. Please replace it with a simple foreach/array_filter-based lookup that works on all supported PHP versions.

Suggested change

$inlineAttachment = array_find($this->inlineAttachments, static function ($attachment) use ($uri) {

return $attachment['cid'] === $uri->path;

});

$inlineAttachment = null;

foreach ($this->inlineAttachments as $attachment) {

if ($attachment['cid'] === $uri->path) {

$inlineAttachment = $attachment;

break;

}

}

Copilot · 2026-03-25T16:28:11Z

src/store/mainStore/actions.js


 import { showError, showWarning, TOAST_DEFAULT_TIMEOUT } from '@nextcloud/dialogs'
 import { translate as t } from '@nextcloud/l10n'
+import { generateUrl } from '@nextcloud/router'


generateUrl is imported but never used in this module. With the repo’s ESLint setup this is likely to fail CI (no-unused-vars). Please remove the unused import.

Suggested change

import { generateUrl } from '@nextcloud/router'

Copilot · 2026-03-25T16:28:11Z

lib/IMAP/MessageMapper.php


+		//		// Serve all files with a content-disposition of "attachment" to prevent Cross-Site Scripting
+		//		$mimePart->setDisposition('attachment');
+		//
+		//		// Extract headers from part
+		//		$cdEl = $mimeHeaders['content-disposition'];
+		//		$contentDisposition = $cdEl instanceof Horde_Mime_Headers_ContentParam
+		//			? array_change_key_case($cdEl->params, CASE_LOWER)
+		//			: null;
+		//		if (!is_null($contentDisposition) && isset($contentDisposition['filename'])) {
+		//			$mimePart->setDispositionParameter('filename', $contentDisposition['filename']);
+		//		} else {
+		//			$ctEl = $mimeHeaders['content-type'];
+		//			$contentTypeParams = $ctEl instanceof Horde_Mime_Headers_ContentParam
+		//				? array_change_key_case($ctEl->params, CASE_LOWER)
+		//				: null;
+		//			if (isset($contentTypeParams['name'])) {
+		//				$mimePart->setContentTypeParameter('name', $contentTypeParams['name']);


There’s a large block of commented-out legacy code left in getAttachment(). Keeping dead code in-line makes future maintenance harder and obscures the current behavior (especially around content-disposition/type handling). Please remove these commented sections now that the new implementation is in place.

Suggested change

// // Serve all files with a content-disposition of "attachment" to prevent Cross-Site Scripting

// $mimePart->setDisposition('attachment');

//

// // Extract headers from part

// $cdEl = $mimeHeaders['content-disposition'];

// $contentDisposition = $cdEl instanceof Horde_Mime_Headers_ContentParam

// ? array_change_key_case($cdEl->params, CASE_LOWER)

// : null;

// if (!is_null($contentDisposition) && isset($contentDisposition['filename'])) {

// $mimePart->setDispositionParameter('filename', $contentDisposition['filename']);

// } else {

// $ctEl = $mimeHeaders['content-type'];

// $contentTypeParams = $ctEl instanceof Horde_Mime_Headers_ContentParam

// ? array_change_key_case($ctEl->params, CASE_LOWER)

// : null;

// if (isset($contentTypeParams['name'])) {

// $mimePart->setContentTypeParameter('name', $contentTypeParams['name']);

kesselb added 4 commits March 24, 2026 22:49

test: Add tests for getAttachment and S/MIME

9659d4e

AI-assisted: Claude Sonnet 4.6 Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

wip forward inline images

9f3126f

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

docs: Annotate array shape

23c07aa

The charset item is always there Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

docs: Annotate return array for getAttachment

362b0c0

Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>

kesselb self-assigned this Mar 24, 2026

kesselb added the 2. developing label Mar 24, 2026

kesselb commented Mar 24, 2026

View reviewed changes

kesselb requested a review from Copilot March 25, 2026 16:22

Copilot started reviewing on behalf of kesselb March 25, 2026 16:23 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Forward inline images#12651

Forward inline images#12651
kesselb wants to merge 4 commits intomainfrom
feat-forward-inline-images

kesselb commented Mar 24, 2026

Uh oh!

kesselb Mar 24, 2026

Uh oh!

kesselb Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (!in_array($part->getMimeId(), $attachmentIds, true)) {
	if (count($attachmentIds) > 0 && !in_array($part->getMimeId(), $attachmentIds, true)) {

-		$inlineAttachment = array_find($this->inlineAttachments, static function ($attachment) use ($uri) {
-			return $attachment['cid'] === $uri->path;
-		});
+		$inlineAttachment = null;
+		foreach ($this->inlineAttachments as $attachment) {
+			if ($attachment['cid'] === $uri->path) {
+				$inlineAttachment = $attachment;
+				break;
+			}
+		}

Conversation

kesselb commented Mar 24, 2026

Uh oh!

kesselb Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

kesselb Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants