Skip to content

Commit e6baa65

Browse files
zeripathlafriks
andauthored
make avatar lookup occur at image request (#10540)
speed up page generation by making avatar lookup occur at the browser not at page generation * Protect against evil email address ".." * hash the complete email address Signed-off-by: Andrew Thornton <[email protected]> Co-Authored-By: Lauris BH <[email protected]>
1 parent a3f9094 commit e6baa65

File tree

13 files changed

+154
-21
lines changed

13 files changed

+154
-21
lines changed

models/avatar.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2020 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package models
6+
7+
import (
8+
"crypto/md5"
9+
"fmt"
10+
"net/url"
11+
"strings"
12+
13+
"code.gitea.io/gitea/modules/cache"
14+
"code.gitea.io/gitea/modules/setting"
15+
)
16+
17+
// EmailHash represents a pre-generated hash map
18+
type EmailHash struct {
19+
Hash string `xorm:"pk varchar(32)"`
20+
Email string `xorm:"UNIQUE NOT NULL"`
21+
}
22+
23+
// GetEmailForHash converts a provided md5sum to the email
24+
func GetEmailForHash(md5Sum string) (string, error) {
25+
return cache.GetString("Avatar:"+md5Sum, func() (string, error) {
26+
emailHash := EmailHash{
27+
Hash: strings.ToLower(strings.TrimSpace(md5Sum)),
28+
}
29+
30+
_, err := x.Get(&emailHash)
31+
return emailHash.Email, err
32+
})
33+
}
34+
35+
// AvatarLink returns an avatar link for a provided email
36+
func AvatarLink(email string) string {
37+
lowerEmail := strings.ToLower(strings.TrimSpace(email))
38+
sum := fmt.Sprintf("%x", md5.Sum([]byte(lowerEmail)))
39+
_, _ = cache.GetString("Avatar:"+sum, func() (string, error) {
40+
emailHash := &EmailHash{
41+
Email: lowerEmail,
42+
Hash: sum,
43+
}
44+
_, _ = x.Insert(emailHash)
45+
return lowerEmail, nil
46+
})
47+
return setting.AppSubURL + "/avatar/" + url.PathEscape(sum)
48+
}

models/migrations/migrations.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,8 @@ var migrations = []Migration{
198198
NewMigration("Add IsSystemWebhook column to webhooks table", addSystemWebhookColumn),
199199
// v132 -> v133
200200
NewMigration("Add Branch Protection Protected Files Column", addBranchProtectionProtectedFilesColumn),
201+
// v133 -> v134
202+
NewMigration("Add EmailHash Table", addEmailHashTable),
201203
}
202204

203205
// Migrate database to current version

models/migrations/v133.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2020 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import "xorm.io/xorm"
8+
9+
func addEmailHashTable(x *xorm.Engine) error {
10+
// EmailHash represents a pre-generated hash map
11+
type EmailHash struct {
12+
Hash string `xorm:"pk varchar(32)"`
13+
Email string `xorm:"UNIQUE NOT NULL"`
14+
}
15+
return x.Sync2(new(EmailHash))
16+
}

models/models.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ func init() {
124124
new(OAuth2Grant),
125125
new(Task),
126126
new(LanguageStat),
127+
new(EmailHash),
127128
)
128129

129130
gonicNames := []string{"SSL", "UID"}

modules/base/tool.go

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,32 @@ func SizedAvatarLink(email string, size int) string {
193193
return avatarURL.String()
194194
}
195195

196-
// AvatarLink returns relative avatar link to the site domain by given email,
197-
// which includes app sub-url as prefix. However, it is possible
198-
// to return full URL if user enables Gravatar-like service.
199-
func AvatarLink(email string) string {
200-
return SizedAvatarLink(email, DefaultAvatarSize)
196+
// SizedAvatarLinkWithDomain returns a sized link to the avatar for the given email
197+
// address.
198+
func SizedAvatarLinkWithDomain(email string, size int) string {
199+
var avatarURL *url.URL
200+
if setting.EnableFederatedAvatar && setting.LibravatarService != nil {
201+
var err error
202+
avatarURL, err = libravatarURL(email)
203+
if err != nil {
204+
return DefaultAvatarLink()
205+
}
206+
} else if !setting.DisableGravatar {
207+
// copy GravatarSourceURL, because we will modify its Path.
208+
copyOfGravatarSourceURL := *setting.GravatarSourceURL
209+
avatarURL = &copyOfGravatarSourceURL
210+
avatarURL.Path = path.Join(avatarURL.Path, HashEmail(email))
211+
} else {
212+
return DefaultAvatarLink()
213+
}
214+
215+
vals := avatarURL.Query()
216+
vals.Set("d", "identicon")
217+
if size != DefaultAvatarSize {
218+
vals.Set("s", strconv.Itoa(size))
219+
}
220+
avatarURL.RawQuery = vals.Encode()
221+
return avatarURL.String()
201222
}
202223

203224
// FileSize calculates the file size and generate user-friendly string.

modules/base/tool_test.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,6 @@ func TestSizedAvatarLink(t *testing.T) {
9090
)
9191
}
9292

93-
func TestAvatarLink(t *testing.T) {
94-
disableGravatar()
95-
assert.Equal(t, "/img/avatar_default.png", AvatarLink("[email protected]"))
96-
97-
enableGravatar(t)
98-
assert.Equal(t,
99-
"https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon",
100-
AvatarLink("[email protected]"),
101-
)
102-
}
103-
10493
func TestFileSize(t *testing.T) {
10594
var size int64 = 512
10695
assert.Equal(t, "512 B", FileSize(size))

modules/cache/cache.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,34 @@ func NewContext() error {
4141
return err
4242
}
4343

44+
// GetString returns the key value from cache with callback when no key exists in cache
45+
func GetString(key string, getFunc func() (string, error)) (string, error) {
46+
if conn == nil || setting.CacheService.TTL == 0 {
47+
return getFunc()
48+
}
49+
if !conn.IsExist(key) {
50+
var (
51+
value string
52+
err error
53+
)
54+
if value, err = getFunc(); err != nil {
55+
return value, err
56+
}
57+
err = conn.Put(key, value, int64(setting.CacheService.TTL.Seconds()))
58+
if err != nil {
59+
return "", err
60+
}
61+
}
62+
value := conn.Get(key)
63+
if v, ok := value.(string); ok {
64+
return v, nil
65+
}
66+
if v, ok := value.(fmt.Stringer); ok {
67+
return v.String(), nil
68+
}
69+
return fmt.Sprintf("%s", conn.Get(key)), nil
70+
}
71+
4472
// GetInt returns key value from cache with callback when no key exists in cache
4573
func GetInt(key string, getFunc func() (int, error)) (int, error) {
4674
if conn == nil || setting.CacheService.TTL == 0 {

modules/repository/commits.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"time"
1111

1212
"code.gitea.io/gitea/models"
13-
"code.gitea.io/gitea/modules/base"
1413
"code.gitea.io/gitea/modules/git"
1514
"code.gitea.io/gitea/modules/log"
1615
api "code.gitea.io/gitea/modules/structs"
@@ -124,7 +123,7 @@ func (pc *PushCommits) AvatarLink(email string) string {
124123
var err error
125124
u, err = models.GetUserByEmail(email)
126125
if err != nil {
127-
pc.avatars[email] = base.AvatarLink(email)
126+
pc.avatars[email] = models.AvatarLink(email)
128127
if !models.IsErrUserNotExist(err) {
129128
log.Error("GetUserByEmail: %v", err)
130129
return ""

modules/repository/commits_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ package repository
66

77
import (
88
"container/list"
9+
"crypto/md5"
10+
"fmt"
911
"testing"
1012
"time"
1113

@@ -114,7 +116,7 @@ func TestPushCommits_AvatarLink(t *testing.T) {
114116
pushCommits.AvatarLink("[email protected]"))
115117

116118
assert.Equal(t,
117-
"https://secure.gravatar.com/avatar/19ade630b94e1e0535b3df7387434154?d=identicon",
119+
"/avatar/"+fmt.Sprintf("%x", md5.Sum([]byte("nonexistent@example.com"))),
118120
pushCommits.AvatarLink("[email protected]"))
119121
}
120122

modules/templates/helper.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func NewFuncMap() []template.FuncMap {
8585
"AllowedReactions": func() []string {
8686
return setting.UI.Reactions
8787
},
88-
"AvatarLink": base.AvatarLink,
88+
"AvatarLink": models.AvatarLink,
8989
"Safe": Safe,
9090
"SafeJS": SafeJS,
9191
"Str2html": Str2html,

routers/repo/blame.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames m
230230
}
231231
avatar = fmt.Sprintf(`<a href="%s/%s"><img class="ui avatar image" src="%s" title="%s" alt=""/></a>`, setting.AppSubURL, url.PathEscape(commit.User.Name), commit.User.RelAvatarLink(), html.EscapeString(authorName))
232232
} else {
233-
avatar = fmt.Sprintf(`<img class="ui avatar image" src="%s" title="%s"/>`, html.EscapeString(base.AvatarLink(commit.Author.Email)), html.EscapeString(commit.Author.Name))
233+
avatar = fmt.Sprintf(`<img class="ui avatar image" src="%s" title="%s"/>`, html.EscapeString(models.AvatarLink(commit.Author.Email)), html.EscapeString(commit.Author.Name))
234234
}
235235
commitInfo.WriteString(fmt.Sprintf(`<div class="blame-info%s"><div class="blame-data"><div class="blame-avatar">%s</div><div class="blame-message"><a href="%s/commit/%s" title="%[5]s">%[5]s</a></div><div class="blame-time">%s</div></div></div>`, attr, avatar, repoLink, part.Sha, html.EscapeString(commit.CommitMessage), commitSince))
236236
} else {

routers/routes/routes.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,8 @@ func RegisterRoutes(m *macaron.Macaron) {
417417
})
418418
// ***** END: User *****
419419

420+
m.Get("/avatar/:hash", user.AvatarByEmailHash)
421+
420422
adminReq := context.Toggle(&context.ToggleOptions{SignInRequired: true, AdminRequired: true})
421423

422424
// ***** START: Admin *****

routers/user/avatar.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
package user
66

77
import (
8+
"errors"
89
"strconv"
910
"strings"
1011

1112
"code.gitea.io/gitea/models"
13+
"code.gitea.io/gitea/modules/base"
1214
"code.gitea.io/gitea/modules/context"
1315
"code.gitea.io/gitea/modules/log"
1416
)
@@ -41,3 +43,26 @@ func Avatar(ctx *context.Context) {
4143

4244
ctx.Redirect(user.RealSizedAvatarLink(size))
4345
}
46+
47+
// AvatarByEmailHash redirects the browser to the appropriate Avatar link
48+
func AvatarByEmailHash(ctx *context.Context) {
49+
hash := ctx.Params(":hash")
50+
if len(hash) == 0 {
51+
ctx.ServerError("invalid avatar hash", errors.New("hash cannot be empty"))
52+
return
53+
}
54+
email, err := models.GetEmailForHash(hash)
55+
if err != nil {
56+
ctx.ServerError("invalid avatar hash", err)
57+
return
58+
}
59+
if len(email) == 0 {
60+
ctx.Redirect(base.DefaultAvatarLink())
61+
return
62+
}
63+
size := ctx.QueryInt("size")
64+
if size == 0 {
65+
size = base.DefaultAvatarSize
66+
}
67+
ctx.Redirect(base.SizedAvatarLinkWithDomain(email, size))
68+
}

0 commit comments

Comments
 (0)