diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 98f8f4c..31d302e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,4 +31,4 @@ jobs: - name: Install dependencies run: bundle install - name: Run test - run: rake test + run: bundle exec rake test diff --git a/Gemfile b/Gemfile index 3dc2883..730174f 100644 --- a/Gemfile +++ b/Gemfile @@ -6,4 +6,5 @@ group :development do gem "bundler" gem "rake" gem "test-unit" + gem "test-unit-ruby-core", git: "https://github.com/ruby/test-unit-ruby-core" end diff --git a/lib/net/smtp.rb b/lib/net/smtp.rb index cca06e6..215b41a 100644 --- a/lib/net/smtp.rb +++ b/lib/net/smtp.rb @@ -642,7 +642,7 @@ def start(*args, helo: nil, user: nil, secret: nil, password: nil, authtype: nil do_start helo, user, secret, authtype return yield(self) ensure - do_finish + quit! end else do_start helo, user, secret, authtype @@ -654,7 +654,7 @@ def start(*args, helo: nil, user: nil, secret: nil, password: nil, authtype: nil # Raises IOError if not started. def finish raise IOError, 'not yet started' unless started? - do_finish + quit! end private @@ -728,15 +728,47 @@ def do_helo(helo_domain) raise end - def do_finish + public + + # Calls #quit and ensures that #disconnect is called. Returns the result + # from #quit. Returns +nil+ when the client is already disconnected or when + # a prior error prevents the client from calling #quit. Unlike #finish, an + # exception will not be raised when the client has not started. + # + # If #quit raises a StandardError, the connection is dropped and the + # exception is re-raised. When exception: :warn is specified, a + # warning is printed and the exception is returned. When exception: + # false is specified, the warning is not printed. This is useful when + # the connection must be dropped, for example in a test suite or due to + # security concerns. + # + # Related: #finish, #quit, #disconnect + def quit!(exception: true) quit if @socket and not @socket.closed? and not @error_occurred + rescue => ex + if exception == :warn + warn "%s during %p #%s: %s" % [ex.class, self, __method__, ex] + elsif exception + raise ex + end + ex ensure + disconnect + end + + # Disconnects the socket without checking if the connection has started yet, + # and without sending a final QUIT message to the server. + # + # Generally, either #finish or #quit! should be used instead. + def disconnect @started = false @error_occurred = false @socket.close if @socket @socket = nil end + private + def requires_smtputf8(address) if address.kind_of? Address !address.address.ascii_only? diff --git a/test/net/smtp/test_smtp.rb b/test/net/smtp/test_smtp.rb index 3b9e245..8abc36a 100644 --- a/test/net/smtp/test_smtp.rb +++ b/test/net/smtp/test_smtp.rb @@ -2,6 +2,9 @@ require 'net/smtp' require 'test/unit' +require "core_assertions" + +Test::Unit::TestCase.include Test::Unit::CoreAssertions module Net class TestSMTP < Test::Unit::TestCase @@ -594,6 +597,85 @@ def test_mailfrom_with_smtputf_detection assert_equal "MAIL FROM: SMTPUTF8\r\n", server.commands.last end + def test_quit + server = FakeServer.start + smtp = Net::SMTP.start 'localhost', server.port + smtp.quit + assert_equal "QUIT\r\n", server.commands.last + + # Already finished: + assert_raise Errno, IOError, EOFError do + smtp.quit + end + + server = FakeServer.start + def server.quit + @sock.puts "400 BUSY\r\n" + end + smtp = Net::SMTP.start 'localhost', server.port + assert_raise Net::SMTPServerBusy do + smtp.quit + end + assert_equal "QUIT\r\n", server.commands.last + assert smtp.started? + end + + def test_quit! + server = FakeServer.start + smtp = Net::SMTP.start 'localhost', server.port + smtp.quit! + assert_equal "QUIT\r\n", server.commands.last + refute smtp.started? + + # Already finished: + smtp.quit! + end + + def test_quit_and_warn + server = FakeServer.start + def server.quit + @sock.puts "400 BUSY\r\n" + end + smtp = Net::SMTP.start 'localhost', server.port + assert_warn(/SMTPServerBusy during .*#quit!/i) do + smtp.quit!(exception: :warn) + end + assert_equal "QUIT\r\n", server.commands.last + refute smtp.started? + end + + def test_quit_and_reraise + server = FakeServer.start + def server.quit + @sock.puts "400 BUSY\r\n" + end + smtp = Net::SMTP.start 'localhost', server.port + assert_raise Net::SMTPServerBusy do + smtp.quit! + end + assert_equal "QUIT\r\n", server.commands.last + refute smtp.started? + end + + def test_quit_and_ignore + server = FakeServer.start + def server.quit + @sock.puts "400 BUSY\r\n" + end + smtp = Net::SMTP.start 'localhost', server.port + smtp.quit!(exception: false) + assert_equal "QUIT\r\n", server.commands.last + refute smtp.started? + end + + def test_disconnect + server = FakeServer.start + smtp = Net::SMTP.start 'localhost', server.port + smtp.disconnect + assert_equal "EHLO localhost\r\n", server.commands.last + refute smtp.started? + end + def fake_server_start(**kw) server = FakeServer.new server.start(**kw)