fix: Invalid UTF-8 commit messages in JSON API responses (#37542) (#37585)

Backport #37542

Co-authored-by: Nicolas <bircni@icloud.com>

---------

Co-authored-by: Nicolas <bircni@icloud.com>

modules/git/commit.go

@@ -37,11 +37,7 @@ type CommitSignature struct {
 
 // Message returns the commit message. Same as retrieving CommitMessage directly.
 func (c *Commit) Message() string {
-	// FIXME: GIT-COMMIT-MESSAGE-ENCODING: this logic is not right
+	return strings.ToValidUTF8(c.CommitMessage, "?")
-	// * When need to use commit message in templates/database, it should be valid UTF-8
-	// * When need to get the original commit message, it should just use "c.CommitMessage"
-	// It's not easy to refactor at the moment, many templates need to be updated and tested
-	return c.CommitMessage
 }
 
 // Summary returns first line of commit message.

modules/git/commit_test.go

@@ -159,6 +159,14 @@ ISO-8859-1`, commitFromReader.Signature.Payload)
 	assert.Equal(t, commitFromReader, commitFromReader2)
 }
 
+func TestCommitMessageSanitizesInvalidUTF8(t *testing.T) {
+	commit := &Commit{
+		CommitMessage: "title \xff\n\n\nbody \xff\n\n\n",
+	}
+	assert.Equal(t, "title ?\n\n\nbody ?\n\n\n", commit.Message())
+	assert.Equal(t, "title ?", commit.Summary())
+}
+
 func TestHasPreviousCommit(t *testing.T) {
 	bareRepo1Path := filepath.Join(testReposDir, "repo1_bare")
 

modules/git/pipeline/lfs_nogogit.go

@@ -11,7 +11,6 @@ import (
 	"encoding/hex"
 	"io"
 	"sort"
-	"strings"
 
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/git/gitcmd"
@@ -102,7 +101,7 @@ func findLFSFileFunc(repo *git.Repository, objectID git.ObjectID, revListReader
 						result := LFSResult{
 							Name:         curPath + string(fname),
 							SHA:          curCommit.ID.String(),
-							Summary:      strings.Split(strings.TrimSpace(curCommit.CommitMessage), "\n")[0],
+							Summary:      curCommit.Summary(),
 							When:         curCommit.Author.When,
 							ParentHashes: curCommit.Parents,
 						}

routers/web/repo/blame.go

@@ -231,7 +231,7 @@ func renderBlameFillFirstBlameRow(repoLink string, avatarUtils *templates.Avatar
 	br.PreviousSha = part.PreviousSha
 	br.PreviousShaURL = fmt.Sprintf("%s/blame/commit/%s/%s", repoLink, url.PathEscape(part.PreviousSha), util.PathEscapeSegments(part.PreviousPath))
 	br.CommitURL = fmt.Sprintf("%s/commit/%s", repoLink, url.PathEscape(part.Sha))
-	br.CommitMessage = commit.CommitMessage
+	br.CommitMessage = commit.Message()
 	br.CommitSince = templates.TimeSince(commit.Author.When)
 }
 

routers/web/repo/compare.go

@@ -435,7 +435,6 @@ func prepareNewPullRequestTitleContent(ci *git_service.CompareInfo, commits []*g
 	}
 
 	if len(commits) == 1 {
-		// FIXME: GIT-COMMIT-MESSAGE-ENCODING: try to convert the encoding for commit message explicitly, ideally it should be done by a git commit struct method
 		c := commits[0]
 		_, content, _ = strings.Cut(strings.TrimSpace(c.UserCommit.CommitMessage), "\n")
 		content = strings.TrimSpace(content)

routers/web/repo/compare_test.go

@@ -78,7 +78,7 @@ func TestNewPullRequestTitleContent(t *testing.T) {
 	assert.Equal(t, "body", content)
 
 	title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{mockCommit("a\xf0\xf0\xf0\nb\xf0\xf0\xf0")})
-	assert.Equal(t, "a?", title) // FIXME: GIT-COMMIT-MESSAGE-ENCODING: "title" doesn't use the same charset converting logic as "content"
+	assert.Equal(t, "a?", title)
 	assert.Equal(t, "b"+string(utf8.RuneError)+string(utf8.RuneError), content)
 
 	title, content = prepareNewPullRequestTitleContent(ci, []*git_model.SignCommitWithStatuses{

services/actions/notifier_helper.go

@@ -320,7 +320,7 @@ func handleWorkflows(
 
 	for _, dwf := range detectedWorkflows {
 		run := &actions_model.ActionRun{
-			Title:             strings.SplitN(commit.CommitMessage, "\n", 2)[0],
+			Title:             commit.Summary(),
 			RepoID:            input.Repo.ID,
 			Repo:              input.Repo,
 			OwnerID:           input.Repo.OwnerID,
@@ -483,7 +483,7 @@ func handleSchedules(
 		}
 
 		run := &actions_model.ActionSchedule{
-			Title:         strings.SplitN(commit.CommitMessage, "\n", 2)[0],
+			Title:         commit.Summary(),
 			RepoID:        input.Repo.ID,
 			Repo:          input.Repo,
 			OwnerID:       input.Repo.OwnerID,

services/actions/workflow.go

@@ -5,7 +5,6 @@ package actions
 
 import (
 	"fmt"
-	"strings"
 
 	actions_model "code.gitea.io/gitea/models/actions"
 	"code.gitea.io/gitea/models/perm"
@@ -98,7 +97,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
 	var entry *git.TreeEntry
 
 	run := &actions_model.ActionRun{
-		Title:             strings.SplitN(runTargetCommit.CommitMessage, "\n", 2)[0],
+		Title:             runTargetCommit.Summary(),
 		RepoID:            repo.ID,
 		Repo:              repo,
 		OwnerID:           repo.OwnerID,

services/agit/agit.go

@@ -154,10 +154,10 @@ func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.
 
 			// create a new pull request
 			if title == "" {
-				title = strings.Split(commit.CommitMessage, "\n")[0]
+				title = commit.Summary()
 			}
 			if description == "" {
-				_, description, _ = strings.Cut(commit.CommitMessage, "\n\n")
+				_, description, _ = strings.Cut(commit.Message(), "\n\n")
 			}
 			if description == "" {
 				description = title

services/convert/convert.go

@@ -214,7 +214,7 @@ func ToTag(repo *repo_model.Repository, t *git.Tag) *api.Tag {
 
 	return &api.Tag{
 		Name:       t.Name,
-		Message:    strings.TrimSpace(t.Message),
+		Message:    strings.ToValidUTF8(strings.TrimSpace(t.Message), "?"), // the trim is not right, 1.27 won't trim
 		ID:         t.ID.String(),
 		Commit:     ToCommitMeta(repo, t),
 		ZipballURL: zipballURL,
@@ -728,7 +728,7 @@ func ToAnnotatedTag(ctx context.Context, repo *repo_model.Repository, t *git.Tag
 		Tag:          t.Name,
 		SHA:          t.ID.String(),
 		Object:       ToAnnotatedTagObject(repo, c),
-		Message:      t.Message,
+		Message:      strings.ToValidUTF8(t.Message, "?"),
 		URL:          repo.APIURL() + "/git/tags/" + t.ID.String(),
 		Tagger:       ToCommitUser(t.Tagger),
 		Verification: ToVerification(ctx, c),

services/convert/wiki.go

@@ -28,7 +28,7 @@ func ToWikiCommit(commit *git.Commit) *api.WikiCommit {
 			},
 			Date: commit.Committer.When.UTC().Format(time.RFC3339),
 		},
-		Message: commit.CommitMessage,
+		Message: commit.Message(),
 	}
 }
 

services/pull/pull.go

@@ -847,7 +847,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
 		maxMsgSize := setting.Repository.PullRequest.DefaultMergeMessageSize
 		for i := len(commits) - 1; i >= 0; i-- {
 			commit := commits[i]
-			msg := strings.TrimSpace(commit.CommitMessage)
+			msg := strings.TrimSpace(commit.Message())
 			if msg == "" {
 				continue
 			}

services/repository/push.go

@@ -402,7 +402,7 @@ func pushUpdateAddTags(ctx context.Context, repo *repo_model.Repository, gitRepo
 		}
 
 		rel, has := relMap[lowerTag]
-		title, note := git.SplitCommitTitleBody(tag.Message, 255)
+		title, note := git.SplitCommitTitleBody(strings.ToValidUTF8(tag.Message, "?"), 255)
 		if !has {
 			rel = &repo_model.Release{
 				RepoID:       repo.ID,