From dbf4d3a0a68b9f46bd2d30eff2fa9f37edd45d43 Mon Sep 17 00:00:00 2001 From: Abhinav Raut Date: Thu, 14 May 2026 22:36:11 +0530 Subject: [PATCH] Fix inline image media not linking on S3 presigned URLs Also simplify the inline-image helpers: one matcher plus stringutil.ExtractUUID, instead of two combined regexes with offset math. --- internal/conversation/message.go | 45 +++--- internal/conversation/message_test.go | 224 +++++++++++++++++++++++--- 2 files changed, 223 insertions(+), 46 deletions(-) diff --git a/internal/conversation/message.go b/internal/conversation/message.go index 60220606..2f8edef1 100644 --- a/internal/conversation/message.go +++ b/internal/conversation/message.go @@ -35,14 +35,8 @@ const ( upgradeWindowTTL = 7 * 24 * time.Hour ) -// For : -// -// group 1 = `]*?\bsrc=["'])(?:https?://[^"'<>\s/]+)?/uploads/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})(?:\?[^"'<>\s]*)?(["'])`, -) +// Matches and captures the URL for downstream parsing. +var imgSrcPattern = regexp.MustCompile(`(?i)]*?\bsrc=["']([^"']*)["']`) // Run starts a pool of worker goroutines to handle message dispatching via inbox's channel and processes incoming messages. It scans for // pending outgoing messages at the specified read interval and pushes them to the outgoing queue to be sent. @@ -991,34 +985,39 @@ func (c *Manager) generateMessagesQuery(baseQuery string, qArgs []interface{}, p return sqlQuery, pageSize, qArgs, nil } -// extractInlineImageUUIDs returns the unique media UUIDs referenced by -// in the body, in order of first appearance. +// extractInlineImageUUIDs returns unique media UUIDs from URLs in order of first appearance, skipping the cid: form. func extractInlineImageUUIDs(content string) []string { - matches := imgSrcUploadsPattern.FindAllStringSubmatch(content, -1) + matches := imgSrcPattern.FindAllStringSubmatch(content, -1) seen := make(map[string]bool, len(matches)) out := make([]string, 0, len(matches)) - for _, sub := range matches { - if len(sub) < 3 { + for _, m := range matches { + url := m[1] + if strings.HasPrefix(url, "cid:") { continue } - if seen[sub[2]] { + u := stringutil.ExtractUUID(url) + if u == "" || seen[u] { continue } - seen[sub[2]] = true - out = append(out, sub[2]) + seen[u] = true + out = append(out, u) } return out } -// rewriteInlineImagesToCID replaces every with -// . +// rewriteInlineImagesToCID rewrites every to . Already-cid form is left alone. func rewriteInlineImagesToCID(content string) string { - return imgSrcUploadsPattern.ReplaceAllStringFunc(content, func(match string) string { - sub := imgSrcUploadsPattern.FindStringSubmatch(match) - if len(sub) < 4 { + return imgSrcPattern.ReplaceAllStringFunc(content, func(match string) string { + sub := imgSrcPattern.FindStringSubmatch(match) + url := sub[1] + if strings.HasPrefix(url, "cid:") { return match } - return sub[1] + "cid:" + inlineContentID(sub[2]) + sub[3] + u := stringutil.ExtractUUID(url) + if u == "" { + return match + } + return strings.Replace(match, url, "cid:"+inlineContentID(u), 1) }) } @@ -1031,7 +1030,7 @@ func (m *Manager) linkInlineMediaToMessage(uuids []string, messageID int) { if err != nil { continue } - if media.Model.String != mmodels.ModelMessages { + if media.Model.Valid && media.Model.String != mmodels.ModelMessages { continue } // Linked to a different message already, leave it. diff --git a/internal/conversation/message_test.go b/internal/conversation/message_test.go index 6790cc3d..d796ced3 100644 --- a/internal/conversation/message_test.go +++ b/internal/conversation/message_test.go @@ -5,8 +5,8 @@ import ( "testing" ) -const testUUID = "abcdef01-2345-6789-abcd-ef0123456789" -const testUUID2 = "11111111-2222-3333-4444-555555555555" +const testUUID = "d0355103-455f-4c7d-b9c7-86e9254fe119" +const testUUID2 = "edb7be78-ef7d-4fe9-888b-22494f0ce076" func TestImgSrcUploadsPattern(t *testing.T) { tests := []struct { @@ -77,8 +77,8 @@ func TestImgSrcUploadsPattern(t *testing.T) { wantUUIDs: []string{testUUID}, }, { - name: "multiline_tag", - body: "", + name: "multiline_tag", + body: "", wantCount: 1, wantUUIDs: []string{testUUID}, }, @@ -90,12 +90,11 @@ func TestImgSrcUploadsPattern(t *testing.T) { wantUUIDs: []string{testUUID, testUUID2}, }, - // (?i) makes hex class case-insensitive too. { - name: "quirk_uppercase_hex_uuid_matches", - body: ``, + name: "uppercase_hex_uuid_matches", + body: ``, wantCount: 1, - wantUUIDs: []string{"ABCDEF01-2345-6789-ABCD-EF0123456789"}, + wantUUIDs: []string{"D0355103-455F-4C7D-B9C7-86E9254FE119"}, }, // `\b` boundary lets data-src match; harmless, no real src to render. { @@ -138,34 +137,130 @@ func TestImgSrcUploadsPattern(t *testing.T) { body: ``, wantCount: 0, }, - { - name: "trailing_path_segment_no_match", - body: ``, - wantCount: 0, - }, { name: "empty_src_no_match", body: ``, wantCount: 0, }, + + { + name: "trailing_path_segment_still_matches", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "s3_path_style_public_url", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "s3_virtual_hosted_url", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "s3_presigned_url_full", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "s3_presigned_url_html_entities", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "s3_nested_bucket_path", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "s3_nested_bucket_path_presigned", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "s3_compatible_endpoint", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "multiple_s3_presigned_urls", + body: `` + + ``, + wantCount: 2, + wantUUIDs: []string{testUUID, testUUID2}, + }, + { + name: "cdn_proxied_url", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "uuid_in_query_param", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "cid_form_skipped", + body: ``, + wantCount: 0, + }, + { + name: "s3_presigned_realistic_long_url", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "fs_store_signed_url", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "fs_store_signed_url_html_entity", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, + { + name: "fs_store_custom_port", + body: ``, + wantCount: 1, + wantUUIDs: []string{testUUID}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - matches := imgSrcUploadsPattern.FindAllStringSubmatch(tt.body, -1) - if len(matches) != tt.wantCount { - t.Fatalf("match count = %d, want %d (matches=%v)", len(matches), tt.wantCount, matches) + got := extractInlineImageUUIDs(tt.body) + if len(got) != tt.wantCount { + t.Fatalf("got %d uuids, want %d (got=%v)", len(got), tt.wantCount, got) } for i, want := range tt.wantUUIDs { - if matches[i][2] != want { - t.Errorf("match %d uuid = %q, want %q", i, matches[i][2], want) + if !strings.EqualFold(got[i], want) { + t.Errorf("uuid %d = %q, want %q", i, got[i], want) } } }) } } - func TestImgSrcUploadsPattern_Adversarial(t *testing.T) { tests := []struct { name string @@ -206,10 +301,10 @@ func TestImgSrcUploadsPattern_Adversarial(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - matches := imgSrcUploadsPattern.FindAllStringSubmatch(tt.body, -1) - if len(matches) != tt.wantCount { - t.Errorf("got %d matches, want %d\nbody: %s\nmatches: %v", - len(matches), tt.wantCount, tt.body, matches) + got := extractInlineImageUUIDs(tt.body) + if len(got) != tt.wantCount { + t.Errorf("got %d uuids, want %d\nbody: %s\nuuids: %v", + len(got), tt.wantCount, tt.body, got) } }) } @@ -251,6 +346,40 @@ func TestExtractInlineImageUUIDs(t *testing.T) { body: ``, want: []string{}, }, + { + name: "mixed_cid_and_s3_extracts_only_s3", + body: `` + + ``, + want: []string{testUUID2}, + }, + { + name: "s3_presigned_url_extracts_uuid", + body: ``, + want: []string{testUUID}, + }, + { + name: "nested_bucket_path_extracts_uuid", + body: ``, + want: []string{testUUID}, + }, + { + name: "dedupes_across_s3_and_relative", + body: `` + + ``, + want: []string{testUUID}, + }, + { + name: "footer_image_ignored_inline_extracted", + body: `
brand
` + + `

