From 5ea89d1fcbe1c7b7fe931c4592b19424208ce93f Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Thu, 9 Mar 2023 11:52:26 +1000
Subject: [PATCH] FIX: UploadReference order by tiebreaker for UploadSecurity
(#20602)
Follow up to 4d2a95ffe6d63b8d2b5019654dd086f6a9bbbd44. Sometimes
due to the original UploadReference migration or other issues,
multiple UploadReference records can have the exact same
created_at date and time. To tiebreak and correct the SQL order
when this happens, we can add a secondary `id ASC` ordering
when we check for the first upload reference.
---
lib/upload_security.rb | 2 +-
spec/lib/upload_security_spec.rb | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/lib/upload_security.rb b/lib/upload_security.rb
index c00c1376ecf..ecdc3c43b40 100644
--- a/lib/upload_security.rb
+++ b/lib/upload_security.rb
@@ -143,7 +143,7 @@ class UploadSecurity
LEFT JOIN posts ON upload_references.target_type = 'Post' AND upload_references.target_id = posts.id
SQL
.where("posts.deleted_at IS NULL")
- .order(created_at: :asc)
+ .order("upload_references.created_at ASC, upload_references.id ASC")
.first
return false if first_reference.blank?
PUBLIC_UPLOAD_REFERENCE_TYPES.include?(first_reference.target_type)
diff --git a/spec/lib/upload_security_spec.rb b/spec/lib/upload_security_spec.rb
index 8834ff4fc40..a4feac8646d 100644
--- a/spec/lib/upload_security_spec.rb
+++ b/spec/lib/upload_security_spec.rb
@@ -257,6 +257,18 @@ RSpec.describe UploadSecurity do
create_secure_post_reference
expect(subject.should_be_secure?).to eq(false)
end
+
+ context "when the created_at dates for upload references are identical" do
+ it "orders by id as well and returns false" do
+ now = Time.zone.now
+ custom_emoji = CustomEmoji.create(name: "meme", upload: upload)
+ create_secure_post_reference
+
+ UploadReference.find_by(target: custom_emoji).update!(created_at: now)
+ UploadReference.find_by(target: post_in_secure_context).update!(created_at: now)
+ expect(subject.should_be_secure?).to eq(false)
+ end
+ end
end
describe "when the upload is first used for a badge" do