From 6fdc0d3273ffad6d62f2f7ed91468efd5bc23ed5 Mon Sep 17 00:00:00 2001 From: ssavutu Date: Fri, 31 Jul 2026 23:27:02 -0400 Subject: [PATCH] feat(media): make image insertion work end to end in the article editor Inserting an image into an article body was only partly wired up. This makes the whole path work and stops the editor offering controls the published page cannot honour. Uploading is now an editor capability. POST /v1/media was admin-only while every user after the first defaults to editor, so an editor dragging an image in got a silent 403: the failed attachment stayed on its local blob URL, looked fine, and vanished on reload. Delete and reindex stay admin-only. Pasting is handled for all three of its shapes. A screenshot off the clipboard arrives with a File and takes the upload path. Rich text copied from another site arrives as a bare remote URL and was previously ignored, leaving the article hotlinking a third party; it is now sideloaded through a new POST /v1/media/fetch. An image copied from another article in this CMS is recognised as already ours and left alone rather than duplicated. Because that endpoint makes the server fetch a caller-supplied URL, the SSRF guard runs in the dialer's Control hook rather than on the URL string, so it inspects the resolved IP on every redirect hop and blocks hostnames resolving to internal space, DNS rebinding, and redirects to the cloud metadata endpoint. Size is capped by reading one byte past the limit rather than trusting Content-Length. Images can now be inserted from the media library via a new toolbar button, backed by the /v1/media/gallery endpoint that already existed and had no callers. The library's alt text comes with the image, so an asset described once is described everywhere it is used, and the picker flags assets that have none. Article bodies now store plain semantic markup -- figure/img/figcaption using the same WordPress class names as the migrated corpus -- instead of Trix's data-trix-attachment JSON, which means nothing outside a Trix editor. The load and save conversions live together in trixImageHtml.ts as explicit inverses. alt is always emitted, since a missing alt attribute is an accessibility failure while alt="" is a valid signal. Failed uploads and imports now remove the attachment and say why, instead of leaving a preview that cannot survive a reload. Finally, image resizing and alignment are removed from the editor. The public site sizes article images entirely in CSS (#article figure img { width: 100% }), which overrides anything an author sets, so both were gestures that appeared to work and changed nothing on the page. Worse, width/height are presentational hints that CSS outranks, so emitting them would have left the width overridden while the height still applied, stretching every image. Drag-to-reorder is kept because it does survive. Alignment already present on migrated content is still preserved through a save; it just can no longer be set here. Co-Authored-By: Claude Opus 5 --- frontend/src/components/MediaPicker.tsx | 243 ++++++++++ frontend/src/components/TrixEditor.css | 71 +-- frontend/src/components/TrixEditor.tsx | 469 ++++++++++--------- frontend/src/components/trixImageHtml.ts | 255 ++++++++++ frontend/src/trix.d.ts | 22 +- server/docs/docs.go | 76 +++ server/docs/swagger.json | 76 +++ server/docs/swagger.yaml | 48 ++ server/internal/handlers/media.go | 322 +++++++++++-- server/internal/handlers/media_fetch_test.go | 112 +++++ server/internal/models/api_responses.go | 6 + server/internal/routes/routes.go | 6 +- server/internal/routes/routes_test.go | 1 + 13 files changed, 1408 insertions(+), 299 deletions(-) create mode 100644 frontend/src/components/MediaPicker.tsx create mode 100644 frontend/src/components/trixImageHtml.ts create mode 100644 server/internal/handlers/media_fetch_test.go diff --git a/frontend/src/components/MediaPicker.tsx b/frontend/src/components/MediaPicker.tsx new file mode 100644 index 0000000..7e28763 --- /dev/null +++ b/frontend/src/components/MediaPicker.tsx @@ -0,0 +1,243 @@ +import { useCallback, useEffect, useRef, useState } from "react" +import { ImageOff, Search, Upload, X } from "lucide-react" +import { useApiFetch } from "../hooks/useApiFetch" + +export type MediaPickerItem = { + id: number + url: string + file_name: string + mime_type?: string + width?: number + height?: number + alt_text?: string +} + +type GalleryResponse = { + media?: MediaPickerItem[] +} + +type MediaPickerProps = { + onSelect: (item: MediaPickerItem) => void + onClose: () => void + title?: string +} + +const PAGE_SIZE = 60 + +async function errorMessage(response: Response, fallback: string) { + try { + const body = (await response.json()) as { error?: string } + return body.error?.trim() || fallback + } catch { + return fallback + } +} + +/** + * Modal for choosing an image from the media library, plus an upload shortcut + * for the common case where the image is not in the library yet. + * + * Backed by /v1/media/gallery, which returns the trimmed picker shape -- + * notably including alt_text, so a chosen image arrives with its description + * already written rather than needing it retyped per article. + */ +function MediaPicker({ onSelect, onClose, title = "Insert image" }: MediaPickerProps) { + const apiFetch = useApiFetch() + const fileInputRef = useRef(null) + + const [items, setItems] = useState([]) + const [searchInput, setSearchInput] = useState("") + const [search, setSearch] = useState("") + const [isLoading, setIsLoading] = useState(true) + const [isUploading, setIsUploading] = useState(false) + const [error, setError] = useState(null) + + useEffect(() => { + const timer = setTimeout(() => setSearch(searchInput.trim()), 300) + return () => clearTimeout(timer) + }, [searchInput]) + + useEffect(() => { + const onKeyDown = (e: KeyboardEvent) => { + if (e.key === "Escape") onClose() + } + window.addEventListener("keydown", onKeyDown) + return () => window.removeEventListener("keydown", onKeyDown) + }, [onClose]) + + useEffect(() => { + const controller = new AbortController() + + const load = async () => { + setIsLoading(true) + setError(null) + try { + const params = new URLSearchParams({ limit: String(PAGE_SIZE) }) + if (search) params.set("search", search) + const response = await apiFetch(`/v1/media/gallery?${params.toString()}`, { + signal: controller.signal, + }) + if (!response.ok) throw new Error(await errorMessage(response, `Request failed (${response.status})`)) + const payload = (await response.json()) as GalleryResponse + if (controller.signal.aborted) return + setItems(payload.media ?? []) + } catch (err) { + if (controller.signal.aborted) return + setError(err instanceof Error ? err.message : "Unable to load media.") + } finally { + if (!controller.signal.aborted) setIsLoading(false) + } + } + + void load() + return () => controller.abort() + }, [apiFetch, search]) + + const handleUpload = useCallback( + async (files: FileList | null) => { + const file = files?.[0] + if (!file) return + setError(null) + setIsUploading(true) + try { + const body = new FormData() + body.append("file", file) + const response = await apiFetch("/v1/media", { method: "POST", body }) + if (!response.ok) { + setError(await errorMessage(response, `Upload failed (${response.status})`)) + return + } + const created = (await response.json()) as { + id: number + path: string + url: string + content_type?: string + width?: number + height?: number + } + // Straight into the document: the author picked a file in order to use + // it, so making them find it in the grid afterwards is a wasted step. + onSelect({ + id: created.id, + url: created.url, + file_name: created.path.split("/").pop() ?? created.path, + mime_type: created.content_type, + width: created.width, + height: created.height, + }) + } catch (err) { + setError(err instanceof Error ? err.message : "Upload failed.") + } finally { + setIsUploading(false) + if (fileInputRef.current) fileInputRef.current.value = "" + } + }, + [apiFetch, onSelect], + ) + + return ( +
+
e.stopPropagation()} + role="dialog" + > +
+

{title}

