[インデックス 13122] ファイルの概要

このコミットは、Go言語の標準ライブラリ net/http パッケージ内の TestServerExpect テストの改善を目的としています。具体的には、テストの失敗時のメッセージをより有用なものにし、エラーが許容される状況では Errorf の代わりに Logf を使用することで、テストの堅牢性とデバッグのしやすさを向上させています。

コミット

commit 5a0333764b1de9c46b2e7fec4cb31a8cadeedb0b
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Tue May 22 10:27:34 2012 -0700

    net/http: improve TestServerExpect
    
    Fail more usefully, and Logf in one place instead of Errorf where
    an error is acceptable.
    
    R=golang-dev, rsc
    CC=golang-dev
    https://golang.org/cl/6221059

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/5a0333764b1de9c46b2e7fec4cb31a8cadeedb0b

元コミット内容

net/http: improve TestServerExpect

Fail more usefully, and Logf in one place instead of Errorf where
an error is acceptable.

変更の背景

このコミットの背景には、net/http パッケージの TestServerExpect テストが、特定のシナリオ、特にクライアントがサーバーの許可なくボディを送信しようとする（HTTP 100-continue 期待ヘッダを送信しないにもかかわらず、大きなボディを即座に送信する）場合に、誤解を招くエラーや不必要なテスト失敗を引き起こす可能性があったという問題があります。

従来のテストでは、このような「不正なボディ」の送信中にサーバーが接続を切断した場合、クライアント側での書き込みエラーや読み取りエラーが t.Fatalf や t.Errorf として報告されていました。しかし、これはTCPの競合状態や、サーバーが意図的に接続を切断した結果として発生する、許容されるべきエラーである場合があります。例えば、サーバーがクライアントからの予期せぬ大量のデータ受信を拒否するために、早期に接続を終了することがあります。この場合、クライアント側で発生する書き込みエラーや読み取りエラーは、テストの失敗として扱われるべきではありません。

このコミットは、このような許容されるエラーを t.Logf で記録するように変更し、本当にテストが失敗すべき状況でのみ t.Fatalf や t.Errorf を使用することで、テストの信頼性とデバッグの効率を向上させています。これにより、開発者はテスト結果から、実際に修正が必要な問題と、許容される動作の結果として発生するエラーとを区別しやすくなります。

前提知識の解説

HTTP 100-continue

HTTP/1.1 プロトコルでは、クライアントが大きなリクエストボディを送信する前に、サーバーがそのリクエストを受け入れる準備ができているかを確認するために Expect: 100-continue ヘッダを使用できます。

1. クライアントの動作: クライアントはまず、リクエストヘッダと Expect: 100-continue ヘッダをサーバーに送信します。リクエストボディはまだ送信しません。
2. サーバーの応答:
   • サーバーがリクエストを受け入れる準備ができている場合、100 Continue ステータスコードを返します。クライアントはこの応答を受け取った後、リクエストボディの送信を開始します。
   • サーバーがリクエストを受け入れない場合（例: 認証失敗、リクエストが大きすぎるなど）、4xx または 5xx のエラーコードを返します。クライアントはこの応答を受け取った後、リクエストボディの送信を中止します。
3. 利点: 大きなリクエストボディを無駄に送信する前に、サーバーがそれを受け入れるかどうかを確認できるため、ネットワーク帯域幅の節約や、サーバーリソースの無駄な消費を防ぐことができます。

このコミットでは、クライアントが 100-continue を期待せずにボディを送信するシナリオ（test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue"）が特に重要になります。この場合、クライアントはサーバーの許可を待たずにボディを送信するため、サーバーが早期に接続を切断する可能性があり、それがクライアント側でのエラーとして現れることがあります。

Go言語のテストにおけるエラー報告 (t.Fatalf, t.Errorf, t.Logf)

Go言語の testing パッケージは、テスト関数内でエラーを報告するためのいくつかのメソッドを提供します。

• t.Fatalf(format string, args ...interface{}):
  • 致命的なエラーを報告し、現在のテスト関数を即座に終了させます。
  • テストがこれ以上続行できないほど深刻な問題が発生した場合に使用します。
  • このメソッドが呼び出されると、テストは失敗とマークされ、そのテスト関数はそこで停止します。
• t.Errorf(format string, args ...interface{}):
  • エラーを報告しますが、現在のテスト関数は終了させません。
  • テストが続行可能であるが、何らかの期待される条件が満たされなかった場合に使用します。
  • テストは失敗とマークされますが、残りのテストコードは実行され続けます。
