archive: stream a small *bytes.Buffer like any reader
klauspost's zstd decodes a reader with Bytes() and Len() under 128 KiB in one go and never drains it, so Extract's and Verify's end-of-stream check read the archive again and refused it as data after the archive (T4 review M1). WithDecodeBuffersBelow(0) turns the shortcut off. A test puts a small archive in a *bytes.Buffer through Extract, the decode Verify runs and Verify itself; it fails without the option. BuildFiltered's comment no longer says the offline move uses it: the sealer archives the nested files directory with Build.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3 files changed,
+50
−7
internal/moves/archive/archive.go+5 −6internal/moves/archive/archive_test.go+39 −0internal/moves/archive/limits.go+6 −1
diff --git a/internal/moves/archive/archive.go b/internal/moves/archive/archive.go| index c48336a52..a017e20ad 100644 |
| --- a/internal/moves/archive/archive.go |
| +++ b/internal/moves/archive/archive.go |
| @@ -430,12 +430,11 @@ func Build(filesDir, envPath string, w io.Writer) (plain string, entries []Entry |
| // accepts. skip is asked each path relative to filesDir, slash-separated |
| // (a top-level entry is its bare name); a skipped directory is skipped |
| // whole, and a skipped entry of any type is never read, typed or counted. |
| -// The env entry is never asked. A nil skip is Build. The offline move |
| -// archives a nested instance directory this way, leaving out the |
| -// platform's own files there (layout.NestedPlatformNames): the archive is |
| -// then the one Build would write over a flat files directory holding the |
| -// same app files, so it extracts flat and PlainDigest of the extraction |
| -// equals plain. |
| +// The env entry is never asked. A nil skip is Build. Nothing calls it with |
| +// a filter today: the offline move's sealer archives a nested instance's |
| +// files directory (<instance dir>/<host>, which holds nothing of the |
| +// platform's but the marker) with Build, and refuses the seal over any |
| +// entry of the app's beside it. |
| func BuildFiltered(filesDir, envPath string, skip func(rel string) bool, w io.Writer) (plain string, entries []Entry, bytes int64, err error) { |
| zw, err := zstd.NewWriter(w, zstd.WithEncoderConcurrency(1), zstd.WithWindowSize(zstdWindow)) |
| if err != nil { |
diff --git a/internal/moves/archive/archive_test.go b/internal/moves/archive/archive_test.go| index 49ef8e1fd..833507a49 100644 |
| --- a/internal/moves/archive/archive_test.go |
| +++ b/internal/moves/archive/archive_test.go |
| @@ -923,6 +923,45 @@ func TestExtractWithoutEnvLeavesNoEnv(t *testing.T) { |
| } |
| } |
| +// A small archive handed over in a *bytes.Buffer (under the decoder's |
| +// 128 KiB shortcut for a reader with Bytes() and Len()) is read as a |
| +// stream like any other: Extract, the decode Verify runs, and Verify itself |
| +// accept it, rather than finding the whole archive still unread after it. |
| +func TestASmallArchiveInABytesBufferExtractsAndVerifies(t *testing.T) { |
| + dir, env := filesFixture(t) |
| + var tarz bytes.Buffer |
| + plain, entries, n, err := Build(dir, env, &tarz) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if tarz.Len() >= 128<<10 { |
| + t.Fatalf("fixture: %d bytes is not under 128 KiB", tarz.Len()) |
| + } |
| + raw := append([]byte(nil), tarz.Bytes()...) |
| + parent := t.TempDir() |
| + files, envOut := filepath.Join(parent, "h"), filepath.Join(parent, "h.env") |
| + if err := Extract(bytes.NewBuffer(append([]byte(nil), raw...)), files, envOut); err != nil { |
| + t.Fatalf("Extract from a *bytes.Buffer: %v", err) |
| + } |
| + if got, _, _, err := PlainDigest(files, envOut); err != nil || got != plain { |
| + t.Fatalf("the extraction digests to %s (%v), want %s", got, err, plain) |
| + } |
| + m := Manifest{V: ManifestVersion, Op: "mv_x", Plain: plain, Bytes: n, Entries: entries, Chunk: ChunkSize} |
| + if got, err := verifyStream(bytes.NewBuffer(append([]byte(nil), raw...)), m); err != nil || got != plain { |
| + t.Fatalf("verifyStream from a *bytes.Buffer: %s, %v", got, err) |
| + } |
| + pub, priv, b := keys(t) |
| + var sealed bytes.Buffer |
| + key, sealedSHA, err := Seal(bytes.NewReader(raw), pub, b, &sealed) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + m.Sealed, m.SealedDataKey = sealedSHA, key |
| + if err := Verify(&sealed, m, priv, b); err != nil { |
| + t.Fatalf("Verify from a *bytes.Buffer: %v", err) |
| + } |
| +} |
| + |
| // sealedArchive is a good archive with its manifest, over a tree large |
| // enough to seal into three chunks. |
| type sealedArchive struct { |
diff --git a/internal/moves/archive/limits.go b/internal/moves/archive/limits.go| index 44f6bf90c..0a4fcd66c 100644 |
| --- a/internal/moves/archive/limits.go |
| +++ b/internal/moves/archive/limits.go |
| @@ -209,8 +209,13 @@ func (b *boundedReader) Read(p []byte) (int, error) { |
| return n, err |
| } |
| +// newDecoder always streams from r. WithDecodeBuffersBelow(0) turns off |
| +// the decoder's shortcut for a reader with Bytes() and Len() (a |
| +// *bytes.Buffer) under 128 KiB, which decodes the whole buffer at once and |
| +// never drains it: requireEOF would then read the archive again and refuse |
| +// it as data after the archive. |
| func newDecoder(r io.Reader) (*zstd.Decoder, error) { |
| - return zstd.NewReader(r, zstd.WithDecoderConcurrency(1), zstd.WithDecoderMaxWindow(zstdWindow)) |
| + return zstd.NewReader(r, zstd.WithDecoderConcurrency(1), zstd.WithDecoderMaxWindow(zstdWindow), zstd.WithDecodeBuffersBelow(0)) |
| } |
| // requireEOF reads the raw stream once more after the decoder finished. A |