fix(image): keep data: URI images on markdown round-trip

[guillaume-flambard]

Fixes #9108.

Editing a markdown file that contains base64 images (data: URI) deleted those images from the file on the next save. The root cause sits in the image parse rules: both the block Image and ImageInline extensions exclude img[src^=data:] when the TipTap allowBase64 option is false, and nothing in this app turns it on. markdown-it parses the data: URI fine, but the img never becomes a document node, so the save writes the file without it. That exclusion is the upstream TipTap default, not a decision this app ever made, and there is nothing in the git history suggesting it was deliberate here.

The change enables allowBase64 on both extensions. The file content is the user's own and the editor's job is to preserve it, not to filter it. Rendering already goes through the ImageView node view, which handles the src as-is.

Regression test added in markdown.spec.js, covering both paths:

• a standalone data: URI image (block level, wrapped in a figure)
• an inline data: URI image inside a paragraph

Both round-trip through the editor byte-identical. Full unit suite: 1582/1582 across 50 files. Before the change the new test fails (the image is dropped), which is the reported data loss.

[max-nextcloud]

Hi @guillaume-flambard

Thanks for tackling this. The approach makes sense to me.

We will need to check if this has any security implications. data: uris may contain other mime types such as text/html or application/javascript.

I hope our CSP have us covered there - but we need to confirm that.

My understanding is that you are only trying to preserve the images in the file - not render them. Is that correct?

[guillaume-flambard]

Hi @max-nextcloud, thanks for looking at this.

On your second question first, because it changes the answer to the first one: no, preserving is not the whole intent. These images are meant to render, and the code for that is already there and predates this PR.

AttachmentResolver.isDirectUrl() already lists data: alongside http:// and https:// (src/services/AttachmentResolver.js:140). A data: src goes through the direct URL branch and comes back as previewUrl and fullUrl untouched. So the rendering path was built for this. The only missing piece was the parse rule, which dropped the node before it ever reached the resolver. That is why the image disappeared on save: it was never in the document to begin with.

On the security side, three things, and one caveat I would rather raise myself.

The CSP does cover it. The server default in lib/public/AppFramework/Http/ContentSecurityPolicy.php is $allowedImageDomains = ["'self'", "data:", "blob:"], so data: is already an allowed image source. It is not an allowed script, object or frame source: $allowedScriptDomains is ["'self'"], $inlineScriptAllowed is false, and both $allowedObjectDomains and $allowedFrameDomains default to empty. A data:text/html or data:application/javascript payload has nowhere to load from under that policy.

The src also only ever reaches one sink. In ImageView.vue it is bound to <img :src> in both branches (lines 32 and 56). The <a> wrappers around them have no href, only click handlers. Nothing puts the src into v-html, an iframe or a navigation target. An <img> with a non-image mime type fails to decode, @load never fires, and v-show="loaded" keeps it hidden. You get a broken image, not an execution path.

The caveat: TipTap's allowBase64 is all or nothing. Upstream it flips the parse selector from img[src]:not([src^="data:"]) to img[src], with no mime filtering, so the parse rule now accepts data: of any type. I think that is fine given the two points above, and given the file content is the user's own. But if you want defense in depth, I am happy to add a parseHTML override on both extensions that only admits data:image/..., so a malformed mime type never becomes a node in the first place. That is a small change and it keeps the fix for #9108 intact.

Tell me which way you prefer and I will push it.

[guillaume-flambard]

Rather than leave you with a question, I pushed the stricter option so you can see both and pick.

Both extensions now narrow the permissive branch instead of accepting any data: URI:

parseHTML() {
	if (!this.options.allowBase64) {
		return [{ tag: 'figure img[src]:not([src^="data:"])' }]
	}
	return [
		{ tag: 'figure img[src]:not([src^="data:"])' },
		{ tag: 'figure img[src^="data:image/"]' },
	]
}

ImageInline gets the same without the figure scope. A data:text/html src now never becomes a node, so the parse rule no longer leans on the CSP and the sink analysis alone.

Writing the test for it turned up something worth knowing: the payload is not lost either. A non image data: URI round-trips as escaped literal text, \![x](data:text/html;base64,...), so the user's characters survive exactly as #9108 asks. The stricter rule only moves where the boundary sits between an image and text, it does not reintroduce data loss. The test asserts that string, which is also what distinguishes this behaviour from the previous commit.

Full suite is green locally, 1583 tests, plus eslint and tsc on the changed files.

If you prefer to keep it minimal and rely on the CSP, drop the last commit and the two line version stands on its own.