diff --git a/server/router/api/v1/attachment_service.go b/server/router/api/v1/attachment_service.go index c56eec793..5bef72b79 100644 --- a/server/router/api/v1/attachment_service.go +++ b/server/router/api/v1/attachment_service.go @@ -360,7 +360,7 @@ func SaveAttachmentBlob(ctx context.Context, profile *profile.Profile, stores *s } if instanceStorageSetting.StorageType == storepb.InstanceStorageSetting_LOCAL { - filepathTemplate := "assets/{timestamp}_{filename}" + filepathTemplate := "assets/{timestamp}_{uuid}_{filename}" if instanceStorageSetting.FilepathTemplate != "" { filepathTemplate = instanceStorageSetting.FilepathTemplate } @@ -377,6 +377,15 @@ func SaveAttachmentBlob(ctx context.Context, profile *profile.Profile, stores *s if !filepath.IsAbs(osPath) { osPath = filepath.Join(profile.Data, osPath) } + osPath = ensureUniqueLocalAttachmentPath(osPath, create.UID) + internalPath = filepath.ToSlash(osPath) + if !filepath.IsAbs(filepath.FromSlash(internalPath)) { + internalPath, err = filepath.Rel(profile.Data, osPath) + if err != nil { + return errors.Wrap(err, "Failed to get relative path") + } + internalPath = filepath.ToSlash(internalPath) + } dir := filepath.Dir(osPath) if err = os.MkdirAll(dir, os.ModePerm); err != nil { return errors.Wrap(err, "Failed to create directory") @@ -514,6 +523,16 @@ func replaceFilenameWithPathTemplate(path, filename string) string { return path } +func ensureUniqueLocalAttachmentPath(path, uid string) string { + if _, err := os.Stat(path); err != nil { + return path + } + + ext := filepath.Ext(path) + base := strings.TrimSuffix(path, ext) + return base + "_" + uid + ext +} + func validateFilename(filename string) bool { // Reject path traversal attempts and make sure no additional directories are created if !filepath.IsLocal(filename) || strings.ContainsAny(filename, "/\\") { diff --git a/server/router/api/v1/test/attachment_service_test.go b/server/router/api/v1/test/attachment_service_test.go index 2ef523bf1..9df22eb13 100644 --- a/server/router/api/v1/test/attachment_service_test.go +++ b/server/router/api/v1/test/attachment_service_test.go @@ -7,6 +7,9 @@ import ( "github.com/stretchr/testify/require" v1pb "github.com/usememos/memos/proto/gen/api/v1" + storepb "github.com/usememos/memos/proto/gen/store" + apiv1 "github.com/usememos/memos/server/router/api/v1" + "github.com/usememos/memos/store" ) func TestCreateAttachment(t *testing.T) { @@ -56,4 +59,56 @@ func TestCreateAttachment(t *testing.T) { require.NoError(t, err) require.Equal(t, "application/octet-stream", attachment.Type) }) + + t.Run("LocalStorage_PathCollisionUsesUniqueReference", func(t *testing.T) { + _, err := ts.Store.UpsertInstanceSetting(ctx, &storepb.InstanceSetting{ + Key: storepb.InstanceSettingKey_STORAGE, + Value: &storepb.InstanceSetting_StorageSetting{ + StorageSetting: &storepb.InstanceStorageSetting{ + StorageType: storepb.InstanceStorageSetting_LOCAL, + FilepathTemplate: "assets/{filename}", + }, + }, + }) + require.NoError(t, err) + + first, err := ts.Service.CreateAttachment(userCtx, &v1pb.CreateAttachmentRequest{ + Attachment: &v1pb.Attachment{ + Filename: "screenshot.png", + Type: "image/png", + Content: []byte("first-image"), + }, + }) + require.NoError(t, err) + + second, err := ts.Service.CreateAttachment(userCtx, &v1pb.CreateAttachmentRequest{ + Attachment: &v1pb.Attachment{ + Filename: "screenshot.png", + Type: "image/png", + Content: []byte("second-image"), + }, + }) + require.NoError(t, err) + + firstUID, err := apiv1.ExtractAttachmentUIDFromName(first.Name) + require.NoError(t, err) + secondUID, err := apiv1.ExtractAttachmentUIDFromName(second.Name) + require.NoError(t, err) + + firstStoreAttachment, err := ts.Store.GetAttachment(ctx, &store.FindAttachment{UID: &firstUID}) + require.NoError(t, err) + secondStoreAttachment, err := ts.Store.GetAttachment(ctx, &store.FindAttachment{UID: &secondUID}) + require.NoError(t, err) + require.NotNil(t, firstStoreAttachment) + require.NotNil(t, secondStoreAttachment) + + require.NotEqual(t, firstStoreAttachment.Reference, secondStoreAttachment.Reference) + + firstBlob, err := ts.Service.GetAttachmentBlob(firstStoreAttachment) + require.NoError(t, err) + secondBlob, err := ts.Service.GetAttachmentBlob(secondStoreAttachment) + require.NoError(t, err) + require.Equal(t, []byte("first-image"), firstBlob) + require.Equal(t, []byte("second-image"), secondBlob) + }) } diff --git a/store/instance_setting.go b/store/instance_setting.go index 4f24ebeec..d66b31f9b 100644 --- a/store/instance_setting.go +++ b/store/instance_setting.go @@ -219,7 +219,7 @@ func (s *Store) GetInstanceNotificationSetting(ctx context.Context) (*storepb.In const ( defaultInstanceStorageType = storepb.InstanceStorageSetting_LOCAL defaultInstanceUploadSizeLimitMb = 30 - defaultInstanceFilepathTemplate = "assets/{timestamp}_{filename}" + defaultInstanceFilepathTemplate = "assets/{timestamp}_{uuid}_{filename}" ) func (s *Store) GetInstanceStorageSetting(ctx context.Context) (*storepb.InstanceStorageSetting, error) { diff --git a/store/test/instance_setting_test.go b/store/test/instance_setting_test.go index 618396da1..1451234ec 100644 --- a/store/test/instance_setting_test.go +++ b/store/test/instance_setting_test.go @@ -196,7 +196,7 @@ func TestInstanceSettingStorageSetting(t *testing.T) { require.NotNil(t, storageSetting) require.Equal(t, storepb.InstanceStorageSetting_LOCAL, storageSetting.StorageType) require.Equal(t, int64(30), storageSetting.UploadSizeLimitMb) - require.Equal(t, "assets/{timestamp}_{filename}", storageSetting.FilepathTemplate) + require.Equal(t, "assets/{timestamp}_{uuid}_{filename}", storageSetting.FilepathTemplate) // Set custom storage setting _, err = ts.UpsertInstanceSetting(ctx, &storepb.InstanceSetting{