Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve avatar uploading / resizing / compressing, remove Fomantic card module #24653

Merged
merged 29 commits into from
May 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7b679b3
fix
wxiaoguang May 11, 2023
8f3a71c
support webp animation
wxiaoguang May 11, 2023
d40c7e6
improve webp detection
wxiaoguang May 11, 2023
4702918
fix
wxiaoguang May 11, 2023
34868ce
Update modules/avatar/avatar.go
wxiaoguang May 11, 2023
90f6e2e
remove unnecessary setting access
wxiaoguang May 11, 2023
ef79c9c
refactor
wxiaoguang May 11, 2023
ceeee5a
fine tune
wxiaoguang May 11, 2023
ab70def
improve comment
wxiaoguang May 11, 2023
d6e10c5
fix comment typo
wxiaoguang May 11, 2023
0bfed61
Apply suggestions from code review
wxiaoguang May 12, 2023
865f18e
Update docs/content/doc/administration/config-cheat-sheet.en-us.md
wxiaoguang May 12, 2023
9c2eb7a
fix typo in comment
wxiaoguang May 12, 2023
b79d492
double default AVATAR_MAX_FILE_SIZE to 2MiB
silverwind May 12, 2023
85e3fb1
double AVATAR_MAX_ORIGIN_SIZE to 256KiB
silverwind May 12, 2023
ceb44e5
reduce AVATAR_RENDERED_SIZE_FACTOR to 2
silverwind May 12, 2023
60410f9
increase AVATAR_MAX_HEIGHT to 4096
silverwind May 12, 2023
1ac0d77
take into account setting.Avatar.RenderedSizeFactor when scaling, red…
silverwind May 12, 2023
c3ac10f
revert AVATAR_MAX_FILE_SIZE to 1MiB
silverwind May 12, 2023
c32812f
update comment
silverwind May 12, 2023
b916c2a
make fmt
silverwind May 12, 2023
eb3037d
replace number in test
silverwind May 12, 2023
2b8133c
Update templates/user/profile.tmpl
wxiaoguang May 13, 2023
f80f89e
fix failed tests
wxiaoguang May 13, 2023
52fb4db
remove fomantic card module and extract all needed styles, fix avatar…
silverwind May 13, 2023
68047ce
add comment
silverwind May 13, 2023
5288391
avatar page fixes
silverwind May 13, 2023
7941519
more fixes, migrate is now pixel-identical
silverwind May 13, 2023
04c5fde
move card styles to own file
silverwind May 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions custom/conf/app.example.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1773,16 +1773,19 @@ ROUTER = console
;; Max Width and Height of uploaded avatars.
;; This is to limit the amount of RAM used when resizing the image.
;AVATAR_MAX_WIDTH = 4096
;AVATAR_MAX_HEIGHT = 3072
;AVATAR_MAX_HEIGHT = 4096
;;
;; The multiplication factor for rendered avatar images.
;; Larger values result in finer rendering on HiDPI devices.
;AVATAR_RENDERED_SIZE_FACTOR = 3
;AVATAR_RENDERED_SIZE_FACTOR = 2
;;
;; Maximum allowed file size for uploaded avatars.
;; This is to limit the amount of RAM used when resizing the image.
;AVATAR_MAX_FILE_SIZE = 1048576
;;
;; If the uploaded file is not larger than this byte size, the image will be used as is, without resizing/converting.
;AVATAR_MAX_ORIGIN_SIZE = 262144
;;
;; Chinese users can choose "duoshuo"
;; or a custom avatar source, like: http://cn.gravatar.com/avatar/
;GRAVATAR_SOURCE = gravatar
Expand Down
7 changes: 4 additions & 3 deletions docs/content/doc/administration/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -792,9 +792,10 @@ and
- `AVATAR_STORAGE_TYPE`: **default**: Storage type defined in `[storage.xxx]`. Default is `default` which will read `[storage]` if no section `[storage]` will be a type `local`.
- `AVATAR_UPLOAD_PATH`: **data/avatars**: Path to store user avatar image files.
- `AVATAR_MAX_WIDTH`: **4096**: Maximum avatar image width in pixels.
- `AVATAR_MAX_HEIGHT`: **3072**: Maximum avatar image height in pixels.
- `AVATAR_MAX_FILE_SIZE`: **1048576** (1Mb): Maximum avatar image file size in bytes.
- `AVATAR_RENDERED_SIZE_FACTOR`: **3**: The multiplication factor for rendered avatar images. Larger values result in finer rendering on HiDPI devices.
- `AVATAR_MAX_HEIGHT`: **4096**: Maximum avatar image height in pixels.
- `AVATAR_MAX_FILE_SIZE`: **1048576** (1MiB): Maximum avatar image file size in bytes.
- `AVATAR_MAX_ORIGIN_SIZE`: **262144** (256KiB): If the uploaded file is not larger than this byte size, the image will be used as is, without resizing/converting.
- `AVATAR_RENDERED_SIZE_FACTOR`: **2**: The multiplication factor for rendered avatar images. Larger values result in finer rendering on HiDPI devices.

