FIX: Drop malformed CC addresses in GroupSmtpEmail job (#14934)

FIX: Drop malformed CC addresses in GroupSmtpEmail job (#14934)

Sometimes, a user may have a malformed email such as test@test.com<mailto:test@test.com their email address, and as a topic participant will be included as a CC email when sending a GroupSmtpEmail. This causes the CC parsing to fail and further down the line in Email::Sender the code to check the CC addresses expects an array but gets a string instead because of the parse failure.

Instead, we can just check if the CC addresses are valid and drop them if they are not in the GroupSmtpEmail job.

diff --git a/app/jobs/regular/group_smtp_email.rb b/app/jobs/regular/group_smtp_email.rb
index 3970eca..61bc09f 100644
--- a/app/jobs/regular/group_smtp_email.rb
+++ b/app/jobs/regular/group_smtp_email.rb
@@ -23,27 +23,29 @@ module Jobs
 
     def execute(args)
       return if quit_email_early?
-
-      group = Group.find_by(id: args[:group_id])
-      return if group.blank?
-
-      post = Post.find_by(id: args[:post_id])
       email = args[:email]
-      cc_addresses = args[:cc_emails]
       recipient_user = User.find_by_email(email, primary: true)
 
+      post = Post.find_by(id: args[:post_id])
       if post.blank?
         return skip(email, nil, recipient_user, :group_smtp_post_deleted)
       end
 
-      if !Topic.exists?(id: post.topic_id)
-        return skip(email, post, recipient_user, :group_smtp_topic_deleted)
-      end
+      group = Group.find_by(id: args[:group_id])
+      return if group.blank?
 
       if !group.smtp_enabled
         return skip(email, post, recipient_user, :group_smtp_disabled_for_group)
       end
 
+      if !Topic.exists?(id: post.topic_id)
+        return skip(email, post, recipient_user, :group_smtp_topic_deleted)
+      end
+
+      cc_addresses = args[:cc_emails].map do |cc|
+        cc.match(EmailValidator.email_regex) ? cc : nil
+      end.compact
+
       # There is a rare race condition causing the Imap::Sync class to create
       # an incoming email and associated post/topic, which then kicks off
       # the PostAlerter to notify others in the PM about a reply in the topic,
diff --git a/spec/jobs/regular/group_smtp_email_spec.rb b/spec/jobs/regular/group_smtp_email_spec.rb
index 8923ab4..81600be 100644
--- a/spec/jobs/regular/group_smtp_email_spec.rb
+++ b/spec/jobs/regular/group_smtp_email_spec.rb
@@ -160,6 +160,16 @@ RSpec.describe Jobs::GroupSmtpEmail do
     expect(email_log.smtp_group_id).to eq(group.id)
   end
 
+  it "drops malformed cc addresses when sending the email" do
+    args2 = args.clone
+    args2[:cc_emails] << "somebadccemail@test.com<mailto:somebadccemail@test.com"
+    subject.execute(args2)
+    expect(ActionMailer::Base.deliveries.count).to eq(1)
+    last_email = ActionMailer::Base.deliveries.last
+    expect(last_email.subject).to eq("Re: Help I need support")
+    expect(last_email.cc).to match_array(["otherguy@test.com", "cormac@lit.com"])
+  end
+
   context "when there are cc_addresses" do
     it "has the cc_addresses and cc_user_ids filled in correctly" do
       subject.execute(args)

GitHub sha: eabe2df8d23f7ec7c08d5db9f98f7e2442f27828

This commit appears in #14934 which was approved by techAPJ. It was merged by martin.