Ver código fonte

Merge branch 'limit-decompression' of bajdev/mpckit into main

bajdev 14 horas atrás
pai
commit
fdc53c38c9

+ 3 - 3
docs/review-fixes-plan.md

@@ -8,7 +8,7 @@ These were settled with the user on 2026-11-12. Agents follow them and do not re
 
 | Topic | Decision |
 | --- | --- |
-| Decompression limit (S2) | 512 MB of decompressed project data. |
+| Decompression limit (S2) | 256 MB of decompressed project data (lowered from 512 MB on 2026-11-12, during PR 2: a blank project is 3 MB and 256 MB holds about half a million recorded events). |
 | Bound paths (S4) | Approved: `LoadPath` and `AssignSamplePath` stop being bound; export methods stop taking a path. |
 | Pad labels (M6) | Always the "A1" format (bank letter, pad number without a leading zero). |
 | Releases | 0.5.2 after PRs 1-3, 0.5.3 after PRs 5-8. PRs in between do not change the version. |
@@ -119,8 +119,8 @@ Each PR starts from `main` after the previous one is merged. The order matters i
 
 **Tasks**
 
-- [ ] Add `const MaxDecompressed = 512 << 20` in `internal/xpj` with a comment giving the reason.
-- [ ] In `Read`, read through `io.LimitReader(zr, MaxDecompressed+1)` and return `xpj: project is larger than 512 MB when decompressed` past the limit.
+- [ ] Add `const MaxDecompressed = 256 << 20` in `internal/xpj` with a comment giving the reason.
+- [ ] In `Read`, read through `io.LimitReader(zr, MaxDecompressed+1)` and return `xpj: project is larger than 256 MB when decompressed` past the limit.
 
 **Done when**
 

+ 11 - 1
internal/xpj/file.go

@@ -69,6 +69,13 @@ func (f *File) Encode() ([]byte, error) {
 	return b.Bytes(), nil
 }
 
+// MaxDecompressed is the largest project Read accepts, in decompressed
+// bytes. A small file can decompress to gigabytes (a 1 MB file padded with
+// spaces used 3.1 GB), so Read stops there instead of running out of
+// memory. Device projects are about 3 MB; 256 MB holds about half a
+// million recorded events.
+const MaxDecompressed = 256 << 20
+
 // Read decompresses and decodes a project file.
 func Read(r io.Reader) (*File, error) {
 	zr, err := gzip.NewReader(r)
@@ -76,10 +83,13 @@ func Read(r io.Reader) (*File, error) {
 		return nil, fmt.Errorf("xpj: not a gzip stream: %w", err)
 	}
 	defer zr.Close()
-	raw, err := io.ReadAll(zr)
+	raw, err := io.ReadAll(io.LimitReader(zr, MaxDecompressed+1))
 	if err != nil {
 		return nil, fmt.Errorf("xpj: decompress: %w", err)
 	}
+	if len(raw) > MaxDecompressed {
+		return nil, fmt.Errorf("xpj: project is larger than %d MB when decompressed", MaxDecompressed>>20)
+	}
 	return Decode(raw)
 }
 

+ 50 - 0
internal/xpj/xpj_test.go

@@ -381,3 +381,53 @@ func TestNilNode(t *testing.T) {
 		t.Error("Get on nil: no error")
 	}
 }
+
+// gzipPadded returns a gzip stream that decompresses to a minimal project
+// whose JSON body is padded with spaces to exactly size bytes.
+func gzipPadded(t *testing.T, size int) []byte {
+	t.Helper()
+	head := "ACVS\n1.3.0.12\nSerialisableProjectData\njson\nLinux\n{\"data\": {}"
+	if size < len(head)+1 {
+		t.Fatalf("size %d too small", size)
+	}
+	var b bytes.Buffer
+	zw := gzip.NewWriter(&b)
+	if _, err := io.WriteString(zw, head); err != nil {
+		t.Fatal(err)
+	}
+	spaces := bytes.Repeat([]byte(" "), 1<<20)
+	for left := size - len(head) - 1; left > 0; {
+		n := min(left, len(spaces))
+		if _, err := zw.Write(spaces[:n]); err != nil {
+			t.Fatal(err)
+		}
+		left -= n
+	}
+	if _, err := io.WriteString(zw, "}"); err != nil {
+		t.Fatal(err)
+	}
+	if err := zw.Close(); err != nil {
+		t.Fatal(err)
+	}
+	return b.Bytes()
+}
+
+func TestDecompressionLimit(t *testing.T) {
+	if testing.Short() {
+		t.Skip("decompresses 512 MB")
+	}
+	// One byte over: refused.
+	_, err := Read(bytes.NewReader(gzipPadded(t, MaxDecompressed+1)))
+	want := "xpj: project is larger than 256 MB when decompressed"
+	if err == nil || err.Error() != want {
+		t.Fatalf("over the limit: got %v, want %q", err, want)
+	}
+	// Exactly at the limit: still decodes.
+	f, err := Read(bytes.NewReader(gzipPadded(t, MaxDecompressed)))
+	if err != nil {
+		t.Fatalf("at the limit: %v", err)
+	}
+	if _, err := f.Data(); err != nil {
+		t.Fatalf("at the limit: %v", err)
+	}
+}

