From 8a19c6b9a25e6336be80496f0216971ae4d2475a Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Sun, 10 Dec 2017 21:36:38 -0800 Subject: [PATCH] Fix avatar URLs (#3069) (#3143) * Fix avatar URLs * import order --- models/user.go | 16 +++++--- modules/base/tool.go | 65 ++++++++++++++++++++++++------- modules/base/tool_test.go | 34 ++++++++++++++-- modules/setting/setting.go | 23 ++++++----- templates/org/header.tmpl | 2 +- templates/org/home.tmpl | 2 +- templates/org/member/members.tmpl | 2 +- templates/user/profile.tmpl | 4 +- 8 files changed, 111 insertions(+), 37 deletions(-) diff --git a/models/user.go b/models/user.go index 4dd2ad8d8..c2a04c4aa 100644 --- a/models/user.go +++ b/models/user.go @@ -315,10 +315,9 @@ func (u *User) generateRandomAvatar(e Engine) error { return nil } -// RelAvatarLink returns relative avatar link to the site domain, -// which includes app sub-url as prefix. However, it is possible -// to return full URL if user enables Gravatar-like service. -func (u *User) RelAvatarLink() string { +// SizedRelAvatarLink returns a relative link to the user's avatar. When +// applicable, the link is for an avatar of the indicated size (in pixels). +func (u *User) SizedRelAvatarLink(size int) string { if u.ID == -1 { return base.DefaultAvatarLink() } @@ -338,7 +337,14 @@ func (u *User) RelAvatarLink() string { return setting.AppSubURL + "/avatars/" + u.Avatar } - return base.AvatarLink(u.AvatarEmail) + return base.SizedAvatarLink(u.AvatarEmail, size) +} + +// RelAvatarLink returns a relative link to the user's avatar. The link +// may either be a sub-URL to this site, or a full URL to an external avatar +// service. +func (u *User) RelAvatarLink() string { + return u.SizedRelAvatarLink(base.DefaultAvatarSize) } // AvatarLink returns user avatar absolute link. diff --git a/modules/base/tool.go b/modules/base/tool.go index 194db772c..1316b8fad 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -16,6 +16,8 @@ import ( "math" "math/big" "net/http" + "net/url" + "path" "strconv" "strings" "time" @@ -197,24 +199,59 @@ func DefaultAvatarLink() string { return setting.AppSubURL + "/img/avatar_default.png" } +// DefaultAvatarSize is a sentinel value for the default avatar size, as +// determined by the avatar-hosting service. +const DefaultAvatarSize = -1 + +// libravatarURL returns the URL for the given email. This function should only +// be called if a federated avatar service is enabled. +func libravatarURL(email string) (*url.URL, error) { + urlStr, err := setting.LibravatarService.FromEmail(email) + if err != nil { + log.Error(4, "LibravatarService.FromEmail(email=%s): error %v", email, err) + return nil, err + } + u, err := url.Parse(urlStr) + if err != nil { + log.Error(4, "Failed to parse libravatar url(%s): error %v", urlStr, err) + return nil, err + } + return u, nil +} + +// SizedAvatarLink returns a sized link to the avatar for the given email +// address. +func SizedAvatarLink(email string, size int) string { + var avatarURL *url.URL + if setting.EnableFederatedAvatar && setting.LibravatarService != nil { + var err error + avatarURL, err = libravatarURL(email) + if err != nil { + return DefaultAvatarLink() + } + } else if !setting.DisableGravatar { + // copy GravatarSourceURL, because we will modify its Path. + copyOfGravatarSourceURL := *setting.GravatarSourceURL + avatarURL = ©OfGravatarSourceURL + avatarURL.Path = path.Join(avatarURL.Path, HashEmail(email)) + } else { + return DefaultAvatarLink() + } + + vals := avatarURL.Query() + vals.Set("d", "identicon") + if size != DefaultAvatarSize { + vals.Set("s", strconv.Itoa(size)) + } + avatarURL.RawQuery = vals.Encode() + return avatarURL.String() +} + // AvatarLink returns relative avatar link to the site domain by given email, // which includes app sub-url as prefix. However, it is possible // to return full URL if user enables Gravatar-like service. func AvatarLink(email string) string { - if setting.EnableFederatedAvatar && setting.LibravatarService != nil { - url, err := setting.LibravatarService.FromEmail(email) - if err != nil { - log.Error(4, "LibravatarService.FromEmail(email=%s): error %v", email, err) - return DefaultAvatarLink() - } - return url - } - - if !setting.DisableGravatar { - return setting.GravatarSource + HashEmail(email) + "?d=identicon" - } - - return DefaultAvatarLink() + return SizedAvatarLink(email, DefaultAvatarSize) } // Seconds-based time units diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index 44ea309f7..ffa17fae0 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -1,11 +1,13 @@ package base import ( + "net/url" "os" "testing" "time" "code.gitea.io/gitea/modules/setting" + "github.com/Unknwon/i18n" macaroni18n "github.com/go-macaron/i18n" "github.com/stretchr/testify/assert" @@ -126,16 +128,40 @@ func TestHashEmail(t *testing.T) { ) } -func TestAvatarLink(t *testing.T) { +const gravatarSource = "https://secure.gravatar.com/avatar/" + +func disableGravatar() { setting.EnableFederatedAvatar = false setting.LibravatarService = nil setting.DisableGravatar = true +} - assert.Equal(t, "/img/avatar_default.png", AvatarLink("")) - +func enableGravatar(t *testing.T) { setting.DisableGravatar = false + var err error + setting.GravatarSourceURL, err = url.Parse(gravatarSource) + assert.NoError(t, err) +} + +func TestSizedAvatarLink(t *testing.T) { + disableGravatar() + assert.Equal(t, "/img/avatar_default.png", + SizedAvatarLink("gitea@example.com", 100)) + + enableGravatar(t) assert.Equal(t, - "353cbad9b58e69c96154ad99f92bedc7?d=identicon", + "https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon&s=100", + SizedAvatarLink("gitea@example.com", 100), + ) +} + +func TestAvatarLink(t *testing.T) { + disableGravatar() + assert.Equal(t, "/img/avatar_default.png", AvatarLink("gitea@example.com")) + + enableGravatar(t) + assert.Equal(t, + "https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon", AvatarLink("gitea@example.com"), ) } diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 032497570..4b0182960 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -325,6 +325,7 @@ var ( // Picture settings AvatarUploadPath string GravatarSource string + GravatarSourceURL *url.URL DisableGravatar bool EnableFederatedAvatar bool LibravatarService *libravatar.Libravatar @@ -1026,18 +1027,22 @@ func NewContext() { if DisableGravatar { EnableFederatedAvatar = false } + if EnableFederatedAvatar || !DisableGravatar { + GravatarSourceURL, err = url.Parse(GravatarSource) + if err != nil { + log.Fatal(4, "Failed to parse Gravatar URL(%s): %v", + GravatarSource, err) + } + } if EnableFederatedAvatar { LibravatarService = libravatar.New() - parts := strings.Split(GravatarSource, "/") - if len(parts) >= 3 { - if parts[0] == "https:" { - LibravatarService.SetUseHTTPS(true) - LibravatarService.SetSecureFallbackHost(parts[2]) - } else { - LibravatarService.SetUseHTTPS(false) - LibravatarService.SetFallbackHost(parts[2]) - } + if GravatarSourceURL.Scheme == "https" { + LibravatarService.SetUseHTTPS(true) + LibravatarService.SetSecureFallbackHost(GravatarSourceURL.Host) + } else { + LibravatarService.SetUseHTTPS(false) + LibravatarService.SetFallbackHost(GravatarSourceURL.Host) } } diff --git a/templates/org/header.tmpl b/templates/org/header.tmpl index 0192ad7d8..806682aca 100644 --- a/templates/org/header.tmpl +++ b/templates/org/header.tmpl @@ -3,7 +3,7 @@
- + {{.DisplayName}}
diff --git a/templates/org/home.tmpl b/templates/org/home.tmpl index 60749e9eb..caef9034b 100644 --- a/templates/org/home.tmpl +++ b/templates/org/home.tmpl @@ -3,7 +3,7 @@
- +
{{.Org.DisplayName}} diff --git a/templates/org/member/members.tmpl b/templates/org/member/members.tmpl index e47008c89..7f0a76361 100644 --- a/templates/org/member/members.tmpl +++ b/templates/org/member/members.tmpl @@ -8,7 +8,7 @@ {{range .Members}}
- +
{{.Name}}
diff --git a/templates/user/profile.tmpl b/templates/user/profile.tmpl index 60355de06..22a7f96ed 100644 --- a/templates/user/profile.tmpl +++ b/templates/user/profile.tmpl @@ -6,11 +6,11 @@
{{if eq .SignedUserName .Owner.Name}} - + {{else}} - + {{end}}