From 319ccbc8551021bbb065ab27b5d9a91b9a672e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20de=20Jager?= Date: Sat, 20 Jun 2026 15:38:54 +0200 Subject: [PATCH 01/11] feat(composer): add "Insert image from URL" to the rich text editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Joël de Jager --- appinfo/routes.php | 5 + lib/Controller/ImageProxyController.php | 142 ++++++++++++++++ src/ckeditor/image/ImageFromUrlPlugin.js | 33 ++++ src/components/TextEditor.vue | 83 +++++++++- src/service/ImageProxyService.js | 20 +++ .../Controller/ImageProxyControllerTest.php | 151 ++++++++++++++++++ 6 files changed, 433 insertions(+), 1 deletion(-) create mode 100644 lib/Controller/ImageProxyController.php create mode 100644 src/ckeditor/image/ImageFromUrlPlugin.js create mode 100644 src/service/ImageProxyService.js create mode 100644 tests/Unit/Controller/ImageProxyControllerTest.php diff --git a/appinfo/routes.php b/appinfo/routes.php index 411af41b6a..5e65106a29 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -305,6 +305,11 @@ 'url' => '/proxy', 'verb' => 'GET' ], + [ + 'name' => 'imageProxy#fetch', + 'url' => '/api/image/proxy', + 'verb' => 'GET' + ], [ 'name' => 'settings#index', 'url' => '/api/settings/provisioning', diff --git a/lib/Controller/ImageProxyController.php b/lib/Controller/ImageProxyController.php new file mode 100644 index 0000000000..a7db2a50c3 --- /dev/null +++ b/lib/Controller/ImageProxyController.php @@ -0,0 +1,142 @@ + tag and that are safe to + * embed. SVG is intentionally excluded as it can carry active content. + */ + private const ALLOWED_MIME_TYPES = [ + 'image/png', + 'image/jpeg', + 'image/gif', + 'image/webp', + 'image/bmp', + ]; + + public function __construct( + string $appName, + IRequest $request, + private IClientService $clientService, + private IRemoteHostValidator $remoteHostValidator, + private IMimeTypeDetector $mimeTypeDetector, + private LoggerInterface $logger, + ) { + parent::__construct($appName, $request); + } + + /** + * Fetch an external image and return it as a data: URI. + * + * @NoAdminRequired + * @UserRateThrottle(limit=50, period=60) + */ + #[UserRateLimit(limit: 50, period: 60)] + public function fetch(string $url): JSONResponse { + $scheme = parse_url($url, PHP_URL_SCHEME); + if ($scheme !== 'http' && $scheme !== 'https') { + return new JSONResponse(['message' => 'Invalid URL'], Http::STATUS_BAD_REQUEST); + } + + // Reject internal/local hosts up front (SSRF). The client throwing a + // LocalServerException below additionally guards against redirects. + if (!$this->remoteHostValidator->isValid($url)) { + return new JSONResponse(['message' => 'Forbidden URL'], Http::STATUS_FORBIDDEN); + } + + $client = $this->clientService->newClient(); + try { + $response = $client->get($url, [ + 'timeout' => 10, + 'stream' => true, + ]); + } catch (LocalServerException $e) { + $this->logger->warning('Blocked image insert from forbidden URL', [ + 'exception' => $e, + ]); + return new JSONResponse(['message' => 'Forbidden URL'], Http::STATUS_FORBIDDEN); + } catch (ClientExceptionInterface $e) { + $this->logger->info('Could not fetch image to insert', [ + 'exception' => $e, + ]); + return new JSONResponse(['message' => 'Could not fetch image'], Http::STATUS_BAD_GATEWAY); + } + + $body = $response->getBody(); + if (!is_resource($body)) { + return new JSONResponse(['message' => 'Could not fetch image'], Http::STATUS_BAD_GATEWAY); + } + + // Read the response incrementally to enforce the size limit without + // buffering arbitrarily large responses in memory. + $content = ''; + while (!feof($body)) { + $chunk = fread($body, 8192); + if ($chunk === false) { + break; + } + $content .= $chunk; + if (strlen($content) > self::MAX_IMAGE_SIZE) { + fclose($body); + return new JSONResponse(['message' => 'Image too large'], Http::STATUS_REQUEST_ENTITY_TOO_LARGE); + } + } + fclose($body); + + // Detect the type from the actual bytes instead of trusting the remote + // Content-Type header. + $mimeType = $this->mimeTypeDetector->detectString($content); + if (!in_array($mimeType, self::ALLOWED_MIME_TYPES, true)) { + return new JSONResponse(['message' => 'Unsupported image type'], Http::STATUS_UNSUPPORTED_MEDIA_TYPE); + } + + $dataUri = 'data:' . $mimeType . ';base64,' . base64_encode($content); + return new JSONResponse(['data' => $dataUri]); + } +} diff --git a/src/ckeditor/image/ImageFromUrlPlugin.js b/src/ckeditor/image/ImageFromUrlPlugin.js new file mode 100644 index 0000000000..4b5a80d1d5 --- /dev/null +++ b/src/ckeditor/image/ImageFromUrlPlugin.js @@ -0,0 +1,33 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import imageUrlIcon from '@mdi/svg/svg/image-plus.svg?raw' +import { translate as t } from '@nextcloud/l10n' +import { ButtonView, Plugin } from 'ckeditor5' + +/** + * Adds a toolbar button that lets the user insert an image by URL. The button + * only fires an event; the surrounding Vue component (TextEditor) opens a dialog, + * downloads the image through the server and inserts it as a data: URI. + */ +export default class ImageFromUrlPlugin extends Plugin { + init() { + const editor = this.editor + + editor.ui.componentFactory.add('imageFromUrl', (locale) => { + const button = new ButtonView(locale) + button.set({ + label: t('mail', 'Insert image from URL'), + icon: imageUrlIcon, + tooltip: true, + isEnabled: true, + }) + button.on('execute', () => { + editor.fire('mail:insertImageFromUrl') + }) + return button + }) + } +} diff --git a/src/components/TextEditor.vue b/src/components/TextEditor.vue index 9f970fdae1..644cc0ede5 100644 --- a/src/components/TextEditor.vue +++ b/src/components/TextEditor.vue @@ -18,13 +18,27 @@ class="editor" @input="onEditorInput" @ready="onEditorReady" /> + + + + diff --git a/src/components/SignatureSettings.vue b/src/components/SignatureSettings.vue index e5bdc18d27..526ddd35a4 100644 --- a/src/components/SignatureSettings.vue +++ b/src/components/SignatureSettings.vue @@ -28,8 +28,9 @@
@@ -66,8 +67,9 @@ import { mapStores } from 'pinia' import IconCheck from 'vue-material-design-icons/Check.vue' import TextEditor from './TextEditor.vue' import logger from '../logger.js' +import { EDITOR_MODE_HTML } from '../store/constants.js' import useMainStore from '../store/mainStore.js' -import { detect, toHtml } from '../util/text.js' +import { detect, toHtml, toPlain } from '../util/text.js' export default { name: 'SignatureSettings', @@ -98,6 +100,10 @@ export default { computed: { ...mapStores(useMainStore), + editorIsHtml() { + return this.account.editorMode === EDITOR_MODE_HTML + }, + identities() { const identities = this.account.aliases.map((alias) => { return { @@ -122,6 +128,15 @@ export default { }, watch: { + // The signature editor follows the account's writing mode. When that mode + // changes (e.g. via the writing-mode setting), re-encode the in-editor + // signature so plain text never shows raw HTML and vice versa. Switching to + // plain text is lossy (images/links are dropped); the writing-mode setting + // warns the user before the change reaches here. + editorIsHtml() { + this.signature = this.formatSignature(this.signature) + }, + async signatureAboveQuote(val, oldVal) { try { await this.mainStore.patchAccount({ @@ -146,9 +161,22 @@ export default { changeIdentity(identity) { logger.debug('select identity', { identity }) this.identity = identity - this.signature = identity.signature - ? toHtml(detect(identity.signature)).value - : '' + this.signature = this.formatSignature(identity.signature) + }, + + /** + * Encodes a stored signature into the format the editor currently expects + * (HTML in rich-text mode, plain text otherwise). + * + * @param {string|null} signature the stored signature + * @return {string} the signature in the active editor format + */ + formatSignature(signature) { + if (!signature) { + return '' + } + const detected = detect(signature) + return this.editorIsHtml ? toHtml(detected).value : toPlain(detected).value }, async deleteSignature() { diff --git a/src/views/Home.vue b/src/views/Home.vue index 89ff8c6d68..7fc60e9716 100644 --- a/src/views/Home.vue +++ b/src/views/Home.vue @@ -58,9 +58,11 @@ export default { data() { return { hasComposerSession: false, - // The currently open account-settings dialog. The account is kept - // after closing so the dialog stays mounted for its out-transition. - settingsAccount: null, + // Id of the account whose settings dialog is open. The id is kept after + // closing so the dialog stays mounted for its out-transition. The account + // object itself is resolved reactively (see settingsAccount) so settings + // sub-components observe store updates live instead of a stale snapshot. + settingsAccountId: null, settingsOpen: false, settingsSection: undefined, } @@ -80,6 +82,12 @@ export default { activeMailbox() { return this.mainStore.getMailbox(this.$route.params.mailboxId) }, + + settingsAccount() { + return this.settingsAccountId !== null + ? this.mainStore.getAccount(this.settingsAccountId) + : null + }, }, watch: { @@ -105,7 +113,7 @@ export default { immediate: true, handler(settings) { if (settings?.accountId) { - this.settingsAccount = this.mainStore.getAccount(settings.accountId) + this.settingsAccountId = settings.accountId this.settingsSection = settings.section // Mount first (settingsAccount), then open on the next tick so the // dialog's `open` watcher fires and runs its on-open side effects. From 01882c4e3109fb08790f93067418f4088b4ea08d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20de=20Jager?= Date: Sun, 21 Jun 2026 21:26:34 +0200 Subject: [PATCH 10/11] fix(message): render stacked images and external SVGs correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - allow HTML5
/
in the mail HTML purifier so CKEditor image figures keep stacking instead of collapsing into inline, side-by-side images - serve proxied external SVG images as sanitised image/svg+xml so browsers render them instead of leaving them blank Signed-off-by: Joël de Jager --- lib/Controller/ProxyController.php | 30 +++++++++ lib/Service/Html.php | 9 +++ tests/Unit/Controller/ProxyControllerTest.php | 64 +++++++++++++++++++ 3 files changed, 103 insertions(+) diff --git a/lib/Controller/ProxyController.php b/lib/Controller/ProxyController.php index 3b44a15254..812037b4f2 100644 --- a/lib/Controller/ProxyController.php +++ b/lib/Controller/ProxyController.php @@ -13,6 +13,7 @@ use OCA\Mail\Html\ProxyHmacGenerator; use OCA\Mail\Http\ProxyDownloadResponse; use OCA\Mail\Service\MailManager; +use OCA\Mail\Service\SvgSanitizer; use OCP\AppFramework\Controller; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; @@ -28,6 +29,9 @@ use Psr\Log\LoggerInterface; use function file_get_contents; use function hash_equals; +use function ltrim; +use function stripos; +use function str_starts_with; #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)] class ProxyController extends Controller { @@ -37,6 +41,7 @@ class ProxyController extends Controller { private LoggerInterface $logger; private ProxyHmacGenerator $hmacGenerator; private MailManager $mailManager; + private SvgSanitizer $svgSanitizer; private ?string $userId; public function __construct(string $appName, @@ -47,6 +52,7 @@ public function __construct(string $appName, ProxyHmacGenerator $hmacGenerator, LoggerInterface $logger, MailManager $mailManager, + SvgSanitizer $svgSanitizer, ?string $userId) { parent::__construct($appName, $request); $this->request = $request; @@ -56,6 +62,7 @@ public function __construct(string $appName, $this->logger = $logger; $this->hmacGenerator = $hmacGenerator; $this->mailManager = $mailManager; + $this->svgSanitizer = $svgSanitizer; $this->userId = $userId; } @@ -112,6 +119,29 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response { $content = file_get_contents(__DIR__ . '/../../img/blocked-image.png'); } + $content = (string)$content; + + // Browsers sniff raster image formats in tags, but they refuse to + // render SVG unless it is served with the image/svg+xml content type. + // Detect and sanitise SVG markup so external SVG logos are displayed + // instead of staying blank. Sanitising also strips any active content in + // case the response is fetched through a direct (non-) navigation. + if ($this->looksLikeSvg($content)) { + $content = $this->svgSanitizer->sanitize($content); + return new ProxyDownloadResponse($content, $src, 'image/svg+xml'); + } + return new ProxyDownloadResponse($content, $src, 'application/octet-stream'); } + + /** + * Heuristically decide whether the given bytes are an SVG document. + */ + private function looksLikeSvg(string $content): bool { + $start = ltrim($content); + $hasSvgPrologue = str_starts_with($start, 'getHTMLDefinition(true); + + // HTMLPurifier defaults to an HTML 4.01 schema which does not know the + // HTML5
/
elements. Without registering them the + // figure wrappers are stripped and the contained images collapse from + // stacked blocks into inline siblings (rendered side by side). Editors + // such as CKEditor wrap images in figures, so keep them as block elements. + $def->addElement('figure', 'Block', 'Flow', 'Common'); + $def->addElement('figcaption', 'Block', 'Flow', 'Common'); + $def->info_attr_transform_post['imagesrc'] = new TransformImageSrc($this->urlGenerator); $def->info_attr_transform_post['cssbackground'] = new TransformStyleURLs($this->urlGenerator); $def->info_attr_transform_post['htmllinks'] = new TransformHTMLLinks(); diff --git a/tests/Unit/Controller/ProxyControllerTest.php b/tests/Unit/Controller/ProxyControllerTest.php index ee6ed5cf0a..0c956fb6b7 100644 --- a/tests/Unit/Controller/ProxyControllerTest.php +++ b/tests/Unit/Controller/ProxyControllerTest.php @@ -15,6 +15,7 @@ use OCA\Mail\Html\ProxyHmacGenerator; use OCA\Mail\Http\ProxyDownloadResponse; use OCA\Mail\Service\MailManager; +use OCA\Mail\Service\SvgSanitizer; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Response; @@ -53,6 +54,9 @@ class ProxyControllerTest extends TestCase { /** @var MailManager|MockObject */ private $mailManager; + /** @var SvgSanitizer|MockObject */ + private $svgSanitizer; + private string $userId = 'user'; /** @var ProxyController */ @@ -68,6 +72,7 @@ protected function setUp(): void { $this->clientService = $this->createMock(IClientService::class); $this->hmacGenerator = $this->createMock(ProxyHmacGenerator::class); $this->mailManager = $this->createMock(MailManager::class); + $this->svgSanitizer = $this->createMock(SvgSanitizer::class); $this->logger = new NullLogger(); } @@ -90,6 +95,7 @@ public function testProxyWithoutCookies(): void { $this->hmacGenerator, $this->logger, $this->mailManager, + $this->svgSanitizer, $this->userId, ); @@ -136,12 +142,67 @@ public function testProxy(): void { $this->hmacGenerator, $this->logger, $this->mailManager, + $this->svgSanitizer, + $this->userId, + ); + + $response = $this->controller->proxy($src, $id, $validHmac); + + $this->assertInstanceOf(ProxyDownloadResponse::class, $response); + } + + public function testProxySanitizesAndServesSvg(): void { + $src = 'https://example.com/logo.svg'; + $id = 1; + $validHmac = 'valid-hmac-hash'; + $content = ''; + $sanitized = ''; + $httpResponse = $this->createMock(IResponse::class); + $this->request->expects(self::once()) + ->method('passesStrictCookieCheck') + ->willReturn(true); + $this->session->expects($this->once()) + ->method('close'); + $this->hmacGenerator->expects($this->once()) + ->method('generate') + ->with($id, $src) + ->willReturn($validHmac); + $this->mailManager->expects($this->once()) + ->method('getMessage') + ->with($this->userId, $id); + $client = $this->getMockBuilder(IClient::class)->getMock(); + $this->clientService->expects($this->once()) + ->method('newClient') + ->willReturn($client); + $client->expects($this->once()) + ->method('get') + ->with($src) + ->willReturn($httpResponse); + $httpResponse->expects($this->once()) + ->method('getBody') + ->willReturn($content); + $this->svgSanitizer->expects($this->once()) + ->method('sanitize') + ->with($content) + ->willReturn($sanitized); + $this->controller = new ProxyController( + $this->appName, + $this->request, + $this->urlGenerator, + $this->session, + $this->clientService, + $this->hmacGenerator, + $this->logger, + $this->mailManager, + $this->svgSanitizer, $this->userId, ); $response = $this->controller->proxy($src, $id, $validHmac); $this->assertInstanceOf(ProxyDownloadResponse::class, $response); + $this->assertSame('image/svg+xml', $response->getHeaders()['Content-Type']); + $this->assertSame($sanitized, $response->render()); } public function testProxyWithInvalidHmac(): void { @@ -172,6 +233,7 @@ public function testProxyWithInvalidHmac(): void { $this->hmacGenerator, $this->logger, $this->mailManager, + $this->svgSanitizer, $this->userId, ); @@ -199,6 +261,7 @@ public function testProxyWithMissingHmacParameters(): void { $this->hmacGenerator, $this->logger, $this->mailManager, + $this->svgSanitizer, $this->userId, ); @@ -231,6 +294,7 @@ public function testProxyWithMessageNotOwnedByUser(): void { $this->hmacGenerator, $this->logger, $this->mailManager, + $this->svgSanitizer, $this->userId, ); From 59f3bff8031a19472b16745fcaa293f7fc4a3ba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=ABl=20de=20Jager?= Date: Sun, 21 Jun 2026 21:48:25 +0200 Subject: [PATCH 11/11] fix(composer): make image alignment explicit and portable when sending MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Block-image alignment was encoded only as CKEditor CSS classes, which recipients do not load. The unstyled (default) figure was left without any inline alignment, so each client rendered it differently — Gmail and Thunderbird centred it while Nextcloud Mail left-aligned it. Inline auto-margins (matching CKEditor's positioning) plus text-align on every image figure, including the default, so images align consistently across all mail clients. Physical margins are used for Outlook compatibility. Signed-off-by: Joël de Jager --- lib/Service/MimeMessage.php | 42 ++++++++++++++++---------- tests/Unit/Service/MimeMessageTest.php | 39 ++++++++++++++++++++++-- 2 files changed, 63 insertions(+), 18 deletions(-) diff --git a/lib/Service/MimeMessage.php b/lib/Service/MimeMessage.php index 7bcd680890..abffd2e593 100644 --- a/lib/Service/MimeMessage.php +++ b/lib/Service/MimeMessage.php @@ -253,18 +253,27 @@ private function normalizeImageDimensions(DOMDocument $doc): void { } /** - * Translates the editor's image-alignment classes into an inline text-align - * style on the wrapping
. Those classes rely on the editor's own - * stylesheet, which recipients do not load; aligning the (inline) image via - * text-align on its block wrapper is honoured by virtually every mail client. - * The unstyled, left-aligned default needs no rewriting. + * Translates the editor's image-alignment classes into inline styles on the + * wrapping
. The editor expresses alignment through CSS classes + * backed by its own stylesheet, which recipients do not load. Without + * inlining, every client falls back to its own default rendering of a bare + *
(some left-align it, others centre it), so even the unstyled + * default must be made explicit to render consistently everywhere. + * + * A block image is positioned by the auto margins of its (width-constrained) + *
box, exactly like the editor does it; text-align additionally + * aligns the image inside a figure that spans the full width. Physical + * margins are used because the Word engine behind Outlook ignores the + * logical margin-inline shorthand. */ private function normalizeImageAlignment(DOMDocument $doc): void { $alignments = [ - 'image-style-block-align-left' => 'left', - 'image-style-align-center' => 'center', - 'image-style-block-align-right' => 'right', + 'image-style-align-center' => 'margin-left: auto; margin-right: auto; text-align: center;', + 'image-style-block-align-right' => 'margin-left: auto; margin-right: 0; text-align: right;', + 'image-style-block-align-left' => 'margin-left: 0; margin-right: auto; text-align: left;', ]; + // The unstyled state is the editor's left-aligned default. + $default = 'margin-left: 0; margin-right: auto; text-align: left;'; foreach ($doc->getElementsByTagName('figure') as $figure) { if (!($figure instanceof DOMElement)) { @@ -272,19 +281,20 @@ private function normalizeImageAlignment(DOMDocument $doc): void { } $classes = $figure->getAttribute('class'); - if ($classes === '') { + if (!str_contains($classes, 'image')) { continue; } - foreach ($alignments as $class => $align) { - if (!str_contains($classes, $class)) { - continue; + $alignment = $default; + foreach ($alignments as $class => $style) { + if (str_contains($classes, $class)) { + $alignment = $style; + break; } - - $style = rtrim(trim($figure->getAttribute('style')), ';'); - $figure->setAttribute('style', ($style === '' ? '' : $style . '; ') . 'text-align: ' . $align); - break; } + + $style = rtrim(trim($figure->getAttribute('style')), ';'); + $figure->setAttribute('style', ($style === '' ? '' : $style . '; ') . $alignment); } } diff --git a/tests/Unit/Service/MimeMessageTest.php b/tests/Unit/Service/MimeMessageTest.php index e00c41909b..48603dbf2e 100644 --- a/tests/Unit/Service/MimeMessageTest.php +++ b/tests/Unit/Service/MimeMessageTest.php @@ -496,11 +496,46 @@ public function testNormalizeImageAlignment(): void { /** @var Horde_Mime_Part[] $subParts */ $subParts = $part->getParts(); $htmlBody = $subParts[1]->getContents(); + $this->assertStringContainsString('margin-left: auto; margin-right: auto', $htmlBody); $this->assertStringContainsString('text-align: center', $htmlBody); } - public function testNormalizeImageAlignmentLeavesDefaultUntouched(): void { - $html = '
'; + public function testNormalizeImageAlignmentRight(): void { + $html = '
'; + + $part = $this->mimeMessage->build( + null, + $html, + false, + [], + ); + + /** @var Horde_Mime_Part[] $subParts */ + $subParts = $part->getParts(); + $htmlBody = $subParts[1]->getContents(); + $this->assertStringContainsString('margin-left: auto; margin-right: 0', $htmlBody); + $this->assertStringContainsString('text-align: right', $htmlBody); + } + + public function testNormalizeImageAlignmentMakesDefaultExplicit(): void { + $html = '
'; + + $part = $this->mimeMessage->build( + null, + $html, + false, + [], + ); + + /** @var Horde_Mime_Part[] $subParts */ + $subParts = $part->getParts(); + $htmlBody = $subParts[1]->getContents(); + $this->assertStringContainsString('margin-left: 0; margin-right: auto', $htmlBody); + $this->assertStringContainsString('text-align: left', $htmlBody); + } + + public function testNormalizeImageAlignmentIgnoresNonImageFigures(): void { + $html = '
'; $part = $this->mimeMessage->build( null,