+ +
+ +
+
+ + setSearchInput(e.target.value)} + placeholder="Search by file name, alt text, or caption..." + type="search" + value={searchInput} + /> +
+ void handleUpload(e.target.files)} + ref={fileInputRef} + type="file" + /> + +
+ + {error && ( +
+ {error} +
+ )} + +
+ {isLoading ? ( +

Loading media...

+ ) : items.length === 0 ? ( +
+ +

{search ? `No results for "${search}"` : "No media items yet."}

+
+ ) : ( +
+ {items.map((item) => ( + + ))} +
+ )} +
+
+
+ ) +} + +export default MediaPicker diff --git a/frontend/src/components/TrixEditor.css b/frontend/src/components/TrixEditor.css index b9e8118..362d674 100644 --- a/frontend/src/components/TrixEditor.css +++ b/frontend/src/components/TrixEditor.css @@ -293,9 +293,10 @@ trix-editor.trix-content .attachment.attachment--preview.attachment--dragging { cursor: grabbing; } -/* Per-image alignment, set during a drop. Overrides the default centering - from .attachment--preview above. Both the inline style and the class are - applied at commit time for redundancy through Trix's HTML serialization. */ +/* Per-image alignment. No longer settable from the editor -- the public site + sizes article images at width:100%, which leaves nothing for a float to do -- + but alignment already present on migrated WordPress content is preserved + through a save, so it is still reflected here to match what will publish. */ trix-editor.trix-content .attachment--preview.attachment--align-left { text-align: left; } @@ -306,35 +307,8 @@ trix-editor.trix-content .attachment--preview.attachment--align-right { text-align: right; } -/* Resize overlay rendered by JS into .trix-editor-wrapper, positioned over - the currently selected image. The overlay itself is click-through so Trix's - own drag-to-reorder still works; only the corner squares grab pointer events. */ -.trix-resize-overlay { - position: absolute; - pointer-events: none; - z-index: 5; -} - -.trix-resize-handle { - position: absolute; - width: 12px; - height: 12px; - background: hsl(var(--primary, 243 75% 59%)); - border: 2px solid hsl(var(--background, 0 0% 100%)); - border-radius: 50%; - box-shadow: 0 1px 3px rgba(0, 0, 0, 0.3); - pointer-events: auto; -} - -.trix-resize-handle--nw { top: -6px; left: -6px; cursor: nwse-resize; } -.trix-resize-handle--ne { top: -6px; right: -6px; cursor: nesw-resize; } -.trix-resize-handle--sw { bottom: -6px; left: -6px; cursor: nesw-resize; } -.trix-resize-handle--se { bottom: -6px; right: -6px; cursor: nwse-resize; } - -/* Drop indicator shown only during an active drag-to-rearrange. The width and - horizontal position shift to hint at the alignment that will be applied on - drop: short bar on the left = left-align, centered short bar = center, - short bar on the right = right-align. */ +/* Drop indicator shown only during an active drag-to-rearrange, marking the + line the image will land on. */ .trix-drop-indicator { position: absolute; height: 3px; @@ -401,3 +375,36 @@ trix-editor.trix-content a:hover::after { z-index: 10; box-shadow: 0 4px 12px rgba(0, 0, 0, 0.15); } + +/* Inline failure/warning banner for image insertion (failed upload, failed + paste import, image inserted without alt text). Sits below the editor rather + than in a toast so it stays visible while the author fixes the problem. */ +.trix-editor-notice { + display: flex; + align-items: flex-start; + justify-content: space-between; + gap: 12px; + margin-top: 8px; + padding: 8px 12px; + border: 1px solid hsl(var(--border, 220 13% 91%)); + border-radius: calc(var(--radius, 0.5rem) - 2px); + background: hsl(var(--muted, 220 14% 96%)); + color: hsl(var(--foreground, 224 71% 4%)); + font-size: 13px; + line-height: 1.4; +} + +.trix-editor-notice button { + flex-shrink: 0; + border: 0; + background: transparent; + color: hsl(var(--muted-foreground, 220 9% 46%)); + font-size: 18px; + line-height: 1; + cursor: pointer; + padding: 0 2px; +} + +.trix-editor-notice button:hover { + color: hsl(var(--foreground, 224 71% 4%)); +} diff --git a/frontend/src/components/TrixEditor.tsx b/frontend/src/components/TrixEditor.tsx index 2d76419..4bc9bc2 100644 --- a/frontend/src/components/TrixEditor.tsx +++ b/frontend/src/components/TrixEditor.tsx @@ -1,8 +1,16 @@ -import { useEffect, useId, useRef } from "react" +import { useCallback, useEffect, useId, useRef, useState } from "react" import "trix" import "trix/dist/trix.css" import "./TrixEditor.css" import { apiBaseUrl } from "../auth/urls" +import MediaPicker, { type MediaPickerItem } from "./MediaPicker" +import { + ALIGNMENTS, + TRIX_ALIGN_CLASS, + articleHtmlToTrix, + contentTypeForUrl, + trixHtmlToArticle, +} from "./trixImageHtml" // Show the filename in the auto-generated caption under attachments, but hide // the file size this matches the upstream Trix demo's defaults and gives users an @@ -12,70 +20,17 @@ if (typeof window !== "undefined" && window.Trix) { window.Trix.config.attachments.preview.caption.size = false } -const IMAGE_CONTENT_TYPES: Record = { - jpg: "image/jpeg", - jpeg: "image/jpeg", - png: "image/png", - gif: "image/gif", - webp: "image/webp", - avif: "image/avif", - svg: "image/svg+xml", -} - -const contentTypeForUrl = (url: string): string => { - const ext = url.split(/[?#]/)[0].split(".").pop()?.toLowerCase() ?? "" - return IMAGE_CONTENT_TYPES[ext] ?? "image/jpeg" -} - -// Trix only restores an image's caption when it's carried in the attachment's -// data attributes. Article HTML imported from WordPress instead puts the -// caption in a plain caption element (block editor:
; -// classic editor:
…<* class="wp-caption-text">), which -// Trix's parser drops onto the next line as body text — the caption "isn't -// picked up by the editor". Rewrite those into Trix's native attachment format -// so loadHTML keeps the caption bound to the image. Containers already -// round-tripped through Trix (they carry data-trix-attachment) are skipped, so -// this is idempotent. -const restoreFigureCaptions = (html: string): string => { - if (typeof window === "undefined" || !window.DOMParser) return html - if (!html.includes(" would silently drop - // the rest, so leave those untouched. - if (container.querySelectorAll("img").length !== 1) continue - if (container.querySelector("figure, .wp-caption")) continue - - const img = container.querySelector("img") - const src = img?.getAttribute("src")?.trim() - const caption = container.querySelector("figcaption, .wp-caption-text")?.textContent?.trim() - if (!img || !src || !caption) continue - - const basename = src.split("/").pop()?.split(/[?#]/)[0] - const replacement = doc.createElement("figure") - replacement.setAttribute("data-trix-attachment", JSON.stringify({ - contentType: contentTypeForUrl(src), - url: src, - filename: img.getAttribute("alt")?.trim() || basename || "image", - // Force inline preview: Trix's previewablePattern excludes svg/avif and - // any extension-less CDN URL, which would otherwise render as a file stub. - previewable: true, - })) - replacement.setAttribute("data-trix-attributes", JSON.stringify({ presentation: "gallery", caption })) - const newImg = doc.createElement("img") - newImg.setAttribute("src", src) - replacement.appendChild(newImg) - container.replaceWith(replacement) - changed = true +// Hosts we may embed directly. A pasted image already served from our own media +// infrastructure needs no sideload — it is the same file we would be copying. +const isOwnMediaUrl = (url: string): boolean => { + try { + const resolved = new URL(url, window.location.href) + if (resolved.origin === window.location.origin) return true + const apiOrigin = new URL(apiBaseUrl(), window.location.href).origin + return resolved.origin === apiOrigin + } catch { + return false } - - return changed ? doc.body.innerHTML : html } type TrixEditorProps = { @@ -90,12 +45,18 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // Tracks the last HTML we emitted so we don't call loadHTML on our own onChange updates, // which would reset the cursor mid-edit. const lastEmittedRef = useRef("") + // Caret position captured before the picker steals focus, so the image lands + // where the author was typing rather than at the top of the document. + const savedRangeRef = useRef<[number, number] | null>(null) + + const [pickerOpen, setPickerOpen] = useState(false) + const [notice, setNotice] = useState(null) useEffect(() => { const editor = editorRef.current if (!editor) return if (value !== lastEmittedRef.current) { - editor.editor.loadHTML(restoreFigureCaptions(value)) + editor.editor.loadHTML(articleHtmlToTrix(value)) lastEmittedRef.current = value } }, [value]) @@ -105,7 +66,12 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { if (!editor) return const handleChange = () => { - const html = editor.value + // Emit the semantic markup we persist, not Trix's internal attachment + // format. lastEmittedRef has to hold the *converted* HTML: it is compared + // against the incoming value prop to decide whether to reload the editor, + // and reloading on our own output would reset the caret on every + // keystroke. + const html = trixHtmlToArticle(editor.value) lastEmittedRef.current = html onChange(html) } @@ -114,17 +80,60 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { return () => { editor.removeEventListener("trix-change", handleChange) } }, [onChange]) - // Uses XHR instead of fetch since fetch doesn't expose upload progress events, - // which Trix needs to render its built-in progress bar. + // Reflect each attachment's stored alignment onto the live figure so our CSS + // can style it. Alignment round-trips as a data-trix-attributes value (Trix + // rebuilds attachment figures from that JSON and would discard a bare class), + // so the class has to be re-applied after every change. useEffect(() => { const editor = editorRef.current if (!editor) return - const handleAttachmentAdd = (event: Event) => { - const { attachment } = event as TrixAttachmentAddEvent - // trix-attachment-add also fires for programmatic URL embeds (no .file) - if (!attachment.file) return + const applyAlignment = () => { + for (const figure of Array.from(editor.querySelectorAll("figure[data-trix-attributes]"))) { + let align: string | undefined + try { + const parsed = JSON.parse(figure.getAttribute("data-trix-attributes") ?? "{}") as { align?: string } + align = parsed.align + } catch { + continue + } + for (const candidate of ALIGNMENTS) { + figure.classList.toggle(TRIX_ALIGN_CLASS[candidate], align === candidate) + } + } + } + + applyAlignment() + editor.addEventListener("trix-change", applyAlignment) + return () => { editor.removeEventListener("trix-change", applyAlignment) } + }, []) + + // Handles every way an image enters the document. Trix fires + // trix-attachment-add for all of them, and which branch runs depends on what + // the attachment carries: + // + // • a File — a dropped/chosen file, or an image pasted straight off the + // clipboard (a screenshot). Uploaded via XHR, which unlike fetch exposes + // progress events so Trix can draw its progress bar. + // • a remote URL and no File — rich text pasted from another site. Copied + // into our own library server-side so the article does not hotlink. + // • one of our own URLs — inserted from the picker; nothing to do. + useEffect(() => { + const editor = editorRef.current + if (!editor) return + + // A failed attachment is removed rather than left behind. Its preview is a + // blob: URL that exists only in this tab: it looks like a working image, + // survives no reload, and is dropped by the serializer, so leaving it in + // place invites the author to publish an article whose image silently + // isn't there. Removing it and saying why is the honest outcome. + const failAttachment = (attachment: TrixAttachment, message: string) => { + attachment.setUploadProgress(100) + attachment.remove() + setNotice(message) + } + const uploadFile = (attachment: TrixAttachment, file: File) => { const xhr = new XMLHttpRequest() xhr.open("POST", `${apiBaseUrl()}/v1/media`, true) // The upload endpoint is session-authenticated, and the API commonly runs @@ -137,60 +146,155 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { } } - // On failure the attachment stays in the editor on its local blob-URL - // preview, so the image can still be moved, captioned, and rearranged. - // That preview does not survive a reload — the saved HTML needs the real - // server URL — so a failed upload has to be retried. xhr.onload = () => { - if (xhr.status === 201) { + if (xhr.status !== 201) { + let detail = `upload failed (${String(xhr.status)})` try { - const { url } = JSON.parse(xhr.responseText) as { url: string } - attachment.setAttributes({ url, href: url }) + const body = JSON.parse(xhr.responseText) as { error?: string } + if (body.error?.trim()) detail = body.error.trim() } catch { - // 201 with an unexpected body: nothing to attach, keep the preview. + // Non-JSON error body; the status code is all we can report. } + failAttachment(attachment, `Could not add ${file.name}: ${detail}`) + return + } + try { + const { url } = JSON.parse(xhr.responseText) as { url: string } + attachment.setAttributes({ url, href: url }) + attachment.setUploadProgress(100) + } catch { + failAttachment(attachment, `Could not add ${file.name}: unexpected server response.`) } - attachment.setUploadProgress(100) } xhr.onerror = () => { - attachment.setUploadProgress(100) + failAttachment(attachment, `Could not add ${file.name}: the upload did not reach the server.`) } const formData = new FormData() - formData.append("file", attachment.file) + formData.append("file", attachment.file ?? file) xhr.send(formData) } + // Pasting rich text from another site brings tags pointing at that + // site. Embedding them as-is would leave the published article depending on + // a third party's server, so take our own copy instead. The fetch is done + // server-side because the browser cannot read cross-origin image bytes. + const sideloadRemote = (attachment: TrixAttachment, sourceUrl: string) => { + attachment.setUploadProgress(5) + fetch(`${apiBaseUrl()}/v1/media/fetch`, { + method: "POST", + credentials: "include", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ url: sourceUrl }), + }) + .then(async (response) => { + if (!response.ok) { + let detail = `import failed (${String(response.status)})` + try { + const body = (await response.json()) as { error?: string } + if (body.error?.trim()) detail = body.error.trim() + } catch { + // Fall through to the status-code message. + } + throw new Error(detail) + } + return (await response.json()) as { url: string } + }) + .then(({ url }) => { + attachment.setAttributes({ url, href: url }) + attachment.setUploadProgress(100) + }) + .catch((err: unknown) => { + // Unlike a failed upload there is no local copy to fall back on, so + // the pasted image cannot be kept in any usable form. + failAttachment( + attachment, + `Could not import the pasted image: ${err instanceof Error ? err.message : "import failed"}`, + ) + }) + } + + const handleAttachmentAdd = (event: Event) => { + const { attachment } = event as TrixAttachmentAddEvent + + if (attachment.file) { + uploadFile(attachment, attachment.file) + return + } + + const url = typeof attachment.getAttribute("url") === "string" ? String(attachment.getAttribute("url")) : "" + if (!url) return + // Already ours (picker insert, or an image copied from another article in + // this CMS) — re-importing would just duplicate the file. + if (isOwnMediaUrl(url)) return + // A blob:/data: URL has no server to fetch from; it arrives with a File + // in every path we support, so reaching here means there is nothing to do. + if (url.startsWith("blob:") || url.startsWith("data:")) return + + sideloadRemote(attachment, url) + } + editor.addEventListener("trix-attachment-add", handleAttachmentAdd) return () => { editor.removeEventListener("trix-attachment-add", handleAttachmentAdd) } }, []) - // Image manipulation overlay: selection, four-corner resize, and - // drag-to-rearrange (vertical drop + left/center/right alignment snap). - // All overlay DOM lives in the wrapper *outside* Trix's contenteditable so - // Trix doesn't overwrite our handles via its MutationObserver. + // Insert a library image at the caret. Alt text comes from the library record, + // so an image described once is described everywhere it is used. + const insertFromLibrary = useCallback((item: MediaPickerItem) => { + setPickerOpen(false) + const editor = editorRef.current + const Trix = window.Trix + if (!editor || !Trix) return + + editor.focus() + if (savedRangeRef.current) { + editor.editor.setSelectedRange(savedRangeRef.current) + savedRangeRef.current = null + } + + const attachment = new Trix.Attachment({ + url: item.url, + href: item.url, + contentType: item.mime_type || contentTypeForUrl(item.url), + filename: item.alt_text || item.file_name, + alt: item.alt_text ?? "", + ...(item.width ? { width: item.width } : {}), + ...(item.height ? { height: item.height } : {}), + // Trix's previewablePattern excludes extension-less URLs, which would + // render a library image as a file stub instead of a preview. + previewable: true, + }) + editor.editor.insertAttachment(attachment) + + if (!item.alt_text) { + setNotice(`Inserted ${item.file_name}, which has no alt text. Add it in the Media library.`) + } + }, []) + + const openPicker = useCallback(() => { + const editor = editorRef.current + // Captured before the modal takes focus; restored on insert. + savedRangeRef.current = editor ? editor.editor.getSelectedRange() : null + setPickerOpen(true) + }, []) + + // Image selection and drag-to-rearrange. All overlay DOM lives in the wrapper + // *outside* Trix's contenteditable so Trix doesn't overwrite it via its + // MutationObserver. + // + // There is deliberately no resize or alignment gesture here. The public site + // sizes article images entirely in CSS (#article figure img { width: 100% }), + // which overrides anything an author sets, so both were controls that appeared + // to work in the editor and changed nothing on the published page. Reordering + // is kept because it does survive. Alignment already stored on legacy + // WordPress content is still preserved through a save -- it just can no longer + // be set from here. useEffect(() => { const editor = editorRef.current const wrapper = wrapperRef.current if (!editor || !wrapper) return - const overlay = document.createElement("div") - overlay.className = "trix-resize-overlay" - overlay.style.display = "none" - - const corners = ["nw", "ne", "sw", "se"] as const - type Corner = (typeof corners)[number] - const handles: Record = {} as Record - for (const corner of corners) { - const handle = document.createElement("div") - handle.className = `trix-resize-handle trix-resize-handle--${corner}` - handle.dataset.corner = corner - overlay.appendChild(handle) - handles[corner] = handle - } - wrapper.appendChild(overlay) - // Drop indicator (a thin horizontal bar shown only during an active drag). const dropIndicator = document.createElement("div") dropIndicator.className = "trix-drop-indicator" @@ -198,82 +302,21 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { wrapper.appendChild(dropIndicator) let activeFigure: HTMLElement | null = null - // Per-gesture cleanup set when a drag or resize is in progress so the - // outer effect's teardown can abort it on unmount (prevents leaked - // document listeners and a stuck `grabbing` cursor). + // Per-gesture cleanup set while a drag is in progress so the outer effect's + // teardown can abort it on unmount (prevents leaked document listeners and + // a stuck `grabbing` cursor). let activeGestureCleanup: (() => void) | null = null - const positionOverlay = () => { - if (!activeFigure) { - overlay.style.display = "none" - return - } - const img = activeFigure.querySelector("img") - const target = img ?? activeFigure - const wrapperRect = wrapper.getBoundingClientRect() - const rect = target.getBoundingClientRect() - overlay.style.display = "block" - overlay.style.top = `${(rect.top - wrapperRect.top).toString()}px` - overlay.style.left = `${(rect.left - wrapperRect.left).toString()}px` - overlay.style.width = `${rect.width.toString()}px` - overlay.style.height = `${rect.height.toString()}px` - } - const selectFigure = (figure: HTMLElement | null) => { if (activeFigure && activeFigure !== figure) { activeFigure.classList.remove("attachment--selected") } activeFigure = figure if (figure) figure.classList.add("attachment--selected") - positionOverlay() - } - - // ── Resize (4 corners, aspect-ratio locked) ──────────────────────────── - const beginResize = (corner: Corner) => (downEvent: MouseEvent) => { - if (!activeFigure) return - const img = activeFigure.querySelector("img") - if (!img) return - downEvent.preventDefault() - downEvent.stopPropagation() - - const startRect = img.getBoundingClientRect() - const startWidth = startRect.width - const startHeight = startRect.height - const aspect = startHeight / startWidth - const startX = downEvent.clientX - const xDirection = corner === "ne" || corner === "se" ? 1 : -1 - - const onMove = (moveEvent: MouseEvent) => { - const dx = (moveEvent.clientX - startX) * xDirection - const newWidth = Math.max(80, Math.round(startWidth + dx)) - const newHeight = Math.max(40, Math.round(newWidth * aspect)) - img.setAttribute("width", String(newWidth)) - img.setAttribute("height", String(newHeight)) - img.style.width = `${newWidth.toString()}px` - img.style.height = `${newHeight.toString()}px` - positionOverlay() - } - const cleanup = () => { - document.removeEventListener("mousemove", onMove) - document.removeEventListener("mouseup", onUp) - } - const onUp = () => { - activeGestureCleanup = null - cleanup() - editor.dispatchEvent(new Event("input", { bubbles: true })) - } - document.addEventListener("mousemove", onMove) - document.addEventListener("mouseup", onUp) - activeGestureCleanup = cleanup - } - - for (const corner of corners) { - handles[corner].addEventListener("mousedown", beginResize(corner)) } // ── Drag-to-rearrange ────────────────────────────────────────────────── - type Alignment = "left" | "center" | "right" - type DropTarget = { block: HTMLElement; insertBefore: boolean; alignment: Alignment } + type DropTarget = { block: HTMLElement; insertBefore: boolean } const DRAG_THRESHOLD_PX = 5 @@ -297,14 +340,6 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { return nearest } - const alignmentFromX = (clientX: number): Alignment => { - const rect = editor.getBoundingClientRect() - const ratio = (clientX - rect.left) / rect.width - if (ratio < 0.3) return "left" - if (ratio > 0.7) return "right" - return "center" - } - const positionDropIndicator = (target: DropTarget) => { const wrapperRect = wrapper.getBoundingClientRect() const editorRect = editor.getBoundingClientRect() @@ -314,22 +349,19 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { const editorLeft = editorRect.left - wrapperRect.left const editorWidth = editorRect.width const indicatorWidth = Math.min(editorWidth * 0.4, 240) - let left = editorLeft + (editorWidth - indicatorWidth) / 2 - if (target.alignment === "left") left = editorLeft + 24 - if (target.alignment === "right") left = editorLeft + editorWidth - indicatorWidth - 24 dropIndicator.style.display = "block" dropIndicator.style.top = `${(y - wrapperRect.top).toString()}px` - dropIndicator.style.left = `${left.toString()}px` + dropIndicator.style.left = `${(editorLeft + (editorWidth - indicatorWidth) / 2).toString()}px` dropIndicator.style.width = `${indicatorWidth.toString()}px` - dropIndicator.dataset.alignment = target.alignment } // Move the attachment via Trix's editor API. Strategy: - // 1. Build the clone HTML with alignment baked in. Strip the stale - // data-trix-id (and matching content-type's sgid hint) so Trix mints - // a fresh attachment on insert rather than colliding with the live - // attachment we're about to remove. + // 1. Clone the figure, stripping the stale data-trix-id so Trix mints a + // fresh attachment on insert rather than colliding with the live + // attachment we're about to remove. data-trix-attributes is left + // untouched, which is what carries any pre-existing alignment through + // the move intact. // 2. Capture a DOM Range anchored on the target block BEFORE removing // the source. Trix's remove() can collapse or detach adjacent empty // blocks, which would invalidate setStartBefore/After afterwards. @@ -343,23 +375,12 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { const attachment = editor.editor.getDocument().getAttachments().find((a) => String(a.id) === trixId) if (!attachment) return if (!target.block.isConnected) return - // No-op if the user dropped right back onto the source's own block. - // findBlockAt no longer skips it so the drop indicator works for - // single-block editors; this is where we treat the same-block drop as - // "nothing to do" and still apply only the alignment change. - const droppedOnSelf = target.block.contains(figure) - if (droppedOnSelf) { - figure.classList.remove("attachment--align-left", "attachment--align-center", "attachment--align-right") - figure.classList.add(`attachment--align-${target.alignment}`) - figure.style.textAlign = target.alignment - editor.dispatchEvent(new Event("input", { bubbles: true })) - return - } + // Dropping onto the figure's own block moves nothing. With alignment gone + // there is no longer any secondary effect to apply, so this is a no-op -- + // and skipping it avoids a needless remove/reinsert of the attachment. + if (target.block.contains(figure)) return const clone = figure.cloneNode(true) as HTMLElement - clone.classList.remove("attachment--align-left", "attachment--align-center", "attachment--align-right") - clone.classList.add(`attachment--align-${target.alignment}`) - clone.style.textAlign = target.alignment clone.removeAttribute("data-trix-id") const html = clone.outerHTML @@ -392,7 +413,6 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { dragging = true figure.classList.add("attachment--dragging") document.body.style.cursor = "grabbing" - overlay.style.display = "none" } const onMove = (moveEvent: MouseEvent) => { @@ -411,11 +431,7 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { } const blockRect = block.getBoundingClientRect() const insertBefore = moveEvent.clientY < blockRect.top + blockRect.height / 2 - dropTarget = { - block, - insertBefore, - alignment: alignmentFromX(moveEvent.clientX), - } + dropTarget = { block, insertBefore } positionDropIndicator(dropTarget) } @@ -451,37 +467,27 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { // ── Selection / drag entry point (capture phase so Trix can't preempt) ── const onEditorMouseDown = (event: MouseEvent) => { const target = event.target as HTMLElement - if (target.closest(".trix-resize-handle")) return const figure = target.closest(".attachment--preview") as HTMLElement | null if (!figure) return beginDrag(figure, event) } const onDocumentMouseDown = (event: MouseEvent) => { const target = event.target as HTMLElement - // Don't deselect when the user is clicking a resize handle or another - // figure; those have their own selection semantics. - if (target.closest(".trix-resize-handle")) return + // Don't deselect when the click lands on another figure; that has its own + // selection semantics. if (target.closest(".attachment--preview")) return selectFigure(null) } - const onReflow = () => positionOverlay() editor.addEventListener("mousedown", onEditorMouseDown, true) document.addEventListener("mousedown", onDocumentMouseDown) - editor.addEventListener("trix-change", onReflow) - window.addEventListener("resize", onReflow) - window.addEventListener("scroll", onReflow, true) return () => { - // Abort any in-progress drag/resize so we don't leave document-level + // Abort any in-progress drag so we don't leave document-level // listeners or a stuck `grabbing` cursor behind on unmount. if (activeGestureCleanup) activeGestureCleanup() editor.removeEventListener("mousedown", onEditorMouseDown, true) document.removeEventListener("mousedown", onDocumentMouseDown) - editor.removeEventListener("trix-change", onReflow) - window.removeEventListener("resize", onReflow) - window.removeEventListener("scroll", onReflow, true) - overlay.remove() dropIndicator.remove() } }, []) @@ -562,6 +568,9 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { + + + @@ -604,6 +613,11 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { + {/* Not a data-trix-action button: this opens our own picker rather + than invoking a built-in Trix action. */} + @@ -636,6 +650,23 @@ function TrixEditor({ value, onChange }: TrixEditorProps) { toolbar={toolbarId} className="trix-content" /> + + {notice && ( +
+ {notice} + +
+ )} + + {pickerOpen && ( + { + setPickerOpen(false) + savedRangeRef.current = null + }} + onSelect={insertFromLibrary} + /> + )}
) } diff --git a/frontend/src/components/trixImageHtml.ts b/frontend/src/components/trixImageHtml.ts new file mode 100644 index 0000000..5bb033b --- /dev/null +++ b/frontend/src/components/trixImageHtml.ts @@ -0,0 +1,255 @@ +// Conversion between the markup Trix keeps in the editor and the markup we +// store on the article. +// +// These two live together because they are inverses and have to stay that way. +// Trix represents an image as
with the +// caption, alt text and alignment encoded in JSON data attributes -- markup that +// means nothing outside a Trix editor. The public site renders article HTML +// directly and has no Trix stylesheet, so what we persist is plain semantic +//
//
using the same WordPress class names as the +// migrated corpus that already renders correctly there. +// +// articleHtmlToTrix runs on load, trixHtmlToArticle on change. Anything one adds +// the other has to understand, or an image loses its caption on a round trip. + +export const ALIGNMENTS = ["left", "center", "right"] as const +export type Alignment = (typeof ALIGNMENTS)[number] + +// The WordPress alignment vocabulary, which the legacy corpus already uses. +const WP_ALIGN_CLASS: Record = { + left: "alignleft", + center: "aligncenter", + right: "alignright", +} + +// Trix has no alignment concept, so in the editor it is a class our overlay +// applies and our CSS styles. +export const TRIX_ALIGN_CLASS: Record = { + left: "attachment--align-left", + center: "attachment--align-center", + right: "attachment--align-right", +} + +const IMAGE_CONTENT_TYPES: Record = { + jpg: "image/jpeg", + jpeg: "image/jpeg", + png: "image/png", + gif: "image/gif", + webp: "image/webp", + avif: "image/avif", + svg: "image/svg+xml", +} + +export const contentTypeForUrl = (url: string): string => { + const ext = url.split(/[?#]/)[0].split(".").pop()?.toLowerCase() ?? "" + return IMAGE_CONTENT_TYPES[ext] ?? "image/jpeg" +} + +// Trix's own parser labels an it picked up with the bare type "image" +// rather than a full MIME type (see processElement in trix.esm.js), and that is +// exactly what a pasted image arrives as. Treating only "image/*" as an image +// would leave every pasted image unserialized. +const isImageContentType = (contentType: string): boolean => + contentType === "image" || contentType.startsWith("image/") + +/** The attachment JSON Trix stores in data-trix-attachment. */ +type AttachmentData = { + contentType?: string + url?: string + href?: string + filename?: string + alt?: string + width?: number + height?: number + previewable?: boolean +} + +/** The piece attributes Trix stores in data-trix-attributes. */ +type AttachmentAttributes = { + caption?: string + presentation?: string + align?: string +} + +const parseJSONAttribute = (el: Element, name: string): T | null => { + const raw = el.getAttribute(name) + if (!raw) return null + try { + const parsed: unknown = JSON.parse(raw) + return parsed && typeof parsed === "object" ? (parsed as T) : null + } catch { + return null + } +} + +const alignmentFromClasses = (el: Element, table: Record): Alignment | null => { + for (const alignment of ALIGNMENTS) { + if (el.classList.contains(table[alignment])) return alignment + } + return null +} + +const asAlignment = (value: unknown): Alignment | null => + ALIGNMENTS.includes(value as Alignment) ? (value as Alignment) : null + +const positiveInt = (value: unknown): number | null => { + const n = typeof value === "string" ? Number.parseInt(value, 10) : typeof value === "number" ? value : NaN + return Number.isFinite(n) && n > 0 ? Math.round(n) : null +} + +const parseDocument = (html: string): Document | null => { + if (typeof window === "undefined" || !window.DOMParser) return null + return new DOMParser().parseFromString(html, "text/html") +} + +// ── Load: article HTML → Trix attachment markup ────────────────────────────── + +/** + * Rewrite stored image figures into Trix's native attachment format so the + * editor keeps the caption bound to the image, exposes the alt text for + * editing, and treats the figure as a movable attachment. + * + * Without this, Trix's parser drops a
onto the next line as body + * text -- the caption "isn't picked up by the editor". It handles both the + * markup this module writes and the two WordPress import shapes (block editor: + *
; classic editor:
…<* + * class="wp-caption-text">). + * + * Idempotent: figures that already carry data-trix-attachment are skipped. + */ +export const articleHtmlToTrix = (html: string): string => { + if (!html.includes(" would silently drop + // the rest, so leave those untouched. + if (container.querySelectorAll("img").length !== 1) continue + if (container.querySelector("figure, .wp-caption")) continue + + const img = container.querySelector("img") + const src = img?.getAttribute("src")?.trim() + if (!img || !src) continue + + const caption = container.querySelector("figcaption, .wp-caption-text")?.textContent?.trim() ?? "" + const alt = img.getAttribute("alt")?.trim() ?? "" + const width = positiveInt(img.getAttribute("width")) + const height = positiveInt(img.getAttribute("height")) + // WordPress puts the alignment on either the container or the image itself. + const align = + alignmentFromClasses(container, WP_ALIGN_CLASS) ?? alignmentFromClasses(img, WP_ALIGN_CLASS) + + const data: AttachmentData = { + contentType: contentTypeForUrl(src), + url: src, + // Trix shows filename in the caption placeholder; alt is the most useful + // thing to surface there, with the basename as a fallback. + filename: alt || src.split("/").pop()?.split(/[?#]/)[0] || "image", + alt, + // Force inline preview: Trix's previewablePattern excludes svg/avif and + // any extension-less CDN URL, which would otherwise render as a file stub. + previewable: true, + } + if (width) data.width = width + if (height) data.height = height + + const attributes: AttachmentAttributes = { presentation: "gallery" } + if (caption) attributes.caption = caption + if (align) attributes.align = align + + const figure = doc.createElement("figure") + figure.setAttribute("data-trix-attachment", JSON.stringify(data)) + figure.setAttribute("data-trix-attributes", JSON.stringify(attributes)) + if (align) figure.classList.add(TRIX_ALIGN_CLASS[align]) + + const newImg = doc.createElement("img") + newImg.setAttribute("src", src) + if (alt) newImg.setAttribute("alt", alt) + figure.appendChild(newImg) + + container.replaceWith(figure) + changed = true + } + + return changed ? doc.body.innerHTML : html +} + +// ── Save: Trix attachment markup → article HTML ────────────────────────────── + +/** + * Rewrite Trix's attachment figures into the semantic markup we persist and the + * public site renders:
. + * + * Non-image attachments are left exactly as Trix wrote them -- this only knows + * how to reduce an image to an , and silently mangling anything else would + * lose data. + * + * All values are written through DOM setAttribute/textContent, so attacker- or + * author-supplied captions and URLs are escaped by the serializer rather than by + * hand-built string concatenation. + */ +export const trixHtmlToArticle = (html: string): string => { + if (!html.includes("data-trix-attachment")) return html + const doc = parseDocument(html) + if (!doc) return html + + let changed = false + + for (const figure of Array.from(doc.querySelectorAll("figure[data-trix-attachment]"))) { + const data = parseJSONAttribute(figure, "data-trix-attachment") + const src = data?.url?.trim() + if (!data || !src) continue + // A still-uploading attachment is on a local blob: URL that means nothing + // once the page reloads. Leave it as Trix has it so the in-progress state + // survives, rather than baking a dead URL into the saved article. + if (src.startsWith("blob:") || src.startsWith("data:")) continue + if (data.contentType && !isImageContentType(data.contentType)) continue + + const attributes = parseJSONAttribute(figure, "data-trix-attributes") + const caption = attributes?.caption?.trim() ?? "" + // Alignment may be a piece attribute (what survives a Trix round trip) or, + // for a figure the overlay just moved, only a class on the live element. + const align = asAlignment(attributes?.align) ?? alignmentFromClasses(figure, TRIX_ALIGN_CLASS) + + // Prefer the attachment's own alt. The inner is Trix-rendered and its + // alt may be a filename Trix filled in rather than authored alt text. + const alt = (data.alt ?? figure.querySelector("img")?.getAttribute("alt") ?? "").trim() + + const out = doc.createElement("figure") + out.classList.add("wp-caption") + if (align) out.classList.add(WP_ALIGN_CLASS[align]) + + const img = doc.createElement("img") + img.setAttribute("src", src) + // Always emit alt, even empty: an with no alt attribute at all is an + // accessibility failure, whereas alt="" is a valid "decorative" signal. + img.setAttribute("alt", alt) + // Deliberately no width/height. The public site sizes article images purely + // in CSS (#article figure img { width: 100% }), and width/height attributes + // are presentational hints that CSS outranks -- so the width would be + // overridden while the height still applied, stretching every image. They + // would normally be worth emitting to reserve layout space, but that only + // holds on a page with a matching `height: auto`. + img.setAttribute("loading", "lazy") + out.appendChild(img) + + if (caption) { + const figcaption = doc.createElement("figcaption") + figcaption.className = "wp-caption-text" + figcaption.textContent = caption + out.appendChild(figcaption) + } + + figure.replaceWith(out) + changed = true + } + + return changed ? doc.body.innerHTML : html +} diff --git a/frontend/src/trix.d.ts b/frontend/src/trix.d.ts index bdcc4cc..501f792 100644 --- a/frontend/src/trix.d.ts +++ b/frontend/src/trix.d.ts @@ -1,6 +1,21 @@ import "trix" declare global { + // The attributes an attachment is constructed with. These are what Trix + // serializes into the figure's data-trix-attachment JSON, so anything added + // here survives a save/load round trip -- which is how alt text is carried. + interface TrixAttachmentAttributes { + url: string + href?: string + contentType?: string + filename?: string + filesize?: number + width?: number + height?: number + alt?: string + previewable?: boolean + } + interface Window { Trix?: { config: { @@ -13,6 +28,7 @@ declare global { } } } + Attachment: new (attributes: TrixAttachmentAttributes) => TrixAttachment } } @@ -27,6 +43,8 @@ declare global { setSelectedRange(range: [number, number] | number): void deleteInDirection(direction: "forward" | "backward"): void insertHTML(html: string): void + insertAttachment(attachment: TrixAttachment): void + insertLineBreak(): void activateAttachment(attachment: TrixAttachment): void } @@ -38,8 +56,10 @@ declare global { interface TrixAttachment { id: number file: File | null + getAttribute(name: string): unknown + getAttributes(): Partial setUploadProgress(value: number): void - setAttributes(attrs: Partial<{ url: string; href: string }>): void + setAttributes(attrs: Partial): void remove(): void } diff --git a/server/docs/docs.go b/server/docs/docs.go index fd6aed6..83964fa 100644 --- a/server/docs/docs.go +++ b/server/docs/docs.go @@ -2047,6 +2047,74 @@ const docTemplate = `{ } } }, + "/v1/media/fetch": { + "post": { + "security": [ + { + "BearerAuth": [] + } + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "media" + ], + "summary": "Sideload a remote image into the library", + "parameters": [ + { + "description": "Remote image URL", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/models.MediaFetchRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/models.MediaUploadResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + }, + "413": { + "description": "Request Entity Too Large", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + }, + "415": { + "description": "Unsupported Media Type", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + }, + "501": { + "description": "Not Implemented", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + }, + "502": { + "description": "Bad Gateway", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + } + } + } + }, "/v1/media/gallery": { "get": { "security": [ @@ -5232,6 +5300,14 @@ const docTemplate = `{ } } }, + "models.MediaFetchRequest": { + "type": "object", + "properties": { + "url": { + "type": "string" + } + } + }, "models.MediaGalleryResponse": { "type": "object", "properties": { diff --git a/server/docs/swagger.json b/server/docs/swagger.json index b6df346..730cf0f 100644 --- a/server/docs/swagger.json +++ b/server/docs/swagger.json @@ -2044,6 +2044,74 @@ } } }, + "/v1/media/fetch": { + "post": { + "security": [ + { + "BearerAuth": [] + } + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "media" + ], + "summary": "Sideload a remote image into the library", + "parameters": [ + { + "description": "Remote image URL", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/models.MediaFetchRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/models.MediaUploadResponse" + } + }, + "400": { + "description": "Bad Request", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + }, + "413": { + "description": "Request Entity Too Large", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + }, + "415": { + "description": "Unsupported Media Type", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + }, + "501": { + "description": "Not Implemented", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + }, + "502": { + "description": "Bad Gateway", + "schema": { + "$ref": "#/definitions/models.ErrorResponse" + } + } + } + } + }, "/v1/media/gallery": { "get": { "security": [ @@ -5229,6 +5297,14 @@ } } }, + "models.MediaFetchRequest": { + "type": "object", + "properties": { + "url": { + "type": "string" + } + } + }, "models.MediaGalleryResponse": { "type": "object", "properties": { diff --git a/server/docs/swagger.yaml b/server/docs/swagger.yaml index 683ed35..0348a7f 100644 --- a/server/docs/swagger.yaml +++ b/server/docs/swagger.yaml @@ -700,6 +700,11 @@ definitions: width: type: integer type: object + models.MediaFetchRequest: + properties: + url: + type: string + type: object models.MediaGalleryResponse: properties: media: @@ -2513,6 +2518,49 @@ paths: summary: Update media metadata tags: - media + /v1/media/fetch: + post: + consumes: + - application/json + parameters: + - description: Remote image URL + in: body + name: request + required: true + schema: + $ref: '#/definitions/models.MediaFetchRequest' + produces: + - application/json + responses: + "201": + description: Created + schema: + $ref: '#/definitions/models.MediaUploadResponse' + "400": + description: Bad Request + schema: + $ref: '#/definitions/models.ErrorResponse' + "413": + description: Request Entity Too Large + schema: + $ref: '#/definitions/models.ErrorResponse' + "415": + description: Unsupported Media Type + schema: + $ref: '#/definitions/models.ErrorResponse' + "501": + description: Not Implemented + schema: + $ref: '#/definitions/models.ErrorResponse' + "502": + description: Bad Gateway + schema: + $ref: '#/definitions/models.ErrorResponse' + security: + - BearerAuth: [] + summary: Sideload a remote image into the library + tags: + - media /v1/media/gallery: get: parameters: diff --git a/server/internal/handlers/media.go b/server/internal/handlers/media.go index 113105e..bfd3124 100644 --- a/server/internal/handlers/media.go +++ b/server/internal/handlers/media.go @@ -17,13 +17,16 @@ import ( _ "image/jpeg" _ "image/png" "io" + "net" "net/http" + "net/url" "os" "path" "path/filepath" "regexp" "strconv" "strings" + "syscall" "time" "server/internal/activity" @@ -53,6 +56,24 @@ const ( // mediaIndexTimeout bounds a background index run so a wedged filesystem // cannot leave the job permanently "running" and block every later attempt. mediaIndexTimeout = 2 * time.Hour + // remoteFetchTimeout bounds a whole sideload (see PostMediaFetch). Pasting + // blocks the editor's attachment on this call, so it has to fail fast rather + // than inherit the server's much longer default. + remoteFetchTimeout = 30 * time.Second + // maxRemoteRedirects caps a sideload's redirect chain. Every hop is + // re-validated by safeDialControl, so this only bounds the work, not the risk. + maxRemoteRedirects = 5 +) + +// Sentinel failures from storeImage, mapped onto status codes by +// writeStoreImageError. The distinction matters to callers: an unsupported type +// is the client's fault, whereas an index failure means the bytes are already +// durable on disk and only the library row is missing. +var ( + errMediaNotConfigured = errors.New("media storage is not configured") + errUnsupportedType = errors.New("unsupported file type") + errStoreFailed = errors.New("failed to store upload") + errIndexFailed = errors.New("file stored but could not be added to the media library") ) // allowedImageTypes maps a sniffed content type to its canonical extension. Only @@ -146,75 +167,284 @@ func PostMedia(conn *sql.DB) http.Handler { } defer file.Close() - // Sniff the real content type from the leading bytes; never trust the - // client-provided extension or Content-Type. - head := make([]byte, 512) - n, err := io.ReadFull(file, head) - if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) && !errors.Is(err, io.EOF) { - writeError(w, http.StatusInternalServerError, "failed to read upload") + stored, err := storeImage(r.Context(), conn, file, header.Filename) + if err != nil { + writeStoreImageError(w, err) return } - contentType := http.DetectContentType(head[:n]) - ext, ok := allowedImageTypes[contentType] - if !ok { - writeError(w, http.StatusUnsupportedMediaType, "unsupported file type: "+contentType) + + activity.LogRequest(r, "media_uploaded", path.Base(stored.Path), "path", stored.Path) + writeJSON(w, http.StatusCreated, stored) + }) +} + +// storeImage validates, stores and indexes a single image, and is the one place +// bytes become a media library entry -- shared by the multipart upload path and +// the paste sideload path so both agree on what is accepted and where it lands. +// +// src must be seekable: sniffing the content type consumes the leading bytes, +// which have to be rewound before the file is written. +func storeImage(ctx context.Context, conn *sql.DB, src io.ReadSeeker, filename string) (models.MediaUploadResponse, error) { + root := mediaRoot() + if root == "" { + return models.MediaUploadResponse{}, errMediaNotConfigured + } + + // Sniff the real content type from the leading bytes; never trust the + // client-provided extension or Content-Type. + head := make([]byte, 512) + n, err := io.ReadFull(src, head) + if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) && !errors.Is(err, io.EOF) { + return models.MediaUploadResponse{}, fmt.Errorf("%w: read: %v", errStoreFailed, err) + } + contentType := http.DetectContentType(head[:n]) + ext, ok := allowedImageTypes[contentType] + if !ok { + return models.MediaUploadResponse{}, fmt.Errorf("%w: %s", errUnsupportedType, contentType) + } + if _, err := src.Seek(0, io.SeekStart); err != nil { + return models.MediaUploadResponse{}, fmt.Errorf("%w: rewind: %v", errStoreFailed, err) + } + + now := time.Now().UTC() + relDir := path.Join(uploadsSubdir, now.Format("2006"), now.Format("01")) + absDir := filepath.Join(root, filepath.FromSlash(relDir)) + if err := os.MkdirAll(absDir, 0o775); err != nil { + slog.Error("media upload: create directory", "dir", absDir, "error", err) + return models.MediaUploadResponse{}, fmt.Errorf("%w: create directory: %v", errStoreFailed, err) + } + + name, written, err := storeUpload(absDir, sanitizeBaseName(filename), ext, src) + if err != nil { + // Worth a log line rather than just a 500: the message the client + // gets cannot say whether the media volume is full, unmounted, or + // simply not writable by this container's uid, and those need very + // different fixes. A permission error here is easy to mistake for a + // mkdir failure, since MkdirAll returns nil for a directory that + // already exists -- which every migrated YYYY/MM directory does. + slog.Error("media upload: store file", "dir", absDir, "error", err) + return models.MediaUploadResponse{}, fmt.Errorf("%w: %v", errStoreFailed, err) + } + + relPath := path.Join(relDir, name) + width, height := imageDimensions(filepath.Join(absDir, name)) + + // The file is already durable at this point. If recording it fails the + // upload is still reported as a failure, but the file is left in place: + // a reindex will adopt it rather than leaving a silent orphan. + id, err := db.InsertMedia(ctx, conn, relPath, contentType, written, width, height) + if err != nil { + return models.MediaUploadResponse{}, fmt.Errorf("%w: %v", errIndexFailed, err) + } + + return models.MediaUploadResponse{ + ID: id, + Path: relPath, + URL: uploadURL(relPath), + ContentType: contentType, + Size: written, + Width: width, + Height: height, + }, nil +} + +// writeStoreImageError maps a storeImage failure onto a response. Only the +// sentinel text is echoed to the client; the wrapped detail stays in the log, +// since it can name filesystem paths. +func writeStoreImageError(w http.ResponseWriter, err error) { + switch { + case errors.Is(err, errMediaNotConfigured): + writeError(w, http.StatusNotImplemented, errMediaNotConfigured.Error()) + case errors.Is(err, errUnsupportedType): + // Safe to echo in full: the only variable part is a sniffed MIME type. + writeError(w, http.StatusUnsupportedMediaType, err.Error()) + case errors.Is(err, errIndexFailed): + writeError(w, http.StatusInternalServerError, errIndexFailed.Error()) + default: + writeError(w, http.StatusInternalServerError, errStoreFailed.Error()) + } +} + +// PostMediaFetch copies an image that lives on someone else's server into the +// media library and returns it in the same shape as an upload. +// +// This is what makes pasting work. When an author pastes rich text from another +// site the clipboard carries tags pointing at that site, and embedding +// them as-is would leave the published article hotlinking a third party: it +// breaks when they move the file, leaks our readers' referrers, and puts content +// we cannot vouch for on the page. Sideloading takes a copy up front instead. +// +// @Summary Sideload a remote image into the library +// @Tags media +// @Accept json +// @Produce json +// @Param request body models.MediaFetchRequest true "Remote image URL" +// @Success 201 {object} models.MediaUploadResponse +// @Failure 400 {object} models.ErrorResponse +// @Failure 413 {object} models.ErrorResponse +// @Failure 415 {object} models.ErrorResponse +// @Failure 502 {object} models.ErrorResponse +// @Failure 501 {object} models.ErrorResponse +// @Security BearerAuth +// @Router /v1/media/fetch [post] +func PostMediaFetch(conn *sql.DB) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if mediaRoot() == "" { + writeError(w, http.StatusNotImplemented, errMediaNotConfigured.Error()) return } - if _, err := file.Seek(0, io.SeekStart); err != nil { - writeError(w, http.StatusInternalServerError, "failed to rewind upload") + + var body models.MediaFetchRequest + if err := json.NewDecoder(io.LimitReader(r.Body, 8<<10)).Decode(&body); err != nil { + writeError(w, http.StatusBadRequest, "invalid JSON body") return } - now := time.Now().UTC() - relDir := path.Join(uploadsSubdir, now.Format("2006"), now.Format("01")) - absDir := filepath.Join(root, filepath.FromSlash(relDir)) - if err := os.MkdirAll(absDir, 0o775); err != nil { - slog.Error("media upload: create directory", "dir", absDir, "error", err) - writeError(w, http.StatusInternalServerError, "failed to create upload directory") + target, err := url.Parse(strings.TrimSpace(body.URL)) + if err != nil || target.Host == "" || (target.Scheme != "http" && target.Scheme != "https") { + writeError(w, http.StatusBadRequest, "url must be an absolute http(s) URL") return } - base := sanitizeBaseName(header.Filename) - name, written, err := storeUpload(absDir, base, ext, file) + req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, target.String(), nil) if err != nil { - // Worth a log line rather than just a 500: the message the client - // gets cannot say whether the media volume is full, unmounted, or - // simply not writable by this container's uid, and those need very - // different fixes. A permission error here is easy to mistake for a - // mkdir failure, since MkdirAll returns nil for a directory that - // already exists -- which every migrated YYYY/MM directory does. - slog.Error("media upload: store file", "dir", absDir, "error", err) - writeError(w, http.StatusInternalServerError, "failed to store upload") + writeError(w, http.StatusBadRequest, "url could not be requested") return } + req.Header.Set("Accept", "image/*") - relPath := path.Join(relDir, name) - absPath := filepath.Join(absDir, name) - width, height := imageDimensions(absPath) + resp, err := remoteImageClient().Do(req) + if err != nil { + // The reason (blocked address, DNS failure, timeout) goes to the log + // rather than the response: echoing it back would turn this endpoint + // into a probe that reports what the server can reach. + slog.Warn("media sideload: fetch failed", "url", target.String(), "error", err) + writeError(w, http.StatusBadGateway, "could not fetch the image from that URL") + return + } + defer resp.Body.Close() - // The file is already durable at this point. If recording it fails the - // upload is still reported as a failure, but the file is left in place: - // a reindex will adopt it rather than leaving a silent orphan. - id, err := db.InsertMedia(r.Context(), conn, relPath, contentType, written, width, height) + if resp.StatusCode != http.StatusOK { + slog.Warn("media sideload: unexpected status", "url", target.String(), "status", resp.StatusCode) + writeError(w, http.StatusBadGateway, fmt.Sprintf("remote server returned %d", resp.StatusCode)) + return + } + + // Buffer to a temp file: storeImage has to seek back after sniffing and a + // response body cannot. Reading one byte past the cap is what detects an + // oversized image -- Content-Length is supplied by the remote server and + // so cannot be trusted as the guard. + maxBytes := maxUploadBytes() + tmp, err := os.CreateTemp("", "sideload-*") if err != nil { - writeError(w, http.StatusInternalServerError, "file stored but could not be added to the media library") + writeError(w, http.StatusInternalServerError, errStoreFailed.Error()) return } + defer func() { + _ = tmp.Close() + _ = os.Remove(tmp.Name()) + }() - activity.LogRequest(r, "media_uploaded", name, "path", relPath) + written, err := io.Copy(tmp, io.LimitReader(resp.Body, maxBytes+1)) + if err != nil { + slog.Warn("media sideload: read failed", "url", target.String(), "error", err) + writeError(w, http.StatusBadGateway, "could not read the image from that URL") + return + } + if written > maxBytes { + writeError(w, http.StatusRequestEntityTooLarge, + fmt.Sprintf("image exceeds maximum size of %d bytes", maxBytes)) + return + } + if _, err := tmp.Seek(0, io.SeekStart); err != nil { + writeError(w, http.StatusInternalServerError, errStoreFailed.Error()) + return + } - writeJSON(w, http.StatusCreated, models.MediaUploadResponse{ - ID: id, - Path: relPath, - URL: uploadURL(relPath), - ContentType: contentType, - Size: written, - Width: width, - Height: height, - }) + stored, err := storeImage(r.Context(), conn, tmp, remoteFileName(target)) + if err != nil { + writeStoreImageError(w, err) + return + } + + activity.LogRequest(r, "media_sideloaded", path.Base(stored.Path), + "path", stored.Path, "source", target.String()) + writeJSON(w, http.StatusCreated, stored) }) } +// remoteImageClient builds the HTTP client used for sideloads. It is created per +// request rather than shared: sideloads are rare, and a fresh client keeps the +// connection pool from holding sockets open to arbitrary third-party hosts. +func remoteImageClient() *http.Client { + dialer := &net.Dialer{Timeout: 10 * time.Second, Control: safeDialControl} + return &http.Client{ + Timeout: remoteFetchTimeout, + Transport: &http.Transport{DialContext: dialer.DialContext}, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= maxRemoteRedirects { + return errors.New("too many redirects") + } + if req.URL.Scheme != "http" && req.URL.Scheme != "https" { + return fmt.Errorf("refusing to follow redirect to scheme %q", req.URL.Scheme) + } + return nil + }, + } +} + +// safeDialControl refuses to open a socket to anything but a public unicast +// address. This is the SSRF guard, and it deliberately sits at dial time rather +// than at URL-parse time: it sees the address actually being connected to, after +// DNS has resolved and on every hop of a redirect chain. Validating the +// hostname up front instead would miss a name that resolves to 127.0.0.1, a +// name that resolves differently on the second lookup (DNS rebinding), and a +// public URL that redirects to the cloud metadata endpoint. +func safeDialControl(_, address string, _ syscall.RawConn) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return fmt.Errorf("unexpected dial address %q", address) + } + ip := net.ParseIP(host) + if ip == nil { + return fmt.Errorf("unresolved dial address %q", address) + } + if !isPublicUnicast(ip) { + return fmt.Errorf("refusing to connect to non-public address %s", ip) + } + return nil +} + +// isPublicUnicast reports whether ip is routable on the public internet, and so +// is not something this server should be tricked into fetching on a caller's +// behalf. Everything internal is rejected: loopback, RFC1918, link-local +// (which covers the 169.254.169.254 cloud metadata endpoint), multicast, and +// the two ranges net.IP has no predicate for. +func isPublicUnicast(ip net.IP) bool { + if ip.IsLoopback() || ip.IsPrivate() || ip.IsUnspecified() || + ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || + ip.IsInterfaceLocalMulticast() || ip.IsMulticast() { + return false + } + if v4 := ip.To4(); v4 != nil { + // Carrier-grade NAT, 100.64.0.0/10. + return !(v4[0] == 100 && v4[1] >= 64 && v4[1] <= 127) + } + // IPv6 unique-local, fc00::/7. + return ip[0]&0xfe != 0xfc +} + +// remoteFileName derives an upload base name from a source URL. The result is +// still passed through sanitizeBaseName, so this only has to produce something +// meaningful, not something safe. +func remoteFileName(u *url.URL) string { + base := path.Base(u.Path) + if base == "." || base == "/" || base == "" { + return "pasted-image" + } + return base +} + // sanitizeBaseName reduces a client filename to a safe, slugified base (no // directory components, no extension). This is the only use of the client name, // so path traversal is impossible regardless of input. diff --git a/server/internal/handlers/media_fetch_test.go b/server/internal/handlers/media_fetch_test.go new file mode 100644 index 0000000..799748a --- /dev/null +++ b/server/internal/handlers/media_fetch_test.go @@ -0,0 +1,112 @@ +package handlers + +import ( + "net" + "net/url" + "testing" +) + +// TestIsPublicUnicast covers the SSRF guard's classification directly. The +// internal cases are the ones that matter: each is an address a pasted image URL +// could resolve to in order to make the server fetch something on the caller's +// behalf. +func TestIsPublicUnicast(t *testing.T) { + tests := []struct { + name string + ip string + want bool + }{ + {"public v4", "93.184.216.34", true}, + {"public v6", "2606:2800:220:1:248:1893:25c8:1946", true}, + {"loopback v4", "127.0.0.1", false}, + {"loopback v4 alternate", "127.19.8.6", false}, + {"loopback v6", "::1", false}, + {"private 10/8", "10.0.0.5", false}, + {"private 172.16/12", "172.16.31.4", false}, + {"private 192.168/16", "192.168.1.1", false}, + {"unspecified", "0.0.0.0", false}, + // The cloud metadata endpoint is the classic SSRF target, and it is + // covered by the link-local check rather than by a rule of its own. + {"cloud metadata", "169.254.169.254", false}, + {"link-local v6", "fe80::1", false}, + {"multicast", "224.0.0.1", false}, + {"carrier-grade NAT low", "100.64.0.1", false}, + {"carrier-grade NAT high", "100.127.255.254", false}, + // 100.63 and 100.128 sit just outside 100.64.0.0/10 and must stay + // reachable -- the mask is easy to get wrong by a byte. + {"below carrier-grade NAT", "100.63.255.255", true}, + {"above carrier-grade NAT", "100.128.0.1", true}, + {"unique-local v6", "fd00::1", false}, + {"unique-local v6 low", "fc00::1", false}, + // fe00::/8 shares its first nibble with fc00::/7 but is not unique-local. + {"above unique-local v6", "fe00::1", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ip := net.ParseIP(tt.ip) + if ip == nil { + t.Fatalf("bad test fixture: %q is not an IP", tt.ip) + } + if got := isPublicUnicast(ip); got != tt.want { + t.Fatalf("isPublicUnicast(%s) = %v, want %v", tt.ip, got, tt.want) + } + }) + } +} + +// TestSafeDialControl checks the dialer hook itself, which is what actually +// blocks the connection. A v4-mapped v6 form is included because that is how a +// resolver can hand back an internal v4 address on a dual-stack host. +func TestSafeDialControl(t *testing.T) { + tests := []struct { + name string + address string + wantErr bool + }{ + {"public", "93.184.216.34:443", false}, + {"loopback", "127.0.0.1:80", true}, + {"private", "10.1.2.3:8080", true}, + {"metadata", "169.254.169.254:80", true}, + {"v4-mapped loopback", "[::ffff:127.0.0.1]:80", true}, + {"no port", "93.184.216.34", true}, + {"unresolved hostname", "example.com:443", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := safeDialControl("tcp", tt.address, nil) + if tt.wantErr && err == nil { + t.Fatalf("safeDialControl(%q) = nil, want error", tt.address) + } + if !tt.wantErr && err != nil { + t.Fatalf("safeDialControl(%q) = %v, want nil", tt.address, err) + } + }) + } +} + +func TestRemoteFileName(t *testing.T) { + tests := []struct { + raw string + want string + }{ + {"https://example.com/wp-content/uploads/2026/07/photo.jpg", "photo.jpg"}, + {"https://example.com/image", "image"}, + {"https://example.com/", "pasted-image"}, + {"https://example.com", "pasted-image"}, + {"https://example.com/a/b/c.png?v=2", "c.png"}, + } + + for _, tt := range tests { + t.Run(tt.raw, func(t *testing.T) { + u, err := url.Parse(tt.raw) + if err != nil { + t.Fatalf("parse: %v", err) + } + if got := remoteFileName(u); got != tt.want { + t.Fatalf("remoteFileName(%q) = %q, want %q", tt.raw, got, tt.want) + } + }) + } +} diff --git a/server/internal/models/api_responses.go b/server/internal/models/api_responses.go index 4763118..27c8577 100644 --- a/server/internal/models/api_responses.go +++ b/server/internal/models/api_responses.go @@ -474,6 +474,12 @@ type MediaGalleryResponse struct { Media []MediaOverview `json:"media"` } +// MediaFetchRequest asks the server to copy a remote image into the library. +// URL must be an absolute http(s) URL; see PostMediaFetch for what is refused. +type MediaFetchRequest struct { + URL string `json:"url"` +} + // MediaUploadResponse describes a stored media asset. Path is the canonical // wp-content-relative path (what to persist as an article's photo_url); URL is // that path rendered through the configured media base for immediate display. diff --git a/server/internal/routes/routes.go b/server/internal/routes/routes.go index a065023..a97d29c 100644 --- a/server/internal/routes/routes.go +++ b/server/internal/routes/routes.go @@ -82,7 +82,11 @@ func Register(mux *http.ServeMux, conn *sql.DB, verifier *oidc.IDTokenVerifier, mux.Handle("GET /v1/media", authMW(handlers.GetMedia(conn))) mux.Handle("GET /v1/media/gallery", authMW(handlers.GetMediaGallery(conn))) mux.Handle("GET /v1/media/{id}", authMW(handlers.GetMediaItem(conn))) - mux.Handle("POST /v1/media", authMW(adminOnly(handlers.PostMedia(conn)))) + // Adding an image is part of writing an article, so uploading (and the paste + // sideload that wraps it) is open to editors. Destructive and bulk + // operations -- delete, reindex -- stay admin-only. + mux.Handle("POST /v1/media", authMW(handlers.PostMedia(conn))) + mux.Handle("POST /v1/media/fetch", authMW(handlers.PostMediaFetch(conn))) // Indexing runs in the background (the walk outlives any proxy timeout), so // starting it and polling its progress are separate endpoints. mux.Handle("POST /v1/media/index", authMW(adminOnly(handlers.PostMediaIndex(conn)))) diff --git a/server/internal/routes/routes_test.go b/server/internal/routes/routes_test.go index 7302f6e..c191e99 100644 --- a/server/internal/routes/routes_test.go +++ b/server/internal/routes/routes_test.go @@ -182,6 +182,7 @@ func TestRegister_MediaEndpointsGated(t *testing.T) { {http.MethodGet, "/v1/media/gallery"}, {http.MethodGet, "/v1/media/1"}, {http.MethodPost, "/v1/media"}, + {http.MethodPost, "/v1/media/fetch"}, {http.MethodPost, "/v1/media/index"}, {http.MethodPatch, "/v1/media/1"}, {http.MethodDelete, "/v1/media/1"},