Fix avatar URLs (#3069)

* Fix avatar URLs

* import order

diff --git a/models/user.go b/models/user.go
index f3a74f5e088764c872776e903d8048f10d4f9674..31af3747c0f28e44ca10280549a774b701664a6a 100644 (file)
--- 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 194db772cfa22347b22e1b1792aa7a4fa30c5604..1316b8fad32271ebb13143f874aa6902c6021357 100644 (file)
--- 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"
 }
 
-// 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 {
+// 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 {
-               url, err := setting.LibravatarService.FromEmail(email)
+               var err error
+               avatarURL, err = libravatarURL(email)
                if err != nil {
-                       log.Error(4, "LibravatarService.FromEmail(email=%s): error %v", email, err)
                        return DefaultAvatarLink()
                }
-               return url
+       } else if !setting.DisableGravatar {
+               // copy GravatarSourceURL, because we will modify its Path.
+               copyOfGravatarSourceURL := *setting.GravatarSourceURL
+               avatarURL = &copyOfGravatarSourceURL
+               avatarURL.Path = path.Join(avatarURL.Path, HashEmail(email))
+       } else {
+               return DefaultAvatarLink()
        }
 
-       if !setting.DisableGravatar {
-               return setting.GravatarSource + HashEmail(email) + "?d=identicon"
+       vals := avatarURL.Query()
+       vals.Set("d", "identicon")
+       if size != DefaultAvatarSize {
+               vals.Set("s", strconv.Itoa(size))
        }
+       avatarURL.RawQuery = vals.Encode()
+       return avatarURL.String()
+}
 
-       return DefaultAvatarLink()
+// 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 {
+       return SizedAvatarLink(email, DefaultAvatarSize)
 }
 
 // Seconds-based time units

diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go
index 44ea309f7c89ae19228804f382c74b73b22ef4e9..ffa17fae0001892cd57df4eaed2cf224eb590026 100644 (file)
--- 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,
+               "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,
-               "353cbad9b58e69c96154ad99f92bedc7?d=identicon",
+               "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 db6f749c0638cf6600823be8111558b6b56471d1..f8da9524132f383885166ec44e57c2d08d54a4c5 100644 (file)
--- a/modules/setting/setting.go
+++ b/modules/setting/setting.go
@@ -326,6 +326,7 @@ var (
        // Picture settings
        AvatarUploadPath      string
        GravatarSource        string
+       GravatarSourceURL     *url.URL
        DisableGravatar       bool
        EnableFederatedAvatar bool
        LibravatarService     *libravatar.Libravatar
@@ -1027,18 +1028,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 0192ad7d828af6ff692bdb0a8468fa52270933fd..806682aca90ede80c2d09c63d53b894420cb0d9b 100644 (file)
--- a/templates/org/header.tmpl
+++ b/templates/org/header.tmpl
@@ -3,7 +3,7 @@
                <div class="ui vertically grid head">
                        <div class="column">
                                <div class="ui header">
-                                       <img class="ui image" src="{{.RelAvatarLink}}?s=100">
+                                       <img class="ui image" src="{{.SizedRelAvatarLink 100}}">
                                        <span class="text thin grey"><a href="{{.HomeLink}}">{{.DisplayName}}</a></span>
 
                                        <div class="ui right">

diff --git a/templates/org/home.tmpl b/templates/org/home.tmpl
index 60749e9eb987f8db6c7d2f65d5b5472f24b5f627..caef9034bbc551f35727ddc31a2094cf50823782 100644 (file)
--- a/templates/org/home.tmpl
+++ b/templates/org/home.tmpl
@@ -3,7 +3,7 @@
        <div class="ui container">
                <div class="ui grid">
                        <div class="ui sixteen wide column">
-                               <img class="ui left" id="org-avatar" src="{{.Org.RelAvatarLink}}?s=140"/>
+                               <img class="ui left" id="org-avatar" src="{{.Org.SizedRelAvatarLink 140}}"/>
                                <div id="org-info">
                                        <div class="ui header">
                                                {{.Org.DisplayName}}

diff --git a/templates/org/member/members.tmpl b/templates/org/member/members.tmpl
index e47008c89ae2bb49ba671aaeb0d024ff722d9317..7f0a763610e3b283922e85f32bd9d0f0f8c855de 100644 (file)
--- a/templates/org/member/members.tmpl
+++ b/templates/org/member/members.tmpl
@@ -8,7 +8,7 @@
                        {{range .Members}}
                                <div class="item ui grid">
                                        <div class="ui one wide column">
-                                               <img class="ui avatar" src="{{.RelAvatarLink}}?s=48">
+                                               <img class="ui avatar" src="{{.SizedRelAvatarLink 48}}">
                                        </div>
                                        <div class="ui three wide column">
                                                <div class="meta"><a href="{{.HomeLink}}">{{.Name}}</a></div>

diff --git a/templates/user/profile.tmpl b/templates/user/profile.tmpl
index 60355de060cc23374f42fdb1df2466a5d724de7d..22a7f96eda5de8d80170fefc4c683bcc44f07e35 100644 (file)
--- a/templates/user/profile.tmpl
+++ b/templates/user/profile.tmpl
@@ -6,11 +6,11 @@
                                <div class="ui card">
                                        {{if eq .SignedUserName .Owner.Name}}
                                                <a class="image poping up" href="{{AppSubUrl}}/user/settings/avatar" id="profile-avatar" data-content="{{.i18n.Tr "user.change_avatar"}}" data-variation="inverted tiny" data-position="bottom center">
-                                                       <img src="{{.Owner.RelAvatarLink}}?s=290" title="{{.Owner.Name}}"/>
+                                                       <img src="{{.Owner.SizedRelAvatarLink 290}}" title="{{.Owner.Name}}"/>
                                                </a>
                                        {{else}}
                                                <span class="image">
-                                                       <img src="{{.Owner.RelAvatarLink}}?s=290" title="{{.Owner.Name}}"/>
+                                                       <img src="{{.Owner.SizedRelAvatarLink 290}}" title="{{.Owner.Name}}"/>
                                                </span>
                                        {{end}}
                                        <div class="content">