RepoAssignment ensure to close before overwrite (#19449)

* check if GitRepo already open and close if

* only run RepoAssignment once

* refactor context helper for api to open GitRepo

2 files changed, 61 insertions, 45 deletions

diff --git a/modules/context/api.go b/modules/context/api.go
index e5c2eeda0a..41d559f5b1 100644
--- a/modules/context/api.go
+++ b/modules/context/api.go
@@ -285,36 +285,6 @@ func APIContexter() func(http.Handler) http.Handler {
 	}
 }
 
-// ReferencesGitRepo injects the GitRepo into the Context
-func ReferencesGitRepo(allowEmpty bool) func(ctx *APIContext) (cancel context.CancelFunc) {
-	return func(ctx *APIContext) (cancel context.CancelFunc) {
-		// Empty repository does not have reference information.
-		if !allowEmpty && ctx.Repo.Repository.IsEmpty {
-			return
-		}
-
-		// For API calls.
-		if ctx.Repo.GitRepo == nil {
-			repoPath := repo_model.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
-			gitRepo, err := git.OpenRepository(ctx, repoPath)
-			if err != nil {
-				ctx.Error(http.StatusInternalServerError, "RepoRef Invalid repo "+repoPath, err)
-				return
-			}
-			ctx.Repo.GitRepo = gitRepo
-			// We opened it, we should close it
-			return func() {
-				// If it's been set to nil then assume someone else has closed it.
-				if ctx.Repo.GitRepo != nil {
-					ctx.Repo.GitRepo.Close()
-				}
-			}
-		}
-
-		return
-	}
-}
-
 // NotFound handles 404s for APIContext
 // String will replace message, errors will be added to a slice
 func (ctx *APIContext) NotFound(objs ...interface{}) {
@@ -340,33 +310,62 @@ func (ctx *APIContext) NotFound(objs ...interface{}) {
 	})
 }
 
-// RepoRefForAPI handles repository reference names when the ref name is not explicitly given
-func RepoRefForAPI(next http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		ctx := GetAPIContext(req)
+// ReferencesGitRepo injects the GitRepo into the Context
+// you can optional skip the IsEmpty check
+func ReferencesGitRepo(allowEmpty ...bool) func(ctx *APIContext) (cancel context.CancelFunc) {
+	return func(ctx *APIContext) (cancel context.CancelFunc) {
 		// Empty repository does not have reference information.
-		if ctx.Repo.Repository.IsEmpty {
+		if ctx.Repo.Repository.IsEmpty && !(len(allowEmpty) != 0 && allowEmpty[0]) {
 			return
 		}
 
-		var err error
-
+		// For API calls.
 		if ctx.Repo.GitRepo == nil {
 			repoPath := repo_model.RepoPath(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
-			ctx.Repo.GitRepo, err = git.OpenRepository(ctx, repoPath)
+			gitRepo, err := git.OpenRepository(ctx, repoPath)
 			if err != nil {
-				ctx.InternalServerError(err)
+				ctx.Error(http.StatusInternalServerError, "RepoRef Invalid repo "+repoPath, err)
 				return
 			}
+			ctx.Repo.GitRepo = gitRepo
 			// We opened it, we should close it
-			defer func() {
+			return func() {
 				// If it's been set to nil then assume someone else has closed it.
 				if ctx.Repo.GitRepo != nil {
 					ctx.Repo.GitRepo.Close()
 				}
-			}()
+			}
+		}
+
+		return
+	}
+}
+
+// RepoRefForAPI handles repository reference names when the ref name is not explicitly given
+func RepoRefForAPI(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		ctx := GetAPIContext(req)
+
+		if ctx.Repo.GitRepo == nil {
+			ctx.InternalServerError(fmt.Errorf("no open git repo"))
+			return
+		}
+
+		if ref := ctx.FormTrim("ref"); len(ref) > 0 {
+			commit, err := ctx.Repo.GitRepo.GetCommit(ref)
+			if err != nil {
+				if git.IsErrNotExist(err) {
+					ctx.NotFound()
+				} else {
+					ctx.Error(http.StatusInternalServerError, "GetBlobByPath", err)
+				}
+				return
+			}
+			ctx.Repo.Commit = commit
+			return
 		}
 
+		var err error
 		refName := getRefName(ctx.Context, RepoRefAny)
 
 		if ctx.Repo.GitRepo.IsBranchExist(refName) {
diff --git a/modules/context/repo.go b/modules/context/repo.go
index a7c9a982c4..4687434455 100644
--- a/modules/context/repo.go
+++ b/modules/context/repo.go
@@ -221,13 +221,21 @@ func (r *Repository) FileExists(path, branch string) (bool, error) {
 
 // GetEditorconfig returns the .editorconfig definition if found in the
 // HEAD of the default repo branch.
-func (r *Repository) GetEditorconfig() (*editorconfig.Editorconfig, error) {
+func (r *Repository) GetEditorconfig(optCommit ...*git.Commit) (*editorconfig.Editorconfig, error) {
 	if r.GitRepo == nil {
 		return nil, nil
 	}
-	commit, err := r.GitRepo.GetBranchCommit(r.Repository.DefaultBranch)
-	if err != nil {
-		return nil, err
+	var (
+		err    error
+		commit *git.Commit
+	)
+	if len(optCommit) != 0 {
+		commit = optCommit[0]
+	} else {
+		commit, err = r.GitRepo.GetBranchCommit(r.Repository.DefaultBranch)
+		if err != nil {
+			return nil, err
+		}
 	}
 	treeEntry, err := commit.GetTreeEntryByPath(".editorconfig")
 	if err != nil {
@@ -407,6 +415,12 @@ func RepoIDAssignment() func(ctx *Context) {
 
 // RepoAssignment returns a middleware to handle repository assignment
 func RepoAssignment(ctx *Context) (cancel context.CancelFunc) {
+	if _, repoAssignmentOnce := ctx.Data["repoAssignmentExecuted"]; repoAssignmentOnce {
+		log.Trace("RepoAssignment was exec already, skipping second call ...")
+		return
+	}
+	ctx.Data["repoAssignmentExecuted"] = true
+
 	var (
 		owner *user_model.User
 		err   error
@@ -602,6 +616,9 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) {
 		ctx.ServerError("RepoAssignment Invalid repo "+repo_model.RepoPath(userName, repoName), err)
 		return
 	}
+	if ctx.Repo.GitRepo != nil {
+		ctx.Repo.GitRepo.Close()
+	}
 	ctx.Repo.GitRepo = gitRepo
 
 	// We opened it, we should close it