From c185a233b1e23e108379ae8423212c5c1b3e2a9d Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 28 Feb 2020 19:52:41 +0000 Subject: [PATCH 01/11] make avatar lookup occur at image request speed up page generation by making avatar lookup occur at the browser not at page generation --- modules/base/tool.go | 2 +- routers/routes/routes.go | 2 ++ routers/user/avatar.go | 13 +++++++++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/modules/base/tool.go b/modules/base/tool.go index cb9b996142a0..067a2b962958 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -211,7 +211,7 @@ func SizedAvatarLink(email string, size int) string { // 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) + return setting.AppSubURL + "/avatar/" + url.PathEscape(email) } // FileSize calculates the file size and generate user-friendly string. diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 9859ebc53938..dd138a5e2ef4 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -417,6 +417,8 @@ func RegisterRoutes(m *macaron.Macaron) { }) // ***** END: User ***** + m.Get("/avatar/:email", user.AvatarByEmail) + adminReq := context.Toggle(&context.ToggleOptions{SignInRequired: true, AdminRequired: true}) // ***** START: Admin ***** diff --git a/routers/user/avatar.go b/routers/user/avatar.go index 045206c50afc..ba5cbc515ecb 100644 --- a/routers/user/avatar.go +++ b/routers/user/avatar.go @@ -9,6 +9,7 @@ import ( "strings" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" ) @@ -41,3 +42,15 @@ func Avatar(ctx *context.Context) { ctx.Redirect(user.RealSizedAvatarLink(size)) } + +// AvatarByEmail redirects the browser to the appropriate Avatar link +func AvatarByEmail(ctx *context.Context) { + email := ctx.Params(":email") + + size := ctx.QueryInt("size") + if size == 0 { + size = base.DefaultAvatarSize + } + + ctx.Redirect(base.SizedAvatarLink(email, size)) +} From bdcf2871b27bbfea05d383d4b4fa27c2f7d540df Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 29 Feb 2020 09:59:27 +0000 Subject: [PATCH 02/11] Protect against evil email address ".." --- modules/base/tool.go | 2 +- routers/routes/routes.go | 2 +- routers/user/avatar.go | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/modules/base/tool.go b/modules/base/tool.go index 067a2b962958..43d510ffb096 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -211,7 +211,7 @@ func SizedAvatarLink(email string, size int) string { // 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 setting.AppSubURL + "/avatar/" + url.PathEscape(email) + return setting.AppSubURL + "/avatar/email-" + url.PathEscape(email) } // FileSize calculates the file size and generate user-friendly string. diff --git a/routers/routes/routes.go b/routers/routes/routes.go index dd138a5e2ef4..4b3e9eb83eb5 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -417,7 +417,7 @@ func RegisterRoutes(m *macaron.Macaron) { }) // ***** END: User ***** - m.Get("/avatar/:email", user.AvatarByEmail) + m.Get("/avatar/email-:email", user.AvatarByEmail) adminReq := context.Toggle(&context.ToggleOptions{SignInRequired: true, AdminRequired: true}) diff --git a/routers/user/avatar.go b/routers/user/avatar.go index ba5cbc515ecb..6da4efc855dc 100644 --- a/routers/user/avatar.go +++ b/routers/user/avatar.go @@ -5,6 +5,7 @@ package user import ( + "errors" "strconv" "strings" @@ -46,7 +47,10 @@ func Avatar(ctx *context.Context) { // AvatarByEmail redirects the browser to the appropriate Avatar link func AvatarByEmail(ctx *context.Context) { email := ctx.Params(":email") - + if email == "" { + ctx.ServerError("invalid email address", errors.New("email cannot be empty")) + return + } size := ctx.QueryInt("size") if size == 0 { size = base.DefaultAvatarSize From 9914e8020714e5df9b3522b6ab3fd2f484e0ac93 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 29 Feb 2020 10:04:38 +0000 Subject: [PATCH 03/11] Fix tests --- modules/base/tool_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index 075b5ed8179c..57130523d733 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -92,11 +92,11 @@ func TestSizedAvatarLink(t *testing.T) { func TestAvatarLink(t *testing.T) { disableGravatar() - assert.Equal(t, "/img/avatar_default.png", AvatarLink("gitea@example.com")) + assert.Equal(t, "/avatar/email-"+url.PathEscape("gitea@example.com"), AvatarLink("gitea@example.com")) enableGravatar(t) assert.Equal(t, - "https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon", + "/avatar/email-"+url.PathEscape("gitea@example.com"), AvatarLink("gitea@example.com"), ) } From 933320fa2997b108dff0356e37851995a149d75f Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 29 Feb 2020 10:59:14 +0000 Subject: [PATCH 04/11] fix tests again --- modules/repository/commits_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index 2f61ce332981..7aa5cf1b099b 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -6,6 +6,7 @@ package repository import ( "container/list" + "net/url" "testing" "time" @@ -114,7 +115,7 @@ func TestPushCommits_AvatarLink(t *testing.T) { pushCommits.AvatarLink("user2@example.com")) assert.Equal(t, - "https://secure.gravatar.com/avatar/19ade630b94e1e0535b3df7387434154?d=identicon", + "/avatar/email-"+url.PathEscape("nonexistent@example.com"), pushCommits.AvatarLink("nonexistent@example.com")) } From 76090098e03fc0f6cc53827b6439823de7ee0ebd Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 29 Feb 2020 15:56:53 +0000 Subject: [PATCH 05/11] Obfuscate the email address in base64 --- modules/base/tool.go | 2 +- modules/base/tool_test.go | 5 +++-- modules/repository/commits_test.go | 4 ++-- routers/routes/routes.go | 2 +- routers/user/avatar.go | 8 +++++++- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/modules/base/tool.go b/modules/base/tool.go index 43d510ffb096..9171d6661609 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -211,7 +211,7 @@ func SizedAvatarLink(email string, size int) string { // 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 setting.AppSubURL + "/avatar/email-" + url.PathEscape(email) + return setting.AppSubURL + "/avatar/" + base64.RawURLEncoding.EncodeToString([]byte(email)) } // FileSize calculates the file size and generate user-friendly string. diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index 57130523d733..0c7aa7691b08 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -1,6 +1,7 @@ package base import ( + "encoding/base64" "net/url" "testing" @@ -92,11 +93,11 @@ func TestSizedAvatarLink(t *testing.T) { func TestAvatarLink(t *testing.T) { disableGravatar() - assert.Equal(t, "/avatar/email-"+url.PathEscape("gitea@example.com"), AvatarLink("gitea@example.com")) + assert.Equal(t, "/avatar/"+base64.RawURLEncoding.EncodeToString([]byte("gitea@example.com")), AvatarLink("gitea@example.com")) enableGravatar(t) assert.Equal(t, - "/avatar/email-"+url.PathEscape("gitea@example.com"), + "/avatar/"+base64.RawURLEncoding.EncodeToString([]byte("gitea@example.com")), AvatarLink("gitea@example.com"), ) } diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index 7aa5cf1b099b..eda839735be7 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -6,7 +6,7 @@ package repository import ( "container/list" - "net/url" + "encoding/base64" "testing" "time" @@ -115,7 +115,7 @@ func TestPushCommits_AvatarLink(t *testing.T) { pushCommits.AvatarLink("user2@example.com")) assert.Equal(t, - "/avatar/email-"+url.PathEscape("nonexistent@example.com"), + "/avatar/"+base64.RawURLEncoding.EncodeToString([]byte("nonexistent@example.com")), pushCommits.AvatarLink("nonexistent@example.com")) } diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 4b3e9eb83eb5..dd138a5e2ef4 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -417,7 +417,7 @@ func RegisterRoutes(m *macaron.Macaron) { }) // ***** END: User ***** - m.Get("/avatar/email-:email", user.AvatarByEmail) + m.Get("/avatar/:email", user.AvatarByEmail) adminReq := context.Toggle(&context.ToggleOptions{SignInRequired: true, AdminRequired: true}) diff --git a/routers/user/avatar.go b/routers/user/avatar.go index 6da4efc855dc..371c7a9c8b9c 100644 --- a/routers/user/avatar.go +++ b/routers/user/avatar.go @@ -5,6 +5,7 @@ package user import ( + "encoding/base64" "errors" "strconv" "strings" @@ -47,7 +48,12 @@ func Avatar(ctx *context.Context) { // AvatarByEmail redirects the browser to the appropriate Avatar link func AvatarByEmail(ctx *context.Context) { email := ctx.Params(":email") - if email == "" { + addr, err := base64.RawURLEncoding.DecodeString(email) + email = string(addr) + if err != nil { + ctx.ServerError("invalid email address", err) + return + } else if email == "" { ctx.ServerError("invalid email address", errors.New("email cannot be empty")) return } From 58593d4ca178dc061ea710e36cf823f0e0b9c94e Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 5 Mar 2020 21:11:19 +0000 Subject: [PATCH 06/11] hash the complete email address --- modules/base/tool.go | 44 +++++++++++++++++++++++++++++- modules/base/tool_test.go | 7 +++-- modules/repository/commits_test.go | 5 ++-- routers/routes/routes.go | 2 +- routers/user/avatar.go | 14 +++------- 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/modules/base/tool.go b/modules/base/tool.go index 9171d6661609..83f5d7f43f1c 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -207,11 +207,53 @@ func SizedAvatarLink(email string, size int) string { return avatarURL.String() } +// SizedAvatarLinkWithDomain returns a sized link to the avatar for the given email +// address. +func SizedAvatarLinkWithDomain(emailHash, domain string, size int) string { + var avatarURL *url.URL + if setting.EnableFederatedAvatar && setting.LibravatarService != nil { + var err error + avatarURL, err = libravatarURL("ignoreme@" + domain) + if err != nil { + return DefaultAvatarLink() + } + // now we replace the hash with the correct one... + avatarPath := avatarURL.EscapedPath() + lastSlash := strings.LastIndexByte(avatarPath, '/') + avatarURL.Path = avatarPath[:lastSlash+1] + emailHash + } else if !setting.DisableGravatar { + // copy GravatarSourceURL, because we will modify its Path. + copyOfGravatarSourceURL := *setting.GravatarSourceURL + avatarURL = ©OfGravatarSourceURL + avatarURL.Path = path.Join(avatarURL.Path, emailHash) + } 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 { - return setting.AppSubURL + "/avatar/" + base64.RawURLEncoding.EncodeToString([]byte(email)) + lowerEmail := strings.ToLower(strings.TrimSpace(email)) + sum := fmt.Sprintf("%x", md5.Sum([]byte(lowerEmail))) + index := strings.IndexByte(email, '@') + domain := "" + if index >= 0 { + domain = email[index+1:] + } + if len(domain) == 0 { + domain = "cdn.libravatar.org" + } + return setting.AppSubURL + "/avatar/" + url.PathEscape(domain) + "/" + url.PathEscape(sum) } // FileSize calculates the file size and generate user-friendly string. diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index 0c7aa7691b08..e34f362c1150 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -1,7 +1,8 @@ package base import ( - "encoding/base64" + "crypto/md5" + "fmt" "net/url" "testing" @@ -93,11 +94,11 @@ func TestSizedAvatarLink(t *testing.T) { func TestAvatarLink(t *testing.T) { disableGravatar() - assert.Equal(t, "/avatar/"+base64.RawURLEncoding.EncodeToString([]byte("gitea@example.com")), AvatarLink("gitea@example.com")) + assert.Equal(t, "/avatar/example.com/"+fmt.Sprintf("%x", md5.Sum([]byte("gitea@example.com"))), AvatarLink("gitea@example.com")) enableGravatar(t) assert.Equal(t, - "/avatar/"+base64.RawURLEncoding.EncodeToString([]byte("gitea@example.com")), + "/avatar/example.com/"+fmt.Sprintf("%x", md5.Sum([]byte("gitea@example.com"))), AvatarLink("gitea@example.com"), ) } diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index eda839735be7..7013bd5f221b 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -6,7 +6,8 @@ package repository import ( "container/list" - "encoding/base64" + "crypto/md5" + "fmt" "testing" "time" @@ -115,7 +116,7 @@ func TestPushCommits_AvatarLink(t *testing.T) { pushCommits.AvatarLink("user2@example.com")) assert.Equal(t, - "/avatar/"+base64.RawURLEncoding.EncodeToString([]byte("nonexistent@example.com")), + "/avatar/example.com/"+fmt.Sprintf("%x", md5.Sum([]byte("nonexistent@example.com"))), pushCommits.AvatarLink("nonexistent@example.com")) } diff --git a/routers/routes/routes.go b/routers/routes/routes.go index dd138a5e2ef4..d73e4a101b7e 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -417,7 +417,7 @@ func RegisterRoutes(m *macaron.Macaron) { }) // ***** END: User ***** - m.Get("/avatar/:email", user.AvatarByEmail) + m.Get("/avatar/:domain/:hash", user.AvatarByEmail) adminReq := context.Toggle(&context.ToggleOptions{SignInRequired: true, AdminRequired: true}) diff --git a/routers/user/avatar.go b/routers/user/avatar.go index 371c7a9c8b9c..c571e701e4b3 100644 --- a/routers/user/avatar.go +++ b/routers/user/avatar.go @@ -5,7 +5,6 @@ package user import ( - "encoding/base64" "errors" "strconv" "strings" @@ -47,13 +46,9 @@ func Avatar(ctx *context.Context) { // AvatarByEmail redirects the browser to the appropriate Avatar link func AvatarByEmail(ctx *context.Context) { - email := ctx.Params(":email") - addr, err := base64.RawURLEncoding.DecodeString(email) - email = string(addr) - if err != nil { - ctx.ServerError("invalid email address", err) - return - } else if email == "" { + domain := ctx.Params(":domain") + hash := ctx.Params(":hash") + if len(domain) == 0 || len(hash) == 0 { ctx.ServerError("invalid email address", errors.New("email cannot be empty")) return } @@ -61,6 +56,5 @@ func AvatarByEmail(ctx *context.Context) { if size == 0 { size = base.DefaultAvatarSize } - - ctx.Redirect(base.SizedAvatarLink(email, size)) + ctx.Redirect(base.SizedAvatarLinkWithDomain(hash, domain, size)) } From 57339c0f306d16f13194423a70996ae73d0cb9b5 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 7 Mar 2020 20:05:34 +0000 Subject: [PATCH 07/11] Move to use a (potentially cached) db entry for emails Signed-off-by: Andrew Thornton --- models/avatar.go | 51 +++++++++++++++++++++++++++++++++++ models/models.go | 1 + modules/base/tool.go | 27 +++---------------- modules/base/tool_test.go | 13 --------- modules/cache/cache.go | 28 +++++++++++++++++++ modules/repository/commits.go | 3 +-- modules/templates/helper.go | 2 +- routers/repo/blame.go | 2 +- routers/routes/routes.go | 2 +- routers/user/avatar.go | 16 ++++++++--- 10 files changed, 99 insertions(+), 46 deletions(-) create mode 100644 models/avatar.go diff --git a/models/avatar.go b/models/avatar.go new file mode 100644 index 000000000000..3cf04c3238ba --- /dev/null +++ b/models/avatar.go @@ -0,0 +1,51 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "crypto/md5" + "fmt" + "net/url" + "strings" + + "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/setting" +) + +var () + +// EmailHash represents a pre-generated hash map +type EmailHash struct { + ID int64 `xorm:"pk autoincr"` + Email string `xorm:"UNIQUE NOT NULL"` + MD5Sum string `xorm:"md5_sum UNIQUE NOT NULL"` +} + +// GetEmailForHash converts a provided md5sum to the email +func GetEmailForHash(md5Sum string) (string, error) { + return cache.GetString("Avatar:"+md5Sum, func() (string, error) { + emailHash := EmailHash{ + MD5Sum: strings.ToLower(strings.TrimSpace(md5Sum)), + } + + _, err := x.Get(&emailHash) + return emailHash.Email, err + }) +} + +// AvatarLink returns an avatar link for a provided email +func AvatarLink(email string) string { + lowerEmail := strings.ToLower(strings.TrimSpace(email)) + sum := fmt.Sprintf("%x", md5.Sum([]byte(lowerEmail))) + _, _ = cache.GetString("Avatar:"+sum, func() (string, error) { + emailHash := &EmailHash{ + Email: lowerEmail, + MD5Sum: sum, + } + _, _ = x.Insert(emailHash) + return lowerEmail, nil + }) + return setting.AppSubURL + "/avatar/" + url.PathEscape(sum) +} diff --git a/models/models.go b/models/models.go index 088445590f3d..55e3182e7fa8 100644 --- a/models/models.go +++ b/models/models.go @@ -123,6 +123,7 @@ func init() { new(OAuth2Grant), new(Task), new(LanguageStat), + new(EmailHash), ) gonicNames := []string{"SSL", "UID"} diff --git a/modules/base/tool.go b/modules/base/tool.go index 336f2b13d614..157bd9bc3d32 100644 --- a/modules/base/tool.go +++ b/modules/base/tool.go @@ -195,23 +195,19 @@ func SizedAvatarLink(email string, size int) string { // SizedAvatarLinkWithDomain returns a sized link to the avatar for the given email // address. -func SizedAvatarLinkWithDomain(emailHash, domain string, size int) string { +func SizedAvatarLinkWithDomain(email string, size int) string { var avatarURL *url.URL if setting.EnableFederatedAvatar && setting.LibravatarService != nil { var err error - avatarURL, err = libravatarURL("ignoreme@" + domain) + avatarURL, err = libravatarURL(email) if err != nil { return DefaultAvatarLink() } - // now we replace the hash with the correct one... - avatarPath := avatarURL.EscapedPath() - lastSlash := strings.LastIndexByte(avatarPath, '/') - avatarURL.Path = avatarPath[:lastSlash+1] + emailHash } else if !setting.DisableGravatar { // copy GravatarSourceURL, because we will modify its Path. copyOfGravatarSourceURL := *setting.GravatarSourceURL avatarURL = ©OfGravatarSourceURL - avatarURL.Path = path.Join(avatarURL.Path, emailHash) + avatarURL.Path = path.Join(avatarURL.Path, HashEmail(email)) } else { return DefaultAvatarLink() } @@ -225,23 +221,6 @@ func SizedAvatarLinkWithDomain(emailHash, domain string, size int) string { 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 { - lowerEmail := strings.ToLower(strings.TrimSpace(email)) - sum := fmt.Sprintf("%x", md5.Sum([]byte(lowerEmail))) - index := strings.IndexByte(email, '@') - domain := "" - if index >= 0 { - domain = email[index+1:] - } - if len(domain) == 0 { - domain = "cdn.libravatar.org" - } - return setting.AppSubURL + "/avatar/" + url.PathEscape(domain) + "/" + url.PathEscape(sum) -} - // FileSize calculates the file size and generate user-friendly string. func FileSize(s int64) string { return humanize.IBytes(uint64(s)) diff --git a/modules/base/tool_test.go b/modules/base/tool_test.go index e34f362c1150..9c1a79e3f2e0 100644 --- a/modules/base/tool_test.go +++ b/modules/base/tool_test.go @@ -1,8 +1,6 @@ package base import ( - "crypto/md5" - "fmt" "net/url" "testing" @@ -92,17 +90,6 @@ func TestSizedAvatarLink(t *testing.T) { ) } -func TestAvatarLink(t *testing.T) { - disableGravatar() - assert.Equal(t, "/avatar/example.com/"+fmt.Sprintf("%x", md5.Sum([]byte("gitea@example.com"))), AvatarLink("gitea@example.com")) - - enableGravatar(t) - assert.Equal(t, - "/avatar/example.com/"+fmt.Sprintf("%x", md5.Sum([]byte("gitea@example.com"))), - AvatarLink("gitea@example.com"), - ) -} - func TestFileSize(t *testing.T) { var size int64 = 512 assert.Equal(t, "512 B", FileSize(size)) diff --git a/modules/cache/cache.go b/modules/cache/cache.go index e3a905e3fabf..859f4a4b47d9 100644 --- a/modules/cache/cache.go +++ b/modules/cache/cache.go @@ -41,6 +41,34 @@ func NewContext() error { return err } +// GetString returns the key value from cache with callback when no key exists in cache +func GetString(key string, getFunc func() (string, error)) (string, error) { + if conn == nil || setting.CacheService.TTL == 0 { + return getFunc() + } + if !conn.IsExist(key) { + var ( + value string + err error + ) + if value, err = getFunc(); err != nil { + return value, err + } + err = conn.Put(key, value, int64(setting.CacheService.TTL.Seconds())) + if err != nil { + return "", err + } + } + value := conn.Get(key) + if v, ok := value.(string); ok { + return v, nil + } + if v, ok := value.(fmt.Stringer); ok { + return v.String(), nil + } + return fmt.Sprintf("%s", conn.Get(key)), nil +} + // GetInt returns key value from cache with callback when no key exists in cache func GetInt(key string, getFunc func() (int, error)) (int, error) { if conn == nil || setting.CacheService.TTL == 0 { diff --git a/modules/repository/commits.go b/modules/repository/commits.go index 7345aaae249f..e02f3d11caab 100644 --- a/modules/repository/commits.go +++ b/modules/repository/commits.go @@ -10,7 +10,6 @@ import ( "time" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" @@ -124,7 +123,7 @@ func (pc *PushCommits) AvatarLink(email string) string { var err error u, err = models.GetUserByEmail(email) if err != nil { - pc.avatars[email] = base.AvatarLink(email) + pc.avatars[email] = models.AvatarLink(email) if !models.IsErrUserNotExist(err) { log.Error("GetUserByEmail: %v", err) return "" diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 9d3206934ec5..b5b49874276d 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -85,7 +85,7 @@ func NewFuncMap() []template.FuncMap { "AllowedReactions": func() []string { return setting.UI.Reactions }, - "AvatarLink": base.AvatarLink, + "AvatarLink": models.AvatarLink, "Safe": Safe, "SafeJS": SafeJS, "Str2html": Str2html, diff --git a/routers/repo/blame.go b/routers/repo/blame.go index f5a2a548e360..beed59ea977a 100644 --- a/routers/repo/blame.go +++ b/routers/repo/blame.go @@ -230,7 +230,7 @@ func renderBlame(ctx *context.Context, blameParts []git.BlamePart, commitNames m } avatar = fmt.Sprintf(``, setting.AppSubURL, url.PathEscape(commit.User.Name), commit.User.RelAvatarLink(), html.EscapeString(authorName)) } else { - avatar = fmt.Sprintf(``, html.EscapeString(base.AvatarLink(commit.Author.Email)), html.EscapeString(commit.Author.Name)) + avatar = fmt.Sprintf(``, html.EscapeString(models.AvatarLink(commit.Author.Email)), html.EscapeString(commit.Author.Name)) } commitInfo.WriteString(fmt.Sprintf(`
%s
%[5]s
%s
`, attr, avatar, repoLink, part.Sha, html.EscapeString(commit.CommitMessage), commitSince)) } else { diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 4e43df0608dd..ef71adbebaea 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -417,7 +417,7 @@ func RegisterRoutes(m *macaron.Macaron) { }) // ***** END: User ***** - m.Get("/avatar/:domain/:hash", user.AvatarByEmail) + m.Get("/avatar/:hash", user.AvatarByEmail) adminReq := context.Toggle(&context.ToggleOptions{SignInRequired: true, AdminRequired: true}) diff --git a/routers/user/avatar.go b/routers/user/avatar.go index c571e701e4b3..697b472fc359 100644 --- a/routers/user/avatar.go +++ b/routers/user/avatar.go @@ -46,15 +46,23 @@ func Avatar(ctx *context.Context) { // AvatarByEmail redirects the browser to the appropriate Avatar link func AvatarByEmail(ctx *context.Context) { - domain := ctx.Params(":domain") hash := ctx.Params(":hash") - if len(domain) == 0 || len(hash) == 0 { - ctx.ServerError("invalid email address", errors.New("email cannot be empty")) + if len(hash) == 0 { + ctx.ServerError("invalid avatar hash", errors.New("hash cannot be empty")) + return + } + email, err := models.GetEmailForHash(hash) + if err != nil { + ctx.ServerError("invalid avatar hash", err) + return + } + if len(email) == 0 { + ctx.Redirect(base.DefaultAvatarLink()) return } size := ctx.QueryInt("size") if size == 0 { size = base.DefaultAvatarSize } - ctx.Redirect(base.SizedAvatarLinkWithDomain(hash, domain, size)) + ctx.Redirect(base.SizedAvatarLinkWithDomain(email, size)) } From c6cca50ba908f6243c120b33ec255f6b65ef73ef Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 8 Mar 2020 20:27:26 +0000 Subject: [PATCH 08/11] As per @lafriks and @guillep2k Signed-off-by: Andrew Thornton --- models/avatar.go | 13 +++++-------- routers/routes/routes.go | 2 +- routers/user/avatar.go | 4 ++-- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/models/avatar.go b/models/avatar.go index 3cf04c3238ba..72ef9f62afae 100644 --- a/models/avatar.go +++ b/models/avatar.go @@ -14,20 +14,17 @@ import ( "code.gitea.io/gitea/modules/setting" ) -var () - // EmailHash represents a pre-generated hash map type EmailHash struct { - ID int64 `xorm:"pk autoincr"` - Email string `xorm:"UNIQUE NOT NULL"` - MD5Sum string `xorm:"md5_sum UNIQUE NOT NULL"` + Hash string `xorm:"pk hash UNIQUE NOT NULL"` + Email string `xorm:"UNIQUE NOT NULL"` } // GetEmailForHash converts a provided md5sum to the email func GetEmailForHash(md5Sum string) (string, error) { return cache.GetString("Avatar:"+md5Sum, func() (string, error) { emailHash := EmailHash{ - MD5Sum: strings.ToLower(strings.TrimSpace(md5Sum)), + Hash: strings.ToLower(strings.TrimSpace(md5Sum)), } _, err := x.Get(&emailHash) @@ -41,8 +38,8 @@ func AvatarLink(email string) string { sum := fmt.Sprintf("%x", md5.Sum([]byte(lowerEmail))) _, _ = cache.GetString("Avatar:"+sum, func() (string, error) { emailHash := &EmailHash{ - Email: lowerEmail, - MD5Sum: sum, + Email: lowerEmail, + Hash: sum, } _, _ = x.Insert(emailHash) return lowerEmail, nil diff --git a/routers/routes/routes.go b/routers/routes/routes.go index ef71adbebaea..e257951d2e9b 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -417,7 +417,7 @@ func RegisterRoutes(m *macaron.Macaron) { }) // ***** END: User ***** - m.Get("/avatar/:hash", user.AvatarByEmail) + m.Get("/avatar/:hash", user.AvatarByEmailHash) adminReq := context.Toggle(&context.ToggleOptions{SignInRequired: true, AdminRequired: true}) diff --git a/routers/user/avatar.go b/routers/user/avatar.go index 697b472fc359..32d05f03cc98 100644 --- a/routers/user/avatar.go +++ b/routers/user/avatar.go @@ -44,8 +44,8 @@ func Avatar(ctx *context.Context) { ctx.Redirect(user.RealSizedAvatarLink(size)) } -// AvatarByEmail redirects the browser to the appropriate Avatar link -func AvatarByEmail(ctx *context.Context) { +// AvatarByEmailHash redirects the browser to the appropriate Avatar link +func AvatarByEmailHash(ctx *context.Context) { hash := ctx.Params(":hash") if len(hash) == 0 { ctx.ServerError("invalid avatar hash", errors.New("hash cannot be empty")) From 08cf87085c7b1ca0de887dea6da8d1e0426f83c0 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 8 Mar 2020 21:58:17 +0000 Subject: [PATCH 09/11] fix unit-tests Signed-off-by: Andrew Thornton --- modules/repository/commits_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go index 7013bd5f221b..cb00e19c2eb1 100644 --- a/modules/repository/commits_test.go +++ b/modules/repository/commits_test.go @@ -116,7 +116,7 @@ func TestPushCommits_AvatarLink(t *testing.T) { pushCommits.AvatarLink("user2@example.com")) assert.Equal(t, - "/avatar/example.com/"+fmt.Sprintf("%x", md5.Sum([]byte("nonexistent@example.com"))), + "/avatar/"+fmt.Sprintf("%x", md5.Sum([]byte("nonexistent@example.com"))), pushCommits.AvatarLink("nonexistent@example.com")) } From 1e6efeb246cac5fe8c2189e8b703625a8230e64d Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 8 Mar 2020 22:20:13 +0000 Subject: [PATCH 10/11] Update models/avatar.go Co-Authored-By: Lauris BH --- models/avatar.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/avatar.go b/models/avatar.go index 72ef9f62afae..311d71462993 100644 --- a/models/avatar.go +++ b/models/avatar.go @@ -16,7 +16,7 @@ import ( // EmailHash represents a pre-generated hash map type EmailHash struct { - Hash string `xorm:"pk hash UNIQUE NOT NULL"` + Hash string `xorm:"pk varchar(32)"` Email string `xorm:"UNIQUE NOT NULL"` } From b5365bad83dd6e7b37cddf16919838d9ea12b1c0 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 27 Mar 2020 10:42:27 +0000 Subject: [PATCH 11/11] Add migration to create the EmailHash table Signed-off-by: Andrew Thornton --- models/migrations/migrations.go | 2 ++ models/migrations/v133.go | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 models/migrations/v133.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index c554121e858b..3f18a18c6d0b 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -198,6 +198,8 @@ var migrations = []Migration{ NewMigration("Add IsSystemWebhook column to webhooks table", addSystemWebhookColumn), // v132 -> v133 NewMigration("Add Branch Protection Protected Files Column", addBranchProtectionProtectedFilesColumn), + // v133 -> v134 + NewMigration("Add EmailHash Table", addEmailHashTable), } // Migrate database to current version diff --git a/models/migrations/v133.go b/models/migrations/v133.go new file mode 100644 index 000000000000..ea0411d470be --- /dev/null +++ b/models/migrations/v133.go @@ -0,0 +1,16 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import "xorm.io/xorm" + +func addEmailHashTable(x *xorm.Engine) error { + // EmailHash represents a pre-generated hash map + type EmailHash struct { + Hash string `xorm:"pk varchar(32)"` + Email string `xorm:"UNIQUE NOT NULL"` + } + return x.Sync2(new(EmailHash)) +}