Bug #3345: webrick test failure on Windows(?)

• Status: Closed
• Assignee: usa (Usaku NAKAMURA)
• Target version: 1.9.2
• ruby -v: ruby 1.9.3dev (2010-05-26 trunk 28018) [i386-mswin32]

[usa (Usaku NAKAMURA)]

=begin
webrickのテストが以下のように失敗します。

1. Failure:
   test_cgi(TestWEBrickCGI) [C:/ruby-trunk/test/webrick/test_cgi.rb:40]:
   webrick log start:
   [2010-05-26 17:32:08] INFO WEBrick 1.3.1
   [2010-05-26 17:32:08] INFO ruby 1.9.3 (2010-05-26) [i386-mswin32]
   [2010-05-26 17:32:08] INFO WEBrick::HTTPServer#start: pid=6292 port=60199
   [2010-05-26 17:32:08] WARN :RequestHandler is deprecated, please use :RequestCallback

[2010-05-26 17:32:09] WARN :RequestHandler is deprecated, please use :RequestCallback

[2010-05-26 17:32:11] WARN :RequestHandler is deprecated, please use :RequestCallback

[2010-05-26 17:32:12] WARN :RequestHandler is deprecated, please use :RequestCallback
[2010-05-26 17:32:12] ERROR Encoding::CompatibilityError: incompatible character encodings: ASCII-8BIT and Windows-31J
C:/ruby-trunk/lib/webrick/httpservlet/filehandler.rb:225:in expand_path' C:/ruby-trunk/lib/webrick/httpservlet/filehandler.rb:225:in prevent_directory_traversal'
C:/ruby-trunk/lib/webrick/httpservlet/filehandler.rb:166:in service' C:/ruby-trunk/lib/webrick/httpserver.rb:111:in service'
C:/ruby-trunk/lib/webrick/httpserver.rb:70:in run' C:/ruby-trunk/lib/webrick/server.rb:183:in block in start_thread'

webrick log end.
<"/\xA4\xDB\xA4\xB2/\xA4\xDB\xA4\xB2"> expected but was
<"\n\n

<TITLE>Internal Server Error</TITLE>\n \n

Internal Server Error

\n incompatible character encodings: ASCII-8BIT and Windows-31J\n

---

\n \n WEBrick/1.3.1 (Ruby/1.9.3/2010-05-26) OpenSSL/1.0.0 at\n 127.0.0.1:60199\n \n \n\n">.

クライアントから渡された「/%A4%DB%A4%B2/%A4%DB%A4%B2」という
パスを実ファイルシステムのパスに変換しようとしているわけですが、
(1) 「/%A4%DB%A4%B2/%A4%DB%A4%B2」をunescapeするとASCII-8BITになる
(2) それをFile.expand_pathに渡すと、filesystem encodingであるはずの
cwdとくっつけようとするのでエラーになる
という話のようです。

というわけで、以下のパッチでいちおう直るんですが、話としては他の
プラットフォームでも発生してしかるべき問題のような気がするのに、
どうも私しか踏んでないっぽいのはなぜでしょう?
また、直し方としてはたしてこれでいいんでしょうか?
どなたか教えてください。

Index: lib/webrick/httputils.rb

--- lib/webrick/httputils.rb (revision 28018)
+++ lib/webrick/httputils.rb (working copy)
@@ -27,7 +27,7 @@ module WEBrick
while ret.sub!(%r'/(?!../)[^/]+/..(?:/|\Z)', '/'); end # /foo/.. => /foo

    raise "abnormal path `#{path}'" if %r{/\.\.(/|\Z)} =~ ret

•  ret
  

•  ret.force_encoding(Encoding.find("filesystem"))
  

  end
  module_function :normalize_path
  =end

[mame (Yusuke Endoh)]

• Assignee set to usa (Usaku NAKAMURA)

=begin
遠藤です。

というわけで、以下のパッチでいちおう直るんですが、話としては他の
プラットフォームでも発生してしかるべき問題のような気がするのに、
どうも私しか踏んでないっぽいのはなぜでしょう?

File.expand_path("/webrick.cgi/\xA4\xDB\xA4\xB2/\xA4\xDB\xA4\xB2".force_encoding("ASCII-8BIT"))

のようなことが実行されていて、先頭が / で始まっているため、Linux など
だと特に何もしないけれど、Windows だと何か (ドライブレター?) を足そう
とするため落ちるのかと思います。

unak さんのパッチだと、Linux でエラーが発生するようになりました。
filesystem encoding は UTF-8 ですが、このパスは UTF-8 として invalid
なので、=~ で正規表現マッチができないことが原因です。

