mirror of
https://github.com/multica-ai/multica.git
synced 2026-06-16 19:29:26 +02:00
fix(attachments): server-driven markdown_url + legacy compat (MUL-3192) (#3991)
Comment / issue / chat images uploaded inside the Desktop app rendered
as the broken-image fallback. The editor was persisting a site-relative
`/api/attachments/<id>/download` URL into markdown — that path only
resolves when the document origin proxies /api to the API host (apps/web
via Next.js rewrite). On Electron's file:// origin it never resolved.
Per GPT-Boy's plan, move the durable-URL choice from the client to the
server so the persisted shape is correct regardless of which client
performed the upload.
Server:
- AttachmentResponse gains a markdown_url field, computed by
buildMarkdownURL from the deployment policy:
• storage URL is already absolute + unsigned (public CDN, S3 public
bucket, LocalStorage with MULTICA_LOCAL_UPLOAD_BASE_URL on https) →
use it verbatim;
• CloudFront-signed mode → never expose the raw S3 URL (private
bucket); return cfg.PublicURL + /api/attachments/<id>/download so
the server can re-sign on every request;
• LocalStorage relative + cfg.PublicURL set → same prefixed API
endpoint;
• cfg.PublicURL unset → fall back to site-relative path so web's
Next.js rewrite still works.
- isDurablePublicURL helper rejects URLs carrying CloudFront / S3
signature query params, so a freshly-signed download_url can never
leak into persistence — the original MUL-3130 bug stays closed.
Frontend:
- Attachment type + AttachmentResponseSchema (and apps/mobile mirror)
carry markdown_url. Schema lenient-defaults to '' so a backend old
enough to predate this field doesn't break clients.
- useFileUpload picks markdownLink with three-layer fallback:
(1) att.markdown_url (modern server),
(2) attachmentDownloadPath(att.id) — legacy site-relative shape,
retained for backends old enough to omit markdown_url,
(3) att.url — no-workspace avatar branch with no attachment-row id.
- attachment.tsx keeps the relative→absolute absolutize pass, but
reframed as the legacy-compat fallback for already-persisted
/api/attachments/<id>/download or /uploads/<key> URLs in old
bodies. New content writes absolute URLs and skips this path.
- ContentEditor still tracks freshly-uploaded records into
AttachmentDownloadProvider so Quick Create's editor can swap the URL
via the resolver during the same session even before the server-side
binding lands.
Tests:
- server/internal/handler/file_test.go: 5 new buildMarkdownURL matrix
tests (public CDN passthrough, CloudFront-signed swap, relative
prefixing, PublicURL unset fallback, trailing-slash strip) + 15
table-driven isDurablePublicURL cases.
- packages/core/hooks/use-file-upload.test.ts: new file, 4 cases
covering modern server / legacy server / no-id avatar / oversize.
- packages/views/editor/attachment.test.tsx + content-editor.test.tsx:
10 cases for the absolutize matrix and in-session attachment merge.
- 6 existing test fixtures updated to include markdown_url.
Verification: 1236 @multica/views tests pass; 514 @multica/core tests
pass (4 new); server handler package tests pass for the new matrix
plus all pre-existing TestAttachmentToResponse* and TestDownload*
cases. Typecheck green for views/core/web/desktop. Lint clean on
touched files.
Quick Create attachment_ids binding (orphaned attachment relationship
on the resulting issue) is a follow-up — it requires a new --attachment-id
CLI flag and daemon prompt-template work and is intentionally scoped
out of this PR.
Refs: MUL-3192
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
Multica Eve
@@ -58,6 +58,7 @@ export const AttachmentSchema: z.ZodType<Attachment> = z.object({
|
||||
filename: z.string(),
|
||||
url: z.string(),
|
||||
download_url: z.string().default(""),
|
||||
markdown_url: z.string().default(""),
|
||||
content_type: z.string().default(""),
|
||||
size_bytes: z.number().default(0),
|
||||
created_at: z.string().default(""),
|
||||
|
||||
@@ -84,10 +84,18 @@ const AttachmentSchema = z.object({
|
||||
// in a new tab — `download_url` and `url` — must be strings, otherwise we'd
|
||||
// happily `window.open(undefined)`. `filename` gates the toast/title and is
|
||||
// also enforced so a missing value falls back to the empty record below.
|
||||
//
|
||||
// `markdown_url` is parsed lenient: a server old enough to predate
|
||||
// MUL-3192 omits the field, in which case the schema defaults it to "".
|
||||
// Callers that need to persist a URL into markdown should go through the
|
||||
// `useFileUpload` helper (which falls back to the legacy
|
||||
// `attachmentDownloadPath` shape when `markdown_url` is empty), so the
|
||||
// empty-string default does not silently break any persistence path.
|
||||
export const AttachmentResponseSchema = z.object({
|
||||
id: z.string(),
|
||||
url: z.string(),
|
||||
download_url: z.string(),
|
||||
markdown_url: z.string().optional().default(""),
|
||||
filename: z.string(),
|
||||
chat_session_id: z.string().nullable().optional(),
|
||||
chat_message_id: z.string().nullable().optional(),
|
||||
@@ -105,6 +113,7 @@ export const EMPTY_ATTACHMENT: Attachment = {
|
||||
filename: "",
|
||||
url: "",
|
||||
download_url: "",
|
||||
markdown_url: "",
|
||||
content_type: "",
|
||||
size_bytes: 0,
|
||||
created_at: "",
|
||||
|
||||
102
packages/core/hooks/use-file-upload.test.ts
Normal file
102
packages/core/hooks/use-file-upload.test.ts
Normal file
@@ -0,0 +1,102 @@
|
||||
/**
|
||||
* @vitest-environment jsdom
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { renderHook, act } from "@testing-library/react";
|
||||
import type { ApiClient } from "../api/client";
|
||||
import type { Attachment } from "../types";
|
||||
import { useFileUpload, type UploadResult } from "./use-file-upload";
|
||||
|
||||
// MUL-3192 — verifies that the URL chosen for markdown persistence is
|
||||
// a durable, server-supplied absolute URL when available, and falls
|
||||
// through to the legacy site-relative shape only when the server didn't
|
||||
// populate `markdown_url` (older deployments) or when there's no
|
||||
// attachment-row id at all (no-workspace avatar branch).
|
||||
|
||||
function makeAttachment(overrides: Partial<Attachment> = {}): Attachment {
|
||||
return {
|
||||
id: "att-1",
|
||||
workspace_id: "ws-1",
|
||||
issue_id: null,
|
||||
comment_id: null,
|
||||
chat_session_id: null,
|
||||
chat_message_id: null,
|
||||
uploader_type: "member",
|
||||
uploader_id: "u-1",
|
||||
filename: "shot.png",
|
||||
url: "/uploads/ws-1/shot.png",
|
||||
download_url: "/api/attachments/att-1/download",
|
||||
markdown_url: "https://api.multica.test/api/attachments/att-1/download",
|
||||
content_type: "image/png",
|
||||
size_bytes: 1,
|
||||
created_at: "2026-06-10T00:00:00Z",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeApi(att: Attachment): ApiClient {
|
||||
return {
|
||||
uploadFile: vi.fn().mockResolvedValue(att),
|
||||
} as unknown as ApiClient;
|
||||
}
|
||||
|
||||
async function runUpload(api: ApiClient): Promise<UploadResult | null> {
|
||||
const { result } = renderHook(() => useFileUpload(api));
|
||||
let upload: UploadResult | null = null;
|
||||
await act(async () => {
|
||||
upload = await result.current.upload(
|
||||
new File(["data"], "shot.png", { type: "image/png" }),
|
||||
);
|
||||
});
|
||||
return upload;
|
||||
}
|
||||
|
||||
describe("useFileUpload — markdownLink picks the durable URL with three-layer fallback", () => {
|
||||
it("prefers att.markdown_url when the server populates it (modern deployment)", async () => {
|
||||
const att = makeAttachment({
|
||||
markdown_url: "https://cdn.multica.test/uploads/abc.png",
|
||||
});
|
||||
const upload = await runUpload(makeApi(att));
|
||||
expect(upload?.markdownLink).toBe("https://cdn.multica.test/uploads/abc.png");
|
||||
// `link` keeps its legacy semantics — same as att.url, used by avatar /
|
||||
// logo callers that persist into long-lived fields.
|
||||
expect(upload?.link).toBe(att.url);
|
||||
});
|
||||
|
||||
it("falls back to the site-relative download path when the server omitted markdown_url (legacy server)", async () => {
|
||||
const att = makeAttachment({ markdown_url: "" });
|
||||
const upload = await runUpload(makeApi(att));
|
||||
// On web this is fine — Next.js rewrite proxies /api/* to the API
|
||||
// host server-side. Non-web surfaces hit attachment.tsx's legacy
|
||||
// absolutize fallback at render time.
|
||||
expect(upload?.markdownLink).toBe("/api/attachments/att-1/download");
|
||||
});
|
||||
|
||||
it("falls back to att.url when there's no attachment-row id (no-workspace avatar branch)", async () => {
|
||||
const att = makeAttachment({
|
||||
id: "",
|
||||
markdown_url: "",
|
||||
url: "https://cdn.multica.test/avatars/u-1.png",
|
||||
});
|
||||
const upload = await runUpload(makeApi(att));
|
||||
expect(upload?.markdownLink).toBe("https://cdn.multica.test/avatars/u-1.png");
|
||||
expect(upload?.link).toBe("https://cdn.multica.test/avatars/u-1.png");
|
||||
});
|
||||
|
||||
it("rejects oversize files before hitting the network", async () => {
|
||||
const att = makeAttachment();
|
||||
const api = makeApi(att);
|
||||
const huge = new File([new ArrayBuffer(1)], "big.bin", {
|
||||
type: "application/octet-stream",
|
||||
});
|
||||
Object.defineProperty(huge, "size", { value: 200 * 1024 * 1024 });
|
||||
|
||||
const { result } = renderHook(() => useFileUpload(api));
|
||||
await expect(
|
||||
act(async () => {
|
||||
await result.current.upload(huge);
|
||||
}),
|
||||
).rejects.toThrow(/100 MB/);
|
||||
expect(api.uploadFile as ReturnType<typeof vi.fn>).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -22,22 +22,33 @@ import { MAX_FILE_SIZE } from "../constants/upload";
|
||||
// so avatar uploads do not get rerouted through
|
||||
// the workspace-membership-gated download endpoint.
|
||||
//
|
||||
// `markdownLink` — the stable per-attachment URL
|
||||
// `/api/attachments/<id>/download`. Safe to embed
|
||||
// in markdown bodies that outlive the current
|
||||
// session because the server self-resolves the
|
||||
// workspace from the attachment row and re-signs /
|
||||
// proxies on every request. Empty when the upload
|
||||
// has no attachment-row id (the no-workspace
|
||||
// avatar branch of UploadFile, which falls back
|
||||
// to `link`). Only the editor's markdown-persisting
|
||||
// flow should use it; everything else uses `link`.
|
||||
// `markdownLink` — the URL the editor writes into markdown bodies.
|
||||
// Source: `att.markdown_url` from the server, which
|
||||
// `buildMarkdownURL` picks per deployment policy
|
||||
// (public CDN durable URL, or
|
||||
// `<MULTICA_PUBLIC_URL>/api/attachments/<id>/download`).
|
||||
// The contract is "absolute, no TTL, loads natively
|
||||
// on every client" — that's what fixes the Desktop /
|
||||
// mobile-webview regression where a site-relative
|
||||
// `/api/attachments/<id>/download` couldn't resolve
|
||||
// against `file://` (MUL-3192).
|
||||
//
|
||||
// Falls back through two layers when the server
|
||||
// didn't populate `markdown_url`:
|
||||
// 1. `attachmentDownloadPath(att.id)` — the legacy
|
||||
// site-relative shape. Works on web (Next
|
||||
// rewrite proxies /api/* to the API host) and
|
||||
// is what older comments persist; render
|
||||
// surfaces handle the absolutize for non-web
|
||||
// clients via attachment.tsx's legacy compat.
|
||||
// 2. `att.url` — the no-workspace avatar branch
|
||||
// where there's no attachment-row id at all.
|
||||
//
|
||||
// MUL-3130 introduced the persisted-image regression by collapsing
|
||||
// these two semantics into a single `link` field. Keeping them
|
||||
// separate is what lets new comments embed a stable URL while
|
||||
// avatar / logo callers continue to write `att.url` into long-lived
|
||||
// fields without picking up workspace-membership gating.
|
||||
// these two semantics into a single `link` field; MUL-3192 followed up
|
||||
// by moving the durable-URL choice from the client to the server so
|
||||
// the persisted shape is correct for the deployment without the client
|
||||
// having to know whether it's running on web / desktop / mobile.
|
||||
export type UploadResult = Attachment & {
|
||||
link: string;
|
||||
markdownLink: string;
|
||||
@@ -49,6 +60,26 @@ export interface UploadContext {
|
||||
chatSessionId?: string;
|
||||
}
|
||||
|
||||
// pickMarkdownLink chooses the URL the editor will write into markdown.
|
||||
//
|
||||
// Order:
|
||||
// 1. `att.markdown_url` — server-provided durable URL. This is the
|
||||
// modern contract introduced in MUL-3192; the server (`buildMarkdownURL`)
|
||||
// decides whether to emit a public CDN URL or an absolute API
|
||||
// endpoint pinned to `MULTICA_PUBLIC_URL` based on the deployment.
|
||||
// 2. `attachmentDownloadPath(att.id)` — site-relative legacy shape,
|
||||
// retained for compatibility with backends old enough to predate
|
||||
// MUL-3192. Web's Next rewrite makes this load; desktop / mobile
|
||||
// surfaces hit the attachment.tsx legacy-absolutize fallback.
|
||||
// 3. `att.url` — no attachment-row id (the no-workspace avatar branch
|
||||
// of UploadFile). Markdown callers fall back to whatever storage
|
||||
// backend produced for the upload; persistence is on the caller.
|
||||
function pickMarkdownLink(att: Attachment): string {
|
||||
if (att.markdown_url) return att.markdown_url;
|
||||
if (att.id) return attachmentDownloadPath(att.id);
|
||||
return att.url;
|
||||
}
|
||||
|
||||
export function useFileUpload(
|
||||
api: ApiClient,
|
||||
onError?: (error: Error) => void,
|
||||
@@ -68,14 +99,7 @@ export function useFileUpload(
|
||||
commentId: ctx?.commentId,
|
||||
chatSessionId: ctx?.chatSessionId,
|
||||
});
|
||||
// Avatar / no-workspace uploads come back without an
|
||||
// attachment-row id (UploadFile's no-workspace branch returns
|
||||
// {id, url, filename} that fails the AttachmentResponseSchema
|
||||
// and degrades to the empty record). In that case the stable
|
||||
// markdown URL doesn't exist — markdown callers fall back to
|
||||
// `link` which mirrors `att.url`.
|
||||
const markdownLink = att.id ? attachmentDownloadPath(att.id) : att.url;
|
||||
return { ...att, link: att.url, markdownLink };
|
||||
return { ...att, link: att.url, markdownLink: pickMarkdownLink(att) };
|
||||
} finally {
|
||||
setUploading(false);
|
||||
}
|
||||
|
||||
@@ -320,6 +320,7 @@ describe("onIssueDeleted", () => {
|
||||
filename: "evidence.png",
|
||||
url: "s3://bucket/evidence.png",
|
||||
download_url: "https://example.test/evidence.png",
|
||||
markdown_url: "https://example.test/api/attachments/att-1/download",
|
||||
content_type: "image/png",
|
||||
size_bytes: 1,
|
||||
created_at: "2025-01-01T00:00:00Z",
|
||||
|
||||
@@ -10,6 +10,31 @@ export interface Attachment {
|
||||
filename: string;
|
||||
url: string;
|
||||
download_url: string;
|
||||
/**
|
||||
* Durable URL the client persists into markdown bodies.
|
||||
*
|
||||
* The server (`buildMarkdownURL` in server/internal/handler/file.go)
|
||||
* computes this per deployment policy:
|
||||
* - public CDN path when storage URL is itself absolute and unsigned;
|
||||
* - otherwise `<MULTICA_PUBLIC_URL>/api/attachments/<id>/download`,
|
||||
* which the server self-resigns / proxies on every request.
|
||||
*
|
||||
* Distinct from `url` (raw storage URL — may be private / site-relative)
|
||||
* and `download_url` (this-response click-time URL — may be a short-lived
|
||||
* CloudFront / S3 signed URL with a TTL). `markdown_url` is contracted
|
||||
* to be safe to embed in markdown bodies that outlive the current
|
||||
* session and to load as a native browser resource fetch on every
|
||||
* supported client (web / desktop / mobile webview). MUL-3192.
|
||||
*
|
||||
* Empty when the response was produced by a server old enough to
|
||||
* predate this field, or by an upload path that did not produce a
|
||||
* persisted attachment row (e.g. the no-workspace avatar branch).
|
||||
* Frontend callers that need to embed a URL into markdown should use
|
||||
* the helper in `useFileUpload` rather than reading this field
|
||||
* directly so the legacy fallbacks (download path / `att.url`) stay
|
||||
* centralized.
|
||||
*/
|
||||
markdown_url: string;
|
||||
content_type: string;
|
||||
size_bytes: number;
|
||||
created_at: string;
|
||||
|
||||
@@ -17,6 +17,7 @@ function makeUpload(overrides: Partial<UploadResult> & { id: string; link: strin
|
||||
uploader_id: "user-1",
|
||||
url: overrides.link,
|
||||
download_url: overrides.link,
|
||||
markdown_url: overrides.link,
|
||||
content_type: "image/png",
|
||||
size_bytes: 1,
|
||||
created_at: new Date(0).toISOString(),
|
||||
|
||||
@@ -143,6 +143,7 @@ function makeAttachment(overrides: Partial<Attachment> = {}): Attachment {
|
||||
filename: "test.bin",
|
||||
url: "https://cdn.example.test/att-1.bin",
|
||||
download_url: "https://cdn.example.test/att-1.bin?Signature=s",
|
||||
markdown_url: "https://cdn.example.test/api/attachments/att-1/download",
|
||||
content_type: "application/octet-stream",
|
||||
size_bytes: 0,
|
||||
created_at: "2026-05-13T00:00:00Z",
|
||||
|
||||
@@ -6,18 +6,27 @@ import type { Attachment as AttachmentRecord } from "@multica/core/types";
|
||||
|
||||
const {
|
||||
getAttachmentTextContentMock,
|
||||
getBaseUrlMock,
|
||||
downloadMock,
|
||||
openExternalMock,
|
||||
openByUrlMock,
|
||||
} = vi.hoisted(() => ({
|
||||
getAttachmentTextContentMock: vi.fn(),
|
||||
// Default: empty base URL so existing tests render site-relative URLs
|
||||
// through the proxy (i.e. exactly the way the web app behaves). The
|
||||
// absolutize-specific suite below overrides this to simulate Desktop /
|
||||
// mobile webview, where the renderer's origin does NOT proxy /api.
|
||||
getBaseUrlMock: vi.fn(() => ""),
|
||||
downloadMock: vi.fn(),
|
||||
openExternalMock: vi.fn(),
|
||||
openByUrlMock: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("@multica/core/api", () => ({
|
||||
api: { getAttachmentTextContent: getAttachmentTextContentMock },
|
||||
api: {
|
||||
getAttachmentTextContent: getAttachmentTextContentMock,
|
||||
getBaseUrl: getBaseUrlMock,
|
||||
},
|
||||
PreviewTooLargeError: class extends Error {},
|
||||
PreviewUnsupportedError: class extends Error {},
|
||||
}));
|
||||
@@ -107,6 +116,7 @@ function makeRecord(overrides: Partial<AttachmentRecord> = {}): AttachmentRecord
|
||||
filename: "shot.png",
|
||||
url: "https://cdn.example.test/att-1.png",
|
||||
download_url: "https://cdn.example.test/att-1.png?Signature=s",
|
||||
markdown_url: "https://cdn.example.test/api/attachments/att-1/download",
|
||||
content_type: "image/png",
|
||||
size_bytes: 1024,
|
||||
created_at: "2026-05-13T00:00:00Z",
|
||||
@@ -124,6 +134,10 @@ function renderWithQuery(ui: ReactElement) {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
resolverState.attachments = [];
|
||||
// Default to "no proxy override" — site-relative URLs stay as-is, mirroring
|
||||
// the web app's same-origin proxy. Tests that simulate Desktop / mobile
|
||||
// webview override per-case via getBaseUrlMock.mockReturnValue(...).
|
||||
getBaseUrlMock.mockReturnValue("");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -240,28 +254,41 @@ describe("Attachment — image dispatch", () => {
|
||||
expect(screen.queryByTitle("Download")).toBeNull();
|
||||
});
|
||||
|
||||
it("stable /api/attachments/<id>/download download_url falls through to the freshly-signed record.url for token-mode loadability (MUL-3130)", () => {
|
||||
// Pinned behavior: when an attachment record carries the default
|
||||
// proxy-mode download_url (a bare /api/attachments/<id>/download
|
||||
// path, which does NOT load as a native <img>/<video> src in
|
||||
// token-mode clients because they can't attach Authorization
|
||||
// headers), the renderer must fall through to record.url. On the
|
||||
// LocalStorage backend record.url carries a freshly-minted
|
||||
// /uploads/<key>?exp&sig query whose signature IS the auth, so
|
||||
// the image loads regardless of how the client is authenticated.
|
||||
// pickInlineMediaURL implements this fallback (attachment.tsx).
|
||||
const signedStorageURL =
|
||||
"/uploads/workspaces/ws-1/freshly-signed.png?exp=99&sig=fresh";
|
||||
it("stable /api/attachments/<id>/download download_url falls through to the durable record.markdown_url for native <img> loadability (MUL-3192)", () => {
|
||||
// After MUL-3192, pickInlineMediaURL prefers `record.markdown_url`
|
||||
// (the server-chosen durable URL — public CDN passthrough or absolute
|
||||
// API endpoint) over the raw `record.url` (which can be a private
|
||||
// bucket URL on S3 / R2 / MinIO presign deployments). The signed
|
||||
// download_url stays the highest-priority pick when present, but the
|
||||
// unsigned `/api/attachments/<id>/download` shape now defers to
|
||||
// markdown_url instead of record.url.
|
||||
const att = makeRecord({
|
||||
url: signedStorageURL,
|
||||
// bare API path — no signature query, so pickInlineMediaURL
|
||||
// skips download_url and uses record.url.
|
||||
// Raw private-bucket URL — must NOT be the rendered src.
|
||||
url: "https://prod.s3.amazonaws.com/key.png",
|
||||
markdown_url: "https://api.multica.test/api/attachments/att-1/download",
|
||||
// bare API path on download_url — no signature query.
|
||||
download_url: "/api/attachments/att-1/download",
|
||||
});
|
||||
renderWithQuery(<Attachment attachment={{ kind: "record", attachment: att }} />);
|
||||
const img = document.querySelector("img");
|
||||
expect(img?.getAttribute("src")).toBe(signedStorageURL);
|
||||
expect(img?.getAttribute("src")).not.toContain("/api/attachments/");
|
||||
expect(img?.getAttribute("src")).toBe(
|
||||
"https://api.multica.test/api/attachments/att-1/download",
|
||||
);
|
||||
expect(img?.getAttribute("src")).not.toContain("prod.s3.amazonaws.com");
|
||||
});
|
||||
|
||||
it("legacy backend (no markdown_url on record) still falls back to record.url", () => {
|
||||
// A backend old enough to predate MUL-3192 omits markdown_url; the
|
||||
// fallback chain bottoms out on record.url, preserving render
|
||||
// behaviour for legacy attachment metadata in the cache.
|
||||
const att = makeRecord({
|
||||
url: "https://cdn.example.test/legacy.png",
|
||||
markdown_url: "",
|
||||
download_url: "/api/attachments/att-1/download",
|
||||
});
|
||||
renderWithQuery(<Attachment attachment={{ kind: "record", attachment: att }} />);
|
||||
const img = document.querySelector("img");
|
||||
expect(img?.getAttribute("src")).toBe("https://cdn.example.test/legacy.png");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -330,3 +357,120 @@ describe("Attachment — file-card dispatch", () => {
|
||||
expect(screen.queryByTitle("Download")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// MUL-3192 — Desktop quick-create: site-relative `/api/attachments/<id>/
|
||||
// download` and `/uploads/<key>` URLs only resolve in environments where the
|
||||
// document origin proxies them to the API host (web via Next.js rewrites).
|
||||
// In Electron desktop the renderer origin is `file://`, so the same path
|
||||
// can't load. The Attachment renderer runs the picked URL through an
|
||||
// absolutize pass that prefixes `apiBaseUrl` when one is configured.
|
||||
describe("Attachment — absolutize site-relative URLs (MUL-3192)", () => {
|
||||
it("prefixes site-relative /api/attachments/<id>/download with apiBaseUrl in Desktop-like environments", () => {
|
||||
getBaseUrlMock.mockReturnValue("https://api.example.test");
|
||||
renderWithQuery(
|
||||
<Attachment
|
||||
attachment={{
|
||||
kind: "url",
|
||||
url: "/api/attachments/abc-1/download",
|
||||
filename: "shot.png",
|
||||
forceKind: "image",
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
const img = document.querySelector("img");
|
||||
expect(img?.getAttribute("src")).toBe(
|
||||
"https://api.example.test/api/attachments/abc-1/download",
|
||||
);
|
||||
});
|
||||
|
||||
it("absolutizes the legacy site-relative /uploads/<key> when record.markdown_url is empty (legacy backend)", () => {
|
||||
// Legacy compat: a backend old enough to predate MUL-3192 omits
|
||||
// markdown_url, so pickInlineMediaURL falls through to record.url.
|
||||
// For LocalStorage with no LOCAL_UPLOAD_BASE_URL configured that's
|
||||
// a site-relative `/uploads/<key>` — the absolutize pass at the
|
||||
// renderer's edge prefixes the apiBaseUrl so Desktop's file:// origin
|
||||
// doesn't resolve it to file:///uploads/...
|
||||
getBaseUrlMock.mockReturnValue("https://api.example.test");
|
||||
const att = makeRecord({
|
||||
url: "/uploads/ws-1/abc.png",
|
||||
markdown_url: "",
|
||||
download_url: "/api/attachments/att-1/download",
|
||||
});
|
||||
renderWithQuery(<Attachment attachment={{ kind: "record", attachment: att }} />);
|
||||
const img = document.querySelector("img");
|
||||
expect(img?.getAttribute("src")).toBe(
|
||||
"https://api.example.test/uploads/ws-1/abc.png",
|
||||
);
|
||||
});
|
||||
|
||||
it("strips a trailing slash on apiBaseUrl so the prefixed URL has exactly one separator", () => {
|
||||
getBaseUrlMock.mockReturnValue("https://api.example.test/");
|
||||
renderWithQuery(
|
||||
<Attachment
|
||||
attachment={{
|
||||
kind: "url",
|
||||
url: "/api/attachments/abc-2/download",
|
||||
filename: "shot.png",
|
||||
forceKind: "image",
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
const img = document.querySelector("img");
|
||||
expect(img?.getAttribute("src")).toBe(
|
||||
"https://api.example.test/api/attachments/abc-2/download",
|
||||
);
|
||||
});
|
||||
|
||||
it("leaves absolute https URLs untouched even when apiBaseUrl is set", () => {
|
||||
getBaseUrlMock.mockReturnValue("https://api.example.test");
|
||||
renderWithQuery(
|
||||
<Attachment
|
||||
attachment={{
|
||||
kind: "url",
|
||||
url: "https://cdn.other.test/foo.png",
|
||||
filename: "foo.png",
|
||||
forceKind: "image",
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
const img = document.querySelector("img");
|
||||
expect(img?.getAttribute("src")).toBe("https://cdn.other.test/foo.png");
|
||||
});
|
||||
|
||||
it("leaves blob: URLs untouched (in-flight upload preview)", () => {
|
||||
getBaseUrlMock.mockReturnValue("https://api.example.test");
|
||||
renderWithQuery(
|
||||
<Attachment
|
||||
attachment={{
|
||||
kind: "url",
|
||||
url: "blob:https://app.local/abc-123",
|
||||
filename: "in-flight.png",
|
||||
forceKind: "image",
|
||||
// uploading=true would hide the toolbar, but the src normalization
|
||||
// path runs regardless — keep it false so we still mount the <img>.
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
const img = document.querySelector("img");
|
||||
expect(img?.getAttribute("src")).toBe("blob:https://app.local/abc-123");
|
||||
});
|
||||
|
||||
it("is a no-op when apiBaseUrl is empty (web app same-origin proxy)", () => {
|
||||
getBaseUrlMock.mockReturnValue("");
|
||||
renderWithQuery(
|
||||
<Attachment
|
||||
attachment={{
|
||||
kind: "url",
|
||||
url: "/api/attachments/abc-3/download",
|
||||
filename: "shot.png",
|
||||
forceKind: "image",
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
const img = document.querySelector("img");
|
||||
// Persisted markdown URL stays site-relative — Next.js rewrite proxies
|
||||
// /api/* to the API host, so the relative path loads through the same
|
||||
// origin as the rendered HTML.
|
||||
expect(img?.getAttribute("src")).toBe("/api/attachments/abc-3/download");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -32,6 +32,7 @@ import {
|
||||
import { toast } from "sonner";
|
||||
import { cn } from "@multica/ui/lib/utils";
|
||||
import { copyText } from "@multica/ui/lib/clipboard";
|
||||
import { api } from "@multica/core/api";
|
||||
import type { Attachment as AttachmentRecord } from "@multica/core/types";
|
||||
import { useT } from "../i18n";
|
||||
import { useAttachmentDownloadResolver } from "./attachment-download-context";
|
||||
@@ -110,7 +111,9 @@ function normalize(
|
||||
return {
|
||||
filename: input.attachment.filename,
|
||||
contentType: input.attachment.content_type,
|
||||
url: pickInlineMediaURL(input.attachment, input.attachment.url),
|
||||
url: absolutizeMediaURL(
|
||||
pickInlineMediaURL(input.attachment, input.attachment.url),
|
||||
),
|
||||
attachmentId: input.attachment.id,
|
||||
record: input.attachment,
|
||||
uploading: false,
|
||||
@@ -131,7 +134,19 @@ function normalize(
|
||||
// picks the URL with embedded credentials when one exists and
|
||||
// falls back to the input URL otherwise so legacy / unresolved
|
||||
// markdown stays on its existing path. See MUL-3130 review.
|
||||
url: record ? pickInlineMediaURL(record, input.url) : input.url,
|
||||
//
|
||||
// After picking the credential-bearing URL we run the absolutize
|
||||
// pass so a site-relative `/api/attachments/...` or `/uploads/...`
|
||||
// path becomes a proper origin-bearing URL when the renderer's
|
||||
// document origin doesn't proxy /api or /uploads to the API host
|
||||
// (Electron desktop, mobile webview). Web with a same-origin
|
||||
// proxy keeps `apiBaseUrl=""` and the helper is a no-op there.
|
||||
// See MUL-3192 — quick-create modal regressed because the freshly-
|
||||
// uploaded image URL stayed site-relative and Electron's renderer
|
||||
// origin (file://) couldn't load it.
|
||||
url: absolutizeMediaURL(
|
||||
record ? pickInlineMediaURL(record, input.url) : input.url,
|
||||
),
|
||||
attachmentId: record?.id,
|
||||
record,
|
||||
uploading: !!input.uploading,
|
||||
@@ -140,36 +155,80 @@ function normalize(
|
||||
};
|
||||
}
|
||||
|
||||
// absolutizeMediaURL is the legacy-compat fallback for old markdown bodies
|
||||
// that persisted a site-relative `/api/attachments/<id>/download` or
|
||||
// `/uploads/<key>` URL.
|
||||
//
|
||||
// The current (post-MUL-3192) write path persists an absolute URL chosen
|
||||
// server-side by `buildMarkdownURL` (see server/internal/handler/file.go),
|
||||
// so new content already loads natively on every client. This helper only
|
||||
// matters for content written BEFORE MUL-3192 — those bodies still carry
|
||||
// the old relative shape, and rendering them on a surface whose document
|
||||
// origin is NOT the API host (Electron desktop, mobile webview) needs the
|
||||
// API base URL pinned in at render time.
|
||||
//
|
||||
// On web, `api.getBaseUrl()` is empty (the Next.js rewrite proxies /api/*
|
||||
// to the API host server-side), so this is a no-op there.
|
||||
//
|
||||
// http(s)://, blob:, and data: URLs are passed through unchanged — they
|
||||
// already carry their own origin.
|
||||
function absolutizeMediaURL(rawUrl: string): string {
|
||||
if (!rawUrl) return rawUrl;
|
||||
if (/^https?:\/\//i.test(rawUrl)) return rawUrl;
|
||||
if (/^blob:/i.test(rawUrl) || /^data:/i.test(rawUrl)) return rawUrl;
|
||||
if (!rawUrl.startsWith("/")) return rawUrl;
|
||||
// The api singleton is a Proxy that returns `undefined` for any property
|
||||
// access before `setApiInstance()` runs (boot ordering, SSR). Optional
|
||||
// chaining lets us cope with that without throwing — pre-init renders
|
||||
// simply keep the site-relative path.
|
||||
const baseUrl = (api.getBaseUrl?.() ?? "").replace(/\/+$/, "");
|
||||
if (!baseUrl) return rawUrl;
|
||||
return `${baseUrl}${rawUrl}`;
|
||||
}
|
||||
|
||||
// pickInlineMediaURL returns the URL most likely to load successfully
|
||||
// inside a native <img>/<video>/<iframe> resource fetch — i.e. without
|
||||
// the calling client attaching an Authorization header.
|
||||
//
|
||||
// The metadata response from the backend offers two URL fields per
|
||||
// attachment row:
|
||||
// The metadata response carries three URL fields per attachment row,
|
||||
// each with a different lifetime / accessibility:
|
||||
//
|
||||
// - `record.url` — for LocalStorage this is a freshly-signed
|
||||
// `/uploads/<key>?exp=<unix>&sig=<HMAC>`
|
||||
// URL whose query string IS the auth (works
|
||||
// for token-mode <img> loads). For S3/
|
||||
// CloudFront it's the raw stored URL with
|
||||
// no signature.
|
||||
// - `record.download_url` — `/api/attachments/<id>/download` in the
|
||||
// default proxy/presign mode (requires
|
||||
// cookie or Authorization header — does
|
||||
// NOT work as a native resource load for
|
||||
// token-mode clients). In CloudFront mode
|
||||
// this is replaced server-side with a
|
||||
// CloudFront-signed URL that DOES work as
|
||||
// a native <img> src.
|
||||
// - `record.download_url` — this-response click-time URL. In
|
||||
// CloudFront-signed mode this is the
|
||||
// signed redirect (works as a native img
|
||||
// src for the duration of the TTL); in
|
||||
// other modes it's the bare API endpoint
|
||||
// (`/api/attachments/<id>/download`) which
|
||||
// requires per-request auth and does NOT
|
||||
// load as a native img on a non-same-site
|
||||
// origin like Desktop's file://.
|
||||
// - `record.markdown_url` — the durable URL the server picked for
|
||||
// persistence (MUL-3192 / `buildMarkdownURL`):
|
||||
// public CDN passthrough when the storage is
|
||||
// public-readable, or `MULTICA_PUBLIC_URL +
|
||||
// /api/attachments/<id>/download` for
|
||||
// private-bucket modes. Aligned with the
|
||||
// server-side policy by construction, so it
|
||||
// beats `record.url` whenever both exist.
|
||||
// - `record.url` — raw storage URL. May be private (S3 /
|
||||
// CloudFront-signed, R2, MinIO) and unable
|
||||
// to load directly. Last-resort fallback
|
||||
// for legacy responses that omit
|
||||
// `markdown_url`.
|
||||
//
|
||||
// Heuristic: when `download_url` is an absolute URL with a recognised
|
||||
// CDN signature query (`Signature` / `Expires` / `Key-Pair-Id` for
|
||||
// CloudFront, `X-Amz-Signature` / `X-Amz-Expires` for raw S3 presigns
|
||||
// that may surface here in future modes), use it. Otherwise use
|
||||
// `record.url`, which carries the LocalStorage `?exp&sig` token and is
|
||||
// the only inline-loadable URL in that backend. Falls back to the
|
||||
// input URL when neither is usable so legacy markdown links keep their
|
||||
// pre-fix behaviour.
|
||||
// Order:
|
||||
//
|
||||
// 1. Signed `download_url` — when CloudFront has minted a signed
|
||||
// redirect for THIS response, use it; the TTL means the signed URL
|
||||
// beats `markdown_url` on first paint (no extra hop through the
|
||||
// API endpoint), and the renderer doesn't persist it so the TTL is
|
||||
// not a problem.
|
||||
// 2. `record.markdown_url` — the durable, server-policy-aligned URL.
|
||||
// Beats raw `record.url` because it never points at a private
|
||||
// bucket (must-fix 2 from MUL-3192 review).
|
||||
// 3. `record.url` — legacy fallback for responses that omit
|
||||
// `markdown_url` (a backend old enough to predate MUL-3192).
|
||||
// 4. The input URL — when there's no record at all.
|
||||
function pickInlineMediaURL(record: AttachmentRecord, fallback: string): string {
|
||||
const dl = record.download_url ?? "";
|
||||
if (
|
||||
@@ -178,6 +237,7 @@ function pickInlineMediaURL(record: AttachmentRecord, fallback: string): string
|
||||
) {
|
||||
return dl;
|
||||
}
|
||||
if (record.markdown_url) return record.markdown_url;
|
||||
if (record.url) return record.url;
|
||||
return fallback;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { fireEvent, render, screen } from "@testing-library/react";
|
||||
import { act, fireEvent, render, screen } from "@testing-library/react";
|
||||
import type { Attachment } from "@multica/core/types";
|
||||
import type { UploadResult } from "@multica/core/hooks/use-file-upload";
|
||||
|
||||
const mockFocus = vi.hoisted(() => vi.fn());
|
||||
const mockSetContent = vi.hoisted(() => vi.fn());
|
||||
@@ -10,6 +12,16 @@ const editorState = vi.hoisted(() => ({
|
||||
markdown: "",
|
||||
}));
|
||||
|
||||
// Records the attachments[] prop the provider received on its most recent
|
||||
// render. Content-editor merges its `attachments` prop with in-session
|
||||
// upload results before passing them down — these tests assert that merged
|
||||
// shape lands here.
|
||||
const providerProps = vi.hoisted<{ attachments: Attachment[] | undefined }>(
|
||||
() => ({ attachments: undefined }),
|
||||
);
|
||||
|
||||
const uploadAndInsertFileMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@tanstack/react-query", () => ({
|
||||
useQueryClient: () => ({}),
|
||||
}));
|
||||
@@ -19,7 +31,7 @@ vi.mock("./extensions", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("./extensions/file-upload", () => ({
|
||||
uploadAndInsertFile: vi.fn(),
|
||||
uploadAndInsertFile: uploadAndInsertFileMock,
|
||||
}));
|
||||
|
||||
vi.mock("./utils/preprocess", () => ({
|
||||
@@ -30,6 +42,19 @@ vi.mock("./bubble-menu", () => ({
|
||||
EditorBubbleMenu: () => null,
|
||||
}));
|
||||
|
||||
vi.mock("./attachment-download-context", () => ({
|
||||
AttachmentDownloadProvider: ({
|
||||
attachments,
|
||||
children,
|
||||
}: {
|
||||
attachments?: Attachment[];
|
||||
children: React.ReactNode;
|
||||
}) => {
|
||||
providerProps.attachments = attachments;
|
||||
return <>{children}</>;
|
||||
},
|
||||
}));
|
||||
|
||||
const editorRef = vi.hoisted<{ current: unknown }>(() => ({ current: null }));
|
||||
const onCreateFired = vi.hoisted(() => ({ value: false }));
|
||||
|
||||
@@ -79,6 +104,7 @@ describe("ContentEditor", () => {
|
||||
editorState.markdown = "";
|
||||
editorRef.current = null;
|
||||
onCreateFired.value = false;
|
||||
providerProps.attachments = undefined;
|
||||
});
|
||||
|
||||
it("focuses the editor when clicking the empty container area", () => {
|
||||
@@ -185,3 +211,154 @@ describe("ContentEditor", () => {
|
||||
expect(mockSetContent).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
function makeAttachment(id: string, overrides: Partial<Attachment> = {}): Attachment {
|
||||
return {
|
||||
id,
|
||||
workspace_id: "ws-1",
|
||||
issue_id: null,
|
||||
comment_id: null,
|
||||
chat_session_id: null,
|
||||
chat_message_id: null,
|
||||
uploader_type: "member",
|
||||
uploader_id: "u-1",
|
||||
filename: `${id}.png`,
|
||||
url: `/uploads/${id}.png`,
|
||||
download_url: `/api/attachments/${id}/download`,
|
||||
markdown_url: `https://api.multica.test/api/attachments/${id}/download`,
|
||||
content_type: "image/png",
|
||||
size_bytes: 1,
|
||||
created_at: "2026-06-10T00:00:00Z",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function asUploadResult(att: Attachment): UploadResult {
|
||||
return { ...att, link: att.url, markdownLink: `/api/attachments/${att.id}/download` };
|
||||
}
|
||||
|
||||
// MUL-3192 — surfaces like the quick-create modal upload images through the
|
||||
// editor without a server-supplied `attachments` prop. Without in-session
|
||||
// tracking, the AttachmentDownloadProvider had nothing to resolve the
|
||||
// freshly-inserted /api/attachments/<id>/download URL against, so
|
||||
// Attachment.normalize() couldn't swap it for a freshly-loadable URL — the
|
||||
// <img> rendered broken on Desktop where the renderer's origin doesn't
|
||||
// proxy /api to the API host. ContentEditor now wraps onUploadFile so the
|
||||
// successful UploadResult lands in the provider as a tracked record.
|
||||
describe("ContentEditor — in-session attachment tracking (MUL-3192)", () => {
|
||||
it("seeds the AttachmentDownloadProvider with the caller-supplied attachments prop", () => {
|
||||
const att = makeAttachment("seed-1");
|
||||
render(<ContentEditor attachments={[att]} />);
|
||||
expect(providerProps.attachments).toEqual([att]);
|
||||
});
|
||||
|
||||
it("appends a successful upload result to the provider's attachments list", async () => {
|
||||
const onUploadFile = vi.fn(async (_file: File) =>
|
||||
asUploadResult(makeAttachment("uploaded-1")),
|
||||
);
|
||||
// Capture the wrapped uploader the editor hands to uploadAndInsertFile,
|
||||
// then invoke it the same way the file-upload extension would.
|
||||
let capturedHandler:
|
||||
| ((file: File) => Promise<UploadResult | null>)
|
||||
| undefined;
|
||||
uploadAndInsertFileMock.mockImplementation(
|
||||
async (_editor: unknown, file: File, handler: typeof capturedHandler) => {
|
||||
capturedHandler = handler;
|
||||
await handler?.(file);
|
||||
},
|
||||
);
|
||||
|
||||
let imperativeRef: { uploadFile: (file: File) => void } | null = null;
|
||||
render(
|
||||
<ContentEditor
|
||||
onUploadFile={onUploadFile}
|
||||
ref={(r) => {
|
||||
imperativeRef = r;
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(providerProps.attachments).toBeUndefined();
|
||||
|
||||
await act(async () => {
|
||||
imperativeRef?.uploadFile(new File(["payload"], "shot.png", { type: "image/png" }));
|
||||
});
|
||||
|
||||
// The wrapper (not the raw caller-supplied uploader) is what reaches
|
||||
// the file-upload extension — that's the layer that captures successful
|
||||
// results into the provider.
|
||||
expect(capturedHandler).toBeTypeOf("function");
|
||||
expect(capturedHandler).not.toBe(onUploadFile);
|
||||
expect(onUploadFile).toHaveBeenCalledTimes(1);
|
||||
|
||||
expect(providerProps.attachments).toHaveLength(1);
|
||||
expect(providerProps.attachments?.[0]?.id).toBe("uploaded-1");
|
||||
});
|
||||
|
||||
it("merges in-session uploads with the caller's attachments prop, preferring the prop on id collision", async () => {
|
||||
// The pre-loaded record carries a freshly-signed download_url; the
|
||||
// upload result for the same id has an older download_url. After merge
|
||||
// the provider should still expose the prop's record so the editor's
|
||||
// resolveAttachment lookup hands back the freshest data.
|
||||
const seeded = makeAttachment("shared-1", {
|
||||
download_url: "https://cdn.example/freshly-signed.png?Signature=fresh",
|
||||
});
|
||||
const collision = makeAttachment("shared-1", {
|
||||
download_url: "https://cdn.example/freshly-signed.png?Signature=stale",
|
||||
});
|
||||
const onUploadFile = vi.fn(async () => asUploadResult(collision));
|
||||
uploadAndInsertFileMock.mockImplementation(
|
||||
async (_e: unknown, file: File, handler: (f: File) => Promise<unknown>) => {
|
||||
await handler(file);
|
||||
},
|
||||
);
|
||||
|
||||
let imperativeRef: { uploadFile: (file: File) => void } | null = null;
|
||||
render(
|
||||
<ContentEditor
|
||||
attachments={[seeded]}
|
||||
onUploadFile={onUploadFile}
|
||||
ref={(r) => {
|
||||
imperativeRef = r;
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
imperativeRef?.uploadFile(new File(["x"], "shared.png", { type: "image/png" }));
|
||||
});
|
||||
|
||||
expect(providerProps.attachments).toHaveLength(1);
|
||||
expect(providerProps.attachments?.[0]?.download_url).toContain("Signature=fresh");
|
||||
});
|
||||
|
||||
it("does not append a duplicate when the same upload result returns twice (paste-then-drop the same blob)", async () => {
|
||||
const result = asUploadResult(makeAttachment("dedup-1"));
|
||||
const onUploadFile = vi.fn(async () => result);
|
||||
uploadAndInsertFileMock.mockImplementation(
|
||||
async (_e: unknown, file: File, handler: (f: File) => Promise<unknown>) => {
|
||||
await handler(file);
|
||||
},
|
||||
);
|
||||
|
||||
let imperativeRef: { uploadFile: (file: File) => void } | null = null;
|
||||
render(
|
||||
<ContentEditor
|
||||
onUploadFile={onUploadFile}
|
||||
ref={(r) => {
|
||||
imperativeRef = r;
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
imperativeRef?.uploadFile(new File(["a"], "a.png", { type: "image/png" }));
|
||||
});
|
||||
await act(async () => {
|
||||
imperativeRef?.uploadFile(new File(["b"], "b.png", { type: "image/png" }));
|
||||
});
|
||||
|
||||
expect(providerProps.attachments).toHaveLength(1);
|
||||
expect(providerProps.attachments?.[0]?.id).toBe("dedup-1");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -34,7 +34,9 @@ import {
|
||||
forwardRef,
|
||||
useEffect,
|
||||
useImperativeHandle,
|
||||
useMemo,
|
||||
useRef,
|
||||
useState,
|
||||
type MouseEvent as ReactMouseEvent,
|
||||
} from "react";
|
||||
import { useEditor, EditorContent } from "@tiptap/react";
|
||||
@@ -168,10 +170,66 @@ const ContentEditor = forwardRef<ContentEditorRef, ContentEditorProps>(
|
||||
const onUpdateRef = useRef(onUpdate);
|
||||
const onSubmitRef = useRef(onSubmit);
|
||||
const onBlurRef = useRef(onBlur);
|
||||
const onUploadFileRef = useRef(onUploadFile);
|
||||
const onUploadFileRef = useRef<
|
||||
((file: File) => Promise<UploadResult | null>) | undefined
|
||||
>(undefined);
|
||||
const mentionContextItemsRef = useRef<MentionItem[]>(mentionContextItems ?? []);
|
||||
const lastEmittedRef = useRef<string | null>(null);
|
||||
|
||||
// In-session record of attachments freshly uploaded through this editor.
|
||||
// Surfaces (like the quick-create modal) that don't have a server-supplied
|
||||
// `attachments` prop still need the AttachmentDownloadProvider to know
|
||||
// about images the user just pasted/dropped — without a record in scope,
|
||||
// Attachment.normalize() can't swap the persisted /api/attachments/<id>/
|
||||
// download URL to a freshly-loadable one, and the <img> renders broken in
|
||||
// any environment where the renderer's origin doesn't proxy /api to the
|
||||
// API host (MUL-3192, Desktop/Electron).
|
||||
const [sessionUploads, setSessionUploads] = useState<Attachment[]>([]);
|
||||
// Wrap the caller-supplied uploader so we can stash each successful result
|
||||
// in `sessionUploads`. The wrapper is rebuilt only when the underlying
|
||||
// `onUploadFile` identity changes, so the inner ref handed to Tiptap stays
|
||||
// stable across renders the way the original passthrough did.
|
||||
const wrappedOnUploadFile = useMemo(() => {
|
||||
if (!onUploadFile) return undefined;
|
||||
return async (file: File): Promise<UploadResult | null> => {
|
||||
const result = await onUploadFile(file);
|
||||
// Only track attachments that carry a persisted id — the no-workspace
|
||||
// avatar branch returns an id-less record that the resolver can't key
|
||||
// off of, and tracking it would just bloat memory without helping
|
||||
// anyone. See useFileUpload's `markdownLink` docstring for why.
|
||||
if (result?.id) {
|
||||
setSessionUploads((prev) =>
|
||||
// Deduplicate on id so a re-upload (or a paste-then-drop of the
|
||||
// same blob) doesn't create a parallel record.
|
||||
prev.some((a) => a.id === result.id) ? prev : [...prev, result],
|
||||
);
|
||||
}
|
||||
return result;
|
||||
};
|
||||
}, [onUploadFile]);
|
||||
|
||||
// Merged list fed to AttachmentDownloadProvider. Caller-supplied attachments
|
||||
// (issue / comment editors that pre-load the full attachments[] from the
|
||||
// server) take precedence — we only append session uploads the caller
|
||||
// doesn't already have, so a parent re-render that includes the same record
|
||||
// doesn't end up with two copies.
|
||||
const providerAttachments = useMemo(() => {
|
||||
if (sessionUploads.length === 0) return attachments;
|
||||
const seen = new Set<string>();
|
||||
const merged: Attachment[] = [];
|
||||
for (const a of attachments ?? []) {
|
||||
if (a.id) seen.add(a.id);
|
||||
merged.push(a);
|
||||
}
|
||||
for (const a of sessionUploads) {
|
||||
if (!seen.has(a.id)) {
|
||||
seen.add(a.id);
|
||||
merged.push(a);
|
||||
}
|
||||
}
|
||||
return merged;
|
||||
}, [attachments, sessionUploads]);
|
||||
|
||||
// Current workspace slug kept in a ref so the click handler always sees the
|
||||
// latest value without recreating the editor. Used by openLink to prefix
|
||||
// legacy /issues/... style paths that lack a workspace slug.
|
||||
@@ -183,7 +241,7 @@ const ContentEditor = forwardRef<ContentEditorRef, ContentEditorProps>(
|
||||
onUpdateRef.current = onUpdate;
|
||||
onSubmitRef.current = onSubmit;
|
||||
onBlurRef.current = onBlur;
|
||||
onUploadFileRef.current = onUploadFile;
|
||||
onUploadFileRef.current = wrappedOnUploadFile;
|
||||
mentionContextItemsRef.current = mentionContextItems ?? [];
|
||||
|
||||
const queryClient = useQueryClient();
|
||||
@@ -390,7 +448,7 @@ const ContentEditor = forwardRef<ContentEditorRef, ContentEditorProps>(
|
||||
if (!editor) return null;
|
||||
|
||||
return (
|
||||
<AttachmentDownloadProvider attachments={attachments}>
|
||||
<AttachmentDownloadProvider attachments={providerAttachments}>
|
||||
<div
|
||||
ref={wrapperRef}
|
||||
className="relative flex flex-1 min-h-full flex-col"
|
||||
|
||||
@@ -55,6 +55,7 @@ function makeUpload(
|
||||
uploader_id: "user-1",
|
||||
url: overrides.link,
|
||||
download_url: overrides.link,
|
||||
markdown_url: overrides.link,
|
||||
content_type: "image/png",
|
||||
size_bytes: 1,
|
||||
created_at: new Date(0).toISOString(),
|
||||
|
||||
@@ -98,6 +98,7 @@ const attachment: Attachment = {
|
||||
filename: "evidence.png",
|
||||
url: "s3://bucket/evidence.png",
|
||||
download_url: "https://example.test/evidence.png",
|
||||
markdown_url: "https://example.test/api/attachments/attachment-1/download",
|
||||
content_type: "image/png",
|
||||
size_bytes: 1,
|
||||
created_at: "2026-01-01T00:00:00Z",
|
||||
|
||||
@@ -65,9 +65,35 @@ type AttachmentResponse struct {
|
||||
Filename string `json:"filename"`
|
||||
URL string `json:"url"`
|
||||
DownloadURL string `json:"download_url"`
|
||||
ContentType string `json:"content_type"`
|
||||
SizeBytes int64 `json:"size_bytes"`
|
||||
CreatedAt string `json:"created_at"`
|
||||
// MarkdownURL is the durable, absolute-when-possible URL the client
|
||||
// SHOULD persist into markdown bodies (issue descriptions, comments,
|
||||
// chat messages). It is computed per deployment policy by
|
||||
// buildMarkdownURL — preferring the storage URL when it is already a
|
||||
// public, durable absolute URL (public CDN / LocalStorage with
|
||||
// MULTICA_LOCAL_UPLOAD_BASE_URL), and otherwise prefixing
|
||||
// MULTICA_PUBLIC_URL onto the stable per-attachment endpoint that the
|
||||
// server self-resigns / proxies on every request.
|
||||
//
|
||||
// Why a separate field from URL / DownloadURL:
|
||||
// - URL is the raw storage object URL — fine for avatar/logo
|
||||
// surfaces but may be private (S3 + CloudFront-signed mode) or
|
||||
// site-relative (LocalStorage with no base URL configured).
|
||||
// - DownloadURL is the URL the renderer uses for THIS response — it
|
||||
// can be a short-lived signed URL (CloudFront, S3 presign) and
|
||||
// therefore must NOT be persisted. It expires.
|
||||
// - MarkdownURL is contracted to be persistable: it never carries a
|
||||
// TTL, and on every supported deployment shape it is loadable as
|
||||
// a native browser resource fetch (no Authorization header required
|
||||
// beyond the cookies/credentials the client already has on the
|
||||
// resolved host).
|
||||
//
|
||||
// MUL-3192 — fixes the Desktop / mobile-webview regression where the
|
||||
// previous site-relative `/api/attachments/<id>/download` link only
|
||||
// resolved when the document origin proxied /api to the API host.
|
||||
MarkdownURL string `json:"markdown_url"`
|
||||
ContentType string `json:"content_type"`
|
||||
SizeBytes int64 `json:"size_bytes"`
|
||||
CreatedAt string `json:"created_at"`
|
||||
}
|
||||
|
||||
func (h *Handler) attachmentToResponse(a db.Attachment) AttachmentResponse {
|
||||
@@ -80,6 +106,7 @@ func (h *Handler) attachmentToResponse(a db.Attachment) AttachmentResponse {
|
||||
Filename: a.Filename,
|
||||
URL: a.Url,
|
||||
DownloadURL: attachmentDownloadPath(id),
|
||||
MarkdownURL: h.buildMarkdownURL(a, id),
|
||||
ContentType: a.ContentType,
|
||||
SizeBytes: a.SizeBytes,
|
||||
CreatedAt: a.CreatedAt.Time.Format("2006-01-02T15:04:05Z07:00"),
|
||||
@@ -110,6 +137,104 @@ func attachmentDownloadPath(id string) string {
|
||||
return "/api/attachments/" + id + "/download"
|
||||
}
|
||||
|
||||
// buildMarkdownURL chooses the durable URL the client persists into
|
||||
// markdown bodies. The contract is "absolute, no TTL, loadable as a native
|
||||
// browser resource fetch on every supported client" (MUL-3192).
|
||||
//
|
||||
// Decision:
|
||||
//
|
||||
// 1. Persist `a.Url` only when the deployment has signaled the storage
|
||||
// backend serves URLs publicly without per-request auth:
|
||||
// - `Storage.CdnDomain()` is non-empty (operator configured a
|
||||
// public-facing base URL — `S3_CDN_DOMAIN` for the S3 backend or
|
||||
// `LOCAL_UPLOAD_BASE_URL` for LocalStorage), AND
|
||||
// - `h.CFSigner` is nil (no per-request CloudFront signing — when
|
||||
// signing is on, the same CDN domain serves PRIVATE content via
|
||||
// time-bounded signed URLs and the raw `a.Url` is unauth-deny),
|
||||
// AND
|
||||
// - `a.Url` is itself an absolute http(s) URL with no signature
|
||||
// query — defends against legacy rows backfilled while baseURL
|
||||
// was unset, and against a freshly-signed `download_url` ever
|
||||
// leaking into `a.Url` (the original MUL-3130 bug).
|
||||
//
|
||||
// 2. Every other shape — CloudFront-signed mode, S3 presign /proxy
|
||||
// against a private bucket without a CDN domain, raw S3 / R2 /
|
||||
// MinIO, LocalStorage with no `LOCAL_UPLOAD_BASE_URL` — uses the
|
||||
// stable per-attachment endpoint that the server self-signs /
|
||||
// proxies on every request, anchored on `MULTICA_PUBLIC_URL` so the
|
||||
// persisted URL keeps working for clients that don't share the
|
||||
// document origin (Desktop / mobile webview).
|
||||
//
|
||||
// 3. Last-resort fallback (no `MULTICA_PUBLIC_URL` configured): emit
|
||||
// the site-relative path. Web's Next.js rewrite handles this; non-
|
||||
// web clients on a deployment without `PublicURL` configured were
|
||||
// already broken before MUL-3192 and stay broken here, but we
|
||||
// don't make them worse.
|
||||
func (h *Handler) buildMarkdownURL(a db.Attachment, id string) string {
|
||||
relPath := attachmentDownloadPath(id)
|
||||
publicURL := strings.TrimRight(h.cfg.PublicURL, "/")
|
||||
|
||||
if h.storageURLIsPubliclyReadable(a.Url) {
|
||||
return a.Url
|
||||
}
|
||||
|
||||
if publicURL != "" {
|
||||
return publicURL + relPath
|
||||
}
|
||||
return relPath
|
||||
}
|
||||
|
||||
// storageURLIsPubliclyReadable returns true when the deployment has signaled
|
||||
// that `a.Url` can be loaded directly by an unauthenticated native browser
|
||||
// fetch — the only case where it is safe to persist `a.Url` into a markdown
|
||||
// body that will outlive the current session.
|
||||
func (h *Handler) storageURLIsPubliclyReadable(rawURL string) bool {
|
||||
if h.Storage == nil || h.CFSigner != nil {
|
||||
// CFSigner != nil is per-request signing; the CDN domain serves
|
||||
// private content via signed URLs and `a.Url` is the raw S3 URL.
|
||||
return false
|
||||
}
|
||||
if h.Storage.CdnDomain() == "" {
|
||||
// No public-facing base URL configured — the storage's URL is
|
||||
// the raw private object URL (S3 / R2 / MinIO) or a site-relative
|
||||
// LocalStorage path that doesn't carry an origin.
|
||||
return false
|
||||
}
|
||||
return isDurablePublicURL(rawURL)
|
||||
}
|
||||
|
||||
// isDurablePublicURL is true when `rawURL` is an absolute http(s) URL that
|
||||
// is safe to persist into long-lived markdown bodies — i.e. it carries no
|
||||
// CloudFront / S3 signature query that would make it expire.
|
||||
func isDurablePublicURL(rawURL string) bool {
|
||||
if rawURL == "" {
|
||||
return false
|
||||
}
|
||||
u, err := url.Parse(rawURL)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
if u.Scheme != "http" && u.Scheme != "https" {
|
||||
return false
|
||||
}
|
||||
if u.Host == "" {
|
||||
return false
|
||||
}
|
||||
q := u.Query()
|
||||
for _, k := range []string{
|
||||
"Signature",
|
||||
"X-Amz-Signature",
|
||||
"Key-Pair-Id",
|
||||
"Expires",
|
||||
"X-Amz-Expires",
|
||||
} {
|
||||
if q.Get(k) != "" {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func normalizeAttachmentDownloadMode(raw string) (attachmentDownloadMode, bool) {
|
||||
switch attachmentDownloadMode(strings.ToLower(strings.TrimSpace(raw))) {
|
||||
case "", attachmentDownloadModeAuto:
|
||||
|
||||
@@ -85,6 +85,15 @@ func (m *mockStorage) KeyFromURL(rawURL string) string {
|
||||
return rawURL
|
||||
}
|
||||
func (m *mockStorage) CdnDomain() string { return "cdn.example.com" }
|
||||
|
||||
// mockStorageNoCdn is a mockStorage variant that returns an empty CdnDomain
|
||||
// to simulate a private S3 / R2 / MinIO deployment where the operator has
|
||||
// NOT configured a public-facing CDN domain. buildMarkdownURL must not
|
||||
// persist `a.Url` for this shape — it would write a private bucket URL
|
||||
// into markdown that no client can load.
|
||||
type mockStorageNoCdn struct{ mockStorage }
|
||||
|
||||
func (m *mockStorageNoCdn) CdnDomain() string { return "" }
|
||||
func (m *mockStorage) GetReader(_ context.Context, key string) (io.ReadCloser, error) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
@@ -958,3 +967,232 @@ func TestIsTextPreviewable(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// MUL-3192 — buildMarkdownURL must emit a durable, absolute-when-possible
|
||||
// URL that loads natively in any client (web, desktop, mobile webview).
|
||||
// `download_url` may be a short-lived signed URL and is unsafe to persist;
|
||||
// `markdown_url` is the contract for "ok to embed in markdown body".
|
||||
//
|
||||
// Matrix:
|
||||
//
|
||||
// - public CDN durable URL ............... reuse a.Url verbatim
|
||||
// - LocalStorage with PublicURL set ....... reuse a.Url (already absolute)
|
||||
// - CloudFront-signed mode ................ never reuse a.Url (raw S3),
|
||||
// prefer absolute API endpoint
|
||||
// - LocalStorage relative + PublicURL set . prefix to absolute API endpoint
|
||||
// - PublicURL unset ....................... fall back to site-relative
|
||||
// (web's Next rewrite handles it)
|
||||
// - signed URL (CloudFront-signed leaked
|
||||
// into a.Url somehow) ................... reject as durable, fall through
|
||||
// to API endpoint to avoid
|
||||
// re-opening MUL-3130
|
||||
|
||||
func TestBuildMarkdownURL_PublicCdnAbsoluteURLReusedVerbatim(t *testing.T) {
|
||||
origPublic := testHandler.cfg.PublicURL
|
||||
origSigner := testHandler.CFSigner
|
||||
origStorage := testHandler.Storage
|
||||
t.Cleanup(func() {
|
||||
testHandler.cfg.PublicURL = origPublic
|
||||
testHandler.CFSigner = origSigner
|
||||
testHandler.Storage = origStorage
|
||||
})
|
||||
testHandler.cfg.PublicURL = "https://api.multica.test"
|
||||
testHandler.CFSigner = nil
|
||||
// mockStorage.CdnDomain() returns "cdn.example.com" — that's the
|
||||
// operator-set signal that the URL host serves content publicly
|
||||
// without per-request auth. Without this, the new gate routes
|
||||
// through the API endpoint to be safe.
|
||||
testHandler.Storage = &mockStorage{}
|
||||
|
||||
id := seedAttachmentURL(t, "https://cdn.multica.test/uploads/abc.png", "abc.png", "image/png", 1)
|
||||
att, err := testHandler.Queries.GetAttachment(context.Background(), db.GetAttachmentParams{
|
||||
ID: parseUUID(id),
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetAttachment: %v", err)
|
||||
}
|
||||
|
||||
resp := testHandler.attachmentToResponse(att)
|
||||
if resp.MarkdownURL != "https://cdn.multica.test/uploads/abc.png" {
|
||||
t.Fatalf("markdown_url = %q, want raw a.Url passthrough", resp.MarkdownURL)
|
||||
}
|
||||
}
|
||||
|
||||
// MUL-3192 review must-fix 1 — `att.url` for a private S3 / R2 / MinIO
|
||||
// bucket is absolute https + unsigned but is NOT publicly readable. The
|
||||
// generic "absolute http(s) without signature" check would have wrongly
|
||||
// persisted it; the gate now also requires `Storage.CdnDomain()` to be
|
||||
// set so the operator has explicitly opted into "URLs from this storage
|
||||
// load directly".
|
||||
func TestBuildMarkdownURL_PrivateBucketWithoutCdnDomainRoutesThroughAPIEndpoint(t *testing.T) {
|
||||
origPublic := testHandler.cfg.PublicURL
|
||||
origSigner := testHandler.CFSigner
|
||||
origStorage := testHandler.Storage
|
||||
t.Cleanup(func() {
|
||||
testHandler.cfg.PublicURL = origPublic
|
||||
testHandler.CFSigner = origSigner
|
||||
testHandler.Storage = origStorage
|
||||
})
|
||||
testHandler.cfg.PublicURL = "https://api.multica.test"
|
||||
testHandler.CFSigner = nil
|
||||
testHandler.Storage = &mockStorageNoCdn{}
|
||||
|
||||
id := seedAttachmentURL(t, "https://prod.s3.amazonaws.com/key.png", "key.png", "image/png", 1)
|
||||
att, err := testHandler.Queries.GetAttachment(context.Background(), db.GetAttachmentParams{
|
||||
ID: parseUUID(id),
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetAttachment: %v", err)
|
||||
}
|
||||
|
||||
resp := testHandler.attachmentToResponse(att)
|
||||
want := "https://api.multica.test/api/attachments/" + id + "/download"
|
||||
if resp.MarkdownURL != want {
|
||||
t.Fatalf("markdown_url = %q, want absolute API endpoint %q (private bucket without explicit CDN must not persist raw S3 URL)", resp.MarkdownURL, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildMarkdownURL_CloudFrontSignedModeNeverPersistsRawStorageURL(t *testing.T) {
|
||||
origPublic := testHandler.cfg.PublicURL
|
||||
origSigner := testHandler.CFSigner
|
||||
t.Cleanup(func() {
|
||||
testHandler.cfg.PublicURL = origPublic
|
||||
testHandler.CFSigner = origSigner
|
||||
})
|
||||
testHandler.cfg.PublicURL = "https://api.multica.test"
|
||||
testHandler.CFSigner = testCloudFrontSigner(t)
|
||||
|
||||
// Raw S3 URL — private bucket, not loadable directly by clients.
|
||||
id := seedAttachmentURL(t, "https://prod.s3.amazonaws.com/key.png", "key.png", "image/png", 1)
|
||||
att, err := testHandler.Queries.GetAttachment(context.Background(), db.GetAttachmentParams{
|
||||
ID: parseUUID(id),
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetAttachment: %v", err)
|
||||
}
|
||||
|
||||
resp := testHandler.attachmentToResponse(att)
|
||||
want := "https://api.multica.test/api/attachments/" + id + "/download"
|
||||
if resp.MarkdownURL != want {
|
||||
t.Fatalf("markdown_url = %q, want absolute API endpoint %q", resp.MarkdownURL, want)
|
||||
}
|
||||
// download_url is allowed to carry a TTL (CloudFront-signed); it's NOT
|
||||
// what the client persists, but it IS what the renderer uses for this
|
||||
// response. The two are intentionally distinct.
|
||||
if resp.DownloadURL == resp.MarkdownURL {
|
||||
t.Fatalf("download_url and markdown_url must differ in CloudFront-signed mode (got identical %q)", resp.DownloadURL)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildMarkdownURL_RelativeStorageURLPrefixedWithPublicURL(t *testing.T) {
|
||||
origPublic := testHandler.cfg.PublicURL
|
||||
origSigner := testHandler.CFSigner
|
||||
t.Cleanup(func() {
|
||||
testHandler.cfg.PublicURL = origPublic
|
||||
testHandler.CFSigner = origSigner
|
||||
})
|
||||
testHandler.cfg.PublicURL = "https://api.multica.test"
|
||||
testHandler.CFSigner = nil
|
||||
|
||||
// LocalStorage without LOCAL_UPLOAD_BASE_URL stores a site-relative URL.
|
||||
id := seedAttachmentURL(t, "/uploads/abc.png", "abc.png", "image/png", 1)
|
||||
att, err := testHandler.Queries.GetAttachment(context.Background(), db.GetAttachmentParams{
|
||||
ID: parseUUID(id),
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetAttachment: %v", err)
|
||||
}
|
||||
|
||||
resp := testHandler.attachmentToResponse(att)
|
||||
want := "https://api.multica.test/api/attachments/" + id + "/download"
|
||||
if resp.MarkdownURL != want {
|
||||
t.Fatalf("markdown_url = %q, want absolute API endpoint %q", resp.MarkdownURL, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildMarkdownURL_PublicURLUnsetFallsBackToSiteRelative(t *testing.T) {
|
||||
origPublic := testHandler.cfg.PublicURL
|
||||
origSigner := testHandler.CFSigner
|
||||
t.Cleanup(func() {
|
||||
testHandler.cfg.PublicURL = origPublic
|
||||
testHandler.CFSigner = origSigner
|
||||
})
|
||||
testHandler.cfg.PublicURL = ""
|
||||
testHandler.CFSigner = nil
|
||||
|
||||
id := seedAttachmentURL(t, "/uploads/abc.png", "abc.png", "image/png", 1)
|
||||
att, err := testHandler.Queries.GetAttachment(context.Background(), db.GetAttachmentParams{
|
||||
ID: parseUUID(id),
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetAttachment: %v", err)
|
||||
}
|
||||
|
||||
resp := testHandler.attachmentToResponse(att)
|
||||
want := "/api/attachments/" + id + "/download"
|
||||
if resp.MarkdownURL != want {
|
||||
t.Fatalf("markdown_url = %q, want site-relative fallback %q", resp.MarkdownURL, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildMarkdownURL_StripsTrailingSlashOnPublicURL(t *testing.T) {
|
||||
origPublic := testHandler.cfg.PublicURL
|
||||
origSigner := testHandler.CFSigner
|
||||
t.Cleanup(func() {
|
||||
testHandler.cfg.PublicURL = origPublic
|
||||
testHandler.CFSigner = origSigner
|
||||
})
|
||||
testHandler.cfg.PublicURL = "https://api.multica.test/"
|
||||
testHandler.CFSigner = nil
|
||||
|
||||
id := seedAttachmentURL(t, "/uploads/abc.png", "abc.png", "image/png", 1)
|
||||
att, err := testHandler.Queries.GetAttachment(context.Background(), db.GetAttachmentParams{
|
||||
ID: parseUUID(id),
|
||||
WorkspaceID: parseUUID(testWorkspaceID),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetAttachment: %v", err)
|
||||
}
|
||||
|
||||
resp := testHandler.attachmentToResponse(att)
|
||||
want := "https://api.multica.test/api/attachments/" + id + "/download"
|
||||
if resp.MarkdownURL != want {
|
||||
t.Fatalf("markdown_url = %q, want exactly one separator %q", resp.MarkdownURL, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsDurablePublicURL(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
url string
|
||||
want bool
|
||||
}{
|
||||
{"absolute https no signature", "https://cdn.multica.test/foo.png", true},
|
||||
{"absolute http no signature", "http://cdn.multica.test/foo.png", true},
|
||||
{"absolute with port + path", "https://cdn.example.test:8080/a/b/c.png", true},
|
||||
{"empty string", "", false},
|
||||
{"site-relative", "/uploads/abc.png", false},
|
||||
{"protocol-relative", "//cdn.example/foo.png", false},
|
||||
{"data URL", "data:image/png;base64,abc", false},
|
||||
{"blob URL", "blob:https://app/abc", false},
|
||||
{"unsupported scheme", "ftp://server/foo", false},
|
||||
{"cloudfront-signed Signature", "https://cdn.example/foo.png?Signature=abc&Key-Pair-Id=K1", false},
|
||||
{"cloudfront-signed Key-Pair-Id alone", "https://cdn.example/foo.png?Key-Pair-Id=K1", false},
|
||||
{"s3-presigned X-Amz-Signature", "https://bucket.s3/foo.png?X-Amz-Signature=abc", false},
|
||||
{"s3-presigned X-Amz-Expires alone", "https://bucket.s3/foo.png?X-Amz-Expires=900", false},
|
||||
{"plain Expires query", "https://cdn.example/foo.png?Expires=99", false},
|
||||
{"unrelated query", "https://cdn.example/foo.png?cache=1", true},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := isDurablePublicURL(tc.url); got != tc.want {
|
||||
t.Errorf("isDurablePublicURL(%q) = %v, want %v", tc.url, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user