An ID on the command line is decimal digits only (closes #185) #189

Merged
clawbot merged 2 commits from issue-185-decimal-ids into next 2026-10-14 03:06:35 +02:00
4 changed files with 171 additions and 30 deletions
+7
View File
@@ -626,6 +626,13 @@ exits 1, or 3 when the server no longer accepts the saved session (see "Session
handling"). `--cache-dir` overrides where the cache lives; without it each handling"). `--cache-dir` overrides where the cache lives; without it each
account gets its own directory under the per-user cache path. account gets its own directory under the per-user cache path.
An ID given to `files --collection`, `get`, `get-thumb` or
`helper fix-missing-thumbnails --file` is written in decimal digits, no larger
than 9007199254740991. Anything else, such as `0x10`, `1e3` or `-1`, is refused
before any request with one line, `Invalid file ID: <arg>` or
`Invalid collection ID: <arg>`, and exit status 1. One bad `--file` ID refuses
the whole command, so nothing is fixed.
`get` and `get-thumb` resolve the file by ID directly, so `--collection` is `get` and `get-thumb` resolve the file by ID directly, so `--collection` is
accepted for backward compatibility but ignored. For a live photo, `get` writes accepted for backward compatibility but ignored. For a live photo, `get` writes
its image and its video, each named after the title with its own extension, as its image and its video, each named after the title with its own extension, as
+9
View File
@@ -25,6 +25,15 @@ declares one.
# Completed Steps # Completed Steps
- 2026-10-14: `files --collection`, `get`, `get-thumb` and
`helper fix-missing-thumbnails --file` take an ID only when it is written in
decimal digits and is a safe integer (issue 185). Before, `Number()` read
`0x10` as 16, `1e3` as 1000 and an empty string as 0, and `--file` dropped an
ID it could not read without a word. Anything else now prints
`Invalid file ID: <arg>` or `Invalid collection ID: <arg>` and exits 1 before
any request; one bad `--file` ID fails the whole command. `parseID` in
`src/cli-commands.ts` does the check for all four.
- 2026-10-06: The README says how to run `quak backup` from cron (issue 170): - 2026-10-06: The README says how to run `quak backup` from cron (issue 170):
log in once as the job's user, a crontab with a backup every night and log in once as the job's user, a crontab with a backup every night and
`--verify` on Sundays appending to a log file, and the `HOME` and `--verify` on Sundays appending to a log file, and the `HOME` and
+28 -19
View File
@@ -97,6 +97,20 @@ const requireSession = (ctx: CliContext): Client | undefined => {
return client; return client;
}; };
// `text` as an ID, or undefined after telling the user it is not one. An ID is
// written in decimal digits and is a safe integer; `Number()` alone would read
// `0x10` as 16, `1e3` as 1000 and "" as 0.
const parseID = (
ctx: CliContext,
kind: "file" | "collection",
text: string,
): number | undefined => {
const id = Number(text);
if (/^[0-9]+$/.test(text) && Number.isSafeInteger(id)) return id;
ctx.stderr.write(`Invalid ${kind} ID: ${text}\n`);
return undefined;
};
// A library client that omits `fetchMLData`, so the point commands below do not // A library client that omits `fetchMLData`, so the point commands below do not
// kick the library's background ML backfill: they read metadata, or fetch one // kick the library's background ML backfill: they read metadata, or fetch one
// file's content, and exit. `backup` and `backup-metadata` handle ML on their // file's content, and exit. `backup` and `backup-metadata` handle ML on their
@@ -247,11 +261,8 @@ export const filesCommand = async (
await init(); await init();
const client = requireSession(ctx); const client = requireSession(ctx);
if (!client) return 3; if (!client) return 3;
const collectionID = Number(opts.collection); const collectionID = parseID(ctx, "collection", opts.collection);
if (!Number.isFinite(collectionID)) { if (collectionID === undefined) return 1;
ctx.stderr.write("Invalid collection ID\n");
return 1;
}
const lib = await openReadLibrary(ctx, client); const lib = await openReadLibrary(ctx, client);
try { try {
@@ -289,11 +300,8 @@ export const getCommand = async (
await init(); await init();
const client = requireSession(ctx); const client = requireSession(ctx);
if (!client) return 3; if (!client) return 3;
const fileID = Number(fileIDStr); const fileID = parseID(ctx, "file", fileIDStr);
if (!Number.isFinite(fileID)) { if (fileID === undefined) return 1;
ctx.stderr.write("Invalid file ID\n");
return 1;
}
const lib = await openReadLibrary(ctx, client); const lib = await openReadLibrary(ctx, client);
try { try {
@@ -347,11 +355,8 @@ export const getThumbCommand = async (
await init(); await init();
const client = requireSession(ctx); const client = requireSession(ctx);
if (!client) return 3; if (!client) return 3;
const fileID = Number(fileIDStr); const fileID = parseID(ctx, "file", fileIDStr);
if (!Number.isFinite(fileID)) { if (fileID === undefined) return 1;
ctx.stderr.write("Invalid file ID\n");
return 1;
}
const lib = await openReadLibrary(ctx, client); const lib = await openReadLibrary(ctx, client);
try { try {
@@ -516,15 +521,19 @@ export const fixMissingThumbnailsCommand = async (
await init(); await init();
const client = requireSession(ctx); const client = requireSession(ctx);
if (!client) return 3; if (!client) return 3;
// One bad --file ID fails the whole command, before anything is fetched.
let fileIDs: number[] = [];
for (const text of opts.file ?? []) {
const fileID = parseID(ctx, "file", text);
if (fileID === undefined) return 1;
fileIDs.push(fileID);
}
const lib = await openReadLibrary(ctx, client); const lib = await openReadLibrary(ctx, client);
try { try {
// Refresh first so files added since the cache was written are found; // Refresh first so files added since the cache was written are found;
// a failed refresh throws. // a failed refresh throws.
await lib.fresh(); await lib.fresh();
let fileIDs: number[]; if (fileIDs.length === 0) {
if (opts.file && opts.file.length > 0) {
fileIDs = opts.file.map(Number).filter(Number.isFinite);
} else {
ctx.stderr.write("Scanning for missing thumbnails...\n"); ctx.stderr.write("Scanning for missing thumbnails...\n");
const missing = await listMissingThumbnails(lib, client, (msg) => { const missing = await listMissingThumbnails(lib, client, (msg) => {
if (!opts.json) ctx.stderr.write(msg + "\n"); if (!opts.json) ctx.stderr.write(msg + "\n");
+127 -11
View File
@@ -511,11 +511,6 @@ describe("files", () => {
expect(await filesCommand(context(), { collection: "9" })).toBe(1); expect(await filesCommand(context(), { collection: "9" })).toBe(1);
expect(stderr.text).toBe("Collection 9 not found\n"); expect(stderr.text).toBe("Collection 9 not found\n");
}); });
it("exits 1 for a collection ID that is not a number", async () => {
expect(await filesCommand(context(), { collection: "abc" })).toBe(1);
expect(stderr.text).toBe("Invalid collection ID\n");
});
}); });
describe("get and get-thumb", () => { describe("get and get-thumb", () => {
@@ -546,12 +541,6 @@ describe("get and get-thumb", () => {
expect(stderr.text).toBe("File 999 not found\n"); expect(stderr.text).toBe("File 999 not found\n");
expect(existsSync(out)).toBe(false); expect(existsSync(out)).toBe(false);
}); });
it("both exit 1 for a file ID that is not a number", async () => {
expect(await getCommand(context(), "abc", {})).toBe(1);
expect(await getThumbCommand(context(), "abc", {})).toBe(1);
expect(stderr.text).toBe("Invalid file ID\nInvalid file ID\n");
});
}); });
// A live photo, which Ente stores as one ZIP, is written as its image and its // A live photo, which Ente stores as one ZIP, is written as its image and its
@@ -1035,3 +1024,130 @@ describe("backup-metadata and the thumbnail helpers refresh first", () => {
expect(stdout.text).toBe(""); expect(stdout.text).toBe("");
}); });
}); });
// `files --collection`, `get`, `get-thumb` and `helper fix-missing-thumbnails
// --file` take an ID only when it is written in decimal digits and is a safe
// integer. `Number()` alone reads `0x10` as 16, `1e3` as 1000 and an empty
// string as 0, so the command would act on a file the user never named.
// Anything else is refused with one line naming it, and exit 1, before any
// request.
//
// These run on a real client, read back from a saved session, over a fake Ente
// server holding an empty account, which records each request it receives. With
// a good ID the command goes on to ask the server, and finds nothing there.
describe("an ID given on the command line", () => {
const NOT_IDS = [
"0x10",
"1e3",
"1.5",
"-1",
"",
"abc",
// One past Number.MAX_SAFE_INTEGER: from here on a number cannot
// hold every integer, so two IDs could read as one.
"9007199254740992",
];
const requests: Request[] = [];
const emptyAccount = (): CliContext => {
const ctx = context();
const key = toBase64(new Uint8Array(32));
saveSession(ctx.sessionDir, {
email: "cli@example.com",
userID: USER_ID,
token: "token",
masterKey: key,
secretKey: key,
publicKey: key,
});
return {
...ctx,
loadSession: (path) =>
loadSession(path, {
fetch: async (url, init) => {
requests.push(new Request(url, init));
return new Response(
JSON.stringify({ collections: [] }),
{ headers: { "content-type": "application/json" } },
);
},
}),
};
};
beforeEach(() => {
requests.length = 0;
});
it("files looks up a decimal collection ID", async () => {
const ctx = emptyAccount();
expect(await filesCommand(ctx, { collection: "1" })).toBe(1);
expect(stderr.text).toBe("Collection 1 not found\n");
expect(requests).not.toHaveLength(0);
});
it.each(NOT_IDS)(
"files refuses --collection %j before any request",
async (arg) => {
const ctx = emptyAccount();
expect(await filesCommand(ctx, { collection: arg })).toBe(1);
expect(stderr.text).toBe(`Invalid collection ID: ${arg}\n`);
expect(stdout.text).toBe("");
expect(requests).toHaveLength(0);
},
);
it("get looks up a decimal file ID", async () => {
expect(await getCommand(emptyAccount(), "100", {})).toBe(1);
expect(stderr.text).toBe("File 100 not found\n");
expect(requests).not.toHaveLength(0);
});
it.each(NOT_IDS)("get refuses %j before any request", async (arg) => {
expect(await getCommand(emptyAccount(), arg, {})).toBe(1);
expect(stderr.text).toBe(`Invalid file ID: ${arg}\n`);
expect(stdout.text).toBe("");
expect(requests).toHaveLength(0);
});
it("get-thumb looks up a decimal file ID", async () => {
expect(await getThumbCommand(emptyAccount(), "100", {})).toBe(1);
expect(stderr.text).toBe("File 100 not found\n");
expect(requests).not.toHaveLength(0);
});
it.each(NOT_IDS)("get-thumb refuses %j before any request", async (arg) => {
expect(await getThumbCommand(emptyAccount(), arg, {})).toBe(1);
expect(stderr.text).toBe(`Invalid file ID: ${arg}\n`);
expect(stdout.text).toBe("");
expect(requests).toHaveLength(0);
});
// The fixer reports each file it was given; on this account neither is
// found, which is a failure, so it exits 1.
it("fix-missing-thumbnails looks up each decimal --file ID", async () => {
const ctx = emptyAccount();
const opts = { file: ["100", "200"], json: true };
expect(await fixMissingThumbnailsCommand(ctx, opts)).toBe(1);
expect(JSON.parse(stdout.text)).toMatchObject([
{ fileID: 100, reason: "file not found in any collection" },
{ fileID: 200, reason: "file not found in any collection" },
]);
expect(requests).not.toHaveLength(0);
});
// One bad ID fails the whole command, so the good one before it is not
// fixed either.
it.each(NOT_IDS)(
"fix-missing-thumbnails refuses --file 100 %j before any request",
async (arg) => {
const ctx = emptyAccount();
const opts = { file: ["100", arg] };
expect(await fixMissingThumbnailsCommand(ctx, opts)).toBe(1);
expect(stderr.text).toBe(`Invalid file ID: ${arg}\n`);
expect(stdout.text).toBe("");
expect(requests).toHaveLength(0);
},
);
});