simplify image Save logic by switching to loaded []byte

note: resize loaded to memory anyway

backend/app/store/image/fs_store.go

@@ -1,11 +1,11 @@
 package image
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"hash/crc64"
 	"io"
+	"io/ioutil"
 	"math"
 	"os"
 	"path"
@@ -41,18 +41,20 @@ type FileSystem struct {
 func (f *FileSystem) Save(fileName string, userID string, r io.Reader) (id string, err error) {
 
 	lr := io.LimitReader(r, int64(f.MaxSize)+1)
+	data, err := ioutil.ReadAll(lr)
+	if err != nil {
+		return "", errors.Wrapf(err, "can't read source data for image %s", fileName)
+	}
+	if len(data) > f.MaxSize {
+		return "", errors.Errorf("file %s is too large (limit=%d)", fileName, f.MaxSize)
+	}
 
 	// read header first, needs it to check if data is valid png/gif/jpeg
-	header := make([]byte, 512)
-	hl, err := lr.Read(header)
-	if err != nil {
-		return "", errors.Wrapf(err, "can't read image header for %s", fileName)
-	}
-	if !isValidImage(header) {
+	if !isValidImage(data[:512]) {
 		return "", errors.Errorf("file %s is not in allowed format", fileName)
 	}
 
-	reader, resized := resize(io.MultiReader(bytes.NewReader(header[:hl]), lr), f.MaxWidth, f.MaxHeight)
+	data, resized := resize(data, f.MaxWidth, f.MaxHeight)
 
 	id = path.Join(userID, guid()) + filepath.Ext(fileName) // make id as user/uuid.ext
 	dst := f.location(f.Staging, id)
@@ -65,27 +67,11 @@ func (f *FileSystem) Save(fileName string, userID string, r io.Reader) (id strin
 		return "", errors.Wrap(err, "can't make image directory")
 	}
 
-	fh, err := os.Create(dst)
-	if err != nil {
-		return "", errors.Wrapf(err, "can't make image file %s", dst)
-	}
-
-	written, err := io.Copy(fh, reader)
-	if err != nil {
+	if err := ioutil.WriteFile(dst, data, 0600); err != nil {
 		return "", errors.Wrapf(err, "can't write image file %s", dst)
 	}
-	if err = fh.Close(); err != nil {
-		return "", errors.Wrapf(err, "can't close image file %s", dst)
-	}
 
-	if written > int64(f.MaxSize) {
-		if err = os.Remove(dst); err != nil {
-			log.Printf("[WARN] can't remove image file %s, %v", dst, err)
-		}
-		return "", errors.Errorf("file %s is too large (%d)", fileName, written)
-	}
-
-	log.Printf("[DEBUG] file %s saved for image %s", fh.Name(), fileName)
+	log.Printf("[DEBUG] file %s saved for image %s, size=%d", dst, fileName, len(data))
 	return id, nil
 }
 

backend/app/store/image/fs_store_test.go

@@ -95,6 +95,27 @@ func TestFsStore_SaveWithResizeJpeg(t *testing.T) {
 	assert.Equal(t, 10786, len(data))
 }
 
+func TestFsStore_SaveNoResizeJpeg(t *testing.T) {
+	svc, _ := prepareImageTest(t)
+	svc.MaxWidth, svc.MaxHeight = 1400, 1300
+	svc.MaxSize = 32000
+
+	fh, err := os.Open("testdata/circles.jpg")
+	defer func() { assert.NoError(t, fh.Close()) }()
+	assert.NoError(t, err)
+	id, err := svc.Save("circles.jpg", "user1", fh)
+	assert.NoError(t, err)
+	assert.Contains(t, id, "user1/")
+	assert.Contains(t, id, ".jpg")
+	t.Log(id)
+
+	img := svc.location(svc.Staging, id)
+	t.Log(img)
+	data, err := ioutil.ReadFile(img)
+	assert.NoError(t, err)
+	assert.Equal(t, 23983, len(data))
+}
+
 func TestFsStore_WrongFormat(t *testing.T) {
 	svc, teardown := prepareImageTest(t)
 	defer teardown()

backend/app/store/image/image.go

@@ -137,26 +137,24 @@ func (s *Service) Close() {
 
 // resize an image of supported format (PNG, JPG, GIF) to the size of "limit" px of the
 // biggest side (width or height) preserving aspect ratio.
-// Returns original reader if resizing is not needed or failed. If resized the reader will be for png format
-// and ok flag will be true.
-func resize(reader io.Reader, limitW, limitH int) (io.Reader, bool) {
-	if reader == nil || limitW <= 0 || limitH <= 0 {
-		return reader, false
+// Returns original data if resizing is not needed or failed.
+// If resized the result will be for png format and ok flag will be true.
+func resize(data []byte, limitW, limitH int) ([]byte, bool) {
+	if data == nil || limitW <= 0 || limitH <= 0 {
+		return data, false
 	}
 
-	var teeBuf bytes.Buffer
-	tee := io.TeeReader(reader, &teeBuf)
-	src, _, err := image.Decode(tee)
+	src, _, err := image.Decode(bytes.NewBuffer(data))
 	if err != nil {
 		log.Printf("[WARN] can't decode image, %s", err)
-		return &teeBuf, false
+		return data, false
 	}
 
 	bounds := src.Bounds()
 	w, h := bounds.Dx(), bounds.Dy()
 	if w <= limitW && h <= limitH || w <= 0 || h <= 0 {
 		log.Printf("[DEBUG] resizing image is smaller that the limit or has 0 size")
-		return &teeBuf, false
+		return data, false
 	}
 
 	newW, newH := getProportionalSizes(w, h, limitW, limitH)
@@ -166,11 +164,12 @@ func resize(reader io.Reader, limitW, limitH int) (io.Reader, bool) {
 	var out bytes.Buffer
 	if err = png.Encode(&out, m); err != nil {
 		log.Printf("[WARN] can't encode resized image to png, %s", err)
-		return &teeBuf, false
+		return data, false
 	}
-	return &out, true
+	return out.Bytes(), true
 }
 
+// getProportionalSizes returns width and height resized by both dimensions proportionally
 func getProportionalSizes(srcW, srcH int, limitW, limitH int) (resW, resH int) {
 
 	if srcW <= limitW && srcH <= limitH {
@@ -190,7 +189,7 @@ func getProportionalSizes(srcW, srcH int, limitW, limitH int) (resW, resH int) {
 	return limitW, int(propH)
 }
 
-// check if file f is a valid image format, i.e. gif, png or jpeg
+// check if file f is a valid image format, i.e. gif, png, jpeg or webp
 func isValidImage(b []byte) bool {
 	ct := http.DetectContentType(b)
 	return ct == "image/gif" || ct == "image/png" || ct == "image/jpeg" || ct == "image/webp"

backend/app/store/image/image_test.go

@@ -4,10 +4,8 @@ import (
 	"bytes"
 	"context"
 	"image"
-	"io"
 	"io/ioutil"
 	"strconv"
-	"strings"
 	"testing"
 	"time"
 
@@ -76,27 +74,21 @@ func TestService_SubmitDelay(t *testing.T) {
 
 func TestService_resize(t *testing.T) {
 
-	checkC := func(t *testing.T, r io.Reader, cExp []byte) {
-		content, err := ioutil.ReadAll(r)
-		require.NoError(t, err)
-		assert.Equal(t, cExp, content)
-	}
-
 	// Reader is nil.
-	resizedR, ok := resize(nil, 100, 100)
-	assert.Nil(t, resizedR)
+	resized, ok := resize(nil, 100, 100)
+	assert.Nil(t, resized)
 	assert.False(t, ok)
 
 	// Negative limit error.
-	resizedR, ok = resize(strings.NewReader("some picture bin data"), -1, -1)
-	require.NotNil(t, resizedR)
-	checkC(t, resizedR, []byte("some picture bin data"))
+	resized, ok = resize([]byte("some picture bin data"), -1, -1)
+	require.NotNil(t, resized)
+	assert.Equal(t, resized, []byte("some picture bin data"))
 	assert.False(t, ok)
 
 	// Decode error.
-	resizedR, ok = resize(strings.NewReader("invalid image content"), 100, 100)
-	assert.NotNil(t, resizedR)
-	checkC(t, resizedR, []byte("invalid image content"))
+	resized, ok = resize([]byte("invalid image content"), 100, 100)
+	assert.NotNil(t, resized)
+	assert.Equal(t, resized, []byte("invalid image content"))
 	assert.False(t, ok)
 
 	cases := []struct {
@@ -112,17 +104,17 @@ func TestService_resize(t *testing.T) {
 		require.Nil(t, err, "can't open test file %s", c.file)
 
 		// No need for resize, image dimensions are smaller than resize limit.
-		resizedR, ok = resize(bytes.NewReader(img), 800, 800)
-		assert.NotNil(t, resizedR, "file %s", c.file)
-		checkC(t, resizedR, img)
+		resized, ok = resize(img, 800, 800)
+		assert.NotNil(t, resized, "file %s", c.file)
+		assert.Equal(t, resized, img)
 		assert.False(t, ok)
 
-		// Resizing to half of width. Check resizedR image format PNG.
-		resizedR, ok = resize(bytes.NewReader(img), 400, 400)
-		assert.NotNil(t, resizedR, "file %s", c.file)
+		// Resizing to half of width. Check resized image format PNG.
+		resized, ok = resize(img, 400, 400)
+		assert.NotNil(t, resized, "file %s", c.file)
 		assert.True(t, ok)
 
-		imgRz, format, err := image.Decode(resizedR)
+		imgRz, format, err := image.Decode(bytes.NewBuffer(resized))
 		assert.Nil(t, err, "file %s", c.file)
 		assert.Equal(t, "png", format, "file %s", c.file)
 		bounds := imgRz.Bounds()