Ensure unique upload filenames by adding a suffix (#1963)

Fixes #1957.

Co-authored-by: Abhinav Raut <abhinav.raut@zerodha.com>

cmd/media.go

@@ -61,8 +61,14 @@ func handleUploadMedia(c echo.Context) error {
 		}
 	}
 
-	// Upload the file.
+	// Sanitize filename.
 	fName := makeFilename(file.Filename)
+
+	// Add a random suffix to the filename to ensure uniqueness.
+	suffix, _ := generateRandomString(6)
+	fName = appendSuffixToFilename(fName, suffix)
+
+	// Upload the file.
 	fName, err = app.media.Put(fName, contentType, src)
 	if err != nil {
 		app.log.Printf("error uploading file: %v", err)

cmd/utils.go

@@ -36,6 +36,13 @@ func makeFilename(fName string) string {
 	return filepath.Base(name)
 }
 
+// appendSuffixToFilename adds a string suffix to the filename while keeping the file extension.
+func appendSuffixToFilename(filename, suffix string) string {
+	ext := filepath.Ext(filename)
+	name := strings.TrimSuffix(filename, ext)
+	return fmt.Sprintf("%s_%s%s", name, suffix, ext)
+}
+
 // makeMsgTpl takes a page title, heading, and message and returns
 // a msgTpl that can be rendered as an HTML view. This is used for
 // rendering arbitrary HTML views with error and success messages.

internal/media/providers/filesystem/filesystem.go

@@ -1,19 +1,14 @@
 package filesystem
 
 import (
-	"crypto/rand"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
-	"regexp"
-	"strconv"
 
 	"github.com/knadh/listmonk/internal/media"
 )
 
-const tmpFilePrefix = "listmonk"
-
 // Opts represents filesystem params
 type Opts struct {
 	UploadPath string `koanf:"upload_path"`
@@ -26,12 +21,6 @@ type Client struct {
 	opts Opts
 }
 
-// This matches filenames, sans extensions, of the format
-// filename_(number). The number is incremented in case
-// new file uploads conflict with existing filenames
-// on the filesystem.
-var fnameRegexp = regexp.MustCompile(`(.+?)_([0-9]+)$`)
-
 // New initialises store for Filesystem provider.
 func New(opts Opts) (media.Store, error) {
 	return &Client{
@@ -45,7 +34,6 @@ func (c *Client) Put(filename string, cType string, src io.ReadSeeker) (string,
 
 	// Get the directory path
 	dir := getDir(c.opts.UploadPath)
-	filename = assertUniqueFilename(dir, filename)
 	o, err := os.OpenFile(filepath.Join(dir, filename), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0664)
 	if err != nil {
 		return "", err
@@ -80,49 +68,6 @@ func (c *Client) Delete(file string) error {
 	return nil
 }
 
-// assertUniqueFilename takes a file path and check if it exists on the disk. If it doesn't,
-// it returns the same name and if it does, it adds a small random hash to the filename
-// and returns that.
-func assertUniqueFilename(dir, fileName string) string {
-	var (
-		ext  = filepath.Ext(fileName)
-		base = fileName[0 : len(fileName)-len(ext)]
-		num  = 0
-	)
-
-	for {
-		// There's no name conflict.
-		if _, err := os.Stat(filepath.Join(dir, fileName)); os.IsNotExist(err) {
-			return fileName
-		}
-
-		// Does the name match the _(num) syntax?
-		r := fnameRegexp.FindAllStringSubmatch(fileName, -1)
-		if len(r) == 1 && len(r[0]) == 3 {
-			num, _ = strconv.Atoi(r[0][2])
-		}
-		num++
-
-		fileName = fmt.Sprintf("%s_%d%s", base, num, ext)
-	}
-}
-
-// generateRandomString generates a cryptographically random, alphanumeric string of length n.
-func generateRandomString(n int) (string, error) {
-	const dictionary = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
-
-	var bytes = make([]byte, n)
-	if _, err := rand.Read(bytes); err != nil {
-		return "", err
-	}
-
-	for k, v := range bytes {
-		bytes[k] = dictionary[v%byte(len(dictionary))]
-	}
-
-	return string(bytes), nil
-}
-
 // getDir returns the current working directory path if no directory is specified,
 // else returns the directory path specified itself.
 func getDir(dir string) string {