Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -1050,3 +1050,18 @@ pathological input, by growth rate), `terminal.fuzz.test.ts` (seeded hostile out
consistent screen, bounded cells, no reply echoing attacker text, nothing while replaying),
`files.resolve.fuzz.test.ts` (random paths through in/out/loop symlinks never resolve or open
outside), `git.hostile-config.test.ts` and `pty.tracker.diff.test.ts` (above).

## 25. Round 16: a shrinking file could hang /api/raw

Release v1.10.7 stopped at its test step on a timeout in the `/api/raw` shrinking-file test, which
had passed on the same commit in `ci`. It was not a slow machine: the same test failed 4 runs in 30
on v1.10.6 and about one in four alone, because the server sometimes never ended the response.

| Finding | Fix |
| --- | --- |
| `ctl.error` right after a chunk could be lost by Bun 1.4.4: the connection stayed open with no end of body, and `idleTimeout: 0` never reclaims it, so a client's `blob()` waited forever | Wait 50 ms inside `pull` before the error. Measured against the real file path: no hang with readers up to 400 ms per chunk or under CPU load. Erroring after `pull` returned, throwing, generator and direct streams, `server.timeout` and padding chunks all still hung |
| The test ran the race once, so it passed most of the time and blocked a release at random | 25 rounds with a watchdog per round, reported as "hang" |
| Looped at 2 MB, the test once saw a clean end on Linux CI. The server only closes cleanly after reading every byte, so the transfer had probably finished before the truncate, but the test counted no bytes and 1200 more rounds did not repeat it | A 256 MB sparse file that cannot finish first, and byte counts: a full transfer is correct, a short one fails as "short by N" |

The delay is measured, not derived: the cause is inside Bun. The looped test is what will notice if
a Bun upgrade changes this.
9 changes: 8 additions & 1 deletion src/server/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import { Candidates, compileQuery, globsOk, createWorkerScanner, type SearchEven

import pkg from "../../package.json" with { type: "json" };

/** How long /api/raw waits before failing a body whose file shrank: see the comment there. */
const RAW_ABORT_DELAY_MS = 50;

export const VERSION: string = pkg.version;

export interface ServeOptions {
Expand Down Expand Up @@ -416,8 +419,12 @@ export function startServer(opts: ServeOptions): { server: Server<SocketData>; u
const { bytesRead } = chunk.length ? await fh.read(chunk, 0, chunk.length, pos) : { bytesRead: 0 };
if (bytesRead === 0) {
await fh.close();
if (pos >= stop) return ctl.close();
// The file shrank after fstat: fail the body rather than send fewer bytes than promised.
return pos < stop ? ctl.error(new Error("file changed while reading")) : ctl.close();
// Bun 1.4.4 can drop an error raised right after a chunk and leave the connection open
// with no end of body (one in four, and idleTimeout is 0); waiting inside pull closes it.
await Bun.sleep(RAW_ABORT_DELAY_MS);
return ctl.error(new Error("file changed while reading"));
}
pos += bytesRead;
ctl.enqueue(chunk.subarray(0, bytesRead));
Expand Down
45 changes: 29 additions & 16 deletions test/http.coverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,24 +511,37 @@ test("/api/raw honours a single byte range (206/416) so media can seek, also und
});

test("/api/raw fails the body instead of sending fewer bytes than content-length when the file shrinks", async () => {
const p = path.join(dir, "shrink.bin");
await writeFile(p, new Uint8Array(2 * 1024 * 1024));
const r = await get("/api/raw?path=shrink.bin");
expect(r.status).toBe(200);
const reader = r.body!.getReader();
const first = await reader.read();
expect(first.done).toBe(false);
await truncate(p, 10);
// The transfer fails (the stream is errored): never a quiet success with a truncated body.
let failed = false;
try {
for (let c = await reader.read(); !c.done; c = await reader.read());
} catch {
failed = true;
// Repeated: Bun can lose the error and leave the connection open, about one run in four. The file
// is large (and sparse) so that the transfer is still running when it shrinks.
const size = 256 * 1024 * 1024;
const outcomes: string[] = [];
for (let i = 0; i < 25; i++) {
const p = path.join(dir, "shrink.bin");
await writeFile(p, "");
await truncate(p, size);
const r = await get("/api/raw?path=shrink.bin");
expect(r.status).toBe(200);
const reader = r.body!.getReader();
const first = await reader.read();
expect(first.done).toBe(false);
let bytes = first.value!.length;
await truncate(p, 10);
const drain = async () => {
try {
for (let c = await reader.read(); !c.done; c = await reader.read()) bytes += c.value.length;
return bytes === size ? "complete" : `short by ${size - bytes}`;
} catch {
return "failed";
}
};
outcomes.push(await Promise.race([drain(), Bun.sleep(1500).then(() => "hang")]));
}
expect(failed).toBe(true);
// Never a hang and never a quiet success with a truncated body. "complete" means every byte had
// already been sent before the file shrank, which is a correct transfer.
expect(outcomes.filter((o) => o !== "failed" && o !== "complete")).toEqual([]);
expect(outcomes).toContain("failed");
expect((await get("/api/info")).status).toBe(200);
});
}, 45_000);

test("round 15: no Bun error page, own-key MIME lookups, whole-byte limits, files-only open", async () => {
// A Host header that makes the URL unparseable: a JSON 400/421, never an HTML page with paths.
Expand Down
Loading