Fix user avatar (#33439)

Author: wxiaoguang

models/user/avatar.go

@@ -38,27 +38,30 @@ func GenerateRandomAvatar(ctx context.Context, u *User) error {
 
 	u.Avatar = avatars.HashEmail(seed)
 
-	// Don't share the images so that we can delete them easily
-	if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error {
-		if err := png.Encode(w, img); err != nil {
-			log.Error("Encode: %v", err)
+	_, 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
+		if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error {
+			if err := png.Encode(w, img); err != nil {
+				log.Error("Encode: %v", err)
+			}
+			return nil
+		}); err != nil {
+			return fmt.Errorf("failed to save avatar %s: %w", u.CustomAvatarRelativePath(), err)
 		}
-		return err
-	}); err != nil {
-		return fmt.Errorf("Failed to create dir %s: %w", u.CustomAvatarRelativePath(), err)
 	}
 
 	if _, err := db.GetEngine(ctx).ID(u.ID).Cols("avatar").Update(u); err != nil {
 		return err
 	}
 
-	log.Info("New random avatar created: %d", u.ID)
 	return nil
 }
 
 // AvatarLinkWithSize returns a link to the user's avatar with size. size <= 0 means default size
 func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string {
-	if u.IsGhost() {
+	if u.IsGhost() || u.IsGiteaActions() {
 		return avatars.DefaultAvatarLink()
 	}
 

models/user/avatar_test.go

@@ -4,13 +4,19 @@
 package user
 
 import (
+	"context"
+	"io"
+	"strings"
 	"testing"
 
 	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/test"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUserAvatarLink(t *testing.T) {
@@ -26,3 +32,37 @@ func TestUserAvatarLink(t *testing.T) {
 	link = u.AvatarLink(db.DefaultContext)
 	assert.Equal(t, "https://localhost/sub-path/avatars/avatar.png", link)
 }
+
+func TestUserAvatarGenerate(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	var err error
+	tmpDir := t.TempDir()
+	storage.Avatars, err = storage.NewLocalStorage(context.Background(), &setting.Storage{Path: tmpDir})
+	require.NoError(t, err)
+
+	u := unittest.AssertExistsAndLoadBean(t, &User{ID: 2})
+
+	// there was no avatar, generate a new one
+	assert.Empty(t, u.Avatar)
+	err = GenerateRandomAvatar(db.DefaultContext, u)
+	require.NoError(t, err)
+	assert.NotEmpty(t, u.Avatar)
+
+	// make sure the generated one exists
+	oldAvatarPath := u.CustomAvatarRelativePath()
+	_, err = storage.Avatars.Stat(u.CustomAvatarRelativePath())
+	require.NoError(t, err)
+	// and try to change its content
+	_, err = storage.Avatars.Save(u.CustomAvatarRelativePath(), strings.NewReader("abcd"), 4)
+	require.NoError(t, err)
+
+	// try to generate again
+	err = GenerateRandomAvatar(db.DefaultContext, u)
+	require.NoError(t, err)
+	assert.Equal(t, oldAvatarPath, u.CustomAvatarRelativePath())
+	f, err := storage.Avatars.Open(u.CustomAvatarRelativePath())
+	require.NoError(t, err)
+	defer f.Close()
+	content, _ := io.ReadAll(f)
+	assert.Equal(t, "abcd", string(content))
+}

models/user/user_system.go

@@ -24,6 +24,10 @@ func NewGhostUser() *User {
 	}
 }
 
+func IsGhostUserName(name string) bool {
+	return strings.EqualFold(name, GhostUserName)
+}
+
 // IsGhost check if user is fake user for a deleted account
 func (u *User) IsGhost() bool {
 	if u == nil {
@@ -48,6 +52,10 @@ const (
 	ActionsEmail    = "[email protected]"
 )
 
+func IsGiteaActionsUserName(name string) bool {
+	return strings.EqualFold(name, ActionsUserName)
+}
+
 // NewActionsUser creates and returns a fake user for running the actions.
 func NewActionsUser() *User {
 	return &User{
@@ -65,6 +73,16 @@ func NewActionsUser() *User {
 	}
 }
 
-func (u *User) IsActions() bool {
+func (u *User) IsGiteaActions() bool {
 	return u != nil && u.ID == ActionsUserID
 }
+
+func GetSystemUserByName(name string) *User {
+	if IsGhostUserName(name) {
+		return NewGhostUser()
+	}
+	if IsGiteaActionsUserName(name) {
+		return NewActionsUser()
+	}
+	return nil
+}

models/user/user_system_test.go

@@ -0,0 +1,32 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package user
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSystemUser(t *testing.T) {
+	u, err := GetPossibleUserByID(db.DefaultContext, -1)
+	require.NoError(t, err)
+	assert.Equal(t, "Ghost", u.Name)
+	assert.Equal(t, "ghost", u.LowerName)
+	assert.True(t, u.IsGhost())
+	assert.True(t, IsGhostUserName("gHost"))
+
+	u, err = GetPossibleUserByID(db.DefaultContext, -2)
+	require.NoError(t, err)
+	assert.Equal(t, "gitea-actions", u.Name)
+	assert.Equal(t, "gitea-actions", u.LowerName)
+	assert.True(t, u.IsGiteaActions())
+	assert.True(t, IsGiteaActionsUserName("Gitea-actionS"))
+
+	_, err = GetPossibleUserByID(db.DefaultContext, -3)
+	require.Error(t, err)
+}

modules/storage/storage.go

@@ -93,7 +93,7 @@ func Clean(storage ObjectStorage) error {
 }
 
 // SaveFrom saves data to the ObjectStorage with path p from the callback
-func SaveFrom(objStorage ObjectStorage, p string, callback func(w io.Writer) error) error {
+func SaveFrom(objStorage ObjectStorage, path string, callback func(w io.Writer) error) error {
 	pr, pw := io.Pipe()
 	defer pr.Close()
 	go func() {
@@ -103,7 +103,7 @@ func SaveFrom(objStorage ObjectStorage, p string, callback func(w io.Writer) err
 		}
 	}()
 
-	_, err := objStorage.Save(p, pr, -1)
+	_, err := objStorage.Save(path, pr, -1)
 	return err
 }
 

routers/web/user/avatar.go

@@ -4,7 +4,6 @@
 package user
 
 import (
-	"strings"
 	"time"
 
 	"code.gitea.io/gitea/models/avatars"
@@ -21,32 +20,23 @@ func cacheableRedirect(ctx *context.Context, location string) {
 	ctx.Redirect(location)
 }
 
-// AvatarByUserName redirect browser to user avatar of requested size
-func AvatarByUserName(ctx *context.Context) {
-	userName := ctx.PathParam(":username")
-	size := int(ctx.PathParamInt64(":size"))
-
-	var user *user_model.User
-	if strings.ToLower(userName) != user_model.GhostUserLowerName {
+// AvatarByUsernameSize redirect browser to user avatar of requested size
+func AvatarByUsernameSize(ctx *context.Context) {
+	username := ctx.PathParam("username")
+	user := user_model.GetSystemUserByName(username)
+	if user == nil {
 		var err error
-		if user, err = user_model.GetUserByName(ctx, userName); err != nil {
-			if user_model.IsErrUserNotExist(err) {
-				ctx.NotFound("GetUserByName", err)
-				return
-			}
-			ctx.ServerError("Invalid user: "+userName, err)
+		if user, err = user_model.GetUserByName(ctx, username); err != nil {
+			ctx.NotFoundOrServerError("GetUserByName", user_model.IsErrUserNotExist, err)
 			return
 		}
-	} else {
-		user = user_model.NewGhostUser()
 	}
-
-	cacheableRedirect(ctx, user.AvatarLinkWithSize(ctx, size))
+	cacheableRedirect(ctx, user.AvatarLinkWithSize(ctx, int(ctx.PathParamInt64("size"))))
 }
 
 // AvatarByEmailHash redirects the browser to the email avatar link
 func AvatarByEmailHash(ctx *context.Context) {
-	hash := ctx.PathParam(":hash")
+	hash := ctx.PathParam("hash")
 	email, err := avatars.GetEmailForHash(ctx, hash)
 	if err != nil {
 		ctx.ServerError("invalid avatar hash: "+hash, err)

routers/web/user/home.go

@@ -734,7 +734,7 @@ func UsernameSubRoute(ctx *context.Context) {
 	switch {
 	case strings.HasSuffix(username, ".png"):
 		if reloadParam(".png") {
-			AvatarByUserName(ctx)
+			AvatarByUsernameSize(ctx)
 		}
 	case strings.HasSuffix(username, ".keys"):
 		if reloadParam(".keys") {

routers/web/web.go

@@ -681,7 +681,7 @@ func registerRoutes(m *web.Router) {
 		m.Get("/activate", auth.Activate)
 		m.Post("/activate", auth.ActivatePost)
 		m.Any("/activate_email", auth.ActivateEmail)
-		m.Get("/avatar/{username}/{size}", user.AvatarByUserName)
+		m.Get("/avatar/{username}/{size}", user.AvatarByUsernameSize)
 		m.Get("/recover_account", auth.ResetPasswd)
 		m.Post("/recover_account", auth.ResetPasswdPost)
 		m.Get("/forgot_password", auth.ForgotPasswd)

services/actions/notifier_helper.go

@@ -117,7 +117,7 @@ func (input *notifyInput) Notify(ctx context.Context) {
 
 func notify(ctx context.Context, input *notifyInput) error {
 	shouldDetectSchedules := input.Event == webhook_module.HookEventPush && input.Ref.BranchName() == input.Repo.DefaultBranch
-	if input.Doer.IsActions() {
+	if input.Doer.IsGiteaActions() {
 		// avoiding triggering cyclically, for example:
 		// a comment of an issue will trigger the runner to add a new comment as reply,
 		// and the new comment will trigger the runner again.