search

bnix / double-bag-ftps

Provides a child class of Net::FTP to support implicit and explicit FTPS.
MIT License
48 stars 26 forks source link

Only close the ftp connection if it's open #14

Closed dmathieu closed 7 years ago

dmathieu commented 8 years ago

If an error occured causing the connection to not be opened, this close will trigger an additional exception, as we can't close a closed socket.

/usr/local/Cellar/ruby/2.3.1/lib/ruby/2.3.0/net/ftp.rb:114:in `read_timeout=': undefined method `read_timeout=' for #<TCPSocket:(closed)> (NoMethodError)
Did you mean?  readline
    from /usr/local/Cellar/ruby/2.3.1/lib/ruby/2.3.0/net/ftp.rb:1175:in `ensure in close'
    from /usr/local/Cellar/ruby/2.3.1/lib/ruby/2.3.0/net/ftp.rb:1175:in `close'
    from /usr/local/lib/ruby/gems/2.3.0/gems/double-bag-ftps-0.1.3/lib/double_bag_ftps.rb:33:in `ensure in open'
    from /usr/local/lib/ruby/gems/2.3.0/gems/double-bag-ftps-0.1.3/lib/double_bag_ftps.rb:33:in `open'

This change makes sure we only close connections if they are open.

wconrad commented 8 years ago

Thanks for the patch, and sorry for my delay in responding to it.

This is very interesting, and looks like it is related to https://github.com/bnix/double-bag-ftps/pull/1/commits/043ba70309ed622302d3aa5c6f3169872d1211ca . This gem forces Ruby's ftp library to work with OpenSSL sockets, but those sockets lack some methods, such as #read_timeout=, that Ruby's ftp library reasonably expects a socket to have. That patch adds those methods right after OpenSSL makes the socket.

Your patch is probably fine, but I want to understand what's happening before merging it.

wconrad commented 8 years ago

By the way: Do we have a code example that reproduces this?

dmathieu commented 8 years ago

Hey,

I haven't experienced this myself. But I have a stacktrace and description here: dmathieu/glynn/issues/64

wconrad commented 7 years ago

Merged... thanks!