From 31ecb4fecf13efab8a3a6c0c8409daa48f928607 Mon Sep 17 00:00:00 2001 From: Gerhard Schlager Date: Tue, 12 Sep 2017 22:35:24 +0200 Subject: [PATCH] FIX: Handle incoming emails without email address in From header (#5177) --- lib/email/processor.rb | 3 ++- lib/email/receiver.rb | 27 ++++++++++++++----- spec/components/email/receiver_spec.rb | 10 ++++++- .../{invalid_from.eml => invalid_from_1.eml} | 0 spec/fixtures/emails/invalid_from_2.eml | 12 +++++++++ spec/fixtures/emails/no_from.eml | 9 +++++++ 6 files changed, 53 insertions(+), 8 deletions(-) rename spec/fixtures/emails/{invalid_from.eml => invalid_from_1.eml} (100%) create mode 100644 spec/fixtures/emails/invalid_from_2.eml create mode 100644 spec/fixtures/emails/no_from.eml diff --git a/lib/email/processor.rb b/lib/email/processor.rb index a13d3814dff..959adcad17d 100644 --- a/lib/email/processor.rb +++ b/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 = {} diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 245321a4468..ed0aeeeea33 100644 --- a/lib/email/receiver.rb +++ b/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] diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 28603db1a03..4a13ac13b08 100644 --- a/spec/components/email/receiver_spec.rb +++ b/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 } diff --git a/spec/fixtures/emails/invalid_from.eml b/spec/fixtures/emails/invalid_from_1.eml similarity index 100% rename from spec/fixtures/emails/invalid_from.eml rename to spec/fixtures/emails/invalid_from_1.eml diff --git a/spec/fixtures/emails/invalid_from_2.eml b/spec/fixtures/emails/invalid_from_2.eml new file mode 100644 index 00000000000..25e4533f0c8 --- /dev/null +++ b/spec/fixtures/emails/invalid_from_2.eml @@ -0,0 +1,12 @@ +Return-Path: +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. diff --git a/spec/fixtures/emails/no_from.eml b/spec/fixtures/emails/no_from.eml new file mode 100644 index 00000000000..d3b536e4902 --- /dev/null +++ b/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.