ignore emails that are from the reply by email addresses (#5843)

config/locales/server.en.yml

@@ -89,6 +89,7 @@ en:
         auto_generated_email_error: "Happens when the 'precedence' header is set to: list, junk, bulk or auto_reply, or when any other header contains: auto-submitted, auto-replied or auto-generated."
         no_body_detected_error: "Happens when we couldn't extract a body and there were no attachments."
         no_sender_detected_error: "Happens when we couldn't find a valid email address in the From header."
+        from_reply_by_address_error: "Happens when the From header matches the reply by email address."
         inactive_user_error: "Happens when the sender is not active."
         silenced_user_error: "Happens when the sender has been silenced."
         bad_destination_address: "Happens when none of the email addresses in To/Cc/Bcc fields matched a configured incoming email address."

lib/email/processor.rb

@@ -36,6 +36,7 @@ module Email
     def handle_failure(mail_string, e)
       message_template = case e
                          when Email::Receiver::NoSenderDetectedError       then return nil
+                         when Email::Receiver::FromReplyByAddressError     then return nil
                          when Email::Receiver::EmptyEmailError             then :email_reject_empty
                          when Email::Receiver::NoBodyDetectedError         then :email_reject_empty
                          when Email::Receiver::UserNotFoundError           then :email_reject_user_not_found

lib/email/receiver.rb

@@ -21,6 +21,7 @@ module Email
     class BouncedEmailError            < ProcessingError; end
     class NoBodyDetectedError          < ProcessingError; end
     class NoSenderDetectedError        < ProcessingError; end
+    class FromReplyByAddressError      < ProcessingError; end
     class InactiveUserError            < ProcessingError; end
     class SilencedUserError            < ProcessingError; end
     class BadDestinationAddress        < ProcessingError; end
@@ -107,6 +108,7 @@ module Email
     def process_internal
       raise BouncedEmailError  if is_bounce?
       raise NoSenderDetectedError if @from_email.blank?
+      raise FromReplyByAddressError if is_from_reply_by_email_address?
       raise ScreenedEmailError if ScreenedEmail.should_block?(@from_email)
 
       user = find_user(@from_email)
@@ -203,6 +205,10 @@ module Email
       true
     end
 
+    def is_from_reply_by_email_address?
+      Email::Receiver.reply_by_email_address_regex.match(@from_email)
+    end
+
     def verp
       @verp ||= all_destinations.select { |to| to[/\+verp-\h{32}@/] }.first
     end
@@ -723,9 +729,13 @@ module Email
       reply_addresses.flatten!
       reply_addresses.select!(&:present?)
       reply_addresses.map! { |a| Regexp.escape(a) }
-      reply_addresses.map! { |a| a.gsub(Regexp.escape("%{reply_key}"), "(\\h{32})") }
+      reply_addresses.map! { |a| a.gsub("\+", "\+?") }
-
+      reply_addresses.map! { |a| a.gsub(Regexp.escape("%{reply_key}"), "(\\h{32})?") }
-      /#{reply_addresses.join("|")}/
+      if reply_addresses.empty?
+        /$a/ # a regex that can never match
+      else
+        /#{reply_addresses.join("|")}/
+      end
     end
 
     def group_incoming_emails_regex

spec/components/email/processor_spec.rb

@@ -87,6 +87,20 @@ describe Email::Processor do
 
   end
 
+  context "from reply to email address" do
+
+    let(:mail) { "From: reply@bar.com\nTo: reply@bar.com\nSubject: FOO BAR\n\nFoo foo bar bar?" }
+
+    it "ignores the email" do
+      Email::Receiver.any_instance.stubs(:process_internal).raises(Email::Receiver::FromReplyByAddressError.new)
+
+      expect {
+        Email::Processor.process!(mail)
+      }.to change { EmailLog.count }.by(0)
+    end
+
+  end
+
   context "mailinglist mirror" do
     before do
       SiteSetting.email_in = true

spec/components/email/receiver_spec.rb

@@ -165,6 +165,10 @@ describe Email::Receiver do
       expect { process(:reply_user_not_matching) }.to raise_error(Email::Receiver::ReplyUserNotMatchingError)
     end
 
+    it "raises a FromReplyByAddressError when the email is from the reply by email address" do
+      expect { process(:from_reply_by_email_address) }.to raise_error(Email::Receiver::FromReplyByAddressError)
+    end
+
     it "raises a TopicNotFoundError when the topic was deleted" do
       topic.update_columns(deleted_at: 1.day.ago)
       expect { process(:reply_user_matching) }.to raise_error(Email::Receiver::TopicNotFoundError)
@@ -679,24 +683,24 @@ describe Email::Receiver do
       SiteSetting.alternative_reply_by_email_addresses = nil
     end
 
-    it "is empty by default" do
+    it "it maches nothing if there is not reply_by_email_address" do
-      expect(Email::Receiver.reply_by_email_address_regex).to eq(//)
+      expect(Email::Receiver.reply_by_email_address_regex).to eq(/$a/)
     end
 
     it "uses 'reply_by_email_address' site setting" do
       SiteSetting.reply_by_email_address = "foo+%{reply_key}@bar.com"
-      expect(Email::Receiver.reply_by_email_address_regex).to eq(/foo\+(\h{32})@bar\.com/)
+      expect(Email::Receiver.reply_by_email_address_regex).to eq(/foo\+?(\h{32})?@bar\.com/)
     end
 
     it "uses 'alternative_reply_by_email_addresses' site setting" do
       SiteSetting.alternative_reply_by_email_addresses = "alt.foo+%{reply_key}@bar.com"
-      expect(Email::Receiver.reply_by_email_address_regex).to eq(/alt\.foo\+(\h{32})@bar\.com/)
+      expect(Email::Receiver.reply_by_email_address_regex).to eq(/alt\.foo\+?(\h{32})?@bar\.com/)
     end
 
     it "combines both 'reply_by_email' settings" do
       SiteSetting.reply_by_email_address = "foo+%{reply_key}@bar.com"
       SiteSetting.alternative_reply_by_email_addresses = "alt.foo+%{reply_key}@bar.com"
-      expect(Email::Receiver.reply_by_email_address_regex).to eq(/foo\+(\h{32})@bar\.com|alt\.foo\+(\h{32})@bar\.com/)
+      expect(Email::Receiver.reply_by_email_address_regex).to eq(/foo\+?(\h{32})?@bar\.com|alt\.foo\+?(\h{32})?@bar\.com/)
     end
 
   end