From ed5e0c8c27a64541ed7d016c50b44109ca4f9e4b Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 22 Feb 2024 12:23:38 +0800 Subject: [PATCH] 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 Co-authored-by: yp05327 <576951401@qq.com> --- modules/git/batch_reader.go | 40 +++++++++++----------- modules/git/blob_nogogit.go | 24 ++----------- modules/git/commit_info_nogogit.go | 3 ++ modules/git/pipeline/lfs_nogogit.go | 4 +++ modules/git/repo_commit_nogogit.go | 3 +- modules/git/repo_language_stats_nogogit.go | 23 +------------ modules/git/repo_tag_nogogit.go | 3 ++ modules/git/repo_tree_nogogit.go | 3 ++ modules/git/tree_nogogit.go | 16 ++------- modules/git/tree_test.go | 27 +++++++++++++++ 10 files changed, 66 insertions(+), 80 deletions(-) create mode 100644 modules/git/tree_test.go diff --git a/modules/git/batch_reader.go b/modules/git/batch_reader.go index 7a44e6295c4e..d8f4886d0625 100644 --- a/modules/git/batch_reader.go +++ b/modules/git/batch_reader.go @@ -203,16 +203,7 @@ headerLoop: } // Discard the rest of the tag - discard := 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 + return id, DiscardFull(rd, size-n+1) } // 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 - 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 + return id, DiscardFull(rd, size-n+1) } // 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 +} diff --git a/modules/git/blob_nogogit.go b/modules/git/blob_nogogit.go index 511332eb5064..6a6edaf5b667 100644 --- a/modules/git/blob_nogogit.go +++ b/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 { - 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 + + return DiscardFull(b.rd, b.n+1) } diff --git a/modules/git/commit_info_nogogit.go b/modules/git/commit_info_nogogit.go index e469d2cab637..a5d18694f738 100644 --- a/modules/git/commit_info_nogogit.go +++ b/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)) diff --git a/modules/git/pipeline/lfs_nogogit.go b/modules/git/pipeline/lfs_nogogit.go index 49390f7c00c6..2ec6031540a8 100644 --- a/modules/git/pipeline/lfs_nogogit.go +++ b/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 + } } } } diff --git a/modules/git/repo_commit_nogogit.go b/modules/git/repo_commit_nogogit.go index d5eb723100a7..73e441b7d9ec 100644 --- a/modules/git/repo_commit_nogogit.go +++ b/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 != nil { + if err := DiscardFull(rd, size+1); err != nil { return nil, err } return nil, ErrNotExist{ diff --git a/modules/git/repo_language_stats_nogogit.go b/modules/git/repo_language_stats_nogogit.go index 1d94ad6c00f4..d68d7d210a38 100644 --- a/modules/git/repo_language_stats_nogogit.go +++ b/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 != nil { + if err := DiscardFull(batchReader, discard); 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 -} diff --git a/modules/git/repo_tag_nogogit.go b/modules/git/repo_tag_nogogit.go index 9080ffcfd782..19023741e30d 100644 --- a/modules/git/repo_tag_nogogit.go +++ b/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()} } diff --git a/modules/git/repo_tree_nogogit.go b/modules/git/repo_tree_nogogit.go index 4fd77df2b824..3ec6fcae9208 100644 --- a/modules/git/repo_tree_nogogit.go +++ b/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(), } diff --git a/modules/git/tree_nogogit.go b/modules/git/tree_nogogit.go index ef598d7e9132..a1b0acdafadd 100644 --- a/modules/git/tree_nogogit.go +++ b/modules/git/tree_nogogit.go @@ -7,7 +7,6 @@ package git import ( "io" - "math" "strings" ) @@ -63,19 +62,8 @@ func (t *Tree) ListEntries() (Entries, error) { } // Not a tree just use ls-tree instead - for sz > math.MaxInt32 { - 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 - } + if err := DiscardFull(rd, sz+1); err != nil { + return nil, err } } diff --git a/modules/git/tree_test.go b/modules/git/tree_test.go new file mode 100644 index 000000000000..6d2b5c84d506 --- /dev/null +++ b/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)) + } +}