ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: koshigoeb@gmail.com
To: ruby-core@ruby-lang.org
Subject: [ruby-core:98419] [Ruby master Bug#16780] Net::FTP PUT command issuing Net::ReadTimeout too quickly
Date: Mon, 18 May 2020 00:21:34 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-85689.20200518002133.37403@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-16780.20200411221736.37403@ruby-lang.org

Issue #16780 has been updated by koshigoe (Masataka SUZUKI).


I'm facing the same problem.

The `Net::FTP#close` suppresses exception about `Socket#shutdown` and `Socket#read`.

- https://github.com/ruby/ruby/blob/a0c7c23c9cec0d0ffcba012279cd652d28ad5bf3/lib/net/ftp.rb#L1340-L1355

~~~ruby
    def close
      if @sock and not @sock.closed?
        begin
          @sock.shutdown(Socket::SHUT_WR) rescue nil
          orig, self.read_timeout = self.read_timeout, 3
          @sock.read rescue nil
        ensure
          @sock.close
          self.read_timeout = orig
        end
      end
    end
~~~

Should I do the same when closing data connection?

~~~diff
diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb
index d1e545c0c8..610027dc38 100644
--- a/lib/net/ftp.rb
+++ b/lib/net/ftp.rb
@@ -634,9 +634,9 @@ def retrbinary(cmd, blocksize, rest_offset = nil) # :yield: data
             while data = conn.read(blocksize)
               yield(data)
             end
-            conn.shutdown(Socket::SHUT_WR)
+            conn.shutdown(Socket::SHUT_WR) rescue nil
             conn.read_timeout = 1
-            conn.read
+            conn.read rescue nil
           ensure
             conn.close if conn
           end
@@ -659,9 +659,9 @@ def retrlines(cmd) # :yield: line
             while line = conn.gets
               yield(line.sub(/\r?\n\z/, ""), !line.match(/\n\z/).nil?)
             end
-            conn.shutdown(Socket::SHUT_WR)
+            conn.shutdown(Socket::SHUT_WR) rescue nil
             conn.read_timeout = 1
-            conn.read
+            conn.read rescue nil
           ensure
             conn.close if conn
           end
@@ -688,9 +688,9 @@ def storbinary(cmd, file, blocksize, rest_offset = nil) # :yield: data
               conn.write(buf)
               yield(buf) if block_given?
             end
-            conn.shutdown(Socket::SHUT_WR)
+            conn.shutdown(Socket::SHUT_WR) rescue nil
             conn.read_timeout = 1
-            conn.read
+            conn.read rescue nil
           ensure
             conn.close if conn
           end
@@ -724,9 +724,9 @@ def storlines(cmd, file) # :yield: line
               conn.write(buf)
               yield(buf) if block_given?
             end
-            conn.shutdown(Socket::SHUT_WR)
+            conn.shutdown(Socket::SHUT_WR) rescue nil
             conn.read_timeout = 1
-            conn.read
+            conn.read rescue nil
           ensure
             conn.close if conn
           end
~~~

----------------------------------------
Bug #16780: Net::FTP PUT command issuing Net::ReadTimeout too quickly
https://bugs.ruby-lang.org/issues/16780#change-85689

* Author: rgerard (Ryan Gerard)
* Status: Assigned
* Priority: Normal
* Assignee: naruse (Yui NARUSE)
* ruby -v: 2.7.1
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
This is my first time writing up an issue for this community, so I apologize if this is written in an abnormal way than your normal issues.

We recently upgraded from ruby 2.6.5 to 2.7.1. Almost immediately, we started seeing issues with some of our application code that utilizes Net::FTP. All calls to #put to an external FTP server we communicate with were resulting in a Net::ReadTimeout exception being throw. Upon examination of the external FTP server, we can see that the file was successfully transferred, although the Net::ReadTimeout exception begin thrown caused confusion on our end.

Looking over the changes that were part of the ruby 2.7.0 release, I believe this commit (https://github.com/ruby/ruby/commit/5be34d6a3310065850c0c530db6936415124b5d9), which was a fix for this bug (https://bugs.ruby-lang.org/issues/16413) is the root of the issue. I translated the japanese, and I understand that the change to add the `begin` and `ensure` blocks were to make sure the connection was closed in case an exception is thrown during the transfer. However, the additional change to the connection read_timeout to 1 second was a little puzzling. I can see that this logic was copied from `retrbinary` and `retrlines`, but I'm uncertain why the connection read_timeout should be changed to 1 second.

I believe the right fix is to remove the change to the connection read_timeout.

Here is the stack trace for the exception we're getting during the call to #put:

``` ruby
Net::ReadTimeout with #<Socket:(closed)>

/usr/local/lib/ruby/2.7.0/net/protocol.rb:217:in `rbuf_fill'
/usr/local/lib/ruby/2.7.0/net/protocol.rb:159:in `read'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:1475:in `read'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:693:in `block (2 levels) in storbinary'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:308:in `with_binary'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:684:in `block in storbinary'
/usr/local/lib/ruby/2.7.0/monitor.rb:202:in `synchronize'
/usr/local/lib/ruby/2.7.0/monitor.rb:202:in `mon_synchronize'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:683:in `storbinary'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:843:in `putbinaryfile'
/usr/local/lib/ruby/2.7.0/net/ftp.rb:871:in `put'
```




-- 
https://bugs.ruby-lang.org/

  parent reply	other threads:[~2020-05-18  0:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-11 22:17 [ruby-core:97838] [Ruby master Bug#16780] Net::FTP PUT command issuing Net::ReadTimeout too quickly ryan.gerard
2020-05-01  8:46 ` [ruby-core:98100] " shugo
2020-05-18  0:21 ` koshigoeb [this message]
2020-05-19  8:45 ` [ruby-core:98440] " shugo
2020-07-25 12:42 ` [ruby-core:99333] " nagachika00

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-85689.20200518002133.37403@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).