From ee9d621d9cf9da2ccc1e70b107ab12364b426472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 7 Mar 2017 23:37:21 +0100 Subject: [PATCH] FIX: surround the FROM alias with " in order to support the @ character --- lib/email.rb | 9 ++++--- lib/email/message_builder.rb | 5 ++-- spec/components/email/message_builder_spec.rb | 24 +++++++++---------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/email.rb b/lib/email.rb index f0fbc28e57c..f37d81aa65d 100644 --- a/lib/email.rb +++ b/lib/email.rb @@ -7,15 +7,15 @@ require_dependency 'email/styles' module Email def self.is_valid?(email) - return false unless String === email parsed = Mail::Address.new(email) - # Don't allow for a TLD by itself list (sam@localhost) # The Grammar is: (local_part "@" domain) / local_part ... need to discard latter - parsed.address == email && parsed.local != parsed.address && parsed.domain && parsed.domain.split(".").length > 1 + parsed.address == email && + parsed.local != parsed.address && + parsed&.domain.split(".").size > 1 rescue Mail::Field::ParseError false end @@ -26,8 +26,7 @@ module Email end def self.cleanup_alias(name) - # TODO: I'm sure there are more, but I can't find a list - name ? name.gsub(/[:<>,]/, '') : name + name ? name.gsub(/[:<>,"]/, '') : name end end diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index 8817cddb77d..990fb010b91 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -207,7 +207,7 @@ module Email SiteSetting.title.blank? if !@opts[:from_alias].blank? - "#{Email.cleanup_alias(@opts[:from_alias])} <#{source}>" + "\"#{Email.cleanup_alias(@opts[:from_alias])}\" <#{source}>" elsif source == SiteSetting.notification_email || source == SiteSetting.reply_by_email_address site_alias_email(source) else @@ -216,7 +216,8 @@ module Email end def site_alias_email(source) - "#{Email.cleanup_alias(SiteSetting.email_site_title.presence || SiteSetting.title)} <#{source}>" + from_alias = SiteSetting.email_site_title.presence || SiteSetting.title + "\"#{Email.cleanup_alias(from_alias)}\" <#{source}>" end end diff --git a/spec/components/email/message_builder_spec.rb b/spec/components/email/message_builder_spec.rb index 06cb06ce0dc..b206bb9ecd0 100644 --- a/spec/components/email/message_builder_spec.rb +++ b/spec/components/email/message_builder_spec.rb @@ -58,12 +58,12 @@ describe Email::MessageBuilder do end it "returns a Reply-To header with the reply key" do - expect(reply_by_email_builder.header_args['Reply-To']).to eq(SiteSetting.title + " ") + expect(reply_by_email_builder.header_args['Reply-To']).to eq("\"#{SiteSetting.title}\" ") end it "cleans up the site title" do - SiteSetting.stubs(:title).returns(">>>Obnoxious Title: Deal, With It<<<") - expect(reply_by_email_builder.header_args['Reply-To']).to eq("Obnoxious Title Deal With It ") + SiteSetting.stubs(:title).returns(">>>Obnoxious Title: Deal, \"With\" It<<<") + expect(reply_by_email_builder.header_args['Reply-To']).to eq("\"Obnoxious Title Deal With It\" ") end end @@ -98,7 +98,7 @@ describe Email::MessageBuilder do end it "returns a Reply-To header with the reply key" do - expect(reply_by_email_builder.header_args['Reply-To']).to eq("Username ") + expect(reply_by_email_builder.header_args['Reply-To']).to eq("\"Username\" ") end end @@ -243,7 +243,7 @@ describe Email::MessageBuilder do it "title setting will be added if present" do SiteSetting.title = "Dog Talk" - expect(build_args[:from]).to eq("Dog Talk <#{SiteSetting.notification_email}>") + expect(build_args[:from]).to eq("\"Dog Talk\" <#{SiteSetting.notification_email}>") end let(:finn_email) { 'finn@adventuretime.ooo' } @@ -256,7 +256,7 @@ describe Email::MessageBuilder do let(:aliased_from) { Email::MessageBuilder.new(to_address, from_alias: "Finn the Dog") } it "allows us to alias the from address" do - expect(aliased_from.build_args[:from]).to eq("Finn the Dog <#{SiteSetting.notification_email}>") + expect(aliased_from.build_args[:from]).to eq("\"Finn the Dog\" <#{SiteSetting.notification_email}>") end let(:custom_aliased_from) { Email::MessageBuilder.new(to_address, @@ -264,28 +264,28 @@ describe Email::MessageBuilder do from: finn_email) } it "allows us to alias a custom from address" do - expect(custom_aliased_from.build_args[:from]).to eq("Finn the Dog <#{finn_email}>") + expect(custom_aliased_from.build_args[:from]).to eq("\"Finn the Dog\" <#{finn_email}>") end it "email_site_title will be added if it's set" do SiteSetting.email_site_title = "The Forum" - expect(build_args[:from]).to eq("The Forum <#{SiteSetting.notification_email}>") + expect(build_args[:from]).to eq("\"The Forum\" <#{SiteSetting.notification_email}>") end it "email_site_title overrides title" do SiteSetting.title = "Dog Talk" SiteSetting.email_site_title = "The Forum" - expect(build_args[:from]).to eq("The Forum <#{SiteSetting.notification_email}>") + expect(build_args[:from]).to eq("\"The Forum\" <#{SiteSetting.notification_email}>") end it "cleans up aliases in the from_alias arg" do builder = Email::MessageBuilder.new(to_address, from_alias: "Finn: the Dog, <3", from: finn_email) - expect(builder.build_args[:from]).to eq("Finn the Dog 3 <#{finn_email}>") + expect(builder.build_args[:from]).to eq("\"Finn the Dog 3\" <#{finn_email}>") end it "cleans up the email_site_title" do - SiteSetting.stubs(:email_site_title).returns("::>>>Best Forum, EU: Award Winning<<<") - expect(build_args[:from]).to eq("Best Forum EU Award Winning <#{SiteSetting.notification_email}>") + SiteSetting.stubs(:email_site_title).returns("::>>>Best \"Forum\", EU: Award Winning<<<") + expect(build_args[:from]).to eq("\"Best Forum EU Award Winning\" <#{SiteSetting.notification_email}>") end end