[ruby/net-ftp] Close the passive connection data socket if there is an error setting up the transfer

Previously, the connection leaked in this case.  This uses
begin/ensure and checking for an error in the ensure block.

An alternative approach would be to not even perform the
connection until after the RETR (or other) command has been
sent.  However, I'm not sure all FTP servers support that.
The current behavior is:

* Send (PASV/EPSV)
* Connect to the host/port returned in 227/229 reply
* Send (RETR/other command)

Changing it to connect after the RETR could break things.
FTP servers might expect that the client has already
connected before sending the RETR.  The alternative
approach is more likely to introduce backwards compatibility
issues, compared to the begin/ensure approach taken here.

Fixes Ruby Bug 17027

https://github.com/ruby/net-ftp/commit/6e8535f076
This commit is contained in:
Jeremy Evans 2021-02-25 11:15:48 -08:00 коммит произвёл Hiroshi SHIBATA
Родитель a86c6cb34d
Коммит 990baec411
2 изменённых файлов: 51 добавлений и 12 удалений

Просмотреть файл

@ -547,18 +547,22 @@ module Net
def transfercmd(cmd, rest_offset = nil) # :nodoc:
if @passive
host, port = makepasv
conn = open_socket(host, port)
if @resume and rest_offset
resp = sendcmd("REST " + rest_offset.to_s)
if !resp.start_with?("3")
begin
conn = open_socket(host, port)
if @resume and rest_offset
resp = sendcmd("REST " + rest_offset.to_s)
if !resp.start_with?("3")
raise FTPReplyError, resp
end
end
resp = sendcmd(cmd)
# skip 2XX for some ftp servers
resp = getresp if resp.start_with?("2")
if !resp.start_with?("1")
raise FTPReplyError, resp
end
end
resp = sendcmd(cmd)
# skip 2XX for some ftp servers
resp = getresp if resp.start_with?("2")
if !resp.start_with?("1")
raise FTPReplyError, resp
ensure
conn.close if conn && $!
end
else
sock = makeport

Просмотреть файл

@ -882,6 +882,41 @@ class FTPTest < Test::Unit::TestCase
end
end
def test_getbinaryfile_error
commands = []
binary_data = ""
server = create_ftp_server { |sock|
sock.print("220 (test_ftp).\r\n")
commands.push(sock.gets)
sock.print("331 Please specify the password.\r\n")
commands.push(sock.gets)
sock.print("230 Login successful.\r\n")
commands.push(sock.gets)
sock.print("200 Switching to Binary mode.\r\n")
line = sock.gets
commands.push(line)
sock.print("450 No Dice\r\n")
}
begin
begin
ftp = Net::FTP.new
ftp.passive = true
ftp.read_timeout *= 5 if defined?(RubyVM::MJIT) && RubyVM::MJIT.enabled? # for --jit-wait
ftp.connect(SERVER_ADDR, server.port)
ftp.login
assert_match(/\AUSER /, commands.shift)
assert_match(/\APASS /, commands.shift)
assert_equal("TYPE I\r\n", commands.shift)
assert_raise(Net::FTPTempError) {ftp.getbinaryfile("foo", nil)}
assert_match(/\A(PASV|EPSV)\r\n/, commands.shift)
ensure
ftp.close if ftp
end
ensure
server.close
end
end
def test_storbinary
commands = []
binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3
@ -1935,7 +1970,7 @@ EOF
assert_equal(nil, commands.shift)
# FIXME: The new_session_cb is known broken for clients in OpenSSL 1.1.0h.
# See https://github.com/openssl/openssl/pull/5967 for details.
if OpenSSL::OPENSSL_LIBRARY_VERSION !~ /OpenSSL 1.1.0h/
if OpenSSL::OPENSSL_LIBRARY_VERSION !~ /OpenSSL 1.1.0h|LibreSSL/
assert_equal(true, session_reused_for_data_connection)
end
ensure
@ -2019,7 +2054,7 @@ EOF
assert_equal("RETR foo\r\n", commands.shift)
assert_equal(nil, commands.shift)
# FIXME: The new_session_cb is known broken for clients in OpenSSL 1.1.0h.
if OpenSSL::OPENSSL_LIBRARY_VERSION !~ /OpenSSL 1.1.0h/
if OpenSSL::OPENSSL_LIBRARY_VERSION !~ /OpenSSL 1.1.0h|LibreSSL/
assert_equal(true, session_reused_for_data_connection)
end
ensure