Merge pull request #179 from abhinavxd/fix-mimetype-detection

fix: improve MIME type detection and content disposition handling for S3 store.

cmd/media.go

@@ -20,10 +20,6 @@ import (
 	"github.com/zerodha/fastglue"
 )
 
-const (
-	thumbPrefix = "thumb_"
-)
-
 // handleMediaUpload handles media uploads.
 func handleMediaUpload(r *fastglue.Request) error {
 	var (
@@ -70,6 +66,12 @@ func handleMediaUpload(r *fastglue.Request) error {
 	srcFileSize := fileHeader.Size
 	srcExt := strings.TrimPrefix(strings.ToLower(filepath.Ext(srcFileName)), ".")
 
+	// Check if file is empty
+	if srcFileSize == 0 {
+		app.lo.Error("error: uploaded file is empty (0 bytes)")
+		return r.SendErrorEnvelope(fasthttp.StatusBadRequest, app.i18n.T("media.fileEmpty"), nil, envelope.InputError)
+	}
+
 	// Check file size
 	consts := app.consts.Load().(*constants)
 	if bytesToMegabytes(srcFileSize) > float64(consts.MaxFileUploadSizeMB) {
@@ -88,7 +90,7 @@ func handleMediaUpload(r *fastglue.Request) error {
 
 	// Delete files on any error.
 	var uuid = uuid.New()
-	thumbName := thumbPrefix + uuid.String()
+	thumbName := image.ThumbPrefix + uuid.String()
 	defer func() {
 		if cleanUp {
 			app.media.Delete(uuid.String())
@@ -105,7 +107,7 @@ func handleMediaUpload(r *fastglue.Request) error {
 			app.lo.Error("error creating thumb image", "error", err)
 			return r.SendErrorEnvelope(fasthttp.StatusInternalServerError, app.i18n.Ts("globals.messages.errorCreating", "name", "{globals.terms.thumbnail}"), nil, envelope.GeneralError)
 		}
-		thumbName, err = app.media.Upload(thumbName, srcContentType, thumbFile)
+		thumbName, _, err = app.media.Upload(thumbName, srcContentType, thumbFile)
 		if err != nil {
 			return sendErrorEnvelope(r, err)
 		}
@@ -124,8 +126,11 @@ func handleMediaUpload(r *fastglue.Request) error {
 		})
 	}
 
+	// Reset ptr.
 	file.Seek(0, 0)
-	_, err = app.media.Upload(uuid.String(), srcContentType, file)
+
+	// Override content type after upload (in case it was detected incorrectly).
+	_, srcContentType, err = app.media.Upload(uuid.String(), srcContentType, file)
 	if err != nil {
 		cleanUp = true
 		app.lo.Error("error uploading file", "error", err)
@@ -156,7 +161,7 @@ func handleServeMedia(r *fastglue.Request) error {
 	}
 
 	// Fetch media from DB.
-	media, err := app.media.Get(0, strings.TrimPrefix(uuid, thumbPrefix))
+	media, err := app.media.Get(0, strings.TrimPrefix(uuid, image.ThumbPrefix))
 	if err != nil {
 		return sendErrorEnvelope(r, err)
 	}
@@ -199,7 +204,7 @@ func handleServeMedia(r *fastglue.Request) error {
 
 		fasthttp.ServeFile(r.RequestCtx, filepath.Join(ko.String("upload.fs.upload_path"), uuid))
 	case "s3":
-		r.RequestCtx.Redirect(app.media.GetURL(uuid), http.StatusFound)
+		r.RequestCtx.Redirect(app.media.GetURL(uuid, media.ContentType, media.Filename), http.StatusFound)
 	}
 	return nil
 }

cmd/messages.go

@@ -55,7 +55,8 @@ func handleGetMessages(r *fastglue.Request) error {
 		total = messages[i].Total
 		// Populate attachment URLs
 		for j := range messages[i].Attachments {
-			messages[i].Attachments[j].URL = app.media.GetURL(messages[i].Attachments[j].UUID)
+			att := messages[i].Attachments[j]
+			messages[i].Attachments[j].URL = app.media.GetURL(att.UUID, att.ContentType, att.Name)
 		}
 		// Redact CSAT survey link
 		messages[i].CensorCSATContent()
@@ -98,7 +99,8 @@ func handleGetMessage(r *fastglue.Request) error {
 	message.CensorCSATContent()
 
 	for j := range message.Attachments {
-		message.Attachments[j].URL = app.media.GetURL(message.Attachments[j].UUID)
+		att := message.Attachments[j]
+		message.Attachments[j].URL = app.media.GetURL(att.UUID, att.ContentType, att.Name)
 	}
 
 	return r.SendEnvelope(message)

go.mod

@@ -1,6 +1,6 @@
 module github.com/abhinavxd/libredesk
 
-go 1.24.3
+go 1.25.0
 
 require (
 	github.com/casbin/casbin/v2 v2.99.0
@@ -10,6 +10,7 @@ require (
 	github.com/emersion/go-message v0.18.1
 	github.com/fasthttp/websocket v1.5.9
 	github.com/ferluci/fast-realip v1.0.1
+	github.com/gabriel-vasile/mimetype v1.4.11
 	github.com/google/uuid v1.6.0
 	github.com/jhillyerd/enmime v1.2.0
 	github.com/jmoiron/sqlx v1.4.0
@@ -28,7 +29,7 @@ require (
 	github.com/lib/pq v1.10.9
 	github.com/mr-karan/balance v0.0.0-20250317053523-d32c6ade6cf1
 	github.com/redis/go-redis/v9 v9.5.5
-	github.com/rhnvrm/simples3 v0.9.2
+	github.com/rhnvrm/simples3 v0.10.1
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.10.0
 	github.com/valyala/fasthttp v1.62.0

go.sum

@@ -47,6 +47,8 @@ github.com/ferluci/fast-realip v1.0.1 h1:zPi0iv7zgOOlM/qJt9mozLz5IjVRAezaqHJoQ0J
 github.com/ferluci/fast-realip v1.0.1/go.mod h1:Ag7xdRQ9GOCL/pwbDe4zJv6SlfYdROArc8O+qIKhRc4=
 github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
 github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
+github.com/gabriel-vasile/mimetype v1.4.11 h1:AQvxbp830wPhHTqc1u7nzoLT+ZFxGY7emj5DR5DYFik=
+github.com/gabriel-vasile/mimetype v1.4.11/go.mod h1:d+9Oxyo1wTzWdyVUPMmXFvp4F9tea18J8ufA774AB3s=
 github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3AaE=
 github.com/go-jose/go-jose/v4 v4.0.5/go.mod h1:s3P1lRrkT8igV8D9OjyL4WRyHvjB6a4JSllnOrmmBOA=
 github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w=
@@ -140,8 +142,8 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/redis/go-redis/v9 v9.5.5 h1:51VEyMF8eOO+NUHFm8fpg+IOc1xFuFOhxs3R+kPu1FM=
 github.com/redis/go-redis/v9 v9.5.5/go.mod h1:hdY0cQFCN4fnSYT6TkisLufl/4W5UIXyv0b/CLO2V2M=
-github.com/rhnvrm/simples3 v0.9.2 h1:XrwsiMnwWf7t/kskvhMYXW6keqp5u3u6t5Va3ltzCQI=
-github.com/rhnvrm/simples3 v0.9.2/go.mod h1:Y+3vYm2V7Y4VijFoJHHTrja6OgPrJ2cBti8dPGkC3sA=
+github.com/rhnvrm/simples3 v0.10.1 h1:V/EbG6tC3F5MS7Vw5D02qIbNJom2jU+FVtGdrmsD67o=
+github.com/rhnvrm/simples3 v0.10.1/go.mod h1:c2xW30bukipkBlWNnXG1wDjq3gykQ6ww2AB/9NHMLMY=
 github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
 github.com/rivo/uniseg v0.4.4 h1:8TfxU8dW6PdqD27gjM8MVNuicgxIjxpm4K7x4jp8sis=
 github.com/rivo/uniseg v0.4.4/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=

i18n/en.json

@@ -332,6 +332,7 @@
   "user.errorGeneratingPasswordToken": "Error generating password token",
   "media.fileSizeTooLarge": "File size too large, please upload a file less than {size} ",
   "media.fileTypeNotAllowed": "File type not allowed",
+  "media.fileEmpty": "This file is 0 bytes, so it will not be attached.",
   "inbox.emptyIMAP": "Empty IMAP config",
   "inbox.emptySMTP": "Empty SMTP config",
   "template.defaultTemplateAlreadyExists": "Default template already exists",

internal/activity_log/activity_log.go

@@ -183,8 +183,7 @@ func (al *Manager) UserAvailability(actorID int, actorEmail, status, ip, targetE
 
 // create creates a new activity log in DB.
 func (m *Manager) create(activityType, activityDescription string, actorID int, targetModelType string, targetModelID int, ip string) error {
-	var activityLog models.ActivityLog
-	if err := m.q.InsertActivity.Get(&activityLog, activityType, activityDescription, actorID, targetModelType, targetModelID, ip); err != nil {
+	if _, err := m.q.InsertActivity.Exec(activityType, activityDescription, actorID, targetModelType, targetModelID, ip); err != nil {
 		m.lo.Error("error inserting activity log", "error", err)
 		return envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorCreating", "name", "{globals.terms.activityLog}"), nil)
 	}

internal/conversation/conversation.go

@@ -112,7 +112,7 @@ type mediaStore interface {
 	Attach(id int, model string, modelID int) error
 	GetByModel(id int, model string) ([]mmodels.Media, error)
 	ContentIDExists(contentID string) (bool, string, error)
-	Upload(fileName, contentType string, content io.ReadSeeker) (string, error)
+	Upload(fileName, contentType string, content io.ReadSeeker) (string, string, error)
 	UploadAndInsert(fileName, contentType, contentID string, modelType null.String, modelID null.Int, content io.ReadSeeker, fileSize int, disposition null.String, meta []byte) (mmodels.Media, error)
 }
 

internal/conversation/message.go

@@ -973,7 +973,7 @@ func (m *Manager) uploadThumbnailForMedia(media mmodels.Media, content []byte) e
 	thumbName := fmt.Sprintf("thumb_%s", media.UUID)
 
 	// Upload the thumbnail
-	if _, err := m.mediaStore.Upload(thumbName, media.ContentType, thumbFile); err != nil {
+	if _, _, err := m.mediaStore.Upload(thumbName, media.ContentType, thumbFile); err != nil {
 		m.lo.Error("error uploading thumbnail", "error", err)
 		return fmt.Errorf("error uploading thumbnail: %w", err)
 	}

internal/image/image.go

@@ -12,6 +12,7 @@ import (
 var (
 	Exts         = []string{"gif", "png", "jpg", "jpeg"}
 	DefThumbSize = 150
+	ThumbPrefix  = "thumb_"
 )
 
 // GetDimensions returns the width and height of the image in the provided file.

internal/media/media.go

@@ -8,12 +8,16 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/http"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/abhinavxd/libredesk/internal/dbutil"
 	"github.com/abhinavxd/libredesk/internal/envelope"
+	"github.com/abhinavxd/libredesk/internal/image"
 	"github.com/abhinavxd/libredesk/internal/media/models"
+	"github.com/gabriel-vasile/mimetype"
 	"github.com/google/uuid"
 	"github.com/jmoiron/sqlx"
 	"github.com/knadh/go-i18n"
@@ -30,7 +34,7 @@ var (
 type Store interface {
 	Put(name, contentType string, content io.ReadSeeker) (string, error)
 	Delete(name string) error
-	GetURL(name string) string
+	GetURL(name, disposition, fileName string) string
 	GetBlob(name string) ([]byte, error)
 	Name() string
 }
@@ -78,8 +82,13 @@ type queries struct {
 
 // UploadAndInsert uploads file on storage and inserts an entry in db.
 func (m *Manager) UploadAndInsert(srcFilename, contentType, contentID string, modelType null.String, modelID null.Int, content io.ReadSeeker, fileSize int, disposition null.String, meta []byte) (models.Media, error) {
-	var uuid = uuid.New()
-	_, err := m.Upload(uuid.String(), contentType, content)
+	var (
+		uuid = uuid.New()
+		err  error
+	)
+
+	// Override content type after upload (in case it was detected incorrectly).
+	_, contentType, err = m.Upload(uuid.String(), contentType, content)
 	if err != nil {
 		return models.Media{}, err
 	}
@@ -92,14 +101,24 @@ func (m *Manager) UploadAndInsert(srcFilename, contentType, contentID string, mo
 	return media, nil
 }
 
-// Upload saves the media file to the storage backend and returns the generated filename.
-func (m *Manager) Upload(fileName, contentType string, content io.ReadSeeker) (string, error) {
+// Upload saves the media file to the storage backend - returns the generated filename and content type (after detection).
+func (m *Manager) Upload(fileName, contentType string, content io.ReadSeeker) (string, string, error) {
+	// On store file is named by UUID to avoid collisions and the actual filename is stored in DB.
+	m.lo.Debug("detecting content type for file before upload", "uuid", fileName, "source_content_type", contentType)
+
+	// Detect content type and override if needed.
+	contentType, err := m.detectContentType(contentType, content)
+	if err != nil {
+		m.lo.Error("error detecting content type", "error", err)
+		return "", "", err
+	}
+
 	fName, err := m.store.Put(fileName, contentType, content)
 	if err != nil {
 		m.lo.Error("error uploading media", "error", err)
-		return "", envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorUploading", "name", "{globals.terms.media}"), nil)
+		return "", "", envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorUploading", "name", "{globals.terms.media}"), nil)
 	}
-	return fName, nil
+	return fName, contentType, nil
 }
 
 // Insert inserts media details into the database and returns the inserted media record.
@@ -122,7 +141,7 @@ func (m *Manager) Get(id int, uuid string) (models.Media, error) {
 		m.lo.Error("error fetching media", "error", err)
 		return media, envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorFetching", "name", "{globals.terms.media}"), nil)
 	}
-	media.URL = m.store.GetURL(media.UUID)
+	media.URL = m.GetURL(media.UUID, media.ContentType, media.Filename)
 	return media, nil
 }
 
@@ -145,8 +164,15 @@ func (m *Manager) GetBlob(name string) ([]byte, error) {
 }
 
 // GetURL returns the URL for accessing a media file by its name.
-func (m *Manager) GetURL(name string) string {
-	return m.store.GetURL(name)
+func (m *Manager) GetURL(uuid, contentType, fileName string) string {
+	// Keep some content types inline.
+	disposition := "attachment"
+	if strings.HasPrefix(contentType, "image/") ||
+		strings.HasPrefix(contentType, "video/") ||
+		contentType == "application/pdf" {
+		disposition = "inline"
+	}
+	return m.store.GetURL(uuid, disposition, fileName)
 }
 
 // Attach associates a media file with a specific model by its ID and model name.
@@ -177,6 +203,12 @@ func (m *Manager) Delete(name string) error {
 			return envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorDeleting", "name", "{globals.terms.media}"), nil)
 		}
 	}
+
+	// Thumbnail files do not exist in the database, only in the storage backend, so return early.
+	if strings.HasPrefix(name, image.ThumbPrefix) {
+		return nil
+	}
+
 	// Delete the media record from the database.
 	if _, err := m.queries.Delete.Exec(name); err != nil {
 		m.lo.Error("error deleting media from db", "error", err)
@@ -214,7 +246,65 @@ func (m *Manager) deleteUnlinkedMessageMedia() error {
 			m.lo.Error("error deleting unlinked media", "error", err)
 			continue
 		}
-		// TODO: If it's an image also delete the `thumb_uuid` image.
+
+		// If it's an image, also delete the `thumb_uuid` image from store.
+		if strings.HasPrefix(mm.ContentType, "image/") {
+			thumbUUID := image.ThumbPrefix + mm.UUID
+			m.lo.Debug("deleting thumbnail for unlinked media", "thumb_uuid", thumbUUID)
+			if err := m.Delete(thumbUUID); err != nil {
+				m.lo.Error("error deleting thumbnail for unlinked media", "error", err)
+			}
+		}
 	}
 	return nil
 }
+
+// detectContentType detects the content type of a file.
+// It trusts the source content type unless it's a generic type like application/octet-stream.
+// For generic types, it uses http.DetectContentType (stdlib) as a fast path,
+// falling back to mimetype library for deeper inspection using magic numbers.
+func (m *Manager) detectContentType(sourceContentType string, content io.ReadSeeker) (string, error) {
+	// Set default if empty
+	if sourceContentType == "" {
+		sourceContentType = "application/octet-stream"
+	}
+
+	// Trust source unless it's a generic/useless type
+	if sourceContentType != "application/octet-stream" &&
+		sourceContentType != "application/data" &&
+		sourceContentType != "application/binary" {
+		m.lo.Debug("detected media content type from trusted source", "detected_type", sourceContentType)
+		return sourceContentType, nil
+	}
+
+	// Ensure we're at the start
+	content.Seek(0, io.SeekStart)
+
+	// Fast path: stdlib
+	buf := make([]byte, 512)
+	n, _ := content.Read(buf)
+	detected := http.DetectContentType(buf[:n])
+
+	// If stdlib gives a useful type, use it.
+	// stdlib defaults to application/octet-stream for unknown types.
+	if detected != "application/octet-stream" {
+		content.Seek(0, io.SeekStart)
+		m.lo.Debug("detected media content type using stdlib", "detected_type", detected, "source_type", sourceContentType)
+		return detected, nil
+	}
+
+	// Slow path: mimetype library
+	content.Seek(0, io.SeekStart)
+	mtype, err := mimetype.DetectReader(content)
+	if err != nil {
+		m.lo.Error("error detecting content type", "error", err)
+		content.Seek(0, io.SeekStart)
+		return sourceContentType, nil
+	}
+
+	detectedType := mtype.String()
+	m.lo.Debug("detected media content type using mimetype lib", "detected_type", detectedType, "source_type", sourceContentType)
+
+	content.Seek(0, io.SeekStart)
+	return detectedType, nil
+}

internal/media/stores/localfs/fs.go

@@ -48,7 +48,7 @@ func (c *Client) Put(filename string, cType string, src io.ReadSeeker) (string,
 }
 
 // GetURL accepts a filename and retrieves the full URL for file.
-func (c *Client) GetURL(name string) string {
+func (c *Client) GetURL(name, _, _ string) string {
 	return fmt.Sprintf("%s%s/%s", c.opts.RootURL(), c.opts.UploadURI, name)
 }
 

internal/media/stores/s3/s3.go

@@ -88,14 +88,15 @@ func (c *Client) Put(name string, cType string, file io.ReadSeeker) (string, err
 
 // GetURL generates a URL to access the file stored in S3.
 // It returns a pre-signed URL for private buckets or a public URL for public buckets.
-func (c *Client) GetURL(name string) string {
+func (c *Client) GetURL(name string, disposition, fileName string) string {
 	if c.opts.BucketType == "private" && c.opts.PublicURL == "" {
 		u := c.s3.GeneratePresignedURL(simples3.PresignedInput{
-			Bucket:        c.opts.Bucket,
-			ObjectKey:     c.makeBucketPath(name),
-			Method:        "GET",
-			Timestamp:     time.Now(),
-			ExpirySeconds: int(c.opts.Expiry.Seconds()),
+			Bucket:                     c.opts.Bucket,
+			ObjectKey:                  c.makeBucketPath(name),
+			Method:                     "GET",
+			Timestamp:                  time.Now(),
+			ExpirySeconds:              int(c.opts.Expiry.Seconds()),
+			ResponseContentDisposition: fmt.Sprintf("%s; filename=\"%s\"", disposition, fileName),
 		})
 		return u
 	}