Skip to content

Commit a8eaf43

Browse files
authored
Fix user avatar (go-gitea#33439)
1 parent b6fd874 commit a8eaf43

File tree

9 files changed

+117
-34
lines changed

9 files changed

+117
-34
lines changed

models/user/avatar.go

+12-9
Original file line numberDiff line numberDiff line change
@@ -38,27 +38,30 @@ func GenerateRandomAvatar(ctx context.Context, u *User) error {
3838

3939
u.Avatar = avatars.HashEmail(seed)
4040

41-
// Don't share the images so that we can delete them easily
42-
if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error {
43-
if err := png.Encode(w, img); err != nil {
44-
log.Error("Encode: %v", err)
41+
_, err = storage.Avatars.Stat(u.CustomAvatarRelativePath())
42+
if err != nil {
43+
// If unable to Stat the avatar file (usually it means non-existing), then try to save a new one
44+
// Don't share the images so that we can delete them easily
45+
if err := storage.SaveFrom(storage.Avatars, u.CustomAvatarRelativePath(), func(w io.Writer) error {
46+
if err := png.Encode(w, img); err != nil {
47+
log.Error("Encode: %v", err)
48+
}
49+
return nil
50+
}); err != nil {
51+
return fmt.Errorf("failed to save avatar %s: %w", u.CustomAvatarRelativePath(), err)
4552
}
46-
return err
47-
}); err != nil {
48-
return fmt.Errorf("Failed to create dir %s: %w", u.CustomAvatarRelativePath(), err)
4953
}
5054

5155
if _, err := db.GetEngine(ctx).ID(u.ID).Cols("avatar").Update(u); err != nil {
5256
return err
5357
}
5458

55-
log.Info("New random avatar created: %d", u.ID)
5659
return nil
5760
}
5861

5962
// AvatarLinkWithSize returns a link to the user's avatar with size. size <= 0 means default size
6063
func (u *User) AvatarLinkWithSize(ctx context.Context, size int) string {
61-
if u.IsGhost() {
64+
if u.IsGhost() || u.IsGiteaActions() {
6265
return avatars.DefaultAvatarLink()
6366
}
6467

models/user/avatar_test.go

+40
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,19 @@
44
package user
55

66
import (
7+
"context"
8+
"io"
9+
"strings"
710
"testing"
811

912
"code.gitea.io/gitea/models/db"
13+
"code.gitea.io/gitea/models/unittest"
1014
"code.gitea.io/gitea/modules/setting"
15+
"code.gitea.io/gitea/modules/storage"
1116
"code.gitea.io/gitea/modules/test"
1217

1318
"github.com/stretchr/testify/assert"
19+
"github.com/stretchr/testify/require"
1420
)
1521

1622
func TestUserAvatarLink(t *testing.T) {
@@ -26,3 +32,37 @@ func TestUserAvatarLink(t *testing.T) {
2632
link = u.AvatarLink(db.DefaultContext)
2733
assert.Equal(t, "https://localhost/sub-path/avatars/avatar.png", link)
2834
}
35+
36+
func TestUserAvatarGenerate(t *testing.T) {
37+
assert.NoError(t, unittest.PrepareTestDatabase())
38+
var err error
39+
tmpDir := t.TempDir()
40+
storage.Avatars, err = storage.NewLocalStorage(context.Background(), &setting.Storage{Path: tmpDir})
41+
require.NoError(t, err)
42+
43+
u := unittest.AssertExistsAndLoadBean(t, &User{ID: 2})
44+
45+
// there was no avatar, generate a new one
46+
assert.Empty(t, u.Avatar)
47+
err = GenerateRandomAvatar(db.DefaultContext, u)
48+
require.NoError(t, err)
49+
assert.NotEmpty(t, u.Avatar)
50+
51+
// make sure the generated one exists
52+
oldAvatarPath := u.CustomAvatarRelativePath()
53+
_, err = storage.Avatars.Stat(u.CustomAvatarRelativePath())
54+
require.NoError(t, err)
55+
// and try to change its content
56+
_, err = storage.Avatars.Save(u.CustomAvatarRelativePath(), strings.NewReader("abcd"), 4)
57+
require.NoError(t, err)
58+
59+
// try to generate again
60+
err = GenerateRandomAvatar(db.DefaultContext, u)
61+
require.NoError(t, err)
62+
assert.Equal(t, oldAvatarPath, u.CustomAvatarRelativePath())
63+
f, err := storage.Avatars.Open(u.CustomAvatarRelativePath())
64+
require.NoError(t, err)
65+
defer f.Close()
66+
content, _ := io.ReadAll(f)
67+
assert.Equal(t, "abcd", string(content))
68+
}

models/user/user_system.go

+19-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ func NewGhostUser() *User {
2424
}
2525
}
2626

27+
func IsGhostUserName(name string) bool {
28+
return strings.EqualFold(name, GhostUserName)
29+
}
30+
2731
// IsGhost check if user is fake user for a deleted account
2832
func (u *User) IsGhost() bool {
2933
if u == nil {
@@ -48,6 +52,10 @@ const (
4852
ActionsEmail = "[email protected]"
4953
)
5054

55+
func IsGiteaActionsUserName(name string) bool {
56+
return strings.EqualFold(name, ActionsUserName)
57+
}
58+
5159
// NewActionsUser creates and returns a fake user for running the actions.
5260
func NewActionsUser() *User {
5361
return &User{
@@ -65,6 +73,16 @@ func NewActionsUser() *User {
6573
}
6674
}
6775

68-
func (u *User) IsActions() bool {
76+
func (u *User) IsGiteaActions() bool {
6977
return u != nil && u.ID == ActionsUserID
7078
}
79+
80+
func GetSystemUserByName(name string) *User {
81+
if IsGhostUserName(name) {
82+
return NewGhostUser()
83+
}
84+
if IsGiteaActionsUserName(name) {
85+
return NewActionsUser()
86+
}
87+
return nil
88+
}

models/user/user_system_test.go

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package user
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/models/db"
10+
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
func TestSystemUser(t *testing.T) {
16+
u, err := GetPossibleUserByID(db.DefaultContext, -1)
17+
require.NoError(t, err)
18+
assert.Equal(t, "Ghost", u.Name)
19+
assert.Equal(t, "ghost", u.LowerName)
20+
assert.True(t, u.IsGhost())
21+
assert.True(t, IsGhostUserName("gHost"))
22+
23+
u, err = GetPossibleUserByID(db.DefaultContext, -2)
24+
require.NoError(t, err)
25+
assert.Equal(t, "gitea-actions", u.Name)
26+
assert.Equal(t, "gitea-actions", u.LowerName)
27+
assert.True(t, u.IsGiteaActions())
28+
assert.True(t, IsGiteaActionsUserName("Gitea-actionS"))
29+
30+
_, err = GetPossibleUserByID(db.DefaultContext, -3)
31+
require.Error(t, err)
32+
}

modules/storage/storage.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func Clean(storage ObjectStorage) error {
9393
}
9494

9595
// SaveFrom saves data to the ObjectStorage with path p from the callback
96-
func SaveFrom(objStorage ObjectStorage, p string, callback func(w io.Writer) error) error {
96+
func SaveFrom(objStorage ObjectStorage, path string, callback func(w io.Writer) error) error {
9797
pr, pw := io.Pipe()
9898
defer pr.Close()
9999
go func() {
@@ -103,7 +103,7 @@ func SaveFrom(objStorage ObjectStorage, p string, callback func(w io.Writer) err
103103
}
104104
}()
105105

106-
_, err := objStorage.Save(p, pr, -1)
106+
_, err := objStorage.Save(path, pr, -1)
107107
return err
108108
}
109109

routers/web/user/avatar.go

+9-19
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package user
55

66
import (
7-
"strings"
87
"time"
98

109
"code.gitea.io/gitea/models/avatars"
@@ -21,32 +20,23 @@ func cacheableRedirect(ctx *context.Context, location string) {
2120
ctx.Redirect(location)
2221
}
2322

24-
// AvatarByUserName redirect browser to user avatar of requested size
25-
func AvatarByUserName(ctx *context.Context) {
26-
userName := ctx.PathParam(":username")
27-
size := int(ctx.PathParamInt64(":size"))
28-
29-
var user *user_model.User
30-
if strings.ToLower(userName) != user_model.GhostUserLowerName {
23+
// AvatarByUsernameSize redirect browser to user avatar of requested size
24+
func AvatarByUsernameSize(ctx *context.Context) {
25+
username := ctx.PathParam("username")
26+
user := user_model.GetSystemUserByName(username)
27+
if user == nil {
3128
var err error
32-
if user, err = user_model.GetUserByName(ctx, userName); err != nil {
33-
if user_model.IsErrUserNotExist(err) {
34-
ctx.NotFound("GetUserByName", err)
35-
return
36-
}
37-
ctx.ServerError("Invalid user: "+userName, err)
29+
if user, err = user_model.GetUserByName(ctx, username); err != nil {
30+
ctx.NotFoundOrServerError("GetUserByName", user_model.IsErrUserNotExist, err)
3831
return
3932
}
40-
} else {
41-
user = user_model.NewGhostUser()
4233
}
43-
44-
cacheableRedirect(ctx, user.AvatarLinkWithSize(ctx, size))
34+
cacheableRedirect(ctx, user.AvatarLinkWithSize(ctx, int(ctx.PathParamInt64("size"))))
4535
}
4636

4737
// AvatarByEmailHash redirects the browser to the email avatar link
4838
func AvatarByEmailHash(ctx *context.Context) {
49-
hash := ctx.PathParam(":hash")
39+
hash := ctx.PathParam("hash")
5040
email, err := avatars.GetEmailForHash(ctx, hash)
5141
if err != nil {
5242
ctx.ServerError("invalid avatar hash: "+hash, err)

routers/web/user/home.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ func UsernameSubRoute(ctx *context.Context) {
734734
switch {
735735
case strings.HasSuffix(username, ".png"):
736736
if reloadParam(".png") {
737-
AvatarByUserName(ctx)
737+
AvatarByUsernameSize(ctx)
738738
}
739739
case strings.HasSuffix(username, ".keys"):
740740
if reloadParam(".keys") {

routers/web/web.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,7 @@ func registerRoutes(m *web.Router) {
681681
m.Get("/activate", auth.Activate)
682682
m.Post("/activate", auth.ActivatePost)
683683
m.Any("/activate_email", auth.ActivateEmail)
684-
m.Get("/avatar/{username}/{size}", user.AvatarByUserName)
684+
m.Get("/avatar/{username}/{size}", user.AvatarByUsernameSize)
685685
m.Get("/recover_account", auth.ResetPasswd)
686686
m.Post("/recover_account", auth.ResetPasswdPost)
687687
m.Get("/forgot_password", auth.ForgotPasswd)

services/actions/notifier_helper.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func (input *notifyInput) Notify(ctx context.Context) {
117117

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

0 commit comments

Comments
 (0)