From 793c3d2f3fbd9c709a1a8acb5d00b6e292abf9c2 Mon Sep 17 00:00:00 2001 From: Abhinav Raut Date: Fri, 15 May 2026 00:44:12 +0530 Subject: [PATCH 1/2] Fix quoted-reply inline images returning no signed URL --- internal/conversation/conversation.go | 1 + internal/conversation/message.go | 100 +++++++++++++++++++++----- internal/conversation/message_test.go | 68 ++++++++++++++++++ internal/media/media.go | 15 ++++ internal/media/queries.sql | 8 +++ 5 files changed, 176 insertions(+), 16 deletions(-) diff --git a/internal/conversation/conversation.go b/internal/conversation/conversation.go index 931750da..8f6b0c2d 100644 --- a/internal/conversation/conversation.go +++ b/internal/conversation/conversation.go @@ -140,6 +140,7 @@ type mediaStore interface { Attach(id int, model string, modelID int) error SetContentID(id int, contentID string) error GetByModel(id int, model string) ([]mmodels.Media, error) + GetByContentIDs(contentIDs []string, conversationUUID string) ([]mmodels.Media, error) ContentIDExists(contentID string) (bool, 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 2f8edef1..5d35e877 100644 --- a/internal/conversation/message.go +++ b/internal/conversation/message.go @@ -343,6 +343,10 @@ func (m *Manager) GetConversationMessages(conversationUUID string, page, pageSiz return messages, pageSize, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } + for i := range messages { + m.populateInlineMedia(&messages[i]) + } + return messages, pageSize, nil } @@ -354,6 +358,8 @@ func (m *Manager) GetMessage(uuid string) (models.Message, error) { return message, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } + m.populateInlineMedia(&message) + // Generate signed URLs for attachments. for i := range message.Attachments { message.Attachments[i].URL = m.mediaStore.GetSignedURL(message.Attachments[i].UUID) @@ -1005,6 +1011,26 @@ func extractInlineImageUUIDs(content string) []string { return out } +// extractInlineContentIDs returns unique content_ids referenced via in the body. +func extractInlineContentIDs(content string) []string { + matches := imgSrcPattern.FindAllStringSubmatch(content, -1) + seen := make(map[string]bool, len(matches)) + out := make([]string, 0, len(matches)) + for _, m := range matches { + url := m[1] + if !strings.HasPrefix(url, "cid:") { + continue + } + cid := strings.TrimPrefix(url, "cid:") + if cid == "" || seen[cid] { + continue + } + seen[cid] = true + out = append(out, cid) + } + return out +} + // rewriteInlineImagesToCID rewrites every to . Already-cid form is left alone. func rewriteInlineImagesToCID(content string) string { return imgSrcPattern.ReplaceAllStringFunc(content, func(match string) string { @@ -1061,35 +1087,26 @@ func (m *Manager) uploadMessageAttachments(message *models.Message) error { } for _, attachment := range message.Attachments { - // Check if this attachment already exists by the content ID, as inline images can be repeated across conversations. contentID := attachment.ContentID if contentID != "" { - // Make content ID MORE unique by prefixing it with the conversation UUID, as content id is not globally unique practically, - // different messages can have the same content ID, I do not have the message ID at this point, so I am using sticking with the conversation UUID - // to make it more unique. - contentID = message.ConversationUUID + "_" + contentID + storedCID, exists, mediaUUID := m.findExistingMedia(contentID, message.ConversationUUID) - exists, uuid, err := m.mediaStore.ContentIDExists(contentID) - if err != nil { - m.lo.Error("error checking media existence by content ID", "content_id", contentID, "error", err) + // Make body's cid match the stored content_id so the read path can find it. + if storedCID != contentID { + message.Content = strings.ReplaceAll(message.Content, fmt.Sprintf("cid:%s", contentID), fmt.Sprintf("cid:%s", storedCID)) } - // This attachment already exists, replace the cid:content_id with the media relative url, not using absolute path as the root path can change. if exists { - m.lo.Debug("attachment with content ID already exists replacing content ID with media relative URL", "content_id", contentID, "media_uuid", uuid) - message.Content = strings.ReplaceAll(message.Content, fmt.Sprintf("cid:%s", attachment.ContentID), "/uploads/"+uuid) + m.lo.Debug("inline attachment exists, reusing", "content_id", storedCID, "media_uuid", mediaUUID) continue } - - // Attachment does not exist, replace the content ID with the new more unique content ID. - message.Content = strings.ReplaceAll(message.Content, fmt.Sprintf("cid:%s", attachment.ContentID), fmt.Sprintf("cid:%s", contentID)) + contentID = storedCID } // Sanitize filename. attachment.Name = stringutil.SanitizeFilename(attachment.Name) - m.lo.Debug("uploading message attachment", "name", attachment.Name, "content_id", contentID, "size", attachment.Size, "content_type", attachment.ContentType, - "content_id", contentID, "disposition", attachment.Disposition) + m.lo.Debug("uploading message attachment", "name", attachment.Name, "content_id", contentID, "size", attachment.Size, "content_type", attachment.ContentType, "disposition", attachment.Disposition) // Upload and insert entry in media table. attachReader := bytes.NewReader(attachment.Content) @@ -1187,6 +1204,44 @@ func (m *Manager) messageExistsBySourceID(messageSourceIDs []string) (int, error return conversationID, nil } +// populateInlineMedia appends quoted inline images (referenced via cid: in the body but not in the attachments list) so resolveContentCIDs can rewrite them to signed URLs. +func (m *Manager) populateInlineMedia(message *models.Message) { + cids := extractInlineContentIDs(message.Content) + if len(cids) == 0 { + return + } + existing := make(map[string]bool, len(message.Attachments)) + for _, a := range message.Attachments { + if a.ContentID != "" { + existing[a.ContentID] = true + } + } + missing := make([]string, 0, len(cids)) + for _, cid := range cids { + if !existing[cid] { + missing = append(missing, cid) + } + } + if len(missing) == 0 { + return + } + medias, err := m.mediaStore.GetByContentIDs(missing, message.ConversationUUID) + if err != nil { + m.lo.Warn("error fetching inline media by content_id", "count", len(missing), "error", err) + return + } + for _, media := range medias { + message.Attachments = append(message.Attachments, attachment.Attachment{ + UUID: media.UUID, + Name: media.Filename, + ContentType: media.ContentType, + ContentID: media.ContentID, + Size: media.Size, + Disposition: media.Disposition.String, + }) + } +} + // fetchMessageAttachments fetches attachments (also inline images) for a single message ID. func (m *Manager) fetchMessageAttachments(messageID int) (attachment.Attachments, error) { var attachments attachment.Attachments @@ -1392,3 +1447,16 @@ func (m *Manager) getMediaPreview(media mmodels.Media) string { func inlineContentID(uuid string) string { return "ldsk-" + uuid } + +// findExistingMedia resolves an inbound cid to its stored form: ldsk-* is left as-is, others are namespaced by conversation to avoid cross-conversation collisions. +func (m *Manager) findExistingMedia(rawContentID, conversationUUID string) (string, bool, string) { + storedCID := rawContentID + if !strings.HasPrefix(rawContentID, "ldsk-") { + storedCID = conversationUUID + "_" + rawContentID + } + exists, mediaUUID, err := m.mediaStore.ContentIDExists(storedCID) + if err != nil { + m.lo.Error("error checking media existence by content ID", "content_id", storedCID, "error", err) + } + return storedCID, exists, mediaUUID +} diff --git a/internal/conversation/message_test.go b/internal/conversation/message_test.go index d796ced3..f94eb23c 100644 --- a/internal/conversation/message_test.go +++ b/internal/conversation/message_test.go @@ -397,6 +397,74 @@ func TestExtractInlineImageUUIDs(t *testing.T) { } } +func TestExtractInlineContentIDs(t *testing.T) { + tests := []struct { + name string + body string + want []string + }{ + { + name: "empty_body", + body: "", + want: []string{}, + }, + { + name: "ignores_non_cid_src", + body: ``, + want: []string{}, + }, + { + name: "single_cid_extracted", + body: ``, + want: []string{"ldsk-" + testUUID}, + }, + { + name: "mixed_cid_and_url_returns_only_cids", + body: ``, + want: []string{"ldsk-" + testUUID2}, + }, + { + name: "single_quotes_around_src", + body: ``, + want: []string{"ldsk-" + testUUID}, + }, + { + name: "empty_cid_skipped", + body: ``, + want: []string{}, + }, + { + name: "multi_with_dedup_and_order", + body: ``, + want: []string{"ldsk-" + testUUID2, "ldsk-" + testUUID}, + }, + { + name: "src_after_other_attributes", + body: `x`, + want: []string{"ldsk-" + testUUID}, + }, + { + name: "uppercase_cid_prefix_not_matched", + body: ``, + want: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := extractInlineContentIDs(tt.body) + if len(got) != len(tt.want) { + t.Fatalf("got %v, want %v", got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("index %d: got %q, want %q", i, got[i], tt.want[i]) + } + } + }) + } +} + func TestRewriteInlineImagesToCID(t *testing.T) { tests := []struct { name string diff --git a/internal/media/media.go b/internal/media/media.go index 8f1e8b0d..baaf22e5 100644 --- a/internal/media/media.go +++ b/internal/media/media.go @@ -21,6 +21,7 @@ import ( "github.com/google/uuid" "github.com/jmoiron/sqlx" "github.com/knadh/go-i18n" + "github.com/lib/pq" "github.com/volatiletech/null/v9" "github.com/zerodha/logf" ) @@ -88,6 +89,7 @@ type queries struct { GetByModel *sqlx.Stmt `query:"get-model-media"` GetUnlinkedMessageMedia *sqlx.Stmt `query:"get-unlinked-message-media"` ContentIDExists *sqlx.Stmt `query:"content-id-exists"` + GetByContentIDs *sqlx.Stmt `query:"get-media-by-content-ids"` SetContentID *sqlx.Stmt `query:"set-media-content-id"` } @@ -191,6 +193,19 @@ func (m *Manager) ContentIDExists(contentID string) (bool, string, error) { return true, uuid, nil } +// GetByContentIDs returns media rows matching any of the given content_ids, scoped to the given conversation to prevent cross-conversation lookup. +func (m *Manager) GetByContentIDs(contentIDs []string, conversationUUID string) ([]models.Media, error) { + out := []models.Media{} + if len(contentIDs) == 0 || conversationUUID == "" { + return out, nil + } + if err := m.queries.GetByContentIDs.Select(&out, pq.Array(contentIDs), conversationUUID); err != nil { + m.lo.Error("error fetching media by content_ids", "error", err) + return nil, fmt.Errorf("fetching media by content_ids: %w", err) + } + return out, nil +} + // GetBlob retrieves the raw binary content of a media file by its name. func (m *Manager) GetBlob(name string) ([]byte, error) { return m.store.GetBlob(name) diff --git a/internal/media/queries.sql b/internal/media/queries.sql index b3e8c1cb..2abdecdc 100644 --- a/internal/media/queries.sql +++ b/internal/media/queries.sql @@ -53,6 +53,14 @@ WHERE model_type = 'messages' -- name: content-id-exists SELECT uuid FROM media WHERE content_id = $1; +-- name: get-media-by-content-ids +SELECT m.id, m.created_at, m.updated_at, m."uuid", m.store, m.filename, m.content_type, m.content_id, m.model_id, m.model_type, m.disposition, m."size", m.meta +FROM media m +INNER JOIN conversation_messages cm ON cm.id = m.model_id +WHERE m.model_type = 'messages' + AND m.content_id = ANY($1) + AND cm.conversation_id = (SELECT id FROM conversations WHERE uuid = $2::uuid LIMIT 1); + -- name: set-media-content-id UPDATE media SET content_id = $2 From 0abd96fb70b8414dba52507db39ad79f4c3cbb00 Mon Sep 17 00:00:00 2001 From: Abhinav Raut Date: Fri, 15 May 2026 02:47:30 +0530 Subject: [PATCH 2/2] Fix quoted image resolution --- cmd/messages.go | 23 +++++++++++++++++++---- cmd/messages_test.go | 6 +++--- internal/conversation/message.go | 30 +++++------------------------- 3 files changed, 27 insertions(+), 32 deletions(-) diff --git a/cmd/messages.go b/cmd/messages.go index 22b6fcab..6acd2b48 100644 --- a/cmd/messages.go +++ b/cmd/messages.go @@ -71,7 +71,8 @@ func handleGetMessages(r *fastglue.Request) error { att := messages[i].Attachments[j] messages[i].Attachments[j].URL = app.media.GetURL(att.UUID, att.ContentType, att.Name) } - resolveContentCIDs(&messages[i], rootURL) + resolveQuotedCIDs(app, &messages[i]) + resolveAttachmentCIDs(&messages[i], rootURL) } // Process CSAT status for all messages (will only affect CSAT messages) @@ -134,7 +135,8 @@ func handleGetMessage(r *fastglue.Request) error { att := message.Attachments[j] message.Attachments[j].URL = app.media.GetURL(att.UUID, att.ContentType, att.Name) } - resolveContentCIDs(&message, rootURL) + resolveQuotedCIDs(app, &message) + resolveAttachmentCIDs(&message, rootURL) return r.SendEnvelope(message) } @@ -269,9 +271,9 @@ func handleSendMessage(r *fastglue.Request) error { return r.SendEnvelope(message) } -// resolveContentCIDs replaces inline image cid: references in email message content +// resolveAttachmentCIDs replaces inline image cid: references in email message content // with actual attachment URLs and resolves relative /uploads/ paths to absolute URLs. -func resolveContentCIDs(msg *cmodels.Message, rootURL string) { +func resolveAttachmentCIDs(msg *cmodels.Message, rootURL string) { for _, att := range msg.Attachments { if att.ContentID != "" && att.URL != "" { msg.Content = strings.ReplaceAll(msg.Content, "cid:"+att.ContentID, att.URL) @@ -282,3 +284,16 @@ func resolveContentCIDs(msg *cmodels.Message, rootURL string) { msg.Content = strings.ReplaceAll(msg.Content, `src='/uploads/`, `src='`+rootURL+`/uploads/`) } } + +// resolveQuotedCIDs replaces cid: refs to media on other messages with signed URLs. +func resolveQuotedCIDs(app *App, msg *cmodels.Message) { + refs, err := app.conversation.GetInlineMediaRefs(msg) + if err != nil { + app.lo.Error("error fetching inline media refs", "conversation_uuid", msg.ConversationUUID, "error", err) + return + } + for _, ref := range refs { + url := app.media.GetURL(ref.UUID, ref.ContentType, ref.Filename) + msg.Content = strings.ReplaceAll(msg.Content, "cid:"+ref.ContentID, url) + } +} diff --git a/cmd/messages_test.go b/cmd/messages_test.go index a41c905c..48674d09 100644 --- a/cmd/messages_test.go +++ b/cmd/messages_test.go @@ -7,7 +7,7 @@ import ( cmodels "github.com/abhinavxd/libredesk/internal/conversation/models" ) -func TestResolveContentCIDs(t *testing.T) { +func TestResolveAttachmentCIDs(t *testing.T) { tests := []struct { name string msg cmodels.Message @@ -270,9 +270,9 @@ func TestResolveContentCIDs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - resolveContentCIDs(&tt.msg, tt.rootURL) + resolveAttachmentCIDs(&tt.msg, tt.rootURL) if tt.msg.Content != tt.want { - t.Errorf("resolveContentCIDs()\n got = %s\n want = %s", tt.msg.Content, tt.want) + t.Errorf("resolveAttachmentCIDs()\n got = %s\n want = %s", tt.msg.Content, tt.want) } }) } diff --git a/internal/conversation/message.go b/internal/conversation/message.go index 5d35e877..9a24922a 100644 --- a/internal/conversation/message.go +++ b/internal/conversation/message.go @@ -343,10 +343,6 @@ func (m *Manager) GetConversationMessages(conversationUUID string, page, pageSiz return messages, pageSize, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - for i := range messages { - m.populateInlineMedia(&messages[i]) - } - return messages, pageSize, nil } @@ -358,8 +354,6 @@ func (m *Manager) GetMessage(uuid string) (models.Message, error) { return message, envelope.NewError(envelope.GeneralError, m.i18n.T("globals.messages.somethingWentWrong"), nil) } - m.populateInlineMedia(&message) - // Generate signed URLs for attachments. for i := range message.Attachments { message.Attachments[i].URL = m.mediaStore.GetSignedURL(message.Attachments[i].UUID) @@ -1204,11 +1198,11 @@ func (m *Manager) messageExistsBySourceID(messageSourceIDs []string) (int, error return conversationID, nil } -// populateInlineMedia appends quoted inline images (referenced via cid: in the body but not in the attachments list) so resolveContentCIDs can rewrite them to signed URLs. -func (m *Manager) populateInlineMedia(message *models.Message) { +// GetInlineMediaRefs returns media referenced via cid: in the body but linked to other messages (quoted history). +func (m *Manager) GetInlineMediaRefs(message *models.Message) ([]mmodels.Media, error) { cids := extractInlineContentIDs(message.Content) if len(cids) == 0 { - return + return nil, nil } existing := make(map[string]bool, len(message.Attachments)) for _, a := range message.Attachments { @@ -1223,23 +1217,9 @@ func (m *Manager) populateInlineMedia(message *models.Message) { } } if len(missing) == 0 { - return - } - medias, err := m.mediaStore.GetByContentIDs(missing, message.ConversationUUID) - if err != nil { - m.lo.Warn("error fetching inline media by content_id", "count", len(missing), "error", err) - return - } - for _, media := range medias { - message.Attachments = append(message.Attachments, attachment.Attachment{ - UUID: media.UUID, - Name: media.Filename, - ContentType: media.ContentType, - ContentID: media.ContentID, - Size: media.Size, - Disposition: media.Disposition.String, - }) + return nil, nil } + return m.mediaStore.GetByContentIDs(missing, message.ConversationUUID) } // fetchMessageAttachments fetches attachments (also inline images) for a single message ID.