FIX: Handle incoming emails without email address in From header (#5177)

lib/email/processor.rb

@@ -36,6 +36,7 @@ module Email
     def handle_failure(mail_string, e, incoming_email)
       message_template = case e
                          when Email::Receiver::EmptyEmailError             then :email_reject_empty
+                         when Email::Receiver::NoSenderDetectedError       then :email_reject_empty
                          when Email::Receiver::NoBodyDetectedError         then :email_reject_empty
                          when Email::Receiver::UserNotFoundError           then :email_reject_user_not_found
                          when Email::Receiver::ScreenedEmailError          then :email_reject_screened_email
@@ -52,7 +53,7 @@ module Email
                          when ActiveRecord::Rollback                       then :email_reject_invalid_post
                          when Email::Receiver::InvalidPostAction           then :email_reject_invalid_post_action
                          when Discourse::InvalidAccess                     then :email_reject_invalid_access
-        else                                                   :email_reject_unrecognized_error
+                         else                                                   :email_reject_unrecognized_error
       end
 
       template_args = {}

lib/email/receiver.rb

@@ -16,6 +16,7 @@ module Email
     class AutoGeneratedEmailError      < ProcessingError; end
     class BouncedEmailError            < ProcessingError; end
     class NoBodyDetectedError          < ProcessingError; end
+    class NoSenderDetectedError        < ProcessingError; end
     class InactiveUserError            < ProcessingError; end
     class BlockedUserError             < ProcessingError; end
     class BadDestinationAddress        < ProcessingError; end
@@ -75,6 +76,7 @@ module Email
 
     def process_internal
       raise BouncedEmailError  if is_bounce?
+      raise NoSenderDetectedError if @from_email.blank?
       raise ScreenedEmailError if ScreenedEmail.should_block?(@from_email)
 
       user = find_or_create_user(@from_email, @from_display_name)
@@ -273,14 +275,27 @@ module Email
         end
       end
 
-      if mail.from[/<[^>]+>/]
-        from_address = mail.from[/<([^>]+)>/, 1]
-        from_display_name = mail.from[/^([^<]+)/, 1]
+      return extract_from_address_and_name(mail.from) if mail.from.is_a? String
+
+      if mail.from.is_a? Mail::AddressContainer
+        mail.from.each do |from|
+          from_address, from_display_name = extract_from_address_and_name(from)
+          return [from_address, from_display_name] if from_address
+        end
       end
 
-      if (from_address.blank? || !from_address["@"]) && mail.from[/\[mailto:[^\]]+\]/]
-        from_address = mail.from[/\[mailto:([^\]]+)\]/, 1]
-        from_display_name = mail.from[/^([^\[]+)/, 1]
+      nil
+    end
+
+    def extract_from_address_and_name(value)
+      if value[/<[^>]+>/]
+        from_address = value[/<([^>]+)>/, 1]
+        from_display_name = value[/^([^<]+)/, 1]
+      end
+
+      if (from_address.blank? || !from_address["@"]) && value[/\[mailto:[^\]]+\]/]
+        from_address = value[/\[mailto:([^\]]+)\]/, 1]
+        from_display_name = value[/^([^\[]+)/, 1]
       end
 
       [from_address&.downcase, from_display_name&.strip]

spec/components/email/receiver_spec.rb

@@ -37,6 +37,10 @@ describe Email::Receiver do
     expect { process(:no_body) }.to raise_error(Email::Receiver::NoBodyDetectedError)
   end
 
+  it "raises a NoSenderDetectedError when the From header is missing" do
+    expect { process(:no_from) }.to raise_error(Email::Receiver::NoSenderDetectedError)
+  end
+
   it "raises an InactiveUserError when the sender is inactive" do
     Fabricate(:user, email: "inactive@bar.com", active: false)
     expect { process(:inactive_sender) }.to raise_error(Email::Receiver::InactiveUserError)
@@ -229,10 +233,14 @@ describe Email::Receiver do
     end
 
     it "handles invalid from header" do
-      expect { process(:invalid_from) }.to change { topic.posts.count }
+      expect { process(:invalid_from_1) }.to change { topic.posts.count }
       expect(topic.posts.last.raw).to eq("This email was sent with an invalid from header field.")
     end
 
+    it "raises a NoSenderDetectedError when the From header doesn't contain an email address" do
+      expect { process(:invalid_from_2) }.to raise_error(Email::Receiver::NoSenderDetectedError)
+    end
+
     it "doesn't raise an AutoGeneratedEmailError when the mail is auto generated but is whitelisted" do
       SiteSetting.auto_generated_whitelist = "foo@bar.com|discourse@bar.com"
       expect { process(:auto_generated_whitelisted) }.to change { topic.posts.count }

spec/fixtures/emails/invalid_from_2.eml

@@ -0,0 +1,12 @@
+Return-Path: <MAILER-DAEMON>
+Date: Fri, 25 Aug 2017 21:25:34 +0000 (UTC)
+From: MAILER-DAEMON (Mail Delivery System)
+Subject: Undelivered Mail Returned to Sender
+To: foo@example.com
+Message-Id: <12345@foo.bar.mail>
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: quoted-printable
+
+This is a mail with an invalid from header that can't be fixed.
+It's missing the sender's email address.

spec/fixtures/emails/no_from.eml

@@ -0,0 +1,9 @@
+To: meat@bar.com
+Date: Mon, 1 Feb 2016 00:12:43 +0100
+Message-ID: <40@foo.bar.mail>
+Mime-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: quoted-printable
+Subject: Nobody sent this
+
+This is an email without a FROM header.