Discard unread data of `git cat-file` (#29297) (#29310)

Backport #29297 by @KN4CK3R

Fixes #29101
Related #29298

Discard all read data to prevent misinterpreting existing data. Some
discard calls were missing in error cases.

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: yp05327 <576951401@qq.com>

modules/git/batch_reader.go

@@ -203,16 +203,7 @@ headerLoop:
 	}
 
 	// Discard the rest of the tag
-	discard := size - n + 1
+	return id, DiscardFull(rd, size-n+1)
-	for discard > math.MaxInt32 {
-		_, err := rd.Discard(math.MaxInt32)
-		if err != nil {
-			return id, err
-		}
-		discard -= math.MaxInt32
-	}
-	_, err := rd.Discard(int(discard))
-	return id, err
 }
 
 // ReadTreeID reads a tree ID from a cat-file --batch stream, throwing away the rest of the stream.
@@ -238,16 +229,7 @@ headerLoop:
 	}
 
 	// Discard the rest of the commit
-	discard := size - n + 1
+	return id, DiscardFull(rd, size-n+1)
-	for discard > math.MaxInt32 {
-		_, err := rd.Discard(math.MaxInt32)
-		if err != nil {
-			return id, err
-		}
-		discard -= math.MaxInt32
-	}
-	_, err := rd.Discard(int(discard))
-	return id, err
 }
 
 // git tree files are a list:
@@ -345,3 +327,21 @@ func init() {
 	_, filename, _, _ := runtime.Caller(0)
 	callerPrefix = strings.TrimSuffix(filename, "modules/git/batch_reader.go")
 }
+
+func DiscardFull(rd *bufio.Reader, discard int64) error {
+	if discard > math.MaxInt32 {
+		n, err := rd.Discard(math.MaxInt32)
+		discard -= int64(n)
+		if err != nil {
+			return err
+		}
+	}
+	for discard > 0 {
+		n, err := rd.Discard(int(discard))
+		discard -= int64(n)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}

modules/git/blob_nogogit.go

@@ -9,7 +9,6 @@ import (
 	"bufio"
 	"bytes"
 	"io"
-	"math"
 
 	"code.gitea.io/gitea/modules/log"
 )
@@ -104,25 +103,6 @@ func (b *blobReader) Read(p []byte) (n int, err error) {
 // Close implements io.Closer
 func (b *blobReader) Close() error {
 	defer b.cancel()
-	if b.n > 0 {
+
-		for b.n > math.MaxInt32 {
+	return DiscardFull(b.rd, b.n+1)
-			n, err := b.rd.Discard(math.MaxInt32)
-			b.n -= int64(n)
-			if err != nil {
-				return err
-			}
-			b.n -= math.MaxInt32
-		}
-		n, err := b.rd.Discard(int(b.n))
-		b.n -= int64(n)
-		if err != nil {
-			return err
-		}
-	}
-	if b.n == 0 {
-		_, err := b.rd.Discard(1)
-		b.n--
-		return err
-	}
-	return nil
 }

modules/git/commit_info_nogogit.go

@@ -151,6 +151,9 @@ func GetLastCommitForPaths(ctx context.Context, commit *Commit, treePath string,
 			return nil, err
 		}
 		if typ != "commit" {
+			if err := DiscardFull(batchReader, size+1); err != nil {
+				return nil, err
+			}
 			return nil, fmt.Errorf("unexpected type: %s for commit id: %s", typ, commitID)
 		}
 		c, err = CommitFromReader(commit.repo, MustIDFromString(commitID), io.LimitReader(batchReader, size))

modules/git/pipeline/lfs_nogogit.go

@@ -170,6 +170,10 @@ func FindLFSFile(repo *git.Repository, hash git.SHA1) ([]*LFSResult, error) {
 				} else {
 					break commitReadingLoop
 				}
+			default:
+				if err := git.DiscardFull(batchReader, size+1); err != nil {
+					return nil, err
+				}
 			}
 		}
 	}

modules/git/repo_commit_nogogit.go

