ruby-changes:66447
From: nagachika <ko1@a...>
Date: Thu, 10 Jun 2021 16:33:29 +0900 (JST)
Subject: [ruby-changes:66447] 6363492817 (ruby_3_0): merge revision(s) 990baec41174a0b4cf7e285cf3185b4ab444437e:
https://git.ruby-lang.org/ruby.git/commit/?id=6363492817 From 6363492817d9dc73da836da852d7cb6a4ace3a8f Mon Sep 17 00:00:00 2001 From: nagachika <nagachika@r...> Date: Thu, 10 Jun 2021 16:06:01 +0900 Subject: 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(-) --- lib/net/ftp.rb | 24 ++++++++++++++---------- test/net/ftp/test_ftp.rb | 39 +++++++++++++++++++++++++++++++++++++-- version.h | 2 +- 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb index 9e57cb3..3536e01 100644 --- a/lib/net/ftp.rb +++ b/lib/net/ftp.rb @@ -547,18 +547,22 @@ module Net https://github.com/ruby/ruby/blob/trunk/lib/net/ftp.rb#L547 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 318e4c8..d610177 100644 --- a/test/net/ftp/test_ftp.rb +++ b/test/net/ftp/test_ftp.rb @@ -882,6 +882,41 @@ class FTPTest < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/net/ftp/test_ftp.rb#L882 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 https://github.com/ruby/ruby/blob/trunk/test/net/ftp/test_ftp.rb#L1970 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 https://github.com/ruby/ruby/blob/trunk/test/net/ftp/test_ftp.rb#L2054 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 99224ae..a88fe9b 100644 --- a/version.h +++ b/version.h @@ -12,7 +12,7 @@ https://github.com/ruby/ruby/blob/trunk/version.h#L12 # 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 -- cgit v1.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/