fix(riff): grow a chunk buffer with what arrives #224

2026-08-26T10:35:45Z

logan commented

2026-08-26 10:35:45 +00:00

The issue. #216 — riff.Parse sized its chunk buffer from the
chunk header, which is four bytes read off the file, so a truncated or
malformed WAV declaring a 4 GB data chunk in a 2 kB file asked the
allocator for 4 GB before the read discovered there was nothing to put
in it. The error was correct; the allocation happened first.

The change. The cheaper of the two answers the issue names, and the
one already sitting eight lines below in the same file: io.CopyN into
a bytes.Buffer, as riff.ID3Chunk has done since #104. The buffer
grows with what actually arrives, the reader stays an io.Reader, and
nothing new is introduced — the alternative (a ceiling from the file's
own size) would need a size that is not to hand.

The assertion is the interesting half. The error is identical on a
build that allocates the gigabyte — unexpected EOF either way — so the
obvious test is green on the broken build. This one measures
runtime.MemStats.TotalAlloc across a Parse of a 42-byte container
whose data chunk claims 1 GiB. Measured on the pre-fix build:
1,073,750,920 bytes. After: under the 1 MiB threshold. It is
deliberately not t.Parallel(), because TotalAlloc is process-wide
and Go resumes paused parallel tests only once the sequential ones have
run.

Verification tiers run.

make lint — 0 issues, all three build configurations.
make test — clean, all three passes, -race throughout.
The new test additionally at -count=5 with and without -race, and
the whole package at -count=5 both ways, to check the measurement is
not flaky.
backend/tagwriter and backend/metadata run explicitly: Parse's
only caller is writeWavTags, and the round-trip tests there are what
say a real WAV still rewrites correctly.

Deliberately not done. No make generate / make bindings (no
.sql, no .templ, no bound signature), no make ui-test / make e2e
(no frontend change and nothing user-visible), no screenshot tier.
CLAUDE.md is untouched because the shape it describes — Parse holds
every chunk in memory, ID3Chunk seeks — is unchanged; only the belief
in a size nobody supplied has gone. One consequence worth knowing: a
zero-length chunk's Data is now nil rather than an empty non-nil
slice, which writeRIFF cannot tell apart since it writes len(Data)
either way.

No new issues filed.

Closes #216

**The issue.** #216 — `riff.Parse` sized its chunk buffer from the chunk header, which is four bytes read off the file, so a truncated or malformed WAV declaring a 4 GB `data` chunk in a 2 kB file asked the allocator for 4 GB before the read discovered there was nothing to put in it. The error was correct; the allocation happened first. **The change.** The cheaper of the two answers the issue names, and the one already sitting eight lines below in the same file: `io.CopyN` into a `bytes.Buffer`, as `riff.ID3Chunk` has done since #104. The buffer grows with what actually arrives, the reader stays an `io.Reader`, and nothing new is introduced — the alternative (a ceiling from the file's own size) would need a size that is not to hand. **The assertion is the interesting half.** The error is *identical* on a build that allocates the gigabyte — `unexpected EOF` either way — so the obvious test is green on the broken build. This one measures `runtime.MemStats.TotalAlloc` across a `Parse` of a 42-byte container whose `data` chunk claims 1 GiB. Measured on the pre-fix build: **1,073,750,920 bytes**. After: under the 1 MiB threshold. It is deliberately not `t.Parallel()`, because `TotalAlloc` is process-wide and Go resumes paused parallel tests only once the sequential ones have run. **Verification tiers run.** - `make lint` — 0 issues, all three build configurations. - `make test` — clean, all three passes, `-race` throughout. - The new test additionally at `-count=5` with and without `-race`, and the whole package at `-count=5` both ways, to check the measurement is not flaky. - `backend/tagwriter` and `backend/metadata` run explicitly: `Parse`'s only caller is `writeWavTags`, and the round-trip tests there are what say a real WAV still rewrites correctly. **Deliberately not done.** No `make generate` / `make bindings` (no `.sql`, no `.templ`, no bound signature), no `make ui-test` / `make e2e` (no frontend change and nothing user-visible), no screenshot tier. CLAUDE.md is untouched because the shape it describes — `Parse` holds every chunk in memory, `ID3Chunk` seeks — is unchanged; only the belief in a size nobody supplied has gone. One consequence worth knowing: a zero-length chunk's `Data` is now nil rather than an empty non-nil slice, which `writeRIFF` cannot tell apart since it writes `len(Data)` either way. No new issues filed. Closes #216

logan added 1 commit 2026-08-26 10:35:45 +00:00

fix(riff): grow a chunk buffer with what arrives

CI / check (push) Skipped

CI / e2e (push) Skipped

CI / check (pull_request) Successful in 6m23s

Details

CI / e2e (pull_request) Successful in 10m8s

Details

a113b7bd62

Parse sized its buffer from the chunk header, which is four bytes read
off the file, so a truncated or malformed WAV declaring a 4 GB data
chunk in a 2 kB file got 4 GB from the allocator before the read
discovered there was nothing to put in it. The error was always right;
the allocation happened first.

io.CopyN into a bytes.Buffer is what ID3Chunk beside it has done since
#104, and needs nothing new: the reader stays an io.Reader and the
buffer grows with what actually arrives.

The regression test measures rather than asserts the error, because the
error is identical on a build that allocates the gigabyte. Measured on
the pre-fix build: 1,073,750,920 bytes of TotalAlloc for a 42-byte
container whose data chunk claimed 1 GiB.

Closes #216