From 4a3cb4a0005af4cd7bc36d3abbcb490d33b2fba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 14 Mar 2016 18:18:58 +0100 Subject: [PATCH] FIX: use MD5 of the email_string when there's no 'Message-Id' --- app/jobs/scheduled/poll_mailbox.rb | 1 - config/locales/server.en.yml | 7 ------- lib/email/receiver.rb | 6 +++--- spec/components/email/receiver_spec.rb | 14 ++++++-------- spec/fixtures/emails/missing_message_id.eml | 5 ++++- 5 files changed, 13 insertions(+), 20 deletions(-) diff --git a/app/jobs/scheduled/poll_mailbox.rb b/app/jobs/scheduled/poll_mailbox.rb index 26b8453eb..28186c01d 100644 --- a/app/jobs/scheduled/poll_mailbox.rb +++ b/app/jobs/scheduled/poll_mailbox.rb @@ -40,7 +40,6 @@ module Jobs message_template = case e when Email::Receiver::EmptyEmailError then :email_reject_empty when Email::Receiver::NoBodyDetectedError then :email_reject_empty - when Email::Receiver::NoMessageIdError then :email_reject_no_message_id when Email::Receiver::AutoGeneratedEmailError then :email_reject_auto_generated when Email::Receiver::InactiveUserError then :email_reject_inactive_user when Email::Receiver::BlockedUserError then :email_reject_blocked_user diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 7831d5e74..0b2c6ae12 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1837,13 +1837,6 @@ en: Your reply was sent from a different email address than the one we expected, so we're not sure if this is the same person. Try sending from another email address, or contact a staff member. - email_reject_no_message_id: - subject_template: "[%{site_name}] Email issue -- No Message Id" - text_body_template: | - We're sorry, but your email message to %{destination} (titled %{former_title}) didn't work. - - We couldn't find a `Message-Id` header in the email. Try sending from a different email address, or contact a staff member. - email_reject_no_account: subject_template: "[%{site_name}] Email issue -- Unknown Account" text_body_template: | diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 9906e7e49..8372763a8 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -1,3 +1,4 @@ +require "digest" require_dependency "new_post_manager" require_dependency "post_action_creator" require_dependency "email/html_cleaner" @@ -9,7 +10,6 @@ module Email class ProcessingError < StandardError; end class EmptyEmailError < ProcessingError; end - class NoMessageIdError < ProcessingError; end class AutoGeneratedEmailError < ProcessingError; end class NoBodyDetectedError < ProcessingError; end class InactiveUserError < ProcessingError; end @@ -29,7 +29,7 @@ module Email raise EmptyEmailError if mail_string.blank? @raw_email = mail_string @mail = Mail.new(@raw_email) - raise NoMessageIdError if @mail.message_id.blank? + @message_id = @mail.message_id.presence || Digest::MD5.hexdigest(mail_string) end def process! @@ -42,7 +42,7 @@ module Email end def find_or_create_incoming_email - IncomingEmail.find_or_create_by(message_id: @mail.message_id) do |ie| + IncomingEmail.find_or_create_by(message_id: @message_id) do |ie| ie.raw = @raw_email ie.subject = subject ie.from_address = @from_email diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 9225e8277..80413c3f5 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -21,14 +21,6 @@ describe Email::Receiver do expect { Email::Receiver.new("") }.to raise_error(Email::Receiver::EmptyEmailError) end - it "raises an NoMessageIdError when 'mail_string' is not an email" do - expect { Email::Receiver.new("wat") }.to raise_error(Email::Receiver::NoMessageIdError) - end - - it "raises an NoMessageIdError when 'mail_string' is missing the message_id" do - expect { Email::Receiver.new(email(:missing_message_id)) }.to raise_error(Email::Receiver::NoMessageIdError) - end - it "raises an AutoGeneratedEmailError when the mail is auto generated" do expect { process(:auto_generated_precedence) }.to raise_error(Email::Receiver::AutoGeneratedEmailError) expect { process(:auto_generated_header) }.to raise_error(Email::Receiver::AutoGeneratedEmailError) @@ -65,6 +57,12 @@ describe Email::Receiver do let(:post) { create_post(topic: topic, user: user) } let!(:email_log) { Fabricate(:email_log, reply_key: reply_key, user: user, topic: topic, post: post) } + it "uses MD5 of 'mail_string' there is no message_id" do + mail_string = email(:missing_message_id) + expect { Email::Receiver.new(mail_string).process! }.to change { IncomingEmail.count } + expect(IncomingEmail.last.message_id).to eq(Digest::MD5.hexdigest(mail_string)) + end + it "raises a ReplyUserNotMatchingError when the email address isn't matching the one we sent the notification to" do expect { process(:reply_user_not_matching) }.to raise_error(Email::Receiver::ReplyUserNotMatchingError) end diff --git a/spec/fixtures/emails/missing_message_id.eml b/spec/fixtures/emails/missing_message_id.eml index 03405bed3..0fbad2fad 100644 --- a/spec/fixtures/emails/missing_message_id.eml +++ b/spec/fixtures/emails/missing_message_id.eml @@ -1,5 +1,8 @@ -From: Foo Bar +From: Foo Bar +To: reply+4f97315cc828096c9cb34c6f1a0d6fe8@bar.com Date: Fri, 15 Jan 2016 00:12:43 +0100 Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit + +Some body