Compare commits
1
Commits
c999e55c47
...
a0e6828693
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a0e6828693 |
@@ -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
@@ -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 };
|
||||||
|
|||||||
@@ -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 &&
|
||||||
|
|||||||
@@ -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", () => {
|
||||||
|
|||||||
@@ -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();
|
||||||
|
|||||||
Reference in New Issue
Block a user