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..8235280b26 --- /dev/null +++ b/lib/Controller/ImageProxyController.php @@ -0,0 +1,170 @@ + tag and that are safe to + * embed. + */ + private const ALLOWED_MIME_TYPES = [ + 'image/png', + 'image/jpeg', + 'image/gif', + 'image/webp', + 'image/bmp', + 'image/svg+xml', + ]; + + public function __construct( + string $appName, + IRequest $request, + private IClientService $clientService, + private IRemoteHostValidator $remoteHostValidator, + private IMimeTypeDetector $mimeTypeDetector, + private SvgSanitizer $svgSanitizer, + 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); + + // finfo frequently reports SVG (which is plain XML) as a text/* type, so + // fall back to sniffing the markup to support SVG logos. + if ($mimeType !== 'image/svg+xml' && $this->looksLikeSvg($content)) { + $mimeType = 'image/svg+xml'; + } + + if (!in_array($mimeType, self::ALLOWED_MIME_TYPES, true)) { + return new JSONResponse(['message' => 'Unsupported image type'], Http::STATUS_UNSUPPORTED_MEDIA_TYPE); + } + + if ($mimeType === 'image/svg+xml') { + $content = $this->svgSanitizer->sanitize($content); + } + + $dataUri = 'data:' . $mimeType . ';base64,' . base64_encode($content); + return new JSONResponse(['data' => $dataUri]); + } + + /** + * Heuristically decide whether the given bytes are an SVG document. + */ + private function looksLikeSvg(string $content): bool { + $start = ltrim($content); + $hasSvgRoot = str_starts_with($start, '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, 'addHeaders($headers); $mimeMessage = new MimeMessage( - new DataUriParser() + new DataUriParser(), + new SvgSanitizer(), ); $mimePart = $mimeMessage->build( diff --git a/lib/Service/Html.php b/lib/Service/Html.php index d56e8f1448..5e37eea584 100755 --- a/lib/Service/Html.php +++ b/lib/Service/Html.php @@ -154,6 +154,15 @@ public function sanitizeHtmlMailBody(int $messageId, string $mailBody, array $in // Rewrite URL for redirection and proxying of content /** @var HTMLPurifier_HTMLDefinition $def */ $def = $config->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/lib/Service/MailTransmission.php b/lib/Service/MailTransmission.php index ad5a4d2a1b..ac87ff41ec 100644 --- a/lib/Service/MailTransmission.php +++ b/lib/Service/MailTransmission.php @@ -133,7 +133,8 @@ public function sendMessage(Account $account, LocalMessage $localMessage): void $mail->addHeaders($headers); $mimeMessage = new MimeMessage( - new DataUriParser() + new DataUriParser(), + new SvgSanitizer(), ); $mimePart = $mimeMessage->build( $localMessage->getBodyPlain(), diff --git a/lib/Service/MimeMessage.php b/lib/Service/MimeMessage.php index 1188d18500..abffd2e593 100644 --- a/lib/Service/MimeMessage.php +++ b/lib/Service/MimeMessage.php @@ -20,9 +20,11 @@ class MimeMessage { private DataUriParser $uriParser; + private SvgSanitizer $svgSanitizer; - public function __construct(DataUriParser $uriParser) { + public function __construct(DataUriParser $uriParser, SvgSanitizer $svgSanitizer) { $this->uriParser = $uriParser; + $this->svgSanitizer = $svgSanitizer; } /** @@ -82,6 +84,8 @@ private function buildMessagePart(?string $contentPlain, ?string $contentHtml, a $doc = Parser::parseToDomDocument($source); array_push($inlineAttachments, ...$this->extractDataUriImages($doc)); $this->rewriteSrcToCid($doc); + $this->normalizeImageDimensions($doc); + $this->normalizeImageAlignment($doc); $htmlContent = $doc->saveHTML(); $htmlPart = new Horde_Mime_Part(); @@ -172,10 +176,19 @@ private function extractDataUriImages(DOMDocument $doc): array { $part->setCharset($dataUri->getParameters()['charset']); $part->setName('embedded_image_' . $id); $part->setDisposition('inline'); - if ($dataUri->isBase64()) { + if ($dataUri->getMediaType() === 'image/svg+xml') { + // Strip any active content from SVGs before they are sent. + $raw = $dataUri->isBase64() + ? base64_decode($dataUri->getData(), true) + : rawurldecode($dataUri->getData()); $part->setTransferEncoding('base64'); + $part->setContents(base64_encode($this->svgSanitizer->sanitize($raw === false ? '' : $raw))); + } else { + if ($dataUri->isBase64()) { + $part->setTransferEncoding('base64'); + } + $part->setContents($dataUri->getData()); } - $part->setContents($dataUri->getData()); $cid = $part->setContentId(); $parts[] = $part; @@ -185,6 +198,106 @@ private function extractDataUriImages(DOMDocument $doc): array { return $parts; } + /** + * Mirrors pixel dimensions set via the editor's resize feature (stored as + * inline CSS) onto the width/height attributes. Many email clients drop + * inline styles and classes but honour these attributes, so without them a + * resized image would fall back to its (often huge) natural size. + */ + private function normalizeImageDimensions(DOMDocument $doc): void { + foreach ($doc->getElementsByTagName('img') as $image) { + if (!($image instanceof DOMElement)) { + continue; + } + + // The editor stores the resize width as inline CSS. For inline images + // it lives on the ; for block images it lives on the wrapping + //
while the aspect ratio stays on the . A linked image + // is nested one level deeper (
), so walk + // up the ancestors to find the figure rather than only checking the + // direct parent. Inspect both the image's and the figure's style. + $figureStyle = ''; + $ancestor = $image->parentNode; + while ($ancestor instanceof DOMElement) { + if (strtolower($ancestor->tagName) === 'figure') { + $figureStyle = $ancestor->getAttribute('style'); + break; + } + $ancestor = $ancestor->parentNode; + } + $style = $image->getAttribute('style') . ';' . $figureStyle; + + if (preg_match('/(?setAttribute('width', (string)$width); + + // Derive the height from the aspect ratio the editor stores so clients + // that ignore CSS keep the correct proportions. + if (preg_match('#aspect-ratio:\s*([\d.]+)\s*/\s*([\d.]+)#i', $style, $ratioMatch) === 1) { + $ratioWidth = (float)$ratioMatch[1]; + $ratioHeight = (float)$ratioMatch[2]; + if ($ratioWidth > 0.0 && $ratioHeight > 0.0) { + $height = (int)round($width * $ratioHeight / $ratioWidth); + if ($height > 0) { + $image->setAttribute('height', (string)$height); + } + } + } + } + } + + /** + * 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-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)) { + continue; + } + + $classes = $figure->getAttribute('class'); + if (!str_contains($classes, 'image')) { + 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 . '; ') . $alignment); + } + } + /** * Rewrites src attributes of img elements that carry a data-cid attribute * back to their cid: reference, as required by the MIME structure for diff --git a/lib/Service/SvgSanitizer.php b/lib/Service/SvgSanitizer.php new file mode 100644 index 0000000000..602ab1a00c --- /dev/null +++ b/lib/Service/SvgSanitizer.php @@ -0,0 +1,105 @@ +/CID context where scripts do + * not execute, but they are still sanitised as defence in depth: any document + * that cannot be parsed safely is dropped entirely. + */ +class SvgSanitizer { + /** Elements that can carry or execute active content. */ + private const FORBIDDEN_ELEMENTS = [ + 'script', + 'foreignObject', + 'handler', + 'listener', + 'set', + ]; + + /** + * @param string $svg The raw (decoded) SVG markup + * @return string The sanitised markup, or an empty string if it cannot be + * parsed safely + */ + public function sanitize(string $svg): string { + if (trim($svg) === '') { + return ''; + } + + // A DOCTYPE or entity declaration is not needed for plain SVG graphics + // and is a common XXE / entity-expansion vector. Reject such documents. + if (preg_match('/loadXML($svg, LIBXML_NONET); + libxml_clear_errors(); + libxml_use_internal_errors($previousErrors); + + if (!$loaded || $dom->documentElement === null) { + return ''; + } + + $xpath = new DOMXPath($dom); + + // Remove dangerous elements. Matching on the local name catches them + // regardless of any namespace prefix (e.g. ). + foreach (self::FORBIDDEN_ELEMENTS as $tag) { + $nodes = $xpath->query('//*[local-name() = "' . $tag . '"]'); + if ($nodes !== false) { + foreach (iterator_to_array($nodes) as $node) { + $node->parentNode?->removeChild($node); + } + } + } + + $elements = $xpath->query('//*'); + if ($elements !== false) { + foreach ($elements as $element) { + if ($element instanceof DOMElement) { + $this->stripDangerousAttributes($element); + } + } + } + + $result = $dom->saveXML($dom->documentElement); + return $result === false ? '' : $result; + } + + private function stripDangerousAttributes(DOMElement $element): void { + /** @var DOMAttr $attribute */ + foreach (iterator_to_array($element->attributes) as $attribute) { + $name = strtolower($attribute->nodeName); + $value = trim($attribute->nodeValue ?? ''); + + // Inline event handlers (onload, onclick, …). + if (str_starts_with($name, 'on')) { + $element->removeAttributeNode($attribute); + continue; + } + + // Only allow same-document references; strip javascript:, external + // and data: URLs from links and resource references. + if (in_array($name, ['href', 'xlink:href'], true) && !str_starts_with($value, '#')) { + $element->removeAttributeNode($attribute); + } + } + } +} diff --git a/src/ckeditor/image/ImageAlignmentPlugin.js b/src/ckeditor/image/ImageAlignmentPlugin.js new file mode 100644 index 0000000000..60bd8f2e8e --- /dev/null +++ b/src/ckeditor/image/ImageAlignmentPlugin.js @@ -0,0 +1,69 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import alignCenterIcon from '@mdi/svg/svg/format-align-center.svg?raw' +import alignLeftIcon from '@mdi/svg/svg/format-align-left.svg?raw' +import alignRightIcon from '@mdi/svg/svg/format-align-right.svg?raw' +import { translate as t } from '@nextcloud/l10n' +import { addToolbarToDropdown, createDropdown, Plugin } from 'ckeditor5' + +const ALIGNMENT_BUTTONS = [ + 'imageStyle:alignBlockLeft', + 'imageStyle:alignCenter', + 'imageStyle:alignBlockRight', +] + +/** + * Registers an "imageAlignment" toolbar item that groups the image alignment + * options into a single dropdown button — the whole button opens the menu, + * exactly like the text-alignment dropdown. CKEditor's built-in image-style + * grouping renders a split button instead (with a separately hoverable arrow), + * which is what this plugin replaces. + */ +export default class ImageAlignmentPlugin extends Plugin { + static get pluginName() { + return 'ImageAlignmentPlugin' + } + + init() { + const editor = this.editor + const factory = editor.ui.componentFactory + + factory.add('imageAlignment', (locale) => { + const dropdown = createDropdown(locale) + const buttons = ALIGNMENT_BUTTONS.map((name) => factory.create(name)) + + addToolbarToDropdown(dropdown, buttons, { + enableActiveItemFocusOnDropdownOpen: true, + }) + + dropdown.buttonView.set({ + label: t('mail', 'Align image'), + icon: alignLeftIcon, + tooltip: true, + }) + + // Reflect the current alignment on the toolbar button, like the text + // alignment dropdown does. + const command = editor.commands.get('imageStyle') + if (command) { + dropdown.buttonView.bind('icon').to(command, 'value', (value) => { + if (value === 'alignCenter') { + return alignCenterIcon + } + if (value === 'alignBlockRight') { + return alignRightIcon + } + return alignLeftIcon + }) + } + + // Only enable the dropdown while an alignment option is available. + dropdown.bind('isEnabled').toMany(buttons, 'isEnabled', (...enabled) => enabled.some(Boolean)) + + return dropdown + }) + } +} diff --git a/src/ckeditor/image/ImageDropdownPlugin.js b/src/ckeditor/image/ImageDropdownPlugin.js new file mode 100644 index 0000000000..bf805c3ee4 --- /dev/null +++ b/src/ckeditor/image/ImageDropdownPlugin.js @@ -0,0 +1,72 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import folderIcon from '@mdi/svg/svg/folder-image.svg?raw' +import imageIcon from '@mdi/svg/svg/image.svg?raw' +import linkIcon from '@mdi/svg/svg/link-variant.svg?raw' +import uploadIcon from '@mdi/svg/svg/upload.svg?raw' +import { translate as t } from '@nextcloud/l10n' +import { ButtonView, createDropdown, Plugin } from 'ckeditor5' + +/** + * Toolbar dropdown to insert an image, offering three options: uploading a file + * from the computer, picking one from the Nextcloud files, or fetching one by + * URL. Each option only fires an event; the surrounding Vue component + * (TextEditor) performs the actual work. + */ +export default class ImageDropdownPlugin extends Plugin { + init() { + const editor = this.editor + + editor.ui.componentFactory.add('imageDropdown', (locale) => { + const dropdown = createDropdown(locale) + dropdown.class = 'mail-image-insert-dropdown' + dropdown.buttonView.set({ + label: t('mail', 'Insert image'), + icon: imageIcon, + tooltip: true, + }) + + const fire = (event) => () => { + dropdown.isOpen = false + editor.fire(event) + } + + dropdown.panelView.children.add(this._createActionButton( + locale, + t('mail', 'Upload from computer'), + uploadIcon, + fire('mail:uploadImageFromComputer'), + )) + dropdown.panelView.children.add(this._createActionButton( + locale, + t('mail', 'Insert with file manager'), + folderIcon, + fire('mail:insertImageFromFiles'), + )) + dropdown.panelView.children.add(this._createActionButton( + locale, + t('mail', 'Insert via URL'), + linkIcon, + fire('mail:insertImageFromUrl'), + )) + + return dropdown + }) + } + + _createActionButton(locale, label, icon, onExecute) { + const button = new ButtonView(locale) + button.set({ + label, + icon, + tooltip: false, + withText: true, + isEnabled: true, + }) + button.on('execute', onExecute) + return button + } +} diff --git a/src/ckeditor/image/ImageLinkFormPlugin.js b/src/ckeditor/image/ImageLinkFormPlugin.js new file mode 100644 index 0000000000..0861d73091 --- /dev/null +++ b/src/ckeditor/image/ImageLinkFormPlugin.js @@ -0,0 +1,53 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { Plugin } from 'ckeditor5' + +/** + * Adjusts the link form when it is opened for an image. An image link has no + * "displayed text" (the image itself is the link content), so that input is + * hidden to avoid confusion. Dismissing the link balloon while the image stays + * selected returns to the image toolbar, which gives the same "back to the + * image menu" navigation as the text-alternative form. + * + * The implementation only reads CKEditor's public API and toggles a DOM style; + * if a future version renames the form pieces it simply does nothing. + */ +export default class ImageLinkFormPlugin extends Plugin { + static get requires() { + return ['LinkUI'] + } + + static get pluginName() { + return 'ImageLinkFormPlugin' + } + + init() { + const editor = this.editor + + if (!editor.plugins.has('ContextualBalloon')) { + return + } + + const linkUI = editor.plugins.get('LinkUI') + const balloon = editor.plugins.get('ContextualBalloon') + + balloon.on('change:visibleView', () => { + const formView = linkUI.formView + const displayedText = formView?.displayedTextInputView + if (!displayedText?.element || balloon.visibleView !== formView) { + return + } + + const imageUtils = editor.plugins.has('ImageUtils') + ? editor.plugins.get('ImageUtils') + : null + const isOnImage = !!imageUtils?.getClosestSelectedImageElement( + editor.model.document.selection, + ) + displayedText.element.style.display = isOnImage ? 'none' : '' + }) + } +} diff --git a/src/ckeditor/image/ImagePastePlugin.js b/src/ckeditor/image/ImagePastePlugin.js new file mode 100644 index 0000000000..08ad470fcf --- /dev/null +++ b/src/ckeditor/image/ImagePastePlugin.js @@ -0,0 +1,87 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { Plugin } from 'ckeditor5' +import logger from '../../logger.js' +import { fetchImageAsDataUri } from '../../service/ImageProxyService.js' + +const EXTERNAL_SRC = /^https?:\/\//i + +/** + * Inlines images that are pasted from another web page. Such images keep their + * original external `src`, which the editor's strict CSP refuses to load, so + * they render as the broken/alt-text placeholder. Fetching them through the + * server-side image proxy turns them into data: URIs that both display in the + * editor and are sent as inline attachments — exactly like the "insert image + * from URL" feature. + */ +export default class ImagePastePlugin extends Plugin { + static get pluginName() { + return 'ImagePastePlugin' + } + + init() { + const model = this.editor.model + + model.document.on('change:data', () => { + const images = [] + for (const change of model.document.differ.getChanges()) { + if (change.type !== 'insert') { + continue + } + const node = change.position?.nodeAfter + if (node) { + this._collectExternalImages(node, images) + } + } + + images.forEach((image) => this._inlineExternalImage(image)) + }) + } + + /** + * Recursively collects images with an external src from an inserted node. + * + * @param {module:engine/model/node~Node} node the inserted node to inspect + * @param {Array} found accumulator of matching image elements + * @private + */ + _collectExternalImages(node, found) { + if (!node.is?.('element')) { + return + } + + const isImage = node.is('element', 'imageBlock') || node.is('element', 'imageInline') + if (isImage && EXTERNAL_SRC.test(node.getAttribute('src') ?? '')) { + found.push(node) + } + + for (const child of node.getChildren()) { + this._collectExternalImages(child, found) + } + } + + /** + * Fetches an external image through the proxy and rewrites its src to the + * returned data: URI. Runs asynchronously, so the model is re-checked before + * the write in case the element was removed in the meantime. + * + * @param {module:engine/model/element~Element} image the image element + * @private + */ + async _inlineExternalImage(image) { + const src = image.getAttribute('src') + try { + const dataUri = await fetchImageAsDataUri(src) + this.editor.model.change((writer) => { + if (image.root?.rootName && image.parent) { + writer.setAttribute('src', dataUri, image) + } + }) + } catch (error) { + logger.error('Could not inline pasted image', { error, src }) + } + } +} diff --git a/src/components/EditorSettings.vue b/src/components/EditorSettings.vue index f8889b85e4..a6f22eff04 100644 --- a/src/components/EditorSettings.vue +++ b/src/components/EditorSettings.vue @@ -8,20 +8,24 @@