1. Failure:
   test_cgi(TestWEBrickCGI) [/home/mame/work/ruby/test/webrick/test_cgi.rb:40]:
   webrick log start:
   [2010-06-16 02:48:55] INFO WEBrick 1.3.1
   [2010-06-16 02:48:55] INFO ruby 1.9.3 (2010-06-16) [i686-linux]
   [2010-06-16 02:48:55] INFO WEBrick::HTTPServer#start: pid=15315 port=38912
   [2010-06-16 02:48:55] WARN :RequestHandler is deprecated, please use :RequestCallback

[2010-06-16 02:48:55] WARN :RequestHandler is deprecated, please use :RequestCallback

[2010-06-16 02:48:55] WARN :RequestHandler is deprecated, please use :RequestCallback

[2010-06-16 02:48:55] WARN :RequestHandler is deprecated, please use :RequestCallback
[2010-06-16 02:48:55] ERROR ArgumentError: invalid byte sequence in UTF-8
/home/mame/work/ruby/lib/webrick/httpserver.rb:196:in =~' /home/mame/work/ruby/lib/webrick/httpserver.rb:196:in scan'
/home/mame/work/ruby/lib/webrick/httpserver.rb:136:in search_servlet' /home/mame/work/ruby/lib/webrick/httpserver.rb:105:in service'
/home/mame/work/ruby/lib/webrick/httpserver.rb:70:in run' /home/mame/work/ruby/lib/webrick/server.rb:183:in block in start_thread'

prevent_directory_traversal の問題を正確に理解していないのですが、
問題がおきるのが Windows だけで、\ の扱いだけの問題だとしたら、
File.expand_path を使わず gsub で処理するといいのではないでしょう
か。

diff --git a/lib/webrick/httpservlet/filehandler.rb b/lib/webrick/httpservlet/filehandler.rb
index 32c1965..50bf27c 100644
--- a/lib/webrick/httpservlet/filehandler.rb
+++ b/lib/webrick/httpservlet/filehandler.rb
@@ -214,17 +214,13 @@ module WEBrick
# character in URI notation. So the value of path_info should be
# normalize before accessing to the filesystem.

•    if trailing_pathsep?(req.path_info)
  

•      # File.expand_path removes the trailing path separator.
  

•      # Adding a character is a workaround to save it.
  

•      #  File.expand_path("/aaa/")        #=> "/aaa"
  

•      #  File.expand_path("/aaa/" + "x")  #=> "/aaa/x"
  

•      expanded = File.expand_path(req.path_info + "x")
  

•      expanded.chop!  # remove trailing "x"
  

•    else
  

•      expanded = File.expand_path(req.path_info)
  

•    is_windows = RUBY_PLATFORM =~ /mswin/  ||
  

•                 RUBY_PLATFORM =~ /mingw/  ||
  

•                 RUBY_PLATFORM =~ /bccwin/ ||
  

•                 RUBY_PLATFORM =~ /wince/
  

•    if is_windows
  

•      req.path_info = req.path_info.gsub("\\", "/")
     end
  

•    req.path_info = expanded
   end
  
   def exec_handler(req, res)
  

--
Yusuke Endoh [e-mail]
=end

[usa (Usaku NAKAMURA)]

=begin
こんにちは、なかむら(う)です。

In message "[ruby-dev:41615] [Bug #3345] webrick test failure on Windows(?)"
on Jun.16,2010 03:13:55, [e-mail] wrote:

担当者 Usaku NAKAMURAにセット

え、なんで?
報告したら担当しないといけないの?

というわけで、以下のパッチでいちおう直るんですが、話としては他の
プラットフォームでも発生してしかるべき問題のような気がするのに、
どうも私しか踏んでないっぽいのはなぜでしょう?

File.expand_path("/webrick.cgi/\xA4\xDB\xA4\xB2/\xA4\xDB\xA4\xB2".force_encoding("ASCII-8BIT"))

のようなことが実行されていて、先頭が / で始まっているため、Linux など
だと特に何もしないけれど、Windows だと何か (ドライブレター?) を足そう
とするため落ちるのかと思います。

なるほど。

unak さんのパッチだと、Linux でエラーが発生するようになりました。
filesystem encoding は UTF-8 ですが、このパスは UTF-8 として invalid
なので、=~ で正規表現マッチができないことが原因です。

そこをいじるのは地獄道っぽいですねえ。

prevent_directory_traversal の問題を正確に理解していないのですが、
問題がおきるのが Windows だけで、\ の扱いだけの問題だとしたら、
File.expand_path を使わず gsub で処理するといいのではないでしょう
か。

残念ながら \ の扱いだけの問題ではないので...
具体的には、短いファイル名形式を利用したアクセスや、NTFSデー
タストリームの主ストリーム明示指定アクセスなどによるアクセス
制御回避を、File.expand_pathの正規化処理を利用して排除してい
ます。
File.expand_pathが行っている正規化処理と同等の処理を全部ここ
に並べるのはさすがにちょっと。

それでは。

U.Nakamura [e-mail]
言っちゃってよかったのかな

=end