• t.Logf(format string, args ...interface{}):
  • テストの実行中に情報をログに出力します。
  • テストの成功/失敗には影響しません。
  • デバッグ情報や、テストの実行フローに関する詳細を記録するために使用します。
  • このコミットでは、許容されるエラーや、テストの失敗と見なすべきではない状況での情報記録に利用されています。

このコミットの変更は、これらのメソッドの適切な使い分けによって、テストの意図をより正確に反映させ、デバッグ体験を向上させることを目的としています。

TCP競合状態 (TCP Race Condition)

TCP競合状態とは、TCP接続において、クライアントとサーバーが同時に特定の操作（例: データの送受信、接続の切断）を行おうとした際に発生する可能性のある状況を指します。

このコミットの文脈では、クライアントがリクエストボディを送信している最中に、サーバーが接続を切断する（例えば、クライアントが 100-continue を期待せずに大きなボディを送信し始めたため、サーバーがそれを拒否して接続を閉じる）というシナリオが考えられます。この場合、クライアントがボディの書き込みを完了する前にサーバーが接続を切断すると、クライアント側で「書き込みエラー」や「読み取りエラー」（サーバーからの応答を待っている間に接続が切断されたため）が発生する可能性があります。

特に、TCPの実装によっては、リクエストボディのデータが失われたと判断された場合にRST（Reset）パケットを送信することがあり、これがクライアント側の読み取りエラーを引き起こすことがあります。このようなエラーは、プロトコル上の問題ではなく、TCPの動作の自然な結果であるため、テストの失敗として扱うべきではありません。このコミットは、このような許容されるTCP競合状態によるエラーを適切に処理し、テストの誤った失敗を防ぐためのものです。

技術的詳細

このコミットは、net/http/serve_test.go ファイル内の TestServerExpect 関数に焦点を当て、HTTPの Expect: 100-continue ヘッダの動作をテストする際の堅牢性を向上させています。主な変更点は以下の通りです。

1. forcedBadBody() ヘルパーメソッドの追加:

   • serverExpectTest 構造体に forcedBadBody() メソッドが追加されました。
   • このメソッドは、現在のテストケースが「サーバーの許可なく（100-continue 期待なしに）ボディを送信し、サーバーがそれを拒否することが分かっている」状況であるかどうかを判定します。
   • 具体的には、contentLength > 0 (ボディがある) かつ !readBody (ボディを読み込まない設定) かつ strings.ToLower(t.expectation) != "100-continue" (100-continueを期待しない) の場合に true を返します。
   • このメソッドの導入により、特定のテストシナリオ（サーバーが早期に接続を切断する可能性がある状況）を明確に識別できるようになりました。

2. リクエストボディ書き込み時のエラーハンドリングの改善:

   • 以前は、リクエストボディの書き込み中にエラーが発生した場合、t.Fatalf を使用してテストを即座に終了させていました。
   • 変更後、fmt.Fprint(conn, body) のエラーチェックにおいて、test.forcedBadBody() が true の場合、つまりサーバーが接続を切断する可能性のある状況では、t.Logf を使用してエラーをログに記録するようになりました。これは、サーバーがすでに接続を切断しているために書き込みが失敗した可能性があり、それが許容されるエラーであるためです。
   • それ以外の状況で書き込みエラーが発生した場合は、引き続き t.Errorf を使用してエラーを報告しますが、テストは続行されます。

3. 応答読み取り時のエラーハンドリングの改善:

   • 以前は、bufr.ReadString('\n') でエラーが発生した場合、t.Fatalf を使用してテストを即座に終了させていました。
   • 変更後、test.forcedBadBody() が true の場合、つまりTCP競合状態によって読み取りエラーが発生する可能性がある状況では、t.Logf を使用してエラーをログに記録するようになりました。これは、クライアントがデータを書き込んでいる最中にサーバーが接続を切断したために読み取りが失敗した可能性があり、それが許容されるエラーであるためです。
   • それ以外の状況で読み取りエラーが発生した場合は、引き続き t.Fatalf を使用してテストを即座に終了させます。

4. エラーメッセージの改善:

   • t.Errorf のメッセージがより詳細になり、期待される応答と実際に受け取った応答の両方を表示するようになりました。これにより、テストが失敗した場合のデバッグが容易になります。

これらの変更により、TestServerExpect は、HTTP 100-continue の動作をより正確にテストできるようになり、特にサーバーが早期に接続を切断するようなエッジケースにおいても、誤ったテスト失敗を減らし、テスト結果の解釈を容易にしています。

