🙃

RackとGoのhttp.Clientの相性問題について

2022/12/04に公開2件

ずっと悩んでいるので色んな意見を聞きたくて、情報を簡単にまとめておきます。

経緯

私は以前社内ISUCONのために以下のリポジトリを作成しました。

https://github.com/catatsuy/private-isu

このリポジトリを作成してから5年後に、この問題を題材に書籍化することになりました。これについてはもちろん嬉しいことではありますが、このリポジトリを作成したときは私は新卒3年目でエンジニアとしての能力も高くなく、業界的にもISUCONの問題作成ノウハウが貯まっているとは言えない状況で、しかも私自身が問題作成ができるのか分からない中で模索しながら作った問題という経緯があります。なので作り込みはかなり甘く、ツッコミどころは正直色々あります。

それもそれで当時の私の技術力で果敢に挑んだ結果ではあるので許して欲しいところではあるのですが、今回解説する問題が発生してしまっているので、その問題を解説します。

まずprivate-isuの参考実装はSPAを利用しておらず、以下のような少し古いオーソドックスなWebアプリケーションの構成をしています。

POSTで投稿をする→DBに保存後にステータスコード302で他のページにリダイレクトする

あまりGoの知識がなかった当時の私はGoのhttp.Clientがリダイレクトを自前で解釈してGETのリクエストを実際に送ってくれることをおもしろがって、その挙動に依存するベンチマーカーを作成しました。なので実はリダイレクトすることしかテストしていないので、他のリダイレクトをするステータスコードに書き換えても特にベンチマークは失敗しないと思います（動作未確認）。

この挙動で以下の問題が発生することが分かりました。

状況を簡単に説明すると

Goのhttp.Clientはmutilpart/form-dataを利用したPOSTリクエストを送った後にリダイレクトする時にContent-Typeの情報を引き継いだままGETリクエストを送る
RackはContent-Type: mutilpart/form-dataを送るとGETであっても情報を取得しようとするが、Goのhttp.ClientはContent-Type以外の情報は引き継がれないのでboundaryが空だと判定されて例外を出力する

これは以下の2つが対応として考えられます。

Goのhttp.ClientはContent-Type: mutilpart/form-dataをGET経由で送らない
- CheckRedirectを上書きすることで実現可能
RackはContent-Type: mutilpart/form-dataの検証をGETの場合は行わない

GoやRack側がこの問題を認識して動いてくれればよかったのですが、最初に紹介したissueを立てた結果、どちらも公式には対応が入りそうにありません（これについても誰か助けて欲しい気持ちです）。

ベンチマーカーを実装する上ではGoでCheckRedirectを上書きすればいいと思うと思います。実はもう1つ面倒な話があります。

それは書籍の中でGoのhttp.Clientを利用しているgrafana/k6を紹介していることです。k6の機能としてCheckRedirectを上書きする方法は現時点では多分ありません。

そしてprivate-isuの参考実装はRuby実装がオリジナルであり、他の言語の実装はすべてRuby実装から移植されたという経緯があります。そのため他の言語実装で動かないならまだ許容できる可能性がありますが、Ruby実装で動かないというのはリポジトリの立場上、許容できません。デフォルトで起動するのもRuby実装であり、Ruby実装でk6を動かせないというのは書籍の立場上も苦しいです。

Rubyのアプリケーション側からできることとして、Rack 3を利用すればRack::Multipart::EmptyContentErrorの例外を出すようになっているので、この例外だけrescueして、元の実装を動かすようにすれば理論上動くはずです。ただ他の言語実装と同じ挙動をさせるのは後続の処理を再開する必要があるので、どういう実装をすればいいのかが私は思いついていません。そして現在のSinatraはまだRack 3を使用していません。よってできることとしては以下の2つかなと思っています。

k6を無視してRackとSinatraのバージョンアップをする
- 書籍の内容が動かなくなる
SinatraがRack 3にバージョンアップするのを待ち、Rack 3前提でコードを書き換える
- どう実装すればいいのか思いついていない

この件は書籍執筆中に発覚した問題で、現在はRackとSinatraのバージョンアップを先送りにしていますが、セキュリティ上の問題もあるのでそろそろ解決するべき時が来ていると思っています。何か意見があれば教えてもらえたら助かります。

Discussion

yzx

HTTP関連RFCでこの問題に関係しそうな箇所をいくつか挙げてみます。
(すべてをちゃんと読んだわけではないので、他にもこの問題に関係する記述や別のRFCがあるかもしれません)

RFC 9112 §6

The message body is identical to the content unless a transfer coding has been applied, as described in Section 6.1.

The presence of a message body in a request is signaled by a Content-Length or Transfer-Encoding header field.

RFC 9110 §9.3.1

Although request message framing is independent of the method used, content received in a GET request has no generally defined semantics, cannot alter the meaning or target of the request, and might lead some implementations to reject the request and close the connection because of its potential as a request smuggling attack (Section 11.2 of [HTTP/1.1]). A client SHOULD NOT generate content in a GET request unless it is made directly to an origin server that has previously indicated, in or out of band, that such a request has a purpose and will be adequately supported. An origin server SHOULD NOT rely on private agreements to receive content, since participants in HTTP communication are often unaware of intermediaries along the request chain.

RFC 9110 §8.6

A user agent SHOULD send Content-Length in a request when the method defines a meaning for enclosed content and it is not sending Transfer-Encoding. For example, a user agent normally sends Content-Length in a POST request even when the value is 0 (indicating empty content). A user agent SHOULD NOT send a Content-Length header field when the request message does not contain content and the method semantics do not anticipate such data

例えばGETメソッドでリクエストにコンテンツが存在しない場合にUAはContent-Lengthを送信すべきでないとありますが、Content-Typeに関してはそのような記述は(ぱっと見)なさそうです。

RFC 9110 §8.2

Representation header fields provide metadata about the representation. When a message includes content, the representation header fields describe how to interpret that data.

とあるので、(Content-Typeのような)表現ヘッダはコンテンツが存在する場合に意味を持つと考えると、 https://github.com/rack/rack/issues/1787#issuecomment-1013917108 で書かれている
空のボディでContent-Typeがmultipart/form-dataだからRFC 2046 section 5.1に違反していると言えるかは微妙な気がします。

ただ、RFC 9110 §15.4 にUAがリダイレクトを自動追従する場合の記述があり、その中に、リクエストメソッドがGETまたはHEADに変更された場合、
Content-Encoding,
Content-Language,
Content-Location,
Content-Type,
Content-Length,
Digest,
Last-Modified
などのコンテンツ固有のヘッダーフィールドを削除するべきとあります。

というわけで、RFC 9110とRFC 9112だけを見て言うと、

Goのhttp.ClientはリダイレクトでGETリクエストを送信する場合、Content-Typeヘッダフィールドを(も)削除する
RackはGETリクエストでメッセージボディがない(Content-Length、Transfer-Encodingいずれも存在しない)場合、Content-Typeヘッダフィールドがあってもそれを無視する(メッセージボディが存在すると想定した処理をしない)

あたりが、妥当な振る舞いなのかなと思います。

catatsuy

ありがとうございます。本件ですが、進展がありまして、今こちらのissueなどで議論中です。

ぜひ議論に参加してもらえると助かります。よろしくお願いします。