MUL-2976: resolve server-relative attachment download_url on Desktop and Mobile

When the backend has no CloudFront signer configured, attachment metadata
returns a server-relative `download_url` like `/api/attachments/{id}/download`.
The CLI already handles relative URLs but the UI clients did not, leaving
the unified-endpoint path failing on Desktop downloads + Mobile Linking +
RN image previews.

Desktop renderer (web + Electron):
- packages/views/editor/use-download-attachment.ts: resolve via
  `resolvePublicFileUrl` from @multica/core/workspace/avatar-url before
  handing to Electron's `downloadURLSafely` bridge or to the web
  placeholder/last-resort window.location redirect.
- packages/views/editor/attachment-preview-modal.tsx: resolve at
  `normalize()` so `<img>`/`<iframe>`/`<video>` srcs are absolute when an
  apiBaseUrl is configured; web with empty base keeps same-origin behaviour.

Mobile (RN):
- apps/mobile/lib/attachment-url.ts: new mirror helper using
  `EXPO_PUBLIC_API_URL` (mobile sharing rule: mirror, don't import core).
- apps/mobile/components/issue/comment-attachment-list.tsx: resolve before
  `Linking.openURL` (RN rejects relative URLs with 'Cannot open URL').
- apps/mobile/lib/markdown/markdown-image.tsx: extend `resolvedUri` memo to
  feed the result through the helper for both `mc://` lookups and pass-through
  paths so the RN image loader receives an absolute URL.

Tests:
- Three new desktop cases in use-download-attachment.test.tsx (relative URL
  resolved against base, base trailing slash trimmed, absolute pass-through).
- New describe block in attachment-preview-modal.test.tsx covering image src,
  iframe src, web same-origin keep-as-is, trailing slash trim, absolute
  pass-through.

Verified: pnpm -C packages/views run typecheck, pnpm -C apps/desktop run
typecheck (node + web), pnpm -C apps/mobile run typecheck, vitest for the
two affected files (40 tests) and the full views suite (1158 tests).

Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
yushen
2026-06-04 13:57:37 +08:00
parent 993fb7d145
commit 5aafabb0af
7 changed files with 310 additions and 15 deletions

View File

@@ -26,6 +26,7 @@ import { Linking, Pressable, View } from "react-native";
import { Ionicons } from "@expo/vector-icons";
import type { Attachment } from "@multica/core/types";
import { MarkdownImage } from "@/lib/markdown/markdown-image";
import { resolveAttachmentUrl } from "@/lib/attachment-url";
import { useColorScheme } from "@/lib/use-color-scheme";
import { THEME } from "@/lib/theme";
import { Text } from "@/components/ui/text";
@@ -108,12 +109,19 @@ function FileCard({
return (
<Pressable
onPress={() => {
// download_url is the signed HTTPS link; opening it hands off to
// download_url is the canonical link opening it hands off to
// Safari which handles auth-token-free download + previewing for
// common types (PDF, txt). Mirrors what the markdown link renderer
// does for `[name](url)`.
if (attachment.download_url) {
void Linking.openURL(attachment.download_url);
//
// The backend may return a server-relative URL like
// `/api/attachments/{id}/download` when no CloudFront signer is
// configured (MUL-2976). RN's `Linking.openURL` requires an
// absolute http(s) URL — it returns "Cannot open URL" otherwise —
// so resolve against `EXPO_PUBLIC_API_URL` first.
const target = resolveAttachmentUrl(attachment.download_url);
if (target) {
void Linking.openURL(target);
}
}}
accessibilityRole="button"

View File

@@ -0,0 +1,44 @@
/**
* Resolve a server-relative attachment URL against the configured API base.
*
* Background: when the backend has no CloudFront signer configured (e.g.
* the self-hosted RustFS / private-S3 case in MUL-2976), `attachment.url`
* and `attachment.download_url` come back as server-relative paths like
* `/api/attachments/{id}/download`. Web is happy with that — same-origin
* `<img src="/api/...">` resolves against the document base — but RN
* needs an absolute http(s) URL for both `Linking.openURL` (`Cannot open
* URL` otherwise) and `<Image source={{ uri }}>` (no document origin to
* resolve against; the request is silently dropped).
*
* Mirrors `packages/core/workspace/avatar-url.ts:resolvePublicFileUrl`
* exactly. We don't import the core helper because its `getBaseUrl()`
* pulls from a singleton ApiClient that lives in `@multica/core/api` —
* not on the mobile sharing whitelist (apps/mobile/CLAUDE.md "mirror,
* don't import"). Mobile reads its own `EXPO_PUBLIC_API_URL` from the
* Expo env, the same value the rest of `data/api.ts` uses.
*
* Contract:
* - null / undefined / "" → null (caller should treat as "no URL").
* - already-absolute URL → returned unchanged.
* - server-relative path → API base + path, with a single boundary
* slash (we trim trailing slashes from the
* base before joining).
*/
const API_URL = process.env.EXPO_PUBLIC_API_URL ?? "";
export function resolveAttachmentUrlWithBase(
rawUrl: string | null | undefined,
baseUrl: string,
): string | null {
if (!rawUrl) return null;
if (!rawUrl.startsWith("/")) return rawUrl;
const trimmedBaseUrl = baseUrl.replace(/\/+$/, "");
return `${trimmedBaseUrl}${rawUrl}`;
}
export function resolveAttachmentUrl(
rawUrl: string | null | undefined,
): string | null {
return resolveAttachmentUrlWithBase(rawUrl, API_URL);
}

View File

@@ -28,6 +28,7 @@ import { useEffect, useMemo, useState } from "react";
import { Image as RNImage, Pressable, View } from "react-native";
import { Image as ExpoImage } from "expo-image";
import type { Attachment } from "@multica/core/types";
import { resolveAttachmentUrl } from "@/lib/attachment-url";
import { useLightbox } from "./lightbox-provider";
interface Props {
@@ -41,9 +42,21 @@ export function MarkdownImage({ uri, attachments }: Props) {
const [aspect, setAspect] = useState<number | null>(null);
const resolvedUri = useMemo(() => {
if (!attachments || attachments.length === 0) return uri;
const match = attachments.find((a) => a.url === uri);
return match?.download_url || uri;
// mc://file/<id> → look up the matching attachment's download_url.
// No match (external link, html https URL, or unresolved mc://) falls
// through to the original uri.
let candidate: string | null | undefined = uri;
if (attachments && attachments.length > 0) {
const match = attachments.find((a) => a.url === uri);
if (match?.download_url) candidate = match.download_url;
}
// The backend may return a server-relative `download_url` (e.g.
// `/api/attachments/{id}/download`) when no CloudFront signer is
// configured — see MUL-2976. RN's image loader has no document
// origin to resolve against, so prepend `EXPO_PUBLIC_API_URL` for
// server-relative paths and let absolute URLs / external links pass
// through unchanged.
return resolveAttachmentUrl(candidate) ?? uri;
}, [uri, attachments]);
useEffect(() => {

View File

@@ -17,6 +17,7 @@ vi.mock("../platform", () => ({
const {
getAttachmentTextContentMock,
downloadMock,
getBaseUrlMock,
FakePreviewTooLargeError,
FakePreviewUnsupportedError,
} = vi.hoisted(() => {
@@ -35,13 +36,19 @@ const {
return {
getAttachmentTextContentMock: vi.fn(),
downloadMock: vi.fn(),
// Default to the web shape (empty base, same-origin). Tests covering
// the desktop-renderer / standalone-shell case override per-test.
getBaseUrlMock: vi.fn(() => ""),
FakePreviewTooLargeError,
FakePreviewUnsupportedError,
};
});
vi.mock("@multica/core/api", () => ({
api: { getAttachmentTextContent: getAttachmentTextContentMock },
api: {
getAttachmentTextContent: getAttachmentTextContentMock,
getBaseUrl: getBaseUrlMock,
},
PreviewTooLargeError: FakePreviewTooLargeError,
PreviewUnsupportedError: FakePreviewUnsupportedError,
}));
@@ -147,6 +154,9 @@ beforeEach(() => {
vi.clearAllMocks();
navState.hasOpenInNewTab = true;
slugState.value = "acme";
// Default to web's same-origin empty base so existing absolute-URL tests
// remain unaffected by the relative-URL resolution added in normalize().
getBaseUrlMock.mockReturnValue("");
});
afterEach(() => {
@@ -260,6 +270,113 @@ describe("AttachmentPreviewModal — dispatch", () => {
});
});
describe("AttachmentPreviewModal — server-relative download_url resolution (MUL-2976)", () => {
// The unified `/api/attachments/{id}/download` endpoint returns a
// server-relative path on non-CloudFront deployments. The web app keeps
// working same-origin because `apiBaseUrl=""`, but the desktop renderer
// is loaded from `app://` / file: / dev-server origin and needs the
// absolute URL — otherwise `<img src>`, `<iframe src>`, `<video src>`
// hit the shell origin and fail.
it("prefixes the configured API base for image previews when download_url is server-relative", () => {
getBaseUrlMock.mockReturnValue("https://api.example.test");
const att = makeAttachment({
filename: "shot.png",
content_type: "image/png",
download_url: "/api/attachments/att-1/download",
});
render(
<AttachmentPreviewModal
source={{ kind: "full", attachment: att }}
open
onClose={() => {}}
/>,
);
const img = document.querySelector("img");
expect(img?.getAttribute("src")).toBe(
"https://api.example.test/api/attachments/att-1/download",
);
});
it("prefixes the configured API base for PDF previews when download_url is server-relative", () => {
getBaseUrlMock.mockReturnValue("https://api.example.test");
const att = makeAttachment({
filename: "manual.pdf",
content_type: "application/pdf",
download_url: "/api/attachments/att-1/download",
});
render(
<AttachmentPreviewModal
source={{ kind: "full", attachment: att }}
open
onClose={() => {}}
/>,
);
const iframe = document.querySelector("iframe");
expect(iframe?.getAttribute("src")).toBe(
"https://api.example.test/api/attachments/att-1/download",
);
});
it("keeps a same-origin relative URL untouched when the configured base is empty (web)", () => {
// Default web shape — empty base. Browser resolves the relative path
// against the document origin, no prefix needed.
const att = makeAttachment({
filename: "shot.png",
content_type: "image/png",
download_url: "/api/attachments/att-1/download",
});
render(
<AttachmentPreviewModal
source={{ kind: "full", attachment: att }}
open
onClose={() => {}}
/>,
);
const img = document.querySelector("img");
expect(img?.getAttribute("src")).toBe("/api/attachments/att-1/download");
});
it("trims a trailing slash on the configured base when joining a relative URL", () => {
getBaseUrlMock.mockReturnValue("https://api.example.test/");
const att = makeAttachment({
filename: "shot.png",
content_type: "image/png",
download_url: "/api/attachments/att-1/download",
});
render(
<AttachmentPreviewModal
source={{ kind: "full", attachment: att }}
open
onClose={() => {}}
/>,
);
const img = document.querySelector("img");
expect(img?.getAttribute("src")).toBe(
"https://api.example.test/api/attachments/att-1/download",
);
});
it("passes an already-absolute CloudFront/presigned download_url through unchanged", () => {
getBaseUrlMock.mockReturnValue("https://api.example.test");
const att = makeAttachment({
filename: "shot.png",
content_type: "image/png",
download_url: "https://cdn.example.test/att-1.png?Signature=s",
});
render(
<AttachmentPreviewModal
source={{ kind: "full", attachment: att }}
open
onClose={() => {}}
/>,
);
const img = document.querySelector("img");
expect(img?.getAttribute("src")).toBe(
"https://cdn.example.test/att-1.png?Signature=s",
);
});
});
describe("AttachmentPreviewModal — error states", () => {
it("shows the too-large fallback on PreviewTooLargeError", async () => {
getAttachmentTextContentMock.mockRejectedValueOnce(new FakePreviewTooLargeError());

View File

@@ -45,6 +45,7 @@ import {
import { Download, ExternalLink, FileText, Loader2, X } from "lucide-react";
import type { Attachment } from "@multica/core/types";
import { paths, useWorkspaceSlug } from "@multica/core/paths";
import { resolvePublicFileUrl } from "@multica/core/workspace/avatar-url";
import { useT } from "../i18n";
import { useNavigation } from "../navigation";
import { openExternal } from "../platform";
@@ -92,18 +93,29 @@ interface PreviewState {
}
function normalize(source: PreviewSource): PreviewState {
// Resolve any server-relative URL (e.g. `/api/attachments/{id}/download`
// returned by the unified-endpoint metadata path when no CloudFront
// signer is configured) against the configured API base. Web with the
// default empty base keeps the relative path and resolves it against
// the page origin — same behaviour as before this PR. Desktop renderer
// (loaded from `app://` / file: / dev-server origin) needs the absolute
// form so `<img src>` / `<iframe src>` / `<video src>` actually point at
// the API server instead of the shell origin.
if (source.kind === "full") {
const mediaUrl =
resolvePublicFileUrl(source.attachment.download_url) ??
source.attachment.download_url;
return {
filename: source.attachment.filename,
contentType: source.attachment.content_type,
mediaUrl: source.attachment.download_url,
mediaUrl,
attachmentId: source.attachment.id,
};
}
return {
filename: source.filename,
contentType: "",
mediaUrl: source.url,
mediaUrl: resolvePublicFileUrl(source.url) ?? source.url,
attachmentId: null,
};
}

View File

@@ -4,9 +4,10 @@ import { act, renderHook, waitFor } from "@testing-library/react";
// Hoisted mock for the API singleton: vi.mock factories cannot reference
// outside-of-scope vars, but vi.hoisted runs before the import graph.
const getAttachmentMock = vi.hoisted(() => vi.fn());
const getBaseUrlMock = vi.hoisted(() => vi.fn(() => ""));
vi.mock("@multica/core/api", () => ({
api: { getAttachment: getAttachmentMock },
api: { getAttachment: getAttachmentMock, getBaseUrl: getBaseUrlMock },
}));
vi.mock("sonner", () => ({
@@ -25,6 +26,10 @@ const SIGNED_URL =
beforeEach(() => {
vi.clearAllMocks();
// Default: web with same-origin API (empty base). Each test that needs
// a non-empty base (desktop standalone, server-relative download URL)
// overrides via getBaseUrlMock.mockReturnValue(...).
getBaseUrlMock.mockReturnValue("");
});
afterEach(() => {
@@ -118,4 +123,81 @@ describe("useDownloadAttachment (desktop)", () => {
expect(downloadURL).not.toHaveBeenCalled();
await waitFor(() => expect(toast.error).toHaveBeenCalled());
});
// MUL-2976: when the backend has no CloudFront signer, `getAttachment`
// returns a server-relative `download_url` like `/api/attachments/.../download`.
// The Electron main-process `downloadURLSafely` requires a parsable
// http(s) URL or it drops the request — so the renderer must resolve
// the path against the configured API base before crossing the bridge.
it("resolves a server-relative download_url against the API base before handing it to the desktop bridge", async () => {
const downloadURL = vi.fn();
(window as unknown as { desktopAPI: { downloadURL: typeof downloadURL } }).desktopAPI = {
downloadURL,
};
getBaseUrlMock.mockReturnValue("https://api.example.test");
getAttachmentMock.mockResolvedValueOnce({
id: "att-1",
url: "https://static.example.test/file.md",
download_url: "/api/attachments/att-1/download",
filename: "file.md",
});
const { result } = renderHook(() => useDownloadAttachment());
await act(async () => {
await result.current("att-1");
});
expect(downloadURL).toHaveBeenCalledWith(
"https://api.example.test/api/attachments/att-1/download",
);
});
it("trims a trailing slash on the API base when resolving a relative download_url", async () => {
const downloadURL = vi.fn();
(window as unknown as { desktopAPI: { downloadURL: typeof downloadURL } }).desktopAPI = {
downloadURL,
};
getBaseUrlMock.mockReturnValue("https://api.example.test/");
getAttachmentMock.mockResolvedValueOnce({
id: "att-1",
url: "/api/attachments/att-1/content",
download_url: "/api/attachments/att-1/download",
filename: "file.md",
});
const { result } = renderHook(() => useDownloadAttachment());
await act(async () => {
await result.current("att-1");
});
expect(downloadURL).toHaveBeenCalledWith(
"https://api.example.test/api/attachments/att-1/download",
);
});
it("passes an already-absolute download_url through unchanged when the bridge is present", async () => {
const downloadURL = vi.fn();
(window as unknown as { desktopAPI: { downloadURL: typeof downloadURL } }).desktopAPI = {
downloadURL,
};
// Even with a non-empty base configured, a CloudFront signed URL
// must not be re-prefixed.
getBaseUrlMock.mockReturnValue("https://api.example.test");
getAttachmentMock.mockResolvedValueOnce({
id: "att-1",
url: "https://cdn.example.test/att-1.bin",
download_url: SIGNED_URL,
filename: "file.md",
});
const { result } = renderHook(() => useDownloadAttachment());
await act(async () => {
await result.current("att-1");
});
expect(downloadURL).toHaveBeenCalledWith(SIGNED_URL);
});
});

View File

@@ -3,6 +3,7 @@
import { useCallback } from "react";
import { toast } from "sonner";
import { api } from "@multica/core/api";
import { resolvePublicFileUrl } from "@multica/core/workspace/avatar-url";
import { useT } from "../i18n";
interface DesktopBridge {
@@ -49,14 +50,23 @@ export function useDownloadAttachment(): (attachmentId: string) => Promise<void>
if (hasDesktopDownloadBridge()) {
try {
const fresh = await api.getAttachment(attachmentId);
if (!fresh.download_url) {
// Server may return a server-relative `download_url`
// (`/api/attachments/{id}/download`) when no CloudFront
// signer is configured — the unified download endpoint chooses
// CloudFront/presign/proxy at request time. Electron's main-side
// `downloadURLSafely` requires `new URL()` to parse to http/https,
// so resolve against the configured API base before we cross the
// bridge. Absolute URLs (legacy CloudFront / S3 presigned) pass
// through unchanged.
const downloadUrl = resolvePublicFileUrl(fresh.download_url);
if (!downloadUrl) {
failed();
return;
}
const bridge = (
window as unknown as { desktopAPI?: DesktopBridge }
).desktopAPI;
await bridge!.downloadURL!(fresh.download_url);
await bridge!.downloadURL!(downloadUrl);
} catch {
failed();
}
@@ -72,17 +82,26 @@ export function useDownloadAttachment(): (attachmentId: string) => Promise<void>
: null;
try {
const fresh = await api.getAttachment(attachmentId);
if (!fresh.download_url) {
// Same relative-URL handling as desktop above. For web the
// `apiBaseUrl` is typically empty (same-origin), so `resolvePublicFileUrl`
// returns the path unchanged and the browser resolves it against
// the page's origin — the existing same-origin behaviour. When a
// non-empty base is configured (e.g. tauri/standalone shells that
// bundle the SPA but talk to a remote API), the resolver yields an
// absolute URL, which works for both `placeholder.location.href`
// and the last-resort `window.location.href` redirect.
const downloadUrl = resolvePublicFileUrl(fresh.download_url);
if (!downloadUrl) {
placeholder?.close();
failed();
return;
}
if (placeholder) {
placeholder.opener = null;
placeholder.location.href = fresh.download_url;
placeholder.location.href = downloadUrl;
} else if (typeof window !== "undefined") {
// Popup blocked outright — last-resort navigate the current tab.
window.location.href = fresh.download_url;
window.location.href = downloadUrl;
}
} catch {
placeholder?.close();