From 4fb7d045a04efb7ea135fc20c07397ad33272f3e Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 20 Sep 2021 16:26:18 +1000 Subject: [PATCH] FIX: Handle forwarded email quotes around Reply-To display name (#14384) The display name can have quotes around it, which does not work with our current comparison of a from field (in this case Reply-To) and another header (X-Original-From), because we are not comparing the two values in the same way. This causes an issue where the commit here: b88d8c8 will not work properly; the forwarded email gets the From address instead of the Reply-To address as intended. --- lib/email/receiver.rb | 5 ++++- spec/components/email/receiver_spec.rb | 18 ++++++++++++++---- ...o_different_to_from_quoted_display_name.eml | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 spec/fixtures/emails/reply_to_different_to_from_quoted_display_name.eml diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index 2b2cd363950..1f9e90b32a4 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -637,7 +637,10 @@ module Email comparison_failed = false comparison_headers.each do |comparison_header| - comparison_failed = true if address_field.to_s != mail_object[comparison_header].to_s + if mail_object[comparison_header].to_s != "#{from_display_name} <#{from_address}>" + comparison_failed = true + break + end end next if comparison_failed diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 3f6e103adc2..7c61a0f62f5 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -870,9 +870,9 @@ describe Email::Receiver do end describe "reply-to header" do - it "handles emails where there is a reply-to address, using that instead of the from address" do + it "handles emails where there is a Reply-To address, using that instead of the from address, if X-Original-From is present" do SiteSetting.block_auto_generated_emails = false - expect { process(:reply_to_different_to_from) }.to change(Topic, :count) + expect { process(:reply_to_different_to_from) }.to change(Topic, :count).by(1) user = User.last incoming = IncomingEmail.find_by(message_id: "3848c3m98r439c348mc349@test.mailinglist.com") topic = incoming.topic @@ -880,9 +880,19 @@ describe Email::Receiver do expect(user.email).to eq("arthurmorgan@reddeadtest.com") end + it "allows for quotes around the display name of the Reply-To address" do + SiteSetting.block_auto_generated_emails = false + expect { process(:reply_to_different_to_from_quoted_display_name) }.to change(Topic, :count).by(1) + user = User.last + incoming = IncomingEmail.find_by(message_id: "3848c3m98r439c348mc349@test.mailinglist.com") + topic = incoming.topic + expect(incoming.from_address).to eq("johnmarston@reddeadtest.com") + expect(user.email).to eq("johnmarston@reddeadtest.com") + end + it "does not use the reply-to address if an X-Original-From header is not present" do SiteSetting.block_auto_generated_emails = false - expect { process(:reply_to_different_to_from_no_x_original) }.to change(Topic, :count) + expect { process(:reply_to_different_to_from_no_x_original) }.to change(Topic, :count).by(1) user = User.last incoming = IncomingEmail.find_by(message_id: "3848c3m98r439c348mc349@test.mailinglist.com") topic = incoming.topic @@ -892,7 +902,7 @@ describe Email::Receiver do it "does not use the reply-to address if the X-Original-From header is different from the reply-to address" do SiteSetting.block_auto_generated_emails = false - expect { process(:reply_to_different_to_from_x_original_different) }.to change(Topic, :count) + expect { process(:reply_to_different_to_from_x_original_different) }.to change(Topic, :count).by(1) user = User.last incoming = IncomingEmail.find_by(message_id: "3848c3m98r439c348mc349@test.mailinglist.com") topic = incoming.topic diff --git a/spec/fixtures/emails/reply_to_different_to_from_quoted_display_name.eml b/spec/fixtures/emails/reply_to_different_to_from_quoted_display_name.eml new file mode 100644 index 00000000000..f74499707c5 --- /dev/null +++ b/spec/fixtures/emails/reply_to_different_to_from_quoted_display_name.eml @@ -0,0 +1,16 @@ +From: 'Marston, John' via Western Support +Reply-To: =?iso-8859-1?Q?Marston=2C_John?= +To: team@bar.com +Subject: I need some support +Date: Fri, 15 Jan 2016 00:12:43 +0100 +Message-ID: <3848c3m98r439c348mc349@test.mailinglist.com> +Mime-Version: 1.0 +Content-Type: text/plain +Content-Transfer-Encoding: 7bit +X-Original-From: Marston, John +Precedence: list +Mailing-list: list westernsupport@test.mailinglist.com; contact + westernsupport+owners@test.mailinglist.com +List-ID: + +Ain't no trouble.