Fix user avatar name (#8547)

Migrate avatar names to include user ID and the md5 hash.

diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go
index 923b5f5759c107a510927ee1a1bdbd0439916dee..00d836695f190e32f97968002d2dae88b94c8cff 100644 (file)
--- 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 (file)
index 0000000..f1603b7
--- /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 0454158de6863bf40254997000aac2d13ee03a4f..e832c2ed519b838fa9ef6d8b2e33aeb7e263ad28 100644 (file)
--- 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)
        }