@@ -121,8 +121,7 @@ func (repo *Repository) getCommitFromBatchReader(rd *bufio.Reader, id SHA1) (*Co
 		return commit, nil
 	default:
 		log.Debug("Unknown typ: %s", typ)
-		_, err = rd.Discard(int(size) + 1)
+		if err := DiscardFull(rd, size+1); err != nil {
-		if err != nil {
 			return nil, err
 		}
 		return nil, ErrNotExist{

modules/git/repo_language_stats_nogogit.go

@@ -6,10 +6,8 @@
 package git
 
 import (
-	"bufio"
 	"bytes"
 	"io"
-	"math"
 	"strings"
 
 	"code.gitea.io/gitea/modules/analyze"
@@ -168,8 +166,7 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
 				return nil, err
 			}
 			content = contentBuf.Bytes()
-			err = discardFull(batchReader, discard)
+			if err := DiscardFull(batchReader, discard); err != nil {
-			if err != nil {
 				return nil, err
 			}
 		}
@@ -212,21 +209,3 @@ func (repo *Repository) GetLanguageStats(commitID string) (map[string]int64, err
 
 	return mergeLanguageStats(sizes), nil
 }
-
-func discardFull(rd *bufio.Reader, discard int64) error {
-	if discard > math.MaxInt32 {
-		n, err := rd.Discard(math.MaxInt32)
-		discard -= int64(n)
-		if err != nil {
-			return err
-		}
-	}
-	for discard > 0 {
-		n, err := rd.Discard(int(discard))
-		discard -= int64(n)
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}

modules/git/repo_tag_nogogit.go

@@ -103,6 +103,9 @@ func (repo *Repository) getTag(tagID SHA1, name string) (*Tag, error) {
 		return nil, err
 	}
 	if typ != "tag" {
+		if err := DiscardFull(rd, size+1); err != nil {
+			return nil, err
+		}
 		return nil, ErrNotExist{ID: tagID.String()}
 	}
 

modules/git/repo_tree_nogogit.go

@@ -58,6 +58,9 @@ func (repo *Repository) getTree(id SHA1) (*Tree, error) {
 		tree.entriesParsed = true
 		return tree, nil
 	default:
+		if err := DiscardFull(rd, size+1); err != nil {
+			return nil, err
+		}
 		return nil, ErrNotExist{
 			ID: id.String(),
 		}

modules/git/tree_nogogit.go

@@ -7,7 +7,6 @@ package git
 
 import (
 	"io"
-	"math"
 	"strings"
 )
 
@@ -63,21 +62,10 @@ func (t *Tree) ListEntries() (Entries, error) {
 		}
 
 		// Not a tree just use ls-tree instead
-		for sz > math.MaxInt32 {
+		if err := DiscardFull(rd, sz+1); err != nil {
-			discarded, err := rd.Discard(math.MaxInt32)
-			sz -= int64(discarded)
-			if err != nil {
 			return nil, err
 		}
 	}
-		for sz > 0 {
-			discarded, err := rd.Discard(int(sz))
-			sz -= int64(discarded)
-			if err != nil {
-				return nil, err
-			}
-		}
-	}
 
 	stdout, _, runErr := NewCommand(t.repo.Ctx, "ls-tree", "-l").AddDynamicArguments(t.ID.String()).RunStdBytes(&RunOpts{Dir: t.repo.Path})
 	if runErr != nil {

modules/git/tree_test.go

@@ -0,0 +1,27 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package git
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSubTree_Issue29101(t *testing.T) {
+	repo, err := openRepositoryWithDefaultContext(filepath.Join(testReposDir, "repo1_bare"))
+	assert.NoError(t, err)
+	defer repo.Close()
+
+	commit, err := repo.GetCommit("ce064814f4a0d337b333e646ece456cd39fab612")
+	assert.NoError(t, err)
+
+	// old code could produce a different error if called multiple times
+	for i := 0; i < 10; i++ {
+		_, err = commit.SubTree("file1.txt")
+		assert.Error(t, err)
+		assert.True(t, IsErrNotExist(err))
+	}
+}