[mame (Yusuke Endoh)]

=begin
遠藤です。

2010年6月16日9:45 U.Nakamura [e-mail]:

担当者 Usaku NAKAMURAにセット

え、なんで?
報告したら担当しないといけないの?

報告者だからではなく、windows のプラットフォームメンテナということで
担当者にさせてもらいました。メンテナがいないライブラリの、プラット
フォーム特有の問題はプラットフォームメンテナに見てもらうしかないかな
と思って。
見たくないということでしたら、WONTFIX と判断する権限はあると思います。

prevent_directory_traversal の問題を正確に理解していないのですが、
問題がおきるのが Windows だけで、\ の扱いだけの問題だとしたら、
File.expand_path を使わず gsub で処理するといいのではないでしょう
か。

残念ながら \ の扱いだけの問題ではないので...
具体的には、短いファイル名形式を利用したアクセスや、NTFSデー
タストリームの主ストリーム明示指定アクセスなどによるアクセス
制御回避を、File.expand_pathの正規化処理を利用して排除してい
ます。
File.expand_pathが行っている正規化処理と同等の処理を全部ここ
に並べるのはさすがにちょっと。

File.expand_path は絶対パスに変換するメソッドであって、正規化に使う
のは筋違いではないかという気がします。
trailing_pathsep? の処理が必要なことが傍証？

筋がどうこう言ってても security issue はしょうがないのでしょうがない
のですが、じゃあこういうパッチはどうでしょうか。
windows では試していませんが。

diff --git a/lib/webrick/httpservlet/filehandler.rb
b/lib/webrick/httpservlet/filehandler.rb
index 32c1965..4887903 100644
--- a/lib/webrick/httpservlet/filehandler.rb
+++ b/lib/webrick/httpservlet/filehandler.rb
@@ -214,16 +214,18 @@ module WEBrick
# character in URI notation. So the value of path_info should be
# normalize before accessing to the filesystem.

•    path = req.path_info.dup.force_encoding(Encoding.find("filesystem"))
     if trailing_pathsep?(req.path_info)
       # File.expand_path removes the trailing path separator.
       # Adding a character is a workaround to save it.
       #  File.expand_path("/aaa/")        #=> "/aaa"
       #  File.expand_path("/aaa/" + "x")  #=> "/aaa/x"
  

•      expanded = File.expand_path(req.path_info + "x")
  

•      expanded = File.expand_path(path + "x")
       expanded.chop!  # remove trailing "x"
     else
  

•      expanded = File.expand_path(req.path_info)
  

•      expanded = File.expand_path(path)
     end
  

•    expanded.force_encoding(req.path_info.encoding)
     req.path_info = expanded
   end
  

--
Yusuke Endoh [e-mail]

=end

[usa (Usaku NAKAMURA)]

=begin
こんにちは、なかむら(う)です。

In message "[ruby-dev:41632] Re: [Bug #3345] webrick test failure on Windows(?)"
on Jun.17,2010 12:39:06, [e-mail] wrote:

担当者 Usaku NAKAMURAにセット

え、なんで?
報告したら担当しないといけないの?

報告者だからではなく、windows のプラットフォームメンテナということで
担当者にさせてもらいました。メンテナがいないライブラリの、プラット
フォーム特有の問題はプラットフォームメンテナに見てもらうしかないかな
と思って。

メール送ってからそういう意味だろうと気付きました。すみません。

見たくないということでしたら、WONTFIX と判断する権限はあると思います。

見たくないというか、よくわかんないから手が出せないだけで、誰
か中身が分かる人には直してほしいです、もちろん。
私としては本件は気分的には被害者なので、責任者出てこいゴルァ
ってなもんです(いや責任者なんてものはいないんだけど)。

残念ながら \ の扱いだけの問題ではないので...
具体的には、短いファイル名形式を利用したアクセスや、NTFSデー
タストリームの主ストリーム明示指定アクセスなどによるアクセス
制御回避を、File.expand_pathの正規化処理を利用して排除してい
ます。
File.expand_pathが行っている正規化処理と同等の処理を全部ここ
に並べるのはさすがにちょっと。

File.expand_path は絶対パスに変換するメソッドであって、正規化に使う
のは筋違いではないかという気がします。
trailing_pathsep? の処理が必要なことが傍証？

筋違いでしょうね。
でもまあ現実として他に少なくともRubyレベルでAPIないんですよね
え。
(Cレベルでもないけど)

筋がどうこう言ってても security issue はしょうがないのでしょうがない
のですが、じゃあこういうパッチはどうでしょうか。
windows では試していませんが。

とりあえずWEBrickのテストは通ってるように見えます。
ので、私としてはこれでかまいませんです。

それでは。

U.Nakamura [e-mail]

=end

[mame (Yusuke Endoh)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

=begin
This issue was solved with changeset r28354.
Usaku, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

=end