From 21e1af32af291d0b5dc3722e7e11ec818c49c1b9 Mon Sep 17 00:00:00 2001
From: Travis Ralston <travpc@gmail.com>
Date: Mon, 21 Aug 2023 21:15:24 -0600
Subject: [PATCH] Fix 404 on thumbnails that are too small
Fixes https://github.com/turt2live/matrix-media-repo/issues/444
Also fixes a memory issue where we'd decode the entire image before deciding that the image is too small for thumbnailing.
---
api/r0/thumbnail.go | 22 ++++++++++++++++++++++
common/errors.go | 1 +
pipelines/pipeline_thumbnail/pipeline.go | 10 +++++++++-
thumbnailing/i/apng.go | 3 ++-
thumbnailing/i/gif.go | 2 +-
thumbnailing/i/jpg.go | 6 ------
thumbnailing/i/mp3.go | 4 ++--
thumbnailing/i/png.go | 12 +-----------
thumbnailing/thumbnail.go | 8 ++++++++
thumbnailing/u/dimensions.go | 10 +++-------
10 files changed, 49 insertions(+), 29 deletions(-)
diff --git a/api/r0/thumbnail.go b/api/r0/thumbnail.go
index 2b4bb43c..bdfb0267 100644
--- a/api/r0/thumbnail.go
+++ b/api/r0/thumbnail.go
@@ -9,6 +9,7 @@ import (
"github.com/turt2live/matrix-media-repo/api/_apimeta"
"github.com/turt2live/matrix-media-repo/api/_responses"
"github.com/turt2live/matrix-media-repo/api/_routers"
+ "github.com/turt2live/matrix-media-repo/database"
"github.com/turt2live/matrix-media-repo/pipelines/pipeline_download"
"github.com/turt2live/matrix-media-repo/pipelines/pipeline_thumbnail"
"github.com/turt2live/matrix-media-repo/util"
@@ -132,6 +133,27 @@ func ThumbnailMedia(r *http.Request, rctx rcontext.RequestContext, user _apimeta
}
} else if errors.Is(err, common.ErrMediaNotYetUploaded) {
return _responses.NotYetUploaded()
+ } else if errors.Is(err, common.ErrMediaDimensionsTooSmall) {
+ if stream == nil {
+ return _responses.NotFoundError() // something went wrong so just 404 the thumbnail
+ }
+
+ // We have a stream, and an error about image size, so we know there should be a media record
+ mediaDb := database.GetInstance().Media.Prepare(rctx)
+ record, err := mediaDb.GetById(server, mediaId)
+ if err != nil {
+ rctx.Log.Error("Unexpected error locating media record: ", err)
+ sentry.CaptureException(err)
+ return _responses.InternalServerError("Unexpected Error")
+ } else {
+ return &_responses.DownloadResponse{
+ ContentType: record.ContentType,
+ Filename: record.UploadName,
+ SizeBytes: record.SizeBytes,
+ Data: stream,
+ TargetDisposition: "infer",
+ }
+ }
}
rctx.Log.Error("Unexpected error locating media: ", err)
sentry.CaptureException(err)
diff --git a/common/errors.go b/common/errors.go
index 023926f2..f9d5c360 100644
--- a/common/errors.go
+++ b/common/errors.go
@@ -15,3 +15,4 @@ var ErrWrongUser = errors.New("wrong user")
var ErrExpired = errors.New("expired")
var ErrAlreadyUploaded = errors.New("already uploaded")
var ErrMediaNotYetUploaded = errors.New("media not yet uploaded")
+var ErrMediaDimensionsTooSmall = errors.New("media is too small dimensionally")
diff --git a/pipelines/pipeline_thumbnail/pipeline.go b/pipelines/pipeline_thumbnail/pipeline.go
index 583870e4..7c4a79c0 100644
--- a/pipelines/pipeline_thumbnail/pipeline.go
+++ b/pipelines/pipeline_thumbnail/pipeline.go
@@ -110,6 +110,14 @@ func Execute(ctx rcontext.RequestContext, origin string, mediaId string, opts Th
// Step 6: Generate the thumbnail and return that
record, r, err := thumbnails.Generate(ctx, mediaRecord, opts.Width, opts.Height, opts.Method, opts.Animated)
if err != nil {
+ if !opts.RecordOnly && errors.Is(err, common.ErrMediaDimensionsTooSmall) {
+ d, err := download.OpenStream(ctx, mediaRecord.Locatable, opts.StartByte, opts.EndByte)
+ if err != nil {
+ return nil, err
+ } else {
+ return d, common.ErrMediaDimensionsTooSmall
+ }
+ }
return nil, err
}
recordSf.OverwriteCacheKey(sfKey, record)
@@ -121,7 +129,7 @@ func Execute(ctx rcontext.RequestContext, origin string, mediaId string, opts Th
// Step 7: Create a limited stream
return download.CreateLimitedStream(ctx, r, opts.StartByte, opts.EndByte)
})
- if errors.Is(err, common.ErrMediaQuarantined) {
+ if errors.Is(err, common.ErrMediaQuarantined) || errors.Is(err, common.ErrMediaDimensionsTooSmall) {
cancel()
return nil, r, err
}
diff --git a/thumbnailing/i/apng.go b/thumbnailing/i/apng.go
index f107246a..12c36616 100644
--- a/thumbnailing/i/apng.go
+++ b/thumbnailing/i/apng.go
@@ -10,6 +10,7 @@ import (
"github.com/kettek/apng"
"github.com/turt2live/matrix-media-repo/common/rcontext"
"github.com/turt2live/matrix-media-repo/thumbnailing/m"
+ "github.com/turt2live/matrix-media-repo/thumbnailing/u"
)
type apngGenerator struct {
@@ -69,7 +70,7 @@ func (d apngGenerator) GenerateThumbnail(b io.Reader, contentType string, width
draw.Draw(frameImg, image.Rect(frame.XOffset, frame.YOffset, frameImg.Rect.Max.X, frameImg.Rect.Max.Y), img, image.Point{X: 0, Y: 0}, draw.Src)
// Do the thumbnailing on the copied frame
- frameThumb, err := pngGenerator{}.GenerateThumbnailImageOf(frameImg, width, height, method, ctx)
+ frameThumb, err := u.MakeThumbnail(frameImg, method, width, height)
if err != nil {
return nil, errors.New("apng: error generating thumbnail frame: " + err.Error())
}
diff --git a/thumbnailing/i/gif.go b/thumbnailing/i/gif.go
index 4e564ead..df406564 100644
--- a/thumbnailing/i/gif.go
+++ b/thumbnailing/i/gif.go
@@ -56,7 +56,7 @@ func (d gifGenerator) GenerateThumbnail(b io.Reader, contentType string, width i
draw.Draw(frameImg, frameImg.Bounds(), img, image.Point{X: 0, Y: 0}, draw.Over)
// Do the thumbnailing on the copied frame
- frameThumb, err := pngGenerator{}.GenerateThumbnailImageOf(frameImg, width, height, method, ctx)
+ frameThumb, err := u.MakeThumbnail(frameImg, method, width, height)
if err != nil {
return nil, errors.New("gif: error generating thumbnail frame: " + err.Error())
}
diff --git a/thumbnailing/i/jpg.go b/thumbnailing/i/jpg.go
index adc77004..7cfd73e4 100644
--- a/thumbnailing/i/jpg.go
+++ b/thumbnailing/i/jpg.go
@@ -43,12 +43,6 @@ func (d jpgGenerator) GenerateThumbnail(b io.Reader, contentType string, width i
return nil, errors.New("jpg: error decoding thumbnail: " + err.Error())
}
- var shouldThumbnail bool
- shouldThumbnail, width, height, _, method = u.AdjustProperties(src, width, height, animated, false, method)
- if !shouldThumbnail {
- return nil, nil
- }
-
thumb, err := u.MakeThumbnail(src, method, width, height)
if err != nil {
return nil, errors.New("jpg: error making thumbnail: " + err.Error())
diff --git a/thumbnailing/i/mp3.go b/thumbnailing/i/mp3.go
index 2fef19ab..2c2e7228 100644
--- a/thumbnailing/i/mp3.go
+++ b/thumbnailing/i/mp3.go
@@ -105,7 +105,7 @@ func (d mp3Generator) GenerateFromStream(audio beep.StreamSeekCloser, format bee
if meta != nil && meta.Picture() != nil {
artwork, _, _ := image.Decode(bytes.NewBuffer(meta.Picture().Data))
if artwork != nil {
- artworkImg, _ = pngGenerator{}.GenerateThumbnailImageOf(artwork, sq, sq, "crop", rcontext.Initial())
+ artworkImg, _ = u.MakeThumbnail(artwork, "crop", sq, sq)
}
}
@@ -129,7 +129,7 @@ func (d mp3Generator) GenerateFromStream(audio beep.StreamSeekCloser, format bee
defer f.Close()
tmp, _, _ := image.Decode(f)
if tmp != nil {
- artworkImg, _ = pngGenerator{}.GenerateThumbnailImageOf(tmp, ax, ay, "crop", rcontext.Initial())
+ artworkImg, _ = u.MakeThumbnail(tmp, "crop", ax, ay)
}
}
if artworkImg == nil {
diff --git a/thumbnailing/i/png.go b/thumbnailing/i/png.go
index 135d9113..e4dd60bc 100644
--- a/thumbnailing/i/png.go
+++ b/thumbnailing/i/png.go
@@ -45,7 +45,7 @@ func (d pngGenerator) GenerateThumbnail(b io.Reader, contentType string, width i
}
func (d pngGenerator) GenerateThumbnailOf(src image.Image, width int, height int, method string, ctx rcontext.RequestContext) (*m.Thumbnail, error) {
- thumb, err := d.GenerateThumbnailImageOf(src, width, height, method, ctx)
+ thumb, err := u.MakeThumbnail(src, method, width, height)
if err != nil || thumb == nil {
return nil, err
}
@@ -67,16 +67,6 @@ func (d pngGenerator) GenerateThumbnailOf(src image.Image, width int, height int
}, nil
}
-func (d pngGenerator) GenerateThumbnailImageOf(src image.Image, width int, height int, method string, ctx rcontext.RequestContext) (image.Image, error) {
- var shouldThumbnail bool
- shouldThumbnail, width, height, _, method = u.AdjustProperties(src, width, height, false, false, method)
- if !shouldThumbnail {
- return nil, nil
- }
-
- return u.MakeThumbnail(src, method, width, height)
-}
-
func init() {
generators = append(generators, pngGenerator{})
}
diff --git a/thumbnailing/thumbnail.go b/thumbnailing/thumbnail.go
index e81f1314..6759ce27 100644
--- a/thumbnailing/thumbnail.go
+++ b/thumbnailing/thumbnail.go
@@ -9,6 +9,7 @@ import (
"github.com/turt2live/matrix-media-repo/common/rcontext"
"github.com/turt2live/matrix-media-repo/thumbnailing/i"
"github.com/turt2live/matrix-media-repo/thumbnailing/m"
+ "github.com/turt2live/matrix-media-repo/thumbnailing/u"
"github.com/turt2live/matrix-media-repo/util"
"github.com/turt2live/matrix-media-repo/util/readers"
)
@@ -43,6 +44,13 @@ func GenerateThumbnail(imgStream io.ReadCloser, contentType string, width int, h
return nil, common.ErrMediaTooLarge
}
+ // TODO: Why does AdjustProperties even take `canAnimate` if it's always been hardcoded to `false`? (see git blame on this comment)
+ var shouldThumbnail bool
+ shouldThumbnail, width, height, _, method = u.AdjustProperties(w, h, width, height, animated, false, method)
+ if !shouldThumbnail {
+ return nil, common.ErrMediaDimensionsTooSmall
+ }
+
return generator.GenerateThumbnail(buffered.GetRewoundReader(), contentType, width, height, method, animated, ctx)
}
diff --git a/thumbnailing/u/dimensions.go b/thumbnailing/u/dimensions.go
index 48659611..1b3665fa 100644
--- a/thumbnailing/u/dimensions.go
+++ b/thumbnailing/u/dimensions.go
@@ -1,12 +1,8 @@
package u
-import (
- "image"
-)
-
-func AdjustProperties(img image.Image, desiredWidth int, desiredHeight int, wantAnimated bool, canAnimate bool, method string) (bool, int, int, bool, string) {
- srcWidth := img.Bounds().Max.X
- srcHeight := img.Bounds().Max.Y
+func AdjustProperties(srcWidth int, srcHeight int, desiredWidth int, desiredHeight int, wantAnimated bool, canAnimate bool, method string) (bool, int, int, bool, string) {
+ //srcWidth := img.Bounds().Max.X
+ //srcHeight := img.Bounds().Max.Y
aspectRatio := float32(srcHeight) / float32(srcWidth)
targetAspectRatio := float32(desiredHeight) / float32(desiredWidth)
--
GitLab