From 17cb1d1375c5415bae46f83ccb819262d14c644b Mon Sep 17 00:00:00 2001 From: Sivert Date: Tue, 15 Sep 2026 08:59:13 +0200 Subject: [PATCH] Choosing a group picture replaced your own avatar (GRYT-1182) GroupDialog uploaded the picture through /api/uploads/avatar, which also sets it as the uploader's avatar on that server. It uses the new /api/uploads/group-icon route now. A server without that route answers 404. The client doesn't retry on the avatar route, since that would bring the bug back. It says the server needs an update and the group keeps its drawn egg. It reads fileId from the reply, which the SVG path always sent, so an SVG group picture no longer errors either. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 1 + package.json | 1 + scripts/check-group-picture-upload.mjs | 80 +++++++++++++++++++ .../socket/src/components/GroupDialog.tsx | 21 +---- .../socket/src/utils/uploadGroupPicture.ts | 30 +++++++ 5 files changed, 116 insertions(+), 17 deletions(-) create mode 100644 scripts/check-group-picture-upload.mjs create mode 100644 src/packages/socket/src/utils/uploadGroupPicture.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62d89986..4d1ac26a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,6 +89,7 @@ jobs: - run: yarn test:card-byline - run: yarn test:link-preview-retry - run: yarn test:stable-attachment-urls + - run: yarn test:group-picture-upload - run: yarn test:member-invite - run: yarn test:addon-capabilities - run: yarn test:actor-name diff --git a/package.json b/package.json index c4595cfd..a1b90300 100644 --- a/package.json +++ b/package.json @@ -103,6 +103,7 @@ "test:card-byline": "node scripts/check-card-byline.mjs", "test:link-preview-retry": "node scripts/check-link-preview-retry.mjs", "test:stable-attachment-urls": "node --disable-warning=ExperimentalWarning scripts/check-stable-attachment-urls.mjs", + "test:group-picture-upload": "node --disable-warning=ExperimentalWarning scripts/check-group-picture-upload.mjs", "test:examples": "node --test examples/examples.test.mjs", "test:member-invite": "node scripts/check-member-invite.mjs", "test:addon-capabilities": "node scripts/check-addon-capabilities.mjs", diff --git a/scripts/check-group-picture-upload.mjs b/scripts/check-group-picture-upload.mjs new file mode 100644 index 00000000..d396ae45 --- /dev/null +++ b/scripts/check-group-picture-upload.mjs @@ -0,0 +1,80 @@ +/* eslint-env node */ + +// A group's picture went through the avatar upload, which also made it the uploader's +// own avatar. It has its own route now, and an older server without it gets no picture. GRYT-1182. + +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { stripTypeScriptTypes } from "node:module"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const root = join(dirname(fileURLToPath(import.meta.url)), ".."); +const read = (path) => readFileSync(join(root, path), "utf8"); + +const HELPER = "src/packages/socket/src/utils/uploadGroupPicture.ts"; +const DIALOG = "src/packages/socket/src/components/GroupDialog.tsx"; + +const helperSource = stripTypeScriptTypes(read(HELPER)).replace(/^export /gm, ""); +const load = (fetch) => + new Function("fetch", `${helperSource}\nreturn { uploadGroupPicture, GROUP_PICTURE_NEEDS_UPDATE };`)(fetch); + +function fakeServer(answer) { + const calls = []; + const fetch = async (url, init) => { + calls.push({ url, init }); + const { status, body } = answer(url); + return { + status, + ok: status >= 200 && status < 300, + json: async () => { + if (typeof body === "string") throw new SyntaxError("not json"); + return body; + }, + }; + }; + return { calls, fetch }; +} + +const picture = new Blob([new Uint8Array([1, 2, 3])], { type: "image/png" }); + +/* ── a current server ───────────────────────────────────────────────────── */ +{ + const server = fakeServer(() => ({ status: 201, body: { fileId: "f-1", processing: false } })); + const { uploadGroupPicture } = load(server.fetch); + + assert.equal(await uploadGroupPicture("http://h", "tok", picture, "g.png"), "f-1"); + assert.equal(server.calls.length, 1); + assert.equal(server.calls[0].url, "http://h/api/uploads/group-icon"); + assert.equal(server.calls[0].init.headers.Authorization, "Bearer tok"); + assert.ok(server.calls[0].init.body.get("file"), "the picture was not in the form"); +} + +/* ── a refusal carries the server's message ─────────────────────────────── */ +{ + const server = fakeServer(() => ({ status: 413, body: { error: "file_too_large", message: "Group picture too large. Max 5.0MB." } })); + const { uploadGroupPicture } = load(server.fetch); + await assert.rejects(uploadGroupPicture("http://h", "tok", picture, "g.png"), /Group picture too large/); +} + +/* ── an older server: no route, and no second try at the avatar one ─────── */ +{ + const server = fakeServer((url) => + url.endsWith("/api/uploads/avatar") + ? { status: 201, body: { avatarFileId: "would-be-your-avatar" } } + : { status: 404, body: "
Cannot POST /api/uploads/group-icon
" }, + ); + const { uploadGroupPicture, GROUP_PICTURE_NEEDS_UPDATE } = load(server.fetch); + + await assert.rejects(uploadGroupPicture("http://h", "tok", picture, "g.png"), (e) => e.message === GROUP_PICTURE_NEEDS_UPDATE); + assert.deepEqual(server.calls.map((c) => c.url), ["http://h/api/uploads/group-icon"], "it fell back to the avatar route"); +} + +/* ── the dialog uses it ─────────────────────────────────────────────────── */ +{ + const dialog = read(DIALOG); + assert.match(dialog, /await uploadGroupPicture\(/, "GroupDialog no longer uploads through uploadGroupPicture"); + assert.doesNotMatch(dialog, /\/api\/uploads\/avatar/, "GroupDialog talks to the avatar route again"); +} + +console.log("group picture upload: ok"); diff --git a/src/packages/socket/src/components/GroupDialog.tsx b/src/packages/socket/src/components/GroupDialog.tsx index f507e5de..64a0157d 100644 --- a/src/packages/socket/src/components/GroupDialog.tsx +++ b/src/packages/socket/src/components/GroupDialog.tsx @@ -10,6 +10,7 @@ import { } from "@/common"; import { conversationTitle, type DirectConversation } from "../hooks/useDirectMessages"; +import { uploadGroupPicture } from "../utils/uploadGroupPicture"; import { EmojiText } from "./EmojiText"; import type { MemberInfo } from "./MemberSidebar"; @@ -112,23 +113,9 @@ export const GroupDialog = ({ try { const token = getServerAccessToken(serverHost); if (!token) throw new Error("Not signed in to this server"); - const form = new FormData(); - form.append("file", file, file.name || "group.png"); - /* The avatar endpoint, because a group picture is the same job. A second - endpoint is a second place for the limits to drift. */ - const response = await fetch(`${getServerHttpBase(serverHost)}/api/uploads/avatar`, { - method: "POST", - headers: { Authorization: `Bearer ${token}` }, - body: form, - }); - const data = (await response.json().catch(() => ({}))) as { - avatarFileId?: string; - message?: string; - }; - if (!response.ok || !data.avatarFileId) { - throw new Error(data.message || "The server would not take that picture"); - } - setIconFileId(data.avatarFileId); + setIconFileId( + await uploadGroupPicture(getServerHttpBase(serverHost), token, file, file.name || "group.png"), + ); } catch (error) { toast.error(error instanceof Error ? error.message : "Could not upload that"); } finally { diff --git a/src/packages/socket/src/utils/uploadGroupPicture.ts b/src/packages/socket/src/utils/uploadGroupPicture.ts new file mode 100644 index 00000000..ae315137 --- /dev/null +++ b/src/packages/socket/src/utils/uploadGroupPicture.ts @@ -0,0 +1,30 @@ +/** Shown when the server predates `/api/uploads/group-icon`. */ +export const GROUP_PICTURE_NEEDS_UPDATE = + "This server needs an update before groups can have their own picture."; + +/** Never falls back to the avatar route, which made the picture your own avatar + too (GRYT-1182). An older server gets no group picture instead. */ +export async function uploadGroupPicture( + base: string, + accessToken: string, + file: Blob, + filename: string, +): Promise { + const form = new FormData(); + form.append("file", file, filename); + const response = await fetch(`${base}/api/uploads/group-icon`, { + method: "POST", + headers: { Authorization: `Bearer ${accessToken}` }, + body: form, + }); + if (response.status === 404) throw new Error(GROUP_PICTURE_NEEDS_UPDATE); + + const data = (await response.json().catch(() => ({}))) as { + fileId?: string; + message?: string; + }; + if (!response.ok || !data.fileId) { + throw new Error(data.message || "The server would not take that picture"); + } + return data.fileId; +}