Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Public disconnect methods #86

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ jobs:
- name: Install dependencies
run: bundle install
- name: Run test
run: rake test
run: bundle exec rake test
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
38 changes: 35 additions & 3 deletions lib/net/smtp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 <tt>exception: :warn</tt> is specified, a
# warning is printed and the exception is returned. When <tt>exception:
# false</tt> 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?
Expand Down
82 changes: 82 additions & 0 deletions test/net/smtp/test_smtp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -594,6 +597,85 @@ def test_mailfrom_with_smtputf_detection
assert_equal "MAIL FROM:<rené@example.com> 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)
Expand Down