+ 46 - 0
pull-requests/limit-decompression/cap-decompressed-project-size.md

@@ -0,0 +1,46 @@
+# Cap decompressed project size
+
+Branch: `limit-decompression` → `main`
+
+PR 2 of the review fixes plan (`docs/review-fixes-plan.md`). Part of 0.5.2; the version does not change in this PR.
+
+## What changed
+
+**Projects over 256 MB decompressed are refused (S2).** `xpj.Read` used to read the whole gzip stream into memory, however large it was. In the review, a 1 MB file padded with spaces used 3.1 GB and took 10 seconds. `Read` now reads through `io.LimitReader` and stops one byte past the new `xpj.MaxDecompressed`, 256 MB. Past that, it returns `xpj: project is larger than 256 MB when decompressed`. A comment on the constant gives the reason.
+
+**The limit is 256 MB, not the plan's 512 MB** (bajdev, 2026-11-12, during this PR). The limit counts only the project file once it is unpacked: the pads, the pool's list of file names and the sequences with their events. WAV files are separate files in the data folder and are not read through `xpj`.
+- A blank project is about 3 MB, almost all of it the 128 empty pad definitions.
+- Each recorded event takes about 480 bytes, and an empty sequence about 12 KB.
+- So 256 MB holds about half a million events, around 4,000 for each of 128 sequences. DemoFile has 225 events in total.
+
+The Decisions table and PR 2's tasks in `docs/review-fixes-plan.md` now say 256 MB.
+
+**Test.** `TestDecompressionLimit` builds its gzip streams in memory with `gzip.NewWriter`; nothing new is committed:
+- A minimal project padded with spaces to one byte over the limit is refused with that exact error.
+- The same project padded to exactly the limit still decodes.
+- Against the old `Read`, the over-limit case loads without error, so the test fails.
+- It takes about 3 seconds, and `go test -short` skips it.
+
+## Notes
+
+**Memory near the limit, as measured on the server:**
+- A file padded to 3.1 GB (6 MB compressed) is now refused after 0.3 seconds, at a peak of about 600 MB. The buffer `io.ReadAll` grows can pass the limit before the check runs.
+- A file exactly at the limit still loads, at a peak of about 925 MB, because the JSON decoder keeps its own copy of the padding.
+- At 512 MB, these were about 1.3 GB and 2.3 GB.
+
+**The test needs memory:** about 860 MB while it runs. `go test -short ./...` skips it.
+
+## Testing
+
+**On the server:**
+- `go test ./...` passes, including the round-trip tests, unchanged.
+- `go vet ./...` and `staticcheck ./...` are clean.
+- `govulncheck ./...` reports only GO-2026-6859 in `net/http`, which waits for Go 1.27.4 in PR 4.
+- `npm install`, `npm run check` (0 errors, 0 warnings) and `npm run build` pass. The lockfile `npm install` wrote was not committed.
+- `wails3 build` builds, and `timeout 15 xvfb-run -a bin/mpckit` starts and runs. The log holds only the usual headless GTK warnings.
+
+**On the laptop (PowerShell, in the repo):**
+1. `git fetch; git checkout limit-decompression`
+2. `go test -run TestDecompressionLimit -v ./internal/xpj`: `PASS`, in a few seconds. It uses about 900 MB of memory.
+3. `go test ./...`: all `ok`.
+4. `wails3 dev`: load `testdata\device\DemoFile.xpj` and `testdata\accepted\MpckitT1.xpj`. Both open as before.