コアとなるコードの変更箇所

src/pkg/net/http/serve_test.go ファイルにおいて、以下の変更が行われました。

1. serverExpectTest 構造体へのメソッド追加:

   --- a/src/pkg/net/http/serve_test.go
   +++ b/src/pkg/net/http/serve_test.go
   @@ -618,6 +618,13 @@ type serverExpectTest struct {
    	expectedResponse string // expected substring in first line of http response
    }
    
   +// forcedBadBody returns whether this test sends an unsolicited body
   +// without asking the server's permission and which we know the server
   +// will deny (possibly before we finish writing the body).
   +func (t serverExpectTest) forcedBadBody() bool {
   +	return t.contentLength > 0 && !t.readBody && strings.ToLower(t.expectation) != "100-continue"
   +}
   +
    var serverExpectTests = []serverExpectTest{
    	// Normal 100-continues, case-insensitive.
    	{100, "100-continue", true, "100 Continue"},
   

2. TestServerExpect 関数内のエラーハンドリングの変更:

   --- a/src/pkg/net/http/serve_test.go
   +++ b/src/pkg/net/http/serve_test.go
   @@ -661,30 +668,47 @@ func TestServerExpect(t *testing.T) {
    		t.Fatalf("Dial: %v", err)
    	}
    	defer conn.Close()
   -		sendf := func(format string, args ...interface{}) {
   -			_, err := fmt.Fprintf(conn, format, args...)
   -			if err != nil {
   -				t.Fatalf("On test %#v, error writing %q: %v", test, format, err)
   -			}
   -		}
    	go func() {
   -			sendf("POST /?readbody=%v HTTP/1.1\r\n"+
   +			_, err := fmt.Fprintf(conn, "POST /?readbody=%v HTTP/1.1\r\n"+
    				"Connection: close\r\n"+
    				"Content-Length: %d\r\n"+
    				"Expect: %s\r\nHost: foo\r\n\r\n",
    				test.readBody, test.contentLength, test.expectation)
   +			if err != nil {
   +				t.Errorf("On test %#v, error writing request headers: %v", test, err)
   +				return
   +			}
   +			// Only send the body immediately if we're acting like an HTTP client
   +			// that doesn't send 100-continue expectations.
    		if test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue" {
    			body := strings.Repeat("A", test.contentLength)
   -				sendf(body)
   +				_, err = fmt.Fprint(conn, body)
   +				if err != nil {
   +					if test.forcedBadBody() {
   +						// Server likely already hung up on us.
   +						// See larger comment below.
   +						t.Logf("On test %#v, acceptable error writing request body: %v", test, err)
   +						return
   +					}
   +					t.Errorf("On test %#v, error writing request body: %v", test, err)
   +				}
    		}
    	}()
    	bufr := bufio.NewReader(conn)
    	line, err := bufr.ReadString('\n')
    	if err != nil {
   -			t.Fatalf("ReadString: %v", err)
   +			if test.forcedBadBody() {
   +				// This is an acceptable failure due to a possible TCP race:
   +				// We were still writing data and the server hung up on us. A TCP
   +				// implementation may send a RST if our request body data was known
   +				// to be lost, which may trigger our reads to fail.
   +				t.Logf("On test %#v, acceptable error from ReadString: %v", test, err)
   +				return
   +			}
   +			t.Fatalf("On test %#v, ReadString: %v", test, err)
    	}
    	if !strings.Contains(line, test.expectedResponse) {
   -			t.Errorf("for test %#v got first line=%q", test, line)
   +			t.Errorf("On test %#v, got first line = %q; want %q", test, line, test.expectedResponse)
    	}
    }
   

コアとなるコードの解説

forcedBadBody() メソッド

func (t serverExpectTest) forcedBadBody() bool {
	return t.contentLength > 0 && !t.readBody && strings.ToLower(t.expectation) != "100-continue"
}

このメソッドは、テストケースが「サーバーの許可なく（Expect: 100-continue ヘッダなしに）リクエストボディを送信し、サーバーがそれを拒否する可能性がある」という特定のシナリオに該当するかどうかを判定します。

• t.contentLength > 0: リクエストボディが存在することを示します。
• !t.readBody: サーバーがリクエストボディを読み込まない設定であることを示します。これは、サーバーがボディを必要としない、または早期に接続を切断する可能性があることを示唆します。
• strings.ToLower(t.expectation) != "100-continue": クライアントが Expect: 100-continue ヘッダを送信していないことを示します。つまり、クライアントはサーバーの許可を待たずにボディを送信しようとしています。

