shithub: furgit

Download patch

ref: b279c7e4dc798b4c8055af8e974edbc8973d4537
parent: b8bd9e766c1940ea2568e94577a7c56f8ff0d2f3
author: Runxi Yu <me@runxiyu.org>
date: Sat Mar 7 18:35:28 EST 2026

format/pack/ingest: Use Options; don't require EOF

--- a/format/pack/ingest/api.go
+++ b/format/pack/ingest/api.go
@@ -8,6 +8,22 @@
 	"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 @@
 	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
 	}
--- a/format/pack/ingest/finalize.go
+++ b/format/pack/ingest/finalize.go
@@ -16,7 +16,7 @@
 	idxFinal := base + ".idx"
 
 	revFinal := ""
-	if state.writeRev {
+	if state.opts.WriteRev {
 		revFinal = base + ".rev"
 	}
 
@@ -30,7 +30,7 @@
 		return Result{}, err
 	}
 
-	if state.writeRev {
+	if state.opts.WriteRev {
 		err := linkTempToFinal(state, state.revTmpName, revFinal)
 		if err != nil {
 			return Result{}, err
--- a/format/pack/ingest/ingest_test.go
+++ b/format/pack/ingest/ingest_test.go
@@ -14,6 +14,18 @@
 	"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 @@
 
 		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 @@
 		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,7 +257,9 @@
 
 		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)
 		}
@@ -246,7 +266,12 @@
 
 		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 @@
 		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")
 		}
@@ -295,5 +323,28 @@
 				t.Fatalf("found finalized pack file after failure: %v", entry.Name())
 			}
 		}
+	})
+}
+
+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")
 	})
 }
--- a/format/pack/ingest/rev_write.go
+++ b/format/pack/ingest/rev_write.go
@@ -14,7 +14,7 @@
 
 // writeRev writes rev index for resolved records.
 func writeRev(state *ingestState) error {
-	if !state.writeRev {
+	if !state.opts.WriteRev {
 		return nil
 	}
 
--- a/format/pack/ingest/scan.go
+++ b/format/pack/ingest/scan.go
@@ -40,7 +40,7 @@
 		}
 	}
 
-	err = state.stream.finishAndFlushTrailer()
+	err = state.stream.finishAndFlushTrailer(state.opts.RequireTrailingEOF)
 	if err != nil {
 		return err
 	}
--- a/format/pack/ingest/state.go
+++ b/format/pack/ingest/state.go
@@ -5,7 +5,6 @@
 	"os"
 
 	"codeberg.org/lindenii/furgit/objectid"
-	"codeberg.org/lindenii/furgit/objectstore"
 )
 
 const (
@@ -17,9 +16,7 @@
 	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 @@
 	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 @@
 		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),
--- a/format/pack/ingest/temp.go
+++ b/format/pack/ingest/temp.go
@@ -26,7 +26,7 @@
 	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()
--- a/format/pack/ingest/thin_fix.go
+++ b/format/pack/ingest/thin_fix.go
@@ -12,11 +12,11 @@
 		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 @@
 
 	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
 		}
--- a/format/pack/ingest/trailer.go
+++ b/format/pack/ingest/trailer.go
@@ -8,8 +8,8 @@
 )
 
 // 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")
 	}
@@ -24,6 +24,19 @@
 	}
 
 	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
 
--