Merge pull request #4148 from tgxworld/dont_reply_to_emails_that_are_autogenerated

FIX: Don't send rejection mailer to bounced emails.

app/jobs/scheduled/poll_mailbox.rb

@@ -25,17 +25,31 @@ module Jobs
         mail_string = popmail.pop
         receiver = Email::Receiver.new(mail_string)
         receiver.process!
+      rescue Email::Receiver::BouncedEmailError => e
+        log_email_process_failure(mail_string, e)
+
+        set_incoming_email_rejection_message(
+          receiver.incoming_email, I18n.t("email.incoming.errors.bounced_email_report")
+        )
+      rescue Email::Receiver::AutoGeneratedEmailReplyError => e
+        log_email_process_failure(mail_string, e)
+
+        set_incoming_email_rejection_message(
+          receiver.incoming_email,
+          I18n.t("email.incoming.errors.auto_generated_email_reply")
+        )
       rescue => e
         rejection_message = handle_failure(mail_string, e)
-        if rejection_message.present? && receiver && receiver.incoming_email
-          receiver.incoming_email.rejection_message = rejection_message.body.to_s
-          receiver.incoming_email.save
+        if rejection_message.present? && receiver && (incoming_email = receiver.incoming_email)
+          set_incoming_email_rejection_message(
+            incoming_email, rejection_message.body.to_s
+          )
         end
       end
     end
 
     def handle_failure(mail_string, e)
-      Rails.logger.warn("Email can not be processed: #{e}\n\n#{mail_string}") if SiteSetting.log_mail_processing_failures
+      log_email_process_failure(mail_string, e)
 
       message_template = case e
         when Email::Receiver::EmptyEmailError             then :email_reject_empty
@@ -70,6 +84,10 @@ module Jobs
         template_args[:rate_limit_description] = e.description
       end
 
+      if message_template == :email_reject_auto_generated
+        template_args[:mark_as_reply_to_auto_generated] = true
+      end
+
       if message_template
         # inform the user about the rejection
         message = Mail::Message.new(mail_string)
@@ -118,5 +136,17 @@ module Jobs
       $redis.zadd(POLL_MAILBOX_ERRORS_KEY, now, now.to_s)
     end
 
+    private
+
+    def set_incoming_email_rejection_message(incoming_email, message)
+      incoming_email.update_attributes!(rejection_message: message)
+    end
+
+    def log_email_process_failure(mail_string, exception)
+      if SiteSetting.log_mail_processing_failures
+        Rails.logger.warn("Email can not be processed: #{exception}\n\n#{mail_string}")
+      end
+    end
+
   end
 end

config/locales/server.en.yml

@@ -64,6 +64,8 @@ en:
         reply_user_not_matching_error: "Happens when a reply came in from a different email address the notification was sent to."
         topic_not_found_error: "Happens when a reply came in but the related topic has been deleted."
         topic_closed_error: "Happens when a reply came in but the related topic has been closed."
+        bounced_email_report: "Email is a bounced email report."
+        auto_generated_email_reply: "Email contains a reply to an auto generated email."
 
   errors: &errors
     format: ! '%{attribute} %{message}'

lib/email/message_builder.rb

@@ -17,6 +17,9 @@ module Email
   class MessageBuilder
     attr_reader :template_args
 
+    REPLY_TO_AUTO_GENERATED_HEADER_KEY = "X-Discourse-Reply-to-Auto-Generated".freeze
+    REPLY_TO_AUTO_GENERATED_HEADER_VALUE = "marked".freeze
+
     def initialize(to, opts=nil)
       @to = to
       @opts = opts || {}
@@ -132,7 +135,11 @@ module Email
     def header_args
       result = {}
       if @opts[:add_unsubscribe_link]
-        result['List-Unsubscribe'] = "<#{template_args[:user_preferences_url]}>" if @opts[:add_unsubscribe_link]
+        result['List-Unsubscribe'] = "<#{template_args[:user_preferences_url]}>"
+      end
+
+      if @opts[:mark_as_reply_to_auto_generated]
+        result[REPLY_TO_AUTO_GENERATED_HEADER_KEY] = REPLY_TO_AUTO_GENERATED_HEADER_VALUE
       end
 
       result['X-Discourse-Post-Id'] = @opts[:post_id].to_s if @opts[:post_id]

lib/email/receiver.rb

@@ -12,6 +12,8 @@ module Email
     class EmptyEmailError             < ProcessingError; end
     class UserNotFoundError           < ProcessingError; end
     class AutoGeneratedEmailError     < ProcessingError; end
+    class AutoGeneratedEmailReplyError < ProcessingError; end
+    class BouncedEmailError           < ProcessingError; end
     class NoBodyDetectedError         < ProcessingError; end
     class InactiveUserError           < ProcessingError; end
     class BlockedUserError            < ProcessingError; end
