Repository: inbucket/inbucket
Pull request #449 (2023-11-29 21:52:47)

storage: $ can be used in place of : in filestore path
Signed-off-by: James Hillyerd <james@hillyerd.com>



Patch:
diff --git a/doc/config.md b/doc/config.md
index e741d02..f555d5a 100644
--- a/doc/config.md
+++ b/doc/config.md
@@ -442,7 +442,8 @@ separated list of key:value pairs.
 #### `file` type parameters
 
 - `path`: Operating system specific path to the directory where mail should be
-  stored.
+  stored.  `$` characters will be replaced with `:` in the final path value,
+  allowing Windows drive letters, i.e. `D$\inbucket`.
 
 #### `memory` type parameters
 
diff --git a/pkg/storage/file/fstore.go b/pkg/storage/file/fstore.go
index b873533..82358f4 100644
--- a/pkg/storage/file/fstore.go
+++ b/pkg/storage/file/fstore.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 
@@ -23,7 +24,7 @@ const indexFileName = "index.gob"
 var (
 	// countChannel is filled with a sequential numbers (0000..9999), which are
 	// used by generateID() to generate unique message IDs.  It's global
-	// because we only want one regardless of the number of DataStore objects
+	// because we only want one regardless of the number of DataStore objects.
 	countChannel = make(chan int, 10)
 )
 
@@ -32,7 +33,7 @@ func init() {
 	go countGenerator(countChannel)
 }
 