-

@@ -31,6 +35,7 @@ diff --git a/src/components/NavigationAccount.vue b/src/components/NavigationAccount.vue index 0d9b24706e..567154de82 100644 --- a/src/components/NavigationAccount.vue +++ b/src/components/NavigationAccount.vue @@ -96,11 +96,6 @@ - @@ -132,7 +127,6 @@ export default { ActionCheckbox, ActionInput, ActionText, - AccountSettings: () => import(/* webpackChunkName: "account-settings" */ './AccountSettings.vue'), DelegationModal: () => import(/* webpackChunkName: "delegation-modal" */ './DelegationModal.vue'), IconInfo, IconSettings, @@ -193,14 +187,6 @@ export default { computed: { ...mapStores(useMainStore), - showSettings() { - return this.mainStore.showSettingsForAccount(this.account.id) - }, - - showSettingsSection() { - return this.mainStore.showSettingsSectionForAccount(this.account.id) - }, - visible() { return this.account.isUnified !== true && this.account.visible !== false }, 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/components/TextEditor.vue b/src/components/TextEditor.vue index 9f970fdae1..b7b47a8d17 100644 --- a/src/components/TextEditor.vue +++ b/src/components/TextEditor.vue @@ -18,13 +18,44 @@ class="editor" @input="onEditorInput" @ready="onEditorReady" /> + + + + + + + +
'; + $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, ); diff --git a/tests/Unit/Service/MimeMessageTest.php b/tests/Unit/Service/MimeMessageTest.php index 146a3bf26c..48603dbf2e 100644 --- a/tests/Unit/Service/MimeMessageTest.php +++ b/tests/Unit/Service/MimeMessageTest.php @@ -17,6 +17,7 @@ use OCA\Mail\Model\NewMessageData; use OCA\Mail\Service\DataUri\DataUriParser; use OCA\Mail\Service\MimeMessage; +use OCA\Mail\Service\SvgSanitizer; class MimeMessageTest extends TestCase { private DataUriParser $uriParser; @@ -27,7 +28,7 @@ protected function setUp(): void { parent::setUp(); $this->uriParser = new DataUriParser(); - $this->mimeMessage = new MimeMessage($this->uriParser); + $this->mimeMessage = new MimeMessage($this->uriParser, new SvgSanitizer()); $this->account = new Account(new MailAccount()); } @@ -413,6 +414,165 @@ public function testEmbeddedImageSkipNonImages(): void { $this->assertStringNotContainsString('data-cid', $htmlBody); } + public function testNormalizeImageDimensions(): void { + $html = '

'; + + $part = $this->mimeMessage->build( + null, + $html, + false, + [], + ); + + $this->assertEquals('multipart/alternative', $part->getType()); + + /** @var Horde_Mime_Part[] $subParts */ + $subParts = $part->getParts(); + $htmlBody = $subParts[1]->getContents(); + $this->assertStringContainsString('width="400"', $htmlBody); + $this->assertStringContainsString('height="100"', $htmlBody); + } + + public function testNormalizeImageDimensionsFromFigure(): void { + $html = '
'; + + $part = $this->mimeMessage->build( + null, + $html, + false, + [], + ); + + /** @var Horde_Mime_Part[] $subParts */ + $subParts = $part->getParts(); + $htmlBody = $subParts[1]->getContents(); + $this->assertStringContainsString('width="200"', $htmlBody); + $this->assertStringContainsString('height="100"', $htmlBody); + } + + public function testNormalizeImageDimensionsFromFigureWrappingLink(): void { + $html = '
'; + + $part = $this->mimeMessage->build( + null, + $html, + false, + [], + ); + + /** @var Horde_Mime_Part[] $subParts */ + $subParts = $part->getParts(); + $htmlBody = $subParts[1]->getContents(); + $this->assertStringContainsString('width="200"', $htmlBody); + $this->assertStringContainsString('height="100"', $htmlBody); + } + + public function testNormalizeImageDimensionsIgnoresPercentageWidth(): void { + $html = '

'; + + $part = $this->mimeMessage->build( + null, + $html, + false, + [], + ); + + /** @var Horde_Mime_Part[] $subParts */ + $subParts = $part->getParts(); + $htmlBody = $subParts[1]->getContents(); + $this->assertStringNotContainsString('width="', $htmlBody); + } + + public function testNormalizeImageAlignment(): 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: auto', $htmlBody); + $this->assertStringContainsString('text-align: center', $htmlBody); + } + + 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, + $html, + false, + [], + ); + + /** @var Horde_Mime_Part[] $subParts */ + $subParts = $part->getParts(); + $htmlBody = $subParts[1]->getContents(); + $this->assertStringNotContainsString('text-align', $htmlBody); + } + + public function testSanitizesInlineSvg(): void { + $svg = ''; + $html = '

'; + + $part = $this->mimeMessage->build( + null, + $html, + false, + [], + ); + + $this->assertEquals('multipart/related', $part->getType()); + + /** @var Horde_Mime_Part[] $relatedSubParts */ + $relatedSubParts = $part->getParts(); + $svgPart = $relatedSubParts[1]; + $this->assertEquals('image/svg+xml', $svgPart->getType()); + + $sanitized = base64_decode($svgPart->getContents()); + $this->assertStringNotContainsString('assertStringContainsString('setCharset('us-ascii'); diff --git a/tests/Unit/Service/SvgSanitizerTest.php b/tests/Unit/Service/SvgSanitizerTest.php new file mode 100644 index 0000000000..f4d576a9e9 --- /dev/null +++ b/tests/Unit/Service/SvgSanitizerTest.php @@ -0,0 +1,82 @@ +sanitizer = new SvgSanitizer(); + } + + public function testRemovesScript(): void { + $svg = ''; + + $result = $this->sanitizer->sanitize($svg); + + $this->assertStringNotContainsString('assertStringContainsString('sanitizer->sanitize($svg); + + $this->assertStringNotContainsString('onload', $result); + $this->assertStringContainsString('width="10"', $result); + } + + public function testRemovesExternalReference(): void { + $svg = '' + . ''; + + $result = $this->sanitizer->sanitize($svg); + + $this->assertStringNotContainsString('evil.example', $result); + $this->assertStringContainsString(''; + + $result = $this->sanitizer->sanitize($svg); + + $this->assertStringContainsString('#icon', $result); + } + + public function testRejectsDoctypeAndEntities(): void { + $svg = ']>' + . ''; + + $this->assertSame('', $this->sanitizer->sanitize($svg)); + } + + public function testReturnsEmptyForInvalidMarkup(): void { + $this->assertSame('', $this->sanitizer->sanitize('this is not <<< svg')); + $this->assertSame('', $this->sanitizer->sanitize('')); + } + + public function testKeepsSafeGraphics(): void { + $svg = '' + . ''; + + $result = $this->sanitizer->sanitize($svg); + + $this->assertStringContainsString('assertStringContainsString('fill="red"', $result); + } +}