Fix data-URI screenshots not opening on click (Chrome blocks data: URL navigation)
The click-to-zoom feature from the previous commit worked for cid:-referenced images but silently failed for images embedded directly as a data: URI — confirmed with a real Playwright click: Chrome refuses to navigate a tab (even a new one, even from a direct user click) to a data: URL, so <a href="data:...' target="_blank"> just does nothing. The cursor still showed zoom-in on hover since that's plain CSS, which is exactly the "лупа появляется, но не нажимается" symptom reported. Fix: imap.ts now extracts every data:image src out of an inbound email's HTML into a real attachment file (deduping identical images embedded more than once), the same way cid: images already were, and rewrites the HTML to point at that attachment's normal /api/attachments/... URL instead. That also shrinks messages.body_html (data URIs can be hundreds of KB sitting in a DB column) and gets data-URI images the same "no duplicate chip below" treatment cid: images already had. attachments.isInline is now its own real column (backfilled from the existing content_id-based cases) instead of being derived from content_id, since a data-URI-derived attachment is inline but was never cid-referenced. sanitizeEmailHtml's link-wrapping step now skips any residual data: src defensively (unwrapped-but-visible beats a link that looks clickable but isn't). Verified against the real deployment with actual browser clicks (Playwright): both a data-URI image and a cid: image now open their full-resolution attachment in a new tab; before this fix the data-URI one silently did nothing. Added a vitest.config.ts (needed for the new test file's @/ import aliases) and unit tests for the extraction/dedup logic. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GteWhnWKTmnXcsd5jx6H7u
This commit is contained in:
1 parent
1a09903326
commit
95522fdedd
10 files changed
+1228
-20
No files matched your search
@@ -0,0 +1,2 @@
|
|||||||
|
ALTER TABLE `attachments` ADD `is_inline` integer DEFAULT false NOT NULL;--> statement-breakpoint
|
||||||
|
UPDATE `attachments` SET `is_inline` = 1 WHERE `content_id` IS NOT NULL;
|
||||||
File diff suppressed because it is too large.
Load diff
@@ -64,6 +64,13 @@
|
|||||||
"when": 1787126651618,
|
"when": 1787126651618,
|
||||||
"tag": "0008_quick_ultragirl",
|
"tag": "0008_quick_ultragirl",
|
||||||
"breakpoints": true
|
"breakpoints": true
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"idx": 9,
|
||||||
|
"version": "6",
|
||||||
|
"when": 1787129904542,
|
||||||
|
"tag": "0009_little_natasha_romanoff",
|
||||||
|
"breakpoints": true
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
}
|
}
|
||||||
@@ -182,11 +182,16 @@ export const attachments = sqliteTable("attachments", {
|
|||||||
sizeBytes: integer("size_bytes").notNull(),
|
sizeBytes: integer("size_bytes").notNull(),
|
||||||
// Relative path under DATA_DIR/attachments/ — see src/lib/attachments/storage.ts.
|
// Relative path under DATA_DIR/attachments/ — see src/lib/attachments/storage.ts.
|
||||||
storageKey: text("storage_key").notNull(),
|
storageKey: text("storage_key").notNull(),
|
||||||
// The email's Content-ID header (no angle brackets) for an inline image —
|
// The email's Content-ID header (no angle brackets) for a cid:-referenced
|
||||||
// only set for attachments extracted from an HTML email body, used to
|
// inline image — used to rewrite `cid:` references in messages.bodyHtml
|
||||||
// rewrite `cid:` references in messages.bodyHtml to this attachment's
|
// to this attachment's serving URL. Null for regular attachments *and*
|
||||||
// serving URL. Null for regular (non-inline) attachments.
|
// for inline images that came from a data: URI instead (see isInline).
|
||||||
contentId: text("content_id"),
|
contentId: text("content_id"),
|
||||||
|
// True for any attachment whose file is already shown inside the
|
||||||
|
// message's bodyHtml (a cid: image, or one extracted from a data: URI —
|
||||||
|
// see lib/mail/imap.ts) — the UI uses this to skip showing it a second
|
||||||
|
// time as a separate chip below the already-rendered body.
|
||||||
|
isInline: integer("is_inline", { mode: "boolean" }).notNull().default(false),
|
||||||
createdAt: timestamps.createdAt,
|
createdAt: timestamps.createdAt,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,35 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { extractDataUriImages } from "./imap";
|
||||||
|
|
||||||
|
const TINY_PNG_B64 = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=";
|
||||||
|
|
||||||
|
describe("extractDataUriImages", () => {
|
||||||
|
it("extracts a data:image src into a real attachment and rewrites the HTML to reference it", async () => {
|
||||||
|
const html = `<p>see</p><img src="data:image/png;base64,${TINY_PNG_B64}" alt="shot">`;
|
||||||
|
const { html: out, attachments } = await extractDataUriImages(html);
|
||||||
|
|
||||||
|
expect(attachments).toHaveLength(1);
|
||||||
|
expect(attachments[0].mimeType).toBe("image/png");
|
||||||
|
expect(attachments[0].isInline).toBe(true);
|
||||||
|
expect(attachments[0].contentId).toBeNull();
|
||||||
|
expect(out).toContain(`/api/attachments/${attachments[0].id}`);
|
||||||
|
expect(out).not.toContain("base64");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("dedupes the same image embedded twice into a single attachment", async () => {
|
||||||
|
const src = `data:image/png;base64,${TINY_PNG_B64}`;
|
||||||
|
const html = `<img src="${src}"><p>middle</p><img src="${src}">`;
|
||||||
|
const { html: out, attachments } = await extractDataUriImages(html);
|
||||||
|
|
||||||
|
expect(attachments).toHaveLength(1);
|
||||||
|
const occurrences = out.split(`/api/attachments/${attachments[0].id}`).length - 1;
|
||||||
|
expect(occurrences).toBe(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves HTML with no data: images untouched", async () => {
|
||||||
|
const html = '<p>hi</p><img src="https://example.com/x.png">';
|
||||||
|
const { html: out, attachments } = await extractDataUriImages(html);
|
||||||
|
expect(attachments).toHaveLength(0);
|
||||||
|
expect(out).toBe(html);
|
||||||
|
});
|
||||||
|
});
|
||||||
+85
-12
@@ -56,21 +56,91 @@ function extractBody(parsed: ParsedMail): string {
|
|||||||
return "(пустое письмо)";
|
return "(пустое письмо)";
|
||||||
}
|
}
|
||||||
|
|
||||||
|
interface ExtractedAttachment {
|
||||||
|
id: string;
|
||||||
|
filename: string;
|
||||||
|
mimeType: string;
|
||||||
|
sizeBytes: number;
|
||||||
|
storageKey: string;
|
||||||
|
contentId: string | null;
|
||||||
|
isInline: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
const DATA_URI_IMG_RE = /src=(["'])(data:image\/[a-zA-Z0-9.+-]+;base64,[A-Za-z0-9+/=]+)\1/g;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Sanitizes the HTML part for display and rewrites `cid:` inline-image
|
* Pulls every `data:image/...;base64,...` embedded directly in an <img src>
|
||||||
* references to the URL the matching attachment will be served from —
|
* out into a real attachment file, replacing it in the HTML with that
|
||||||
* `attachmentIdByCid` must already reflect the IDs the attachments are about
|
* attachment's serving URL. Two things this fixes: (1) a data: URI can be
|
||||||
* to be inserted with (see handleRawMessage, which generates them upfront
|
* hundreds of KB, sitting in the messages table's body_html column instead
|
||||||
* for exactly this reason). Data-URI images need no rewriting; the browser
|
* of the filesystem; (2) browsers (Chrome specifically) block navigating a
|
||||||
* renders those natively once real HTML (not flattened text) reaches the UI.
|
* tab to a data: URL, so a <a href="data:..." target="_blank"> around the
|
||||||
|
* image — the whole point of the click-to-zoom feature — silently does
|
||||||
|
* nothing. A real /api/attachments/{id} URL has neither problem. Identical
|
||||||
|
* data URIs (the same image embedded twice) are deduped to one attachment.
|
||||||
*/
|
*/
|
||||||
function extractHtmlBody(parsed: ParsedMail, attachmentIdByCid: Map<string, string>): string | null {
|
export async function extractDataUriImages(html: string): Promise<{ html: string; attachments: ExtractedAttachment[] }> {
|
||||||
if (typeof parsed.html !== "string") return null;
|
const attachmentIdByDataUri = new Map<string, string>();
|
||||||
let html = parsed.html;
|
const attachments: ExtractedAttachment[] = [];
|
||||||
|
|
||||||
|
for (const match of html.matchAll(DATA_URI_IMG_RE)) {
|
||||||
|
const dataUri = match[2];
|
||||||
|
if (attachmentIdByDataUri.has(dataUri)) continue;
|
||||||
|
|
||||||
|
const [meta, base64Payload] = dataUri.slice("data:".length).split(";base64,");
|
||||||
|
if (!base64Payload) continue;
|
||||||
|
const buffer = Buffer.from(base64Payload, "base64");
|
||||||
|
if (buffer.length === 0) continue;
|
||||||
|
|
||||||
|
try {
|
||||||
|
const { storageKey, sizeBytes } = await saveAttachment(buffer);
|
||||||
|
const attachmentId = crypto.randomUUID();
|
||||||
|
const extension = meta.split("/")[1]?.split("+")[0] || "png";
|
||||||
|
attachments.push({
|
||||||
|
id: attachmentId,
|
||||||
|
filename: `image-${attachments.length + 1}.${extension}`,
|
||||||
|
mimeType: meta || "image/png",
|
||||||
|
sizeBytes,
|
||||||
|
storageKey,
|
||||||
|
contentId: null,
|
||||||
|
isInline: true,
|
||||||
|
});
|
||||||
|
attachmentIdByDataUri.set(dataUri, attachmentId);
|
||||||
|
} catch (err) {
|
||||||
|
console.error("[mail] failed to save data-URI image:", err);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
let rewritten = html;
|
||||||
|
for (const [dataUri, attachmentId] of attachmentIdByDataUri) {
|
||||||
|
rewritten = rewritten.split(dataUri).join(`/api/attachments/${attachmentId}`);
|
||||||
|
}
|
||||||
|
return { html: rewritten, attachments };
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sanitizes the HTML part for display and rewrites `cid:`/data-URI inline
|
||||||
|
* images to the URL the matching attachment will be served from —
|
||||||
|
* `attachmentIdByCid` must already reflect the IDs the cid attachments are
|
||||||
|
* about to be inserted with (see handleRawMessage, which generates them
|
||||||
|
* upfront for exactly this reason). Data-URI images are extracted into
|
||||||
|
* their own attachments here instead (see extractDataUriImages) — the
|
||||||
|
* caller must fold the returned attachments into the same insert.
|
||||||
|
*/
|
||||||
|
async function extractHtmlBody(
|
||||||
|
parsed: ParsedMail,
|
||||||
|
attachmentIdByCid: Map<string, string>,
|
||||||
|
): Promise<{ html: string | null; attachments: ExtractedAttachment[] }> {
|
||||||
|
if (typeof parsed.html !== "string") return { html: null, attachments: [] };
|
||||||
|
|
||||||
|
const { html: withDataUrisExtracted, attachments } = await extractDataUriImages(parsed.html);
|
||||||
|
|
||||||
|
let html = withDataUrisExtracted;
|
||||||
for (const [cid, attachmentId] of attachmentIdByCid) {
|
for (const [cid, attachmentId] of attachmentIdByCid) {
|
||||||
html = html.split(`cid:${cid}`).join(`/api/attachments/${attachmentId}`);
|
html = html.split(`cid:${cid}`).join(`/api/attachments/${attachmentId}`);
|
||||||
}
|
}
|
||||||
return sanitizeEmailHtml(html);
|
|
||||||
|
return { html: sanitizeEmailHtml(html), attachments };
|
||||||
}
|
}
|
||||||
|
|
||||||
async function handleRawMessage(source: Buffer): Promise<void> {
|
async function handleRawMessage(source: Buffer): Promise<void> {
|
||||||
@@ -101,21 +171,24 @@ async function handleRawMessage(source: Buffer): Promise<void> {
|
|||||||
sizeBytes,
|
sizeBytes,
|
||||||
storageKey,
|
storageKey,
|
||||||
contentId: part.cid ?? null,
|
contentId: part.cid ?? null,
|
||||||
|
isInline: Boolean(part.cid),
|
||||||
});
|
});
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
console.error("[mail] failed to save attachment:", err);
|
console.error("[mail] failed to save attachment:", err);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const { html: bodyHtml, attachments: dataUriAttachments } = await extractHtmlBody(parsed, attachmentIdByCid);
|
||||||
|
|
||||||
await recordEmailInboundMessage({
|
await recordEmailInboundMessage({
|
||||||
fromEmail: from.address,
|
fromEmail: from.address,
|
||||||
fromName: from.name || from.address,
|
fromName: from.name || from.address,
|
||||||
subject: parsed.subject ?? "",
|
subject: parsed.subject ?? "",
|
||||||
body: extractBody(parsed),
|
body: extractBody(parsed),
|
||||||
bodyHtml: extractHtmlBody(parsed, attachmentIdByCid),
|
bodyHtml,
|
||||||
messageId: parsed.messageId ?? null,
|
messageId: parsed.messageId ?? null,
|
||||||
referencedMessageIds: referencedIds,
|
referencedMessageIds: referencedIds,
|
||||||
attachments: attachmentInputs,
|
attachments: [...attachmentInputs, ...dataUriAttachments],
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -50,8 +50,17 @@ describe("sanitizeEmailHtml", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("wraps a bare image in a link to its own full-size src, opened in a new tab", () => {
|
it("wraps a bare image in a link to its own full-size src, opened in a new tab", () => {
|
||||||
|
// A non-data: src, since Chrome refuses to navigate a tab to a data:
|
||||||
|
// URL — by the time sanitizeEmailHtml runs, imap.ts has already
|
||||||
|
// extracted data:image srcs into real /api/attachments/... URLs.
|
||||||
|
const out = sanitizeEmailHtml('<img src="/api/attachments/abc-123" alt="shot">');
|
||||||
|
expect(out).toMatch(/<a href="\/api\/attachments\/abc-123"[^>]*target="_blank"[^>]*><img[^>]*><\/a>/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("leaves a residual data: URI image unwrapped rather than linking somewhere Chrome won't navigate", () => {
|
||||||
const out = sanitizeEmailHtml('<img src="data:image/png;base64,iVBORw0KGgo=" alt="shot">');
|
const out = sanitizeEmailHtml('<img src="data:image/png;base64,iVBORw0KGgo=" alt="shot">');
|
||||||
expect(out).toMatch(/<a href="data:image\/png;base64,iVBORw0KGgo="[^>]*target="_blank"[^>]*><img[^>]*><\/a>/);
|
expect(out).not.toContain("<a ");
|
||||||
|
expect(out).toContain('src="data:image/png;base64,iVBORw0KGgo="');
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does not double-wrap an image the sender already linked themselves", () => {
|
it("does not double-wrap an image the sender already linked themselves", () => {
|
||||||
|
|||||||
@@ -68,6 +68,14 @@ export function sanitizeEmailHtml(html: string): string {
|
|||||||
* Runs after sanitizeHtml, on already-sanitized output — the href is just
|
* Runs after sanitizeHtml, on already-sanitized output — the href is just
|
||||||
* the img's own already-scheme-validated src, so this can't reintroduce
|
* the img's own already-scheme-validated src, so this can't reintroduce
|
||||||
* anything sanitizeHtml would have stripped.
|
* anything sanitizeHtml would have stripped.
|
||||||
|
*
|
||||||
|
* data: URIs are skipped here — imap.ts extracts every data:image src into
|
||||||
|
* a real attachment (and a real /api/attachments/... URL) before this ever
|
||||||
|
* runs, specifically because Chrome silently refuses to navigate a tab to a
|
||||||
|
* data: URL, so a link built from one would look clickable but do nothing.
|
||||||
|
* Anything still starting with "data:" at this point is a residual case
|
||||||
|
* that extraction didn't catch — better left unwrapped (a plain image) than
|
||||||
|
* wrapped in a link that appears to work but doesn't.
|
||||||
*/
|
*/
|
||||||
function wrapBareImagesInLinks(html: string): string {
|
function wrapBareImagesInLinks(html: string): string {
|
||||||
const $ = cheerio.load(html, null, false);
|
const $ = cheerio.load(html, null, false);
|
||||||
@@ -75,7 +83,7 @@ function wrapBareImagesInLinks(html: string): string {
|
|||||||
const $img = $(el);
|
const $img = $(el);
|
||||||
if ($img.closest("a").length > 0) return;
|
if ($img.closest("a").length > 0) return;
|
||||||
const src = $img.attr("src");
|
const src = $img.attr("src");
|
||||||
if (!src) return;
|
if (!src || src.startsWith("data:")) return;
|
||||||
// Built via .attr(), not string-interpolated HTML — src is
|
// Built via .attr(), not string-interpolated HTML — src is
|
||||||
// attacker-influenced (an already scheme-validated but otherwise
|
// attacker-influenced (an already scheme-validated but otherwise
|
||||||
// arbitrary data:/http(s) URL), and interpolating it into an HTML
|
// arbitrary data:/http(s) URL), and interpolating it into an HTML
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ type AttachmentInput = {
|
|||||||
sizeBytes: number;
|
sizeBytes: number;
|
||||||
storageKey: string;
|
storageKey: string;
|
||||||
contentId?: string | null;
|
contentId?: string | null;
|
||||||
|
isInline?: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
function toTicketDTO(ticket: typeof tickets.$inferSelect, customerName: string, tags: TagDTO[]): TicketDTO {
|
function toTicketDTO(ticket: typeof tickets.$inferSelect, customerName: string, tags: TagDTO[]): TicketDTO {
|
||||||
@@ -109,7 +110,7 @@ async function getAttachmentsForMessages(messageIds: string[]): Promise<Map<stri
|
|||||||
});
|
});
|
||||||
for (const row of rows) {
|
for (const row of rows) {
|
||||||
const list = map.get(row.messageId) ?? [];
|
const list = map.get(row.messageId) ?? [];
|
||||||
list.push({ id: row.id, filename: row.filename, mimeType: row.mimeType, sizeBytes: row.sizeBytes, isInline: Boolean(row.contentId) });
|
list.push({ id: row.id, filename: row.filename, mimeType: row.mimeType, sizeBytes: row.sizeBytes, isInline: row.isInline });
|
||||||
map.set(row.messageId, list);
|
map.set(row.messageId, list);
|
||||||
}
|
}
|
||||||
return map;
|
return map;
|
||||||
@@ -155,7 +156,7 @@ async function appendMessage(params: {
|
|||||||
filename: a.filename,
|
filename: a.filename,
|
||||||
mimeType: a.mimeType,
|
mimeType: a.mimeType,
|
||||||
sizeBytes: a.sizeBytes,
|
sizeBytes: a.sizeBytes,
|
||||||
isInline: Boolean(a.contentId),
|
isInline: a.isInline,
|
||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,10 @@
|
|||||||
|
import { defineConfig } from "vitest/config";
|
||||||
|
import path from "node:path";
|
||||||
|
|
||||||
|
export default defineConfig({
|
||||||
|
resolve: {
|
||||||
|
alias: {
|
||||||
|
"@": path.resolve(__dirname, "./src"),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
Reference in new issue
Block a user