-// Populates the channel with numbers
+// Populates the channel with numbers.
 func countGenerator(c chan int) {
 	for i := 0; true; i = (i + 1) % 10000 {
 		c <- i
@@ -40,7 +41,7 @@ func countGenerator(c chan int) {
 }
 
 // Store implements DataStore aand is the root of the mail storage
-// hiearchy.  It provides access to Mailbox objects
+// hiearchy.  It provides access to Mailbox objects.
 type Store struct {
 	hashLock      storage.HashLock
 	path          string
@@ -50,13 +51,14 @@ type Store struct {
 	extHost       *extension.Host
 }
 
-// New creates a new DataStore object using the specified path
+// New creates a new DataStore object using the specified path.
 func New(cfg config.Storage, extHost *extension.Host) (storage.Store, error) {
 	path := cfg.Params["path"]
 	if path == "" {
 		return nil, fmt.Errorf("'path' parameter not specified")
 	}
-	mailPath := filepath.Join(path, "mail")
+
+	mailPath := getMailPath(path)
 	if _, err := os.Stat(mailPath); err != nil {
 		// Mail datastore does not yet exist, create it.
 		if err = os.MkdirAll(mailPath, 0770); err != nil {
@@ -88,16 +90,19 @@ func (fs *Store) AddMessage(m storage.Message) (id string, err error) {
 	if err != nil {
 		return "", err
 	}
+
 	// Create a new message.
 	fm, err := mb.newMessage()
 	if err != nil {
 		return "", err
 	}
+
 	// Ensure mailbox directory exists.
 	if err := mb.createDir(); err != nil {
 		return "", err
 	}
-	// Write the message content
+
+	// Write the message content.
 	file, err := os.Create(fm.rawPath())
 	if err != nil {
 		return "", err
@@ -105,23 +110,24 @@ func (fs *Store) AddMessage(m storage.Message) (id string, err error) {
 	w := bufio.NewWriter(file)
 	size, err := io.Copy(w, r)
 	if err != nil {
-		// Try to remove the file
+		// Try to remove the file.
 		_ = file.Close()
 		_ = os.Remove(fm.rawPath())
 		return "", err
 	}
 	_ = r.Close()
 	if err := w.Flush(); err != nil {
-		// Try to remove the file
+		// Try to remove the file.
 		_ = file.Close()
 		_ = os.Remove(fm.rawPath())
 		return "", err
 	}
 	if err := file.Close(); err != nil {
-		// Try to remove the file
+		// Try to remove the file.
 		_ = os.Remove(fm.rawPath())
 		return "", err
 	}
+
 	// Update the index.
 	fm.Fdate = m.Date()
 	fm.Ffrom = m.From()
@@ -130,10 +136,11 @@ func (fs *Store) AddMessage(m storage.Message) (id string, err error) {
 	fm.Fsubject = m.Subject()
 	mb.messages = append(mb.messages, fm)
 	if err := mb.writeIndex(); err != nil {
-		// Try to remove the file
+		// Try to remove the file.
 		_ = os.Remove(fm.rawPath())
 		return "", err
 	}
+
 	return fm.Fid, nil
 }
 
@@ -158,11 +165,13 @@ func (fs *Store) MarkSeen(mailbox, id string) error {
 	mb := fs.mbox(mailbox)
 	mb.Lock()
 	defer mb.Unlock()
+
 	if !mb.indexLoaded {
 		if err := mb.readIndex(); err != nil {
 			return err
 		}
 	}
+
 	for _, m := range mb.messages {
 		if m.Fid == id {
 			if m.Fseen {
@@ -173,6 +182,7 @@ func (fs *Store) MarkSeen(mailbox, id string) error {
 			break
 		}
 	}
+
 	return mb.writeIndex()
 }
 
@@ -210,19 +220,22 @@ func (fs *Store) VisitMailboxes(f func([]storage.Message) (cont bool)) error {
 	if err != nil {
 		return err
 	}
-	// Loop over level 1 directories
+
+	// Loop over level 1 directories.
 	for _, name1 := range names1 {
 		names2, err := readDirNames(fs.mailPath, name1)
 		if err != nil {
 			return err
 		}
-		// Loop over level 2 directories
+
+		// Loop over level 2 directories.
 		for _, name2 := range names2 {
 			names3, err := readDirNames(fs.mailPath, name1, name2)
 			if err != nil {
 				return err
 			}
-			// Loop over mailboxes
+
+			// Loop over mailboxes.
 			for _, name3 := range names3 {
 				mb := fs.mboxFromHash(name3)
 				mb.RLock()
@@ -247,6 +260,7 @@ func (fs *Store) mbox(mailbox string) *mbox {
 	s2 := hash[0:6]
 	path := filepath.Join(fs.mailPath, s1, s2, hash)
 	indexPath := filepath.Join(path, indexFileName)
+
 	return &mbox{
 		RWMutex:   fs.hashLock.Get(hash),
 		store:     fs,
@@ -263,6 +277,7 @@ func (fs *Store) mboxFromHash(hash string) *mbox {
 	s2 := hash[0:6]
 	path := filepath.Join(fs.mailPath, s1, s2, hash)
 	indexPath := filepath.Join(path, indexFileName)
+
 	return &mbox{
 		RWMutex:   fs.hashLock.Get(hash),
 		store:     fs,
@@ -297,6 +312,14 @@ func generateID(date time.Time) string {
 	return generatePrefix(date) + "-" + fmt.Sprintf("%04d", <-countChannel)
 }
 
+// getMailPath converts a filestore `path` parameter into the effective mail store path.
+// Within the path, '$' is replaced with ':' to support Windows drive letters with our
+// env->config map syntax.
+func getMailPath(base string) string {
+	path := strings.ReplaceAll(base, "$", ":")
+	return filepath.Join(path, "mail")
+}
+
 // readDirNames returns a slice of filenames in the specified directory or an error.
 func readDirNames(elem ...string) ([]string, error) {
 	f, err := os.Open(filepath.Join(elem...))


Test patch:
diff --git a/pkg/storage/file/fstore_test.go b/pkg/storage/file/fstore_test.go
index fccd835..794c534 100644
--- a/pkg/storage/file/fstore_test.go
+++ b/pkg/storage/file/fstore_test.go
@@ -19,6 +19,7 @@ import (
 	"github.com/inbucket/inbucket/v3/pkg/storage"
 	"github.com/inbucket/inbucket/v3/pkg/test"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // TestSuite runs storage package test suite on file store.
@@ -33,6 +34,24 @@ func TestSuite(t *testing.T) {
 		})
 }
 
+// Test filestore initialization.
+func TestFSNew(t *testing.T) {
+	// Should fail if no path specified.
+	ds, err := New(config.Storage{}, extension.NewHost())
+	assert.ErrorContains(t, err, "parameter not specified")
+	assert.Nil(t, ds)
+}
+
+func TestFSGetMailPath(t *testing.T) {
+	// Path should have `mail` dir appended.
+	got := getMailPath(`one`)
+	assert.Regexp(t, "^one.mail$", got, "Expected one/mail or similar")
+
+	// Path should convert `$` to `:`.
+	got = getMailPath(`C$\inbucket`)
+	assert.Regexp(t, "^C:.inbucket.mail$", got, "Expected C:\\inbucket\\mail or similar")
+}
+
 // Test directory structure created by filestore
 func TestFSDirStructure(t *testing.T) {
 	ds, logbuf := setupDataStore(config.Storage{}, extension.NewHost())
@@ -167,14 +186,14 @@ func TestGetLatestMessage(t *testing.T) {
 
 	// Test get the latest message
 	msg, err = ds.GetMessage(mbName, "latest")
-	assert.Nil(t, err)
+	require.NoError(t, err)
 	assert.True(t, msg.ID() == id2, "Expected %q to be equal to %q", msg.ID(), id2)
 
 	// Deliver test message 3
 	id3, _ := deliverMessage(ds, mbName, "test 3", time.Now())
 
 	msg, err = ds.GetMessage(mbName, "latest")
-	assert.Nil(t, err)
+	require.NoError(t, err)
 	assert.True(t, msg.ID() == id3, "Expected %q to be equal to %q", msg.ID(), id3)
 
 	// Test wrong id