femsci/forgejo
femsci/forgejo
1
0
Fork 0
forked from NYANDEV/forgejo
Code Pull requests Activity

Prevent hang in git cat-file if the repository is not a valid repository (Partial #17991) (#17992)

Browse source 
* Prevent hang in git cat-file if the repository is not a valid repository (Partial #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>

* placate lint

Signed-off-by: Andrew Thornton <art27@cantab.net>

* fix compilation bug

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Add the missing directories to the testrepos

* fixup! Add the missing directories to the testrepos

* and ensure that all of the other places have the objects directories too

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
This commit is contained in:
zeripath 2021-12-17 02:08:00 +00:00 committed by GitHub
parent 3a77465e4e
commit 3ae4c4898b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 150 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
integrations/integration_test.go
View file

@ -251,6 +251,26 @@ func prepareTestEnv(t testing.TB, skip ...int) func() {
assert.NoError(t, util.RemoveAll(setting.RepoRootPath))
assert.NoError(t, util.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath))
ownerDirs, err := os.ReadDir(setting.RepoRootPath)
if err != nil {
assert.NoError(t, err, "unable to read the new repo root: %v\n", err)
}
for _, ownerDir := range ownerDirs {
if !ownerDir.Type().IsDir() {
continue
}
repoDirs, err := os.ReadDir(filepath.Join(setting.RepoRootPath, ownerDir.Name()))
if err != nil {
assert.NoError(t, err, "unable to read the new repo root: %v\n", err)
}
for _, repoDir := range repoDirs {
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "pack"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "info"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "heads"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "tag"), 0755)
}
}
return deferFn
}
@ -529,4 +549,23 @@ func resetFixtures(t *testing.T) {
assert.NoError(t, models.LoadFixtures())
assert.NoError(t, util.RemoveAll(setting.RepoRootPath))
assert.NoError(t, util.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath))
ownerDirs, err := os.ReadDir(setting.RepoRootPath)
if err != nil {
assert.NoError(t, err, "unable to read the new repo root: %v\n", err)
}
for _, ownerDir := range ownerDirs {
if !ownerDir.Type().IsDir() {
continue
}
repoDirs, err := os.ReadDir(filepath.Join(setting.RepoRootPath, ownerDir.Name()))
if err != nil {
assert.NoError(t, err, "unable to read the new repo root: %v\n", err)
}
for _, repoDir := range repoDirs {
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "pack"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "info"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "heads"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "tag"), 0755)
}
}
}

19
integrations/migration-test/migration_test.go
View file

@ -61,6 +61,25 @@ func initMigrationTest(t *testing.T) func() {
assert.True(t, len(setting.RepoRootPath) != 0)
assert.NoError(t, util.RemoveAll(setting.RepoRootPath))
assert.NoError(t, util.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath))
ownerDirs, err := os.ReadDir(setting.RepoRootPath)
if err != nil {
assert.NoError(t, err, "unable to read the new repo root: %v\n", err)
}
for _, ownerDir := range ownerDirs {
if !ownerDir.Type().IsDir() {
continue
}
repoDirs, err := os.ReadDir(filepath.Join(setting.RepoRootPath, ownerDir.Name()))
if err != nil {
assert.NoError(t, err, "unable to read the new repo root: %v\n", err)
}
for _, repoDir := range repoDirs {
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "pack"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "info"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "heads"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "tag"), 0755)
}
}
git.CheckLFSVersion()
setting.InitDBConfig()

19
models/migrations/migrations_test.go
View file

@ -205,6 +205,25 @@ func prepareTestEnv(t *testing.T, skip int, syncModels ...interface{}) (*xorm.En
assert.NoError(t, com.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"),
setting.RepoRootPath))
ownerDirs, err := os.ReadDir(setting.RepoRootPath)
if err != nil {
assert.NoError(t, err, "unable to read the new repo root: %v\n", err)
}
for _, ownerDir := range ownerDirs {
if !ownerDir.Type().IsDir() {
continue
}
repoDirs, err := os.ReadDir(filepath.Join(setting.RepoRootPath, ownerDir.Name()))
if err != nil {
assert.NoError(t, err, "unable to read the new repo root: %v\n", err)
}
for _, repoDir := range repoDirs {
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "pack"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "info"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "heads"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "tag"), 0755)
}
}
if err := deleteDB(); err != nil {
t.Errorf("unable to reset database: %v", err)

37
models/unit_tests.go
View file

@ -87,6 +87,26 @@ func MainTest(m *testing.M, pathToGiteaRoot string) {
fatalTestError("util.CopyDir: %v\n", err)
}
ownerDirs, err := os.ReadDir(setting.RepoRootPath)
if err != nil {
fatalTestError("unable to read the new repo root: %v\n", err)
}
for _, ownerDir := range ownerDirs {
if !ownerDir.Type().IsDir() {
continue
}
repoDirs, err := os.ReadDir(filepath.Join(setting.RepoRootPath, ownerDir.Name()))
if err != nil {
fatalTestError("unable to read the new repo root: %v\n", err)
}
for _, repoDir := range repoDirs {
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "pack"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "info"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "heads"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "tag"), 0755)
}
}
exitStatus := m.Run()
if err = util.RemoveAll(setting.RepoRootPath); err != nil {
fatalTestError("util.RemoveAll: %v\n", err)
@ -128,6 +148,23 @@ func PrepareTestEnv(t testing.TB) {
assert.NoError(t, util.RemoveAll(setting.RepoRootPath))
metaPath := filepath.Join(giteaRoot, "integrations", "gitea-repositories-meta")
assert.NoError(t, util.CopyDir(metaPath, setting.RepoRootPath))
ownerDirs, err := os.ReadDir(setting.RepoRootPath)
assert.NoError(t, err)
for _, ownerDir := range ownerDirs {
if !ownerDir.Type().IsDir() {
continue
}
repoDirs, err := os.ReadDir(filepath.Join(setting.RepoRootPath, ownerDir.Name()))
assert.NoError(t, err)
for _, repoDir := range repoDirs {
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "pack"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "objects", "info"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "heads"), 0755)
_ = os.MkdirAll(filepath.Join(setting.RepoRootPath, ownerDir.Name(), repoDir.Name(), "refs", "tag"), 0755)
}
}
base.SetupGiteaRoot() // Makes sure GITEA_ROOT is set
}

