Skip to content

Commit 87aca64

Browse files
committed
Extract public quit!/disconnect from do_finish
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>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. 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. fix docs fix doc
1 parent 5b60464 commit 87aca64

2 files changed

Lines changed: 101 additions & 3 deletions

File tree

lib/net/smtp.rb

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,7 @@ def start(*args, helo: nil, user: nil, secret: nil, password: nil, authtype: nil
642642
do_start helo, user, secret, authtype
643643
return yield(self)
644644
ensure
645-
do_finish
645+
quit!
646646
end
647647
else
648648
do_start helo, user, secret, authtype
@@ -654,7 +654,7 @@ def start(*args, helo: nil, user: nil, secret: nil, password: nil, authtype: nil
654654
# Raises IOError if not started.
655655
def finish
656656
raise IOError, 'not yet started' unless started?
657-
do_finish
657+
quit!
658658
end
659659

660660
private
@@ -728,15 +728,47 @@ def do_helo(helo_domain)
728728
raise
729729
end
730730

731-
def do_finish
731+
public
732+
733+
# Calls #quit and ensures that #disconnect is called. Returns the result
734+
# from #quit. Returns +nil+ when the client is already disconnected or when
735+
# a prior error prevents the client from calling #quit. Unlike #finish, an
736+
# exception will not be raised when the client has not started.
737+
#
738+
# If #quit raises a StandardError, the connection is dropped and the
739+
# exception is re-raised. When <tt>exception: :warn</tt> is specified, a
740+
# warning is printed and the exception is returned. When <tt>exception:
741+
# false</tt> is specified, the warning is not printed. This is useful when
742+
# the connection must be dropped, for example in a test suite or due to
743+
# security concerns.
744+
#
745+
# Related: #finish, #quit, #disconnect
746+
def quit!(exception: true)
732747
quit if @socket and not @socket.closed? and not @error_occurred
748+
rescue => ex
749+
if exception == :warn
750+
warn "%s during %p #%s: %s" % [ex.class, self, __method__, ex]
751+
elsif exception
752+
raise ex
753+
end
754+
ex
733755
ensure
756+
disconnect
757+
end
758+
759+
# Disconnects the socket without checking if the connection has started yet,
760+
# and without sending a final QUIT message to the server.
761+
#
762+
# Generally, either #finish or #quit! should be used instead.
763+
def disconnect
734764
@started = false
735765
@error_occurred = false
736766
@socket.close if @socket
737767
@socket = nil
738768
end
739769

770+
private
771+
740772
def requires_smtputf8(address)
741773
if address.kind_of? Address
742774
!address.address.ascii_only?

test/net/smtp/test_smtp.rb

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,72 @@ def test_mailfrom_with_smtputf_detection
594594
assert_equal "MAIL FROM:<rené@example.com> SMTPUTF8\r\n", server.commands.last
595595
end
596596

597+
def test_quit
598+
server = FakeServer.start
599+
smtp = Net::SMTP.start 'localhost', server.port
600+
smtp.quit
601+
assert_equal "QUIT\r\n", server.commands.last
602+
603+
# Already finished:
604+
assert_raise Errno, IOError, EOFError do
605+
smtp.quit
606+
end
607+
608+
server = FakeServer.start
609+
def server.quit
610+
@sock.puts "400 BUSY\r\n"
611+
end
612+
smtp = Net::SMTP.start 'localhost', server.port
613+
assert_raise Net::SMTPServerBusy do
614+
smtp.quit
615+
end
616+
assert_equal "QUIT\r\n", server.commands.last
617+
assert smtp.started?
618+
end
619+
620+
def test_quit!
621+
server = FakeServer.start
622+
smtp = Net::SMTP.start 'localhost', server.port
623+
smtp.quit!
624+
assert_equal "QUIT\r\n", server.commands.last
625+
refute smtp.started?
626+
627+
# Already finished:
628+
smtp.quit!
629+
end
630+
631+
def test_quit_and_reraise
632+
server = FakeServer.start
633+
def server.quit
634+
@sock.puts "400 BUSY\r\n"
635+
end
636+
smtp = Net::SMTP.start 'localhost', server.port
637+
assert_raise Net::SMTPServerBusy do
638+
smtp.quit!
639+
end
640+
assert_equal "QUIT\r\n", server.commands.last
641+
refute smtp.started?
642+
end
643+
644+
def test_quit_and_ignore
645+
server = FakeServer.start
646+
def server.quit
647+
@sock.puts "400 BUSY\r\n"
648+
end
649+
smtp = Net::SMTP.start 'localhost', server.port
650+
smtp.quit!(exception: false)
651+
assert_equal "QUIT\r\n", server.commands.last
652+
refute smtp.started?
653+
end
654+
655+
def test_disconnect
656+
server = FakeServer.start
657+
smtp = Net::SMTP.start 'localhost', server.port
658+
smtp.disconnect
659+
assert_equal "EHLO localhost\r\n", server.commands.last
660+
refute smtp.started?
661+
end
662+
597663
def fake_server_start(**kw)
598664
server = FakeServer.new
599665
server.start(**kw)

0 commit comments

Comments
 (0)