Skip to content

Commit b07f390

Browse files
committed
Prevent hang in git cat-file if the repository is not a valid repository (Partial go-gitea#17991)
Unfortunately it appears that if git cat-file is run in an invalid repository it will hang until stdin is closed. This will result in deadlocked /pulls pages and dangling git cat-file calls if a broken repository is tried to be reviewed or pulls exists for a broken repository. Signed-off-by: Andrew Thornton <art27@cantab.net>
1 parent fc8c23e commit b07f390

File tree

6 files changed

+33
-2
lines changed

6 files changed

+33
-2
lines changed

modules/git/batch_reader.go

+11
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,17 @@ type WriteCloserError interface {
2727
CloseWithError(err error) error
2828
}
2929

30+
func EnsureValidGitRepository(ctx context.Context, repoPath string) error {
31+
stderr := strings.Builder{}
32+
err := NewCommandContext(ctx, "rev-parse").
33+
SetDescription(fmt.Sprintf("%s rev-parse [repo_path: %s]", GitExecutable, repoPath)).
34+
RunInDirFullPipeline(repoPath, nil, &stderr, nil)
35+
if err != nil {
36+
return ConcatenateError(err, (&stderr).String())
37+
}
38+
return nil
39+
}
40+
3041
// CatFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function
3142
func CatFileBatchCheck(repoPath string) (WriteCloserError, *bufio.Reader, func()) {
3243
batchStdinReader, batchStdinWriter := io.Pipe()

modules/git/repo_base_nogogit.go

+5
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ func OpenRepository(repoPath string) (*Repository, error) {
4343
return nil, errors.New("no such file or directory")
4444
}
4545

46+
// Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first!
47+
if err := EnsureValidGitRepository(ctx, repoPath); err != nil {
48+
return nil, err
49+
}
50+
4651
repo := &Repository{
4752
Path: repoPath,
4853
tagCache: newObjectCache(),

modules/git/repo_commit_nogogit.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ func (repo *Repository) ResolveReference(name string) (string, error) {
3838
func (repo *Repository) GetRefCommitID(name string) (string, error) {
3939
wr, rd, cancel := repo.CatFileBatchCheck()
4040
defer cancel()
41-
_, _ = wr.Write([]byte(name + "\n"))
41+
_, err := wr.Write([]byte(name + "\n"))
42+
if err != nil {
43+
return "", err
44+
}
4245
shaBs, _, _, err := ReadBatchLine(rd)
4346
if IsErrNotExist(err) {
4447
return "", ErrNotExist{name, ""}

modules/indexer/code/bleve.go

+6
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,12 @@ func (b *BleveIndexer) Index(repo *models.Repository, sha string, changes *repoC
276276
batch := gitea_bleve.NewFlushingBatch(b.indexer, maxBatchSize)
277277
if len(changes.Updates) > 0 {
278278

279+
// Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first!
280+
if err := git.EnsureValidGitRepository(git.DefaultContext, repo.RepoPath()); err != nil {
281+
log.Error("Unable to open git repo: %s for %-v: %v", repo.RepoPath(), repo, err)
282+
return err
283+
}
284+
279285
batchWriter, batchReader, cancel := git.CatFileBatch(repo.RepoPath())
280286
defer cancel()
281287

modules/indexer/code/elastic_search.go

+5
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,11 @@ func (b *ElasticSearchIndexer) addDelete(filename string, repo *models.Repositor
248248
func (b *ElasticSearchIndexer) Index(repo *models.Repository, sha string, changes *repoChanges) error {
249249
reqs := make([]elastic.BulkableRequest, 0)
250250
if len(changes.Updates) > 0 {
251+
// Now because of some insanity with git cat-file not immediately failing if not run in a valid git directory we need to run git rev-parse first!
252+
if err := git.EnsureValidGitRepository(git.DefaultContext, repo.RepoPath()); err != nil {
253+
log.Error("Unable to open git repo: %s for %-v: %v", repo.RepoPath(), repo, err)
254+
return err
255+
}
251256

252257
batchWriter, batchReader, cancel := git.CatFileBatch(repo.RepoPath())
253258
defer cancel()

services/pull/pull.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,8 @@ func GetIssuesLastCommitStatus(issues models.IssueList) (map[int64]*models.Commi
713713
if !ok {
714714
gitRepo, err = git.OpenRepository(issue.Repo.RepoPath())
715715
if err != nil {
716-
return nil, err
716+
log.Error("Cannot open git repository %-v for issue #%d[%d]. Error: %v", issue.Repo, issue.Index, issue.ID, err)
717+
continue
717718
}
718719
gitRepos[issue.RepoID] = gitRepo
719720
}

0 commit comments

Comments
 (0)