飲み物だから太らない


Y.Sawaの日記と言うか、雑記と言うか。
(ちょっと)まじめなページ
この日記の内容は私及び第三者の意志に関わらず、私の所属するいかなる組織の意見・知識とは無関係です。

2010-02-03

java.net.HttpURLConnection が設計ミスを起こしている件 23:43

JavaHTTPクライアントを作ろうと思ったときには、おそらく二つくらいのアプローチがあって、ひとつApache JakartaプロジェクトのHTTPClientを使う方法、もうひとつjava.net.HttpURLConnection を使う方法だ。まあもちろん、自前でTCPクライアントの上に乗せてもいいのだが、そんな車輪の再発明するくらいならもっと別のことに労力を使うべきだと思う。

というわけで、そのうちひとつjava.net.HttpURLConnectionの件。HTTPは最初だけ大文字、後小文字の癖に、URLは全部大文字というよくわからんネーミングセンスについてはとりあえず置いておくとして、設計的によくわからないというか、何がしたいのか意味が不明なところがあるのでそれについて書く。

http://java.sun.com/j2se/1.5.0/docs/api/java/net/HttpURLConnection.html

HttpURLConnectionは、URLオブジェクトからコネクションを張ることで以下のように接続を行う。

 URL url = new URL("http://example.com/");
 HttpURLConnection conn = (HttpURLConnection)url.openConnection();
 conn.setRequestMethod("GET");
 conn.connect();

で、その後データを取得する。この際使用するのがBufferedReaderなのはバイナリが帰ってくるかもしれないから仕方ない。

 BufferedReader br = new BufferedReader(new InputStreamReader(conn.getInputStream()));

問題なのは、この

 conn.getInputStream()

が、throw IOExceptionとなっていることなのだ。これはリファレンスによれば、

if an I/O error occurs while creating the input stream.

と書いてある。これ、実際にどういうときに起きるかといえば、他にもあるかもしれないがサーバエラーを返したときなのだ。つまり、サーバが4xxまたは5xxのエラーレスポンスを返したときには、なぜかInputStreamは生成されない。nullが返るのではなくてExceptionが飛んでくる。そしてその場合にはgetErrorStream()で取得したStreamHTTP ResponseBodyが返ってくる。

