fix(editor): render JWT-gated /api/attachments images via blob URLs - #682
Merged
Conversation
Browser <img> tags cannot send Authorization headers, so direct loads against `/api/attachments/:pageId/:filename` always returned 401 in edit mode — pasted uploads and Confluence-synced images alike never rendered while editing, even though the upload itself succeeded and the file served correctly with a JWT. ArticleViewer (read mode) already worked around this by rewriting srcs to authenticated blob URLs via fetchAuthenticatedBlob. The Editor had no equivalent. This adds an addNodeView() on the ConfluenceImage extension that: - Renders the <img> with TipTap's stock contenteditable/draggable bits. - For `/api/attachments/...` or `/api/local-attachments/...` srcs, fetches with the bearer token and renders via a blob URL; falls through for any other src (data:, external, …). - Never mutates `node.attrs.src`, so `editor.getHTML()` keeps serialising the canonical URL on save — blob URLs stay DOM-only. - Revokes the blob URL on node update/destroy. Verified end-to-end with Playwright: paste of a 1×1 PNG now renders with naturalWidth > 0, and editor.getHTML() output contains the canonical /api/attachments src (no blob: leaks into persisted HTML). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… in image NodeView Two bugs found in code review of #682: 1. Destroy + in-flight fetch race: the previous `currentSrc !== src` guard only covered src-change races. If the NodeView was destroyed while a fetch was still pending, the promise would resolve, find currentSrc unchanged, assign `blobUrl = url` and call `dom.setAttribute('src', url)` — leaking the blob URL forever (destroy already revoked the previous blobUrl, which was null at the time, and won't fire again). Now a `destroyed` flag short-circuits the resolve handler and revokes the orphaned URL. 2. Update path didn't remove stale attributes. Iterating newNode.attrs and setAttribute-ing only the present keys meant an attribute removed on the new node (e.g. user clearing `alt`) lingered on the DOM until the editor remounted. A `writtenAttrKeys` set now tracks what we've written and removes anything that disappeared. Also adds 6 NodeView tests covering: blob-URL rewrite for both attachment prefixes, no-fetch for external srcs, canonical src preserved in getHTML, blob revocation on unmount, and the destroy-before-resolve race. Full suite: 72/72 pass (was 66, +6 new). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gression test Two test improvements from the re-review of #682: 1. The save-safety test ("keeps the canonical /api/attachments src in editor.getHTML()") was a silent no-op. It waited on `onChange`, which only fires on transactions — none were dispatched, so `capturedHtml` stayed empty and the assertions were guarded behind a never-taken `if (capturedHtml)`. The test passed even when the implementation would have leaked blob URLs into saved HTML. Now grabs the editor via `onEditorReady` and calls `getHTML` / `getJSON` directly. Asserts both the serialized HTML and every image node's `attrs.src` hold the canonical /api/attachments URL — no `blob:` anywhere in the persisted shape. 2. New test pinning the attr-removal half of the previous follow-up: dispatches a transaction that clears `alt` on an existing image node, then asserts the DOM <img> no longer has the `alt` attribute. Without the `syncAttrs` cleanup loop, the stale value would linger until the editor remounted. Pre-fix the test would fail; post-fix it passes. Full suite: 73/73 pass (was 72, +1 net — the no-op test now actually runs, plus the new attr-removal test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
<img>tags cannot sendAuthorizationheaders, and/api/attachments/:pageId/:filenameis gated byfastify.authenticate.fetchAuthenticatedBlob(lines 250-350). The Editor had no equivalent.addNodeView()on theConfluenceImageextension. It fetches/api/attachments/...(and/api/local-attachments/...) URLs with the bearer token and renders via a blob URL, while leavingnode.attrs.srcuntouched so saves persist the canonical URL.Why this was hard to notice
The upload pipeline succeeds and the file serves correctly if you request it with auth. The browser just couldn't get past the 401, and the result looked like a silently broken image. Read-mode worked, edit-mode didn't.
Out of scope (separate issue filed)
Page imports / pasted HTML with relative
<img src=\"../_images/…\">references — these don't resolve to any backend endpoint and the dev server's SPA-fallback masks the 404. Tracking separately.Test plan
npm run typecheck -w frontend)Editor.test.tsx,ArticleViewer.test.tsx,use-authenticated-src.test.ts<img>rendered withnaturalWidth: 1,editor.getHTML()output<img src=\"/api/attachments/1/…\">(canonical URL, noblob:leaked into persisted HTML)🤖 Generated with Claude Code