From 904ffacb0e521218ba1f41e2e5c26d9ac41c9969 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= <mcsinyx@disroot.org>
Date: Thu, 20 Jan 2022 01:10:08 +0700
Subject: [PATCH] maildir,notmuch: avoid leaking open files

Previously, Message.NewReader returned the wrapped buffered reader
without a reference to the opened file, so the files descriptors
were left unclosed after reading.  Now, the file reader is returned
directly and closed on the call site.  Buffering is not needed here
because it is an implementation detail of go-message.

Fixes: https://todo.sr.ht/~rjarry/aerc/9
---
 worker/lib/parse.go       |  3 ++-
 worker/lib/parse_test.go  | 20 +++++++-------------
 worker/maildir/message.go |  9 ++-------
 worker/maildir/search.go  |  1 +
 worker/maildir/worker.go  |  1 +
 worker/notmuch/message.go |  9 ++-------
 worker/notmuch/worker.go  |  1 +
 7 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/worker/lib/parse.go b/worker/lib/parse.go
index 87f94bd..6d73a1f 100644
--- a/worker/lib/parse.go
+++ b/worker/lib/parse.go
@@ -211,7 +211,7 @@ func parseAddressList(h *mail.Header, key string) ([]*mail.Address, error) {
 
 // RawMessage is an interface that describes a raw message
 type RawMessage interface {
-	NewReader() (io.Reader, error)
+	NewReader() (io.ReadCloser, error)
 	ModelFlags() ([]models.Flag, error)
 	Labels() ([]string, error)
 	UID() uint32
@@ -225,6 +225,7 @@ func MessageInfo(raw RawMessage) (*models.MessageInfo, error) {
 	if err != nil {
 		return nil, err
 	}
+	defer r.Close()
 	msg, err := message.Read(r)
 	if err != nil {
 		return nil, fmt.Errorf("could not read message: %v", err)
diff --git a/worker/lib/parse_test.go b/worker/lib/parse_test.go
index 5c0a1b1..1219098 100644
--- a/worker/lib/parse_test.go
+++ b/worker/lib/parse_test.go
@@ -1,9 +1,9 @@
 package lib
 
 import (
-	"bytes"
 	"io"
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -36,23 +36,17 @@ func TestMessageInfoHandledError(t *testing.T) {
 }
 
 type mockRawMessage struct {
-	body []byte
-}
-
-func newMockRawMessage(body []byte) *mockRawMessage {
-	return &mockRawMessage{
-		body: body,
-	}
+	path string
 }
 
 func newMockRawMessageFromPath(p string) *mockRawMessage {
-	b, err := ioutil.ReadFile(p)
-	die(err)
-	return newMockRawMessage(b)
+	return &mockRawMessage{
+		path: p,
+	}
 }
 
-func (m *mockRawMessage) NewReader() (io.Reader, error) {
-	return bytes.NewReader(m.body), nil
+func (m *mockRawMessage) NewReader() (io.ReadCloser, error) {
+	return os.Open(m.path)
 }
 func (m *mockRawMessage) ModelFlags() ([]models.Flag, error) { return nil, nil }
 func (m *mockRawMessage) Labels() ([]string, error)          { return nil, nil }
diff --git a/worker/maildir/message.go b/worker/maildir/message.go
index dbc9ade..b73e083 100644
--- a/worker/maildir/message.go
+++ b/worker/maildir/message.go
@@ -1,7 +1,6 @@
 package maildir
 
 import (
-	"bufio"
 	"fmt"
 	"io"
 
@@ -20,12 +19,8 @@ type Message struct {
 }
 
 // NewReader reads a message into memory and returns an io.Reader for it.
-func (m Message) NewReader() (io.Reader, error) {
-	f, err := m.dir.Open(m.key)
-	if err != nil {
-		return nil, err
-	}
-	return bufio.NewReader(f), nil
+func (m Message) NewReader() (io.ReadCloser, error) {
+	return m.dir.Open(m.key)
 }
 
 // Flags fetches the set of flags currently applied to the message.
diff --git a/worker/maildir/search.go b/worker/maildir/search.go
index edfb8d8..7f97fb1 100644
--- a/worker/maildir/search.go
+++ b/worker/maildir/search.go
@@ -159,6 +159,7 @@ func (w *Worker) searchKey(key uint32, criteria *searchCriteria,
 		if err != nil {
 			return false, err
 		}
+		defer reader.Close()
 		bytes, err := ioutil.ReadAll(reader)
 		if err != nil {
 			return false, err
diff --git a/worker/maildir/worker.go b/worker/maildir/worker.go
index 621b19e..a671d73 100644
--- a/worker/maildir/worker.go
+++ b/worker/maildir/worker.go
@@ -432,6 +432,7 @@ func (w *Worker) handleFetchFullMessages(msg *types.FetchFullMessages) error {
 			w.worker.Logger.Printf("could not get message reader: %v", err)
 			return err
 		}
+		defer r.Close()
 		w.worker.PostMessage(&types.FullMessage{
 			Message: types.RespondTo(msg),
 			Content: &models.FullMessage{
diff --git a/worker/notmuch/message.go b/worker/notmuch/message.go
index c95c9ba..e39fb17 100644
--- a/worker/notmuch/message.go
+++ b/worker/notmuch/message.go
@@ -4,7 +4,6 @@
 package notmuch
 
 import (
-	"bufio"
 	"fmt"
 	"io"
 	"os"
@@ -23,16 +22,12 @@ type Message struct {
 }
 
 // NewReader returns a reader for a message
-func (m *Message) NewReader() (io.Reader, error) {
+func (m *Message) NewReader() (io.ReadCloser, error) {
 	name, err := m.Filename()
 	if err != nil {
 		return nil, err
 	}
-	f, err := os.Open(name)
-	if err != nil {
-		return nil, err
-	}
-	return bufio.NewReader(f), nil
+	return os.Open(name)
 }
 
 // MessageInfo populates a models.MessageInfo struct for the message.
diff --git a/worker/notmuch/worker.go b/worker/notmuch/worker.go
index 575cd56..d95d6ba 100644
--- a/worker/notmuch/worker.go
+++ b/worker/notmuch/worker.go
@@ -367,6 +367,7 @@ func (w *worker) handleFetchFullMessages(msg *types.FetchFullMessages) error {
 			w.w.Logger.Printf("could not get message reader: %v", err)
 			return err
 		}
+		defer r.Close()
 		w.w.PostMessage(&types.FullMessage{
 			Message: types.RespondTo(msg),
 			Content: &models.FullMessage{