From bfc0f3f4cdfd8b17575109ab485366b220a2932a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 29 Apr 2024 16:56:16 +0200
Subject: [PATCH] FIX: prevent duplicate attachments in incoming emails - take
 2

This is a follow up of https://github.com/discourse/discourse/commit/5fcb7c262ddfff0a8f0a08d0a8eb90a942122721

It was missing the case where secure uploads is enabled, which creates a copy of the upload no matter what.

So this checks for the original_sha1 of the uploads as well when checking for duplicates.
---
 lib/email/receiver.rb           | 16 +++++++++-------
 spec/lib/email/receiver_spec.rb | 13 +++++++++++++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb
index 49cd938b3c7..a7583541d98 100644
--- a/lib/email/receiver.rb
+++ b/lib/email/receiver.rb
@@ -1359,11 +1359,12 @@ module Email
     def add_attachments(raw, user, options = {})
       raw = raw.dup
 
-      previous_upload_ids =
-        UploadReference
-          .where(target_id: Post.where(topic_id: options[:topic_id]).select(:id))
-          .pluck(:upload_id)
-          .uniq
+      upload_ids =
+        UploadReference.where(
+          target_id: Post.where(topic_id: options[:topic_id]).select(:id),
+        ).pluck("DISTINCT upload_id")
+
+      upload_shas = Upload.where(id: upload_ids).pluck("DISTINCT COALESCE(original_sha1, sha1)")
 
       rejected_attachments = []
       attachments.each do |attachment|
@@ -1374,6 +1375,7 @@ module Email
           # create the upload for the user
           opts = { for_group_message: options[:is_group_message] }
           upload = UploadCreator.new(tmp, attachment.filename, opts).create_for(user.id)
+          upload_sha = upload.original_sha1.presence || upload.sha1
           if upload.errors.empty?
             # try to inline images
             if attachment.content_type&.start_with?("image/")
@@ -1390,10 +1392,10 @@ module Email
                 end
               elsif raw[/\[image:[^\]]*\]/i]
                 raw.sub!(/\[image:[^\]]*\]/i, UploadMarkdown.new(upload).to_markdown)
-              elsif !previous_upload_ids.include?(upload.id)
+              elsif !upload_ids.include?(upload.id) && !upload_shas.include?(upload_sha)
                 raw << "\n\n#{UploadMarkdown.new(upload).to_markdown}\n\n"
               end
-            elsif !previous_upload_ids.include?(upload.id)
+            elsif !upload_ids.include?(upload.id) && !upload_shas.include?(upload_sha)
               raw << "\n\n#{UploadMarkdown.new(upload).to_markdown}\n\n"
             end
           else
diff --git a/spec/lib/email/receiver_spec.rb b/spec/lib/email/receiver_spec.rb
index 351f105b454..32e30f2191b 100644
--- a/spec/lib/email/receiver_spec.rb
+++ b/spec/lib/email/receiver_spec.rb
@@ -766,6 +766,19 @@ RSpec.describe Email::Receiver do
       expect(topic.posts.last.raw).not_to match %r{upload://}
     end
 
+    it "tries not to repeat duplicate secure attachments" do
+      setup_s3
+      stub_s3_store
+      SiteSetting.secure_uploads = true
+      SiteSetting.authorized_extensions = "jpg"
+
+      expect { process(:logo_1) }.to change { UploadReference.count }.by(1)
+      expect(topic.posts.last.raw).to match %r{upload://}
+
+      expect { process(:logo_2) }.not_to change { UploadReference.count }
+      expect(topic.posts.last.raw).not_to match %r{upload://}
+    end
+
     it "works with removed attachments" do
       SiteSetting.authorized_extensions = "jpg"