Permalink
Browse files

Turn on STARTTLS if it is available in Net::SMTP (added in Ruby 1.8.7…

…) and the SMTP server supports it [#1336 state:committed]

Signed-off-by: David Heinemeier Hansson <david@loudthinking.com>
  • Loading branch information...
granth authored and dhh committed Nov 6, 2008
1 parent 6406a87 commit 732c724df61bc8b780dc42817625b25a321908e4
@@ -1,3 +1,8 @@
*2.2.1 [RC2 or 2.2 final]*

* Turn on STARTTLS if it is available in Net::SMTP (added in Ruby 1.8.7) and the SMTP server supports it (This is required for Gmail's SMTP server) #1336 [Grant Hollingworth]


*2.2.0 [RC1] (October 24th, 2008)*

* Add layout functionality to mailers [Pratik]
@@ -663,8 +663,10 @@ def perform_delivery_smtp(mail)
mail.ready_to_send
sender = mail['return-path'] || mail.from

Net::SMTP.start(smtp_settings[:address], smtp_settings[:port], smtp_settings[:domain],
smtp_settings[:user_name], smtp_settings[:password], smtp_settings[:authentication]) do |smtp|
smtp = Net::SMTP.new(smtp_settings[:address], smtp_settings[:port])
smtp.enable_starttls_auto if smtp.respond_to?(:enable_starttls_auto)
smtp.start(smtp_settings[:domain], smtp_settings[:user_name], smtp_settings[:password],
smtp_settings[:authentication]) do |smtp|
smtp.sendmail(mail.encoded, sender, destinations)
end
end
@@ -24,11 +24,15 @@ def initialize
def sendmail(mail, from, to)
@@deliveries << [mail, from, to]
end

def start(*args)
yield self
end
end

class Net::SMTP
def self.start(*args)
yield MockSMTP.new
def self.new(*args)
MockSMTP.new
end
end

@@ -938,6 +938,20 @@ def test_body_is_stored_as_an_ivar
mail = TestMailer.create_body_ivar(@recipient)
assert_equal "body: foo\nbar: baz", mail.body
end

def test_starttls_is_enabled_if_supported
MockSMTP.any_instance.expects(:respond_to?).with(:enable_starttls_auto).returns(true)
MockSMTP.any_instance.expects(:enable_starttls_auto)
ActionMailer::Base.delivery_method = :smtp
TestMailer.deliver_signed_up(@recipient)
end

def test_starttls_is_disabled_if_not_supported
MockSMTP.any_instance.expects(:respond_to?).with(:enable_starttls_auto).returns(false)
MockSMTP.any_instance.expects(:enable_starttls_auto).never
ActionMailer::Base.delivery_method = :smtp
TestMailer.deliver_signed_up(@recipient)
end
end

end # uses_mocha

14 comments on commit 732c724

@juliamae

This comment has been minimized.

Copy link

juliamae replied Nov 12, 2008

NICE

@mdemare

This comment has been minimized.

Copy link

mdemare replied Nov 23, 2008

I don’t think this should be turned on if the server doesn’t have a certificate. Not all servers do, and it broke my ExceptionNotification, so I realized it two days late.

@davidw

This comment has been minimized.

Copy link
Contributor

davidw replied Dec 8, 2008

This is causing problems for me as well:

OpenSSL::SSL::SSLError (hostname was not match with the server certificate)

@dhh

This comment has been minimized.

Copy link
Member

dhh replied Dec 9, 2008

Sounds reasonable that we should have an option or at least a fallback for this. Anyone wants to give it a stab?

@darragh

This comment has been minimized.

Copy link
Contributor

darragh replied Dec 9, 2008

While the issue remains – perhaps it’s helpful to point out a quick workaround if you’re using postfix.

Disable tls by setting
smtpd_use_tls=no

if you are determined to use tls then really you should set up your certs/keys and enforce it with
smtpd_tls_auth_only=yes

more details here:
http://postfix.state-of-mind.de/patrick.koetter/smtpauth/postfix_tls_support.html
alternatively -

@electic

This comment has been minimized.

Copy link

electic replied Dec 28, 2008

This is likely a architecture oversight. I upgraded my system and it died cause the hosts were mismatched. This is common on hosting environments and there should be at the least, a option in the settings that either ignores it or at least gives the option not to use ssl.

@pboling

This comment has been minimized.

Copy link
Contributor

pboling replied Jan 10, 2009

This is causing a lot of problems for my Capistrano Mailer plugin when used with Rails 2.2.2. So essentially at this point, it’s either dig into postfix, or you’re SOL?

@NZKoz

This comment has been minimized.

Copy link
Member

NZKoz replied Jan 11, 2009

@pboling: We’re happy to take a patch to make this optional, roll it into 2-2-stable and have it fixed in 2.2.3.

Want to take a crack at it?

@josevalim

This comment has been minimized.

Copy link
Contributor

josevalim replied Jan 11, 2009

I took a crack at it. :)

Lighthouse ticket and patch here: http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1731-make-enable_starttls_auto-opt-in-in-actionmailer

@bansalakhil

This comment has been minimized.

Copy link

bansalakhil replied Jan 28, 2009

Thanks, darragh.

I am able to make it working by smtpd_use_tls=no

@bradgessler

This comment has been minimized.

Copy link

bradgessler replied Feb 2, 2009

This is killing a lot of rails apps that need to talk to broken SMTP servers. josevalim is on the right track with the patch at http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1731-make-enable_starttls_auto-opt-in-in-actionmailer

@josevalim

This comment has been minimized.

Copy link
Contributor

josevalim replied Feb 2, 2009

Yeah, and I really think it should be false by default! As soon people moves to Rails 2.3 and see that their mailers are not working properly, IRC and mailing lists will be very busy. :)

@coderberry

This comment has been minimized.

Copy link

coderberry replied Feb 3, 2009

This won’t work with 1.8.6 correct? Is this causing woes to anyone else using non-ssl AuthSMTP?

@KernComm

This comment has been minimized.

Copy link

KernComm replied Mar 11, 2010

Guys,

Use action_mailer_optional_tls ( http://github.com/collectiveidea/action_mailer_optional_tls ) plugin, it made my life more easier :) Thank you Everybody :)

Cheers,
Rajesh

Please sign in to comment.