finn/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Prevent double use of git cat-file session. (#29298)

Browse source 
Fixes the reason why #29101 is hard to replicate.
Related #29297

Create a repo with a file with minimum size 4097 bytes (I use 10000) and
execute the following code:
```go
gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, <repo>)
assert.NoError(t, err)

commit, err := gitRepo.GetCommit(<sha>)
assert.NoError(t, err)

entry, err := commit.GetTreeEntryByPath(<file>)
assert.NoError(t, err)

b := entry.Blob()

// Create a reader
r, err := b.DataAsync()
assert.NoError(t, err)
defer r.Close()

// Create a second reader
r2, err := b.DataAsync()
assert.NoError(t, err) // Should be no error but is ErrNotExist
defer r2.Close()
```

The problem is the check in `CatFileBatch`:

79217ea63c/modules/git/repo_base_nogogit.go (L81-L87)
`Buffered() > 0` is used to check if there is a "operation" in progress
at the moment. This is a problem because we can't control the internal
buffer in the `bufio.Reader`. The code above demonstrates a sequence
which initiates an operation for which the code thinks there is no
active processing. The second call to `DataAsync()` therefore reuses the
existing instances instead of creating a new batch reader.

(cherry picked from commit f74c869221624092999097af38b6f7fae4701420)
This commit is contained in:
KN4CK3R 2024-02-21 19:54:17 +01:00 • committed by Earl Warren
parent c88ae2e382
commit 5f79550a0d
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
2 changed files with 59 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

21
modules/git/repo_base_nogogit.go
View file

@ -27,10 +27,12 @@ type Repository struct {
gpgSettings *GPGSettings
batchInUse bool
batchCancel context.CancelFunc
batchReader *bufio.Reader
batchWriter WriteCloserError
checkInUse bool
checkCancel context.CancelFunc
checkReader *bufio.Reader
checkWriter WriteCloserError
@ -79,23 +81,28 @@ func OpenRepository(ctx context.Context, repoPath string) (*Repository, error) {
// CatFileBatch obtains a CatFileBatch for this repository
func (repo *Repository) CatFileBatch(ctx context.Context) (WriteCloserError, *bufio.Reader, func()) {
if repo.batchCancel == nil || repo.batchReader.Buffered() > 0 {
if repo.batchCancel == nil || repo.batchInUse {
log.Debug("Opening temporary cat file batch for: %s", repo.Path)
return CatFileBatch(ctx, repo.Path)
}
return repo.batchWriter, repo.batchReader, func() {}
repo.batchInUse = true
return repo.batchWriter, repo.batchReader, func() {
repo.batchInUse = false
}
}
// CatFileBatchCheck obtains a CatFileBatchCheck for this repository
func (repo *Repository) CatFileBatchCheck(ctx context.Context) (WriteCloserError, *bufio.Reader, func()) {
if repo.checkCancel == nil || repo.checkReader.Buffered() > 0 {
log.Debug("Opening temporary cat file batch-check: %s", repo.Path)
if repo.checkCancel == nil || repo.checkInUse {
log.Debug("Opening temporary cat file batch-check for: %s", repo.Path)
return CatFileBatchCheck(ctx, repo.Path)
}
return repo.checkWriter, repo.checkReader, func() {}
repo.checkInUse = true
return repo.checkWriter, repo.checkReader, func() {
repo.checkInUse = false
}
}
// Close this repository, in particular close the underlying gogitStorage if this is not nil
func (repo *Repository) Close() (err error) {
if repo == nil {
return nil
@ -105,12 +112,14 @@ func (repo *Repository) Close() (err error) {
repo.batchReader = nil
repo.batchWriter = nil
repo.batchCancel = nil
repo.batchInUse = false
}
if repo.checkCancel != nil {
repo.checkCancel()
repo.checkCancel = nil
repo.checkReader = nil
repo.checkWriter = nil
repo.checkInUse = false
}
repo.LastCommitCache = nil
repo.tagCache = nil