fix: bad empty file error handling (#3293)
Maintain Release Merge PR / update-release-pr (push) Has been cancelled
Notify HeyPuter / notify (push) Has been cancelled
release-please / release-please (push) Has been cancelled

This commit is contained in:
Daniel Salazar
2026-06-23 16:13:41 -07:00
committed by GitHub
parent b2108c0216
commit 34a7595a6d
6 changed files with 152 additions and 5 deletions
@@ -765,6 +765,69 @@ describe('LegacyFSController.copy', () => {
`/${username}/Pictures/renamed-copy`,
);
});
it('copies an empty file (no backing S3 object) without erroring', async () => {
// Empty files created via /touch have size 0 and no S3 object —
// bucket is null. Copy must clone them as empty-file entries rather
// than issuing a CopyObject, which would throw S3 NoSuchKey.
const { actor } = await makeUser();
const username = actor.user!.username!;
const src = `/${username}/Documents/empty.txt`;
await withActor(actor, () =>
controller.touch(
makeReq({ body: { path: src }, actor }),
makeRes().res,
),
);
const { res, captured } = makeRes();
await withActor(actor, () =>
controller.copy(
makeReq({
body: { source: src, destination: `/${username}/Pictures` },
actor,
}),
res,
),
);
const body = captured.body as Array<{ copied: { path: string } }>;
expect(body[0].copied.path).toBe(`/${username}/Pictures/empty.txt`);
const copied = await server.stores.fsEntry.getEntryByPath(
`/${username}/Pictures/empty.txt`,
);
expect(copied).not.toBeNull();
expect(copied!.size).toBe(0);
// Original is untouched.
expect(await server.stores.fsEntry.getEntryByPath(src)).not.toBeNull();
});
it('reads an empty file as empty content without deleting it', async () => {
// Reading an empty file (no S3 object) must not throw NoSuchKey nor
// trip the ghost-file cleanup, which would delete the entry.
const { actor } = await makeUser();
const username = actor.user!.username!;
const src = `/${username}/Documents/readme-empty.txt`;
await withActor(actor, () =>
controller.touch(
makeReq({ body: { path: src }, actor }),
makeRes().res,
),
);
const entry = (await server.stores.fsEntry.getEntryByPath(src))!;
const download = await server.services.fs.readContent(entry);
const chunks: Buffer[] = [];
for await (const chunk of download.body) {
chunks.push(Buffer.from(chunk as Uint8Array));
}
expect(Buffer.concat(chunks).byteLength).toBe(0);
expect(download.contentLength).toBe(0);
// The entry must still exist — the ghost handler must NOT have run.
expect(
await server.stores.fsEntry.getEntryByPath(src),
).not.toBeNull();
});
});
// ── move ────────────────────────────────────────────────────────────
@@ -292,6 +292,22 @@ describe('loadFileInput FS path', () => {
expect(result.fsEntry?.path).toBe(path);
});
it('returns empty bytes for an empty file with no backing S3 object', async () => {
// Files created via touch have size 0 and a null bucket — no S3
// object exists. loadFileInput must return empty content rather than
// throwing NoSuchKey from getObjectStream.
const { actor, userId } = await makeUser();
const username = actor.user!.username!;
const path = `/${username}/Documents/empty.txt`;
const entry = await withActor(actor, () =>
server.services.fs.touch(userId, { path }),
);
const result = await callLoadFileInput(actor, path);
expect(result.buffer.byteLength).toBe(0);
expect(result.fsEntry?.uuid).toBe(entry.uuid);
});
it('also accepts a `{ uuid }` object reference', async () => {
const { actor, userId } = await makeUser();
const username = actor.user!.username!;
+18
View File
@@ -22,6 +22,7 @@ import type { Actor } from '../../core/actor.js';
import { HttpError } from '../../core/http/HttpError.js';
import type { FSService } from '../../services/fs/FSService.js';
import { expandTildePath, resolveNode } from '../../services/fs/resolveNode.js';
import { hasNoBackingS3Object } from '../../stores/fs/FSEntry.js';
import type { FSEntryStore } from '../../stores/fs/FSEntryStore.js';
import type { S3ObjectStore } from '../../stores/fs/S3ObjectStore.js';
import { mimeFromName } from '../../util/fileSigning.js';
@@ -175,6 +176,23 @@ export async function loadFileInput(
// `path`/`uid`/`uuid` (e.g. AI chat `puter_path` content parts) could
// exfiltrate any user's file. Must run before the S3 read below.
await fsService.checkFSAccess(entry, actor, 'read');
// Empty files (created via `touch`) have no backing S3 object —
// getObjectStream would throw NoSuchKey, so return empty content.
if (hasNoBackingS3Object(entry)) {
return {
buffer: Buffer.alloc(0),
filename: entry.name,
mimeType: mimeFromName(entry.name) ?? 'application/octet-stream',
fsEntry: {
uuid: entry.uuid,
path: entry.path,
bucket: entry.bucket,
bucketRegion: entry.bucketRegion,
size: entry.size,
sqlId: entry.id,
},
};
}
const objectKey = entry.uuid;
const { body, contentType, contentLength } =
await stores.s3Object.getObjectStream(
+7 -5
View File
@@ -729,11 +729,13 @@ export class PuterServer {
if (status < 500) return;
if (SKIP_ALERT_PREFIXES.test(legacyCode)) return;
}
const signature = isHttp
? err.legacyCode || err.code || err.message
: err instanceof Error
? err.message
: String(err);
const signature = !isHttp
? err instanceof Error
? err.message
: String(err)
: status >= 500
? `${err.legacyCode || err.code || 'http'}:${err.message}`
: err.legacyCode || err.code || err.message;
const routePath =
(req as unknown as { route?: { path?: string } }).route
?.path ?? req.path;
+37
View File
@@ -31,6 +31,7 @@ import {
FSEntry,
FSEntryCreateInput,
FSEntryWriteInput,
hasNoBackingS3Object,
PendingUploadCreateInput,
PendingUploadSession,
} from '../../stores/fs/FSEntry.js';
@@ -2806,6 +2807,24 @@ export class FSService extends PuterService {
{ legacyCode: 'shortcut_target_not_found' },
);
}
// Empty files (created via `touch`/`createNonFileEntry` with kind
// 'empty-file') have no backing S3 object — `bucket` is null. Reading
// one would throw NoSuchKey and, worse, trip #handleGhostFile, which
// deletes the entry as if it were an orphan. Return an empty stream
// instead. (A real file whose object is genuinely missing keeps a
// non-null bucket, so it still falls through to the ghost path below.)
if (hasNoBackingS3Object(entry)) {
return {
body: Readable.from([]),
contentLength: 0,
contentType: null,
contentRange: null,
etag: null,
lastModified: entry.modified
? new Date(entry.modified * 1000)
: null,
};
}
const objectKey = entry.uuid;
try {
return await this.stores.s3Object.getObjectStream(
@@ -3646,6 +3665,24 @@ export class FSService extends PuterService {
});
}
// Empty files (created via `touch`/`createNonFileEntry` with kind
// 'empty-file') have no backing S3 object — `bucket` is null and there
// is nothing to CopyObject. Issuing one would throw NoSuchKey, so clone
// the source as another empty-file entry instead of touching S3.
if (hasNoBackingS3Object(source)) {
return this.stores.fsEntry.createNonFileEntry({
userId,
parent: destinationParent,
name: newName,
kind: 'empty-file',
metadata: source.metadata,
thumbnail: source.thumbnail,
associatedAppId: source.associatedAppId,
isPublic: source.isPublic,
immutable: source.immutable,
});
}
const newUuid = uuidv4();
const sourceObjectKey = source.uuid;
const resolvedBucket = this.stores.s3Object.resolveBucket(
+11
View File
@@ -53,6 +53,17 @@ export interface FSEntry {
suggestedApps: unknown[]; // TODO DS: type with app row
}
/**
* True when an entry has no backing S3 object. Empty files (size 0) created
* via `touch`/`createNonFileEntry` never upload to S3 and leave `bucket` null;
* real files always store a non-null bucket on write, so a null bucket on a
* size-0 entry reliably means there is nothing in S3 to read or copy. A real
* file whose object went missing keeps its bucket, so this stays false and the
* usual missing-object handling still applies.
*/
export const hasNoBackingS3Object = (entry: FSEntry): boolean =>
(entry.size ?? 0) === 0 && entry.bucket === null;
export interface FSEntrySubdomain {
uuid: string;
address: string; // `${config.protocol}://${subdomain}.${'puter.site'|'puter.work'}` depending on wether dir or file