diff --git a/backend/app/indexer/sanitize.py b/backend/app/indexer/sanitize.py index 189bb19..e7f7aff 100644 --- a/backend/app/indexer/sanitize.py +++ b/backend/app/indexer/sanitize.py @@ -17,7 +17,12 @@ import nh3 # Bump when the sanitization policy changes; body_parts.sanitizer_v records it so a # render re-sanitizes stale content from the raw blob (SPEC §10.4). -SANITIZER_VERSION = 4 +# v5: this server's own inline-image URLs quoted in a message load from here. +SANITIZER_VERSION = 5 + +# This server's inline-image URL, however it ended up written: "/api/…", +# "https://host/api/…", or mangled to "http:///api/…" by an earlier copy. +INTERNAL_CID = re.compile(r"/api/messages/([0-9a-fA-F-]{36})/cid/([^\"'?#\s<>]+)") _ALLOWED_TAGS: set[str] = { # structure @@ -202,6 +207,13 @@ def sanitize_html( if lowered.startswith("cid:"): content_id = value.strip()[4:] return f"/api/messages/{message_id}/cid/{content_id}" + # This server's own inline-image URL, quoted from another message + # (by an earlier version, or another of the user's own mails) — + # possibly mangled to "http:///api/…", which a browser takes for a + # host named "api": load it from here, never look that host up. + internal = INTERNAL_CID.search(value) + if internal is not None and not lowered.startswith("data:"): + return f"/api/messages/{internal.group(1)}/cid/{internal.group(2)}" if lowered.startswith(("http:", "https:")): state["has_remote"] = True return value if allow_remote_images else None # block by default (FR-23) diff --git a/backend/app/outbox/draft_sync.py b/backend/app/outbox/draft_sync.py index 8400891..36c2306 100644 --- a/backend/app/outbox/draft_sync.py +++ b/backend/app/outbox/draft_sync.py @@ -17,6 +17,7 @@ from sqlalchemy import select from app.db import session_scope from app.models import Account, Draft, Mailbox, Message +from app.outbox import inline_images from app.outbox.attachments import load_attachments from app.outbox.mime_build import build_mime from app.sync.provider_factory import build_provider @@ -68,7 +69,15 @@ async def push_draft_to_imap_job(ctx: dict[str, Any], draft_id: str) -> None: else None ) attachments = await load_attachments(session, user_id, draft) - _, sent_raw, _, _ = build_mime(account, draft, reply_to, attachments) + html, inline = await inline_images.embed( + session, + user_id, + str(draft.payload.get("html", "") or ""), + account.email.rsplit("@", 1)[-1], + ) + _, sent_raw, _, _ = build_mime( + account, draft, reply_to, attachments, html=html, inline=inline + ) drafts_mb = await session.scalar( select(Mailbox).where(Mailbox.account_id == account.id, Mailbox.special_use == "drafts") ) diff --git a/backend/app/outbox/inline_images.py b/backend/app/outbox/inline_images.py new file mode 100644 index 0000000..56504ae --- /dev/null +++ b/backend/app/outbox/inline_images.py @@ -0,0 +1,103 @@ +"""Inline images in an outgoing message's HTML. + +A reply quotes the original as the reading pane shows it, so an image embedded +in the original (``cid:`` — a signature logo, a pasted screenshot) appears in +the quote as this server's own URL, ``/api/messages/{id}/cid/{content-id}``. +Sent as is, that URL means nothing to the recipient (and to anyone else, +including ourselves once the original is gone). Before sending, each such image +is put back into the message as an inline part with a new Content-ID — as any +mail program does — and one that can no longer be had is left out. +""" + +from __future__ import annotations + +import asyncio +import re +import uuid +from dataclasses import dataclass +from email.utils import make_msgid +from urllib.parse import unquote + +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.indexer import attachments as att_store +from app.indexer.sanitize import INTERNAL_CID +from app.models import Attachment, Message + +_IMG = re.compile(r"]*?\bsrc\s*=\s*([\"'])(.*?)\1[^>]*>", re.IGNORECASE | re.DOTALL) + + +@dataclass(frozen=True) +class InlinePart: + cid: str # without angle brackets + mime_type: str + filename: str + data: bytes + + +def internal_cid(src: str) -> tuple[uuid.UUID, str] | None: + """The (message id, content id) an image URL of this server points at.""" + value = src.strip() + if value.lower().startswith(("cid:", "data:")): + return None + m = INTERNAL_CID.search(value) + if m is None: + return None + try: + return uuid.UUID(m.group(1)), unquote(m.group(2)) + except ValueError: + return None + + +async def embed( + session: AsyncSession, user_id: uuid.UUID, html: str, domain: str +) -> tuple[str, list[InlinePart]]: + """``html`` with this server's image URLs turned into ``cid:`` references, + and the parts to send them in.""" + parts: dict[tuple[uuid.UUID, str], InlinePart | None] = {} + + for match in _IMG.finditer(html): + target = internal_cid(match.group(2)) + if target is None or target in parts: + continue + message_id, content_id = target + found = ( + await session.execute( + select(Attachment, Message.blob_sha256) + .join(Message, Message.id == Attachment.message_id) + .where( + Attachment.message_id == message_id, + Attachment.content_id == content_id, + Message.user_id == user_id, + ) + .limit(1) + ) + ).first() + part: InlinePart | None = None + if found is not None: + att, message_sha = found + try: + data = await asyncio.to_thread( + att_store.load, user_id, message_sha, att.blob_sha256, att.part_index + ) + cid = make_msgid(domain=domain)[1:-1] + part = InlinePart(cid, att.mime_type, att.filename, data) + except FileNotFoundError: + part = None + parts[target] = part + + def replace(match: re.Match[str]) -> str: + target = internal_cid(match.group(2)) + if target is None: + return match.group(0) + part = parts.get(target) + if part is None: + return "" # gone: no broken image for the recipient + quote = match.group(1) + return match.group(0).replace( + f"src={quote}{match.group(2)}{quote}", f"src={quote}cid:{part.cid}{quote}", 1 + ) + + out = _IMG.sub(replace, html) if parts else html + return out, [p for p in parts.values() if p is not None] diff --git a/backend/app/outbox/mime_build.py b/backend/app/outbox/mime_build.py index 1684c64..9fb188f 100644 --- a/backend/app/outbox/mime_build.py +++ b/backend/app/outbox/mime_build.py @@ -14,6 +14,7 @@ from email.utils import format_datetime, make_msgid import nh3 from app.models import Account, Draft, Message +from app.outbox.inline_images import InlinePart def _html_to_text(html: str) -> str: @@ -49,6 +50,9 @@ def build_mime( draft: Draft, reply_to: Message | None, attachments: list[tuple[str, str, bytes]] | None = None, + *, + html: str | None = None, + inline: list[InlinePart] | None = None, ) -> tuple[bytes, bytes, str, list[str]]: """Return ``(transmit_raw, sent_raw, envelope_from, envelope_to)`` for ``draft``. @@ -59,13 +63,17 @@ def build_mime( ``attachments`` is a list of ``(filename, mime_type, data)`` uploaded via the compose Attach action; each is added as a file attachment part. + + ``html`` replaces the draft's HTML when given (images already turned into + ``cid:`` references by app/outbox/inline_images.py), and ``inline`` are the + image parts those references name, sent alongside the HTML. """ payload = draft.payload to = _addr_list(payload.get("to")) cc = _addr_list(payload.get("cc")) bcc = _addr_list(payload.get("bcc")) subject = str(payload.get("subject", "")) - html = str(payload.get("html", "") or "") + html = html if html is not None else str(payload.get("html", "") or "") text = str(payload.get("text", "") or "") msg = EmailMessage() @@ -91,6 +99,19 @@ def build_mime( if html: msg.set_content(text or _html_to_text(html)) msg.add_alternative(html, subtype="html") + if inline: + html_part = next(p for p in msg.walk() if p.get_content_type() == "text/html") + assert isinstance(html_part, EmailMessage) + for part in inline: + maintype, _, subtype = part.mime_type.partition("/") + html_part.add_related( + part.data, + maintype=maintype or "image", + subtype=subtype or "png", + cid=f"<{part.cid}>", + filename=part.filename, + disposition="inline", + ) else: msg.set_content(text) diff --git a/backend/tests/outbox/test_inline_images.py b/backend/tests/outbox/test_inline_images.py new file mode 100644 index 0000000..1c58a9c --- /dev/null +++ b/backend/tests/outbox/test_inline_images.py @@ -0,0 +1,133 @@ +"""Images quoted from another message go out inside the message, not as links +to this server; such links already sent load from here when shown.""" + +from __future__ import annotations + +import email +import io +import shutil +import uuid +from collections.abc import AsyncIterator +from datetime import UTC, datetime +from email import policy +from email.message import EmailMessage +from pathlib import Path +from types import SimpleNamespace + +import pytest_asyncio +from PIL import Image +from sqlalchemy import delete +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine +from sqlalchemy.pool import NullPool + +from app.config import get_settings +from app.identity.passwords import hash_password +from app.indexer.indexer import persist_message +from app.indexer.sanitize import sanitize_html +from app.models import Account, Mailbox, User +from app.outbox import inline_images +from app.outbox.mime_build import build_mime +from app.providers.base import RemoteMessage + + +def _png() -> bytes: + out = io.BytesIO() + Image.new("RGB", (40, 20), (10, 120, 200)).save(out, "PNG") + return out.getvalue() + + +LOGO = _png() + + +@pytest_asyncio.fixture +async def ctx() -> AsyncIterator[tuple[AsyncSession, uuid.UUID, uuid.UUID]]: + engine = create_async_engine(get_settings().database_url, poolclass=NullPool) + db = async_sessionmaker(engine, expire_on_commit=False)() + user = User(handle=f"inl{uuid.uuid4().hex[:8]}@test.local", password_hash=hash_password("pw")) + db.add(user) + await db.flush() + account = Account( + user_id=user.id, + email="me@example.com", + imap_config={}, + smtp_config={}, + secret_ct=b"", + discovery_src="manual", + ) + db.add(account) + await db.flush() + inbox = Mailbox(account_id=account.id, user_id=user.id, path="INBOX", special_use="inbox") + db.add(inbox) + await db.commit() + m = EmailMessage() + m["Subject"] = "Signed" + m["From"] = "Ben " + m["To"] = "me@example.com" + m["Message-ID"] = "" + m.set_content("hello") + m.add_alternative('