14
modules/git/batch_reader.go
View file

@ -27,6 +27,20 @@ type WriteCloserError interface {
CloseWithError(err error) error
}
// EnsureValidGitRepository runs git rev-parse in the repository path - thus ensuring that the repository is a valid repository.
// Run before opening git cat-file.
// This is needed otherwise the git cat-file will hang for invalid repositories.
func EnsureValidGitRepository(ctx context.Context, repoPath string) error {
stderr := strings.Builder{}
err := NewCommandContext(ctx, "rev-parse").
SetDescription(fmt.Sprintf("%s rev-parse [repo_path: %s]", GitExecutable, repoPath)).
RunInDirFullPipeline(repoPath, nil, &stderr, nil)
if err != nil {
return ConcatenateError(err, (&stderr).String())
}
return nil
}
// CatFileBatchCheck opens git cat-file --batch-check in the provided repo and returns a stdin pipe, a stdout reader and cancel function
func CatFileBatchCheck(repoPath string) (WriteCloserError, *bufio.Reader, func()) {
batchStdinReader, batchStdinWriter := io.Pipe()

5
modules/git/repo_base_nogogit.go
View file

@ -43,6 +43,11 @@ func OpenRepository(repoPath string) (*Repository, error) {
return nil, errors.New("no such file or directory")
}
// 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!
if err := EnsureValidGitRepository(DefaultContext, repoPath); err != nil {
return nil, err
}
repo := &Repository{
Path: repoPath,
tagCache: newObjectCache(),

5
modules/git/repo_commit_nogogit.go
View file

@ -38,7 +38,10 @@ func (repo *Repository) ResolveReference(name string) (string, error) {
func (repo *Repository) GetRefCommitID(name string) (string, error) {
wr, rd, cancel := repo.CatFileBatchCheck()
defer cancel()
_, _ = wr.Write([]byte(name + "\n"))
_, err := wr.Write([]byte(name + "\n"))
if err != nil {
return "", err
}
shaBs, _, _, err := ReadBatchLine(rd)
if IsErrNotExist(err) {
return "", ErrNotExist{name, ""}

6
modules/indexer/code/bleve.go
View file

@ -276,6 +276,12 @@ func (b *BleveIndexer) Index(repo *models.Repository, sha string, changes *repoC
batch := gitea_bleve.NewFlushingBatch(b.indexer, maxBatchSize)
if len(changes.Updates) > 0 {
// 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!
if err := git.EnsureValidGitRepository(git.DefaultContext, repo.RepoPath()); err != nil {
log.Error("Unable to open git repo: %s for %-v: %v", repo.RepoPath(), repo, err)
return err
}
batchWriter, batchReader, cancel := git.CatFileBatch(repo.RepoPath())
defer cancel()

5
modules/indexer/code/elastic_search.go
View file

@ -248,6 +248,11 @@ func (b *ElasticSearchIndexer) addDelete(filename string, repo *models.Repositor
func (b *ElasticSearchIndexer) Index(repo *models.Repository, sha string, changes *repoChanges) error {
reqs := make([]elastic.BulkableRequest, 0)
if len(changes.Updates) > 0 {
// 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!
if err := git.EnsureValidGitRepository(git.DefaultContext, repo.RepoPath()); err != nil {
log.Error("Unable to open git repo: %s for %-v: %v", repo.RepoPath(), repo, err)
return err
}
batchWriter, batchReader, cancel := git.CatFileBatch(repo.RepoPath())
defer cancel()

3
services/pull/pull.go
View file

@ -713,7 +713,8 @@ func GetIssuesLastCommitStatus(issues models.IssueList) (map[int64]*models.Commi
if !ok {
gitRepo, err = git.OpenRepository(issue.Repo.RepoPath())
if err != nil {
return nil, err
log.Error("Cannot open git repository %-v for issue #%d[%d]. Error: %v", issue.Repo, issue.Index, issue.ID, err)
continue
}
gitRepos[issue.RepoID] = gitRepo
}