Преглед изворни кода

Cap decompressed project size at 512 MB

xpj.Read read the whole gzip stream into memory, so a small file
padded with spaces could use gigabytes (S2). Read now stops one byte
past MaxDecompressed and returns a plain error. TestDecompressionLimit
checks one byte over is refused and exactly the limit still decodes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude пре 14 часа
родитељ
комит
1f726b36af

+ 10 - 1
internal/xpj/file.go

@@ -69,6 +69,12 @@ 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.
+const MaxDecompressed = 512 << 20
+
 // Read decompresses and decodes a project file.
 func Read(r io.Reader) (*File, error) {
 	zr, err := gzip.NewReader(r)
@@ -76,10 +82,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 1 GB")
+	}
+	// One byte over: refused.
+	_, err := Read(bytes.NewReader(gzipPadded(t, MaxDecompressed+1)))
+	want := "xpj: project is larger than 512 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)
+	}
+}

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

@@ -0,0 +1,42 @@
+# 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 512 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` (512 MB, as decided). Past that, it returns `xpj: project is larger than 512 MB when decompressed`. A comment on the constant gives the reason. Device projects are about 3 MB decompressed, so real projects are far below the limit.
+
+**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 11 seconds, and `go test -short` skips it.
+
+## Notes
+
+**Memory is still high near the limit.** The limit stops the read early, but memory still grows well past 512 MB:
+- **Over the limit:** a file padded to 3.1 GB (6 MB compressed) is now refused after 1.3 seconds, at a peak of about 1.3 GB. The buffer `io.ReadAll` grows can reach roughly 2.5 times what it holds before the check runs.
+- **Exactly at the limit:** a file that passes the check still costs about 2.3 GB while it decodes, because the JSON decoder keeps its own copy of the padding.
+
+This is what the 512 MB decision implies, and it no longer grows without bound. If you want it lower, there are two options for a later PR:
+- Lower the limit. Device projects are about 3 MB, so 64 MB would still leave plenty of room.
+- Read into a buffer sized up front, which saves the growth but not the decoder's copy.
+
+**The test needs memory.** `TestDecompressionLimit` peaks at about 2.3 GB while it decodes the at-the-limit file. The server ran it with no trouble alongside the other tests. Use `go test -short ./...` to skip 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 about 10–20 seconds. It uses about 2.3 GB of memory.
+3. `go test ./...`: all `ok`.
+4. `wails3 dev`: load `testdata\device\DemoFile.xpj` and `testdata\accepted\MpckitT1.xpj`. Both open as before.