Hello

` + + ``, + want: []string{testUUID}, + }, + { + name: "fs_store_signed_url_extracts", + body: ``, + want: []string{testUUID}, + }, } for _, tt := range tests { @@ -294,6 +423,40 @@ func TestRewriteInlineImagesToCID(t *testing.T) { body: ``, want: ``, }, + { + name: "s3_presigned_url_rewritten_to_cid", + body: ``, + want: ``, + }, + { + name: "s3_presigned_url_with_html_entities_rewritten", + body: ``, + want: ``, + }, + { + name: "s3_nested_path_rewritten", + body: ``, + want: ``, + }, + { + name: "s3_virtual_hosted_rewritten", + body: ``, + want: ``, + }, + { + name: "multiple_s3_urls_rewritten", + body: `` + + ``, + want: `` + + ``, + }, + { + name: "mixed_cid_and_s3_leaves_cid_alone", + body: `` + + ``, + want: `` + + ``, + }, { name: "preserves_other_attributes", body: `hi`, @@ -348,4 +511,19 @@ func TestRewriteInlineImagesToCID(t *testing.T) { t.Errorf("expected 0 URL-form UUIDs after rewrite, got %v", leftover) } }) + + t.Run("round_trip_s3_presigned_to_cid", func(t *testing.T) { + body := `` + rewritten := rewriteInlineImagesToCID(body) + if strings.Contains(rewritten, "amazonaws.com") { + t.Errorf("rewritten body still contains presigned URL: %s", rewritten) + } + if strings.Contains(rewritten, "X-Amz-Signature") { + t.Errorf("rewritten body still contains X-Amz-Signature: %s", rewritten) + } + leftover := extractInlineImageUUIDs(rewritten) + if len(leftover) != 0 { + t.Errorf("expected 0 URL-form UUIDs after rewrite, got %v", leftover) + } + }) }