diff --git a/backend/app/store/image/fs_store.go b/backend/app/store/image/fs_store.go index c7c8f7d0..e87d975c 100644 --- a/backend/app/store/image/fs_store.go +++ b/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 } diff --git a/backend/app/store/image/fs_store_test.go b/backend/app/store/image/fs_store_test.go index a1e26bb1..64247e11 100644 --- a/backend/app/store/image/fs_store_test.go +++ b/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() diff --git a/backend/app/store/image/image.go b/backend/app/store/image/image.go index 1f99b35b..e9fd8d4f 100644 --- a/backend/app/store/image/image.go +++ b/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" diff --git a/backend/app/store/image/image_test.go b/backend/app/store/image/image_test.go index ba8b2e3c..157dce5f 100644 --- a/backend/app/store/image/image_test.go +++ b/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()