- `REPOSITORY_AVATAR_STORAGE_TYPE`: **default**: Storage type defined in `[storage.xxx]`. Default is `default` which will read `[storage]` if no section `[storage]` will be a type `local`.
- `REPOSITORY_AVATAR_UPLOAD_PATH`: **data/repo-avatars**: Path to store repository avatar image files.
Expand Down
4 changes: 2 additions & 2 deletions docs/content/doc/administration/config-cheat-sheet.zh-cn.md
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ menu:
- `AVATAR_STORAGE_TYPE`: **local**: 头像存储类型,可以为 `local` 或 `minio`,分别支持本地文件系统和 minio 兼容的API。
- `AVATAR_UPLOAD_PATH`: **data/avatars**: 存储头像的文件系统路径。
- `AVATAR_MAX_WIDTH`: **4096**: 头像最大宽度,单位像素。
- `AVATAR_MAX_HEIGHT`: **3072**: 头像最大高度,单位像素。
- `AVATAR_MAX_FILE_SIZE`: **1048576** (1Mb): 头像最大大小。
- `AVATAR_MAX_HEIGHT`: **4096**: 头像最大高度,单位像素。
- `AVATAR_MAX_FILE_SIZE`: **1048576** (1MiB): 头像最大大小。

- `REPOSITORY_AVATAR_STORAGE_TYPE`: **local**: 仓库头像存储类型,可以为 `local` 或 `minio`,分别支持本地文件系统和 minio 兼容的API。
- `REPOSITORY_AVATAR_UPLOAD_PATH`: **data/repo-avatars**: 存储仓库头像的路径。
Expand Down
70 changes: 55 additions & 15 deletions modules/avatar/avatar.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ package avatar

