browse.cgiのpathに関する問題群

h-inoue commented 9 years ago

'+' unescape問題
- そもそもbrowse.cgiのpathにunescapeされたpathが渡ってきている。内部でunescapeする必要なし？
- java-applet を開く際に3度ほどbrowse.cgiが呼ばれているが、2度目以降は幾度もunescapeされたpathが渡ってきているなので、helper.params['path']に'+'が残っていない。
どこでunescapeされるのかはっきりしないのならば、'+'などの危険な文字をパスに使うことは避けたほうが良さそうです。
日本語ディレクトリ名が存在する場合にうまく表示されない。

ファイル名をforce_encoding("UTF-8")すればよさそう。

h-inoue commented 9 years ago

失礼、後者は手元で作って試したからでした。post.cgiでエンコードしてましたね。

krtx commented 9 years ago

log の id を urlsafe なハッシュとかにすると良いでしょうか。

h-inoue commented 9 years ago

たぶん提出されたファイルにも同様の現象が発生するんですよね．とりあえずはlogのidを変えるのでよいかもしれません．

h-inoue commented 9 years ago

調査し直したところ，2種類の問題に分けられました．

+付きdir以下のファイルに対して，「直接開く」uriが動作しない
+付きdir以下の.classに対して，appletが動作しない
- 1は単純なバグ

file_view.jsに

FileView.encodepath = ...
        [ '\\+', '%2B' ],

のように，+の書き換え規則を足せばよい．

2は複雑

「直接開く」等のuriでは，危険な文字を含むdir以下のファイルを指している場合，末尾に?path=...なるクエリが付く．例えば，plus+dir/TestApplet.classへのuriは以下のようになる．

/browse/userid/report/plus+dir/TestApplet.class?path=plus%2Bdir/TestApplet.class

すると，以下の書き換え規則が適用される．

RewriteCond %{QUERY_STRING} (?:^|&)path=
RewriteRule ^(.*?)/browse/([^/]+)/([^/]+)/.+$ $1/api/browse.cgi?user=$2&report=$3 [L,QSA]

mod_rewriteのQSAパラメータにより，先のクエリはescapeされた状態でくっつけられる． ...&path=plus&2Bdir/TestApplet.class のように．

ところがAppletタグだと，?path=...なるクエリは付けられない．なので，以下の規則が適用される．

RewriteCond %{QUERY_STRING} !(?:^|&)path=
RewriteRule ^(.*?)/browse/([^/]+)/([^/]+)/(.+)$ $1/api/browse.cgi?user=$2&report=$3&path=$4 [L]

すると，$4にはescapeされていない状態のパラメータplus+dir/TestApplet.class が渡り，最終的にbrowse.cgiで一度unescapeされるときに+がspaceとなり，404.

skymountain commented 9 years ago

1は単純なバグ

file_view.jsに

FileView.encodepath = ... [ '+', '%2B' ],

のように，+の書き換え規則を足せばよい．

これって多分 encodeURIComponent を使うべきとこなんですよね．（記憶が間違ってなければ)元々のコードでも自前でエンコードしていたんですが，理由はよくわかりません...

h-inoue commented 9 years ago

encodeURIComponentだと '/' もエンコードされるためまずそうに見えます．このencodePathはuriのpathを組み立てるのに使われているので．

skymountain commented 9 years ago

encodeURIComponentだと '/' もエンコードされるためまずそうに見えます．このencodePathはuriのpathを組み立てるのに使われているので．

わかりにくい言い方でしたけど，自分が言ってたのは '?path=' + encodeURICompoennt(path)' でいいんじゃないかということでした．(自前で置換すると ? | & | + 以外は大丈夫かと心配になる)

あと Apache の rewrite engine を前提にしていいんであれば path パラメータを廃止して生パスを rewrite 側で URI encode してやればいい気がします ([B] オプション http://httpd.apache.org/docs/2.2/mod/mod_rewrite.html#rewriteflags とか使って) Rack 側も書き換えが programmable なので対応できる？

h-inoue commented 9 years ago

わかりにくい言い方でしたけど，自分が言ってたのは '?path=' + encodeURICompoennt(path)' でいいんじゃないかということでした．(自前で置換すると ? | & | + 以外は大丈夫かと心配になる)

そちらはその通りですね．epathを使い回しているのはよくわからない．

rack-rewriteでも同様のことができるならばそれでよさそうですね．

h-inoue commented 9 years ago

rewriteをこんな具合にしてやって，VirtualHostの設定にAllowEncodedSlashes Onを足すと，apacheでは動きました．後者の設定弄らないと駄目なのはどうなの，という感じですが．

RewriteEngine on
RewriteBase /
RewriteRule ^.*$ %{REQUEST_URI} [C,DPI]
RewriteCond %{QUERY_STRING} (?:^|&)path=
RewriteRule ^/(.*?)/browse/([^/]+)/([^/]+)/.+$ $1/api/browse.cgi?user=$2&report=$3 [B,L,QSA]
RewriteCond %{QUERY_STRING} !(?:^|&)path=
RewriteRule ^/(.*?)/browse/([^/]+)/([^/]+)/(.+)$ $1/api/browse.cgi?user=$2&report=$3&path=$4 [B,L]
RewriteRule ^(.*)$ $1 [R=404]

(RewriteBase と Bフラグを足しています)

h-inoue commented 9 years ago

close #380

SoftwareFoundationGroupAtKyotoU / automata

browse.cgiのpathに関する問題群 #177