An ID on the command line is decimal digits only (closes #185)
check / check (push) Waiting to run
check / check (push) Waiting to run
files --collection, get, get-thumb and helper fix-missing-thumbnails --file read an ID with Number(), so 0x10 meant 16, 1e3 meant 1000 and an empty string meant 0, and --file dropped an ID it could not read. parseID in src/cli-commands.ts now takes an ID only when it is decimal digits and a safe integer; anything else prints "Invalid file ID: ARG" or "Invalid collection ID: ARG" and the command exits 1. The fixer checks every --file ID before it opens the library, so one bad ID fails the whole command before any request. Model: opus-5-5
This commit was merged in pull request #189.
This commit is contained in:
@@ -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
|
||||
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
|
||||
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
|
||||
|
||||
@@ -25,6 +25,15 @@ declares one.
|
||||
|
||||
# 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):
|
||||
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
|
||||
|
||||
+28
-19
@@ -97,6 +97,20 @@ const requireSession = (ctx: CliContext): Client | undefined => {
|
||||
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
|
||||
// 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
|
||||
@@ -247,11 +261,8 @@ export const filesCommand = async (
|
||||
await init();
|
||||
const client = requireSession(ctx);
|
||||
if (!client) return 3;
|
||||
const collectionID = Number(opts.collection);
|
||||
if (!Number.isFinite(collectionID)) {
|
||||
ctx.stderr.write("Invalid collection ID\n");
|
||||
return 1;
|
||||
}
|
||||
const collectionID = parseID(ctx, "collection", opts.collection);
|
||||
if (collectionID === undefined) return 1;
|
||||
|
||||
const lib = await openReadLibrary(ctx, client);
|
||||
try {
|
||||
@@ -289,11 +300,8 @@ export const getCommand = async (
|
||||
await init();
|
||||
const client = requireSession(ctx);
|
||||
if (!client) return 3;
|
||||
const fileID = Number(fileIDStr);
|
||||
if (!Number.isFinite(fileID)) {
|
||||
ctx.stderr.write("Invalid file ID\n");
|
||||
return 1;
|
||||
}
|
||||
const fileID = parseID(ctx, "file", fileIDStr);
|
||||
if (fileID === undefined) return 1;
|
||||
|
||||
const lib = await openReadLibrary(ctx, client);
|
||||
try {
|
||||
@@ -347,11 +355,8 @@ export const getThumbCommand = async (
|
||||
await init();
|
||||
const client = requireSession(ctx);
|
||||
if (!client) return 3;
|
||||
const fileID = Number(fileIDStr);
|
||||
if (!Number.isFinite(fileID)) {
|
||||
ctx.stderr.write("Invalid file ID\n");
|
||||
return 1;
|
||||
}
|
||||
const fileID = parseID(ctx, "file", fileIDStr);
|
||||
if (fileID === undefined) return 1;
|
||||
|
||||
const lib = await openReadLibrary(ctx, client);
|
||||
try {
|
||||
@@ -516,15 +521,19 @@ export const fixMissingThumbnailsCommand = async (
|
||||
await init();
|
||||
const client = requireSession(ctx);
|
||||
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);
|
||||
try {
|
||||
// Refresh first so files added since the cache was written are found;
|
||||
// a failed refresh throws.
|
||||
await lib.fresh();
|
||||
let fileIDs: number[];
|
||||
if (opts.file && opts.file.length > 0) {
|
||||
fileIDs = opts.file.map(Number).filter(Number.isFinite);
|
||||
} else {
|
||||
if (fileIDs.length === 0) {
|
||||
ctx.stderr.write("Scanning for missing thumbnails...\n");
|
||||
const missing = await listMissingThumbnails(lib, client, (msg) => {
|
||||
if (!opts.json) ctx.stderr.write(msg + "\n");
|
||||
|
||||
+127
-11
@@ -511,11 +511,6 @@ describe("files", () => {
|
||||
expect(await filesCommand(context(), { collection: "9" })).toBe(1);
|
||||
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", () => {
|
||||
@@ -546,12 +541,6 @@ describe("get and get-thumb", () => {
|
||||
expect(stderr.text).toBe("File 999 not found\n");
|
||||
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
|
||||
@@ -1035,3 +1024,130 @@ describe("backup-metadata and the thumbnail helpers refresh first", () => {
|
||||
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);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user