Bun.file().slice(): count a negative index back from the file's real end

RO
1mo ago
+221-66

Problem

  • Bun.file(path).slice(-5).text() returns the first 5 bytes, not the last 5. slice(0, -1) keeps the last byte. Reproduced on 1.4.3.
  • Blob::get_slice (src/runtime/webcore/Blob.rs) counts a negative index back from self.size. Until something stats a Bun.file(), that size is MAX_SIZE. So slice(-5) gets the offset MAX_SIZE - 5. The read ignores the failed seek and starts at byte 0.

Fix

  • For a negative index on a file Blob with an unknown end, get_slice stats the file and counts back from its size.
  • The stat is not cached, so the parent does not change. A positive index, a pipe, and an S3 store take the old path.
  • Verified: test/js/web/fetch/blob.test.ts (13 of 16 new tests fail on main).
  • Self-reviewed: 7 concerns raised, 5 addressed (Notes).

Background

  • A Blob is a window (offset, size) on a store. The size MAX_SIZE means "read to EOF".
  • W3C slice() needs the size when it runs: max(size + index, 0).
  • Considered resolve_size() in get_slice (#33601). It caches the stat, so the parent misses an append.

Downsides

  • A negative index on a handle that nothing stat'd costs 1 stat per slice() call. Main and every other slice() call run 0.
  • A file with st_size 0 gives an empty slice, as in Node. On main, a fresh Bun.file("/proc/version").slice(0, -3) read the whole file.
  • Bun.serve now sends the last bytes for slice(-n), with 206. On main it sent an empty body, and serve.test.ts expected that.
Notes

History

  • I found the bug by reading the code for #41209. The one external report is #1311 (2022): slice(-32) returned an empty string. It was closed in 2023 as fixed, but its repro read .size first, and that hides the bug.
  • #33601 fixed the same bug. It called resolve_size() on every slice of a file that was not stat'd, also for a positive index. It was closed without a comment after #39201 merged. This change stats only for a negative index.
  • #43910 names this PR as the owner of a negative index on a handle that nothing stat'd. #39390 and #33687 edit the same lines in get_slice. Their logic does not overlap with this change. #43938 is the same index math for an S3 store, which this PR does not change.

Measurements

Linux x64. "main" is the 1.4.3 canary (367d939d9). "branch" is a debug build of this PR.

A matrix of 2166 cases compares Bun.file() slices with in-memory Blob slices: 6 parents (fresh, stat'd, and four slices) by 19 values for start and end.

mainbranch
text() or stream() differs from the Blob5650
.size differs, with a negative index850

Stat-family syscalls for 50 slice() calls (gdb, catch syscall newfstatat statx fstat, process startup taken off):

50 calls ofmainbranch
Bun.file(p).slice(-5)050
Bun.file(p).slice(0, -1)050
Bun.file(p).slice(3).slice(-5)050
Bun.file(p).slice(0, 5)00
Bun.file(p).slice(5)00
f.size once, then f.slice(-5)11

The parent handle after f.slice(-n), where f is a Bun.file that nothing stat'd:

mainbranch
append 10 bytes to a 10-byte file, then f.size2020
unlink the file, then await f.exists()falsefalse
Bun.write(f, "NEW"), then process.exit(0): runs of 10 where the file is NEW1010

A file whose st_size is 0. Node 26.3.0 with fs.openAsBlob gives the size 0 and "" for each row.

main, freshmain, after .sizebranch
/proc/version, slice(-5)size 5, ""size 0, ""size 0, ""
/proc/version, slice(0, -3)size 4503599627370492, the whole filesize 0, ""size 0, ""
empty file, slice(-5)size 5, ""size 0, ""size 0, ""
empty file, slice(0, -3)size 4503599627370492, ""size 0, ""size 0, ""

Bun.serve with a 26-byte file:

bodymainbranch
Bun.file(p).slice(-5)206, Content-Length: 0, no body206, Content-Length: 5, vwxyz
Bun.file(p).slice(-26)200, Content-Length: 26, no body, the response stalls200, 26 bytes
Bun.file(fd).slice(0, 5)200, Content-Length: 26, the response stallsthe same

Not in this PR

  • .size, exists(), and expect().toHaveLength() of a slice of a handle that nothing stat'd. They do not look at the file. With a 10-byte file, Bun.file(p).slice(3).size is 4503599627370492, slice(5, 5000).size is 4995, and await Bun.file(p).slice(0, 5).exists() is false. After .size on the parent the results are 7, 5, and true. This occurs for a positive index, in get_size, get_size_for_bindings, and get_exists_sync. This PR makes .size correct only when an index is negative. #43910 lists the rest as open. No open PR owns it. A fix has to stat in each call without the cache, or wait for one owner of the stat cache.
  • The last bytes of a file whose st_size is 0 and that has content (procfs). The length is not known without a read, so slice() cannot find the window. It needs a window that counts from the end and that each reader resolves.
  • Bun.serve with Bun.file(fd).slice(...) stalls on main and on this branch, for a positive index too. The server sends a Content-Length for the whole file.

Design

  • Considered a window that counts from the end, which each reader resolves. ReadFile, FileReader, do_sendfile, Bun.write, and spawn stdin would each need it, and .size of the slice still needs the stat.
  • The self-review asked to reuse resolved_size(). That call caches the stat. With the cache, f.slice(-5) followed by an append made f.size, f.text(), and new Response(f).body stop at the old size. So size_for_relative_index shares only the stat call (stat_file) with resolve_file_stat.
  • Self-review, the 2 concerns not addressed. The first is a shared unknown-end check in get_size. It is the first item of "Not in this PR". The second is a test that pins Bun.file(p).slice(0, 100).size === 100 for a small file. That value is the over-report that #32794 and #33360 are about.
  • The window is fixed when slice() runs. If the file grows later, slice(-5) reads the 5 bytes that were last at that time. A handle after .size does the same on main.
  • The status check in the good-range loops of serve.test.ts now comes from the expected body length. The server sends 206 when the body is shorter than the file. For every range that was in the list before, the result is the same.
  • Bun.stdin.slice(-5) over a pipe gives the same result as on main. When stdin is a regular file, it now gives the last 5 bytes.

Tests

  • With main's src/, a debug build fails 13 tests of blob.test.ts and passes 113. With this PR it passes 126. The 3 tests of the new describe that pass on main are guards: -Infinity, an object index, and the unlink.
  • Also ran with the debug build: serve.test.ts, bun-serve-file.test.ts, serve-file-slice-read-error.test.ts, bun-file-read.test.ts, bun-file-fd-read.test.ts, bun-file.test.ts, bun-file-exists.test.js, bun-stdin-slice.test.ts, structured-clone-blob-file.test.ts, bun-write.test.js, body.test.ts, fetch-file-upload.test.ts.
  • Three tests fail in my environment with and without this change: the port 1003 test (it runs as root), the /bun:info test (an egress proxy answers), and bun-write.test.js "on large files" (it takes about 5 s on a debug build).

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/fetch/blob.test.ts, test/js/bun/http/serve.test.ts