From b279c7e4dc798b4c8055af8e974edbc8973d4537 Mon Sep 17 00:00:00 2001 From: Runxi Yu Date: Sat, 7 Mar 2026 23:35:28 +0800 Subject: format/pack/ingest: Use Options; don't require EOF --- format/pack/ingest/api.go | 22 +++++++++++--- format/pack/ingest/finalize.go | 4 +-- format/pack/ingest/ingest_test.go | 61 +++++++++++++++++++++++++++++++++++---- format/pack/ingest/rev_write.go | 2 +- format/pack/ingest/scan.go | 2 +- format/pack/ingest/state.go | 13 ++------- format/pack/ingest/temp.go | 2 +- format/pack/ingest/thin_fix.go | 6 ++-- format/pack/ingest/trailer.go | 17 +++++++++-- 9 files changed, 100 insertions(+), 29 deletions(-) diff --git a/format/pack/ingest/api.go b/format/pack/ingest/api.go index 9e222c1d..f24bf83d 100644 --- a/format/pack/ingest/api.go +++ b/format/pack/ingest/api.go @@ -8,6 +8,22 @@ import ( "codeberg.org/lindenii/furgit/objectstore" ) +// Options controls one pack ingest operation. +type Options struct { + // FixThin appends missing local bases for thin packs. + FixThin bool + // WriteRev writes a .rev alongside the .pack and .idx. + WriteRev bool + // Base supplies existing objects for thin-pack fixup. + Base objectstore.Store + // RequireTrailingEOF requires the source to hit EOF after the pack trailer. + // + // This is suitable for exact pack-file readers, but should be disabled for + // full-duplex transport streams like receive-pack where the peer keeps the + // connection open to read the server response. + RequireTrailingEOF bool +} + // Result describes one successful ingest transaction. type Result struct { // PackName is the destination-relative filename of the written .pack. @@ -39,11 +55,9 @@ func Ingest( src io.Reader, destination *os.Root, algo objectid.Algorithm, - fixThin bool, - writeRev bool, - base objectstore.Store, + opts Options, ) (Result, error) { - state, err := newIngestState(src, destination, algo, fixThin, writeRev, base) + state, err := newIngestState(src, destination, algo, opts) if err != nil { return Result{}, err } diff --git a/format/pack/ingest/finalize.go b/format/pack/ingest/finalize.go index 9b04a7c1..e4dbe72f 100644 --- a/format/pack/ingest/finalize.go +++ b/format/pack/ingest/finalize.go @@ -16,7 +16,7 @@ func finalizeArtifacts(state *ingestState) (Result, error) { idxFinal := base + ".idx" revFinal := "" - if state.writeRev { + if state.opts.WriteRev { revFinal = base + ".rev" } @@ -30,7 +30,7 @@ func finalizeArtifacts(state *ingestState) (Result, error) { return Result{}, err } - if state.writeRev { + if state.opts.WriteRev { err := linkTempToFinal(state, state.revTmpName, revFinal) if err != nil { return Result{}, err diff --git a/format/pack/ingest/ingest_test.go b/format/pack/ingest/ingest_test.go index a40f9ad9..8a88eb7f 100644 --- a/format/pack/ingest/ingest_test.go +++ b/format/pack/ingest/ingest_test.go @@ -14,6 +14,18 @@ import ( "codeberg.org/lindenii/furgit/objectid" ) +type noExtraReadReader struct { + reader *bytes.Reader +} + +func (r *noExtraReadReader) Read(p []byte) (int, error) { + if r.reader.Len() == 0 { + return 0, errors.New("unexpected extra read after pack trailer") + } + + return r.reader.Read(p) +} + // fixturePath returns one fixture file path for the selected algorithm. func fixturePath(t *testing.T, algo objectid.Algorithm, name string) string { t.Helper() @@ -161,7 +173,10 @@ func TestIngestNonThinPackWritesPackIdxRev(t *testing.T) { packRoot := receiver.OpenPackRoot(t) - result, err := ingest.Ingest(bytes.NewReader(packBytes), packRoot, algo, false, true, nil) + result, err := ingest.Ingest(bytes.NewReader(packBytes), packRoot, algo, ingest.Options{ + WriteRev: true, + RequireTrailingEOF: true, + }) if err != nil { t.Fatalf("Ingest: %v", err) } @@ -206,7 +221,10 @@ func TestIngestThinPackWithoutFixReturnsUnresolved(t *testing.T) { receiver := testgit.NewRepo(t, testgit.RepoOptions{ObjectFormat: algo, Bare: true}) packRoot := receiver.OpenPackRoot(t) - _, err := ingest.Ingest(bytes.NewReader(thinPack), packRoot, algo, false, true, nil) + _, err := ingest.Ingest(bytes.NewReader(thinPack), packRoot, algo, ingest.Options{ + WriteRev: true, + RequireTrailingEOF: true, + }) if err == nil { t.Fatal("Ingest error = nil, want error") } @@ -239,14 +257,21 @@ func TestIngestThinPackWithFixThin(t *testing.T) { packRoot := receiver.OpenPackRoot(t) - _, err := ingest.Ingest(bytes.NewReader(basePack), packRoot, algo, false, false, nil) + _, err := ingest.Ingest(bytes.NewReader(basePack), packRoot, algo, ingest.Options{ + RequireTrailingEOF: true, + }) if err != nil { t.Fatalf("ingest base pack: %v", err) } receiverRepo := receiver.OpenRepository(t) - result, err := ingest.Ingest(bytes.NewReader(thinPack), packRoot, algo, true, true, receiverRepo.Objects()) + result, err := ingest.Ingest(bytes.NewReader(thinPack), packRoot, algo, ingest.Options{ + FixThin: true, + WriteRev: true, + Base: receiverRepo.Objects(), + RequireTrailingEOF: true, + }) if err != nil { t.Fatalf("Ingest(thin): %v", err) } @@ -276,7 +301,10 @@ func TestIngestPackTrailerMismatch(t *testing.T) { receiver := testgit.NewRepo(t, testgit.RepoOptions{ObjectFormat: algo, Bare: true}) packRoot := receiver.OpenPackRoot(t) - _, err := ingest.Ingest(bytes.NewReader(packBytes), packRoot, algo, false, true, nil) + _, err := ingest.Ingest(bytes.NewReader(packBytes), packRoot, algo, ingest.Options{ + WriteRev: true, + RequireTrailingEOF: true, + }) if err == nil { t.Fatal("Ingest error = nil, want error") } @@ -297,3 +325,26 @@ func TestIngestPackTrailerMismatch(t *testing.T) { } }) } + +func TestIngestCanFinishWithoutTrailingEOF(t *testing.T) { + t.Parallel() + + testgit.ForEachAlgorithm(t, func(t *testing.T, algo objectid.Algorithm) { //nolint:thelper + head := fixtureOID(t, algo, "head") + packBytes := fixtureBytes(t, algo, "nonthin.pack") + + receiver := testgit.NewRepo(t, testgit.RepoOptions{ObjectFormat: algo, Bare: true}) + packRoot := receiver.OpenPackRoot(t) + + result, err := ingest.Ingest(&noExtraReadReader{reader: bytes.NewReader(packBytes)}, packRoot, algo, ingest.Options{ + WriteRev: true, + }) + if err != nil { + t.Fatalf("Ingest without trailing EOF: %v", err) + } + + receiver.UpdateRef(t, "refs/heads/main", head) + _ = receiver.Run(t, "verify-pack", "-v", filepath.Join("objects", "pack", result.IdxName)) + _ = receiver.Run(t, "fsck", "--full", "--strict", "--no-progress", "--no-dangling") + }) +} diff --git a/format/pack/ingest/rev_write.go b/format/pack/ingest/rev_write.go index 8d30474a..5a2f5375 100644 --- a/format/pack/ingest/rev_write.go +++ b/format/pack/ingest/rev_write.go @@ -14,7 +14,7 @@ const ( // writeRev writes rev index for resolved records. func writeRev(state *ingestState) error { - if !state.writeRev { + if !state.opts.WriteRev { return nil } diff --git a/format/pack/ingest/scan.go b/format/pack/ingest/scan.go index 36b7f75e..d1252d00 100644 --- a/format/pack/ingest/scan.go +++ b/format/pack/ingest/scan.go @@ -40,7 +40,7 @@ func streamPackAndScan(state *ingestState) error { } } - err = state.stream.finishAndFlushTrailer() + err = state.stream.finishAndFlushTrailer(state.opts.RequireTrailingEOF) if err != nil { return err } diff --git a/format/pack/ingest/state.go b/format/pack/ingest/state.go index 2263e8a1..cbc412e3 100644 --- a/format/pack/ingest/state.go +++ b/format/pack/ingest/state.go @@ -5,7 +5,6 @@ import ( "os" "codeberg.org/lindenii/furgit/objectid" - "codeberg.org/lindenii/furgit/objectstore" ) const ( @@ -17,9 +16,7 @@ type ingestState struct { src io.Reader destination *os.Root algo objectid.Algorithm - fixThin bool - writeRev bool - base objectstore.Store + opts Options packFile *os.File packTmpName string @@ -49,9 +46,7 @@ func newIngestState( src io.Reader, destination *os.Root, algo objectid.Algorithm, - fixThin bool, - writeRev bool, - base objectstore.Store, + opts Options, ) (*ingestState, error) { if algo.Size() == 0 { return nil, objectid.ErrInvalidAlgorithm @@ -61,9 +56,7 @@ func newIngestState( src: src, destination: destination, algo: algo, - fixThin: fixThin, - writeRev: writeRev, - base: base, + opts: opts, offsetToRecord: make(map[uint64]int), objectToRecord: make(map[objectid.ObjectID]int), baseCache: newDeltaBaseCache(defaultDeltaBaseCacheMaxBytes), diff --git a/format/pack/ingest/temp.go b/format/pack/ingest/temp.go index 597ec487..2f898609 100644 --- a/format/pack/ingest/temp.go +++ b/format/pack/ingest/temp.go @@ -26,7 +26,7 @@ func openTemporaryArtifacts(state *ingestState) error { revName := "" var revFile *os.File - if state.writeRev { + if state.opts.WriteRev { revName, revFile, err = createTempFile(state.destination, "tmp_rev_") if err != nil { _ = idxFile.Close() diff --git a/format/pack/ingest/thin_fix.go b/format/pack/ingest/thin_fix.go index f21ef98a..cdee8748 100644 --- a/format/pack/ingest/thin_fix.go +++ b/format/pack/ingest/thin_fix.go @@ -12,11 +12,11 @@ func maybeFixThin(state *ingestState) error { return nil } - if !state.fixThin { + if !state.opts.FixThin { return &ThinPackUnresolvedError{Count: len(state.unresolvedRefDeltas)} } - if state.base == nil { + if state.opts.Base == nil { return &ThinPackUnresolvedError{Count: len(state.unresolvedRefDeltas)} } @@ -48,7 +48,7 @@ func maybeFixThin(state *ingestState) error { baseIDs := unresolvedThinBaseIDs(state) for _, id := range baseIDs { - ty, content, err := state.base.ReadBytesContent(id) + ty, content, err := state.opts.Base.ReadBytesContent(id) if err != nil { continue } diff --git a/format/pack/ingest/trailer.go b/format/pack/ingest/trailer.go index ea945b10..30ba3bb8 100644 --- a/format/pack/ingest/trailer.go +++ b/format/pack/ingest/trailer.go @@ -8,8 +8,8 @@ import ( ) // finishAndFlushTrailer reads trailer hash bytes, verifies trailer checksum, -// and ensures no trailing garbage remains in stream. -func (scanner *streamScanner) finishAndFlushTrailer() error { +// and optionally requires the source stream to hit EOF afterward. +func (scanner *streamScanner) finishAndFlushTrailer(requireTrailingEOF bool) error { if scanner.hashSize <= 0 { return fmt.Errorf("format/pack/ingest: invalid hash size") } @@ -25,6 +25,19 @@ func (scanner *streamScanner) finishAndFlushTrailer() error { scanner.packTrailer = append(scanner.packTrailer[:0], trailer...) + if scanner.n-scanner.off > 0 { + return fmt.Errorf("format/pack/ingest: pack has trailing garbage") + } + + if !requireTrailingEOF { + computed := scanner.hash.Sum(nil) + if !bytes.Equal(computed, trailer) { + return &PackTrailerMismatchError{} + } + + return nil + } + var probe [1]byte n, err := scanner.Read(probe[:]) -- cgit v1.3.1-10-gc9f91