From 5710d5d771537d106fbe3488355ded00e514c073 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Tue, 19 Jan 2021 09:49:50 +1000
Subject: [PATCH] FIX: Do not process pop3 mails > 1 week old (#11740)
This adds a safe default to not process pop3 emails when the pop3 polling option is set up that are > 1 week old. This is to avoid the situation where an older mailbox is used, which causes us to go and process all emails in that mailbox, sending out error emails to the senders of emails which cannot be parsed successfully.
---
app/jobs/scheduled/poll_mailbox.rb | 17 ++++++++++---
spec/jobs/poll_mailbox_spec.rb | 39 ++++++++++++++++++++++++++----
2 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb
index a016ec2036c..4a9ee8bf100 100644
--- a/app/jobs/scheduled/poll_mailbox.rb
+++ b/app/jobs/scheduled/poll_mailbox.rb
@@ -19,8 +19,8 @@ module Jobs
SiteSetting.pop3_polling_enabled?
end
- def process_popmail(popmail)
- Email::Processor.process!(popmail.pop)
+ def process_popmail(mail_string)
+ Email::Processor.process!(mail_string)
end
POLL_MAILBOX_TIMEOUT_ERROR_KEY = "poll_mailbox_timeout_error_key"
@@ -38,7 +38,9 @@ module Jobs
pop3.start(SiteSetting.pop3_polling_username, SiteSetting.pop3_polling_password) do |pop|
pop.each_mail do |p|
- process_popmail(p)
+ mail_string = p.pop
+ break if mail_too_old?(mail_string)
+ process_popmail(mail_string)
p.delete if SiteSetting.pop3_polling_delete_from_server?
end
end
@@ -69,6 +71,15 @@ module Jobs
Discourse.redis.zcard(POLL_MAILBOX_ERRORS_KEY).to_i
end
+ def mail_too_old?(mail_string)
+ mail = Mail.new(mail_string)
+ date_header = mail.header['Date']
+ return false if date_header.blank?
+
+ date = Time.parse(date_header.to_s)
+ date < 1.week.ago
+ end
+
def mark_as_errored!
now = Time.now.to_i
Discourse.redis.zadd(POLL_MAILBOX_ERRORS_KEY, now, now.to_s)
diff --git a/spec/jobs/poll_mailbox_spec.rb b/spec/jobs/poll_mailbox_spec.rb
index 267913034d4..87fecc9dcfd 100644
--- a/spec/jobs/poll_mailbox_spec.rb
+++ b/spec/jobs/poll_mailbox_spec.rb
@@ -72,14 +72,38 @@ describe Jobs::PollMailbox do
end
context "has emails" do
+ let(:oldmail) { file_from_fixtures("old_destination.eml", "emails").read }
+
+ # the date is dynamic here because there is a 1 week cutoff for
+ # the pop3 polling
+ let(:example_email) do
+ email = <<~EMAIL
+ Return-Path: <one@foo.com>
+ From: One <one@foo.com>
+ To: team@bar.com
+ Subject: Testing email
+ Date: #{1.day.ago.strftime("%a, %d %b %Y")} 03:12:43 +0100
+ Message-ID: <34@foo.bar.mail>
+ Mime-Version: 1.0
+ Content-Type: text/plain
+ Content-Transfer-Encoding: 7bit
+
+ This is an email example.
+ EMAIL
+ end
before do
- mail1 = Net::POPMail.new(3, nil, nil, nil)
- mail2 = Net::POPMail.new(3, nil, nil, nil)
+ mail1 = Net::POPMail.new(1, nil, nil, nil)
+ mail2 = Net::POPMail.new(2, nil, nil, nil)
mail3 = Net::POPMail.new(3, nil, nil, nil)
+ mail4 = Net::POPMail.new(4, nil, nil, nil)
Net::POP3.any_instance.stubs(:start).yields(Net::POP3.new(nil, nil))
- Net::POP3.any_instance.stubs(:mails).returns([mail1, mail2, mail3])
+ Net::POP3.any_instance.stubs(:mails).returns([mail1, mail2, mail3, mail4])
Net::POP3.any_instance.expects(:delete_all).never
- poller.stubs(:process_popmail)
+ mail1.stubs(:pop).returns(example_email)
+ mail2.stubs(:pop).returns(example_email)
+ mail3.stubs(:pop).returns(example_email)
+ mail4.stubs(:pop).returns(oldmail)
+ poller.expects(:process_popmail).times(3)
end
it "deletes emails from server when when deleting emails from server is enabled" do
@@ -93,6 +117,11 @@ describe Jobs::PollMailbox do
SiteSetting.pop3_polling_delete_from_server = false
poller.poll_pop3
end
+
+ it "does not process emails > 1 week old" do
+ SiteSetting.pop3_polling_delete_from_server = false
+ poller.poll_pop3
+ end
end
end
@@ -100,7 +129,7 @@ describe Jobs::PollMailbox 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)
+ Jobs::PollMailbox.new.process_popmail(pop_mail.pop)
end
it "does not reply to a bounced email" do