Don't generate thumbnails for webp (#2388)

* Don't generate thumbnails for animated webp
* Debug log when writing thumbnail to disk

pkg/image/thumbnail.go

@@ -3,6 +3,8 @@ package image
 import (
 	"bytes"
 	"errors"
+	"fmt"
+	"image"
 	"os/exec"
 	"runtime"
 	"sync"
@@ -14,7 +16,10 @@ import (
 var vipsPath string
 var once sync.Once
 
-var ErrUnsupportedFormat = errors.New("unsupported image format")
+var (
+	// ErrNotSupportedForThumbnail is returned if the image format is not supported for thumbnail generation
+	ErrNotSupportedForThumbnail = errors.New("unsupported image format for thumbnail")
+)
 
 type ThumbnailEncoder struct {
 	ffmpeg ffmpeg.Encoder
@@ -45,7 +50,7 @@ func NewThumbnailEncoder(ffmpegEncoder ffmpeg.Encoder) ThumbnailEncoder {
 // GetThumbnail returns the thumbnail image of the provided image resized to
 // the provided max size. It resizes based on the largest X/Y direction.
 // It returns nil and an error if an error occurs reading, decoding or encoding
-// the image.
+// the image, or if the image is not suitable for thumbnails.
 func (e *ThumbnailEncoder) GetThumbnail(img *models.Image, maxSize int) ([]byte, error) {
 	reader, err := openSourceImage(img.Path)
 	if err != nil {
@@ -57,13 +62,24 @@ func (e *ThumbnailEncoder) GetThumbnail(img *models.Image, maxSize int) ([]byte,
 		return nil, err
 	}
 
-	_, format, err := DecodeSourceImage(img)
+	data := buf.Bytes()
+
+	// use NewBufferString to copy the buffer, rather than reuse it
+	_, format, err := image.DecodeConfig(bytes.NewBufferString(string(data)))
 	if err != nil {
 		return nil, err
 	}
 
-	if format != nil && *format == "gif" {
-		return buf.Bytes(), nil
+	animated := format == formatGif
+
+	// #2266 - if image is webp, then determine if it is animated
+	if format == formatWebP {
+		animated = isWebPAnimated(data)
+	}
+
+	// #2266 - don't generate a thumbnail for animated images
+	if animated {
+		return nil, fmt.Errorf("%w: %s", ErrNotSupportedForThumbnail, format)
 	}
 
 	// vips has issues loading files from stdin on Windows

pkg/image/webp.go

@@ -0,0 +1,46 @@
+package image
+
+import (
+	"bytes"
+)
+
+const (
+	formatWebP = "webp"
+	formatGif  = "gif"
+)
+
+// https://developers.google.com/speed/webp/docs/riff_container
+func isWebPAnimated(buf []byte) bool {
+	const (
+		webPHeaderStart = 8
+		webPHeaderEnd   = 12
+		webPHeader      = "WEBP"
+
+		animationHeaderLoc    = 16
+		minAnimSignatureIndex = 20
+
+		maxSize = 48
+	)
+
+	// truncate the buffer to the max size
+	if len(buf) > maxSize {
+		buf = buf[:maxSize]
+	}
+
+	isWebp := len(buf) >= webPHeaderEnd && string(buf[webPHeaderStart:webPHeaderEnd]) == "WEBP" // is WEBP
+
+	if isWebp {
+		const animBit byte = 1 << 1
+		if len(buf) > minAnimSignatureIndex {
+			// Animation Bit is set and ANIM header is present
+			return (buf[animationHeaderLoc]&animBit == animBit) && containsAnimSignature(buf[minAnimSignatureIndex:])
+		}
+	}
+	return false
+}
+
+// https://developers.google.com/speed/webp/docs/riff_container#animation
+func containsAnimSignature(buf []byte) bool {
+	index := bytes.Index(buf, []byte("ANIM"))
+	return index != -1
+}

pkg/image/webp_internal_test.go

@@ -0,0 +1,66 @@
+package image
+
+import "testing"
+
+func Test_isWebPAnimated(t *testing.T) {
+	tests := []struct {
+		name string
+		buf  []byte
+		want bool
+	}{
+		{
+			"basic animated",
+			[]byte{
+				0x52, 0x49, 0x46, 0x46, 0xb2, 0x3a, 0x17, 0x00, 0x57, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x58,
+				0x0a, 0x00, 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x7f, 0x02, 0x00, 0x55, 0x01, 0x00, 0x41, 0x4e,
+				0x49, 0x4d, 0x06, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x41, 0x4e, 0x4d, 0x46,
+			},
+			true,
+		},
+		{
+			"static webp",
+			[]byte{
+				0x52, 0x49, 0x46, 0x46, 0x68, 0x76, 0x00, 0x00, 0x57, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x20,
+				0x5c, 0x76, 0x00, 0x00, 0xd2, 0xbe, 0x01, 0x9d, 0x01, 0x2a, 0x26, 0x02, 0x70, 0x01, 0x3e, 0xd5,
+				0x4e, 0x97, 0x43, 0xa2, 0x06, 0x16, 0xd1, 0xb4, 0x88, 0x03, 0x51, 0x39, 0xb7, 0x13, 0x33, 0x75,
+			},
+			false,
+		},
+		{
+			"false animated bit",
+			[]byte{
+				0x52, 0x49, 0x46, 0x46, 0xb2, 0x3a, 0x17, 0x00, 0x57, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x58,
+				0x09, 0x00, 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x7f, 0x02, 0x00, 0x55, 0x01, 0x00, 0x41, 0x4e,
+				0x49, 0x4d, 0x06, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x41, 0x4e, 0x4d, 0x46,
+			},
+			false,
+		},
+		{
+			"ANIM out of range",
+			[]byte{
+				0x52, 0x49, 0x46, 0x46, 0xb2, 0x3a, 0x17, 0x00, 0x57, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x58,
+				0x0a, 0x00, 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x7f, 0x02, 0x00, 0x55, 0x01, 0x00, 0x3e, 0xd5,
+				0x4e, 0x97, 0x43, 0xa2, 0x06, 0x16, 0xd1, 0xb4, 0x88, 0x03, 0x51, 0x39, 0xb7, 0x13, 0x33, 0x75,
+				0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d,
+			},
+			false,
+		},
+		{
+			"not webp",
+			[]byte{
+				0x52, 0x49, 0x46, 0x46, 0xb2, 0x3a, 0x17, 0x00, 0x58, 0x45, 0x42, 0x50, 0x56, 0x50, 0x38, 0x58,
+				0x0a, 0x00, 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x7f, 0x02, 0x00, 0x55, 0x01, 0x00, 0x3e, 0xd5,
+				0x4e, 0x97, 0x43, 0xa2, 0x06, 0x16, 0xd1, 0xb4, 0x88, 0x03, 0x51, 0x39, 0xb7, 0x13, 0x33, 0x75,
+				0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d, 0x41, 0x4e, 0x49, 0x4d,
+			},
+			false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isWebPAnimated(tt.buf); got != tt.want {
+				t.Errorf("isWebPAnimated() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}