Skip to content

Commit

Permalink
Extract public quit!/disconnect from do_finish
Browse files Browse the repository at this point in the history
This renames `#do_finish` to `#quit!`, makes it public, adds an option
to convert exceptions into warnings, and extracts the contents of the
ensure block into a new public `#disconnect` method.  The motivation for
making these public is given in the rdoc.

As documented in the rdoc, `#quit!`:
> 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, this 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>warning:
> 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.

As documented in the rdoc, `#disconnect`:
> 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.
  • Loading branch information
nevans committed May 8, 2024
1 parent 5b60464 commit 6b2de76
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 3 deletions.
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>warning:
# 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
66 changes: 66 additions & 0 deletions test/net/smtp/test_smtp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,72 @@ 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 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_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

0 comments on commit 6b2de76

Please sign in to comment.