@@ -62,6 +64,8 @@ module Email
       body, @elided = select_body
       body ||= ""
 
+      raise BouncedEmailError if (@mail.bounced? && !@mail.retryable?)
+      raise AutoGeneratedEmailReplyError if check_reply_to_auto_generated_header
       raise AutoGeneratedEmailError if is_auto_generated?
       raise NoBodyDetectedError     if body.blank? && !@mail.has_attachments?
       raise InactiveUserError       if !user.active && !user.staged
@@ -424,6 +428,20 @@ module Email
       !topic.topic_allowed_groups.where("group_id IN (SELECT group_id FROM group_users WHERE user_id = ?)", user.id).exists?
     end
 
+    private
+
+    def check_reply_to_auto_generated_header
+      headers = Mail::Header.new(@mail.body.to_s.gsub("\r\n\r\n", "\r\n")).to_a
+
+      index = headers.find_index do |f|
+        f.name == Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_KEY
+      end
+
+      if index
+        headers[index].value == Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_VALUE
+      end
+    end
+
   end
 
 end

spec/components/email/message_builder_spec.rb

@@ -136,10 +136,15 @@ describe Email::MessageBuilder do
 
   context "header args" do
 
-    let(:message_with_header_args) { Email::MessageBuilder.new(to_address,
-                                                               body: 'hello world',
-                                                               topic_id: 1234,
-                                                               post_id: 4567) }
+    let(:message_with_header_args) do
+      Email::MessageBuilder.new(
+        to_address,
+        body: 'hello world',
+        topic_id: 1234,
+        post_id: 4567,
+        mark_as_reply_to_auto_generated: true
+      )
+    end
 
     it "passes through a post_id" do
       expect(message_with_header_args.header_args['X-Discourse-Post-Id']).to eq('4567')
@@ -149,6 +154,12 @@ describe Email::MessageBuilder do
       expect(message_with_header_args.header_args['X-Discourse-Topic-Id']).to eq('1234')
     end
 
+    it "marks the email as replying to an auto generated email" do
+      expect(message_with_header_args.header_args[
+        Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_KEY
+      ]).to eq(Email::MessageBuilder::REPLY_TO_AUTO_GENERATED_HEADER_VALUE)
+    end
+
   end
 
   context "unsubscribe link" do

spec/components/email/receiver_spec.rb

@@ -8,10 +8,6 @@ describe Email::Receiver do
     SiteSetting.reply_by_email_address = "reply+%{reply_key}@bar.com"
   end
 
-  def email(email_name)
-    fixture_file("emails/#{email_name}.eml")
-  end
-
   def process(email_name)
     Email::Receiver.new(email(email_name)).process!
   end
@@ -54,6 +50,17 @@ describe Email::Receiver do
     expect { process(:bad_destinations) }.to raise_error(Email::Receiver::BadDestinationAddress)
   end
 
+  it "raises an BouncerEmailError when email is a bounced email" do
+    expect { process(:bounced_email) }.to raise_error(Email::Receiver::BouncedEmailError)
+  end
+
+  it "raises an AutoGeneratedEmailReplyError when email contains a reply marked
+    as reply to an auto generated email".squish do
+
+    expect { process(:bounced_email_2) }
+      .to raise_error(Email::Receiver::AutoGeneratedEmailReplyError)
+  end
+
   context "reply" do
 
     let(:reply_key) { "4f97315cc828096c9cb34c6f1a0d6fe8" }

spec/jobs/poll_mailbox_spec.rb

@@ -44,4 +44,32 @@ describe Jobs::PollMailbox do
     end
   end
 
+  describe "#process_popmail" do
+    def process_popmail(email_name)
+      pop_mail = stub("pop mail")
+      pop_mail.expects(:pop).returns(email(email_name))
+      Jobs::PollMailbox.new.process_popmail(pop_mail)
+    end
+
+    it "does not reply to a bounced email" do
+      expect { process_popmail(:bounced_email) }.to_not change { ActionMailer::Base.deliveries.count }
+
+      incoming_email = IncomingEmail.last
+
+      expect(incoming_email.rejection_message).to eq(
+        I18n.t("email.incoming.errors.bounced_email_report")
+      )
+    end
+
+    it "does not reply to an email containing a reply to an auto generated email" do
+      expect { process_popmail(:bounced_email_2) }.to_not change { ActionMailer::Base.deliveries.count }
+
+      incoming_email = IncomingEmail.last
+
+      expect(incoming_email.rejection_message).to eq(
+        I18n.t("email.incoming.errors.auto_generated_email_reply")
+      )
+    end
+  end
+
 end

spec/support/helpers.rb

@@ -79,4 +79,8 @@ module Helpers
     result
   end
 
+  def email(email_name)
+    fixture_file("emails/#{email_name}.eml")
+  end
+
 end