Merge pull request #1480 from ahorek/disabling-starttls-auto

SMTP: allow disabling starttls_auto since it's now true by default in Ruby 3

CHANGELOG.rdoc

@@ -20,6 +20,7 @@ Compatibility:
 * Multipart Content-Type no longer includes a needless charset param. (kirikak2)
 * Replies prefix subject with "Re: " instead of "RE: " per 5322 3.6.5. (mashedcode)
 * Gracefully handle multiple, possibly-invalid headers for what should be singular fields. (rosa)
+* SMTP delivery with enable_tls/starttls/starttls_auto: false now disables these options, since starttls is now :auto by default in upstream net-smtp. (jeremy)
 
 Features:
 * Message#inspect_structure and PartsList#inspect_structure pretty-print the hierarchy of message parts. (TylerRick)

lib/mail/network/delivery_methods/smtp.rb

@@ -111,17 +111,33 @@ def start_smtp_session(&block)
 
       def build_smtp_session
         Net::SMTP.new(settings[:address], settings[:port]).tap do |smtp|
-          if settings[:tls] || settings[:ssl]
-            if smtp.respond_to?(:enable_tls)
+          tls = settings[:tls] || settings[:ssl]
+          if !tls.nil?
+            case tls
+            when true
               smtp.enable_tls(ssl_context)
+            when false
+              smtp.disable_tls
+            else
+              raise ArgumentError, "Unrecognized :tls value #{settings[:tls].inspect}; expected true, false, or nil"
             end
-          elsif settings[:enable_starttls]
-            if smtp.respond_to?(:enable_starttls)
+          elsif settings.include?(:enable_starttls) && !settings[:enable_starttls].nil?
+            case settings[:enable_starttls]
+            when true
               smtp.enable_starttls(ssl_context)
+            when false
+              smtp.disable_starttls
+            else
+              raise ArgumentError, "Unrecognized :enable_starttls value #{settings[:enable_starttls].inspect}; expected true, false, or nil"
             end
-          elsif settings[:enable_starttls_auto]
-            if smtp.respond_to?(:enable_starttls_auto)
+          elsif settings.include?(:enable_starttls_auto) && !settings[:enable_starttls_auto].nil?
+            case settings[:enable_starttls_auto]
+            when true
               smtp.enable_starttls_auto(ssl_context)
+            when false
+              smtp.disable_starttls_auto
+            else
+              raise ArgumentError, "Unrecognized :enable_starttls_auto value #{settings[:enable_starttls_auto].inspect}; expected true, false, or nil"
             end
           end
 

spec/mail/network/delivery_methods/smtp_spec.rb

@@ -5,24 +5,10 @@
 describe "SMTP Delivery Method" do
 
   before(:each) do
+    MockSMTP.reset
+
     # Reset all defaults back to original state
-    Mail.defaults do
-      delivery_method :smtp, { :address              => "localhost",
-                               :port                 => 25,
-                               :domain               => 'localhost.localdomain',
-                               :user_name            => nil,
-                               :password             => nil,
-                               :authentication       => nil,
-                               :enable_starttls      => nil,
-                               :enable_starttls_auto => true,
-                               :openssl_verify_mode  => nil,
-                               :tls                  => nil,
-                               :ssl                  => nil,
-                               :open_timeout         => nil,
-                               :read_timeout         => nil
-                                }
-    end
-    MockSMTP.clear_deliveries
+    Mail.defaults { delivery_method :smtp, {} }
   end
 
   describe "general usage" do
@@ -207,7 +193,7 @@ def redefine_verify_none(new_value)
 
       message.deliver!
 
-      expect(MockSMTP.security).to eq :enable_starttls_auto
+      expect(MockSMTP.starttls).to eq :auto
     end
 
     it 'should allow forcing STARTTLS' do
@@ -228,7 +214,28 @@ def redefine_verify_none(new_value)
 
       message.deliver!
 
-      expect(MockSMTP.security).to eq :enable_starttls
+      expect(MockSMTP.starttls).to eq :always
+    end
+
+    it 'should allow disabling automatic STARTTLS' do
+      message = Mail.new do
+        from 'mikel@test.lindsaar.net'
+        to 'ada@test.lindsaar.net'
+        subject 'Re: No way!'
+        body 'Yeah sure'
+        delivery_method :smtp, { :address         => "localhost",
+                                 :port            => 25,
+                                 :domain          => 'localhost.localdomain',
+                                 :user_name       => nil,
+                                 :password        => nil,
+                                 :authentication  => nil,
+                                 :enable_starttls => false }
+
+      end
+
+      message.deliver!
+
+      expect(MockSMTP.starttls).to eq false
     end
   end
 

spec/mail/network_spec.rb

@@ -180,7 +180,7 @@ class MyRetriever; def initialize(settings); end; end
 
     before(:each) do
       # Set the delivery method to test as the default
-      MockSMTP.clear_deliveries
+      MockSMTP.reset
     end
 
     it "should deliver a mail message" do

spec/spec_helper.rb

@@ -70,21 +70,44 @@ def strip_heredoc(string)
   string.gsub(/^[ \t]{#{indent}}/, '')
 end
 
+class Net::SMTP
+  class << self
+    alias unstubbed_new new
+  end
+
+  def self.new(*args)
+    MockSMTP.new
+  end
+end
+
 # Original mockup from ActionMailer
 class MockSMTP
   attr_accessor :open_timeout, :read_timeout
 
+  def self.reset
+    test = Net::SMTP.unstubbed_new('example.com')
+    @@tls = test.tls?
+    @@starttls = test.starttls?
+
+    @@deliveries = []
+  end
+
+  reset
+
   def self.deliveries
     @@deliveries
   end
 
-  def self.security
-    @@security
+  def self.tls
+    @@tls
+  end
+
+  def self.starttls
+    @@starttls
   end
 
   def initialize
-    @@deliveries = []
-    @@security = nil
+    self.class.reset
   end
 
   def sendmail(mail, from, to)
@@ -104,37 +127,31 @@ def finish
     return true
   end
 
-  def self.clear_deliveries
-    @@deliveries = []
-  end
-
-  def self.clear_security
-    @@security = nil
-  end
-
   def enable_tls(context)
-    raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @@security && @@security != :enable_tls
-    @@security = :enable_tls
+    raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @@starttls == :always
+    @@tls = true
     context
   end
 
+  def disable_tls
+    @@tls = false
+  end
+
   def enable_starttls(context = nil)
-    raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @@security == :enable_tls
-    @@security = :enable_starttls
+    raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @@tls
+    @@starttls = :always
     context
   end
 
   def enable_starttls_auto(context)
-    raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @@security == :enable_tls
-    @@security = :enable_starttls_auto
+    raise ArgumentError, "SMTPS and STARTTLS is exclusive" if @@tls
+    @@starttls = :auto
     context
   end
-end
-
-class Net::SMTP
-  def self.new(*args)
-    MockSMTP.new
+  def disable_starttls
+    @@starttls = false
   end
+
 end
 
 class MockPopMail