mirror of
https://github.com/HeyPuter/puter.git
synced 2026-08-23 22:47:19 +00:00
fix: thumbnails breaking after copy — duplicate the S3 object per copy (#3494)
A copied fsentry kept its source's thumbnail pointer verbatim, so both rows shared one S3 thumbnail object. fs.remove.node deletes the pointed-to object, so the first removal among the sharers — e.g. the remove performed by a replace-on-copy — broke every other sharer's thumbnail. FSService already emits fs.copy.node for exactly this reason (its doc comment describes the duplication), but the thumbnails extension never subscribed to it. Add the missing handler: S3-copy the thumbnail to a freshly minted key and repoint the copied row, under the same only-keys-we-minted trust rule as the read/remove paths. If the shared object is already gone (pre-fix damage), null the pointer instead of leaving the row advertising a thumbnail it doesn't have.
This commit is contained in:
@@ -15,6 +15,7 @@ import {
|
||||
import { PuterServer } from '../src/backend/server.ts';
|
||||
import { setupTestServer } from '../src/backend/testUtil.ts';
|
||||
import {
|
||||
handleFsCopyNodeThumbnail,
|
||||
handleFsRemoveNodeThumbnail,
|
||||
handleThumbnailCreated,
|
||||
handleThumbnailRead,
|
||||
@@ -401,3 +402,107 @@ describe('thumbnails extension — handleFsRemoveNodeThumbnail', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('thumbnails extension — handleFsCopyNodeThumbnail', () => {
|
||||
let server: PuterServer;
|
||||
let s3: S3Client;
|
||||
|
||||
beforeAll(async () => {
|
||||
server = await setupTestServer();
|
||||
s3 = server.clients.s3.get();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await server?.shutdown();
|
||||
});
|
||||
|
||||
it('duplicates the shared thumbnail object and repoints the copied row', async () => {
|
||||
const sourceKey = mintedKey();
|
||||
const body = Buffer.from(TINY_PNG_BASE64, 'base64');
|
||||
await s3.send(
|
||||
new PutObjectCommand({
|
||||
Bucket: BUCKET,
|
||||
Key: sourceKey,
|
||||
Body: body,
|
||||
ContentType: 'image/png',
|
||||
}),
|
||||
);
|
||||
|
||||
const copyUuid = crypto.randomUUID();
|
||||
const db = { write: vi.fn().mockResolvedValue(undefined) };
|
||||
await handleFsCopyNodeThumbnail(
|
||||
{
|
||||
copy: {
|
||||
thumbnail: `s3://${BUCKET}/${sourceKey}`,
|
||||
uuid: copyUuid,
|
||||
},
|
||||
},
|
||||
{ s3, bucketName: BUCKET, db },
|
||||
);
|
||||
|
||||
// The copied row was repointed at a fresh object...
|
||||
expect(db.write).toHaveBeenCalledTimes(1);
|
||||
const [, params] = db.write.mock.calls[0] as [string, [string, string]];
|
||||
const [newPointer, updatedUuid] = params;
|
||||
expect(updatedUuid).toBe(copyUuid);
|
||||
expect(newPointer.startsWith(`s3://${BUCKET}/thumbnails/`)).toBe(true);
|
||||
expect(newPointer).not.toBe(`s3://${BUCKET}/${sourceKey}`);
|
||||
|
||||
// ...whose content matches, while the source object survives — so
|
||||
// deleting either entry can no longer break the other's thumbnail.
|
||||
const newKey = newPointer.slice(`s3://${BUCKET}/`.length);
|
||||
const duplicated = await s3.send(
|
||||
new GetObjectCommand({ Bucket: BUCKET, Key: newKey }),
|
||||
);
|
||||
expect(
|
||||
(await streamToBuffer(duplicated.Body as never)).equals(body),
|
||||
).toBe(true);
|
||||
const original = await s3.send(
|
||||
new GetObjectCommand({ Bucket: BUCKET, Key: sourceKey }),
|
||||
);
|
||||
expect(original.ContentType).toBe('image/png');
|
||||
});
|
||||
|
||||
it('drops the pointer when the shared object is already gone', async () => {
|
||||
const copyUuid = crypto.randomUUID();
|
||||
const db = { write: vi.fn().mockResolvedValue(undefined) };
|
||||
await handleFsCopyNodeThumbnail(
|
||||
{
|
||||
copy: {
|
||||
thumbnail: `s3://${BUCKET}/${mintedKey()}`, // never uploaded
|
||||
uuid: copyUuid,
|
||||
},
|
||||
},
|
||||
{ s3, bucketName: BUCKET, db },
|
||||
);
|
||||
|
||||
expect(db.write).toHaveBeenCalledTimes(1);
|
||||
const [sql, params] = db.write.mock.calls[0] as [string, [string]];
|
||||
expect(sql).toContain('NULL');
|
||||
expect(params).toEqual([copyUuid]);
|
||||
});
|
||||
|
||||
it('does not duplicate an object the pointer names but we did not mint', async () => {
|
||||
const foreignKey = crypto.randomUUID(); // shaped like an fs object key
|
||||
const db = { write: vi.fn().mockResolvedValue(undefined) };
|
||||
await handleFsCopyNodeThumbnail(
|
||||
{
|
||||
copy: {
|
||||
thumbnail: `s3://${BUCKET}/${foreignKey}`,
|
||||
uuid: crypto.randomUUID(),
|
||||
},
|
||||
},
|
||||
{ s3, bucketName: BUCKET, db },
|
||||
);
|
||||
expect(db.write).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('is a no-op when the copy has no thumbnail', async () => {
|
||||
const db = { write: vi.fn().mockResolvedValue(undefined) };
|
||||
await handleFsCopyNodeThumbnail(
|
||||
{ copy: { thumbnail: null, uuid: crypto.randomUUID() } },
|
||||
{ s3, bucketName: BUCKET, db },
|
||||
);
|
||||
expect(db.write).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import {
|
||||
CopyObjectCommand,
|
||||
DeleteObjectCommand,
|
||||
GetObjectCommand,
|
||||
PutObjectCommand,
|
||||
@@ -280,6 +281,55 @@ export const handleThumbnailRead = async (
|
||||
}
|
||||
};
|
||||
|
||||
export const handleFsCopyNodeThumbnail = async (
|
||||
payload: { copy?: { thumbnail?: string | null; uuid?: string } | null },
|
||||
deps: {
|
||||
s3: S3Client;
|
||||
bucketName: string;
|
||||
db: { write: (sql: string, params: unknown[]) => Promise<unknown> };
|
||||
},
|
||||
): Promise<void> => {
|
||||
const copy = payload.copy;
|
||||
const thumbnailUrl = copy?.thumbnail;
|
||||
if (!copy || !copy.uuid || typeof thumbnailUrl !== 'string') return;
|
||||
|
||||
// Same trust rule as the read and remove paths: only touch objects this
|
||||
// extension minted.
|
||||
const sourceKey = resolveThumbnailKey(thumbnailUrl);
|
||||
if (!sourceKey) return;
|
||||
|
||||
// The copied row points at the SAME S3 object as its source, and
|
||||
// fs.remove.node deletes the pointed-to object — so the first removal
|
||||
// among the sharers (an overwrite, a trash purge) would break every
|
||||
// other sharer's thumbnail. Give the copy an object of its own.
|
||||
const newKey = mintThumbnailKey();
|
||||
try {
|
||||
await deps.s3.send(
|
||||
new CopyObjectCommand({
|
||||
Bucket: deps.bucketName,
|
||||
CopySource: `${deps.bucketName}/${sourceKey}`,
|
||||
Key: newKey,
|
||||
}),
|
||||
);
|
||||
} catch (err) {
|
||||
// The shared object is already gone (e.g. a sharer was removed
|
||||
// before this fix existed) — the pointer is dead either way, so
|
||||
// drop it rather than leave the row advertising a thumbnail it
|
||||
// doesn't have.
|
||||
await deps.db.write(
|
||||
'UPDATE `fsentries` SET `thumbnail` = NULL WHERE `uuid` = ?',
|
||||
[copy.uuid],
|
||||
);
|
||||
console.warn('[thumbnails] failed to duplicate thumbnail on copy', err);
|
||||
return;
|
||||
}
|
||||
|
||||
await deps.db.write(
|
||||
'UPDATE `fsentries` SET `thumbnail` = ? WHERE `uuid` = ?',
|
||||
[`s3://${deps.bucketName}/${newKey}`, copy.uuid],
|
||||
);
|
||||
};
|
||||
|
||||
export const handleFsRemoveNodeThumbnail = async (
|
||||
payload: { target: { thumbnail?: string | null } },
|
||||
deps: { s3: S3Client; bucketName: string },
|
||||
@@ -335,6 +385,23 @@ extension.on('thumbnail.read', async (_key, entry: Record<string, unknown>) => {
|
||||
});
|
||||
});
|
||||
|
||||
// -- fs.copy.node ----------------------------------------------------
|
||||
// A copied entry initially shares its source's thumbnail object; duplicate
|
||||
// it so removing either entry can't break the other's thumbnail.
|
||||
|
||||
extension.on('fs.copy.node', async (_key, payload) => {
|
||||
await handleFsCopyNodeThumbnail(
|
||||
payload as {
|
||||
copy?: { thumbnail?: string | null; uuid?: string } | null;
|
||||
},
|
||||
{
|
||||
s3: getClient(),
|
||||
bucketName: thumbnailBucketName,
|
||||
db: clients.db,
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
// -- fs.remove.node --------------------------------------------------
|
||||
// Delete S3 thumbnail when the file is removed.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user