From 3246c3cc9264fcc5f525f15728f0eeea68587705 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Thu, 18 Feb 2021 20:15:02 +0200 Subject: [PATCH] DEV: Update mail and use fork (#10639) Version 2.8 brings some changes to how address fields are handled and this commits updates that and should also include a fix which handles encoded attachment filenames. The fork contains a bugfix to correctly decode mail attachments. --- Gemfile | 2 +- Gemfile.lock | 11 ++++-- lib/email/receiver.rb | 45 +++++++++++----------- spec/components/email/receiver_spec.rb | 8 ++++ spec/components/imap/imap_helper.rb | 2 +- spec/fixtures/emails/encoded_filename.eml | 46 +++++++++++++++++++++++ 6 files changed, 86 insertions(+), 28 deletions(-) create mode 100644 spec/fixtures/emails/encoded_filename.eml diff --git a/Gemfile b/Gemfile index 80bf241db0d..f9572ed7d93 100644 --- a/Gemfile +++ b/Gemfile @@ -40,7 +40,7 @@ gem 'actionview_precompiler', require: false gem 'seed-fu' -gem 'mail', require: false +gem 'mail', git: 'https://github.com/discourse/mail.git', require: false gem 'mini_mime' gem 'mini_suffix' diff --git a/Gemfile.lock b/Gemfile.lock index 9af91bc9fc8..9b763f59b31 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,10 @@ +GIT + remote: https://github.com/discourse/mail.git + revision: 5b700fc95ee66378e0cf2559abc73c8bc3062a4b + specs: + mail (2.8.0.edge) + mini_mime (>= 0.1.1) + GEM remote: https://rubygems.org/ specs: @@ -192,8 +199,6 @@ GEM nokogiri (>= 1.5.9) lru_redux (1.1.0) lz4-ruby (0.3.3) - mail (2.7.1) - mini_mime (>= 0.1.1) maxminddb (0.1.22) memory_profiler (1.0.0) message_bus (3.3.4) @@ -517,7 +522,7 @@ DEPENDENCIES logster lru_redux lz4-ruby - mail + mail! maxminddb memory_profiler message_bus diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index c83a7b8dfc6..ed2b8c8fc82 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -564,8 +564,7 @@ module Email return unless mail[:from] if mail[:from].errors.blank? - mail[:from].address_list.addresses.each do |address_field| - address_field.decoded + mail[:from].each do |address_field| from_address = address_field.address from_display_name = address_field.display_name.try(:to_s) return [from_address&.downcase, from_display_name&.strip] if from_address["@"] @@ -1243,29 +1242,29 @@ module Email def add_other_addresses(post, sender) %i(to cc bcc).each do |d| - if @mail[d] && @mail[d].address_list && @mail[d].address_list.addresses - @mail[d].address_list.addresses.each do |address_field| - begin - address_field.decoded - email = address_field.address.downcase - display_name = address_field.display_name.try(:to_s) - next unless email["@"] - if should_invite?(email) - user = find_or_create_user(email, display_name) - if user && can_invite?(post.topic, user) - post.topic.topic_allowed_users.create!(user_id: user.id) - TopicUser.auto_notification_for_staging(user.id, post.topic_id, TopicUser.notification_reasons[:auto_watch]) - post.topic.add_small_action(sender, "invited_user", user.username, import_mode: @opts[:import_mode]) - end - # cap number of staged users created per email - if @staged_users.count > SiteSetting.maximum_staged_users_per_email - post.topic.add_moderator_post(sender, I18n.t("emails.incoming.maximum_staged_user_per_email_reached"), import_mode: @opts[:import_mode]) - return - end + next if @mail[d].blank? + + @mail[d].each do |address_field| + begin + address_field.decoded + email = address_field.address.downcase + display_name = address_field.display_name.try(:to_s) + next unless email["@"] + if should_invite?(email) + user = find_or_create_user(email, display_name) + if user && can_invite?(post.topic, user) + post.topic.topic_allowed_users.create!(user_id: user.id) + TopicUser.auto_notification_for_staging(user.id, post.topic_id, TopicUser.notification_reasons[:auto_watch]) + post.topic.add_small_action(sender, "invited_user", user.username, import_mode: @opts[:import_mode]) + end + # cap number of staged users created per email + if @staged_users.count > SiteSetting.maximum_staged_users_per_email + post.topic.add_moderator_post(sender, I18n.t("emails.incoming.maximum_staged_user_per_email_reached"), import_mode: @opts[:import_mode]) + return end - rescue ActiveRecord::RecordInvalid, EmailNotAllowed - # don't care if user already allowed or the user's email address is not allowed end + rescue ActiveRecord::RecordInvalid, EmailNotAllowed + # don't care if user already allowed or the user's email address is not allowed end end end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 38b2d5aa3cb..6613f037a5d 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -646,6 +646,14 @@ describe Email::Receiver do MD end + it "can decode attachments" do + SiteSetting.authorized_extensions = "pdf" + Fabricate(:group, incoming_email: "one@foo.com") + + process(:encoded_filename) + expect(Upload.last.original_filename).to eq("This is a test.pdf") + end + context "when attachment is rejected" do it "sends out the warning email" do expect { process(:attached_txt_file) }.to change { EmailLog.count }.by(1) diff --git a/spec/components/imap/imap_helper.rb b/spec/components/imap/imap_helper.rb index 99bcc412240..1248fa0cae7 100644 --- a/spec/components/imap/imap_helper.rb +++ b/spec/components/imap/imap_helper.rb @@ -18,7 +18,7 @@ def EmailFabricator(options) email += "Cc: #{options[:cc]}\n" if options[:cc] email += "In-Reply-To: #{options[:in_reply_to]}\n" if options[:in_reply_to] email += "References: #{options[:in_reply_to]}\n" if options[:in_reply_to] - email += "Message-ID: #{options[:message_id]}\n" if options[:message_id] + email += "Message-ID: <#{options[:message_id]}>\n" if options[:message_id] email += "Subject: #{options[:subject] || "This is a test email subhect"}\n" email += "Mime-Version: 1.0\n" email += "Content-Type: #{options[:content_type] || "text/plain;\n charset=UTF-8"}\n" diff --git a/spec/fixtures/emails/encoded_filename.eml b/spec/fixtures/emails/encoded_filename.eml new file mode 100644 index 00000000000..8f51f6f1fb5 --- /dev/null +++ b/spec/fixtures/emails/encoded_filename.eml @@ -0,0 +1,46 @@ +From xxxxxxxxx.xxxxxxx@gmail.com Sun May 8 19:07:09 2005 +Return-Path: +Message-ID: +Date: Sun, 8 May 2005 14:09:11 -0500 +From: xxxxxxxxx xxxxxxx +Reply-To: xxxxxxxxx xxxxxxx +To: one@foo.com +Subject: Fwd: Signed email causes file attachments +In-Reply-To: +Mime-Version: 1.0 +Content-Type: multipart/mixed; + boundary="----=_Part_5028_7368284.1115579351471" +References: + +------=_Part_5028_7368284.1115579351471 +Content-Type: text/plain; charset=ISO-8859-1 +Content-Transfer-Encoding: quoted-printable +Content-Disposition: inline + +We should not include these files or vcards as attachments. + +---------- Forwarded message ---------- +From: xxxxx xxxxxx +Date: May 8, 2005 1:17 PM +Subject: Signed email causes file attachments +To: xxxxxxx@xxxxxxxxxx.com + + +Hi, + +Test attachments with Base64 encoded filename. + + +------=_Part_5028_7368284.1115579351471 +Content-Type: application/pdf; name==?utf-8?B?VGhpcyBpcyBhIHRlc3QucGRm?= +Content-Transfer-Encoding: base64 +Content-Disposition: attachment; filename==?utf-8?B?VGhpcyBpcyBhIHRlc3QucGRm?= + +MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAQAAoIIGFDCCAs0w +ggI2oAMCAQICAw5c+TANBgkqhkiG9w0BAQQFADBiMQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhh +d3RlIENvbnN1bHRpbmcgKFB0eSkgTHRkLjEsMCoGA1UEAxMjVGhhd3RlIFBlcnNvbmFsIEZyZWVt +YWlsIElzc3VpbmcgQ0EwHhcNMDUwMzI5MDkzOTEwWhcNMDYwMzI5MDkzOTEwWjBCMR8wHQYDVQQD +ExZUaGF3dGUgRnJlZW1haWwgTWVtYmVyMR8wHQYJKoZIhvcNAQkBFhBzbWhhdW5jaEBtYWMuY29t +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAn90dPsYS3LjfMY211OSYrDQLzwNYPlAL +7+/0XA+kdy8/rRnyEHFGwhNCDmg0B6pxC7z3xxJD/8GfCd+IYUUNUQV5m9MkxfP9pTVXZVIYLaBw +------=_Part_5028_7368284.1115579351471--