Fixes #1957: Ensure unique S3 object keys by adding numeric suffix.

cmd/media.go

@@ -57,8 +57,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

@@ -35,6 +35,14 @@ 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)
+	newName := fmt.Sprintf("%s_%s%s", name, suffix, ext)
+	return newName
+}
+
 // 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 {

internal/media/providers/s3/s3.go

@@ -64,15 +64,16 @@ func NewS3Store(opt Opt) (media.Store, error) {
 
 // Put takes in the filename, the content type and file object itself and uploads to S3.
 func (c *Client) Put(name string, cType string, file io.ReadSeeker) (string, error) {
+	// Paths inside the bucket should not start with /.
+	objectkey := c.makeBucketPath(name)
+
 	// Upload input parameters
 	p := simples3.UploadInput{
 		Bucket:      c.opts.Bucket,
 		ContentType: cType,
 		FileName:    name,
 		Body:        file,
-
-		// Paths inside the bucket should not start with /.
-		ObjectKey: c.makeBucketPath(name),
+		ObjectKey:   objectkey,
 	}
 
 	if c.opts.BucketType == "public" {