Skip to content

Commit 71900a4

Browse files
Copilotkolaente
andcommitted
Fix type assertion error in avatar upload provider with logging and graceful recovery
Co-authored-by: kolaente <13721712+kolaente@users.noreply.github.com>
1 parent fb6f0b7 commit 71900a4

2 files changed

Lines changed: 114 additions & 1 deletion

File tree

pkg/modules/avatar/upload/upload.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package upload
1818

1919
import (
2020
"bytes"
21+
"fmt"
2122
"image"
2223
"image/png"
2324
"io"
@@ -56,6 +57,11 @@ func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType
5657
result, err := keyvalue.Remember(cacheKey, func() (any, error) {
5758
log.Debugf("Uploaded avatar for user %d and size %d not cached, resizing and caching.", u.ID, size)
5859

60+
// Check if user has an avatar file ID
61+
if u.AvatarFileID == 0 {
62+
return nil, fmt.Errorf("user %d has no avatar file", u.ID)
63+
}
64+
5965
// If we get this far, the avatar is either not cached at all or not in this size
6066
f := &files.File{ID: u.AvatarFileID}
6167
if err := f.LoadFileByID(); err != nil {
@@ -92,7 +98,20 @@ func (p *Provider) GetAvatar(u *user.User, size int64) (avatar []byte, mimeType
9298
return nil, "", err
9399
}
94100

95-
cachedAvatar := result.(CachedAvatar)
101+
// Safe type assertion to handle cases where cached data might be corrupted or in legacy format
102+
cachedAvatar, ok := result.(CachedAvatar)
103+
if !ok {
104+
// Log the type mismatch with the actual stored value for debugging
105+
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)
106+
107+
// Clear the invalid cache entry
108+
if err := keyvalue.Del(cacheKey); err != nil {
109+
log.Errorf("Failed to clear invalid cache entry for key %s: %v", cacheKey, err)
110+
}
111+
112+
// Regenerate the avatar by calling the function again (without the corrupted cache)
113+
return p.GetAvatar(u, size)
114+
}
96115

97116
return cachedAvatar.Content, cachedAvatar.MimeType, nil
98117
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// Vikunja is a to-do list application to facilitate your life.
2+
// Copyright 2018-present Vikunja and contributors. All rights reserved.
3+
//
4+
// This program is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU Affero General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// This program is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU Affero General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU Affero General Public License
15+
// along with this program. If not, see <https://www.gnu.org/licenses/>.
16+
17+
package upload
18+
19+
import (
20+
"os"
21+
"testing"
22+
23+
"code.vikunja.io/api/pkg/log"
24+
"code.vikunja.io/api/pkg/modules/keyvalue"
25+
"code.vikunja.io/api/pkg/user"
26+
"github.com/stretchr/testify/assert"
27+
"github.com/stretchr/testify/require"
28+
)
29+
30+
// TestMain initializes the test environment
31+
func TestMain(m *testing.M) {
32+
// Initialize logger for tests
33+
log.InitLogger()
34+
35+
os.Exit(m.Run())
36+
}
37+
38+
func TestGetAvatar_HandlesInvalidCachedType(t *testing.T) {
39+
// Initialize storage for testing
40+
keyvalue.InitStorage()
41+
42+
provider := &Provider{}
43+
44+
// Create a test user
45+
testUser := &user.User{
46+
ID: 999999, // Use a high ID to avoid conflicts
47+
AvatarFileID: 0, // No avatar file ID to avoid actual file operations
48+
}
49+
50+
// Simulate corrupted cached data by storing a string instead of CachedAvatar
51+
cacheKey := CacheKeyPrefix + "999999_64"
52+
err := keyvalue.Put(cacheKey, "corrupted_string_data")
53+
require.NoError(t, err)
54+
55+
// This should not panic but should handle the type assertion gracefully
56+
// and return an error (since there's no actual avatar file)
57+
avatar, mimeType, err := provider.GetAvatar(testUser, 64)
58+
59+
// The function should handle the type assertion failure gracefully
60+
// and attempt to regenerate the avatar (which will fail due to no file)
61+
assert.Error(t, err)
62+
assert.Nil(t, avatar)
63+
assert.Empty(t, mimeType)
64+
}
65+
66+
func TestGetAvatar_HandlesValidCachedType(t *testing.T) {
67+
// Initialize storage for testing
68+
keyvalue.InitStorage()
69+
70+
provider := &Provider{}
71+
72+
// Create a test user
73+
testUser := &user.User{
74+
ID: 888888, // Use a different ID to avoid cache conflicts
75+
AvatarFileID: 0,
76+
}
77+
78+
// Store a valid cached avatar
79+
cacheKey := CacheKeyPrefix + "888888_32"
80+
validCachedAvatar := CachedAvatar{
81+
Content: []byte("fake_image_data"),
82+
MimeType: "image/png",
83+
}
84+
err := keyvalue.Put(cacheKey, validCachedAvatar)
85+
require.NoError(t, err)
86+
87+
// This should work correctly with the valid cached data
88+
avatar, mimeType, err := provider.GetAvatar(testUser, 32)
89+
90+
// Should return the cached data successfully
91+
assert.NoError(t, err)
92+
assert.Equal(t, []byte("fake_image_data"), avatar)
93+
assert.Equal(t, "image/png", mimeType)
94+
}

0 commit comments

Comments
 (0)