hello

', subtype="html") + m.get_payload()[1].add_related( + LOGO, "image", "png", cid="", filename="logo.png", disposition="inline" + ) + remote = RemoteMessage( + uid=1, raw=m.as_bytes(), flags=frozenset(), internal_date=datetime.now(UTC) + ) + result = await persist_message( + db, + user_id=user.id, + account_id=account.id, + mailbox_id=inbox.id, + mailbox_special_use="inbox", + remote=remote, + ) + await db.commit() + assert result is not None + try: + yield db, user.id, result.message.id + finally: + await db.execute(delete(User).where(User.id == user.id)) + await db.commit() + await db.close() + await engine.dispose() + shutil.rmtree(Path(get_settings().blob_root) / str(user.id), ignore_errors=True) + + +async def test_quoted_images_travel_inside_the_reply(ctx) -> None: # type: ignore[no-untyped-def] + db, user_id, original = ctx + gone = uuid.uuid4() + html = ( + "

Thanks!

" + f'logo' + f'' # same image, mangled + f'' # its message is gone + '
' + ) + out, parts = await inline_images.embed(db, user_id, html, "example.com") + assert len(parts) == 1 and parts[0].data == LOGO + assert out.count(f'src="cid:{parts[0].cid}"') == 2 + assert "/api/messages/" not in out and 'alt="logo"' in out + assert "https://example.com/banner.png" in out # other images untouched + + account = SimpleNamespace(email="me@example.com", display_name="Me") + draft = SimpleNamespace( + payload={"to": ["ben@example.org"], "subject": "Re: Signed", "html": html} + ) + raw, _, _, _ = build_mime(account, draft, None, [], html=out, inline=parts) # type: ignore[arg-type] + sent = email.message_from_bytes(raw, policy=policy.default) + related = [p for p in sent.walk() if p.get_content_type() == "multipart/related"] + assert related, "the HTML and its image belong together" + image = next(p for p in sent.walk() if p.get_content_type() == "image/png") + assert image["Content-ID"] == f"<{parts[0].cid}>" + assert image.get_payload(decode=True) == LOGO + + +def test_links_already_sent_load_from_here() -> None: + mid = uuid.uuid4() + for src in ( + f"http:///api/messages/{mid}/cid/logo@x", + f"https://mail.example.com/api/messages/{mid}/cid/logo@x", + f"/api/messages/{mid}/cid/logo@x", + ): + result = sanitize_html(f'', uuid.uuid4(), allow_remote_images=True) + assert f'src="/api/messages/{mid}/cid/logo@x"' in result.html, src + assert not result.has_remote_images diff --git a/frontend/src/components/thread/MessageBody.tsx b/frontend/src/components/thread/MessageBody.tsx index d59f228..18577d0 100644 --- a/frontend/src/components/thread/MessageBody.tsx +++ b/frontend/src/components/thread/MessageBody.tsx @@ -118,9 +118,13 @@ export function MessageBody({ // else: relative / cid: / javascript: / data: / tel: — blocked, no navigation. }, []); - const onLoad = () => { - const doc = frameRef.current?.contentDocument; - if (!doc) return; + // Set up the frame (sizing, link clicks, the quote toggle) as soon as its + // document is parsed — not at its load event, which waits for every image, + // remote ones included: until then the frame would stay at its first height. + const preparedRef = useRef(null); + const prepare = useCallback((doc: Document) => { + if (preparedRef.current === doc) return; + preparedRef.current = doc; // The frame is auto-sized to its content, so it must never scroll itself — the // thread panel is the scroll container. Hiding the document's own vertical // overflow removes any phantom scrollbar left by sub-pixel/line-box rounding @@ -134,14 +138,42 @@ export function MessageBody({ doc.addEventListener("click", handleFrameClick); // Re-measure as images finish loading (they change the content height). doc.querySelectorAll("img").forEach((img) => { - if (!img.complete) img.addEventListener("load", resize, { once: true }); + if (!img.complete) { + img.addEventListener("load", resize, { once: true }); + img.addEventListener("error", resize, { once: true }); + } }); observerRef.current?.disconnect(); const ro = new ResizeObserver(resize); if (doc.body) ro.observe(doc.body); observerRef.current = ro; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + const onLoad = () => { + const doc = frameRef.current?.contentDocument; + if (!doc) return; + prepare(doc); + resize(); // everything has loaded: one last measure }; + // Watch for the message's document to be parsed (the frame starts on an + // empty about:blank document, which does not count). + useEffect(() => { + if (data === undefined) return; + let frame = 0; + const check = () => { + const doc = frameRef.current?.contentDocument; + if (doc && doc.location.href === "about:srcdoc" && doc.readyState !== "loading" && doc.body) { + prepare(doc); + return; + } + frame = requestAnimationFrame(check); + }; + frame = requestAnimationFrame(check); + return () => cancelAnimationFrame(frame); + }, [data, prepare]); + useEffect(() => () => observerRef.current?.disconnect(), []); const loadImages = async (persist: boolean) => {