Ensure valid git author names passed in signatures

integrations/api_user_orgs_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"testing"
 
+	"code.gitea.io/gitea/models"
 	api "code.gitea.io/sdk/gitea"
 	"github.com/stretchr/testify/assert"
 )
@@ -23,14 +24,16 @@ func TestUserOrgs(t *testing.T) {
 	req := NewRequest(t, "GET", urlStr)
 	resp := session.MakeRequest(t, req, http.StatusOK)
 	var orgs []*api.Organization
+	user3 := models.AssertExistsAndLoadBean(t, &models.User{Name: "user3"}).(*models.User)
+
 	DecodeJSON(t, resp, &orgs)
 
 	assert.Equal(t, []*api.Organization{
 		{
 			ID:          3,
-			UserName:    "user3",
-			FullName:    "User Three",
-			AvatarURL:   "https://secure.gravatar.com/avatar/97d6d9441ff85fdc730e02a6068d267b?d=identicon",
+			UserName:    user3.Name,
+			FullName:    user3.FullName,
+			AvatarURL:   user3.AvatarLink(),
 			Description: "",
 			Website:     "",
 			Location:    "",
@@ -48,13 +51,14 @@ func TestMyOrgs(t *testing.T) {
 	resp := session.MakeRequest(t, req, http.StatusOK)
 	var orgs []*api.Organization
 	DecodeJSON(t, resp, &orgs)
+	user3 := models.AssertExistsAndLoadBean(t, &models.User{Name: "user3"}).(*models.User)
 
 	assert.Equal(t, []*api.Organization{
 		{
 			ID:          3,
-			UserName:    "user3",
-			FullName:    "User Three",
-			AvatarURL:   "https://secure.gravatar.com/avatar/97d6d9441ff85fdc730e02a6068d267b?d=identicon",
+			UserName:    user3.Name,
+			FullName:    user3.FullName,
+			AvatarURL:   user3.AvatarLink(),
 			Description: "",
 			Website:     "",
 			Location:    "",

integrations/user_test.go

@@ -47,6 +47,7 @@ func TestRenameInvalidUsername(t *testing.T) {
 		"%2f..",
 		"%00",
 		"thisHas ASpace",
+		"p<A>tho>lo<gical",
 	}
 
 	session := loginUser(t, "user2")

models/fixtures/user.yml

@@ -19,7 +19,7 @@
   id: 2
   lower_name: user2
   name: user2
-  full_name: User Two
+  full_name: "   < U<se>r Tw<o > ><  "
   email: user2@example.com
   passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password
   type: 0 # individual
@@ -37,7 +37,7 @@
   id: 3
   lower_name: user3
   name: user3
-  full_name: User Three
+  full_name: " <<<< >> >> > >> > >>> >> "
   email: user3@example.com
   passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password
   type: 1 # organization
@@ -53,7 +53,7 @@
   id: 4
   lower_name: user4
   name: user4
-  full_name: User Four
+  full_name: "          "
   email: user4@example.com
   passwd: 7d93daa0d1e6f2305cc8fa496847d61dc7320bb16262f9c55dd753480207234cdd96a93194e408341971742f4701772a025a # password
   type: 0 # individual

models/issue_reaction_test.go

@@ -99,7 +99,7 @@ func TestIssueReactionCount(t *testing.T) {
 	reactions := issue1.Reactions.GroupByType()
 	assert.Len(t, reactions["heart"], 4)
 	assert.Equal(t, 2, reactions["heart"].GetMoreUserCount())
-	assert.Equal(t, "User One, User Two", reactions["heart"].GetFirstUsers())
+	assert.Equal(t, user1.DisplayName()+", "+user2.DisplayName(), reactions["heart"].GetFirstUsers())
 	assert.True(t, reactions["heart"].HasUser(1))
 	assert.False(t, reactions["heart"].HasUser(5))
 	assert.False(t, reactions["heart"].HasUser(0))

models/user.go

@@ -416,7 +416,7 @@ func (u *User) GetFollowing(page int) ([]*User, error) {
 // NewGitSig generates and returns the signature of given user.
 func (u *User) NewGitSig() *git.Signature {
 	return &git.Signature{
-		Name:  u.DisplayName(),
+		Name:  u.GitName(),
 		Email: u.getEmail(),
 		When:  time.Now(),
 	}
@@ -629,12 +629,33 @@ func (u *User) GetOrganizations(all bool) error {
 // DisplayName returns full name if it's not empty,
 // returns username otherwise.
 func (u *User) DisplayName() string {
-	if len(u.FullName) > 0 {
-		return u.FullName
+	trimmed := strings.TrimSpace(u.FullName)
+	if len(trimmed) > 0 {
+		return trimmed
 	}
 	return u.Name
 }
 
+func gitSafeName(name string) string {
+	return strings.TrimSpace(strings.NewReplacer("\n", "", "<", "", ">", "").Replace(name))
+}
+
+// GitName returns a git safe name
+func (u *User) GitName() string {
+	gitName := gitSafeName(u.FullName)
+	if len(gitName) > 0 {
+		return gitName
+	}
+	// Although u.Name should be safe if created in our system
+	// LDAP users may have bad names
+	gitName = gitSafeName(u.Name)
+	if len(gitName) > 0 {
+		return gitName
+	}
+	// Totally pathological name so it's got to be:
+	return fmt.Sprintf("user-%d", u.ID)
+}
+
 // ShortName ellipses username to length
 func (u *User) ShortName(length int) string {
 	return base.EllipsisString(u.Name, length)

models/user_test.go

@@ -6,6 +6,7 @@ package models
 
 import (
 	"math/rand"
+	"strings"
 	"testing"
 
 	"code.gitea.io/gitea/modules/setting"
@@ -181,3 +182,34 @@ func TestGetOrgRepositoryIDs(t *testing.T) {
 	// User 5's team has no access to any repo
 	assert.Len(t, accessibleRepos, 0)
 }
+
+func TestNewGitSig(t *testing.T) {
+	users := make([]*User, 0, 20)
+	sess := x.NewSession()
+	defer sess.Close()
+	sess.Find(&users)
+
+	for _, user := range users {
+		sig := user.NewGitSig()
+		assert.NotContains(t, sig.Name, "<")
+		assert.NotContains(t, sig.Name, ">")
+		assert.NotContains(t, sig.Name, "\n")
+		assert.NotEqual(t, len(strings.TrimSpace(sig.Name)), 0)
+	}
+}
+
+func TestDisplayName(t *testing.T) {
+	users := make([]*User, 0, 20)
+	sess := x.NewSession()
+	defer sess.Close()
+	sess.Find(&users)
+
+	for _, user := range users {
+		displayName := user.DisplayName()
+		assert.Equal(t, strings.TrimSpace(displayName), displayName)
+		if len(strings.TrimSpace(user.FullName)) == 0 {
+			assert.Equal(t, user.Name, displayName)
+		}
+		assert.NotEqual(t, len(strings.TrimSpace(displayName)), 0)
+	}
+}