Skip to content

Commit 63d4284

Browse files
authored
Merge pull request #179 from abhinavxd/fix-mimetype-detection
fix: improve MIME type detection and content disposition handling for S3 store.
2 parents e9bd9e6 + 2331a2c commit 63d4284

File tree

12 files changed

+139
-37
lines changed

12 files changed

+139
-37
lines changed

cmd/media.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,6 @@ import (
2020
"github.com/zerodha/fastglue"
2121
)
2222

23-
const (
24-
thumbPrefix = "thumb_"
25-
)
26-
2723
// handleMediaUpload handles media uploads.
2824
func handleMediaUpload(r *fastglue.Request) error {
2925
var (
@@ -70,6 +66,12 @@ func handleMediaUpload(r *fastglue.Request) error {
7066
srcFileSize := fileHeader.Size
7167
srcExt := strings.TrimPrefix(strings.ToLower(filepath.Ext(srcFileName)), ".")
7268

69+
// Check if file is empty
70+
if srcFileSize == 0 {
71+
app.lo.Error("error: uploaded file is empty (0 bytes)")
72+
return r.SendErrorEnvelope(fasthttp.StatusBadRequest, app.i18n.T("media.fileEmpty"), nil, envelope.InputError)
73+
}
74+
7375
// Check file size
7476
consts := app.consts.Load().(*constants)
7577
if bytesToMegabytes(srcFileSize) > float64(consts.MaxFileUploadSizeMB) {
@@ -88,7 +90,7 @@ func handleMediaUpload(r *fastglue.Request) error {
8890

8991
// Delete files on any error.
9092
var uuid = uuid.New()
91-
thumbName := thumbPrefix + uuid.String()
93+
thumbName := image.ThumbPrefix + uuid.String()
9294
defer func() {
9395
if cleanUp {
9496
app.media.Delete(uuid.String())
@@ -105,7 +107,7 @@ func handleMediaUpload(r *fastglue.Request) error {
105107
app.lo.Error("error creating thumb image", "error", err)
106108
return r.SendErrorEnvelope(fasthttp.StatusInternalServerError, app.i18n.Ts("globals.messages.errorCreating", "name", "{globals.terms.thumbnail}"), nil, envelope.GeneralError)
107109
}
108-
thumbName, err = app.media.Upload(thumbName, srcContentType, thumbFile)
110+
thumbName, _, err = app.media.Upload(thumbName, srcContentType, thumbFile)
109111
if err != nil {
110112
return sendErrorEnvelope(r, err)
111113
}
@@ -124,8 +126,11 @@ func handleMediaUpload(r *fastglue.Request) error {
124126
})
125127
}
126128

129+
// Reset ptr.
127130
file.Seek(0, 0)
128-
_, err = app.media.Upload(uuid.String(), srcContentType, file)
131+
132+
// Override content type after upload (in case it was detected incorrectly).
133+
_, srcContentType, err = app.media.Upload(uuid.String(), srcContentType, file)
129134
if err != nil {
130135
cleanUp = true
131136
app.lo.Error("error uploading file", "error", err)
@@ -156,7 +161,7 @@ func handleServeMedia(r *fastglue.Request) error {
156161
}
157162

158163
// Fetch media from DB.
159-
media, err := app.media.Get(0, strings.TrimPrefix(uuid, thumbPrefix))
164+
media, err := app.media.Get(0, strings.TrimPrefix(uuid, image.ThumbPrefix))
160165
if err != nil {
161166
return sendErrorEnvelope(r, err)
162167
}
@@ -199,7 +204,7 @@ func handleServeMedia(r *fastglue.Request) error {
199204

200205
fasthttp.ServeFile(r.RequestCtx, filepath.Join(ko.String("upload.fs.upload_path"), uuid))
201206
case "s3":
202-
r.RequestCtx.Redirect(app.media.GetURL(uuid), http.StatusFound)
207+
r.RequestCtx.Redirect(app.media.GetURL(uuid, media.ContentType, media.Filename), http.StatusFound)
203208
}
204209
return nil
205210
}

cmd/messages.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ func handleGetMessages(r *fastglue.Request) error {
5555
total = messages[i].Total
5656
// Populate attachment URLs
5757
for j := range messages[i].Attachments {
58-
messages[i].Attachments[j].URL = app.media.GetURL(messages[i].Attachments[j].UUID)
58+
att := messages[i].Attachments[j]
59+
messages[i].Attachments[j].URL = app.media.GetURL(att.UUID, att.ContentType, att.Name)
5960
}
6061
// Redact CSAT survey link
6162
messages[i].CensorCSATContent()
@@ -98,7 +99,8 @@ func handleGetMessage(r *fastglue.Request) error {
9899
message.CensorCSATContent()
99100

100101
for j := range message.Attachments {
101-
message.Attachments[j].URL = app.media.GetURL(message.Attachments[j].UUID)
102+
att := message.Attachments[j]
103+
message.Attachments[j].URL = app.media.GetURL(att.UUID, att.ContentType, att.Name)
102104
}
103105

104106
return r.SendEnvelope(message)

go.mod

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/abhinavxd/libredesk
22

3-
go 1.24.3
3+
go 1.25.0
44

55
require (
66
github.com/casbin/casbin/v2 v2.99.0
@@ -10,6 +10,7 @@ require (
1010
github.com/emersion/go-message v0.18.1
1111
github.com/fasthttp/websocket v1.5.9
1212
github.com/ferluci/fast-realip v1.0.1
13+
github.com/gabriel-vasile/mimetype v1.4.11
1314
github.com/google/uuid v1.6.0
1415
github.com/jhillyerd/enmime v1.2.0
1516
github.com/jmoiron/sqlx v1.4.0
@@ -28,7 +29,7 @@ require (
2829
github.com/lib/pq v1.10.9
2930
github.com/mr-karan/balance v0.0.0-20250317053523-d32c6ade6cf1
3031
github.com/redis/go-redis/v9 v9.5.5
31-
github.com/rhnvrm/simples3 v0.9.2
32+
github.com/rhnvrm/simples3 v0.10.1
3233
github.com/spf13/pflag v1.0.5
3334
github.com/stretchr/testify v1.10.0
3435
github.com/valyala/fasthttp v1.62.0

go.sum

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ github.com/ferluci/fast-realip v1.0.1 h1:zPi0iv7zgOOlM/qJt9mozLz5IjVRAezaqHJoQ0J
4747
github.com/ferluci/fast-realip v1.0.1/go.mod h1:Ag7xdRQ9GOCL/pwbDe4zJv6SlfYdROArc8O+qIKhRc4=
4848
github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY=
4949
github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
50+
github.com/gabriel-vasile/mimetype v1.4.11 h1:AQvxbp830wPhHTqc1u7nzoLT+ZFxGY7emj5DR5DYFik=
51+
github.com/gabriel-vasile/mimetype v1.4.11/go.mod h1:d+9Oxyo1wTzWdyVUPMmXFvp4F9tea18J8ufA774AB3s=
5052
github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3AaE=
5153
github.com/go-jose/go-jose/v4 v4.0.5/go.mod h1:s3P1lRrkT8igV8D9OjyL4WRyHvjB6a4JSllnOrmmBOA=
5254
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
140142
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
141143
github.com/redis/go-redis/v9 v9.5.5 h1:51VEyMF8eOO+NUHFm8fpg+IOc1xFuFOhxs3R+kPu1FM=
142144
github.com/redis/go-redis/v9 v9.5.5/go.mod h1:hdY0cQFCN4fnSYT6TkisLufl/4W5UIXyv0b/CLO2V2M=
143-
github.com/rhnvrm/simples3 v0.9.2 h1:XrwsiMnwWf7t/kskvhMYXW6keqp5u3u6t5Va3ltzCQI=
144-
github.com/rhnvrm/simples3 v0.9.2/go.mod h1:Y+3vYm2V7Y4VijFoJHHTrja6OgPrJ2cBti8dPGkC3sA=
145+
github.com/rhnvrm/simples3 v0.10.1 h1:V/EbG6tC3F5MS7Vw5D02qIbNJom2jU+FVtGdrmsD67o=
146+
github.com/rhnvrm/simples3 v0.10.1/go.mod h1:c2xW30bukipkBlWNnXG1wDjq3gykQ6ww2AB/9NHMLMY=
145147
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
146148
github.com/rivo/uniseg v0.4.4 h1:8TfxU8dW6PdqD27gjM8MVNuicgxIjxpm4K7x4jp8sis=
147149
github.com/rivo/uniseg v0.4.4/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=

i18n/en.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@
332332
"user.errorGeneratingPasswordToken": "Error generating password token",
333333
"media.fileSizeTooLarge": "File size too large, please upload a file less than {size} ",
334334
"media.fileTypeNotAllowed": "File type not allowed",
335+
"media.fileEmpty": "This file is 0 bytes, so it will not be attached.",
335336
"inbox.emptyIMAP": "Empty IMAP config",
336337
"inbox.emptySMTP": "Empty SMTP config",
337338
"template.defaultTemplateAlreadyExists": "Default template already exists",

internal/activity_log/activity_log.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,7 @@ func (al *Manager) UserAvailability(actorID int, actorEmail, status, ip, targetE
183183

184184
// create creates a new activity log in DB.
185185
func (m *Manager) create(activityType, activityDescription string, actorID int, targetModelType string, targetModelID int, ip string) error {
186-
var activityLog models.ActivityLog
187-
if err := m.q.InsertActivity.Get(&activityLog, activityType, activityDescription, actorID, targetModelType, targetModelID, ip); err != nil {
186+
if _, err := m.q.InsertActivity.Exec(activityType, activityDescription, actorID, targetModelType, targetModelID, ip); err != nil {
188187
m.lo.Error("error inserting activity log", "error", err)
189188
return envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorCreating", "name", "{globals.terms.activityLog}"), nil)
190189
}

internal/conversation/conversation.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ type mediaStore interface {
112112
Attach(id int, model string, modelID int) error
113113
GetByModel(id int, model string) ([]mmodels.Media, error)
114114
ContentIDExists(contentID string) (bool, string, error)
115-
Upload(fileName, contentType string, content io.ReadSeeker) (string, error)
115+
Upload(fileName, contentType string, content io.ReadSeeker) (string, string, error)
116116
UploadAndInsert(fileName, contentType, contentID string, modelType null.String, modelID null.Int, content io.ReadSeeker, fileSize int, disposition null.String, meta []byte) (mmodels.Media, error)
117117
}
118118

internal/conversation/message.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -973,7 +973,7 @@ func (m *Manager) uploadThumbnailForMedia(media mmodels.Media, content []byte) e
973973
thumbName := fmt.Sprintf("thumb_%s", media.UUID)
974974

975975
// Upload the thumbnail
976-
if _, err := m.mediaStore.Upload(thumbName, media.ContentType, thumbFile); err != nil {
976+
if _, _, err := m.mediaStore.Upload(thumbName, media.ContentType, thumbFile); err != nil {
977977
m.lo.Error("error uploading thumbnail", "error", err)
978978
return fmt.Errorf("error uploading thumbnail: %w", err)
979979
}

internal/image/image.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
var (
1313
Exts = []string{"gif", "png", "jpg", "jpeg"}
1414
DefThumbSize = 150
15+
ThumbPrefix = "thumb_"
1516
)
1617

1718
// GetDimensions returns the width and height of the image in the provided file.

internal/media/media.go

Lines changed: 101 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,16 @@ import (
88
"errors"
99
"fmt"
1010
"io"
11+
"net/http"
1112
"os"
13+
"strings"
1214
"time"
1315

1416
"github.com/abhinavxd/libredesk/internal/dbutil"
1517
"github.com/abhinavxd/libredesk/internal/envelope"
18+
"github.com/abhinavxd/libredesk/internal/image"
1619
"github.com/abhinavxd/libredesk/internal/media/models"
20+
"github.com/gabriel-vasile/mimetype"
1721
"github.com/google/uuid"
1822
"github.com/jmoiron/sqlx"
1923
"github.com/knadh/go-i18n"
@@ -30,7 +34,7 @@ var (
3034
type Store interface {
3135
Put(name, contentType string, content io.ReadSeeker) (string, error)
3236
Delete(name string) error
33-
GetURL(name string) string
37+
GetURL(name, disposition, fileName string) string
3438
GetBlob(name string) ([]byte, error)
3539
Name() string
3640
}
@@ -78,8 +82,13 @@ type queries struct {
7882

7983
// UploadAndInsert uploads file on storage and inserts an entry in db.
8084
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) {
81-
var uuid = uuid.New()
82-
_, err := m.Upload(uuid.String(), contentType, content)
85+
var (
86+
uuid = uuid.New()
87+
err error
88+
)
89+
90+
// Override content type after upload (in case it was detected incorrectly).
91+
_, contentType, err = m.Upload(uuid.String(), contentType, content)
8392
if err != nil {
8493
return models.Media{}, err
8594
}
@@ -92,14 +101,24 @@ func (m *Manager) UploadAndInsert(srcFilename, contentType, contentID string, mo
92101
return media, nil
93102
}
94103

95-
// Upload saves the media file to the storage backend and returns the generated filename.
96-
func (m *Manager) Upload(fileName, contentType string, content io.ReadSeeker) (string, error) {
104+
// Upload saves the media file to the storage backend - returns the generated filename and content type (after detection).
105+
func (m *Manager) Upload(fileName, contentType string, content io.ReadSeeker) (string, string, error) {
106+
// On store file is named by UUID to avoid collisions and the actual filename is stored in DB.
107+
m.lo.Debug("detecting content type for file before upload", "uuid", fileName, "source_content_type", contentType)
108+
109+
// Detect content type and override if needed.
110+
contentType, err := m.detectContentType(contentType, content)
111+
if err != nil {
112+
m.lo.Error("error detecting content type", "error", err)
113+
return "", "", err
114+
}
115+
97116
fName, err := m.store.Put(fileName, contentType, content)
98117
if err != nil {
99118
m.lo.Error("error uploading media", "error", err)
100-
return "", envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorUploading", "name", "{globals.terms.media}"), nil)
119+
return "", "", envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorUploading", "name", "{globals.terms.media}"), nil)
101120
}
102-
return fName, nil
121+
return fName, contentType, nil
103122
}
104123

105124
// 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) {
122141
m.lo.Error("error fetching media", "error", err)
123142
return media, envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorFetching", "name", "{globals.terms.media}"), nil)
124143
}
125-
media.URL = m.store.GetURL(media.UUID)
144+
media.URL = m.GetURL(media.UUID, media.ContentType, media.Filename)
126145
return media, nil
127146
}
128147

@@ -145,8 +164,15 @@ func (m *Manager) GetBlob(name string) ([]byte, error) {
145164
}
146165

147166
// GetURL returns the URL for accessing a media file by its name.
148-
func (m *Manager) GetURL(name string) string {
149-
return m.store.GetURL(name)
167+
func (m *Manager) GetURL(uuid, contentType, fileName string) string {
168+
// Keep some content types inline.
169+
disposition := "attachment"
170+
if strings.HasPrefix(contentType, "image/") ||
171+
strings.HasPrefix(contentType, "video/") ||
172+
contentType == "application/pdf" {
173+
disposition = "inline"
174+
}
175+
return m.store.GetURL(uuid, disposition, fileName)
150176
}
151177

152178
// 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 {
177203
return envelope.NewError(envelope.GeneralError, m.i18n.Ts("globals.messages.errorDeleting", "name", "{globals.terms.media}"), nil)
178204
}
179205
}
206+
207+
// Thumbnail files do not exist in the database, only in the storage backend, so return early.
208+
if strings.HasPrefix(name, image.ThumbPrefix) {
209+
return nil
210+
}
211+
180212
// Delete the media record from the database.
181213
if _, err := m.queries.Delete.Exec(name); err != nil {
182214
m.lo.Error("error deleting media from db", "error", err)
@@ -214,7 +246,65 @@ func (m *Manager) deleteUnlinkedMessageMedia() error {
214246
m.lo.Error("error deleting unlinked media", "error", err)
215247
continue
216248
}
217-
// TODO: If it's an image also delete the `thumb_uuid` image.
249+
250+
// If it's an image, also delete the `thumb_uuid` image from store.
251+
if strings.HasPrefix(mm.ContentType, "image/") {
252+
thumbUUID := image.ThumbPrefix + mm.UUID
253+
m.lo.Debug("deleting thumbnail for unlinked media", "thumb_uuid", thumbUUID)
254+
if err := m.Delete(thumbUUID); err != nil {
255+
m.lo.Error("error deleting thumbnail for unlinked media", "error", err)
256+
}
257+
}
218258
}
219259
return nil
220260
}
261+
262+
// detectContentType detects the content type of a file.
263+
// It trusts the source content type unless it's a generic type like application/octet-stream.
264+
// For generic types, it uses http.DetectContentType (stdlib) as a fast path,
265+
// falling back to mimetype library for deeper inspection using magic numbers.
266+
func (m *Manager) detectContentType(sourceContentType string, content io.ReadSeeker) (string, error) {
267+
// Set default if empty
268+
if sourceContentType == "" {
269+
sourceContentType = "application/octet-stream"
270+
}
271+
272+
// Trust source unless it's a generic/useless type
273+
if sourceContentType != "application/octet-stream" &&
274+
sourceContentType != "application/data" &&
275+
sourceContentType != "application/binary" {
276+
m.lo.Debug("detected media content type from trusted source", "detected_type", sourceContentType)
277+
return sourceContentType, nil
278+
}
279+
280+
// Ensure we're at the start
281+
content.Seek(0, io.SeekStart)
282+
283+
// Fast path: stdlib
284+
buf := make([]byte, 512)
285+
n, _ := content.Read(buf)
286+
detected := http.DetectContentType(buf[:n])
287+
288+
// If stdlib gives a useful type, use it.
289+
// stdlib defaults to application/octet-stream for unknown types.
290+
if detected != "application/octet-stream" {
291+
content.Seek(0, io.SeekStart)
292+
m.lo.Debug("detected media content type using stdlib", "detected_type", detected, "source_type", sourceContentType)
293+
return detected, nil
294+
}
295+
296+
// Slow path: mimetype library
297+
content.Seek(0, io.SeekStart)
298+
mtype, err := mimetype.DetectReader(content)
299+
if err != nil {
300+
m.lo.Error("error detecting content type", "error", err)
301+
content.Seek(0, io.SeekStart)
302+
return sourceContentType, nil
303+
}
304+
305+
detectedType := mtype.String()
306+
m.lo.Debug("detected media content type using mimetype lib", "detected_type", detectedType, "source_type", sourceContentType)
307+
308+
content.Seek(0, io.SeekStart)
309+
return detectedType, nil
310+
}

0 commit comments

Comments
 (0)