Improve handling of non-square avatars (#7025)

* Crop avatar before resizing (#1268)

Signed-off-by: Rob Watson <rfwatson@users.noreply.github.com>

* Fix spelling error

Signed-off-by: Rob Watson <rfwatson@users.noreply.github.com>

go.mod

@@ -90,6 +90,7 @@ require (
 	github.com/mschoch/smat v0.0.0-20160514031455-90eadee771ae // indirect
 	github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc
 	github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5
+	github.com/oliamb/cutter v0.2.2
 	github.com/philhofer/fwd v1.0.0 // indirect
 	github.com/pkg/errors v0.8.1 // indirect
 	github.com/pquerna/otp v0.0.0-20160912161815-54653902c20e

go.sum

@@ -244,6 +244,8 @@ github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc h1:z1PgdCCmYYVL0BoJT
 github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc/go.mod h1:np1wUFZ6tyoke22qDJZY40URn9Ae51gX7ljIWXN5TJs=
 github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5 h1:BvoENQQU+fZ9uukda/RzCAL/191HHwJA5b13R6diVlY=
 github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5/go.mod h1:jpp1/29i3P1S/RLdc7JQKbRpFeM1dOBd8T9ki5s+AY8=
+github.com/oliamb/cutter v0.2.2 h1:Lfwkya0HHNU1YLnGv2hTkzHfasrSMkgv4Dn+5rmlk3k=
+github.com/oliamb/cutter v0.2.2/go.mod h1:4BenG2/4GuRBDbVm/OPahDVqbrOemzpPiG5mi1iryBU=
 github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.7.0 h1:WSHQ+IS43OoUrWtD1/bbclrwK8TTH5hzp+umCiuxHgs=
 github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=

models/user.go

@@ -6,15 +6,13 @@
 package models
 
 import (
-	"bytes"
 	"container/list"
 	"crypto/md5"
 	"crypto/sha256"
 	"crypto/subtle"
 	"encoding/hex"
 	"errors"
 	"fmt"
-	"image"
 
 	// Needed for jpeg support
 	_ "image/jpeg"
@@ -39,7 +37,6 @@ import (
 	"github.com/go-xorm/builder"
 	"github.com/go-xorm/core"
 	"github.com/go-xorm/xorm"
-	"github.com/nfnt/resize"
 	"golang.org/x/crypto/pbkdf2"
 	"golang.org/x/crypto/ssh"
 )
@@ -457,24 +454,11 @@ func (u *User) IsPasswordSet() bool {
 // UploadAvatar saves custom avatar for user.
 // FIXME: split uploads to different subdirs in case we have massive users.
 func (u *User) UploadAvatar(data []byte) error {
-	imgCfg, _, err := image.DecodeConfig(bytes.NewReader(data))
+	m, err := avatar.Prepare(data)
 	if err != nil {
-		return fmt.Errorf("DecodeConfig: %v", err)
-	}
-	if imgCfg.Width > setting.AvatarMaxWidth {
-		return fmt.Errorf("Image width is to large: %d > %d", imgCfg.Width, setting.AvatarMaxWidth)
-	}
-	if imgCfg.Height > setting.AvatarMaxHeight {
-		return fmt.Errorf("Image height is to large: %d > %d", imgCfg.Height, setting.AvatarMaxHeight)
-	}
-
-	img, _, err := image.Decode(bytes.NewReader(data))
-	if err != nil {
-		return fmt.Errorf("Decode: %v", err)
+		return err
 	}
 
-	m := resize.Resize(avatar.AvatarSize, avatar.AvatarSize, img, resize.NearestNeighbor)
-
 	sess := x.NewSession()
 	defer sess.Close()
 	if err = sess.Begin(); err != nil {
@@ -497,7 +481,7 @@ func (u *User) UploadAvatar(data []byte) error {
 	}
 	defer fw.Close()
 
-	if err = png.Encode(fw, m); err != nil {
+	if err = png.Encode(fw, *m); err != nil {
 		return fmt.Errorf("Encode: %v", err)
 	}
 

modules/avatar/avatar.go

@@ -5,13 +5,20 @@
 package avatar
 
 import (
+	"bytes"
 	"fmt"
 	"image"
 	"image/color/palette"
+	// Enable PNG support:
+	_ "image/png"
 	"math/rand"
 	"time"
 
+	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/issue9/identicon"
+	"github.com/nfnt/resize"
+	"github.com/oliamb/cutter"
 )
 
 // AvatarSize returns avatar's size
@@ -42,3 +49,46 @@ func RandomImageSize(size int, data []byte) (image.Image, error) {
 func RandomImage(data []byte) (image.Image, error) {
 	return RandomImageSize(AvatarSize, 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))
+	if err != nil {
+		return nil, fmt.Errorf("DecodeConfig: %v", err)
+	}
+	if imgCfg.Width > setting.AvatarMaxWidth {
+		return nil, fmt.Errorf("Image width is too large: %d > %d", imgCfg.Width, setting.AvatarMaxWidth)
+	}
+	if imgCfg.Height > setting.AvatarMaxHeight {
+		return nil, fmt.Errorf("Image height is too large: %d > %d", imgCfg.Height, setting.AvatarMaxHeight)
+	}
+
+	img, _, err := image.Decode(bytes.NewReader(data))
+	if err != nil {
+		return nil, fmt.Errorf("Decode: %v", err)
+	}
+
+	if imgCfg.Width != imgCfg.Height {
+		var newSize, ax, ay int
+		if imgCfg.Width > imgCfg.Height {
+			newSize = imgCfg.Height
+			ax = (imgCfg.Width - imgCfg.Height) / 2
+		} else {
+			newSize = imgCfg.Width
+			ay = (imgCfg.Height - imgCfg.Width) / 2
+		}
+
+		img, err = cutter.Crop(img, cutter.Config{
+			Width:  newSize,
+			Height: newSize,
+			Anchor: image.Point{ax, ay},
+		})
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	img = resize.Resize(AvatarSize, AvatarSize, img, resize.NearestNeighbor)
+	return &img, nil
+}

modules/avatar/avatar_test.go

@@ -5,8 +5,11 @@
 package avatar
 
 import (
+	"io/ioutil"
 	"testing"
 
+	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -17,3 +20,49 @@ func Test_RandomImage(t *testing.T) {
 	_, err = RandomImageSize(0, []byte("gogs@local"))
 	assert.Error(t, err)
 }
+
+func Test_PrepareWithPNG(t *testing.T) {
+	setting.AvatarMaxWidth = 4096
+	setting.AvatarMaxHeight = 4096
+
+	data, err := ioutil.ReadFile("testdata/avatar.png")
+	assert.NoError(t, err)
+
+	imgPtr, err := Prepare(data)
+	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) {
+	setting.AvatarMaxWidth = 4096
+	setting.AvatarMaxHeight = 4096
+
+	data, err := ioutil.ReadFile("testdata/avatar.jpeg")
+	assert.NoError(t, err)
+
+	imgPtr, err := Prepare(data)
+	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) {
+	setting.AvatarMaxWidth = 5
+	setting.AvatarMaxHeight = 5
+
+	_, err := Prepare([]byte{})
+	assert.EqualError(t, err, "DecodeConfig: image: unknown format")
+}
+func Test_PrepareWithInvalidImageSize(t *testing.T) {
+	setting.AvatarMaxWidth = 5
+	setting.AvatarMaxHeight = 5
+
+	data, err := ioutil.ReadFile("testdata/avatar.png")
+	assert.NoError(t, err)
+
+	_, err = Prepare(data)
+	assert.EqualError(t, err, "Image width is too large: 10 > 5")
+}