Compare commits

Author SHA1 Message Date
clawbot a0e6828693 Pin three untested guards: download timer, URL fragment, short APP1 (closes #89)
check / check (push) Successful in 15s
The download idle deadline's timer is unref'd so it never holds the
process open, and a test checks no timer is left after a download
completes or fails. A test covers the rejection of "#" in a request
path. The EXIF scan compares the Exif header only when the APP1 length
is at least 8, so it never reads the next segment's bytes.

Model: opus-5-5
2026-09-23 01:17:43 +00:00
5 changed files with 75 additions and 3 deletions
+6
View File
@@ -18,6 +18,12 @@ Tag v1.0.0.
# Completed Steps # Completed Steps
- 2026-09-23: Pinned three guards reviewers found untested (issue 89). The
download idle deadline's timer is unref'd, so it can never keep the process
alive, and a test checks no timer is left after a download completes or fails.
A test covers the rejection of `#` in a request path. The EXIF scan compares
the `Exif` header only in an APP1 segment of length 8 or more, so it never
reads the next segment's bytes, with a test for a short one.
- 2026-09-23: Made the download deadline an idle deadline (issue 24). - 2026-09-23: Made the download deadline an idle deadline (issue 24).
`downloadTimeoutMs` now aborts a file or thumbnail download only after no `downloadTimeoutMs` now aborts a file or thumbnail download only after no
bytes have arrived for that long, default 60 seconds, instead of bounding the bytes have arrived for that long, default 60 seconds, instead of bounding the
+3 -2
View File
@@ -49,8 +49,8 @@ export interface StreamOptions {
// An abort signal that fires once `ms` pass without a call to `restart`. It // An abort signal that fires once `ms` pass without a call to `restart`. It
// aborts with a `TimeoutError`, the same reason `AbortSignal.timeout()` gives, // aborts with a `TimeoutError`, the same reason `AbortSignal.timeout()` gives,
// so the retry classifier treats an idle download exactly as it treats any // so the retry classifier treats an idle download exactly as it treats any
// other deadline. `stop` must be called when the download ends, or the timer // other deadline. `stop` must be called when the download ends. The timer is
// keeps the process alive until it fires. // unref'd, so even one left running never keeps the process alive.
const idleDeadline = (ms: number) => { const idleDeadline = (ms: number) => {
const controller = new AbortController(); const controller = new AbortController();
let timer: ReturnType<typeof setTimeout> | undefined; let timer: ReturnType<typeof setTimeout> | undefined;
@@ -65,6 +65,7 @@ const idleDeadline = (ms: number) => {
), ),
); );
}, ms); }, ms);
timer.unref();
}; };
restart(); restart();
return { signal: controller.signal, restart, stop }; return { signal: controller.signal, restart, stop };
+3 -1
View File
@@ -45,8 +45,10 @@ export const extractExifFromJpeg = (
error: `segment length ${len} at byte ${offset} runs past the end of the file`, error: `segment length ${len} at byte ${offset} runs past the end of the file`,
}; };
if (marker === 0xe1) { if (marker === 0xe1) {
// APP1 — check for "Exif\0\0" header // APP1 — check for "Exif\0\0" header. A length under 8 has no
// room for it, and the bytes compared would be the next segment's.
if ( if (
len >= 8 &&
buf[offset + 4] === 0x45 && buf[offset + 4] === 0x45 &&
buf[offset + 5] === 0x78 && buf[offset + 5] === 0x78 &&
buf[offset + 6] === 0x69 && buf[offset + 6] === 0x69 &&
+56
View File
@@ -474,6 +474,16 @@ describe("ApiClient request URLs", () => {
); );
expect(calls).toHaveLength(0); expect(calls).toHaveLength(0);
}); });
it("rejects a path that carries a fragment", async () => {
const { fetch, calls } = recordingFetch();
const client = new ApiClient({ fetch });
await expect(client.getJSON("/diff#top")).rejects.toThrow(
/must not contain "\?" or "#"/,
);
expect(calls).toHaveLength(0);
});
}); });
describe("ApiError", () => { describe("ApiError", () => {
@@ -980,6 +990,52 @@ describe("ApiClient timeouts", () => {
} }
expect(joined).toEqual(payload); expect(joined).toEqual(payload);
}); });
it("leaves no timer pending after a download completes or fails", async () => {
vi.useFakeTimers();
try {
const { fetch } = scriptedFetch(
streamResponse(new Uint8Array([1, 2, 3])),
textResponse("gone", 404),
);
const client = new ApiClient({
fetch,
retry: { ...noWait, attempts: 1 },
});
expect(await readAll(await client.getFileStream(1))).toBe(3);
expect(vi.getTimerCount()).toBe(0);
await expect(client.getFileStream(2)).rejects.toBeInstanceOf(
ApiError,
);
expect(vi.getTimerCount()).toBe(0);
} finally {
vi.useRealTimers();
}
});
it("never lets the download timer keep the process alive", async () => {
const spy = vi.spyOn(globalThis, "setTimeout");
try {
const { fetch } = scriptedFetch(
streamResponse(new Uint8Array([1])),
);
const client = new ApiClient({
fetch,
downloadTimeoutMs: 12_345,
retry: noWait,
});
const stream = await client.getFileStream(1);
const i = spy.mock.calls.findIndex((call) => call[1] === 12_345);
const timer = spy.mock.results[i]!.value as NodeJS.Timeout;
expect(timer.hasRef()).toBe(false);
await stream.cancel();
} finally {
spy.mockRestore();
}
});
}); });
describe("ApiClient error typing", () => { describe("ApiClient error typing", () => {
+7
View File
@@ -51,6 +51,13 @@ describe("extractExifFromJpeg", () => {
expect(extractExifFromJpeg(bytes(SOI, app0, SOS))).toEqual({}); expect(extractExifFromJpeg(bytes(SOI, app0, SOS))).toEqual({});
}); });
it("ignores an APP1 segment too short to hold the Exif header", () => {
// Length 7 leaves room for "Exif\0" only. Without the length check
// the scan compared the header against bytes past the segment.
const short = app1(EXIF_HEADER.slice(0, 5));
expect(extractExifFromJpeg(bytes(SOI, short, SOS))).toEqual({});
});
it("reports a JPEG truncated inside a segment header", () => { it("reports a JPEG truncated inside a segment header", () => {
const scan = extractExifFromJpeg(bytes(SOI, [0xff, 0xe1, 0x00])); const scan = extractExifFromJpeg(bytes(SOI, [0xff, 0xe1, 0x00]));
expect(scan.exif).toBeUndefined(); expect(scan.exif).toBeUndefined();