[ruby-dev:39548] [Bug #2291] Net::FTPでソケットをオープンする前にbinary=を呼び出すと落ちる

[Nobuyoshi Nakada-3]

Bug #2291: Net::FTPでソケットをオープンする前にbinary=を呼び出すと落ちる
http://redmine.ruby-lang.org/issues/show/2291

起票者: Akira Matsuda
ステータス: Open, 優先度: Normal
カテゴリ: lib, Target version: 1.9.2
ruby -v: ruby 1.9.2dev (2009-09-29 trunk 25153) [i386-darwin9.8.0]

% cat ftp_binary.rb
require 'net/ftp'
f = Net::FTP.new
f.binary = true

% ruby ftp_binary.rb
/Users/matsuda/ruby_trunk/lib/ruby/1.9.1/net/ftp.rb:224:in `putline': undefined method `write' for nil:NilClass (NoMethodError)
        from /Users/matsuda/ruby_trunk/lib/ruby/1.9.1/net/ftp.rb:291:in `block in voidcmd'
        from /Users/matsuda/ruby_trunk/lib/ruby/1.9.1/monitor.rb:190:in `mon_synchronize'
        from /Users/matsuda/ruby_trunk/lib/ruby/1.9.1/net/ftp.rb:290:in `voidcmd'
        from /Users/matsuda/ruby_trunk/lib/ruby/1.9.1/net/ftp.rb:146:in `binary='
        from ftp_binary.rb:3:in `<main>'


----------------------------------------
http://redmine.ruby-lang.org

[Nobuyoshi Nakada-3]

チケット #2291 が更新されました。 (by Yui NARUSE)

ステータス OpenからAssignedに変更
担当者 Shugo Maedaにセット

diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb
index 52bd375..239bd3c 100644
--- a/lib/net/ftp.rb
+++ b/lib/net/ftp.rb
@@ -132,6 +132,7 @@ module Net
       @passive = false
       @debug_mode = false
       @resume = false
+      @sock = nil
       if host
        connect(host)
        if user
@@ -143,7 +144,7 @@ module Net
     def binary=(newmode)
       if newmode != @binary
         @binary = newmode
-        @binary ? voidcmd("TYPE I") : voidcmd("TYPE A")
+        @binary ? voidcmd("TYPE I") : voidcmd("TYPE A") unless closed?
       end
     end

こんな感じですかね
----------------------------------------
http://redmine.ruby-lang.org/issues/show/2291

----------------------------------------
http://redmine.ruby-lang.org

[Nobuyoshi Nakada-3]

チケット #2291 が更新されました。 (by Yui NARUSE)

ステータス AssignedからClosedに変更
進捗 % 0から100に変更

This issue was solved with changeset r25518.
Akira, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

----------------------------------------
http://redmine.ruby-lang.org/issues/show/2291

----------------------------------------
http://redmine.ruby-lang.org

[Nobuyoshi Nakada-3]

チケット #2291 が更新されました。 (by Shugo Maeda)

ステータス ClosedからAssignedに変更

この修正で問題ないとしたら、ちょっとAPIが変だと思うので、もうちょっと考えさせてください。
# 一応commit前に自分に確認してもらえるとうれしいです。
----------------------------------------
http://redmine.ruby-lang.org/issues/show/2291

----------------------------------------
http://redmine.ruby-lang.org

[Shugo Maeda]

前田です。

2009年10月27日23:55 Shugo Maeda <redmine@...>:
> この修正で問題ないとしたら、ちょっとAPIが変だと思うので、もうちょっと考えさせてください。

やっぱりこの修正だと、ftp.connect前にftp.binary = trueとすると、その後にloginやgetbinaryfile
などでbinary modeにセットしようとしてもTYPEコマンドが発行されなくなってしまうようです。
# Net::FTP#binary=は、新しいモードが現在と同じだとTYPEコマンドを発行しないため。

(1) connect前にftp.binary = trueとしたことを覚えておいて、ftp.connect時にTYPEコマンドを
    発行するようにする。
(2) connect前にftp.binary=実行した場合は、潔く(?)例外を挙げる。

といった対応が考えられますが、特にconnect前にNet::FTP#binary=を使いたい理由があるでしょうか。
なければ(2)の方がシンプルなので、(2)にしたいのですが。

--
Shugo Maeda

[Nobuyoshi Nakada-3]

チケット #2291 が更新されました。 (by Yui NARUSE)


あぁ、仰るとおりです。

>(1) connect前にftp.binary = trueとしたことを覚えておいて、ftp.connect時にTYPEコマンドを
>    発行するようにする。

こちらだったら以下のような感じですかね。

diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb
index 239bd3c..351e624 100644
--- a/lib/net/ftp.rb
+++ b/lib/net/ftp.rb
@@ -128,7 +128,7 @@ module Net
     #
     def initialize(host = nil, user = nil, passwd = nil, acct = nil)
       super()
-      @binary = false
+      @binary = nil
       @passive = false
       @debug_mode = false
       @resume = false
@@ -194,6 +194,7 @@ module Net
        @sock = open_socket(host, port)
        voidresp
       end
+      @binary ? voidcmd("TYPE I") : voidcmd("TYPE A") unless @binary.nil?
     end

     #
----------------------------------------
http://redmine.ruby-lang.org/issues/show/2291

----------------------------------------
http://redmine.ruby-lang.org

[Shugo Maeda]

前田です。

2009年10月28日0:48 Yui NARUSE <redmine@...>:
>>(1) connect前にftp.binary = trueとしたことを覚えておいて、ftp.connect時にTYPEコマンドを
>>    発行するようにする。
>
> こちらだったら以下のような感じですかね。

よく考えたらlogin後でないとTYPEコマンドを発行できないですね。
また、現状では、Net::FTP#loginでbinary = trueにしているので、login前に
値を設定しても意味がありません。

このあたりは1.8と挙動が違うのですが、意図としてはデフォルトをbinary mode
にしたいということです。
1.9でNet::FTP#binary=がTYPEコマンドを発行するようになったのは、
getbinaryfileなどの呼び出しのたびにTYPEコマンドが発行されるのを
避けるためです。
この影響で、binary=の設定がlogin後でないとできなくなったので、loginメソッド
の中でself.binary = trueとするように修正されました。
# そもそも元のNet::FTP#binary=自体が自分が書いたコードじゃない気がするので
# 経緯をはっきり覚えてないんですが。

(1)の方針で行くなら、FTPサーバ上のモードを表すインスタンス変数を行しく
導入する必要がある気がします。

とりあえず、r25518のbinary=の方の修正はいったんrevertしておきます。

--
Shugo Maeda

[Shugo Maeda]

前田です。

2009年10月28日11:09 Shugo Maeda <shugo@...>:
> (1)の方針で行くなら、FTPサーバ上のモードを表すインスタンス変数を行しく
> 導入する必要がある気がします。

すみません、T-Codeのせいでありえないtypoをしてしまいました。
sub(/行しく/, "新しく")してください。

--
Shugo Maeda

[Shugo Maeda]

前田です。

2009年10月28日11:09 Shugo Maeda <shugo@...>:
> (1)の方針で行くなら、FTPサーバ上のモードを表すインスタンス変数を行しく
> 導入する必要がある気がします。
>
> とりあえず、r25518のbinary=の方の修正はいったんrevertしておきます。

結局ログイン状態を表すインスタンス変数を導入して、@binaryのデフォルト値を
trueにしました。
login時に@binaryの値によって適切なTYPEコマンドを発行するようにしました。

--
Shugo Maeda

[Nobuyoshi Nakada-3]

チケット #2291 が更新されました。 (by Shugo Maeda)

ステータス AssignedからClosedに変更

This issue was solved with changeset r25519.
Akira, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

----------------------------------------
http://redmine.ruby-lang.org/issues/show/2291

----------------------------------------
http://redmine.ruby-lang.org