summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornagachika <nagachika@ruby-lang.org>2021-06-10 16:06:01 +0900
committernagachika <nagachika@ruby-lang.org>2021-06-10 16:06:01 +0900
commit6363492817d9dc73da836da852d7cb6a4ace3a8f (patch)
treeb512ea83ce9308bd1fcfe28aaba230e2dc6915ee
parentced669aed0de19090d1ba85eb9881becb693a735 (diff)
merge revision(s) 990baec41174a0b4cf7e285cf3185b4ab444437e:
[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 --- lib/net/ftp.rb | 24 ++++++++++++++---------- test/net/ftp/test_ftp.rb | 39 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 12 deletions(-)
-rw-r--r--lib/net/ftp.rb24
-rw-r--r--test/net/ftp/test_ftp.rb39
-rw-r--r--version.h2
3 files changed, 52 insertions, 13 deletions
diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb
index 9e57cb363f..3536e01ba3 100644
--- a/lib/net/ftp.rb
+++ b/lib/net/ftp.rb
@@ -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
diff --git a/test/net/ftp/test_ftp.rb b/test/net/ftp/test_ftp.rb
index 318e4c8856..d610177769 100644
--- a/test/net/ftp/test_ftp.rb
+++ b/test/net/ftp/test_ftp.rb
@@ -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
diff --git a/version.h b/version.h
index 99224ae528..a88fe9bb27 100644
--- a/version.h
+++ b/version.h
@@ -12,7 +12,7 @@
# define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
#define RUBY_VERSION_TEENY 2
#define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
-#define RUBY_PATCHLEVEL 97
+#define RUBY_PATCHLEVEL 98
#define RUBY_RELEASE_YEAR 2021
#define RUBY_RELEASE_MONTH 6