commit 73e9cca7019c15a92cb4cd320034652590513198
parent 55ad6dee716112e1a6c95cd53af0680ab3e8679a
Author: tobi <31960611+tsmethurst@users.noreply.github.com>
Date: Mon, 21 Mar 2022 13:41:44 +0100
[bugfix] Close ReadClosers properly in the media package (#434)
* defer lock reader
* close readers when finished with them
* close the reader in the teereader when finished
Diffstat:
4 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/internal/media/processingemoji.go b/internal/media/processingemoji.go
@@ -29,6 +29,7 @@ import (
"time"
"codeberg.org/gruf/go-store/kv"
+ "github.com/sirupsen/logrus"
"github.com/superseriousbusiness/gotosocial/internal/db"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/uris"
@@ -169,6 +170,15 @@ func (p *ProcessingEmoji) store(ctx context.Context) error {
return fmt.Errorf("store: error executing data function: %s", err)
}
+ // defer closing the reader when we're done with it
+ defer func() {
+ if rc, ok := reader.(io.ReadCloser); ok {
+ if err := rc.Close(); err != nil {
+ logrus.Errorf("store: error closing readcloser: %s", err)
+ }
+ }
+ }()
+
// extract no more than 261 bytes from the beginning of the file -- this is the header
firstBytes := make([]byte, maxFileHeaderBytes)
if _, err := reader.Read(firstBytes); err != nil {
@@ -205,13 +215,6 @@ func (p *ProcessingEmoji) store(ctx context.Context) error {
return fmt.Errorf("store: error storing stream: %s", err)
}
- // if the original reader is a readcloser, close it since we're done with it now
- if rc, ok := reader.(io.ReadCloser); ok {
- if err := rc.Close(); err != nil {
- return fmt.Errorf("store: error closing readcloser: %s", err)
- }
- }
-
p.read = true
if p.postData != nil {
diff --git a/internal/media/processingmedia.go b/internal/media/processingmedia.go
@@ -29,6 +29,7 @@ import (
"time"
"codeberg.org/gruf/go-store/kv"
+ "github.com/sirupsen/logrus"
terminator "github.com/superseriousbusiness/exif-terminator"
"github.com/superseriousbusiness/gotosocial/internal/db"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
@@ -260,6 +261,15 @@ func (p *ProcessingMedia) store(ctx context.Context) error {
return fmt.Errorf("store: error executing data function: %s", err)
}
+ // defer closing the reader when we're done with it
+ defer func() {
+ if rc, ok := reader.(io.ReadCloser); ok {
+ if err := rc.Close(); err != nil {
+ logrus.Errorf("store: error closing readcloser: %s", err)
+ }
+ }
+ }()
+
// extract no more than 261 bytes from the beginning of the file -- this is the header
firstBytes := make([]byte, maxFileHeaderBytes)
if _, err := reader.Read(firstBytes); err != nil {
@@ -305,6 +315,15 @@ func (p *ProcessingMedia) store(ctx context.Context) error {
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 {
+ logrus.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)
@@ -317,14 +336,6 @@ func (p *ProcessingMedia) store(ctx context.Context) error {
return fmt.Errorf("store: error storing stream: %s", err)
}
p.attachment.Cached = true
-
- // if the original reader is a readcloser, close it since we're done with it now
- if rc, ok := reader.(io.ReadCloser); ok {
- if err := rc.Close(); err != nil {
- return fmt.Errorf("store: error closing readcloser: %s", err)
- }
- }
-
p.read = true
if p.postData != nil {
diff --git a/internal/processing/media/getfile.go b/internal/processing/media/getfile.go
@@ -180,9 +180,15 @@ func (p *processor) getAttachmentContent(ctx context.Context, requestingAccount
return nil, 0, err
}
- // everything read from the readCloser by the media manager will be written into the bufferedWriter
- teeReader := io.TeeReader(readCloser, bufferedWriter)
- return teeReader, fileSize, nil
+ // Make a TeeReader so that everything read from the readCloser by the media manager will be written into the bufferedWriter.
+ // We wrap this in a teeReadCloser which implements io.ReadCloser, so that whoever uses the teeReader can close the readCloser
+ // when they're done with it.
+ trc := teeReadCloser{
+ teeReader: io.TeeReader(readCloser, bufferedWriter),
+ close: readCloser.Close,
+ }
+
+ return trc, fileSize, nil
}
// close the pipewriter after data has been piped into it, so the reader on the other side doesn't block;
diff --git a/internal/processing/media/util.go b/internal/processing/media/util.go
@@ -20,6 +20,7 @@ package media
import (
"fmt"
+ "io"
"strconv"
"strings"
)
@@ -61,3 +62,16 @@ func parseFocus(focus string) (focusx, focusy float32, err error) {
focusy = float32(fy)
return
}
+
+type teeReadCloser struct {
+ teeReader io.Reader
+ close func() error
+}
+
+func (t teeReadCloser) Read(p []byte) (n int, err error) {
+ return t.teeReader.Read(p)
+}
+
+func (t teeReadCloser) Close() error {
+ return t.close()
+}
