diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index 581c7c517..7e42fce10 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -16,6 +16,7 @@ Performance: * Close pull request 488 - Speed up field construction & comparison (bpot) Bugs: +* Fix sendmail delivery to addresses with a leading hyphen (lifo, jeremy) * Correctly format mbox From headers per RFC4155 (bpot, jeremy) * Fix bogus '=' at the end of some quoted-printable messages (jeremy) * Shouldn't be fooled into encoding strings on 1.8 by unrelated Encoding constant (emiellohr, jeremy) diff --git a/lib/mail.rb b/lib/mail.rb index 1261ce5b2..7829b22b7 100644 --- a/lib/mail.rb +++ b/lib/mail.rb @@ -29,7 +29,6 @@ module Mail # :doc: require 'mail/core_extensions/nil' require 'mail/core_extensions/object' require 'mail/core_extensions/string' - require 'mail/core_extensions/shell_escape' require 'mail/core_extensions/smtp' if RUBY_VERSION < '1.9.3' require 'mail/indifferent_hash' diff --git a/lib/mail/check_delivery_params.rb b/lib/mail/check_delivery_params.rb index 8b01daae8..0a287ca78 100644 --- a/lib/mail/check_delivery_params.rb +++ b/lib/mail/check_delivery_params.rb @@ -1,30 +1,22 @@ module Mail - module CheckDeliveryParams + def check_delivery_params(mail) + envelope_from = mail.return_path || mail.sender || mail.from_addrs.first + if envelope_from.blank? + raise ArgumentError.new('A sender (Return-Path, Sender or From) required to send a message') + end - def self.included(klass) - klass.class_eval do - - def check_params(mail) - envelope_from = mail.return_path || mail.sender || mail.from_addrs.first - if envelope_from.blank? - raise ArgumentError.new('A sender (Return-Path, Sender or From) required to send a message') - end - - destinations ||= mail.destinations if mail.respond_to?(:destinations) && mail.destinations - if destinations.blank? - raise ArgumentError.new('At least one recipient (To, Cc or Bcc) is required to send a message') - end - - message ||= mail.encoded if mail.respond_to?(:encoded) - if message.blank? - raise ArgumentError.new('A encoded content is required to send a message') - end - - [envelope_from, destinations, message] - end + destinations ||= mail.destinations if mail.respond_to?(:destinations) && mail.destinations + if destinations.blank? + raise ArgumentError.new('At least one recipient (To, Cc or Bcc) is required to send a message') + end + message ||= mail.encoded if mail.respond_to?(:encoded) + if message.blank? + raise ArgumentError.new('A encoded content is required to send a message') end + + [envelope_from, destinations, message] end end end diff --git a/lib/mail/core_extensions/shell_escape.rb b/lib/mail/core_extensions/shell_escape.rb deleted file mode 100644 index 5287cab42..000000000 --- a/lib/mail/core_extensions/shell_escape.rb +++ /dev/null @@ -1,56 +0,0 @@ -# encoding: utf-8 - -# The following is an adaptation of ruby 1.9.2's shellwords.rb file, -# it is modified to include '+' in the allowed list to allow for -# sendmail to accept email addresses as the sender with a + in them -# -module Mail - module ShellEscape - # Escapes a string so that it can be safely used in a Bourne shell - # command line. - # - # Note that a resulted string should be used unquoted and is not - # intended for use in double quotes nor in single quotes. - # - # open("| grep #{Shellwords.escape(pattern)} file") { |pipe| - # # ... - # } - # - # +String#shellescape+ is a shorthand for this function. - # - # open("| grep #{pattern.shellescape} file") { |pipe| - # # ... - # } - # - def escape_for_shell(str) - # An empty argument will be skipped, so return empty quotes. - return "''" if str.empty? - - str = str.dup - - # Process as a single byte sequence because not all shell - # implementations are multibyte aware. - str.gsub!(/([^A-Za-z0-9_\s\+\-.,:\/@])/n, "\\\\\\1") - - # A LF cannot be escaped with a backslash because a backslash + LF - # combo is regarded as line continuation and simply ignored. - str.gsub!(/\n/, "'\n'") - - return str - end - - module_function :escape_for_shell - end -end - -class String - # call-seq: - # str.shellescape => string - # - # Escapes +str+ so that it can be safely used in a Bourne shell - # command line. See +Shellwords::shellescape+ for details. - # - def escape_for_shell - Mail::ShellEscape.escape_for_shell(self) - end -end \ No newline at end of file diff --git a/lib/mail/network/delivery_methods/exim.rb b/lib/mail/network/delivery_methods/exim.rb index 4b0a3092d..f3b5800f9 100644 --- a/lib/mail/network/delivery_methods/exim.rb +++ b/lib/mail/network/delivery_methods/exim.rb @@ -1,5 +1,3 @@ -require 'mail/check_delivery_params' - module Mail # A delivery method implementation which sends via exim. @@ -38,15 +36,12 @@ module Mail # # mail.deliver! class Exim < Sendmail - include Mail::CheckDeliveryParams - def initialize(values) self.settings = { :location => '/usr/sbin/exim', :arguments => '-i -t' }.merge(values) end def self.call(path, arguments, destinations, mail) - check_params(mail) popen "#{path} #{arguments}" do |io| io.puts mail.encoded.to_lf io.flush diff --git a/lib/mail/network/delivery_methods/file_delivery.rb b/lib/mail/network/delivery_methods/file_delivery.rb index 253cc353b..56457193a 100644 --- a/lib/mail/network/delivery_methods/file_delivery.rb +++ b/lib/mail/network/delivery_methods/file_delivery.rb @@ -28,7 +28,7 @@ def initialize(values) attr_accessor :settings def deliver!(mail) - check_params(mail) + check_delivery_params(mail) if ::File.respond_to?(:makedirs) ::File.makedirs settings[:location] diff --git a/lib/mail/network/delivery_methods/sendmail.rb b/lib/mail/network/delivery_methods/sendmail.rb index c49db7fe9..a4090b514 100644 --- a/lib/mail/network/delivery_methods/sendmail.rb +++ b/lib/mail/network/delivery_methods/sendmail.rb @@ -47,14 +47,15 @@ def initialize(values) attr_accessor :settings def deliver!(mail) - check_params(mail) + check_delivery_params(mail) envelope_from = mail.return_path || mail.sender || mail.from_addrs.first - return_path = "-f " + '"' + envelope_from.escape_for_shell + '"' if envelope_from + return_path = "-f #{self.class.shellquote(envelope_from)}" if envelope_from arguments = [settings[:arguments], return_path].compact.join(" ") - self.class.call(settings[:location], arguments, mail.destinations.collect(&:escape_for_shell).join(" "), mail) + quoted_destinations = mail.destinations.collect { |d| self.class.shellquote(d) } + self.class.call(settings[:location], arguments, quoted_destinations.join(' '), mail) end def self.call(path, arguments, destinations, mail) @@ -73,5 +74,18 @@ def self.popen(command, &block) IO.popen command, 'w+', :err => :out, &block end end + + # The following is an adaptation of ruby 1.9.2's shellwords.rb file, + # it is modified to include '+' in the allowed list to allow for + # sendmail to accept email addresses as the sender with a + in them. + def self.shellquote(address) + # Process as a single byte sequence because not all shell + # implementations are multibyte aware. + # + # A LF cannot be escaped with a backslash because a backslash + LF + # combo is regarded as line continuation and simply ignored. Strip it. + escaped = address.gsub(/([^A-Za-z0-9_\s\+\-.,:\/@])/n, "\\\\\\1").gsub("\n", '') + %("#{escaped}") + end end end diff --git a/lib/mail/network/delivery_methods/smtp.rb b/lib/mail/network/delivery_methods/smtp.rb index 2aaf43bb4..a095385a0 100644 --- a/lib/mail/network/delivery_methods/smtp.rb +++ b/lib/mail/network/delivery_methods/smtp.rb @@ -95,7 +95,7 @@ def initialize(values) # Send the message via SMTP. # The from and to attributes are optional. If not set, they are retrieve from the Message. def deliver!(mail) - envelope_from, destinations, message = check_params(mail) + envelope_from, destinations, message = check_delivery_params(mail) smtp = Net::SMTP.new(settings[:address], settings[:port]) if settings[:tls] || settings[:ssl] diff --git a/lib/mail/network/delivery_methods/smtp_connection.rb b/lib/mail/network/delivery_methods/smtp_connection.rb index 126099809..b17e5c695 100644 --- a/lib/mail/network/delivery_methods/smtp_connection.rb +++ b/lib/mail/network/delivery_methods/smtp_connection.rb @@ -51,7 +51,7 @@ def initialize(values) # Send the message via SMTP. # The from and to attributes are optional. If not set, they are retrieve from the Message. def deliver!(mail) - envelope_from, destinations, message = check_params(mail) + envelope_from, destinations, message = check_delivery_params(mail) response = smtp.sendmail(message, envelope_from, destinations) settings[:return_response] ? response : self diff --git a/lib/mail/network/delivery_methods/test_mailer.rb b/lib/mail/network/delivery_methods/test_mailer.rb index 2ff29f91d..a6b42f4ba 100644 --- a/lib/mail/network/delivery_methods/test_mailer.rb +++ b/lib/mail/network/delivery_methods/test_mailer.rb @@ -36,7 +36,7 @@ def initialize(values) attr_accessor :settings def deliver!(mail) - check_params(mail) + check_delivery_params(mail) Mail::TestMailer.deliveries << mail end diff --git a/spec/mail/network/delivery_methods/exim_spec.rb b/spec/mail/network/delivery_methods/exim_spec.rb index 422c4b7c4..d8b9e90ae 100644 --- a/spec/mail/network/delivery_methods/exim_spec.rb +++ b/spec/mail/network/delivery_methods/exim_spec.rb @@ -29,7 +29,7 @@ Mail::Exim.should_receive(:call).with('/usr/sbin/exim', '-i -t -f "roger@test.lindsaar.net"', - 'marcel@test.lindsaar.net bob@test.lindsaar.net', + '"marcel@test.lindsaar.net" "bob@test.lindsaar.net"', mail) mail.deliver! end @@ -53,7 +53,7 @@ Mail::Exim.should_receive(:call).with('/usr/sbin/exim', '-i -t -f "return@test.lindsaar.net"', - 'to@test.lindsaar.net', + '"to@test.lindsaar.net"', mail) mail.deliver @@ -76,7 +76,7 @@ Mail::Exim.should_receive(:call).with('/usr/sbin/exim', '-i -t -f "sender@test.lindsaar.net"', - 'to@test.lindsaar.net', + '"to@test.lindsaar.net"', mail) mail.deliver @@ -97,7 +97,7 @@ Mail::Exim.should_receive(:call).with('/usr/sbin/exim', '-i -t -f "from@test.lindsaar.net"', - 'to@test.lindsaar.net', + '"to@test.lindsaar.net"', mail) mail.deliver end @@ -117,7 +117,24 @@ Mail::Exim.should_receive(:call).with('/usr/sbin/exim', '-i -t -f "\"from+suffix test\"@test.lindsaar.net"', - 'to@test.lindsaar.net', + '"to@test.lindsaar.net"', + mail) + mail.deliver + end + + it "should quote the destinations to ensure leading -hyphen doesn't confuse exim" do + Mail.defaults do + delivery_method :exim + end + + mail = Mail.new do + to '-hyphen@test.lindsaar.net' + from 'from@test.lindsaar.net' + end + + Mail::Exim.should_receive(:call).with('/usr/sbin/exim', + '-i -t -f "from@test.lindsaar.net"', + '"-hyphen@test.lindsaar.net"', mail) mail.deliver end @@ -136,7 +153,7 @@ Mail::Exim.should_receive(:call).with('/usr/sbin/exim', '-f "from@test.lindsaar.net"', - 'marcel@test.lindsaar.net bob@test.lindsaar.net', + '"marcel@test.lindsaar.net" "bob@test.lindsaar.net"', mail) mail.deliver! end @@ -154,7 +171,7 @@ Mail::Exim.should_receive(:call).with('/usr/sbin/exim', "-f \"\\\"foo\\\\\\\"\\;touch /tmp/PWNED\\;\\\\\\\"\\\"@blah.com\"", - 'marcel@test.lindsaar.net', + '"marcel@test.lindsaar.net"', mail) mail.deliver! end diff --git a/spec/mail/network/delivery_methods/sendmail_spec.rb b/spec/mail/network/delivery_methods/sendmail_spec.rb index 32c09b687..958442ffc 100644 --- a/spec/mail/network/delivery_methods/sendmail_spec.rb +++ b/spec/mail/network/delivery_methods/sendmail_spec.rb @@ -29,7 +29,7 @@ Mail::Sendmail.should_receive(:call).with('/usr/sbin/sendmail', '-i -t -f "roger@test.lindsaar.net"', - 'marcel@test.lindsaar.net bob@test.lindsaar.net', + '"marcel@test.lindsaar.net" "bob@test.lindsaar.net"', mail) mail.deliver! end @@ -45,7 +45,7 @@ subject 'invalid RFC2822' end - Mail::Sendmail.should_receive(:popen).with('/usr/sbin/sendmail -i -t -f "roger@test.lindsaar.net" marcel@test.lindsaar.net bob@test.lindsaar.net') + Mail::Sendmail.should_receive(:popen).with('/usr/sbin/sendmail -i -t -f "roger@test.lindsaar.net" "marcel@test.lindsaar.net" "bob@test.lindsaar.net"') mail.deliver! end @@ -69,7 +69,7 @@ Mail::Sendmail.should_receive(:call).with('/usr/sbin/sendmail', '-i -t -f "return@test.lindsaar.net"', - 'to@test.lindsaar.net', + '"to@test.lindsaar.net"', mail) mail.deliver @@ -92,7 +92,7 @@ Mail::Sendmail.should_receive(:call).with('/usr/sbin/sendmail', '-i -t -f "sender@test.lindsaar.net"', - 'to@test.lindsaar.net', + '"to@test.lindsaar.net"', mail) mail.deliver @@ -113,7 +113,7 @@ Mail::Sendmail.should_receive(:call).with('/usr/sbin/sendmail', '-i -t -f "from@test.lindsaar.net"', - 'to@test.lindsaar.net', + '"to@test.lindsaar.net"', mail) mail.deliver end @@ -133,7 +133,24 @@ Mail::Sendmail.should_receive(:call).with('/usr/sbin/sendmail', '-i -t -f "\"from+suffix test\"@test.lindsaar.net"', - 'to@test.lindsaar.net', + '"to@test.lindsaar.net"', + mail) + mail.deliver + end + + it "should quote the destinations to ensure leading -hyphen doesn't confuse sendmail" do + Mail.defaults do + delivery_method :sendmail + end + + mail = Mail.new do + to '-hyphen@test.lindsaar.net' + from 'from@test.lindsaar.net' + end + + Mail::Sendmail.should_receive(:call).with('/usr/sbin/sendmail', + '-i -t -f "from@test.lindsaar.net"', + '"-hyphen@test.lindsaar.net"', mail) mail.deliver end @@ -152,7 +169,7 @@ Mail::Sendmail.should_receive(:call).with('/usr/sbin/sendmail', '-f "from@test.lindsaar.net"', - 'marcel@test.lindsaar.net bob@test.lindsaar.net', + '"marcel@test.lindsaar.net" "bob@test.lindsaar.net"', mail) mail.deliver! end @@ -170,7 +187,7 @@ Mail::Sendmail.should_receive(:call).with('/usr/sbin/sendmail', "-f \"\\\"foo\\\\\\\"\\;touch /tmp/PWNED\\;\\\\\\\"\\\"@blah.com\"", - "\\\"foo\\\\\\\"\\;touch /tmp/PWNED\\;\\\\\\\"\\\"@blah.com", + %("\\\"foo\\\\\\\"\\;touch /tmp/PWNED\\;\\\\\\\"\\\"@blah.com"), mail) mail.deliver! end