Bun.file().slice(): count a negative index back from the file's real end
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 fromself.size. Until something stats aBun.file(), that size isMAX_SIZE. Soslice(-5)gets the offsetMAX_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_slicestats 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 sizeMAX_SIZEmeans "read to EOF". - W3C
slice()needs the size when it runs:max(size + index, 0). - Considered
resolve_size()inget_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
statperslice()call. Main and every otherslice()call run 0. - A file with
st_size0 gives an empty slice, as in Node. On main, a freshBun.file("/proc/version").slice(0, -3)read the whole file. Bun.servenow sends the last bytes forslice(-n), with 206. On main it sent an empty body, andserve.test.tsexpected 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.sizefirst, 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.
| main | branch | |
|---|---|---|
text() or stream() differs from the Blob | 565 | 0 |
.size differs, with a negative index | 85 | 0 |
Stat-family syscalls for 50 slice() calls (gdb, catch syscall newfstatat statx fstat, process startup taken off):
| 50 calls of | main | branch |
|---|---|---|
Bun.file(p).slice(-5) | 0 | 50 |
Bun.file(p).slice(0, -1) | 0 | 50 |
Bun.file(p).slice(3).slice(-5) | 0 | 50 |
Bun.file(p).slice(0, 5) | 0 | 0 |
Bun.file(p).slice(5) | 0 | 0 |
f.size once, then f.slice(-5) | 1 | 1 |
The parent handle after f.slice(-n), where f is a Bun.file that nothing stat'd:
| main | branch | |
|---|---|---|
append 10 bytes to a 10-byte file, then f.size | 20 | 20 |
unlink the file, then await f.exists() | false | false |
Bun.write(f, "NEW"), then process.exit(0): runs of 10 where the file is NEW | 10 | 10 |
A file whose st_size is 0. Node 26.3.0 with fs.openAsBlob gives the size 0 and "" for each row.
| main, fresh | main, after .size | branch | |
|---|---|---|---|
/proc/version, slice(-5) | size 5, "" | size 0, "" | size 0, "" |
/proc/version, slice(0, -3) | size 4503599627370492, the whole file | size 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:
| body | main | branch |
|---|---|---|
Bun.file(p).slice(-5) | 206, Content-Length: 0, no body | 206, Content-Length: 5, vwxyz |
Bun.file(p).slice(-26) | 200, Content-Length: 26, no body, the response stalls | 200, 26 bytes |
Bun.file(fd).slice(0, 5) | 200, Content-Length: 26, the response stalls | the same |
Not in this PR
.size,exists(), andexpect().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).sizeis 4503599627370492,slice(5, 5000).sizeis 4995, andawait Bun.file(p).slice(0, 5).exists()isfalse. After.sizeon the parent the results are 7, 5, andtrue. This occurs for a positive index, inget_size,get_size_for_bindings, andget_exists_sync. This PR makes.sizecorrect 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_sizeis 0 and that has content (procfs). The length is not known without a read, soslice()cannot find the window. It needs a window that counts from the end and that each reader resolves. Bun.servewithBun.file(fd).slice(...)stalls on main and on this branch, for a positive index too. The server sends aContent-Lengthfor 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.sizeof 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 madef.size,f.text(), andnew Response(f).bodystop at the old size. Sosize_for_relative_indexshares only the stat call (stat_file) withresolve_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 pinsBun.file(p).slice(0, 100).size === 100for 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.sizedoes the same on main. - The status check in the good-range loops of
serve.test.tsnow 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 ofblob.test.tsand 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:infotest (an egress proxy answers), andbun-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