Add require signed commit for protected branch

models/pull.go

@@ -152,16 +152,18 @@ func (pr *PullRequest) LoadProtectedBranch() (err error) {
 }
 
 func (pr *PullRequest) loadProtectedBranch(e Engine) (err error) {
-	if pr.BaseRepo == nil {
-		if pr.BaseRepoID == 0 {
-			return nil
-		}
-		pr.BaseRepo, err = getRepositoryByID(e, pr.BaseRepoID)
-		if err != nil {
-			return
+	if pr.ProtectedBranch == nil {
+		if pr.BaseRepo == nil {
+			if pr.BaseRepoID == 0 {
+				return nil
+			}
+			pr.BaseRepo, err = getRepositoryByID(e, pr.BaseRepoID)
+			if err != nil {
+				return
+			}
 		}
+		pr.ProtectedBranch, err = getProtectedBranchBy(e, pr.BaseRepo.ID, pr.BaseBranch)
 	}
-	pr.ProtectedBranch, err = getProtectedBranchBy(e, pr.BaseRepo.ID, pr.BaseBranch)
 	return
 }
 

modules/git/commit_reader.go

@@ -27,56 +27,70 @@ func CommitFromReader(gitRepo *Repository, sha plumbing.Hash, reader io.Reader)
 	pgpsig := false
 
 	scanner := bufio.NewScanner(reader)
+	// Split by '\n' but include the '\n'
+	scanner.Split(func(data []byte, atEOF bool) (advance int, token []byte, err error) {
+		if atEOF && len(data) == 0 {
+			return 0, nil, nil
+		}
+		if i := bytes.IndexByte(data, '\n'); i >= 0 {
+			// We have a full newline-terminated line.
+			return i + 1, data[0 : i+1], nil
+		}
+		// If we're at EOF, we have a final, non-terminated line. Return it.
+		if atEOF {
+			return len(data), data, nil
+		}
+		// Request more data.
+		return 0, nil, nil
+	})
+
 	for scanner.Scan() {
 		line := scanner.Bytes()
 		if pgpsig {
 			if len(line) > 0 && line[0] == ' ' {
-				line = bytes.TrimLeft(line, " ")
-				_, _ = signatureSB.Write(line)
-				_ = signatureSB.WriteByte('\n')
+				_, _ = signatureSB.Write(line[1:])
 				continue
 			} else {
 				pgpsig = false
 			}
 		}
 
 		if !message {
+			// This is probably not correct but is copied from go-gits interpretation...
 			trimmed := bytes.TrimSpace(line)
 			if len(trimmed) == 0 {
 				message = true
-				_, _ = payloadSB.WriteString("\n")
+				_, _ = payloadSB.Write(line)
 				continue
 			}
 
 			split := bytes.SplitN(trimmed, []byte{' '}, 2)
+			var data []byte
+			if len(split) > 1 {
+				data = split[1]
+			}
 
 			switch string(split[0]) {
 			case "tree":
-				commit.Tree = *NewTree(gitRepo, plumbing.NewHash(string(split[1])))
+				commit.Tree = *NewTree(gitRepo, plumbing.NewHash(string(data)))
 				_, _ = payloadSB.Write(line)
-				_ = payloadSB.WriteByte('\n')
 			case "parent":
-				commit.Parents = append(commit.Parents, plumbing.NewHash(string(split[1])))
+				commit.Parents = append(commit.Parents, plumbing.NewHash(string(data)))
 				_, _ = payloadSB.Write(line)
-				_ = payloadSB.WriteByte('\n')
 			case "author":
 				commit.Author = &Signature{}
-				commit.Author.Decode(split[1])
+				commit.Author.Decode(data)
 				_, _ = payloadSB.Write(line)
-				_ = payloadSB.WriteByte('\n')
 			case "committer":
 				commit.Committer = &Signature{}
-				commit.Committer.Decode(split[1])
+				commit.Committer.Decode(data)
 				_, _ = payloadSB.Write(line)
-				_ = payloadSB.WriteByte('\n')
 			case "gpgsig":
-				_, _ = signatureSB.Write(split[1])
-				_ = signatureSB.WriteByte('\n')
+				_, _ = signatureSB.Write(data)
 				pgpsig = true
 			}
 		} else {
 			_, _ = messageSB.Write(line)
-			_ = messageSB.WriteByte('\n')
 		}
 	}
 	commit.CommitMessage = messageSB.String()

options/locale/locale_en-US.ini

@@ -1115,7 +1115,7 @@ milestones.filter_sort.most_issues = Most issues
 milestones.filter_sort.least_issues = Least issues
 
 signing.will_sign = This commit will be signed with key '%s'
-signing.wont_sign.error = There was an error whilst checking if we could sign
+signing.wont_sign.error = There was an error whilst checking if the commit could be signed
 signing.wont_sign.nokey = There is no key available to sign this commit
 signing.wont_sign.never = Commits are never signed
 signing.wont_sign.always = Commits are always signed

routers/api/v1/repo/pull.go

@@ -645,6 +645,7 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) {
 			return
 		}
 		ctx.Error(http.StatusMethodNotAllowed, fmt.Sprintf("Protected branch %s requires signed commits but this merge would not be signed", pr.BaseBranch), err)
+		return
 	}
 
 	if len(form.Do) == 0 {

routers/private/hook.go

@@ -29,7 +29,7 @@ import (
 func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env []string) error {
 	stdoutReader, stdoutWriter, err := os.Pipe()
 	if err != nil {
-		log.Error("Unable to create os.Pipe in %s", repo.Path)
+		log.Error("Unable to create os.Pipe for %s", repo.Path)
 		return err
 	}
 	defer func() {
@@ -72,7 +72,7 @@ func readAndVerifyCommitsFromShaReader(input io.ReadCloser, repo *git.Repository
 func readAndVerifyCommit(sha string, repo *git.Repository, env []string) error {
 	stdoutReader, stdoutWriter, err := os.Pipe()
 	if err != nil {
-		log.Error("Unable to create pipe in %s: %v", repo.Path, err)
+		log.Error("Unable to create pipe for %s: %v", repo.Path, err)
 		return err
 	}
 	defer func() {

services/pull/merge.go

@@ -472,10 +472,8 @@ func getDiffTree(repoPath, baseBranch, headBranch string) (string, error) {
 
 // IsSignedIfRequired check if merge will be signed if required
 func IsSignedIfRequired(pr *models.PullRequest, doer *models.User) (bool, error) {
-	if pr.ProtectedBranch == nil {
-		if err := pr.LoadProtectedBranch(); err != nil {
-			return false, err
-		}
+	if err := pr.LoadProtectedBranch(); err != nil {
+		return false, err
 	}
 
 	if pr.ProtectedBranch == nil || !pr.ProtectedBranch.RequireSignedCommits {