From b88c4f366321b502095ef4a8dad299bafa42a289 Mon Sep 17 00:00:00 2001 From: cxymds Date: Mon, 9 Nov 2026 19:08:28 +0800 Subject: [PATCH 1/2] fix: preview untyped objects as safe text Detect bounded UTF-8 content when object metadata has no specific MIME type so extensionless text no longer opens a blank iframe. Keep unsupported content downloadable and preserve the selected key and version for preview downloads. --- app/(dashboard)/browser/content.tsx | 38 ++++- components/object/info.tsx | 3 +- components/object/preview-modal.tsx | 96 ++++++------- hooks/use-object-preview-text.ts | 47 ++++++ hooks/use-object.ts | 3 +- lib/object-preview.ts | 92 +++++++++++- tests/hooks/use-object-preview-text.test.ts | 120 ++++++++++++++++ tests/lib/object-preview-source.test.js | 9 +- tests/lib/object-preview.test.ts | 151 ++++++++++++++++++++ 9 files changed, 494 insertions(+), 65 deletions(-) create mode 100644 hooks/use-object-preview-text.ts create mode 100644 tests/hooks/use-object-preview-text.test.ts create mode 100644 tests/lib/object-preview.test.ts diff --git a/app/(dashboard)/browser/content.tsx b/app/(dashboard)/browser/content.tsx index 06689671..0bd52499 100644 --- a/app/(dashboard)/browser/content.tsx +++ b/app/(dashboard)/browser/content.tsx @@ -19,6 +19,9 @@ import { ObjectPreviewModal } from "@/components/object/preview-modal" import { useObject } from "@/hooks/use-object" import { usePermissions } from "@/hooks/use-permissions" import { isMissingBucketError } from "@/lib/bucket-access" +import { getAttachmentContentDisposition } from "@/lib/content-disposition" +import { downloadUrl } from "@/lib/export-file" +import { createObjectPreviewInfo } from "@/lib/object-preview" interface BrowserContentProps { bucketName: string @@ -44,6 +47,7 @@ export function BrowserContent({ bucketName, keyPath = "", preview = false, prev const [refreshTrigger, setRefreshTrigger] = React.useState(0) const [showPreview, setShowPreview] = React.useState(false) const [previewObject, setPreviewObject] = React.useState | null>(null) + const [previewBucketName, setPreviewBucketName] = React.useState("") const objectApi = useObject(bucketName) const canUploadObjects = canCapability("objects.upload", { bucket: bucketName, prefix }) @@ -88,12 +92,35 @@ export function BrowserContent({ bucketName, keyPath = "", preview = false, prev async ({ key, data }: { key?: string; data?: Record }) => { if (!key) return const info = data ?? (await objectApi.getObjectInfo(key)) - setPreviewObject(info as Record) + setPreviewObject(createObjectPreviewInfo(key, info)) + setPreviewBucketName(bucketName) setShowPreview(true) }, - [objectApi], + [objectApi, bucketName], ) + const previewObjectKey = typeof previewObject?.Key === "string" ? previewObject.Key : "" + const canDownloadPreview = + previewBucketName === bucketName && + canCapability("objects.download", { bucket: bucketName, objectKey: previewObjectKey }) + const downloadPreview = async () => { + if (!canDownloadPreview || !previewObjectKey) return + try { + const filename = previewObjectKey.split("/").pop() ?? "" + const versionId = typeof previewObject?.VersionId === "string" ? previewObject.VersionId : undefined + const url = await objectApi.getSignedUrl( + previewObjectKey, + 3600, + getAttachmentContentDisposition(filename), + versionId, + ) + downloadUrl(url, filename) + message.success(t("Download ready")) + } catch (error) { + message.error((error as Error)?.message ?? t("Download Failed")) + } + } + const clearPreviewQueryParams = React.useCallback(() => { if (!searchParams.has("preview") && !searchParams.has("previewKey")) return @@ -206,7 +233,12 @@ export function BrowserContent({ bucketName, keyPath = "", preview = false, prev canUpload={canUploadObjects} /> - + ) } diff --git a/components/object/info.tsx b/components/object/info.tsx index 1a437447..57a2b05e 100644 --- a/components/object/info.tsx +++ b/components/object/info.tsx @@ -37,6 +37,7 @@ import { useMessage } from "@/lib/feedback/message" import { useDialog } from "@/lib/feedback/dialog" import { downloadUrl } from "@/lib/export-file" import { getAttachmentContentDisposition } from "@/lib/content-disposition" +import { createObjectPreviewInfo } from "@/lib/object-preview" import { normalizeDateToIso } from "@/lib/safe-date" import { formatDateTimeWithOffset } from "@/lib/functions" import { @@ -306,7 +307,7 @@ export function ObjectInfo({ bucketName, objectKey, open, onOpenChange, onPrevie { expiresIn: 3600 }, ), ]) - const data = { ...head, SignedUrl: signed } + const data = createObjectPreviewInfo(key, { ...head, SignedUrl: signed }, version) setShowVersions(false) onPreview({ key, data }) } catch (err) { diff --git a/components/object/preview-modal.tsx b/components/object/preview-modal.tsx index b4c517e7..e2030cbb 100644 --- a/components/object/preview-modal.tsx +++ b/components/object/preview-modal.tsx @@ -10,23 +10,16 @@ import { RiCloseLine, RiFullscreenExitLine, RiFullscreenLine } from "@remixicon/ import { PdfViewer } from "@/components/object/pdf-viewer" import { ParquetViewer } from "@/components/object/parquet-viewer" import { TiffViewer } from "@/components/object/tiff-viewer" -import { getObjectPreviewMode, normalizePreviewContentType } from "@/lib/object-preview" +import { + getObjectPreviewMode, + isSafeTextPreview, + MAX_TEXT_PREVIEW_BYTES, + normalizePreviewContentType, +} from "@/lib/object-preview" +import { useObjectPreviewText } from "@/hooks/use-object-preview-text" import Image from "next/image" -const SAFE_TEXT_MIMES = [ - "application/json", - "application/jsonl", - "application/ndjson", - "application/x-ndjson", - "application/xml", - "text/plain", - "text/xml", - "text/csv", - "text/markdown", -] -const SAFE_TEXT_EXTENSIONS = [".txt", ".json", ".jsonl", ".ndjson", ".xml", ".csv", ".md", ".yml", ".yaml"] const SAFE_IMAGE_EXTENSIONS = [".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".ico", ".tif", ".tiff"] -const ALLOWED_SIZE = 1024 * 1024 * 2 // 2MB const PARQUET_MIMES = ["application/vnd.apache.parquet", "application/x-parquet", "application/parquet"] const PARQUET_EXTENSIONS = [".parquet", ".pq"] @@ -34,6 +27,7 @@ const PARQUET_EXTENSIONS = [".parquet", ".pq"] interface ObjectPreviewModalProps { show: boolean onShowChange: (show: boolean) => void + onDownload?: () => Promise object: { ContentType?: string ContentLength?: number @@ -51,13 +45,6 @@ type FullscreenElement = HTMLElement & { webkitRequestFullscreen?: () => Promise | void } -function isSafeTextPreview(contentType: string, objectKey: string, objectSize: number) { - if (objectSize > ALLOWED_SIZE) return false - if (SAFE_TEXT_MIMES.includes(contentType)) return true - const keyLower = objectKey.toLowerCase() - return SAFE_TEXT_EXTENSIONS.some((ext) => keyLower.endsWith(ext)) -} - function isImagePreview(contentType: string, objectKey: string) { const keyLower = objectKey.toLowerCase() const isSvg = contentType === "image/svg+xml" || keyLower.endsWith(".svg") @@ -109,10 +96,8 @@ function requestFullscreen(element: FullscreenElement): Promise { return Promise.resolve() } -export function ObjectPreviewModal({ show, onShowChange, object }: ObjectPreviewModalProps) { +export function ObjectPreviewModal({ show, onShowChange, onDownload, object }: ObjectPreviewModalProps) { const { t } = useTranslation() - const [textContent, setTextContent] = React.useState("") - const [loading, setLoading] = React.useState(false) const [audioLoadError, setAudioLoadError] = React.useState(false) const [videoLoadError, setVideoLoadError] = React.useState(false) const [isFormatted, setIsFormatted] = React.useState(true) @@ -139,17 +124,32 @@ export function ObjectPreviewModal({ show, onShowChange, object }: ObjectPreview const canRenderPdf = hasPreviewUrl && isPdfPreview(normalizedContentType) const canRenderParquet = hasPreviewUrl && isParquetPreview(normalizedContentType, objectKey) const canRenderTiff = hasPreviewUrl && isTiffPreview(objectKey) - const previewMode = getObjectPreviewMode({ + const initialPreviewMode = getObjectPreviewMode({ hasPreviewUrl, contentType, + objectSize, canRenderText, canRenderImage, canRenderPdf, canRenderParquet, canRenderTiff, }) + const { status: textStatus, content: textContent } = useObjectPreviewText({ + show, + mode: initialPreviewMode, + url: previewUrl, + }) + const previewMode = + textStatus === "error" + ? "error" + : textStatus === "unsupported" + ? "download" + : initialPreviewMode === "detect-text" && textStatus === "ready" + ? "text" + : initialPreviewMode + const loading = textStatus === "loading" const isImageMode = previewMode === "image" - const isCompactPreview = previewMode === "audio" || previewMode === "download" + const isCompactPreview = previewMode === "audio" || previewMode === "download" || previewMode === "error" const getFormattedContent = () => { if (!isJson || !isFormatted) return textContent @@ -162,30 +162,8 @@ export function ObjectPreviewModal({ show, onShowChange, object }: ObjectPreview } React.useEffect(() => { - if (show && previewMode === "text" && previewUrl) { - const controller = new AbortController() - setLoading(true) - setIsFormatted(true) - setTextContent("") - fetch(previewUrl, { signal: controller.signal }) - .then((response) => { - if (!response.ok) throw new Error(`HTTP ${response.status}`) - return response.text() - }) - .then(setTextContent) - .catch((error: unknown) => { - if ((error as Error)?.name !== "AbortError") setTextContent(t("Preview unavailable")) - }) - .finally(() => { - if (!controller.signal.aborted) setLoading(false) - }) - - return () => controller.abort() - } else if (!show) { - setTextContent("") - setLoading(false) - } - }, [show, previewMode, previewUrl, t]) + setIsFormatted(true) + }, [show, previewUrl]) React.useEffect(() => { setAudioLoadError(false) @@ -295,7 +273,7 @@ export function ObjectPreviewModal({ show, onShowChange, object }: ObjectPreview if (loading) { return (
- +
) } @@ -440,12 +418,22 @@ export function ObjectPreviewModal({ show, onShowChange, object }: ObjectPreview case "tiff": return case "download": + case "error": default: return (
- {t("Cannot Preview", { - contentType: contentType || "unknown", - })} +

+ {previewMode === "error" + ? t("Preview unavailable") + : objectSize > MAX_TEXT_PREVIEW_BYTES + ? t("File is too large to preview, please download to view") + : t("Cannot Preview", { contentType: contentType || "unknown" })} +

+ {onDownload && ( + + )}
) } diff --git a/hooks/use-object-preview-text.ts b/hooks/use-object-preview-text.ts new file mode 100644 index 00000000..bf958b15 --- /dev/null +++ b/hooks/use-object-preview-text.ts @@ -0,0 +1,47 @@ +"use client" + +import { useEffect, useState } from "react" +import { readObjectPreviewText, type ObjectPreviewMode } from "@/lib/object-preview" + +interface PreviewTextOptions { + show: boolean + mode: ObjectPreviewMode + url: string +} + +interface PreviewTextResult { + url: string + mode: ObjectPreviewMode + status: "ready" | "unsupported" | "error" + content: string +} + +export function useObjectPreviewText({ show, mode, url }: PreviewTextOptions) { + const [result, setResult] = useState(null) + const shouldLoad = show && Boolean(url) && (mode === "text" || mode === "detect-text") + + useEffect(() => { + setResult(null) + if (!shouldLoad) return + + const controller = new AbortController() + fetch(url, { signal: controller.signal }) + .then((response) => { + if (!response.ok) throw new Error(`HTTP ${response.status}`) + return mode === "detect-text" ? readObjectPreviewText(response) : response.text() + }) + .then((content) => { + if (controller.signal.aborted) return + setResult({ url, mode, status: content === null ? "unsupported" : "ready", content: content ?? "" }) + }) + .catch(() => { + if (!controller.signal.aborted) setResult({ url, mode, status: "error", content: "" }) + }) + + return () => controller.abort() + }, [shouldLoad, mode, url]) + + if (!shouldLoad) return { status: "idle" as const, content: "" } + if (result?.url !== url || result.mode !== mode) return { status: "loading" as const, content: "" } + return { status: result.status, content: result.content } +} diff --git a/hooks/use-object.ts b/hooks/use-object.ts index 4332cb41..cb3fea46 100644 --- a/hooks/use-object.ts +++ b/hooks/use-object.ts @@ -49,11 +49,12 @@ export function useObject(bucket: string) { ) const getSignedUrlFn = useCallback( - async (key: string, expiresIn = 3600, responseContentDisposition?: string) => { + async (key: string, expiresIn = 3600, responseContentDisposition?: string, versionId?: string) => { const command = new GetObjectCommand({ Bucket: bucket, Key: key, ResponseContentDisposition: responseContentDisposition, + VersionId: versionId === "00000000-0000-0000-0000-000000000000" ? undefined : versionId, }) return getSignedUrl(client, command, { expiresIn }) }, diff --git a/lib/object-preview.ts b/lib/object-preview.ts index 1a60ad21..35beb113 100644 --- a/lib/object-preview.ts +++ b/lib/object-preview.ts @@ -1,8 +1,35 @@ -type ObjectPreviewMode = "text" | "image" | "audio" | "video" | "pdf" | "parquet" | "sandbox" | "download" | "tiff" +export type ObjectPreviewMode = + | "text" + | "detect-text" + | "image" + | "audio" + | "video" + | "pdf" + | "parquet" + | "sandbox" + | "download" + | "tiff" + +export const MAX_TEXT_PREVIEW_BYTES = 1024 * 1024 * 2 + +const SAFE_TEXT_MIMES = [ + "application/json", + "application/jsonl", + "application/ndjson", + "application/x-ndjson", + "application/xml", + "text/plain", + "text/xml", + "text/csv", + "text/markdown", +] +const SAFE_TEXT_EXTENSIONS = [".txt", ".json", ".jsonl", ".ndjson", ".xml", ".csv", ".md", ".yml", ".yaml"] +const GENERIC_CONTENT_TYPES = ["", "application/octet-stream", "binary/octet-stream"] interface ObjectPreviewOptions { hasPreviewUrl: boolean contentType: string + objectSize: number canRenderText: boolean canRenderImage: boolean canRenderPdf: boolean @@ -14,9 +41,64 @@ export function normalizePreviewContentType(contentType: string) { return contentType.split(";")[0]?.trim().toLowerCase() ?? "" } +export function createObjectPreviewInfo(key: string, info: T, versionId?: string) { + return { ...info, Key: key, ...(versionId === undefined ? {} : { VersionId: versionId }) } +} + +export function isSafeTextPreview(contentType: string, objectKey: string, objectSize: number) { + if (objectSize > MAX_TEXT_PREVIEW_BYTES) return false + if (SAFE_TEXT_MIMES.includes(contentType)) return true + const keyLower = objectKey.toLowerCase() + return SAFE_TEXT_EXTENSIONS.some((ext) => keyLower.endsWith(ext)) +} + +/** Read bounded content before deciding whether a generic object is UTF-8 text. */ +export async function readObjectPreviewText(response: Response): Promise { + if (Number(response.headers.get("Content-Length")) > MAX_TEXT_PREVIEW_BYTES) { + await response.body?.cancel() + return null + } + const reader = response.body?.getReader() + if (!reader) return null + + const chunks: Uint8Array[] = [] + let size = 0 + try { + while (true) { + const { done, value } = await reader.read() + if (done) break + size += value.byteLength + if (size > MAX_TEXT_PREVIEW_BYTES) { + await reader.cancel() + return null + } + chunks.push(value) + } + } finally { + reader.releaseLock() + } + if (!size) return null + + const bytes = new Uint8Array(size) + let offset = 0 + for (const chunk of chunks) { + bytes.set(chunk, offset) + offset += chunk.byteLength + } + let text: string + try { + text = new TextDecoder("utf-8", { fatal: true }).decode(bytes) + } catch { + return null + } + // Allow ordinary text whitespace while rejecting binary control characters. + return !text.length || /[\u0000-\u0008\u000b\u000e-\u001f\u007f-\u009f]/u.test(text) ? null : text +} + export function getObjectPreviewMode({ hasPreviewUrl, contentType, + objectSize, canRenderText, canRenderImage, canRenderPdf, @@ -31,5 +113,11 @@ export function getObjectPreviewMode({ if (canRenderPdf) return "pdf" if (canRenderTiff) return "tiff" if (canRenderImage) return "image" - return canRenderText ? "text" : "sandbox" + if (canRenderText) return "text" + if (GENERIC_CONTENT_TYPES.includes(normalizedContentType)) { + return Number.isFinite(objectSize) && objectSize >= 0 && objectSize <= MAX_TEXT_PREVIEW_BYTES + ? "detect-text" + : "download" + } + return "sandbox" } diff --git a/tests/hooks/use-object-preview-text.test.ts b/tests/hooks/use-object-preview-text.test.ts new file mode 100644 index 00000000..1412cc04 --- /dev/null +++ b/tests/hooks/use-object-preview-text.test.ts @@ -0,0 +1,120 @@ +import test, { type TestContext } from "node:test" +import assert from "node:assert/strict" +import { act, createElement } from "react" +import { createRoot } from "react-dom/client" +import { Window } from "happy-dom" +import { useObjectPreviewText } from "../../hooks/use-object-preview-text" + +type Options = Parameters[0] + +async function mountPreview(t: TestContext, fetcher: typeof fetch, initial: Partial = {}) { + const browser = new Window() + const globals = { + window: browser, + document: browser.document, + fetch: fetcher, + IS_REACT_ACT_ENVIRONMENT: true, + } + const descriptors = Object.keys(globals).map( + (key) => [key, Object.getOwnPropertyDescriptor(globalThis, key)] as const, + ) + for (const [key, value] of Object.entries(globals)) { + Object.defineProperty(globalThis, key, { configurable: true, writable: true, value }) + } + const container = browser.document.createElement("div") + browser.document.body.appendChild(container) + const root = createRoot(container as unknown as Element) + let props: Options = { show: true, mode: "detect-text", url: "/fred", ...initial } + let result: ReturnType + function Preview(options: Options) { + result = useObjectPreviewText(options) + return null + } + t.after(async () => { + await act(async () => root.unmount()) + await browser.happyDOM.close() + for (const [key, descriptor] of descriptors) { + if (descriptor) Object.defineProperty(globalThis, key, descriptor) + else Reflect.deleteProperty(globalThis, key) + } + }) + const render = async (changes: Partial = {}) => { + props = { ...props, ...changes } + await act(async () => root.render(createElement(Preview, props))) + } + await render() + return { render, result: () => result } +} + +test("extensionless text becomes ready for the text renderer", async (t) => { + const preview = await mountPreview(t, async () => new Response("this is a text file\n")) + assert.deepEqual(preview.result(), { status: "ready", content: "this is a text file\n" }) +}) + +test("binary content becomes unsupported rather than an iframe or an error", async (t) => { + const preview = await mountPreview(t, async () => new Response(Uint8Array.of(0, 1, 2))) + assert.deepEqual(preview.result(), { status: "unsupported", content: "" }) +}) + +test("HTTP and network failures become visible errors", async (t) => { + const preview = await mountPreview(t, async () => new Response("Access denied", { status: 403 })) + assert.deepEqual(preview.result(), { status: "error", content: "" }) +}) + +test("a rejected fetch is not classified as unsupported content", async (t) => { + const preview = await mountPreview(t, async () => { + throw new TypeError("Failed to fetch") + }) + assert.deepEqual(preview.result(), { status: "error", content: "" }) +}) + +test("pending content shows loading until the request resolves", async (t) => { + const response = Promise.withResolvers() + const preview = await mountPreview(t, () => response.promise) + assert.equal(preview.result().status, "loading") + await act(async () => response.resolve(new Response("fred"))) + assert.equal(preview.result().status, "ready") +}) + +test("closing the modal aborts the request and clears preview content", async (t) => { + const response = Promise.withResolvers() + let signal: AbortSignal | null | undefined + const preview = await mountPreview(t, (_url, options) => { + signal = options?.signal + return response.promise + }) + await preview.render({ show: false }) + assert.equal(signal?.aborted, true) + await act(async () => response.resolve(new Response("stale content"))) + assert.deepEqual(preview.result(), { status: "idle", content: "" }) +}) + +test("late responses from a previous object cannot replace the current preview", async (t) => { + const previous = Promise.withResolvers() + const current = Promise.withResolvers() + const preview = await mountPreview(t, (url) => (url === "/fred" ? previous.promise : current.promise)) + await preview.render({ url: "/current" }) + await act(async () => current.resolve(new Response("current content"))) + await act(async () => previous.resolve(new Response("previous content"))) + assert.deepEqual(preview.result(), { status: "ready", content: "current content" }) +}) + +test("a changed preview renderer clears a pending text request", async (t) => { + const response = Promise.withResolvers() + const preview = await mountPreview(t, () => response.promise) + await preview.render({ mode: "image" }) + await act(async () => response.resolve(new Response("previous content"))) + assert.deepEqual(preview.result(), { status: "idle", content: "" }) +}) + +test("closed and non-text previews never fetch text", async (t) => { + const fetcher = t.mock.fn(async () => new Response("unused")) + const preview = await mountPreview(t, fetcher, { show: false }) + await preview.render({ show: true, mode: "download" }) + assert.equal(fetcher.mock.callCount(), 0) +}) + +test("existing explicit text decoding retains its behavior", async (t) => { + const preview = await mountPreview(t, async () => new Response(Uint8Array.of(0xff)), { mode: "text" }) + assert.deepEqual(preview.result(), { status: "ready", content: "�" }) +}) diff --git a/tests/lib/object-preview-source.test.js b/tests/lib/object-preview-source.test.js index bcd482b4..63da1171 100644 --- a/tests/lib/object-preview-source.test.js +++ b/tests/lib/object-preview-source.test.js @@ -6,6 +6,7 @@ import { getObjectPreviewMode } from "../../lib/object-preview.ts" const previewOptions = { hasPreviewUrl: true, contentType: "application/octet-stream", + objectSize: 20, canRenderText: false, canRenderImage: false, canRenderPdf: false, @@ -34,7 +35,7 @@ test("object preview dispatches normalized video MIME types to the native video }) test("object preview preserves non-media dispatch behavior", () => { - assert.equal(getObjectPreviewMode(previewOptions), "sandbox") + assert.equal(getObjectPreviewMode(previewOptions), "detect-text") assert.equal(getObjectPreviewMode({ ...previewOptions, canRenderPdf: true }), "pdf") assert.equal(getObjectPreviewMode({ ...previewOptions, canRenderImage: true }), "image") assert.equal(getObjectPreviewMode({ ...previewOptions, canRenderText: true }), "text") @@ -43,7 +44,7 @@ test("object preview preserves non-media dispatch behavior", () => { test("object preview renders native media controls without relaxing the fallback sandbox", () => { const source = fs.readFileSync("components/object/preview-modal.tsx", "utf8") - assert.match(source, /const previewMode = getObjectPreviewMode\(\{/) + assert.match(source, /const initialPreviewMode = getObjectPreviewMode\(\{/) assert.match(source, /switch \(previewMode\)/) assert.match(source, /case "audio":[\s\S]* setAudioLoadError\(true\)\}/) @@ -80,10 +81,10 @@ test("object preview modal only uses the PDF viewer for application/pdf content" }) test("object text preview aborts stale requests and rejects non-success responses", () => { - const source = fs.readFileSync("components/object/preview-modal.tsx", "utf8") + const source = fs.readFileSync("hooks/use-object-preview-text.ts", "utf8") assert.match(source, /const controller = new AbortController\(\)/) - assert.match(source, /fetch\(previewUrl, \{ signal: controller\.signal \}\)/) + assert.match(source, /fetch\(url, \{ signal: controller\.signal \}\)/) assert.match(source, /if \(!response\.ok\) throw new Error/) assert.match(source, /return \(\) => controller\.abort\(\)/) }) diff --git a/tests/lib/object-preview.test.ts b/tests/lib/object-preview.test.ts new file mode 100644 index 00000000..9692928e --- /dev/null +++ b/tests/lib/object-preview.test.ts @@ -0,0 +1,151 @@ +import test from "node:test" +import assert from "node:assert/strict" +import { + getObjectPreviewMode, + createObjectPreviewInfo, + isSafeTextPreview, + MAX_TEXT_PREVIEW_BYTES, + readObjectPreviewText, +} from "../../lib/object-preview" + +const previewOptions = { + hasPreviewUrl: true, + contentType: "application/octet-stream", + objectSize: 20, + canRenderText: false, + canRenderImage: false, + canRenderPdf: false, + canRenderParquet: false, + canRenderTiff: false, +} + +test("preview metadata keeps the selected key when a HEAD response has no object key", () => { + assert.equal(createObjectPreviewInfo("folder/fred", { ContentLength: 20 }).Key, "folder/fred") +}) + +test("version preview keeps the selected version when response headers do not expose it", () => { + assert.equal(createObjectPreviewInfo("fred", { SignedUrl: "/fred?versionId=old" }, "old").VersionId, "old") +}) + +test("ordinary previews preserve the version returned by HEAD", () => { + assert.equal(createObjectPreviewInfo("fred", { VersionId: "current" }).VersionId, "current") +}) + +for (const contentType of [ + "", + "application/octet-stream", + "binary/octet-stream", + " Application/Octet-Stream; foo=bar ", +]) { + test(`small objects with ${contentType || "missing MIME metadata"} are checked for text`, () => { + assert.equal(getObjectPreviewMode({ ...previewOptions, contentType }), "detect-text") + }) +} + +test("generic objects above the preview limit offer download without probing", () => { + assert.equal(getObjectPreviewMode({ ...previewOptions, objectSize: MAX_TEXT_PREVIEW_BYTES + 1 }), "download") +}) + +test("generic objects with invalid sizes are not probed", () => { + for (const objectSize of [NaN, Infinity, -1]) { + assert.equal(getObjectPreviewMode({ ...previewOptions, objectSize }), "download") + } +}) + +test("recognized renderers take priority over generic MIME detection", () => { + assert.equal(getObjectPreviewMode({ ...previewOptions, canRenderImage: true }), "image") + assert.equal(getObjectPreviewMode({ ...previewOptions, canRenderParquet: true }), "parquet") + assert.equal(getObjectPreviewMode({ ...previewOptions, canRenderTiff: true }), "tiff") +}) + +test("explicit content types retain the sandbox renderer", () => { + assert.equal(getObjectPreviewMode({ ...previewOptions, contentType: "text/html" }), "sandbox") +}) + +test("existing text MIME and extension previews retain their size limit", () => { + assert.equal(isSafeTextPreview("text/plain", "fred", 20), true) + assert.equal(isSafeTextPreview("application/octet-stream", "fred.TXT", 20), true) + assert.equal(isSafeTextPreview("application/octet-stream", "fred", 20), false) + assert.equal(isSafeTextPreview("text/plain", "fred", MAX_TEXT_PREVIEW_BYTES + 1), false) +}) + +test("extensionless UTF-8 text is decoded completely, including non-ASCII content", async () => { + const text = "this is a text file\n中文日志\t🙂\r\n" + assert.equal(await readObjectPreviewText(new Response(text)), text) +}) + +test("UTF-8 sequences split across network chunks are preserved", async () => { + const bytes = new TextEncoder().encode("日志🙂") + const body = new ReadableStream({ + start(controller) { + for (const byte of bytes) controller.enqueue(Uint8Array.of(byte)) + controller.close() + }, + }) + assert.equal(await readObjectPreviewText(new Response(body)), "日志🙂") +}) + +test("HTML-looking content stays literal text", async () => { + const text = '' + assert.equal(await readObjectPreviewText(new Response(text)), text) +}) + +test("binary control characters anywhere in the file reject text preview", async () => { + assert.equal(await readObjectPreviewText(new Response("a".repeat(10000) + "\u0000binary")), null) +}) + +test("invalid and incomplete UTF-8 reject text preview", async () => { + for (const bytes of [Uint8Array.of(0xff, 0xfe), Uint8Array.of(0xe4, 0xb8)]) { + assert.equal(await readObjectPreviewText(new Response(bytes)), null) + } +}) + +test("an empty object offers download instead of a blank text area", async () => { + assert.equal(await readObjectPreviewText(new Response("")), null) +}) + +test("a BOM-only object is empty, while BOM-prefixed text is previewed", async () => { + assert.equal(await readObjectPreviewText(new Response(Uint8Array.of(0xef, 0xbb, 0xbf))), null) + assert.equal(await readObjectPreviewText(new Response("\uFEFF日志")), "日志") +}) + +test("text exactly at the limit can be previewed", async () => { + const text = "a".repeat(MAX_TEXT_PREVIEW_BYTES) + assert.equal(await readObjectPreviewText(new Response(text)), text) +}) + +test("oversized response headers cancel the body before reading", async () => { + let cancelled = false + const body = new ReadableStream({ + cancel: () => { + cancelled = true + }, + }) + const response = new Response(body, { headers: { "Content-Length": String(MAX_TEXT_PREVIEW_BYTES + 1) } }) + assert.equal(await readObjectPreviewText(response), null) + assert.equal(cancelled, true) +}) + +test("actual response bytes enforce the limit when length metadata is wrong", async () => { + let cancelled = false + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array(MAX_TEXT_PREVIEW_BYTES + 1)) + }, + cancel: () => { + cancelled = true + }, + }) + assert.equal(await readObjectPreviewText(new Response(body, { headers: { "Content-Length": "20" } })), null) + assert.equal(cancelled, true) +}) + +test("network stream failures remain errors rather than unsupported content", async () => { + const failure = new Error("Connection lost") + const body = new ReadableStream({ + start(controller) { + controller.error(failure) + }, + }) + await assert.rejects(readObjectPreviewText(new Response(body)), failure) +}) From 2ada331411148a57883995b76abb7a073deba497 Mon Sep 17 00:00:00 2001 From: cxymds Date: Tue, 10 Nov 2026 09:58:43 +0800 Subject: [PATCH 2/2] fix: align preview downloads with version permissions Keep ordinary downloads unversioned instead of inheriting HEAD metadata. Track explicitly selected versions separately and require object-scoped GetObjectVersion permission for those downloads. --- app/(dashboard)/browser/content.tsx | 10 +++-- lib/object-preview.ts | 2 +- lib/permission-capabilities.ts | 2 + tests/lib/object-preview-permissions.test.ts | 43 ++++++++++++++++++++ tests/lib/object-preview.test.ts | 23 +++++++++-- 5 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 tests/lib/object-preview-permissions.test.ts diff --git a/app/(dashboard)/browser/content.tsx b/app/(dashboard)/browser/content.tsx index 0bd52499..76372aad 100644 --- a/app/(dashboard)/browser/content.tsx +++ b/app/(dashboard)/browser/content.tsx @@ -100,19 +100,23 @@ export function BrowserContent({ bucketName, keyPath = "", preview = false, prev ) const previewObjectKey = typeof previewObject?.Key === "string" ? previewObject.Key : "" + const previewVersionId = + typeof previewObject?.SelectedVersionId === "string" ? previewObject.SelectedVersionId : undefined const canDownloadPreview = previewBucketName === bucketName && - canCapability("objects.download", { bucket: bucketName, objectKey: previewObjectKey }) + canCapability(previewVersionId ? "objects.version.download" : "objects.download", { + bucket: bucketName, + objectKey: previewObjectKey, + }) const downloadPreview = async () => { if (!canDownloadPreview || !previewObjectKey) return try { const filename = previewObjectKey.split("/").pop() ?? "" - const versionId = typeof previewObject?.VersionId === "string" ? previewObject.VersionId : undefined const url = await objectApi.getSignedUrl( previewObjectKey, 3600, getAttachmentContentDisposition(filename), - versionId, + previewVersionId, ) downloadUrl(url, filename) message.success(t("Download ready")) diff --git a/lib/object-preview.ts b/lib/object-preview.ts index 35beb113..ebcc165d 100644 --- a/lib/object-preview.ts +++ b/lib/object-preview.ts @@ -42,7 +42,7 @@ export function normalizePreviewContentType(contentType: string) { } export function createObjectPreviewInfo(key: string, info: T, versionId?: string) { - return { ...info, Key: key, ...(versionId === undefined ? {} : { VersionId: versionId }) } + return { ...info, Key: key, ...(versionId === undefined ? {} : { SelectedVersionId: versionId }) } } export function isSafeTextPreview(contentType: string, objectKey: string, objectSize: number) { diff --git a/lib/permission-capabilities.ts b/lib/permission-capabilities.ts index 6bb1ec2d..f1d1645c 100644 --- a/lib/permission-capabilities.ts +++ b/lib/permission-capabilities.ts @@ -40,6 +40,7 @@ export type ConsoleCapability = | "objects.legalHold.edit" | "objects.retention.edit" | "objects.version.view" + | "objects.version.download" | "objects.version.restore" | "objects.share" | "accessKeys.create" @@ -101,6 +102,7 @@ const CAPABILITY_REQUIREMENTS: Record ({ + Version: "2012-10-17", + Statement: [{ Effect: "Allow", Action: [action], Resource: ["arn:aws:s3:::preview-bucket/folder/fred"] }], +}) + +test("current-object read permission allows ordinary preview downloads", () => { + assert.equal(hasConsoleCapability(policy("s3:GetObject"), "objects.download", context), true) +}) + +test("current-object read permission does not allow selected-version downloads", () => { + assert.equal(hasConsoleCapability(policy("s3:GetObject"), "objects.version.download", context), false) +}) + +test("selected-version downloads use version read permission without requiring current-object read", () => { + assert.equal(hasConsoleCapability(policy("s3:GetObjectVersion"), "objects.version.download", context), true) +}) + +test("version download permission is scoped to the selected object", () => { + assert.equal( + hasConsoleCapability(policy("s3:GetObjectVersion"), "objects.version.download", { + ...context, + objectKey: "private/fred", + }), + false, + ) +}) + +test("an explicit version-read denial overrides a wildcard object-read grant", () => { + const denied: ConsolePolicy = { + Version: "2012-10-17", + Statement: [ + { Effect: "Allow", Action: ["s3:GetObject*"], Resource: ["arn:aws:s3:::preview-bucket/*"] }, + { Effect: "Deny", Action: ["s3:GetObjectVersion"], Resource: ["arn:aws:s3:::preview-bucket/folder/fred"] }, + ], + } + assert.equal(hasConsoleCapability(denied, "objects.version.download", context), false) +}) diff --git a/tests/lib/object-preview.test.ts b/tests/lib/object-preview.test.ts index 9692928e..ebb1354f 100644 --- a/tests/lib/object-preview.test.ts +++ b/tests/lib/object-preview.test.ts @@ -24,11 +24,28 @@ test("preview metadata keeps the selected key when a HEAD response has no object }) test("version preview keeps the selected version when response headers do not expose it", () => { - assert.equal(createObjectPreviewInfo("fred", { SignedUrl: "/fred?versionId=old" }, "old").VersionId, "old") + assert.equal(createObjectPreviewInfo("fred", { SignedUrl: "/fred?versionId=old" }, "old").SelectedVersionId, "old") }) -test("ordinary previews preserve the version returned by HEAD", () => { - assert.equal(createObjectPreviewInfo("fred", { VersionId: "current" }).VersionId, "current") +test("ordinary previews do not select the version returned by HEAD for download", () => { + assert.equal(createObjectPreviewInfo("fred", { VersionId: "current" }).SelectedVersionId, undefined) +}) + +test("an explicitly selected version stays separate from HEAD metadata", () => { + assert.deepEqual(createObjectPreviewInfo("fred", { VersionId: "head-version" }, "selected-version"), { + Key: "fred", + VersionId: "head-version", + SelectedVersionId: "selected-version", + }) +}) + +test("passing version preview metadata to the browser preserves the explicit selection", () => { + const data = createObjectPreviewInfo("fred", { SignedUrl: "/fred?versionId=old" }, "old") + assert.equal(createObjectPreviewInfo("fred", data).SelectedVersionId, "old") +}) + +test("a normalized zero version does not inherit the version returned by HEAD", () => { + assert.equal(createObjectPreviewInfo("fred", { VersionId: "head-version" }, undefined).SelectedVersionId, undefined) }) for (const contentType of [