From e3a5b8321240e489bb9048ecf2caccf73c0abc36 Mon Sep 17 00:00:00 2001 From: Masudur Rahman Date: Sat, 28 Dec 2019 00:27:59 +0600 Subject: [PATCH] Fix user avatar name (#8547) Migrate avatar names to include user ID and the md5 hash. --- models/migrations/migrations.go | 2 + models/migrations/v115.go | 112 ++++++++++++++++++++++++++++++++ models/user.go | 6 +- 3 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 models/migrations/v115.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 923b5f5759..00d836695f 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -284,6 +284,8 @@ var migrations = []Migration{ NewMigration("new feature: change target branch of pull requests", featureChangeTargetBranch), // v114 -> v115 NewMigration("Remove authentication credentials from stored URL", sanitizeOriginalURL), + // v115 -> v116 + NewMigration("add user_id prefix to existing user avatar name", renameExistingUserAvatarName), } // Migrate database to current version diff --git a/models/migrations/v115.go b/models/migrations/v115.go new file mode 100644 index 0000000000..f1603b7976 --- /dev/null +++ b/models/migrations/v115.go @@ -0,0 +1,112 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "crypto/md5" + "fmt" + "io/ioutil" + "os" + "path/filepath" + + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + + "xorm.io/xorm" +) + +func renameExistingUserAvatarName(x *xorm.Engine) error { + sess := x.NewSession() + defer sess.Close() + + type User struct { + ID int64 `xorm:"pk autoincr"` + LowerName string `xorm:"UNIQUE NOT NULL"` + Avatar string + } + deleteList := make(map[string]struct{}) + start := 0 + for { + if err := sess.Begin(); err != nil { + return fmt.Errorf("session.Begin: %v", err) + } + users := make([]*User, 0, 50) + if err := sess.Table("user").Asc("id").Limit(50, start).Find(&users); err != nil { + return fmt.Errorf("select users from id [%d]: %v", start, err) + } + if len(users) == 0 { + _ = sess.Rollback() + break + } + + log.Info("select users [%d - %d]", start, start+len(users)) + start += 50 + + for _, user := range users { + oldAvatar := user.Avatar + + if _, err := os.Stat(filepath.Join(setting.AvatarUploadPath, oldAvatar)); err != nil { + log.Warn("[user: %s] os.Stat: %v", user.LowerName, err) + // avatar doesn't exist in the storage + // no need to move avatar and update database + // we can just skip this + continue + } + + newAvatar, err := copyOldAvatarToNewLocation(user.ID, oldAvatar) + if err != nil { + _ = sess.Rollback() + return fmt.Errorf("[user: %s] %v", user.LowerName, err) + } else if newAvatar == oldAvatar { + continue + } + + user.Avatar = newAvatar + if _, err := sess.ID(user.ID).Cols("avatar").Update(user); err != nil { + _ = sess.Rollback() + return fmt.Errorf("[user: %s] user table update: %v", user.LowerName, err) + } + + deleteList[filepath.Join(setting.AvatarUploadPath, oldAvatar)] = struct{}{} + } + if err := sess.Commit(); err != nil { + _ = sess.Rollback() + return fmt.Errorf("commit session: %v", err) + } + } + + for file := range deleteList { + if err := os.Remove(file); err != nil { + log.Warn("os.Remove: %v", err) + } + } + return nil +} + +// copyOldAvatarToNewLocation copies oldAvatar to newAvatarLocation +// and returns newAvatar location +func copyOldAvatarToNewLocation(userID int64, oldAvatar string) (string, error) { + fr, err := os.Open(filepath.Join(setting.AvatarUploadPath, oldAvatar)) + if err != nil { + return "", fmt.Errorf("os.Open: %v", err) + } + defer fr.Close() + + data, err := ioutil.ReadAll(fr) + if err != nil { + return "", fmt.Errorf("ioutil.ReadAll: %v", err) + } + + newAvatar := fmt.Sprintf("%x", md5.Sum([]byte(fmt.Sprintf("%d-%x", userID, md5.Sum(data))))) + if newAvatar == oldAvatar { + return newAvatar, nil + } + + if err := ioutil.WriteFile(filepath.Join(setting.AvatarUploadPath, newAvatar), data, 0666); err != nil { + return "", fmt.Errorf("ioutil.WriteFile: %v", err) + } + + return newAvatar, nil +} diff --git a/models/user.go b/models/user.go index 0454158de6..e832c2ed51 100644 --- a/models/user.go +++ b/models/user.go @@ -521,7 +521,11 @@ func (u *User) UploadAvatar(data []byte) error { } u.UseCustomAvatar = true - u.Avatar = fmt.Sprintf("%x", md5.Sum(data)) + // Different users can upload same image as avatar + // If we prefix it with u.ID, it will be separated + // Otherwise, if any of the users delete his avatar + // Other users will lose their avatars too. + u.Avatar = fmt.Sprintf("%x", md5.Sum([]byte(fmt.Sprintf("%d-%x", u.ID, md5.Sum(data))))) if err = updateUser(sess, u); err != nil { return fmt.Errorf("updateUser: %v", err) }