これらの条件がすべて満たされる場合、このテストケースは「不正なボディ」を強制的に送信しており、サーバーが接続を早期に切断する可能性が高いと判断されます。

リクエストボディ書き込み時のエラーハンドリング

			_, err := fmt.Fprintf(conn, "POST /?readbody=%v HTTP/1.1\r\n"+
				"Connection: close\r\n"+
				"Content-Length: %d\r\n"+
				"Expect: %s\r\nHost: foo\r\n\r\n",
				test.readBody, test.contentLength, test.expectation)
			if err != nil {
				t.Errorf("On test %#v, error writing request headers: %v", test, err)
				return
			}
			// Only send the body immediately if we're acting like an HTTP client
			// that doesn't send 100-continue expectations.
			if test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue" {
				body := strings.Repeat("A", test.contentLength)
				_, err = fmt.Fprint(conn, body)
				if err != nil {
					if test.forcedBadBody() {
						// Server likely already hung up on us.
						// See larger comment below.
						t.Logf("On test %#v, acceptable error writing request body: %v", test, err)
						return
					}
					t.Errorf("On test %#v, error writing request body: %v", test, err)
				}
			}

この部分では、リクエストヘッダとボディの書き込みが行われます。

• ヘッダ書き込み: ヘッダの書き込みでエラーが発生した場合は、t.Errorf でエラーを報告し、テスト関数を終了します。これは、ヘッダの書き込みは通常成功すべきであり、失敗は深刻な問題を示すためです。
• ボディ書き込み: test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue" の条件が真の場合、つまり 100-continue を期待せずにボディを即座に送信する場合に、ボディの書き込みが行われます。
  • ボディの書き込みでエラーが発生した場合、まず test.forcedBadBody() をチェックします。
  • test.forcedBadBody() が true の場合（サーバーが早期に接続を切断する可能性のあるシナリオ）、t.Logf を使用してエラーをログに記録します。これは、サーバーがすでに接続を切断しているために書き込みが失敗した可能性があり、それが許容されるエラーであるためです。return でこのゴルーチンを終了します。
  • それ以外の場合（予期せぬボディ書き込みエラー）、t.Errorf を使用してエラーを報告します。

応答読み取り時のエラーハンドリング

		bufr := bufio.NewReader(conn)
		line, err := bufr.ReadString('\n')
		if err != nil {
			if test.forcedBadBody() {
				// This is an acceptable failure due to a possible TCP race:
				// We were still writing data and the server hung up on us. A TCP
				// implementation may send a RST if our request body data was known
				// to be lost, which may trigger our reads to fail.
				t.Logf("On test %#v, acceptable error from ReadString: %v", test, err)
				return
			}
			t.Fatalf("On test %#v, ReadString: %v", test, err)
		}

この部分では、サーバーからの応答の最初の行を読み取ります。

• bufr.ReadString('\n') でエラーが発生した場合、まず test.forcedBadBody() をチェックします。
• test.forcedBadBody() が true の場合（TCP競合状態によって読み取りエラーが発生する可能性のあるシナリオ）、t.Logf を使用してエラーをログに記録します。これは、クライアントがデータを書き込んでいる最中にサーバーが接続を切断したために読み取りが失敗した可能性があり、それが許容されるエラーであるためです。return でテスト関数を終了します。
• それ以外の場合（予期せぬ読み取りエラー）、t.Fatalf を使用してエラーを報告し、テストを即座に終了させます。

エラーメッセージの改善

		if !strings.Contains(line, test.expectedResponse) {
			t.Errorf("On test %#v, got first line = %q; want %q", test, line, test.expectedResponse)
		}

応答の最初の行が期待される文字列を含んでいない場合のエラーメッセージが改善されました。以前は t.Errorf("for test %#v got first line=%q", test, line) でしたが、変更後は t.Errorf("On test %#v, got first line = %q; want %q", test, line, test.expectedResponse) となり、期待される応答も表示されるようになりました。これにより、テストが失敗した際に何が期待され、何が実際に得られたのかが明確になり、デバッグが容易になります。

関連リンク

• HTTP/1.1: Protocol Parameters - 10.1.1 100 Continue
• Go言語 testing パッケージのドキュメント

参考にした情報源リンク

• Go言語の公式ドキュメント
• HTTP/1.1 RFC 2616
• TCP/IPに関する一般的な知識