diff --git a/integrations/api_user_orgs_test.go b/integrations/api_user_orgs_test.go index f8372d611..9b250c063 100644 --- a/integrations/api_user_orgs_test.go +++ b/integrations/api_user_orgs_test.go @@ -9,7 +9,9 @@ import ( "net/http" "testing" + "code.gitea.io/gitea/models" api "code.gitea.io/sdk/gitea" + "github.com/stretchr/testify/assert" ) @@ -23,14 +25,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 +52,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: "", diff --git a/integrations/user_test.go b/integrations/user_test.go index 7ff986d54..a6ad164d6 100644 --- a/integrations/user_test.go +++ b/integrations/user_test.go @@ -47,6 +47,7 @@ func TestRenameInvalidUsername(t *testing.T) { "%2f..", "%00", "thisHas ASpace", + "ptho>lor Tw >< " 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 diff --git a/models/issue_reaction_test.go b/models/issue_reaction_test.go index 8363e534d..bbd8cf29f 100644 --- a/models/issue_reaction_test.go +++ b/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)) diff --git a/models/user.go b/models/user.go index 0d8f60886..4ab78ec04 100644 --- a/models/user.go +++ b/models/user.go @@ -417,7 +417,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(), } @@ -630,12 +630,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) diff --git a/models/user_test.go b/models/user_test.go index ba700d365..9d011f32a 100644 --- a/models/user_test.go +++ b/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) + } +}