あるサンプルソースコードはこのため、こんなことをやっている(http://java.sun.com/j2se/1.5.0/docs/guide/net/http-keepalive.html)。

try {
	URL a = new URL(args[0]);
	URLConnection urlc = a.openConnection();
	is = conn.getInputStream();
	int ret = 0;
	while ((ret = is.read(buf)) > 0) {
	  processBuf(buf);
	}
	// close the inputstream
	is.close();
} catch (IOException e) {
	try {
		respCode = ((HttpURLConnection)conn).getResponseCode();
		es = ((HttpURLConnection)conn).getErrorStream();
		int ret = 0;
		// read the response body
		while ((ret = es.read(buf)) > 0) {
			processBuf(buf);
		}
		// close the errorstream
		es.close();
	} catch(IOException ex) {
		// deal with the exception
	}
}

このソースコードは、まあ目的が違って永続セッションを張ろうとしているので(サーバエラーを返した時点で終了するのがクライアントとして正しい動作なので)これでいいのだろうけども、「とりあえずInputStreamを取得して、Exceptionが出たらErrorStream使おう」とかいう態度はどうかと思う。例外は、正常形で投げられてはいけないのだ。

HTTPClientを書こうとした人間が、サーバの4xxエラーまたは5xxエラーを正常系とみなすか異常系とみなすかは不明である。サーバにとって異常系なのは明らかだが、例えばサーバエラーリンク切れ発見するテストクライアントであれば、異常系ではない。

4xxや5xxがクライアントにとっても異常系だと主張するにしても、getInputStream()で例外を投げるのはタイミングがおかしい。エラーbodyを取得する前にわかっているのだから、その前に例外を投げるべきだ。

あるいは、以下のような実装も考えられる。

 conn.connect();
 InputStream stream;
 if(conn.isSuccess()){
   stream = conn.getInputStream();
 }else{
   stream = conn.getErrorStream();
 }

しかし、isSuccessのようなメソッドは実装されていない。また、本質的な話としてこのHttpURLConnection#getInputStreamが例外を投げるケースはResponse Codeが何番なのかということはドキュメントのどこにも書いていないことが挙げられる。したがって、このようなisSuccessを自前で作ることすらできないのだ。

実際に正しくやろうと思ったら現状では以下のようにするしかないが、これで正しいかどうかは誰も保障してくれないのだ。

 InputStream stream;
 int responseCode = conn.getResponseCode();
 if(responseCode / 100 == 4 || responseCode / 100 == 5){
  stream = conn.getErrorStream();
 }else{
  stream = conn.getInputStream();
 }

明らかに設計ミスだと思うんですが。

pぁpぁ 2010/02/04 01:36 ライブラリ設計の良し悪しはあんまりわかんないんだけど、
> 「とりあえずInputStreamを取得して、Exceptionが出たらErrorStream使おう」とかいう態度はどうかと思う。
例外になっちゃってるものは仕方ないとして、「getInputStreamでの例外」と「その後の入出力での例外」を区別すればいくらかマシになる気はする。たとえばこんな感じで
InputStreamAndException getInputStreamAndExceptionFrom(HttpURLConnection conn) {
try {
return new InputStreamAndException(conn.getInputStream(), null);
}
catch (IOException ioe) {
return new InputStreamAndException(conn.getErrorStream(), ioe);
}
}

InputStreamAndErrorの定義は察して。ほかにもthrows IOExceptionしておいてtryの前にconnectやgetResponseCodeしてみるとか、返値でInputStreamとペアにするのは例外じゃなくてResponseCodeにしてみるとか、工夫できると思う

HIROXHIROX 2010/02/04 19:22 なんか宗教的な話に見えるんですが、例外をいやがる理由が謎です。
「テストするクライアントが〜」とか言い出すと異常系なんてこの世から無くなるのでは?

succeedsucceed 2010/02/04 21:36 >pla
うーん。これはこれでどうなんだろう。
もし受け取った人が、throw nullとかやったらその瞬間ぬるぽるので危険というか、わけがわからなくなると思う。
reesponseCodeならいいけど、そういうのは最初から取得できるのだから、わざわざ多値返ししてやるほどのことは無いと思う。
>HIROX
宗教といえばそうかもしれないけど、Javaの設計思想の問題だと思うんだよね。
例えば、例外でも動くからって、1から100までの和の計算で
int i = 100, sum = 0;
try{
while(true){
sum += i;
i--;
int tmp = 1/i;
}
}catch(Exception e){
System.out.println(sum);
}
なんてコードが許されるか、って問題。
検査例外は業務エラーを出力するべきであって、業務エラーじゃないものを例外としてはいけないのだと思うよ。
http://blogs.msdn.com/nakama/archive/2009/01/09/net-java.aspx
で、クライアントにとって業務エラーかどうかを決めるのは、現代のHTTPの使われ方から考えて、サーバじゃないと思う。

HIROXHIROX 2010/02/04 22:32 > 現代のHTTP
JDK1.1からあるクラスに食ってかかっても。。。
互換性もあるし普通はライブラリなんて妥協の産物ですよ。

今回の目的ならこれで十分では。
InputStream stream;
try {
stream = conn.getInputStream();
} catch(Exception e) {
try {
stream = conn.getErrorStream();
} catch(Exception e) {
// disconnected, invalid HTTP response, etc.
}
}

単に叩くのではなく、建設的により良い実装案をキボンヌ。

succeedsucceed 2010/02/04 22:48 > JDK1.1からあるクラスに食ってかかっても。。。
> 互換性もあるし普通はライブラリなんて妥協の産物ですよ。
ダウト。以下を見れば、昔はこんな実装してなかったことがわかる。
http://docs.sun.com/app/docs/doc/816-3973/6ma7ftaqg?l=ja&a=view
この変更の一部として、現在、HttpURLConnection.getErrorStream を使用すると、サーバから戻されたエラーページを読み取ることができます。J2SE 1.4 より前では、getErrorStream は常に null を戻していました。また、メソッド HttpURLConnection.getResponseCode は J2SE 1.4 で正しく動作します。

succeedsucceed 2010/02/04 22:52 後、よりよい案としてならば、書いてあるように単純にisSuccessメソッド追加するだけなんですけど。併せてドキュメントにちゃんと書いてくれればそれでいい。
本来から言えば、HTTPなのにInputStreamとErrorStreamがある時点でおかしいんだけど、そこは目をつぶろう。URLConnectionの中にはそれをサポートするものもあるかもしれないから。

おrzおrz 2010/02/04 23:53 豚澤は中堅で何やってんの?

HIROXHIROX 2010/02/05 00:09 確かにInputStreamとErrorStreamの両方があるのは変ですが、一度例外を投げてしまうように作ってしまったのだからこのまま通すのは仕方ないんじゃないでしょうか。何がダウトなのかわかんないです。エラー時もInputStreamで返すようにすると非互換が大きくなりますよね?

InputStreamとErrorStreamのどちらで読むべきかを知るためのメソッドがあってもいいかもしれませんが、上記workaroundで簡単に対処できる&エラーのレスポンスを読むケースは希なので追加すべきかどうかは微妙ってとこじゃないでしょうか。
少なくともisSuccessだと何の成否を返すのかわからない(connectの成否を返すように見える)ので良くないと思います。

pぁpぁ 2010/02/05 00:58 > reesponseCodeならいいけど、そういうのは最初から取得できるのだから、わざわざ多値返ししてやるほどのことは無いと思う。
「Response Codeがとれる程度にまともに接続できてるならbody部分のストリームがほしい」という要望だとすれば、Response Codeとペアにするのは結構意味があると思うんだ。値が入っているなら「間違いなく成功した」って分かる。
> もし受け取った人が、throw nullとかやったらその瞬間ぬるぽるので危険というか
nullが返ってくるかもしれないところでnullチェックするのは当然じゃね?もちろんドキュメントはほしいけど。もっというならOptionalみたく静的チェックがほしいけど・・・Javaじゃむりね。
ただこの場合、例外部分にnull入ってるのは「成功したとき」なので、そんなときに何でそこの例外投げようとするかなっていう。
あと、getInputStreamは例外投げないで null 返す方がマシと言っているようにみえるのだけれど、それでいて失敗ステータスとして返す値にnullが入ってるのはだめっていうのはよくわからない。

succeedsucceed 2010/02/06 21:40 >おrz
某社の依頼を受けてデバッグするだけの簡単なお仕事です。

>HIROX
ちょっと論点がわからないので書かれてることにのみ反論。
(1) 張ったURLを読む限りにおいては、1.4.2以前は
正常レスポンス→今と同じ動作
エラーレスポンス→InputStreamにBodyが入る
だと思うんだけど。
(2)isSuccessじゃなくて、isSuccessResponseとか、そういう名前の問題を主張している?
(3)エラーのレスポンスを読むケースが稀ってのは経験則の話をしている?

>pぁ
意図を把握。要は、Responseオブジェクトが返ってくればいいのね。
> getInputStreamは例外投げないで null 返す方がマシと言っているようにみえるのだけれど、それでいて失敗ステータスとして返す値にnullが入ってるのはだめっていうのはよくわからない
普通のオブジェクトにnull使うのはやるけど、Exceptionにnull使うって言うパターンを知らなかったので。
まあ、このあたりはどっちかって言うならInputStreamAndExceptionクラスのメソッドの方で適当にカプセル化してインターフェースとドキュメント用意するのが妥当かもね。
それこそ、成功時にgetExceptionを呼び出したらそのときに別のSuccessResponseExceptionが飛んでくるみたいな。SuccessなのにExceptionとはこれ如何に。

pぁpぁ 2010/02/07 01:39 > 要は、Responseオブジェクトが返ってくればいいのね。
そういうとらえかたもできるかもね。
とはいえ、標準ライブラリにかぶせるラッパーはよほど意味があるとき以外はあんまり厚くない方がいいんじゃないかなと思っていて、ソースみて10秒で理解できるようにできるならそれが望ましい。staticメソッド一個で済むのをわざわざ「オブジェクト指向的に」複雑にしちゃうのって誰が得するんだろ
> それこそ、成功時にgetExceptionを呼び出したらそのときに別のSuccessResponseExceptionが飛んでくるみたいな。SuccessなのにExceptionとはこれ如何に。
死ぬほど使いづらそう
> 普通のオブジェクトにnull使うのはやるけど、Exceptionにnull使うって言うパターンを知らなかったので。
そういう問題かなあ。単に、中で失敗したときExceptionがとれるから、それを失敗ステータス代わりにしてみたっていうだけなんだけど...

あと横レス
> (1) 張ったURLを読む限りにおいては、1.4.2以前は
> 正常レスポンス→今と同じ動作
> エラーレスポンス→InputStreamにBodyが入る
> だと思うんだけど。
貼ってくれたURLによると
> J2SE 1.4 より前では、ファイルタイプが判明しており、応答コードが 400 以上になった場合、FileNotFoundException がスローされます。
であり、かつ
> J2SE 1.4 より前では、getErrorStream は常に null を戻していました。
なので、「ファイルタイプが判明している」ときはエラーのレスポンスを取得する手段がそもそも無かったのでは?

HIROXHIROX 2010/02/07 14:22 なんか反論ありきで事実の認識から違うし何を言っても仕方無さそうなのでもういいです><

(1)はpぁ氏が書いてくれたとおりです。

succeedsucceed 2010/02/08 01:03 んー、まあいいけど。
最初から最後まで、主張はブレさせたつもりないんだけどな。
1. サーバエラーはクライアントの異常じゃねえ。
2. クライアントの異常じゃないのにException投げんな
3. 100歩譲るにせよ、せめてドキュメントに書け
位の話に対して反論ポイントがおかしいと思ってるんだけど。

それはともかく、pぁ氏の案に対してなんで奇妙に感じるかと思ってよく考えてみたら
> とはいえ、標準ライブラリにかぶせるラッパーはよほど意味があるとき以外はあんまり厚くない方がいいんじゃないかなと思っていて
というのを読んでやっとわかった。このクラスが標準クラスの癖に厚くなってないのが問題なんじゃなかろうか。
Responseオブジェクトを返すような実装の標準クラスのラッパとしてHTTPURLConnectionがあるならなんとも思わないし、GJだと思う。
pぁ氏の案は、概念的にはラッパというよりデラッパになってるから、気持ち悪かったんだろうなあ。