diff --git a/docs/api.md b/docs/api.md index 82ce842..01badc8 100644 --- a/docs/api.md +++ b/docs/api.md @@ -27,7 +27,7 @@ Consult `openapi.json` for every field and response. | Signals | `GET /signals` | `PUT` or `DELETE /signals/{sub}/{remoteId}` | | Mentions | `GET /mentions?direction=inbound` or `outbound` | `PATCH /mentions/{id}` with `hidden` | | Settings | `GET /settings` | `PATCH /settings` | -| Media | Included in item detail | `POST /media` with a multipart file (`inline=true` when the client places it in the text), `DELETE /media/{id}` (detaches; deletes the file only when no published version shows it) | +| Media | Included in item detail | `POST /media` with a multipart file (`inline=true` when the client places it in the text; the same file sent again to the same item in the same placement returns the existing row with `200` and `duplicate: true`), `DELETE /media/{id}` (detaches; deletes the file only when no published version shows it) | | Imported items | `GET /imports/{sub}/{id}`, `GET /imports/{sub}/{id}/history`, `GET /imports/{sub}/{id}/versions/{v}` (history and public versions, read from the origin) | The subscription importer manages these items | | Reading | `GET /reading`, `GET /reading/imported` (imported items only, cursor-paged; see below) | Read only | | Read state | `readVersion` on each imported Reading entry | `PUT`/`DELETE /reading/{sub}/{remoteId}/read`, `POST /reading/read`, `POST /reading/unread` | diff --git a/migrations/0028_media_sha256.sql b/migrations/0028_media_sha256.sql new file mode 100644 index 0000000..9041894 --- /dev/null +++ b/migrations/0028_media_sha256.sql @@ -0,0 +1,21 @@ +-- The upload guard (blygger-studio#7): a SHA-256 of an attachment's bytes, so +-- an identical file uploaded again to the same item returns the existing row +-- instead of storing a second copy. NULL for rows uploaded before this +-- migration (not backfilled) and for uploads not attached to an item. +ALTER TABLE media ADD COLUMN sha256 TEXT; +CREATE INDEX media_item_sha256 ON media(item_id, sha256); + +-- The polling cache's change trigger (0024) lists every column it watches, and +-- an oracle test holds it to the table's full column set. Recreated with +-- `sha256` added; otherwise identical to 0024's. +DROP TRIGGER IF EXISTS change_media_update; +CREATE TRIGGER change_media_update +AFTER UPDATE ON media +WHEN OLD."alt" IS NOT NEW."alt" OR OLD."created" IS NOT NEW."created" OR OLD."id" IS NOT NEW."id" OR OLD."inline" IS NOT NEW."inline" OR OLD."item_id" IS NOT NEW."item_id" OR OLD."mime" IS NOT NEW."mime" OR OLD."r2_key" IS NOT NEW."r2_key" OR OLD."sha256" IS NOT NEW."sha256" +BEGIN + SELECT RAISE(ABORT, 'missing change state') WHERE NOT EXISTS(SELECT 1 FROM change_state WHERE id=1); + UPDATE change_state SET + items=items+(1), + feed=feed+(1) + WHERE id=1; +END; diff --git a/openapi.json b/openapi.json index 1ad20a4..cfb2449 100644 --- a/openapi.json +++ b/openapi.json @@ -5417,6 +5417,39 @@ } }, "responses": { + "200": { + "description": "Already attached", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "url": { + "type": "string" + }, + "mime": { + "type": "string" + }, + "duplicate": { + "type": "boolean", + "enum": [ + true + ] + } + }, + "required": [ + "id", + "url", + "mime", + "duplicate" + ] + } + } + } + }, "201": { "description": "Success", "content": { diff --git a/sdk/generated/types.gen.ts b/sdk/generated/types.gen.ts index 67ca1d3..8d3ea3d 100644 --- a/sdk/generated/types.gen.ts +++ b/sdk/generated/types.gen.ts @@ -1800,6 +1800,15 @@ export type UploadMediaErrors = { export type UploadMediaError = UploadMediaErrors[keyof UploadMediaErrors]; export type UploadMediaResponses = { + /** + * Already attached + */ + 200: { + id: string; + url: string; + mime: string; + duplicate: true; + }; /** * Success */ diff --git a/sdk/generated/zod.gen.ts b/sdk/generated/zod.gen.ts index d103ace..faa50d9 100644 --- a/sdk/generated/zod.gen.ts +++ b/sdk/generated/zod.gen.ts @@ -858,14 +858,19 @@ export const zUploadMediaBody = z.object({ inline: z.enum(['true', 'false']).optional() }); -/** - * Success - */ -export const zUploadMediaResponse = z.object({ - id: z.string(), - url: z.string(), - mime: z.string() -}); +export const zUploadMediaResponse = z.union([ + z.object({ + id: z.string(), + url: z.string(), + mime: z.string(), + duplicate: z.literal(true) + }), + z.object({ + id: z.string(), + url: z.string(), + mime: z.string() + }) +]); export const zDeleteMediaPath = z.object({ id: z.string().min(1) diff --git a/src/api.ts b/src/api.ts index 3b96a1b..9a705da 100644 --- a/src/api.ts +++ b/src/api.ts @@ -23,6 +23,7 @@ import { getMedia, getSettings, getVersion, + findDuplicateMedia, insertMedia, pinVersion, publish, @@ -47,7 +48,7 @@ import { BLOCKING, staleThreads, threadFreshness } from "./freshness.ts"; import { draftChangeNote } from "./change-note.ts"; import { platformFetchFor } from "./importer/http.ts"; import { reconcileIndex } from "./importer/poll.ts"; -import { isFollowableUrl, isValidTimeZone, newMediaId, normalizeMount, nowIso } from "./util.ts"; +import { hex, isFollowableUrl, isValidTimeZone, newMediaId, normalizeMount, nowIso } from "./util.ts"; const MEDIA_TYPES: Record = { "image/png": "png", @@ -408,9 +409,19 @@ api.openapi(routes.uploadMedia, async (c) => { if (typeof itemId === "string" && itemId && !(await getItem(c.env.DB, itemId))) { return c.json({ error: "item_id not found" }, 404); } + const bytes = await file.arrayBuffer(); + // The studio sends inline=true for an upload it places in the text + // (studio#24): shown only where its line is, never appended. + const inline = form?.get("inline") === "true" ? 1 : 0; + // The same file (bytes and type) uploaded to the same item in the same + // placement is the same attachment (studio#7): return the row already + // there, store nothing. + const sha256 = typeof itemId === "string" && itemId ? hex(new Uint8Array(await crypto.subtle.digest("SHA-256", bytes))) : null; + const existing = sha256 ? await findDuplicateMedia(c.env.DB, itemId as string, sha256, file.type, inline) : null; + if (existing) return c.json({ id: existing.id, url: existing.r2_key, mime: existing.mime, duplicate: true }, 200); const id = newMediaId(); const r2Key = `media/${id}.${ext}`; - await c.env.MEDIA.put(r2Key, await file.arrayBuffer(), { httpMetadata: { contentType: file.type } }); + await c.env.MEDIA.put(r2Key, bytes, { httpMetadata: { contentType: file.type } }); const alt = form?.get("alt"); const row = await insertMedia(c.env.DB, { id, @@ -418,10 +429,8 @@ api.openapi(routes.uploadMedia, async (c) => { r2_key: r2Key, mime: file.type, alt: typeof alt === "string" ? alt : null, - // The studio sends inline=true for an upload it places in the text - // (studio#24): shown only where its line is, never appended. - inline: form?.get("inline") === "true" ? 1 : 0, - }, c.get('draftOnlyMedia') === true); + inline, + }, c.get('draftOnlyMedia') === true, sha256); if (!row) { await c.env.MEDIA.delete(r2Key); c.header('WWW-Authenticate', 'Bearer error="insufficient_scope", scope="owner:draft owner:publish"'); diff --git a/src/contract/routes.ts b/src/contract/routes.ts index df153f3..c30b38e 100644 --- a/src/contract/routes.ts +++ b/src/contract/routes.ts @@ -136,5 +136,7 @@ export const routes = { // Read routes contributed by the extensions in this repository (extensions/catalog.ts). ...extensionRoutes, }; +// An identical file uploaded again to the same item returns the existing row (studio#7). +routes.uploadMedia.responses[200] = { description: "Already attached", content: json(z.object({ id: z.string(), url: z.string(), mime: z.string(), duplicate: z.literal(true) })) }; // The resolve/confirm operation has two successful response shapes and statuses. routes.createSubscription.responses[201] = { description: "Subscribed", content: json(subscribed) }; diff --git a/src/model.ts b/src/model.ts index 2101503..bcefe2c 100644 --- a/src/model.ts +++ b/src/model.ts @@ -648,17 +648,26 @@ export async function insertMedia( db: D1Database, row: Omit, unpublishedOnly = false, + sha256: string | null = null, ): Promise { const created = nowIso(); // Evaluate publication and insert in one statement. A publisher can run while // the R2 upload awaits; a separate earlier read is not an authorization gate. const result = await db - .prepare("INSERT INTO media (id, item_id, r2_key, mime, alt, created, inline) SELECT ?, ?, ?, ?, ?, ?, ? WHERE ? = 0 OR ? IS NULL OR EXISTS (SELECT 1 FROM items WHERE id = ? AND version = 0)") - .bind(row.id, row.item_id, row.r2_key, row.mime, row.alt, created, row.inline, unpublishedOnly ? 1 : 0, row.item_id, row.item_id) + .prepare("INSERT INTO media (id, item_id, r2_key, mime, alt, created, inline, sha256) SELECT ?, ?, ?, ?, ?, ?, ?, ? WHERE ? = 0 OR ? IS NULL OR EXISTS (SELECT 1 FROM items WHERE id = ? AND version = 0)") + .bind(row.id, row.item_id, row.r2_key, row.mime, row.alt, created, row.inline, sha256, unpublishedOnly ? 1 : 0, row.item_id, row.item_id) .run(); return result.meta.changes ? { ...row, created } : null; } +/** The item's earlier upload of the same file (bytes and type) in the same placement, if any (migration 0028, studio#7). */ +export async function findDuplicateMedia(db: D1Database, itemId: string, sha256: string, mime: string, inline: number): Promise { + return db + .prepare("SELECT id, item_id, r2_key, mime, alt, created, inline FROM media WHERE item_id = ? AND sha256 = ? AND mime = ? AND inline = ? ORDER BY created ASC LIMIT 1") + .bind(itemId, sha256, mime, inline) + .first(); +} + export async function getMedia(db: D1Database, id: string): Promise { return db.prepare("SELECT * FROM media WHERE id = ?").bind(id).first(); } diff --git a/test/mcp-media.oracle.test.ts b/test/mcp-media.oracle.test.ts index b03b616..e2119b5 100644 --- a/test/mcp-media.oracle.test.ts +++ b/test/mcp-media.oracle.test.ts @@ -73,7 +73,14 @@ describe('MCP multipart media transport parity', () => { expect(await (await d.fetch('/blyg/items/' + item.id + '.json')).json()).toEqual(before); expect(await storedCount()).toEqual({ r2: 0, rows: 0 }); const permitted = await d.credential(['owner:draft', 'owner:publish']); - expect((await result(await d.upload(permitted, { file: { filename: 'boundary.png', contentType: 'image/png', dataBase64: 'aW1hZ2U=' }, item_id: item.id }))).error).toBe(false); + const attached = await result(await d.upload(permitted, { file: { filename: 'boundary.png', contentType: 'image/png', dataBase64: 'aW1hZ2U=' }, item_id: item.id })); + expect(attached.error).toBe(false); + // The same file again under draft scope is the attached row (studio#7): returned, nothing written. + const stored = await storedCount(), after = await (await d.fetch('/blyg/items/' + item.id + '.json')).json(); + const again = await result(await d.upload(token, { file: { filename: 'boundary.png', contentType: 'image/png', dataBase64: 'aW1hZ2U=' }, item_id: item.id })); + expect(again.error).toBe(false); expect(again.value).toEqual({ ...attached.value, duplicate: true }); + expect(await storedCount()).toEqual(stored); + expect(await (await d.fetch('/blyg/items/' + item.id + '.json')).json()).toEqual(after); }); it('discovers uploadMedia for draft scopes', async () => { @@ -84,16 +91,19 @@ describe('MCP multipart media transport parity', () => { }); it('uses the same bytes, MIME types and attachment metadata as multipart REST', async () => { const d = await driver(), token = await d.credential(['owner:draft']); - const item = await (await d.owner('/api/items', { content_md: 'media parity' })).json() as any; + // One item per transport: the same file sent twice to one item is a + // duplicate (studio#7) and would not exercise MCP's own write. + const restItem = await (await d.owner('/api/items', { content_md: 'media parity (REST)' })).json() as any; + const mcpItem = await (await d.owner('/api/items', { content_md: 'media parity (MCP)' })).json() as any; const bytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 1, 2, 3]); for (const contentType of ['image/png', 'image/jpeg', 'image/gif', 'image/webp', 'image/svg+xml']) { - const extra = { item_id: item.id, alt: 'a diagram', inline: 'true' }; - const rest = await d.restUpload(bytes, contentType, extra); expect(rest.status).toBe(201); + const extra = { alt: 'a diagram', inline: 'true' }; + const rest = await d.restUpload(bytes, contentType, { item_id: restItem.id, ...extra }); expect(rest.status).toBe(201); const expected = await rest.json() as any; - const observed = await result(await d.upload(token, { file: { filename: 'same-file.dat', contentType, dataBase64: base64(bytes) }, ...extra })); - expect(observed.error).toBe(false); expect(observed.value.mime).toBe(expected.mime); + const observed = await result(await d.upload(token, { file: { filename: 'same-file.dat', contentType, dataBase64: base64(bytes) }, item_id: mcpItem.id, ...extra })); + expect(observed.error).toBe(false); expect(observed.value.mime).toBe(expected.mime); expect(observed.value.duplicate).toBeUndefined(); expect(observed.value.url.split('.').pop()).toBe(expected.url.split('.').pop()); - for (const value of [expected, observed.value]) { + for (const [value, item] of [[expected, restItem], [observed.value, mcpItem]]) { expect((await d.fetch('/blyg/' + value.url)).status).toBe(404); const publicBytes = await d.fetch('/blyg/' + value.url, { headers: { cookie: d.cookie } }); expect(publicBytes.status).toBe(200); expect(publicBytes.headers.get('content-type')).toBe(contentType); diff --git a/test/media-visibility-security.oracle.test.ts b/test/media-visibility-security.oracle.test.ts index 8233205..4092b2e 100644 --- a/test/media-visibility-security.oracle.test.ts +++ b/test/media-visibility-security.oracle.test.ts @@ -28,8 +28,8 @@ async function fixture() { const login = await receive('/blyg/studio/login', { method: 'POST', headers: { 'CF-Connecting-IP': 'fd00:' + crypto.randomUUID().replaceAll('-', '').match(/.{4}/g)!.slice(0,7).join(':') }, body: new URLSearchParams({ password: env.OWNER_PASSWORD }) }); expect(login.status).toBe(302); const cookie = login.headers.get('set-cookie')!.split(';')[0]; const owner = (path: string, method = 'GET', body?: unknown) => receive(path, { method, headers: { cookie, 'Content-Type': 'application/json' }, ...(body === undefined ? {} : { body: JSON.stringify(body) }) }); - const upload = async (item?: string, inline = false) => { - const form = new FormData(); form.set('file', new File(['private fixture bytes'], 'private.png', { type: 'image/png' })); + const upload = async (item?: string, inline = false, bytes = 'private fixture bytes') => { + const form = new FormData(); form.set('file', new File([bytes], 'private.png', { type: 'image/png' })); if (item) form.set('item_id', item); if (inline) form.set('inline', 'true'); const response = await receive('/api/media', { method: 'POST', headers: { cookie }, body: form }); expect(response.status).toBe(201); return response.json() as Promise<{id:string;url:string}>; @@ -75,7 +75,8 @@ it.each([false, true])('withdrawal keeps published media and leaves later upload // Attaching to an ever-published item is itself a publish act (publish scope); // an inline upload is public only once published text shows it. if (inline) { - const later = await f.upload(id, true); + // Other bytes: the same file again would be the published row (studio#7). + const later = await f.upload(id, true, 'later fixture bytes'); expect((await receive('/blyg/' + later.url)).status, 'an inline upload no published text showed stays private on a withdrawn item').toBe(404); } }); diff --git a/test/media.test.ts b/test/media.test.ts index f55bb77..e9f5b0e 100644 --- a/test/media.test.ts +++ b/test/media.test.ts @@ -84,3 +84,41 @@ describe("avatar resolution (session 19)", () => { }); }); + +describe("duplicate uploads (studio#7, migration 0028)", () => { + const png = (...b: number[]) => new File([new Uint8Array([0x89, 0x50, 0x4e, 0x47, ...b])], "p.png", { type: "image/png" }); + + it("returns the existing row for the same bytes on the same item, and stores nothing", async () => { + const cookie = await login(); + const id = await createAndPublish(cookie, "fragment with a repeated image"); + const first = await upload(cookie, png(1, 2), { item_id: id, alt: "first" }); + expect(first.status).toBe(201); + const again = await upload(cookie, png(1, 2), { item_id: id, alt: "second" }); + expect(again.status).toBe(200); + expect(again.json).toEqual({ id: first.json.id, url: first.json.url, mime: "image/png", duplicate: true }); + + const item = await (await getPublic(`/blyg/items/${id}.json`)).json(); + expect(item.media).toEqual([{ url: first.json.url, mime: "image/png", alt: "first" }]); + }); + + it("stores a new row for different bytes, another type, another item, another placement, or no item", async () => { + const cookie = await login(); + const a = await createAndPublish(cookie, "item a"); + const b = await createAndPublish(cookie, "item b"); + const base = await upload(cookie, png(3), { item_id: a }); + expect(base.status).toBe(201); + const others = [ + await upload(cookie, png(4), { item_id: a }), + await upload(cookie, new File([new Uint8Array([0x89, 0x50, 0x4e, 0x47, 3])], "p.webp", { type: "image/webp" }), { item_id: a }), + await upload(cookie, png(3), { item_id: b }), + await upload(cookie, png(3), { item_id: a, inline: "true" }), + await upload(cookie, png(3)), + await upload(cookie, png(3)), + ]; + for (const r of others) { + expect(r.status).toBe(201); + expect(r.json.duplicate).toBeUndefined(); + } + expect(new Set([base, ...others].map((r) => r.json.id)).size).toBe(7); + }); +});