From f516bbfa12c19f43c91cc77889a53458094612b9 Mon Sep 17 00:00:00 2001 From: Abhinav Raut Date: Tue, 25 Nov 2025 16:06:36 +0530 Subject: [PATCH] fix: improve MIME type detection and content disposition handling for media uploads for s3 store. --- cmd/media.go | 17 +++--- cmd/messages.go | 6 +- go.mod | 4 ++ go.sum | 4 +- internal/conversation/conversation.go | 2 +- internal/conversation/message.go | 2 +- internal/image/image.go | 1 + internal/media/media.go | 84 +++++++++++++++++++++++---- internal/media/stores/localfs/fs.go | 2 +- internal/media/stores/s3/s3.go | 13 +++-- 10 files changed, 102 insertions(+), 33 deletions(-) diff --git a/cmd/media.go b/cmd/media.go index 9bffec76..ebc5e8ae 100644 --- a/cmd/media.go +++ b/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 ( @@ -88,7 +84,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 +101,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 +120,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 +155,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 +198,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(media.UUID, media.ContentType, media.Filename), http.StatusFound) } return nil } diff --git a/cmd/messages.go b/cmd/messages.go index f48f7fab..be9315f3 100644 --- a/cmd/messages.go +++ b/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) diff --git a/go.mod b/go.mod index 65af307a..23101eaf 100644 --- a/go.mod +++ b/go.mod @@ -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 @@ -78,3 +79,6 @@ require ( golang.org/x/text v0.31.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +// TODO: Push simple s3 change to upstream +replace github.com/rhnvrm/simples3 => /home/abhinavr/projects/simples3 diff --git a/go.sum b/go.sum index a55a6089..884861a0 100644 --- a/go.sum +++ b/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,6 @@ 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/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= diff --git a/internal/conversation/conversation.go b/internal/conversation/conversation.go index 823ee23b..78eac0bf 100644 --- a/internal/conversation/conversation.go +++ b/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) } diff --git a/internal/conversation/message.go b/internal/conversation/message.go index 8106bc64..855cd4cd 100644 --- a/internal/conversation/message.go +++ b/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) } diff --git a/internal/image/image.go b/internal/image/image.go index ddb9f878..96d0ca98 100644 --- a/internal/image/image.go +++ b/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. diff --git a/internal/media/media.go b/internal/media/media.go index 3e42aeb4..210d152c 100644 --- a/internal/media/media.go +++ b/internal/media/media.go @@ -9,11 +9,14 @@ import ( "fmt" "io" "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 +33,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 +81,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 +100,19 @@ 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) { + contentType, err := m.detectContentType(fileName, contentType, content) + if err != nil { + 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 +135,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.store.GetURL(media.UUID, media.ContentType, media.Filename) return media, nil } @@ -145,8 +158,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 +197,12 @@ func (m *Manager) Delete(name string) error { return envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorDeleting", "name", "{globals.terms.media}"), nil) } } + + // Return thumbs don't exist in DB, just on store. + 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 +240,43 @@ 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 := "thumb_" + 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 from its filename or content. +// If contentType is unreliable, it attempts to detect the proper type using mimetype library. +// Returns the detected content type or an error if seeking fails after content detection. +func (m *Manager) detectContentType(fileName, contentType string, content io.ReadSeeker) (string, error) { + m.lo.Debug("detecting content type for file", "filename", fileName, "content_type", contentType) + + if contentType != "" && contentType != "application/octet-stream" { + return contentType, nil + } + + mtype, err := mimetype.DetectReader(content) + if err != nil { + m.lo.Error("error detecting content type", "filename", fileName, "error", err) + return "application/octet-stream", nil + } + + detectedType := mtype.String() + m.lo.Debug("detected content type for file", "filename", fileName, "detected_content_type", detectedType, "previous_content_type", contentType) + + // Reset ptr. + if _, err := content.Seek(0, io.SeekStart); err != nil { + m.lo.Error("error seeking to start after content type detection", "error", err) + return "", envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorUploading", "name", "{globals.terms.media}"), nil) + } + + return detectedType, nil +} diff --git a/internal/media/stores/localfs/fs.go b/internal/media/stores/localfs/fs.go index 7b082b6c..42e3f2c2 100644 --- a/internal/media/stores/localfs/fs.go +++ b/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) } diff --git a/internal/media/stores/s3/s3.go b/internal/media/stores/s3/s3.go index 9c19b069..3ffa8d8b 100644 --- a/internal/media/stores/s3/s3.go +++ b/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 }