diff --git a/pkg/modules/avatar/initials/initials.go b/pkg/modules/avatar/initials/initials.go index dffe499f0..d9e0a905d 100644 --- a/pkg/modules/avatar/initials/initials.go +++ b/pkg/modules/avatar/initials/initials.go @@ -18,6 +18,7 @@ package initials import ( "bytes" + "fmt" "image" "image/color" "image/draw" @@ -133,6 +134,15 @@ func getCacheKey(prefix string, keys ...int64) string { } func getAvatarForUser(u *user.User) (fullSizeAvatar *image.RGBA64, err error) { + return getAvatarForUserWithDepth(u, 0) +} + +func getAvatarForUserWithDepth(u *user.User, recursionDepth int) (fullSizeAvatar *image.RGBA64, err error) { + // Prevent infinite recursion - max 3 attempts + if recursionDepth >= 3 { + return nil, fmt.Errorf("maximum recursion depth reached while generating avatar for user %d", u.ID) + } + cacheKey := getCacheKey("full", u.ID) result, err := keyvalue.Remember(cacheKey, func() (any, error) { @@ -155,7 +165,20 @@ func getAvatarForUser(u *user.User) (fullSizeAvatar *image.RGBA64, err error) { return nil, err } - aa := result.(image.RGBA64) + // Safe type assertion to handle cases where cached data might be corrupted or in legacy format + aa, ok := result.(image.RGBA64) + if !ok { + // Log the type mismatch with the actual stored value for debugging + log.Errorf("Invalid cached image type for user %d. Expected image.RGBA64, got %T with value: %+v. Clearing cache and regenerating.", u.ID, result, result) + + // Clear the invalid cache entry + if err := keyvalue.Del(cacheKey); err != nil { + log.Errorf("Failed to clear invalid cache entry for key %s: %v", cacheKey, err) + } + + // Regenerate the avatar by calling the function again (without the corrupted cache) + return getAvatarForUserWithDepth(u, recursionDepth+1) + } return &aa, nil } @@ -168,6 +191,15 @@ type CachedAvatar struct { // GetAvatar returns an initials avatar for a user func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType string, err error) { + return p.getAvatarWithDepth(u, size, 0) +} + +func (p *Provider) getAvatarWithDepth(u *user.User, size int64, recursionDepth int) (avatar []byte, mimeType string, err error) { + // Prevent infinite recursion - max 3 attempts + if recursionDepth >= 3 { + return nil, "", fmt.Errorf("maximum recursion depth reached while generating avatar for user %d, size %d", u.ID, size) + } + cacheKey := getCacheKey("resized", u.ID, size) result, err := keyvalue.Remember(cacheKey, func() (any, error) { @@ -197,6 +229,20 @@ func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType return nil, "", err } - cachedAvatar := result.(CachedAvatar) + // Safe type assertion to handle cases where cached data might be corrupted or in legacy format + cachedAvatar, ok := result.(CachedAvatar) + if !ok { + // Log the type mismatch with the actual stored value for debugging + log.Errorf("Invalid cached avatar type for user %d, size %d. Expected CachedAvatar, got %T with value: %+v. Clearing cache and regenerating.", u.ID, size, result, result) + + // Clear the invalid cache entry + if err := keyvalue.Del(cacheKey); err != nil { + log.Errorf("Failed to clear invalid cache entry for key %s: %v", cacheKey, err) + } + + // Regenerate the avatar by calling the function again (without the corrupted cache) + return p.getAvatarWithDepth(u, size, recursionDepth+1) + } + return cachedAvatar.Content, cachedAvatar.MimeType, nil } diff --git a/pkg/modules/avatar/initials/initials_test.go b/pkg/modules/avatar/initials/initials_test.go new file mode 100644 index 000000000..8a81b1308 --- /dev/null +++ b/pkg/modules/avatar/initials/initials_test.go @@ -0,0 +1,179 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package initials + +import ( + "image" + "os" + "testing" + + "code.vikunja.io/api/pkg/log" + "code.vikunja.io/api/pkg/modules/keyvalue" + "code.vikunja.io/api/pkg/user" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMain initializes the test environment +func TestMain(m *testing.M) { + // Initialize logger for tests + log.InitLogger() + + os.Exit(m.Run()) +} + +func TestGetAvatar(t *testing.T) { + // Initialize storage for testing + keyvalue.InitStorage() + + t.Run("handles invalid cached type", func(t *testing.T) { + provider := &Provider{} + + // Create a test user + testUser := &user.User{ + ID: 999999, // Use a high ID to avoid conflicts + Name: "Test User", + Username: "testuser", + } + + // Simulate corrupted cached data by storing a string instead of CachedAvatar + cacheKey := getCacheKey("resized", testUser.ID, 64) + err := keyvalue.Put(cacheKey, "corrupted_string_data") + require.NoError(t, err) + + // This should not panic but should handle the type assertion gracefully + // and regenerate the avatar + avatar, mimeType, err := provider.GetAvatar(testUser, 64) + + // The function should handle the type assertion failure gracefully + // and regenerate the avatar successfully + require.NoError(t, err) + assert.NotNil(t, avatar) + assert.Equal(t, "image/png", mimeType) + assert.NotEmpty(t, avatar, "Avatar should contain image data") + }) + + t.Run("handles valid cached type", func(t *testing.T) { + provider := &Provider{} + + // Create a test user + testUser := &user.User{ + ID: 888888, // Use a different ID to avoid cache conflicts + Name: "Valid User", + Username: "validuser", + } + + // Store a valid cached avatar + cacheKey := getCacheKey("resized", testUser.ID, 32) + validCachedAvatar := CachedAvatar{ + Content: []byte("fake_image_data"), + MimeType: "image/png", + } + err := keyvalue.Put(cacheKey, validCachedAvatar) + require.NoError(t, err) + + // This should work correctly with the valid cached data + avatar, mimeType, err := provider.GetAvatar(testUser, 32) + + // Should return the cached data successfully + require.NoError(t, err) + assert.Equal(t, []byte("fake_image_data"), avatar) + assert.Equal(t, "image/png", mimeType) + }) + + t.Run("generates valid initials", func(t *testing.T) { + provider := &Provider{} + + // Test with name + testUser1 := &user.User{ + ID: 555555, + Name: "John Doe", + Username: "johndoe", + } + + avatar1, mimeType1, err1 := provider.GetAvatar(testUser1, 128) + require.NoError(t, err1) + assert.NotNil(t, avatar1) + assert.Equal(t, "image/png", mimeType1) + assert.NotEmpty(t, avatar1) + + // Test with username when name is empty + testUser2 := &user.User{ + ID: 444444, + Name: "", // Empty name + Username: "jane_smith", + } + + avatar2, mimeType2, err2 := provider.GetAvatar(testUser2, 128) + require.NoError(t, err2) + assert.NotNil(t, avatar2) + assert.Equal(t, "image/png", mimeType2) + assert.NotEmpty(t, avatar2) + }) +} + +func TestGetAvatarForUser(t *testing.T) { + // Initialize storage for testing + keyvalue.InitStorage() + + t.Run("handles invalid cached type", func(t *testing.T) { + // Create a test user + testUser := &user.User{ + ID: 777777, // Use another unique ID + Name: "Full Size Test User", + Username: "fullsizeuser", + } + + // Simulate corrupted cached data by storing a string instead of image.RGBA64 + cacheKey := getCacheKey("full", testUser.ID) + err := keyvalue.Put(cacheKey, "corrupted_image_data") + require.NoError(t, err) + + // This should not panic but should handle the type assertion gracefully + // and regenerate the full size avatar + fullAvatar, err := getAvatarForUser(testUser) + + // The function should handle the type assertion failure gracefully + // and generate a new avatar successfully + require.NoError(t, err) + assert.NotNil(t, fullAvatar) + assert.IsType(t, &image.RGBA64{}, fullAvatar) + }) + + t.Run("handles valid cached type", func(t *testing.T) { + // Create a test user + testUser := &user.User{ + ID: 666666, // Use another unique ID + Name: "Valid Full Size User", + Username: "validfulluser", + } + + // Create a valid image.RGBA64 for caching + validImage := image.NewRGBA64(image.Rect(0, 0, 64, 64)) + cacheKey := getCacheKey("full", testUser.ID) + err := keyvalue.Put(cacheKey, *validImage) + require.NoError(t, err) + + // This should work correctly with the valid cached data + fullAvatar, err := getAvatarForUser(testUser) + + // Should return the cached image successfully + require.NoError(t, err) + assert.NotNil(t, fullAvatar) + assert.IsType(t, &image.RGBA64{}, fullAvatar) + }) +} diff --git a/pkg/modules/avatar/upload/upload.go b/pkg/modules/avatar/upload/upload.go index 0b0e1cef5..ddf5b52be 100644 --- a/pkg/modules/avatar/upload/upload.go +++ b/pkg/modules/avatar/upload/upload.go @@ -18,6 +18,7 @@ package upload import ( "bytes" + "fmt" "image" "image/png" "io" @@ -51,11 +52,25 @@ type CachedAvatar struct { // GetAvatar returns an uploaded user avatar func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType string, err error) { + return p.getAvatarWithDepth(u, size, 0) +} + +func (p *Provider) getAvatarWithDepth(u *user.User, size int64, recursionDepth int) (avatar []byte, mimeType string, err error) { + // Prevent infinite recursion - max 3 attempts + if recursionDepth >= 3 { + return nil, "", fmt.Errorf("maximum recursion depth reached while generating avatar for user %d, size %d", u.ID, size) + } + cacheKey := CacheKeyPrefix + strconv.Itoa(int(u.ID)) + "_" + strconv.FormatInt(size, 10) result, err := keyvalue.Remember(cacheKey, func() (any, error) { log.Debugf("Uploaded avatar for user %d and size %d not cached, resizing and caching.", u.ID, size) + // Check if user has an avatar file ID + if u.AvatarFileID == 0 { + return nil, fmt.Errorf("user %d has no avatar file", u.ID) + } + // If we get this far, the avatar is either not cached at all or not in this size f := &files.File{ID: u.AvatarFileID} if err := f.LoadFileByID(); err != nil { @@ -92,7 +107,20 @@ func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType return nil, "", err } - cachedAvatar := result.(CachedAvatar) + // Safe type assertion to handle cases where cached data might be corrupted or in legacy format + cachedAvatar, ok := result.(CachedAvatar) + if !ok { + // Log the type mismatch with the actual stored value for debugging + log.Errorf("Invalid cached avatar type for user %d, size %d. Expected CachedAvatar, got %T with value: %+v. Clearing cache and regenerating.", u.ID, size, result, result) + + // Clear the invalid cache entry + if err := keyvalue.Del(cacheKey); err != nil { + log.Errorf("Failed to clear invalid cache entry for key %s: %v", cacheKey, err) + } + + // Regenerate the avatar by calling the function again (without the corrupted cache) + return p.getAvatarWithDepth(u, size, recursionDepth+1) + } return cachedAvatar.Content, cachedAvatar.MimeType, nil } diff --git a/pkg/modules/avatar/upload/upload_test.go b/pkg/modules/avatar/upload/upload_test.go new file mode 100644 index 000000000..50efb16d3 --- /dev/null +++ b/pkg/modules/avatar/upload/upload_test.go @@ -0,0 +1,93 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package upload + +import ( + "os" + "testing" + + "code.vikunja.io/api/pkg/log" + "code.vikunja.io/api/pkg/modules/keyvalue" + "code.vikunja.io/api/pkg/user" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMain initializes the test environment +func TestMain(m *testing.M) { + // Initialize logger for tests + log.InitLogger() + + os.Exit(m.Run()) +} + +func TestGetAvatar(t *testing.T) { + // Initialize storage for testing + keyvalue.InitStorage() + + t.Run("handles invalid cached type", func(t *testing.T) { + provider := &Provider{} + + // Create a test user + testUser := &user.User{ + ID: 999999, // Use a high ID to avoid conflicts + AvatarFileID: 0, // No avatar file ID to avoid actual file operations + } + + // Simulate corrupted cached data by storing a string instead of CachedAvatar + cacheKey := CacheKeyPrefix + "999999_64" + err := keyvalue.Put(cacheKey, "corrupted_string_data") + require.NoError(t, err) + + // This should not panic but should handle the type assertion gracefully + // and return an error (since there's no actual avatar file) + avatar, mimeType, err := provider.GetAvatar(testUser, 64) + + // The function should handle the type assertion failure gracefully + // and attempt to regenerate the avatar (which will fail due to no file) + require.Error(t, err) + assert.Nil(t, avatar) + assert.Empty(t, mimeType) + }) + + t.Run("handles valid cached type", func(t *testing.T) { + provider := &Provider{} + + // Create a test user + testUser := &user.User{ + ID: 888888, // Use a different ID to avoid cache conflicts + AvatarFileID: 0, + } + + // Store a valid cached avatar + cacheKey := CacheKeyPrefix + "888888_32" + validCachedAvatar := CachedAvatar{ + Content: []byte("fake_image_data"), + MimeType: "image/png", + } + err := keyvalue.Put(cacheKey, validCachedAvatar) + require.NoError(t, err) + + // This should work correctly with the valid cached data + avatar, mimeType, err := provider.GetAvatar(testUser, 32) + + // Should return the cached data successfully + require.NoError(t, err) + assert.Equal(t, []byte("fake_image_data"), avatar) + assert.Equal(t, "image/png", mimeType) + }) +}