From 338af89d5641008be579df9c554d0a1db190a009 Mon Sep 17 00:00:00 2001 From: Unknwon Date: Wed, 17 Feb 2016 22:47:06 -0500 Subject: #2650 fix possbility that use email as pusher user name Remove the possibility of using email as user name when user actually push through combination of email and password with HTTP. Also refactor update action function to replcae tons of arguments with single PushUpdateOptions struct. And define the user who pushes code as pusher, therefore variable names shouldn't be confusing any more. --- models/update.go | 90 +++++++++++++++++++++++++++++++------------------------- 1 file changed, 50 insertions(+), 40 deletions(-) (limited to 'models') diff --git a/models/update.go b/models/update.go index 5c6ea7f6..66775ea9 100644 --- a/models/update.go +++ b/models/update.go @@ -10,7 +10,7 @@ import ( "os/exec" "strings" - "github.com/gogits/git-module" + git "github.com/gogits/git-module" "github.com/gogits/gogs/modules/log" ) @@ -65,94 +65,104 @@ func ListToPushCommits(l *list.List) *PushCommits { return &PushCommits{l.Len(), commits, "", nil} } -func Update(refName, oldCommitID, newCommitID, userName, repoUserName, repoName string, userID int64) error { - isNew := strings.HasPrefix(oldCommitID, "0000000") - if isNew && - strings.HasPrefix(newCommitID, "0000000") { - return fmt.Errorf("old rev and new rev both 000000") +type PushUpdateOptions struct { + RefName string + OldCommitID string + NewCommitID string + PusherID int64 + PusherName string + RepoUserName string + RepoName string +} + +// PushUpdate must be called for any push actions in order to +// generates necessary push action history feeds. +func PushUpdate(opts PushUpdateOptions) (err error) { + isNewRef := strings.HasPrefix(opts.OldCommitID, "0000000") + isDelRef := strings.HasPrefix(opts.NewCommitID, "0000000") + if isNewRef && isDelRef { + return fmt.Errorf("Old and new revisions both start with 000000") } - f := RepoPath(repoUserName, repoName) + repoPath := RepoPath(opts.RepoUserName, opts.RepoName) gitUpdate := exec.Command("git", "update-server-info") - gitUpdate.Dir = f - gitUpdate.Run() + gitUpdate.Dir = repoPath + if err = gitUpdate.Run(); err != nil { + return fmt.Errorf("Fail to call 'git update-server-info': %v", err) + } - isDel := strings.HasPrefix(newCommitID, "0000000") - if isDel { - log.GitLogger.Info("del rev", refName, "from", userName+"/"+repoName+".git", "by", userID) + if isDelRef { + log.GitLogger.Info("Reference '%s' has been deleted from '%s/%s' by %d", + opts.RefName, opts.RepoUserName, opts.RepoName, opts.PusherName) return nil } - gitRepo, err := git.OpenRepository(f) + gitRepo, err := git.OpenRepository(repoPath) if err != nil { - return fmt.Errorf("runUpdate.Open repoId: %v", err) + return fmt.Errorf("OpenRepository: %v", err) } - user, err := GetUserByName(repoUserName) + repoUser, err := GetUserByName(opts.RepoUserName) if err != nil { - return fmt.Errorf("runUpdate.GetUserByName: %v", err) + return fmt.Errorf("GetUserByName: %v", err) } - repo, err := GetRepositoryByName(user.Id, repoName) + repo, err := GetRepositoryByName(repoUser.Id, opts.RepoName) if err != nil { - return fmt.Errorf("runUpdate.GetRepositoryByName userId: %v", err) + return fmt.Errorf("GetRepositoryByName: %v", err) } // Push tags. - if strings.HasPrefix(refName, "refs/tags/") { - tagName := git.RefEndName(refName) - tag, err := gitRepo.GetTag(tagName) + if strings.HasPrefix(opts.RefName, "refs/tags/") { + tag, err := gitRepo.GetTag(git.RefEndName(opts.RefName)) if err != nil { - log.GitLogger.Fatal(4, "runUpdate.GetTag: %v", err) + return fmt.Errorf("gitRepo.GetTag: %v", err) } + // When tagger isn't available, fall back to get committer email. var actEmail string if tag.Tagger != nil { actEmail = tag.Tagger.Email } else { cmt, err := tag.Commit() if err != nil { - log.GitLogger.Fatal(4, "runUpdate.GetTag Commit: %v", err) + return fmt.Errorf("tag.Commit: %v", err) } actEmail = cmt.Committer.Email } commit := &PushCommits{} - - if err = CommitRepoAction(userID, user.Id, userName, actEmail, - repo.ID, repoUserName, repoName, refName, commit, oldCommitID, newCommitID); err != nil { - log.GitLogger.Fatal(4, "CommitRepoAction: %s/%s:%v", repoUserName, repoName, err) + if err = CommitRepoAction(opts.PusherID, repoUser.Id, opts.PusherName, actEmail, + repo.ID, opts.RepoUserName, opts.RepoName, opts.RefName, commit, opts.OldCommitID, opts.NewCommitID); err != nil { + return fmt.Errorf("CommitRepoAction (tag): %v", err) } return err } - newCommit, err := gitRepo.GetCommit(newCommitID) + newCommit, err := gitRepo.GetCommit(opts.NewCommitID) if err != nil { - return fmt.Errorf("runUpdate GetCommit of newCommitId: %v", err) + return fmt.Errorf("gitRepo.GetCommit: %v", err) } // Push new branch. var l *list.List - if isNew { + if isNewRef { l, err = newCommit.CommitsBeforeLimit(10) if err != nil { - return fmt.Errorf("CommitsBefore: %v", err) + return fmt.Errorf("newCommit.CommitsBeforeLimit: %v", err) } } else { - l, err = newCommit.CommitsBeforeUntil(oldCommitID) + l, err = newCommit.CommitsBeforeUntil(opts.OldCommitID) if err != nil { - return fmt.Errorf("CommitsBeforeUntil: %v", err) + return fmt.Errorf("newCommit.CommitsBeforeUntil: %v", err) } } - if err != nil { - return fmt.Errorf("runUpdate.Commit repoId: %v", err) - } - - if err = CommitRepoAction(userID, user.Id, userName, user.Email, - repo.ID, repoUserName, repoName, refName, ListToPushCommits(l), oldCommitID, newCommitID); err != nil { - return fmt.Errorf("runUpdate.models.CommitRepoAction: %s/%s:%v", repoUserName, repoName, err) + if err = CommitRepoAction(opts.PusherID, repoUser.Id, opts.PusherName, repoUser.Email, + repo.ID, opts.RepoUserName, opts.RepoName, opts.RefName, ListToPushCommits(l), + opts.OldCommitID, opts.NewCommitID); err != nil { + return fmt.Errorf("CommitRepoAction (branch): %v", err) } return nil } -- cgit v1.2.3