SMTP: extend dot-stuffing workaround to SMTPConnection delivery method

Refactor SMTP delivery method to use SMTPConnection, ensuring we don't
end up with divergent SMTP delivery behaviors.

References #683

Author: jeremy

lib/mail/network/delivery_methods/smtp.rb

@@ -76,79 +76,74 @@ module Mail
   class SMTP
     attr_accessor :settings
 
+    DEFAULTS = {
+      :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,
+      :ssl                  => nil,
+      :tls                  => nil,
+      :open_timeout         => nil,
+      :read_timeout         => nil
+    }
+
     def initialize(values)
-      self.settings = { :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,
-                        :ssl                  => nil,
-                        :tls                  => nil,
-                        :open_timeout         => nil,
-                        :read_timeout         => nil
-                      }.merge!(values)
+      self.settings = DEFAULTS.merge(values)
     end
 
-    # Send the message via SMTP.
-    # The from and to attributes are optional. If not set, they are retrieve from the Message.
     def deliver!(mail)
-      smtp_from, smtp_to, message = Mail::CheckDeliveryParams.check(mail)
-
-      smtp = Net::SMTP.new(settings[:address], settings[:port])
-      if settings[:tls] || settings[:ssl]
-        if smtp.respond_to?(:enable_tls)
-          smtp.enable_tls(ssl_context)
-        end
-      elsif settings[:enable_starttls]
-        if smtp.respond_to?(:enable_starttls)
-          smtp.enable_starttls(ssl_context)
-        end
-      elsif settings[:enable_starttls_auto]
-        if smtp.respond_to?(:enable_starttls_auto)
-          smtp.enable_starttls_auto(ssl_context)
-        end
+      response = start_smtp_session do |smtp|
+        Mail::SMTPConnection.new(:connection => smtp, :return_response => true).deliver!(mail)
       end
-      smtp.open_timeout = settings[:open_timeout] if settings[:open_timeout]
-      smtp.read_timeout = settings[:read_timeout] if settings[:read_timeout]
 
-      response = nil
-      smtp.start(settings[:domain], settings[:user_name], settings[:password], settings[:authentication]) do |smtp_obj|
-        response = smtp_obj.sendmail(dot_stuff(message), smtp_from, smtp_to)
-      end
-
-      if settings[:return_response]
-        response
-      else
-        self
-      end
+      settings[:return_response] ? response : self
     end
 
     private
+      def start_smtp_session(&block)
+        build_smtp_session.start(settings[:domain], settings[:user_name], settings[:password], settings[:authentication], &block)
+      end
 
-    # This is Net::SMTP's job, but before Ruby 2.x it does not dot-stuff
-    # an unterminated last line: https://bugs.ruby-lang.org/issues/9627
-    def dot_stuff(message)
-      message.gsub(/(\r\n\.)(\r\n|$)/, '\1.\2')
-    end
-
-    # Allow SSL context to be configured via settings, for Ruby >= 1.9
-    # Just returns openssl verify mode for Ruby 1.8.x
-    def ssl_context
-      openssl_verify_mode = settings[:openssl_verify_mode]
+      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)
+              smtp.enable_tls(ssl_context)
+            end
+          elsif settings[:enable_starttls]
+            if smtp.respond_to?(:enable_starttls)
+              smtp.enable_starttls(ssl_context)
+            end
+          elsif settings[:enable_starttls_auto]
+            if smtp.respond_to?(:enable_starttls_auto)
+              smtp.enable_starttls_auto(ssl_context)
+            end
+          end
 
-      if openssl_verify_mode.kind_of?(String)
-        openssl_verify_mode = OpenSSL::SSL.const_get("VERIFY_#{openssl_verify_mode.upcase}")
+          smtp.open_timeout = settings[:open_timeout] if settings[:open_timeout]
+          smtp.read_timeout = settings[:read_timeout] if settings[:read_timeout]
+        end
       end
 
-      context = Net::SMTP.default_ssl_context
-      context.verify_mode = openssl_verify_mode if openssl_verify_mode
-      context.ca_path = settings[:ca_path] if settings[:ca_path]
-      context.ca_file = settings[:ca_file] if settings[:ca_file]
-      context
-    end
+      # Allow SSL context to be configured via settings, for Ruby >= 1.9
+      # Just returns openssl verify mode for Ruby 1.8.x
+      def ssl_context
+        openssl_verify_mode = settings[:openssl_verify_mode]
+
+        if openssl_verify_mode.kind_of?(String)
+          openssl_verify_mode = OpenSSL::SSL.const_get("VERIFY_#{openssl_verify_mode.upcase}")
+        end
+
+        context = Net::SMTP.default_ssl_context
+        context.verify_mode = openssl_verify_mode if openssl_verify_mode
+        context.ca_path = settings[:ca_path] if settings[:ca_path]
+        context.ca_file = settings[:ca_file] if settings[:ca_file]
+        context
+      end
   end
 end

lib/mail/network/delivery_methods/smtp_connection.rb

@@ -50,9 +50,17 @@ def initialize(values)
     # The from and to attributes are optional. If not set, they are retrieve from the Message.
     def deliver!(mail)
       smtp_from, smtp_to, message = Mail::CheckDeliveryParams.check(mail)
-      response = smtp.sendmail(message, smtp_from, smtp_to)
+
+      response = smtp.sendmail(dot_stuff(message), smtp_from, smtp_to)
 
       settings[:return_response] ? response : self
     end
+
+    private
+      # This is Net::SMTP's job, but before Ruby 2.x it does not dot-stuff
+      # an unterminated last line: https://bugs.ruby-lang.org/issues/9627
+      def dot_stuff(message)
+        message.gsub(/(\r\n\.)(\r\n|$)/, '\1.\2')
+      end
   end
 end

spec/mail/network/delivery_methods/smtp_connection_spec.rb

@@ -15,6 +15,18 @@
     Mail.delivery_method.smtp.finish
   end
 
+  it "dot-stuff unterminated last line of the message" do
+    mail = Mail.deliver do
+      from 'from@example.com'
+      to 'to@example.com'
+      subject 'dot-stuff last line'
+      body "this is a test\n.\nonly a test\n."
+    end
+
+    message, from, to = MockSMTP.deliveries.first
+    expect(Mail.new(message).decoded).to eq("this is a test\n..\nonly a test\n..")
+  end
+
   it "should send an email using open SMTP connection" do
     mail = Mail.deliver do
       from    'roger@test.lindsaar.net'