Feature #11454: FTP client misbehaves in the block passed to FTP#list when using passive mode

• Status: Closed
• Assignee: shugo (Shugo Maeda)
• Target version: -

[srikps (Srikanth Shreenivas)]

If a block is passed to "FTP#list" method, and if we try to download a binary file using "getbinaryfile" inside the block, and if the "passive" flag is set to "true", then the FTP client seems to get confused in reading the server responses and reports an error.

Example code:

require 'net/ftp'
ftp = Net::FTP.new('127.0.0.1')
ftp.login "srikps", ""

ftp.passive = true
ftp.debug_mode = true

ftp.list('*') do |f| 
	ftp.getbinaryfile('sample.jpg')
end

The debug output:

put: TYPE A
get: 200 Type set to A
put: PASV
get: 227 Entering Passive Mode (127,0,0,1,35,67)
put: LIST *
get: 150 Opening data channel for directory listing of "/*"
put: TYPE I
get: 226 Successfully transferred "/*"
put: PASV
get: 200 Type set to I
put: TYPE A
get: 227 Entering Passive Mode (127,0,0,1,35,86)
put: TYPE I
get: 200 Type set to A
E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:981:in `parse227': 200 Type set to I (Net::FTPReplyError)
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:395:in `makepasv'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:407:in `transfercmd'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:491:in `block (2 levels) in retrbinary'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:199:in `with_binary'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:489:in `block in retrbinary'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/monitor.rb:211:in `mon_synchronize'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:488:in `retrbinary'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:621:in `getbinaryfile'
        from ftp.rb:13:in `block in <main>'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:522:in `block (3 levels) in retrlines'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:519:in `loop'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:519:in `block (2 levels) in retrlines'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:199:in `with_binary'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:516:in `block in retrlines'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/monitor.rb:211:in `mon_synchronize'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:515:in `retrlines'
        from E:/RubyInstall-2.2/lib/ruby/2.2.0/net/ftp.rb:764:in `list'
        from ftp.rb:11:in `<main>'

I have written a detailed explanation of this issue in my Stack Overflow answer.
http://stackoverflow.com/questions/31955406/why-do-i-get-200-type-set-to-i-netftpreplyerror/32024255#32024255

[StackOverFlow answer [http://stackoverflow.com/questions/31955406/why-do-i-get-200-type-set-to-i-netftpreplyerror/32024255#32024255]]

I propose that implementation of "list" method be changed to the following:

    def list(*args, &block) # :yield: line
      cmd = "LIST"
      args.each do |arg|
        cmd = cmd + " " + arg.to_s
      end
      
	  # First lets fetch all the lines
	  lines = []
      retrlines(cmd) do |line|
        lines << line
      end

	  if block
        yield lines
      else
        return lines
      end
    end

[srikps (Srikanth Shreenivas)]

• Assignee set to shugo (Shugo Maeda)

I think in my proposed change:

yield lines

should be

lines.each { |line| yield line }

I have prepared the changes in my fork - link given below -
https://github.com/srikanthps/ruby/commit/9b7f5d9bfd653bd2829620682eeef67f2e0bbdea

Please review the change and if it is fine, let me know whether I can submit a pull request

[srikps (Srikanth Shreenivas)]

Please note that update URL of Git Diff is: https://github.com/srikanthps/ruby/commit/9b7f5d9bfd653bd2829620682eeef67f2e0bbdea

Srikanth Shreenivas wrote:

I think in my proposed change:

yield lines

should be

lines.each { |line| yield line }

I have prepared the changes in my fork - link given below -
https://github.com/srikanthps/ruby/commit/9b7f5d9bfd653bd2829620682eeef67f2e0bbdea

Please review the change and if it is fine, let me know whether I can submit a pull request

[shugo (Shugo Maeda)]

• Status changed from Open to Closed

Applied in changeset ruby-trunk:r51791.

---

• lib/net/ftp.rb (list): fetch all the lines before yielding a block
  to allow other commands in the block. [Feature #11454]
  Patched by Srikanth Shreenivas.

[hsbt (Hiroshi SHIBATA)]

• Project changed from 14 to Ruby master