[bugfix] Wrap media reader in length reader to determine length if no content-length given (#848)

* use lengthReader 2 determine fileSize if not given

* update tests

* small fixes

* go fmt

internal/media/manager_test.go

@@ -40,6 +40,177 @@ type ManagerTestSuite struct {
 	MediaStandardTestSuite
 }
 
+func (suite *ManagerTestSuite) TestEmojiProcessBlocking() {
+	ctx := context.Background()
+
+	data := func(_ context.Context) (io.Reader, int, error) {
+		// load bytes from a test image
+		b, err := os.ReadFile("./test/rainbow-original.png")
+		if err != nil {
+			panic(err)
+		}
+		return bytes.NewBuffer(b), len(b), nil
+	}
+
+	emojiID := "01GDQ9G782X42BAMFASKP64343"
+	emojiURI := "http://localhost:8080/emoji/01GDQ9G782X42BAMFASKP64343"
+
+	processingEmoji, err := suite.manager.ProcessEmoji(ctx, data, nil, "rainbow_test", emojiID, emojiURI, nil)
+	suite.NoError(err)
+
+	// do a blocking call to fetch the emoji
+	emoji, err := processingEmoji.LoadEmoji(ctx)
+	suite.NoError(err)
+	suite.NotNil(emoji)
+
+	// make sure it's got the stuff set on it that we expect
+	suite.Equal(emojiID, emoji.ID)
+
+	// file meta should be correctly derived from the image
+	suite.Equal("image/png", emoji.ImageContentType)
+	suite.Equal("image/png", emoji.ImageStaticContentType)
+	suite.Equal(36702, emoji.ImageFileSize)
+
+	// now make sure the emoji is in the database
+	dbEmoji, err := suite.db.GetEmojiByID(ctx, emojiID)
+	suite.NoError(err)
+	suite.NotNil(dbEmoji)
+
+	// make sure the processed emoji file is in storage
+	processedFullBytes, err := suite.storage.Get(ctx, emoji.ImagePath)
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytes)
+
+	// load the processed bytes from our test folder, to compare
+	processedFullBytesExpected, err := os.ReadFile("./test/rainbow-original.png")
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytesExpected)
+
+	// the bytes in storage should be what we expected
+	suite.Equal(processedFullBytesExpected, processedFullBytes)
+
+	// now do the same for the thumbnail and make sure it's what we expected
+	processedStaticBytes, err := suite.storage.Get(ctx, emoji.ImageStaticPath)
+	suite.NoError(err)
+	suite.NotEmpty(processedStaticBytes)
+
+	processedStaticBytesExpected, err := os.ReadFile("./test/rainbow-static.png")
+	suite.NoError(err)
+	suite.NotEmpty(processedStaticBytesExpected)
+
+	suite.Equal(processedStaticBytesExpected, processedStaticBytes)
+}
+
+func (suite *ManagerTestSuite) TestEmojiProcessBlockingTooLarge() {
+	ctx := context.Background()
+
+	data := func(_ context.Context) (io.Reader, int, error) {
+		// load bytes from a test image
+		b, err := os.ReadFile("./test/big-panda.gif")
+		if err != nil {
+			panic(err)
+		}
+		return bytes.NewBuffer(b), len(b), nil
+	}
+
+	emojiID := "01GDQ9G782X42BAMFASKP64343"
+	emojiURI := "http://localhost:8080/emoji/01GDQ9G782X42BAMFASKP64343"
+
+	processingEmoji, err := suite.manager.ProcessEmoji(ctx, data, nil, "big_panda", emojiID, emojiURI, nil)
+	suite.NoError(err)
+
+	// do a blocking call to fetch the emoji
+	emoji, err := processingEmoji.LoadEmoji(ctx)
+	suite.EqualError(err, "store: given emoji fileSize (645688b) is larger than allowed size (51200b)")
+	suite.Nil(emoji)
+}
+
+func (suite *ManagerTestSuite) TestEmojiProcessBlockingTooLargeNoSizeGiven() {
+	ctx := context.Background()
+
+	data := func(_ context.Context) (io.Reader, int, error) {
+		// load bytes from a test image
+		b, err := os.ReadFile("./test/big-panda.gif")
+		if err != nil {
+			panic(err)
+		}
+		return bytes.NewBuffer(b), len(b), nil
+	}
+
+	emojiID := "01GDQ9G782X42BAMFASKP64343"
+	emojiURI := "http://localhost:8080/emoji/01GDQ9G782X42BAMFASKP64343"
+
+	processingEmoji, err := suite.manager.ProcessEmoji(ctx, data, nil, "big_panda", emojiID, emojiURI, nil)
+	suite.NoError(err)
+
+	// do a blocking call to fetch the emoji
+	emoji, err := processingEmoji.LoadEmoji(ctx)
+	suite.EqualError(err, "store: given emoji fileSize (645688b) is larger than allowed size (51200b)")
+	suite.Nil(emoji)
+}
+
+func (suite *ManagerTestSuite) TestEmojiProcessBlockingNoFileSizeGiven() {
+	ctx := context.Background()
+
+	data := func(_ context.Context) (io.Reader, int, error) {
+		// load bytes from a test image
+		b, err := os.ReadFile("./test/rainbow-original.png")
+		if err != nil {
+			panic(err)
+		}
+		return bytes.NewBuffer(b), -1, nil
+	}
+
+	emojiID := "01GDQ9G782X42BAMFASKP64343"
+	emojiURI := "http://localhost:8080/emoji/01GDQ9G782X42BAMFASKP64343"
+
+	// process the media with no additional info provided
+	processingEmoji, err := suite.manager.ProcessEmoji(ctx, data, nil, "rainbow_test", emojiID, emojiURI, nil)
+	suite.NoError(err)
+
+	// do a blocking call to fetch the emoji
+	emoji, err := processingEmoji.LoadEmoji(ctx)
+	suite.NoError(err)
+	suite.NotNil(emoji)
+
+	// make sure it's got the stuff set on it that we expect
+	suite.Equal(emojiID, emoji.ID)
+
+	// file meta should be correctly derived from the image
+	suite.Equal("image/png", emoji.ImageContentType)
+	suite.Equal("image/png", emoji.ImageStaticContentType)
+	suite.Equal(36702, emoji.ImageFileSize)
+
+	// now make sure the emoji is in the database
+	dbEmoji, err := suite.db.GetEmojiByID(ctx, emojiID)
+	suite.NoError(err)
+	suite.NotNil(dbEmoji)
+
+	// make sure the processed emoji file is in storage
+	processedFullBytes, err := suite.storage.Get(ctx, emoji.ImagePath)
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytes)
+
+	// load the processed bytes from our test folder, to compare
+	processedFullBytesExpected, err := os.ReadFile("./test/rainbow-original.png")
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytesExpected)
+
+	// the bytes in storage should be what we expected
+	suite.Equal(processedFullBytesExpected, processedFullBytes)
+
+	// now do the same for the thumbnail and make sure it's what we expected
+	processedStaticBytes, err := suite.storage.Get(ctx, emoji.ImageStaticPath)
+	suite.NoError(err)
+	suite.NotEmpty(processedStaticBytes)
+
+	processedStaticBytesExpected, err := os.ReadFile("./test/rainbow-static.png")
+	suite.NoError(err)
+	suite.NotEmpty(processedStaticBytesExpected)
+
+	suite.Equal(processedStaticBytesExpected, processedStaticBytes)
+}
+
 func (suite *ManagerTestSuite) TestSimpleJpegProcessBlocking() {
 	ctx := context.Background()
 
@@ -112,6 +283,152 @@ func (suite *ManagerTestSuite) TestSimpleJpegProcessBlocking() {
 	suite.Equal(processedThumbnailBytesExpected, processedThumbnailBytes)
 }
 
+func (suite *ManagerTestSuite) TestSimpleJpegProcessBlockingNoContentLengthGiven() {
+	ctx := context.Background()
+
+	data := func(_ context.Context) (io.Reader, int, error) {
+		// load bytes from a test image
+		b, err := os.ReadFile("./test/test-jpeg.jpg")
+		if err != nil {
+			panic(err)
+		}
+		// give length as -1 to indicate unknown
+		return bytes.NewBuffer(b), -1, nil
+	}
+
+	accountID := "01FS1X72SK9ZPW0J1QQ68BD264"
+
+	// process the media with no additional info provided
+	processingMedia, err := suite.manager.ProcessMedia(ctx, data, nil, accountID, nil)
+	suite.NoError(err)
+	// fetch the attachment id from the processing media
+	attachmentID := processingMedia.AttachmentID()
+
+	// do a blocking call to fetch the attachment
+	attachment, err := processingMedia.LoadAttachment(ctx)
+	suite.NoError(err)
+	suite.NotNil(attachment)
+
+	// make sure it's got the stuff set on it that we expect
+	// the attachment ID and accountID we expect
+	suite.Equal(attachmentID, attachment.ID)
+	suite.Equal(accountID, attachment.AccountID)
+
+	// file meta should be correctly derived from the image
+	suite.EqualValues(gtsmodel.Original{
+		Width: 1920, Height: 1080, Size: 2073600, Aspect: 1.7777777777777777,
+	}, attachment.FileMeta.Original)
+	suite.EqualValues(gtsmodel.Small{
+		Width: 512, Height: 288, Size: 147456, Aspect: 1.7777777777777777,
+	}, attachment.FileMeta.Small)
+	suite.Equal("image/jpeg", attachment.File.ContentType)
+	suite.Equal("image/jpeg", attachment.Thumbnail.ContentType)
+	suite.Equal(269739, attachment.File.FileSize)
+	suite.Equal("LiBzRk#6V[WF_NvzV@WY_3rqV@a$", attachment.Blurhash)
+
+	// now make sure the attachment is in the database
+	dbAttachment, err := suite.db.GetAttachmentByID(ctx, attachmentID)
+	suite.NoError(err)
+	suite.NotNil(dbAttachment)
+
+	// make sure the processed file is in storage
+	processedFullBytes, err := suite.storage.Get(ctx, attachment.File.Path)
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytes)
+
+	// load the processed bytes from our test folder, to compare
+	processedFullBytesExpected, err := os.ReadFile("./test/test-jpeg-processed.jpg")
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytesExpected)
+
+	// the bytes in storage should be what we expected
+	suite.Equal(processedFullBytesExpected, processedFullBytes)
+
+	// now do the same for the thumbnail and make sure it's what we expected
+	processedThumbnailBytes, err := suite.storage.Get(ctx, attachment.Thumbnail.Path)
+	suite.NoError(err)
+	suite.NotEmpty(processedThumbnailBytes)
+
+	processedThumbnailBytesExpected, err := os.ReadFile("./test/test-jpeg-thumbnail.jpg")
+	suite.NoError(err)
+	suite.NotEmpty(processedThumbnailBytesExpected)
+
+	suite.Equal(processedThumbnailBytesExpected, processedThumbnailBytes)
+}
+
+func (suite *ManagerTestSuite) TestSimpleJpegProcessBlockingReadCloser() {
+	ctx := context.Background()
+
+	data := func(_ context.Context) (io.Reader, int, error) {
+		// open test image as a file
+		f, err := os.Open("./test/test-jpeg.jpg")
+		if err != nil {
+			panic(err)
+		}
+		// give length as -1 to indicate unknown
+		return f, -1, nil
+	}
+
+	accountID := "01FS1X72SK9ZPW0J1QQ68BD264"
+
+	// process the media with no additional info provided
+	processingMedia, err := suite.manager.ProcessMedia(ctx, data, nil, accountID, nil)
+	suite.NoError(err)
+	// fetch the attachment id from the processing media
+	attachmentID := processingMedia.AttachmentID()
+
+	// do a blocking call to fetch the attachment
+	attachment, err := processingMedia.LoadAttachment(ctx)
+	suite.NoError(err)
+	suite.NotNil(attachment)
+
+	// make sure it's got the stuff set on it that we expect
+	// the attachment ID and accountID we expect
+	suite.Equal(attachmentID, attachment.ID)
+	suite.Equal(accountID, attachment.AccountID)
+
+	// file meta should be correctly derived from the image
+	suite.EqualValues(gtsmodel.Original{
+		Width: 1920, Height: 1080, Size: 2073600, Aspect: 1.7777777777777777,
+	}, attachment.FileMeta.Original)
+	suite.EqualValues(gtsmodel.Small{
+		Width: 512, Height: 288, Size: 147456, Aspect: 1.7777777777777777,
+	}, attachment.FileMeta.Small)
+	suite.Equal("image/jpeg", attachment.File.ContentType)
+	suite.Equal("image/jpeg", attachment.Thumbnail.ContentType)
+	suite.Equal(269739, attachment.File.FileSize)
+	suite.Equal("LiBzRk#6V[WF_NvzV@WY_3rqV@a$", attachment.Blurhash)
+
+	// now make sure the attachment is in the database
+	dbAttachment, err := suite.db.GetAttachmentByID(ctx, attachmentID)
+	suite.NoError(err)
+	suite.NotNil(dbAttachment)
+
+	// make sure the processed file is in storage
+	processedFullBytes, err := suite.storage.Get(ctx, attachment.File.Path)
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytes)
+
+	// load the processed bytes from our test folder, to compare
+	processedFullBytesExpected, err := os.ReadFile("./test/test-jpeg-processed.jpg")
+	suite.NoError(err)
+	suite.NotEmpty(processedFullBytesExpected)
+
+	// the bytes in storage should be what we expected
+	suite.Equal(processedFullBytesExpected, processedFullBytes)
+
+	// now do the same for the thumbnail and make sure it's what we expected
+	processedThumbnailBytes, err := suite.storage.Get(ctx, attachment.Thumbnail.Path)
+	suite.NoError(err)
+	suite.NotEmpty(processedThumbnailBytes)
+
+	processedThumbnailBytesExpected, err := os.ReadFile("./test/test-jpeg-thumbnail.jpg")
+	suite.NoError(err)
+	suite.NotEmpty(processedThumbnailBytesExpected)
+
+	suite.Equal(processedThumbnailBytesExpected, processedThumbnailBytes)
+}
+
 func (suite *ManagerTestSuite) TestPngNoAlphaChannelProcessBlocking() {
 	ctx := context.Background()
 

internal/media/processingemoji.go

@@ -171,11 +171,6 @@ func (p *ProcessingEmoji) store(ctx context.Context) error {
 		return fmt.Errorf("store: error executing data function: %s", err)
 	}
 
-	maxSize := config.GetMediaEmojiRemoteMaxSize()
-	if fileSize > maxSize {
-		return fmt.Errorf("store: emoji size (%db) is larger than allowed emojiRemoteMaxSize (%db)", fileSize, maxSize)
-	}
-
 	// defer closing the reader when we're done with it
 	defer func() {
 		if rc, ok := reader.(io.ReadCloser); ok {
@@ -211,16 +206,42 @@ func (p *ProcessingEmoji) store(ctx context.Context) error {
 	p.emoji.ImageURL = uris.GenerateURIForAttachment(p.instanceAccountID, string(TypeEmoji), string(SizeOriginal), p.emoji.ID, extension)
 	p.emoji.ImagePath = fmt.Sprintf("%s/%s/%s/%s.%s", p.instanceAccountID, TypeEmoji, SizeOriginal, p.emoji.ID, extension)
 	p.emoji.ImageContentType = contentType
-	p.emoji.ImageFileSize = fileSize
 
 	// concatenate the first bytes with the existing bytes still in the reader (thanks Mara)
-	multiReader := io.MultiReader(bytes.NewBuffer(firstBytes), reader)
+	readerToStore := io.MultiReader(bytes.NewBuffer(firstBytes), reader)
+
+	var maxEmojiSize int
+	if p.emoji.Domain == "" {
+		maxEmojiSize = config.GetMediaEmojiLocalMaxSize()
+	} else {
+		maxEmojiSize = config.GetMediaEmojiRemoteMaxSize()
+	}
+
+	// if we know the fileSize already, make sure it's not bigger than our limit
+	var checkedSize bool
+	if fileSize > 0 {
+		checkedSize = true
+		if fileSize > maxEmojiSize {
+			return fmt.Errorf("store: given emoji fileSize (%db) is larger than allowed size (%db)", fileSize, maxEmojiSize)
+		}
+	}
 
 	// store this for now -- other processes can pull it out of storage as they please
-	if err := p.storage.PutStream(ctx, p.emoji.ImagePath, multiReader); err != nil && err != storage.ErrAlreadyExists {
+	if fileSize, err = putStream(ctx, p.storage, p.emoji.ImagePath, readerToStore, fileSize); err != nil && err != storage.ErrAlreadyExists {
 		return fmt.Errorf("store: error storing stream: %s", err)
 	}
 
+	// if we didn't know the fileSize yet, we do now, so check if we need to
+	if !checkedSize && fileSize > maxEmojiSize {
+		defer func() {
+			if err := p.storage.Delete(ctx, p.emoji.ImagePath); err != nil {
+				log.Errorf("store: error removing too-large emoji from the store: %s", err)
+			}
+		}()
+		return fmt.Errorf("store: discovered emoji fileSize (%db) is larger than allowed emojiRemoteMaxSize (%db)", fileSize, maxEmojiSize)
+	}
+
+	p.emoji.ImageFileSize = fileSize
 	p.read = true
 
 	if p.postData != nil {

internal/media/processingmedia.go

@@ -268,7 +268,6 @@ func (p *ProcessingMedia) store(ctx context.Context) error {
 	if err != nil {
 		return fmt.Errorf("store: error executing data function: %s", err)
 	}
-	log.Tracef("store: reading %d bytes from data function for media %s", fileSize, p.attachment.URL)
 
 	// defer closing the reader when we're done with it
 	defer func() {
@@ -304,49 +303,48 @@ func (p *ProcessingMedia) store(ctx context.Context) error {
 	extension := split[1] // something like 'jpeg'
 
 	// concatenate the cleaned up first bytes with the existing bytes still in the reader (thanks Mara)
-	multiReader := io.MultiReader(bytes.NewBuffer(firstBytes), reader)
+	readerToStore := io.MultiReader(bytes.NewBuffer(firstBytes), reader)
 
-	// we'll need to clean exif data from the first bytes; while we're
+	// use the extension to derive the attachment type
-	// here, we can also use the extension to derive the attachment type
+	// and, while we're in here, clean up exif data from
-	var clean io.Reader
+	// the image if we already know the fileSize
 	switch extension {
 	case mimeGif:
 		p.attachment.Type = gtsmodel.FileTypeImage
-		clean = multiReader // nothing to clean from a gif
 	case mimeJpeg, mimePng:
 		p.attachment.Type = gtsmodel.FileTypeImage
-		purged, err := terminator.Terminate(multiReader, fileSize, extension)
+		if fileSize > 0 {
-		if err != nil {
+			var err error
-			return fmt.Errorf("store: exif error: %s", err)
+			readerToStore, err = terminator.Terminate(readerToStore, fileSize, extension)
+			if err != nil {
+				return fmt.Errorf("store: exif error: %s", err)
+			}
+			defer func() {
+				if rc, ok := readerToStore.(io.ReadCloser); ok {
+					if err := rc.Close(); err != nil {
+						log.Errorf("store: error closing terminator reader: %s", err)
+					}
+				}
+			}()
 		}
-		clean = purged
 	default:
 		return fmt.Errorf("store: couldn't process %s", extension)
 	}
 
-	// defer closing the clean reader when we're done with it
-	defer func() {
-		if rc, ok := clean.(io.ReadCloser); ok {
-			if err := rc.Close(); err != nil {
-				log.Errorf("store: error closing clean readcloser: %s", err)
-			}
-		}
-	}()
-
 	// now set some additional fields on the attachment since
 	// we know more about what the underlying media actually is
 	p.attachment.URL = uris.GenerateURIForAttachment(p.attachment.AccountID, string(TypeAttachment), string(SizeOriginal), p.attachment.ID, extension)
-	p.attachment.File.Path = fmt.Sprintf("%s/%s/%s/%s.%s", p.attachment.AccountID, TypeAttachment, SizeOriginal, p.attachment.ID, extension)
 	p.attachment.File.ContentType = contentType
-	p.attachment.File.FileSize = fileSize
+	p.attachment.File.Path = fmt.Sprintf("%s/%s/%s/%s.%s", p.attachment.AccountID, TypeAttachment, SizeOriginal, p.attachment.ID, extension)
 
 	// store this for now -- other processes can pull it out of storage as they please
-	if err := p.storage.PutStream(ctx, p.attachment.File.Path, clean); err != nil && err != storage.ErrAlreadyExists {
+	if fileSize, err = putStream(ctx, p.storage, p.attachment.File.Path, readerToStore, fileSize); err != nil && err != storage.ErrAlreadyExists {
 		return fmt.Errorf("store: error storing stream: %s", err)
 	}
 
 	cached := true
 	p.attachment.Cached = &cached
+	p.attachment.File.FileSize = fileSize
 	p.read = true
 
 	if p.postData != nil {

internal/media/util.go

@@ -19,12 +19,15 @@
 package media
 
 import (
+	"context"
 	"errors"
 	"fmt"
+	"io"
 	"time"
 
 	"github.com/h2non/filetype"
 	"github.com/superseriousbusiness/gotosocial/internal/log"
+	"github.com/superseriousbusiness/gotosocial/internal/storage"
 )
 
 // AllSupportedMIMETypes just returns all media
@@ -144,3 +147,31 @@ func parseOlderThan(olderThanDays int) (time.Time, error) {
 
 	return olderThan, nil
 }
+
+// lengthReader wraps a reader and reads the length of total bytes written as it goes.
+type lengthReader struct {
+	source io.Reader
+	length int
+}
+
+func (r *lengthReader) Read(b []byte) (int, error) {
+	n, err := r.source.Read(b)
+	r.length += n
+	return n, err
+}
+
+// putStream either puts a file with a known fileSize into storage directly, and returns the
+// fileSize unchanged, or it wraps the reader with a lengthReader and returns the discovered
+// fileSize.
+func putStream(ctx context.Context, storage storage.Driver, key string, r io.Reader, fileSize int) (int, error) {
+	if fileSize > 0 {
+		return fileSize, storage.PutStream(ctx, key, r)
+	}
+
+	lr := &lengthReader{
+		source: r,
+	}
+
+	err := storage.PutStream(ctx, key, lr)
+	return lr.length, err
+}