ruby-changes:23699
From: naruse <ko1@a...>
Date: Mon, 21 May 2012 21:56:31 +0900 (JST)
Subject: [ruby-changes:23699] naruse:r35750 (trunk): * lib/net/ftp.rb (Net::FTP#retrbinary): close only if conn is not nil
naruse 2012-05-21 21:56:17 +0900 (Mon, 21 May 2012) New Revision: 35750 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=35750 Log: * lib/net/ftp.rb (Net::FTP#retrbinary): close only if conn is not nil because transfercmd may fail and return nil. * lib/net/ftp.rb (Net::FTP#retrlines): ditto. Modified files: trunk/ChangeLog trunk/lib/net/ftp.rb trunk/test/net/ftp/test_ftp.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 35749) +++ ChangeLog (revision 35750) @@ -1,3 +1,10 @@ +Mon May 21 19:20:25 2012 NARUSE, Yui <naruse@r...> + + * lib/net/ftp.rb (Net::FTP#retrbinary): close only if conn is not nil + because transfercmd may fail and return nil. + + * lib/net/ftp.rb (Net::FTP#retrlines): ditto. + Mon May 21 15:10:28 2012 Akinori MUSHA <knu@i...> * ext/syslog/syslog.c: Classify constants and macros into several Index: lib/net/ftp.rb =================================================================== --- lib/net/ftp.rb (revision 35749) +++ lib/net/ftp.rb (revision 35750) @@ -494,7 +494,7 @@ conn.read_timeout = 1 conn.read ensure - conn.close + conn.close if conn end voidresp end @@ -510,16 +510,19 @@ def retrlines(cmd) # :yield: line synchronize do with_binary(false) do - conn = transfercmd(cmd) - loop do - line = conn.gets - break if line == nil - yield(line.sub(/\r?\n\z/, ""), !line.match(/\n\z/).nil?) + begin + conn = transfercmd(cmd) + loop do + line = conn.gets + break if line == nil + yield(line.sub(/\r?\n\z/, ""), !line.match(/\n\z/).nil?) + end + conn.shutdown(Socket::SHUT_WR) + conn.read_timeout = 1 + conn.read + ensure + conn.close if conn end - conn.shutdown(Socket::SHUT_WR) - conn.read_timeout = 1 - conn.read - conn.close voidresp end end Index: test/net/ftp/test_ftp.rb =================================================================== --- test/net/ftp/test_ftp.rb (revision 35749) +++ test/net/ftp/test_ftp.rb (revision 35750) @@ -23,6 +23,19 @@ end end + def test_connect_fail + server = create_ftp_server { |sock| + sock.print("421 Service not available, closing control connection.\r\n") + } + begin + ftp = Net::FTP.new + assert_raise(Net::FTPTempError){ ftp.connect(SERVER_ADDR, server.port) } + ensure + ftp.close if ftp + server.close + end + end + def test_parse227 ftp = Net::FTP.new host, port = ftp.send(:parse227, "227 Entering Passive Mode (192,168,0,1,12,34)") @@ -149,6 +162,48 @@ end end + def test_login_fail1 + commands = [] + server = create_ftp_server { |sock| + sock.print("220 (test_ftp).\r\n") + commands.push(sock.gets) + sock.print("502 Command not implemented.\r\n") + } + begin + begin + ftp = Net::FTP.new + ftp.connect(SERVER_ADDR, server.port) + assert_raise(Net::FTPPermError){ ftp.login } + ensure + ftp.close if ftp + end + ensure + server.close + end + end + + def test_login_fail2 + commands = [] + 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("530 Not logged in.\r\n") + } + begin + begin + ftp = Net::FTP.new + ftp.connect(SERVER_ADDR, server.port) + assert_raise(Net::FTPPermError){ ftp.login } + ensure + ftp.close if ftp + end + ensure + server.close + end + end + # TODO: How can we test open_timeout? sleep before accept cannot delay # connections. def _test_open_timeout_exceeded @@ -369,6 +424,57 @@ end end + def test_list_fail + commands = [] + list_lines = [ + "-rw-r--r-- 1 0 0 0 Mar 30 11:22 foo.txt", + "-rw-r--r-- 1 0 0 0 Mar 30 11:22 bar.txt", + "-rw-r--r-- 1 0 0 0 Mar 30 11:22 baz.txt" + ] + 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") + commands.push(sock.gets) + sock.print("200 Switching to ASCII mode.\r\n") + line = sock.gets + commands.push(line) + port_args = line.slice(/\APORT (.*)/, 1).split(/,/) + host = port_args[0, 4].join(".") + port = port_args[4, 2].map(&:to_i).inject {|x, y| (x << 8) + y} + sock.print("200 PORT command successful.\r\n") + commands.push(sock.gets) + sock.print("553 Requested action not taken.\r\n") + commands.push(sock.gets) + sock.print("200 Switching to Binary mode.\r\n") + } + begin + begin + ftp = Net::FTP.new + ftp.read_timeout = 0.2 + 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::FTPPermError){ ftp.list } + assert_equal("TYPE A\r\n", commands.shift) + assert_match(/\APORT /, commands.shift) + assert_equal("LIST\r\n", commands.shift) + assert_equal("TYPE I\r\n", commands.shift) + assert_equal(nil, commands.shift) + ensure + ftp.close if ftp + end + ensure + server.close + end + end + def test_retrbinary_read_timeout_exceeded commands = [] binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3 @@ -479,6 +585,47 @@ end end + def test_retrbinary_fail + commands = [] + binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3 + 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) + port_args = line.slice(/\APORT (.*)/, 1).split(/,/) + host = port_args[0, 4].join(".") + port = port_args[4, 2].map(&:to_i).inject {|x, y| (x << 8) + y} + sock.print("200 PORT command successful.\r\n") + commands.push(sock.gets) + sock.print("550 Requested action not taken.\r\n") + } + begin + begin + ftp = Net::FTP.new + ftp.read_timeout = 0.2 + 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::FTPPermError){ ftp.retrbinary("RETR foo", 1024) } + assert_match(/\APORT /, commands.shift) + assert_equal("RETR foo\r\n", commands.shift) + assert_equal(nil, 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 @@ -526,6 +673,48 @@ end end + def test_storbinary_fail + commands = [] + binary_data = (0..0xff).map {|i| i.chr}.join * 4 * 3 + stored_data = nil + 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) + port_args = line.slice(/\APORT (.*)/, 1).split(/,/) + host = port_args[0, 4].join(".") + port = port_args[4, 2].map(&:to_i).inject {|x, y| (x << 8) + y} + sock.print("200 PORT command successful.\r\n") + commands.push(sock.gets) + sock.print("452 Requested file action aborted.\r\n") + } + begin + begin + ftp = Net::FTP.new + ftp.read_timeout = 0.2 + 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.storbinary("STOR foo", StringIO.new(binary_data), 1024) } + assert_match(/\APORT /, commands.shift) + assert_equal("STOR foo\r\n", commands.shift) + assert_equal(nil, commands.shift) + ensure + ftp.close if ftp + end + ensure + server.close + end + end + def test_abort commands = [] server = create_ftp_server { |sock| -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/