fix(avatar): use sha256 and inline the federated avatar lookup (#38843)

- Hash emails with sha256. Gravatar moved to sha256, and both it and libravatar.org serve the same image for either hash. - Drop `strk.kbt.io/projects/go/libravatar` for a 46 line inline SRV lookup. It could not bound or cancel its DNS query and panicked on an unexpected resolver error. The replacement carries the request context and a 3s timeout. - Fix federated avatars querying DNS for every avatar on every render. `loadAvatarSetting` compared a cache field that was never assigned, so each call rebuilt the resolver and dropped its cache. That cache is gone, both settings are read where they are used. - Migration 348 recreates `email_hash` with a 64 char hash column and a `hash_type` column, so a later algorithm change can tell old rows apart. The MD5 rows are unreachable and their `UNIQUE` email index would reject the SHA256 replacements. - Fix a re-saved avatar form replacing an uploaded avatar with a random one. - Remove the `duoshuo` `GRAVATAR_SOURCE` alias, that service shut down in 2017. - Remove dead i18n key. Fixes: https://github.com/go-gitea/gitea/issues/34284 Fixes: https://github.com/go-gitea/gitea/issues/28110 Docs: https://gitea.com/gitea/docs/pulls/499 Signed-off-by: silverwind <me@silverwind.io>
2026-08-11 21:48:41 +00:00 · 2026-08-11 01:13:28 +02:00
parent 52d0e18dac
commit e3ee28f15b
17 changed files with 221 additions and 216 deletions
@@ -5,21 +5,18 @@ package avatars

 import (
 	"context"
-	"crypto/md5"
-	"encoding/hex"
-	"fmt"
 	"net/url"
 	"path"
 	"strconv"
 	"strings"
-	"sync/atomic"

 	"gitea.dev/models/db"
+	"gitea.dev/modules/avatar"
+	"gitea.dev/modules/base"
 	"gitea.dev/modules/cache"
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/setting"

-	"strk.kbt.io/projects/go/libravatar"
 	"xorm.io/builder"
 )

@@ -30,77 +27,38 @@ const (
 	DefaultAvatarPixelSize = 28
 )

-// EmailHash represents a pre-generated hash map (mainly used by LibravatarURL, it queries email server's DNS records)
+const emailHashType = "sha256" // so a later algorithm change can tell old rows apart
+
+// EmailHash keeps the email out of the rendered page
 type EmailHash struct {
-	Hash  string `xorm:"pk varchar(32)"`
-	Email string `xorm:"UNIQUE NOT NULL"`
+	Hash     string `xorm:"pk varchar(64)"`
+	Email    string `xorm:"UNIQUE(email_hashtype) NOT NULL"`
+	HashType string `xorm:"UNIQUE(email_hashtype) NOT NULL varchar(16)"`
 }

 func init() {
 	db.RegisterModel(new(EmailHash))
 }

-type avatarSettingStruct struct {
-	defaultAvatarLink string
-	gravatarSource    string
-	gravatarSourceURL *url.URL
-	libravatar        *libravatar.Libravatar
-}
-
-var avatarSettingAtomic atomic.Pointer[avatarSettingStruct]
-
-func loadAvatarSetting() (*avatarSettingStruct, error) {
-	s := avatarSettingAtomic.Load()
-	if s == nil || s.gravatarSource != setting.GravatarSource {
-		s = &avatarSettingStruct{}
-		u, err := url.Parse(setting.AppSubURL)
-		if err != nil {
-			return nil, fmt.Errorf("unable to parse AppSubURL: %w", err)
-		}
-
-		u.Path = path.Join(u.Path, "/assets/img/avatar_default.png")
-		s.defaultAvatarLink = u.String()
-
-		s.gravatarSourceURL, err = url.Parse(setting.GravatarSource)
-		if err != nil {
-			return nil, fmt.Errorf("unable to parse GravatarSource %q: %w", setting.GravatarSource, err)
-		}
-
-		s.libravatar = libravatar.New()
-		if s.gravatarSourceURL.Scheme == "https" {
-			s.libravatar.SetUseHTTPS(true)
-			s.libravatar.SetSecureFallbackHost(s.gravatarSourceURL.Host)
-		} else {
-			s.libravatar.SetUseHTTPS(false)
-			s.libravatar.SetFallbackHost(s.gravatarSourceURL.Host)
-		}
-
-		avatarSettingAtomic.Store(s)
-	}
-	return s, nil
-}
-
 // DefaultAvatarLink the default avatar link
 func DefaultAvatarLink() string {
-	a, err := loadAvatarSetting()
-	if err != nil {
-		log.Error("Failed to loadAvatarSetting: %v", err)
-		return ""
-	}
-	return a.defaultAvatarLink
+	return setting.AppSubURL + "/assets/img/avatar_default.png"
 }

-// HashEmail hashes email address to MD5 string. https://en.gravatar.com/site/implement/hash/
+// HashEmail hashes an email address the way avatar services address it. https://docs.gravatar.com/api/avatars/images/
 func HashEmail(email string) string {
-	m := md5.New()
-	_, _ = m.Write([]byte(strings.ToLower(strings.TrimSpace(email))))
-	return hex.EncodeToString(m.Sum(nil))
+	return base.EncodeSha256(strings.ToLower(strings.TrimSpace(email)))
 }

-// GetEmailForHash converts a provided md5sum to the email
-func GetEmailForHash(ctx context.Context, md5Sum string) (string, error) {
-	return cache.GetString("Avatar:"+md5Sum, func() (string, error) {
-		emailHash, has, err := db.Get[EmailHash](ctx, builder.Eq{"`hash`": strings.ToLower(strings.TrimSpace(md5Sum))})
+func emailHashCacheKey(hash string) string {
+	return cache.SafeCacheKey("Avatar", hash)
+}
+
+// GetEmailForHash converts a provided hash to the email
+func GetEmailForHash(ctx context.Context, hash string) (string, error) {
+	hash = strings.ToLower(strings.TrimSpace(hash))
+	return cache.GetString(emailHashCacheKey(hash), func() (string, error) {
+		emailHash, has, err := db.Get[EmailHash](ctx, builder.Eq{"`hash`": hash})
 		if err != nil {
 			return "", err
 		} else if !has {
@@ -110,50 +68,19 @@ func GetEmailForHash(ctx context.Context, md5Sum string) (string, error) {
 	})
 }

-// LibravatarURL returns the URL for the given email. Slow due to the DNS lookup.
-// This function should only be called if a federated avatar service is enabled.
-func LibravatarURL(email string) (*url.URL, error) {
-	a, err := loadAvatarSetting()
-	if err != nil {
-		return nil, err
-	}
-	urlStr, err := a.libravatar.FromEmail(email)
-	if err != nil {
-		log.Error("LibravatarService.FromEmail(email=%s): error %v", email, err)
-		return nil, err
-	}
-	u, err := url.Parse(urlStr)
-	if err != nil {
-		log.Error("Failed to parse libravatar url(%s): error %v", urlStr, err)
-		return nil, err
-	}
-	return u, nil
-}
-
-// saveEmailHash returns an avatar link for a provided email,
-// the email and hash are saved into database, which will be used by GetEmailForHash later
+// saveEmailHash returns the hash and stores the pair for GetEmailForHash
 func saveEmailHash(ctx context.Context, email string) string {
 	lowerEmail := strings.ToLower(strings.TrimSpace(email))
 	emailHash := HashEmail(lowerEmail)
-	_, _ = cache.GetString("Avatar:"+emailHash, func() (string, error) {
-		emailHash := &EmailHash{
-			Email: lowerEmail,
-			Hash:  emailHash,
+	// a key of its own, GetEmailForHash caches an unknown hash as empty
+	_, _ = cache.GetString(cache.SafeCacheKey("AvatarStored", emailHash), func() (string, error) {
+		// the check keeps a duplicate key error out of a transaction the caller may hold
+		has, err := db.Exist[EmailHash](ctx, builder.Eq{"`hash`": emailHash})
+		if err == nil && !has {
+			_, err = db.GetEngine(ctx).Insert(&EmailHash{Email: lowerEmail, Hash: emailHash, HashType: emailHashType})
+			cache.Remove(emailHashCacheKey(emailHash)) // a lookup may have cached it as unknown
 		}
-		// OK we're going to open a session just because I think that that might hide away any problems with postgres reporting errors
-		if err := db.WithTx(ctx, func(ctx context.Context) error {
-			has, err := db.GetEngine(ctx).Where("email = ? AND hash = ?", emailHash.Email, emailHash.Hash).Get(new(EmailHash))
-			if has || err != nil {
-				// Seriously we don't care about any DB problems just return the lowerEmail - we expect the transaction to fail most of the time
-				return nil
-			}
-			_, _ = db.GetEngine(ctx).Insert(emailHash)
-			return nil
-		}); err != nil {
-			// Seriously we don't care about any DB problems just return the lowerEmail - we expect the transaction to fail most of the time
-			return lowerEmail, nil
-		}
-		return lowerEmail, nil
+		return lowerEmail, err // an error must leave the hash unmarked
 	})
 	return emailHash
 }
@@ -174,59 +101,48 @@ func GenerateUserAvatarImageLink(userAvatar string, size int) string {
 	return setting.AppSubURL + "/avatars/" + url.PathEscape(userAvatar)
 }

-// generateRecognizedAvatarURL generate a recognized avatar (Gravatar/Libravatar) URL, it modifies the URL so the parameter is passed by a copy
-func generateRecognizedAvatarURL(u url.URL, size int) string {
-	urlQuery := u.Query()
+func generateSourceAvatarURL(source url.URL, email string, size int) string {
+	source.Path = path.Join(source.Path, HashEmail(email))
+	urlQuery := source.Query()
 	urlQuery.Set("d", "identicon")
 	if size > 0 {
 		urlQuery.Set("s", strconv.Itoa(size))
 	}
-	u.RawQuery = urlQuery.Encode()
-	return u.String()
+	source.RawQuery = urlQuery.Encode()
+	return source.String()
 }

-// generateEmailAvatarLink returns a email avatar link.
-// if final is true, it may use a slow path (eg: query DNS).
-// if final is false, it always uses a fast path.
+// generateEmailAvatarLink returns a email avatar link, a final link may query DNS
 func generateEmailAvatarLink(ctx context.Context, email string, size int, final bool) string {
 	email = strings.TrimSpace(email)
 	if email == "" {
 		return DefaultAvatarLink()
 	}

-	avatarSetting, err := loadAvatarSetting()
-	if err != nil {
+	federated := setting.Config().Picture.EnableFederatedAvatar.Value(ctx)
+	if federated && !final {
+		// return a 302 link, so page rendering never waits for the DNS query
+		link := setting.AppSubURL + "/avatar/" + url.PathEscape(saveEmailHash(ctx, email))
+		if size > 0 {
+			link += "?size=" + strconv.Itoa(size)
+		}
+		return link
+	}
+	if !federated && setting.Config().Picture.DisableGravatar.Value(ctx) {
 		return DefaultAvatarLink()
 	}

-	enableFederatedAvatar := setting.Config().Picture.EnableFederatedAvatar.Value(ctx)
-	if enableFederatedAvatar {
-		emailHash := saveEmailHash(ctx, email)
-		if final {
-			// for final link, we can spend more time on slow external query
-			var avatarURL *url.URL
-			if avatarURL, err = LibravatarURL(email); err != nil {
-				return DefaultAvatarLink()
-			}
-			return generateRecognizedAvatarURL(*avatarURL, size)
-		}
-		// for non-final link, we should return fast (use a 302 redirection link)
-		urlStr := setting.AppSubURL + "/avatar/" + url.PathEscape(emailHash)
-		if size > 0 {
-			urlStr += "?size=" + strconv.Itoa(size)
-		}
-		return urlStr
+	source, err := url.Parse(setting.GravatarSource)
+	if err != nil {
+		log.Error("unable to parse GravatarSource %q: %v", setting.GravatarSource, err)
+		return DefaultAvatarLink()
 	}
-
-	disableGravatar := setting.Config().Picture.DisableGravatar.Value(ctx)
-	if !disableGravatar {
-		// copy GravatarSourceURL, because we will modify its Path.
-		avatarURLCopy := *avatarSetting.gravatarSourceURL
-		avatarURLCopy.Path = path.Join(avatarURLCopy.Path, HashEmail(email))
-		return generateRecognizedAvatarURL(avatarURLCopy, size)
+	if federated {
+		if host := avatar.LookupFederatedHost(ctx, email, source.Scheme == "https"); host != "" {
+			source.Host, source.Path = host, "/avatar"
+		}
 	}
-
-	return DefaultAvatarLink()
+	return generateSourceAvatarURL(*source, email, size)
 }

 // GenerateEmailAvatarFastLink returns a avatar link (fast, the link may be a delegated one: "/avatar/${hash}")
@@ -4,54 +4,48 @@
 package avatars_test

 import (
+	"strconv"
 	"testing"

 	avatars_model "gitea.dev/models/avatars"
 	system_model "gitea.dev/models/system"
+	"gitea.dev/models/unittest"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/setting/config"

 	"github.com/stretchr/testify/assert"
 )

-const gravatarSource = "https://secure.gravatar.com/avatar/"
-
-func disableGravatar(t *testing.T) {
-	err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.EnableFederatedAvatar.DynKey(): "false"})
-	assert.NoError(t, err)
-	err = system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "true"})
-	assert.NoError(t, err)
-}
-
-func enableGravatar(t *testing.T) {
-	err := system_model.SetSettings(t.Context(), map[string]string{setting.Config().Picture.DisableGravatar.DynKey(): "false"})
-	assert.NoError(t, err)
-	setting.GravatarSource = gravatarSource
-}
-
-func TestHashEmail(t *testing.T) {
-	assert.Equal(t,
-		"d41d8cd98f00b204e9800998ecf8427e",
-		avatars_model.HashEmail(""),
-	)
-	assert.Equal(t,
-		"353cbad9b58e69c96154ad99f92bedc7",
-		avatars_model.HashEmail("gitea@example.com"),
-	)
-}
-
-func TestSizedAvatarLink(t *testing.T) {
+func TestEmailAvatarLink(t *testing.T) {
+	const email = "gitea@example.com"
+	const emailHash = "72af1071d72449afe29e816060e78e78fd85829ba6e2497aa1d4eedd3c9dc611"
 	setting.AppSubURL = "/testsuburl"
+	setting.GravatarSource = "https://secure.gravatar.com/avatar/"

-	disableGravatar(t)
-	config.GetDynGetter().InvalidateCache()
+	assert.Equal(t, emailHash, avatars_model.HashEmail(" Gitea@Example.com "))
+
+	setAvatarConfig := func(disableGravatar, enableFederatedAvatar bool) {
+		assert.NoError(t, system_model.SetSettings(t.Context(), map[string]string{
+			setting.Config().Picture.DisableGravatar.DynKey():       strconv.FormatBool(disableGravatar),
+			setting.Config().Picture.EnableFederatedAvatar.DynKey(): strconv.FormatBool(enableFederatedAvatar),
+		}))
+		config.GetDynGetter().InvalidateCache()
+	}
+
+	setAvatarConfig(true, false)
 	assert.Equal(t, "/testsuburl/assets/img/avatar_default.png",
-		avatars_model.GenerateEmailAvatarFastLink(t.Context(), "gitea@example.com", 100))
+		avatars_model.GenerateEmailAvatarFastLink(t.Context(), email, 100))

-	enableGravatar(t)
-	config.GetDynGetter().InvalidateCache()
-	assert.Equal(t,
-		"https://secure.gravatar.com/avatar/353cbad9b58e69c96154ad99f92bedc7?d=identicon&s=100",
-		avatars_model.GenerateEmailAvatarFastLink(t.Context(), "gitea@example.com", 100),
-	)
+	setAvatarConfig(false, false)
+	assert.Equal(t, "https://secure.gravatar.com/avatar/"+emailHash+"?d=identicon&s=100",
+		avatars_model.GenerateEmailAvatarFastLink(t.Context(), email, 100))
+
+	// the DNS query waits until the browser follows the link
+	setAvatarConfig(false, true)
+	assert.Equal(t, "/testsuburl/avatar/"+emailHash+"?size=100",
+		avatars_model.GenerateEmailAvatarFastLink(t.Context(), email, 100))
+	storedEmail, err := avatars_model.GetEmailForHash(t.Context(), emailHash)
+	assert.NoError(t, err)
+	assert.Equal(t, email, storedEmail)
+	assert.Equal(t, "sha256", unittest.AssertExistsAndLoadBean(t, &avatars_model.EmailHash{Hash: emailHash}, unittest.OrderBy("hash")).HashType)
 }
@@ -16,6 +16,7 @@ import (
 	"gitea.dev/modules/log"
 	"gitea.dev/modules/setting"
 	"gitea.dev/modules/storage"
+	"gitea.dev/modules/util"
 )

 // CustomAvatarRelativePath returns user custom avatar relative path.
@@ -25,21 +26,13 @@ func (u *User) CustomAvatarRelativePath() string {

 // GenerateRandomAvatar generates a random avatar for user.
 func GenerateRandomAvatar(ctx context.Context, u *User) error {
-	seed := u.Email
-	if len(seed) == 0 {
-		seed = u.Name
-	}
+	seed := []byte(util.IfZero(u.Email, u.Name))
+	u.Avatar = avatar.HashAvatar(u.ID, seed)

-	img := avatar.RandomImageDefaultSize([]byte(seed))
-
-	u.Avatar = avatars.HashEmail(seed)
-
-	_, err := storage.Avatars.Stat(u.CustomAvatarRelativePath())
-	if err != nil {
-		// If unable to Stat the avatar file (usually it means non-existing), then try to save a new one
-		// Don't share the images so that we can delete them easily
+	// a failed Stat usually means the file is not there yet
+	if _, err := storage.Avatars.Stat(u.CustomAvatarRelativePath()); err != nil {
 		if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error {
-			return png.Encode(w, img)
+			return png.Encode(w, avatar.RandomImageDefaultSize(seed))
 		}); err != nil {
 			return fmt.Errorf("failed to save avatar %s: %w", u.CustomAvatarRelativePath(), err)
 		}