fix(nwsync): hash through a section reader, pin the encoder claim (#76)
ci / ci (pull_request) Successful in 3m30s
ci / ci (pull_request) Successful in 3m30s
Review follow-ups. checkArtifactKey drained the *os.File to EOF, so the file offset was left at the end; correct only because everything after it uses ReadAt. Hash a section reader instead, so no later sequential read can silently see nothing. The zstd concurrency comment claimed byte-identical output but nothing asserted it, so add the test. erf.Read now says outright that Data must not be mutated, since payloads alias one buffer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+3
-2
@@ -389,8 +389,9 @@ func ReadPayload(r io.ReaderAt, entry IndexEntry) ([]byte, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Read materialises a whole archive. Payloads are subslices of the buffer the
|
// Read materialises a whole archive. Payloads are subslices of the buffer the
|
||||||
// archive was read into, so nothing is copied twice; callers that only need one
|
// archive was read into, so nothing is copied twice: a caller must not mutate
|
||||||
// resource at a time should use ReadIndex instead.
|
// Data. Callers that only need one resource at a time should use ReadIndex
|
||||||
|
// instead, which never holds the archive at all.
|
||||||
func Read(r io.Reader) (Archive, error) {
|
func Read(r io.Reader) (Archive, error) {
|
||||||
data, err := io.ReadAll(r)
|
data, err := io.ReadAll(r)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -88,7 +88,9 @@ func Emit(options EmitOptions) (EmitResult, error) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return EmitResult{}, fmt.Errorf("read artifact: %w", err)
|
return EmitResult{}, fmt.Errorf("read artifact: %w", err)
|
||||||
}
|
}
|
||||||
if err := checkArtifactKey(options.ArtifactKey, artifact); err != nil {
|
// A section reader, not the file itself: hashing must not move the file
|
||||||
|
// offset out from under everything that reads the artifact afterwards.
|
||||||
|
if err := checkArtifactKey(options.ArtifactKey, io.NewSectionReader(artifact, 0, info.Size())); err != nil {
|
||||||
return EmitResult{}, err
|
return EmitResult{}, err
|
||||||
}
|
}
|
||||||
name := options.As
|
name := options.As
|
||||||
|
|||||||
@@ -1,7 +1,9 @@
|
|||||||
package nwsync
|
package nwsync
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"math/rand"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"runtime"
|
"runtime"
|
||||||
@@ -9,9 +11,31 @@ import (
|
|||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/klauspost/compress/zstd"
|
||||||
|
|
||||||
"git.westgate.pw/ShadowsOverWestgate/sow-tools/internal/erf"
|
"git.westgate.pw/ShadowsOverWestgate/sow-tools/internal/erf"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// TestSingleThreadedEncoderMatchesDefault pins the claim the blob encoder's
|
||||||
|
// concurrency setting rests on: it saves memory only, and a published blob is
|
||||||
|
// the same bytes either way.
|
||||||
|
func TestSingleThreadedEncoderMatchesDefault(t *testing.T) {
|
||||||
|
standard, err := zstd.NewWriter(nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
defer standard.Close()
|
||||||
|
|
||||||
|
body := make([]byte, 4<<20)
|
||||||
|
random := rand.New(rand.NewSource(1))
|
||||||
|
random.Read(body[:len(body)/2])
|
||||||
|
for _, size := range []int{0, 1, 4 << 10, len(body)} {
|
||||||
|
if !bytes.Equal(blobEncoder.EncodeAll(body[:size], nil), standard.EncodeAll(body[:size], nil)) {
|
||||||
|
t.Fatalf("%d bytes compress differently at concurrency 1", size)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// resourceSize is one payload in the memory fixtures. Real haks hold a few MB
|
// resourceSize is one payload in the memory fixtures. Real haks hold a few MB
|
||||||
// per resource, and peak memory is meant to track that, not the archive.
|
// per resource, and peak memory is meant to track that, not the archive.
|
||||||
const resourceSize = 1 << 20
|
const resourceSize = 1 << 20
|
||||||
|
|||||||
Reference in New Issue
Block a user