diff --git a/src/routes/uploadQuarantine.test.ts b/src/routes/uploadQuarantine.test.ts index 9edc153c..01235409 100644 --- a/src/routes/uploadQuarantine.test.ts +++ b/src/routes/uploadQuarantine.test.ts @@ -14,6 +14,7 @@ import { getFile } from "../db/sqlite/messages"; import { createServerConfigIfNotExists, setServerRole } from "../db/sqlite/servers"; import { getUserByServerId, upsertUser } from "../db/sqlite/users"; import { refreshWorkerCapabilities } from "../services/workerCapabilities"; +import { buildMemberList } from "../socket/utils/clients"; import { initStorage, putObject } from "../storage"; import { generateAccessToken, generateFileToken } from "../utils/jwt"; @@ -117,4 +118,29 @@ describe("uploads with a worker that clears quarantine", () => { const fileId = (await upload(carol, "banner")).body.bannerFileId as string; assert.ok((await getFile(fileId))?.s3_key.startsWith("banners/")); }); + + it("takes a banner video only from a worker that transcodes, and never decodes it here", async () => { + const dave = await member("dave"); + const sendVideo = async () => { + const form = new FormData(); + form.append("file", new Blob([new Uint8Array(Buffer.from("not decoded by the server"))], { type: "video/mp4" }), "clip.mp4"); + const res = await fetch(`${base}/api/uploads/banner`, { method: "POST", headers: auth(dave), body: form }); + return { status: res.status, body: (await res.json()) as Record }; + }; + + await workerSays(["quarantine-v1"]); + const refused = await sendVideo(); + assert.equal(refused.status, 415); + assert.equal(refused.body.error, "video_unsupported"); + + await workerSays(["quarantine-v1", "video-v1"]); + const taken = await sendVideo(); + assert.equal(taken.status, 201, JSON.stringify(taken.body)); + const file = await getFile(taken.body.bannerFileId as string); + assert.ok(file?.s3_key.startsWith("quarantine/banners/")); + assert.equal(file?.mime, "video/mp4"); + const row = (await buildMemberList({})).find((m) => m.serverUserId === dave.serverUserId); + assert.equal(row?.bannerVideo, true, "the member list says it plays"); + assert.equal(row?.avatarVideo, false); + }); }); diff --git a/src/routes/uploads.ts b/src/routes/uploads.ts index 8e47b444..a683aeb9 100644 --- a/src/routes/uploads.ts +++ b/src/routes/uploads.ts @@ -24,7 +24,10 @@ import { quarantineUpload, waitOutOfQuarantine } from "../services/quarantineUpl /** How long a read waits for the worker to write a fresh upload out; a test sets it short. */ const QUARANTINE_WAIT_MS = Number(process.env.GRYT_QUARANTINE_WAIT_MS) || 15_000; import { storeUploadedFile } from "../services/storeUploadedFile"; -import { workerClearsQuarantine } from "../services/workerCapabilities"; +import { workerClearsQuarantine, workerTranscodesVideo } from "../services/workerCapabilities"; + +/** What a banner or avatar video may be sent as; the worker writes all of them out as AV1 MP4. */ +const VIDEO_TYPES = new Set(["video/mp4", "video/webm", "video/quicktime"]); /** SVG is on this list only because it has been through sanitizeSvg(), is drawn through ``, and is sandboxed by the CSP below. That header is required. */ @@ -63,7 +66,10 @@ function uploadAvatarToMemory(field: string) { Promise.resolve() .then(async () => { const cfg = await getServerConfig().catch(() => null); - const maxBytes = typeof cfg?.avatar_max_bytes === "number" ? cfg.avatar_max_bytes : DEFAULT_AVATAR_MAX_BYTES; + const avatarMax = typeof cfg?.avatar_max_bytes === "number" ? cfg.avatar_max_bytes : DEFAULT_AVATAR_MAX_BYTES; + const uploadMax = typeof cfg?.upload_max_bytes === "number" ? cfg.upload_max_bytes : DEFAULT_UPLOAD_MAX_BYTES; + // A video banner or avatar follows the upload limit; a picture is held to the avatar one below. + const maxBytes = workerTranscodesVideo() && avatarMax > 0 && uploadMax > 0 ? Math.max(avatarMax, uploadMax) : avatarMax; const limits = typeof maxBytes === "number" && maxBytes > 0 ? { fileSize: maxBytes } : undefined; multer({ storage: multer.memoryStorage(), limits }).single(field)(req, res, next); }) @@ -150,7 +156,12 @@ const storeAvatarImage = (purpose: "avatar" | "group" | "webhook" | "banner") => (req: Request, res: Response, next: NextFunction): void => { const file = req.file; if (!file) { res.status(400).json({ error: "file_required", message: "file is required" }); return; } - if (!(file.mimetype || "").startsWith("image/")) { res.status(400).json({ error: "invalid_file", message: "Only image files are allowed" }); return; } + const isVideo = VIDEO_TYPES.has((file.mimetype || "").toLowerCase()) && (purpose === "avatar" || purpose === "banner"); + if (isVideo && !workerTranscodesVideo()) { + res.status(415).json({ error: "video_unsupported", message: "This server can't take a video here. Use a picture instead." }); + return; + } + if (!isVideo && !(file.mimetype || "").startsWith("image/")) { res.status(400).json({ error: "invalid_file", message: "Only image files are allowed" }); return; } const what = { avatar: "Avatar", group: "Group picture", webhook: "Webhook avatar", banner: "Banner" }[purpose]; const disableS3 = (process.env.DISABLE_S3 || "").toLowerCase() === "true"; @@ -164,7 +175,9 @@ const storeAvatarImage = (purpose: "avatar" | "group" | "webhook" | "banner") => Promise.resolve() .then(async () => { const cfg = await getServerConfig().catch(() => null); - const maxBytes = (typeof cfg?.avatar_max_bytes === "number" ? cfg.avatar_max_bytes : DEFAULT_AVATAR_MAX_BYTES); + const maxBytes = isVideo + ? (typeof cfg?.upload_max_bytes === "number" ? cfg.upload_max_bytes : DEFAULT_UPLOAD_MAX_BYTES) + : (typeof cfg?.avatar_max_bytes === "number" ? cfg.avatar_max_bytes : DEFAULT_AVATAR_MAX_BYTES); // No exemption for animated files: a file over the limit is refused // whatever is in it. @@ -215,7 +228,8 @@ const storeAvatarImage = (purpose: "avatar" | "group" | "webhook" | "banner") => } // With a worker that clears quarantine, the server never decodes the file itself. - const stored = (purpose === "avatar" || purpose === "banner") && workerClearsQuarantine() + // A video only ever gets here with a worker that transcodes it. + const stored = (purpose === "avatar" || purpose === "banner") && (isVideo || workerClearsQuarantine()) ? { ok: true as const, processing: true, ...(await quarantineUpload({ bucket, use: purpose === "avatar" ? "avatars" : "banners", diff --git a/src/services/workerCapabilities.ts b/src/services/workerCapabilities.ts index 17d82e14..31d33d00 100644 --- a/src/services/workerCapabilities.ts +++ b/src/services/workerCapabilities.ts @@ -5,21 +5,33 @@ import consola from "consola"; const REFRESH_MS = 60_000; let clears = false; +let video = false; let timer: NodeJS.Timeout | null = null; export function workerClearsQuarantine(): boolean { return clears; } +/** The worker transcodes banner and avatar videos in its jail; without it, video is refused. */ +export function workerTranscodesVideo(): boolean { + return video; +} + export async function refreshWorkerCapabilities(url = process.env.IMAGE_WORKER_URL, fetchImpl: typeof fetch = fetch): Promise { - if (!url) return (clears = false); + if (!url) { + video = false; + return (clears = false); + } try { const res = await fetchImpl(`${url.replace(/\/$/, "")}/health`, { signal: AbortSignal.timeout(5_000) }); const body = res.ok ? ((await res.json()) as { capabilities?: unknown }) : null; - const next = Array.isArray(body?.capabilities) && body.capabilities.includes("quarantine-v1"); + const caps: unknown[] = Array.isArray(body?.capabilities) ? body.capabilities : []; + const next = caps.includes("quarantine-v1"); + video = next && caps.includes("video-v1"); if (next !== clears) consola.info(`[uploads] Quarantine through the image worker ${next ? "on" : "off"}`); return (clears = next); } catch { + video = false; return (clears = false); } } diff --git a/src/socket/utils/clients.ts b/src/socket/utils/clients.ts index 0020d14b..dfd96092 100644 --- a/src/socket/utils/clients.ts +++ b/src/socket/utils/clients.ts @@ -201,13 +201,14 @@ export async function buildMemberList(clientsInfo: Clients) { // list carries all of them so a client can draw the rest as chips. const rolesByMember = await listRolesByMember(); - // Avatar colours, so a voice tile can match the person rather than a hash of - // their id. Null until the image worker has been round; the client falls back. + // Avatar colours for voice tiles, and each avatar and banner's type, so a client + // knows whether it plays (GRYT-1664). Colours are null until the worker has been round. const avatarFiles = await getFilesByIds( registeredUsers - .map((u) => u.avatar_file_id) + .flatMap((u) => [u.avatar_file_id, u.banner_file_id]) .filter((id): id is string => Boolean(id)), ); + const isVideo = (fileId: string | null | undefined) => !!fileId && !!avatarFiles.get(fileId)?.mime?.startsWith("video/"); // Checked now rather than at upload, so taking the permission away takes the banner down too. const bannerShown = new Set(); @@ -260,6 +261,9 @@ export async function buildMemberList(clientsInfo: Clients) { ? avatarFiles.get(user.avatar_file_id)?.dominant_color ?? null : null, bannerFileId: bannerShown.has(user.server_user_id) ? user.banner_file_id : null, + // True when the file plays rather than shows; its thumbnail is the still poster. + avatarVideo: isVideo(user.avatar_file_id), + bannerVideo: bannerShown.has(user.server_user_id) && isVideo(user.banner_file_id), // `avatarFileId` is still set, because saving a design uploads a PNG // that an older client shows. Passed through as stored. avatarWorn: user.avatar_worn, @@ -321,6 +325,8 @@ export function memberStateHash(members: MemberListEntry[]): string { avatarFileId: m.avatarFileId, avatarColor: m.avatarColor, bannerFileId: m.bannerFileId, + avatarVideo: m.avatarVideo, + bannerVideo: m.bannerVideo, // Designing a new owl changes nothing else about a member, so without // this line it would change nothing anybody sees. avatarWorn: m.avatarWorn,