diff --git a/app/models/upload.rb b/app/models/upload.rb index cea39988f8e..60b39315c3c 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -433,6 +433,10 @@ class Upload < ActiveRecord::Base self.sha1_from_base62_encoded($2) if url =~ %r{(upload://)?([a-zA-Z0-9]+)(\..*)?} end + def self.sha1_from_long_url(url) + $2 if url =~ URL_REGEX || url =~ OptimizedImage::URL_REGEX + end + def self.sha1_from_base62_encoded(encoded_sha1) sha1 = Base62.decode(encoded_sha1).to_s(16) diff --git a/lib/email/sender.rb b/lib/email/sender.rb index fceb484322b..abf39dc919b 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -129,6 +129,8 @@ module Email # always set a default Message ID from the host @message.header["Message-ID"] = Email::MessageIdService.generate_default + post = nil + topic = nil if topic_id.present? && post_id.present? post = Post.find_by(id: post_id, topic_id: topic_id) @@ -138,7 +140,6 @@ module Email topic = post.topic return skip(SkippedEmailLog.reason_types[:sender_topic_deleted]) if topic.blank? - add_attachments(post) add_identification_field_headers(topic, post) # See https://www.ietf.org/rfc/rfc2919.txt for the List-ID @@ -249,6 +250,10 @@ module Email # Parse the HTML again so we can make any final changes before # sending style = Email::Styles.new(@message.html_part.body.to_s) + if post.present? + @stripped_secure_upload_shas = style.stripped_upload_sha_map.values + add_attachments(post) + end # Suppress images from short emails if SiteSetting.strip_images_from_short_emails && @@ -386,7 +391,17 @@ module Email end def should_attach_image?(upload, optimized_1X = nil) - return if !SiteSetting.secure_uploads_allow_embed_images_in_emails || !upload.secure? + if !SiteSetting.secure_uploads_allow_embed_images_in_emails || + # Sometimes images in a post have a secure URL but are not secure uploads, + # for example if a user uploads an image to a public post then copies the markdown + # into a PM which sends an email, so we have to make sure we attached those + # stripped images here as well. + ( + !upload.secure? && !@stripped_secure_upload_shas.include?(upload.sha1) && + !@stripped_secure_upload_shas.include?(optimized_1X&.sha1) + ) + return + end if (optimized_1X&.filesize || upload.filesize) > SiteSetting.secure_uploads_max_email_embed_image_size_kb.kilobytes return diff --git a/lib/email/styles.rb b/lib/email/styles.rb index d1976f3c517..c248e9cc595 100644 --- a/lib/email/styles.rb +++ b/lib/email/styles.rb @@ -314,26 +314,36 @@ module Email @@plugin_callbacks.each { |block| block.call(@fragment, @opts) } end - def inline_secure_images(attachments, attachments_index) - stripped_media = @fragment.css("[data-stripped-secure-media], [data-stripped-secure-upload]") - upload_shas = {} - stripped_media.each do |div| - url = div["data-stripped-secure-media"] || div["data-stripped-secure-upload"] - filename = File.basename(url) - filename_bare = filename.gsub(File.extname(filename), "") - sha1 = filename_bare.partition("_").first - upload_shas[url] = sha1 - end - uploads = Upload.select(:original_filename, :sha1).where(sha1: upload_shas.values) + def stripped_media + @stripped_media ||= + @fragment.css("[data-stripped-secure-media], [data-stripped-secure-upload]") + end + def stripped_upload_sha_map + @stripped_upload_sha_map ||= + begin + upload_shas = {} + stripped_media.each do |div| + url = div["data-stripped-secure-media"] || div["data-stripped-secure-upload"] + upload_shas[url] = Upload.sha1_from_long_url(url) + end + upload_shas + end + end + + def stripped_secure_image_uploads + upload_shas = stripped_upload_sha_map + Upload.select(:original_filename, :sha1).where(sha1: upload_shas.values) + end + + def inline_secure_images(attachments, attachments_index) + uploads = stripped_secure_image_uploads + upload_shas = stripped_upload_sha_map stripped_media.each do |div| upload = uploads.find do |upl| upl.sha1 == - ( - upload_shas[div["data-stripped-secure-media"]] || - upload_shas[div["data-stripped-secure-upload"]] - ) + upload_shas[div["data-stripped-secure-media"] || div["data-stripped-secure-upload"]] end next if !upload diff --git a/spec/lib/email/sender_spec.rb b/spec/lib/email/sender_spec.rb index b88f95712e5..fb79a136873 100644 --- a/spec/lib/email/sender_spec.rb +++ b/spec/lib/email/sender_spec.rb @@ -651,7 +651,15 @@ RSpec.describe Email::Sender do ) expect(message.html_part.body).to include("cid:") expect(message.html_part.body).to include("embedded-secure-image") + end + + it "embeds an image with a secure URL that has an upload that is not secure" do + @secure_image.update_secure_status(override: false) + Email::Sender.new(message, :valid_type).send expect(message.attachments.length).to eq(4) + expect(message.attachments["logo.png"].body.raw_source.force_encoding("UTF-8")).to eq( + File.read(@secure_image_file), + ) end it "uses correct UTF-8 encoding for the body of the email" do diff --git a/spec/lib/email/styles_spec.rb b/spec/lib/email/styles_spec.rb index 0363358a7f5..549d94701ca 100644 --- a/spec/lib/email/styles_spec.rb +++ b/spec/lib/email/styles_spec.rb @@ -294,7 +294,7 @@ RSpec.describe Email::Styles do end end - describe "inline_secure_images" do + describe "#inline_secure_images" do before do setup_s3 SiteSetting.secure_uploads = true diff --git a/spec/models/upload_spec.rb b/spec/models/upload_spec.rb index 6ac71da0f8c..e1fab6b426b 100644 --- a/spec/models/upload_spec.rb +++ b/spec/models/upload_spec.rb @@ -324,6 +324,32 @@ RSpec.describe Upload do end end + describe ".sha1_from_long_url" do + it "should be able to get the sha1 from a regular upload URL" do + expect( + Upload.sha1_from_long_url( + "https://cdn.test.com/test/original/4X/7/6/5/1b6453892473a467d07372d45eb05abc2031647a.png", + ), + ).to eq("1b6453892473a467d07372d45eb05abc2031647a") + end + + it "should be able to get the sha1 from a secure upload URL" do + expect( + Upload.sha1_from_long_url( + "#{Discourse.base_url}\/secure-uploads/original/1X/1b6453892473a467d07372d45eb05abc2031647a.png", + ), + ).to eq("1b6453892473a467d07372d45eb05abc2031647a") + end + + it "doesn't get a sha1 for a URL that does not match our scheme" do + expect( + Upload.sha1_from_long_url( + "#{Discourse.base_url}\/blah/1b6453892473a467d07372d45eb05abc2031647a.png", + ), + ).to eq(nil) + end + end + describe "#base62_sha1" do it "should return the right value" do upload.update!(sha1: "0000c513e1da04f7b4e99230851ea2aafeb8cc4e")