Unify hashing for avatar (#22289)

- Unify the hashing code for repository and user avatars into a
function.
- Use a sane hash function instead of MD5.
- Only require hashing once instead of twice(w.r.t. hashing for user
avatar).
- Improve the comment for the hashing code of why it works.

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Yarden Shoham <hrsi88@gmail.com>
This commit is contained in:
Gusted 2023-01-02 22:46:39 +01:00 committed by GitHub
parent fcd6ceef2b
commit 96797fed31
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 32 additions and 11 deletions

28
modules/avatar/hash.go Normal file
View File

@ -0,0 +1,28 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package avatar
import (
"crypto/sha256"
"encoding/hex"
"strconv"
)
// HashAvatar will generate a unique string, which ensures that when there's a
// different unique ID while the data is the same, it will generate a different
// output. It will generate the output according to:
// HEX(HASH(uniqueID || - || data))
// The hash being used is SHA256.
// The sole purpose of the unique ID is to generate a distinct hash Such that
// two unique IDs with the same data will have a different hash output.
// The "-" byte is important to ensure that data cannot be modified such that
// the first byte is a number, which could lead to a "collision" with the hash
// of another unique ID.
func HashAvatar(uniqueID int64, data []byte) string {
h := sha256.New()
h.Write([]byte(strconv.FormatInt(uniqueID, 10)))
h.Write([]byte{'-'})
h.Write(data)
return hex.EncodeToString(h.Sum(nil))
}

View File

@ -5,7 +5,6 @@ package repository
import ( import (
"context" "context"
"crypto/md5"
"fmt" "fmt"
"image/png" "image/png"
"io" "io"
@ -27,7 +26,7 @@ func UploadAvatar(repo *repo_model.Repository, data []byte) error {
return err return err
} }
newAvatar := fmt.Sprintf("%d-%x", repo.ID, md5.Sum(data)) newAvatar := avatar.HashAvatar(repo.ID, data)
if repo.Avatar == newAvatar { // upload the same picture if repo.Avatar == newAvatar { // upload the same picture
return nil return nil
} }

View File

@ -5,14 +5,13 @@ package repository
import ( import (
"bytes" "bytes"
"crypto/md5"
"fmt"
"image" "image"
"image/png" "image/png"
"testing" "testing"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/avatar"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -28,7 +27,7 @@ func TestUploadAvatar(t *testing.T) {
err := UploadAvatar(repo, buff.Bytes()) err := UploadAvatar(repo, buff.Bytes())
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, fmt.Sprintf("%d-%x", 10, md5.Sum(buff.Bytes())), repo.Avatar) assert.Equal(t, avatar.HashAvatar(10, buff.Bytes()), repo.Avatar)
} }
func TestUploadBigAvatar(t *testing.T) { func TestUploadBigAvatar(t *testing.T) {

View File

@ -5,7 +5,6 @@ package user
import ( import (
"context" "context"
"crypto/md5"
"fmt" "fmt"
"image/png" "image/png"
"io" "io"
@ -241,11 +240,7 @@ func UploadAvatar(u *user_model.User, data []byte) error {
defer committer.Close() defer committer.Close()
u.UseCustomAvatar = true u.UseCustomAvatar = true
// Different users can upload same image as avatar u.Avatar = avatar.HashAvatar(u.ID, data)
// 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 = user_model.UpdateUserCols(ctx, u, "use_custom_avatar", "avatar"); err != nil { if err = user_model.UpdateUserCols(ctx, u, "use_custom_avatar", "avatar"); err != nil {
return fmt.Errorf("updateUser: %w", err) return fmt.Errorf("updateUser: %w", err)
} }