import (
"bytes"
"errors"
"fmt"
"image"
"image/color"
"image/png"

_ "image/gif" // for processing gif images
_ "image/jpeg" // for processing jpeg images
_ "image/png" // for processing png images

"code.gitea.io/gitea/modules/avatar/identicon"
"code.gitea.io/gitea/modules/setting"
Expand All @@ -22,8 +23,11 @@ import (
_ "golang.org/x/image/webp" // for processing webp images
)

// AvatarSize returns avatar's size
const AvatarSize = 290
// DefaultAvatarSize is the target CSS pixel size for avatar generation. It is
// multiplied by setting.Avatar.RenderedSizeFactor and the resulting size is the
// usual size of avatar image saved on server, unless the original file is smaller
// than the size after resizing.
const DefaultAvatarSize = 256

// RandomImageSize generates and returns a random avatar image unique to input data
// in custom size (height and width).
Expand All @@ -39,28 +43,44 @@ func RandomImageSize(size int, data []byte) (image.Image, error) {
// RandomImage generates and returns a random avatar image unique to input data
// in default size (height and width).
func RandomImage(data []byte) (image.Image, error) {
return RandomImageSize(AvatarSize, data)
return RandomImageSize(DefaultAvatarSize*setting.Avatar.RenderedSizeFactor, data)
}

// Prepare accepts a byte slice as input, validates it contains an image of an
// acceptable format, and crops and resizes it appropriately.
func Prepare(data []byte) (*image.Image, error) {
imgCfg, _, err := image.DecodeConfig(bytes.NewReader(data))
// processAvatarImage process the avatar image data, crop and resize it if necessary.
// the returned data could be the original image if no processing is needed.
func processAvatarImage(data []byte, maxOriginSize int64) ([]byte, error) {
imgCfg, imgType, err := image.DecodeConfig(bytes.NewReader(data))
if err != nil {
return nil, fmt.Errorf("DecodeConfig: %w", err)
return nil, fmt.Errorf("image.DecodeConfig: %w", err)
}

// for safety, only accept known types explicitly
if imgType != "png" && imgType != "jpeg" && imgType != "gif" && imgType != "webp" {
return nil, errors.New("unsupported avatar image type")
}

// do not process image which is too large, it would consume too much memory
if imgCfg.Width > setting.Avatar.MaxWidth {
return nil, fmt.Errorf("Image width is too large: %d > %d", imgCfg.Width, setting.Avatar.MaxWidth)
return nil, fmt.Errorf("image width is too large: %d > %d", imgCfg.Width, setting.Avatar.MaxWidth)
}
if imgCfg.Height > setting.Avatar.MaxHeight {
return nil, fmt.Errorf("Image height is too large: %d > %d", imgCfg.Height, setting.Avatar.MaxHeight)
return nil, fmt.Errorf("image height is too large: %d > %d", imgCfg.Height, setting.Avatar.MaxHeight)
Comment on lines +59 to +67
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

util.NewInvalidArgumentErrorf?
Also, shouldn't we print the unsupported image type for debugging reasons?

}

// If the origin is small enough, just use it, then APNG could be supported,
// otherwise, if the image is processed later, APNG loses animation.
// And one more thing, webp is not fully supported, for animated webp, image.DecodeConfig works but Decode fails.
// So for animated webp, if the uploaded file is smaller than maxOriginSize, it will be used, if it's larger, there will be an error.
if len(data) < int(maxOriginSize) {
return data, nil
}

img, _, err := image.Decode(bytes.NewReader(data))
if err != nil {
return nil, fmt.Errorf("Decode: %w", err)
return nil, fmt.Errorf("image.Decode: %w", err)
}

// try to crop and resize the origin image if necessary
if imgCfg.Width != imgCfg.Height {
var newSize, ax, ay int
if imgCfg.Width > imgCfg.Height {
Expand All @@ -74,13 +94,33 @@ func Prepare(data []byte) (*image.Image, error) {
img, err = cutter.Crop(img, cutter.Config{
Width: newSize,
Height: newSize,
Anchor: image.Point{ax, ay},
Anchor: image.Point{X: ax, Y: ay},
})
if err != nil {
return nil, err
}
}

img = resize.Resize(AvatarSize, AvatarSize, img, resize.Bilinear)
return &img, nil
targetSize := uint(DefaultAvatarSize * setting.Avatar.RenderedSizeFactor)
img = resize.Resize(targetSize, targetSize, img, resize.Bilinear)

// try to encode the cropped/resized image to png
bs := bytes.Buffer{}
if err = png.Encode(&bs, img); err != nil {
return nil, err
}
resized := bs.Bytes()

// usually the png compression is not good enough, use the original image (no cropping/resizing) if the origin is smaller
if len(data) <= len(resized) {
return data, nil
}

return resized, nil
}

// ProcessAvatarImage process the avatar image data, crop and resize it if necessary.
// the returned data could be the original image if no processing is needed.
func ProcessAvatarImage(data []byte) ([]byte, error) {
return processAvatarImage(data, setting.Avatar.MaxOriginSize)
}
95 changes: 79 additions & 16 deletions modules/avatar/avatar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
package avatar

import (
"bytes"
"image"
"image/png"
"os"
"testing"

Expand All @@ -25,49 +28,109 @@ func Test_RandomImage(t *testing.T) {
assert.NoError(t, err)
}

func Test_PrepareWithPNG(t *testing.T) {
func Test_ProcessAvatarPNG(t *testing.T) {
setting.Avatar.MaxWidth = 4096
setting.Avatar.MaxHeight = 4096

data, err := os.ReadFile("testdata/avatar.png")
assert.NoError(t, err)

imgPtr, err := Prepare(data)
_, err = processAvatarImage(data, 262144)
assert.NoError(t, err)

assert.Equal(t, 290, (*imgPtr).Bounds().Max.X)
assert.Equal(t, 290, (*imgPtr).Bounds().Max.Y)
}

func Test_PrepareWithJPEG(t *testing.T) {
func Test_ProcessAvatarJPEG(t *testing.T) {
setting.Avatar.MaxWidth = 4096
setting.Avatar.MaxHeight = 4096

data, err := os.ReadFile("testdata/avatar.jpeg")
assert.NoError(t, err)

imgPtr, err := Prepare(data)
_, err = processAvatarImage(data, 262144)
assert.NoError(t, err)

assert.Equal(t, 290, (*imgPtr).Bounds().Max.X)
assert.Equal(t, 290, (*imgPtr).Bounds().Max.Y)
}

func Test_PrepareWithInvalidImage(t *testing.T) {
func Test_ProcessAvatarInvalidData(t *testing.T) {
setting.Avatar.MaxWidth = 5
setting.Avatar.MaxHeight = 5

_, err := Prepare([]byte{})
assert.EqualError(t, err, "DecodeConfig: image: unknown format")
_, err := processAvatarImage([]byte{}, 12800)
assert.EqualError(t, err, "image.DecodeConfig: image: unknown format")
}

func Test_PrepareWithInvalidImageSize(t *testing.T) {
func Test_ProcessAvatarInvalidImageSize(t *testing.T) {
setting.Avatar.MaxWidth = 5
setting.Avatar.MaxHeight = 5

data, err := os.ReadFile("testdata/avatar.png")
assert.NoError(t, err)

_, err = Prepare(data)
assert.EqualError(t, err, "Image width is too large: 10 > 5")
_, err = processAvatarImage(data, 12800)
assert.EqualError(t, err, "image width is too large: 10 > 5")
}

func Test_ProcessAvatarImage(t *testing.T) {
setting.Avatar.MaxWidth = 4096
setting.Avatar.MaxHeight = 4096
scaledSize := DefaultAvatarSize * setting.Avatar.RenderedSizeFactor

newImgData := func(size int, optHeight ...int) []byte {
width := size
height := size
if len(optHeight) == 1 {
height = optHeight[0]
}
img := image.NewRGBA(image.Rect(0, 0, width, height))
bs := bytes.Buffer{}
err := png.Encode(&bs, img)
assert.NoError(t, err)
return bs.Bytes()
}

// if origin image canvas is too large, crop and resize it
origin := newImgData(500, 600)
result, err := processAvatarImage(origin, 0)
assert.NoError(t, err)
assert.NotEqual(t, origin, result)
decoded, err := png.Decode(bytes.NewReader(result))
assert.NoError(t, err)
assert.EqualValues(t, scaledSize, decoded.Bounds().Max.X)
assert.EqualValues(t, scaledSize, decoded.Bounds().Max.Y)

// if origin image is smaller than the default size, use the origin image
origin = newImgData(1)
result, err = processAvatarImage(origin, 0)
assert.NoError(t, err)
assert.Equal(t, origin, result)

// use the origin image if the origin is smaller
origin = newImgData(scaledSize + 100)
result, err = processAvatarImage(origin, 0)
assert.NoError(t, err)
assert.Less(t, len(result), len(origin))

// still use the origin image if the origin doesn't exceed the max-origin-size
origin = newImgData(scaledSize + 100)
result, err = processAvatarImage(origin, 262144)
assert.NoError(t, err)
assert.Equal(t, origin, result)

// allow to use known image format (eg: webp) if it is small enough
origin, err = os.ReadFile("testdata/animated.webp")
assert.NoError(t, err)
result, err = processAvatarImage(origin, 262144)
assert.NoError(t, err)
assert.Equal(t, origin, result)

// do not support unknown image formats, eg: SVG may contain embedded JS
origin = []byte("<svg></svg>")
_, err = processAvatarImage(origin, 262144)
assert.ErrorContains(t, err, "image: unknown format")

// make sure the canvas size limit works
setting.Avatar.MaxWidth = 5
setting.Avatar.MaxHeight = 5
origin = newImgData(10)
_, err = processAvatarImage(origin, 262144)
assert.ErrorContains(t, err, "image width is too large: 10 > 5")
}
Binary file added modules/avatar/testdata/animated.webp
Binary file not shown.
7 changes: 3 additions & 4 deletions modules/repository/commits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package repository
import (
"crypto/md5"
"fmt"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -136,13 +137,11 @@ func TestPushCommits_AvatarLink(t *testing.T) {
enableGravatar(t)

assert.Equal(t,
"https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon&s=84",
"https://secure.gravatar.com/avatar/ab53a2911ddf9b4817ac01ddcd3d975f?d=identicon&s="+strconv.Itoa(28*setting.Avatar.RenderedSizeFactor),
pushCommits.AvatarLink(db.DefaultContext, "user2@example.com"))

assert.Equal(t,
"https://secure.gravatar.com/avatar/"+
fmt.Sprintf("%x", md5.Sum([]byte("nonexistent@example.com")))+
"?d=identicon&s=84",
fmt.Sprintf("https://secure.gravatar.com/avatar/%x?d=identicon&s=%d", md5.Sum([]byte("nonexistent@example.com")), 28*setting.Avatar.RenderedSizeFactor),
pushCommits.AvatarLink(db.DefaultContext, "nonexistent@example.com"))
}

Expand Down
17 changes: 10 additions & 7 deletions modules/setting/picture.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,23 @@

package setting

// settings
// Avatar settings

var (
// Picture settings
Avatar = struct {
Storage

MaxWidth int
MaxHeight int
MaxFileSize int64
MaxOriginSize int64
RenderedSizeFactor int
}{
MaxWidth: 4096,
MaxHeight: 3072,
MaxHeight: 4096,
MaxFileSize: 1048576,
RenderedSizeFactor: 3,
MaxOriginSize: 262144,
RenderedSizeFactor: 2,
}

GravatarSource string
Expand All @@ -44,9 +46,10 @@ func loadPictureFrom(rootCfg ConfigProvider) {
Avatar.Storage = getStorage(rootCfg, "avatars", storageType, avatarSec)

Avatar.MaxWidth = sec.Key("AVATAR_MAX_WIDTH").MustInt(4096)
Avatar.MaxHeight = sec.Key("AVATAR_MAX_HEIGHT").MustInt(3072)
Avatar.MaxHeight = sec.Key("AVATAR_MAX_HEIGHT").MustInt(4096)
Avatar.MaxFileSize = sec.Key("AVATAR_MAX_FILE_SIZE").MustInt64(1048576)
Avatar.RenderedSizeFactor = sec.Key("AVATAR_RENDERED_SIZE_FACTOR").MustInt(3)
Avatar.MaxOriginSize = sec.Key("AVATAR_MAX_ORIGIN_SIZE").MustInt64(262144)
Avatar.RenderedSizeFactor = sec.Key("AVATAR_RENDERED_SIZE_FACTOR").MustInt(2)

switch source := sec.Key("GRAVATAR_SOURCE").MustString("gravatar"); source {
case "duoshuo":
Expand Down Expand Up @@ -94,5 +97,5 @@ func loadRepoAvatarFrom(rootCfg ConfigProvider) {
RepoAvatar.Storage = getStorage(rootCfg, "repo-avatars", storageType, repoAvatarSec)

RepoAvatar.Fallback = sec.Key("REPOSITORY_AVATAR_FALLBACK").MustString("none")
RepoAvatar.FallbackImage = sec.Key("REPOSITORY_AVATAR_FALLBACK_IMAGE").MustString("/assets/img/repo_default.png")
RepoAvatar.FallbackImage = sec.Key("REPOSITORY_AVATAR_FALLBACK_IMAGE").MustString(AppSubURL + "/assets/img/repo_default.png")
}
Loading