Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

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

このコミットは、src/pkg/net/http/serve_test.go ファイルに対する変更です。具体的には、テストコード内の重複を排除し、HTTPボディの送信ロジックをリファクタリングしています。

コミット

net/http: refactor body logic in test

This just eliminates some duplication. Also add a pointer to RFC 1122, in case this comes up again.

R=golang-dev, bradfitz CC=golang-dev https://golang.org/cl/6229044

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

https://github.com/golang/go/commit/1c445755300ddda23a8c2ad6aeee3a98f60f6077

元コミット内容

commit 1c445755300ddda23a8c2ad6aeee3a98f60f6077
Author: Russ Cox <rsc@golang.org>
Date:   Tue May 22 13:46:53 2012 -0400

    net/http: refactor body logic in test
    
    This just eliminates some duplication.
    Also add a pointer to RFC 1122, in case
    this comes up again.
    
    R=golang-dev, bradfitz
    CC=golang-dev
    https://golang.org/cl/6229044
---\n src/pkg/net/http/serve_test.go | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

変更の背景

このコミットの主な目的は、net/http パッケージのテストコード、特に serve_test.go 内のボディ送信ロジックにおける重複を排除することです。既存のコードには、HTTPクライアントが100-continue期待ヘッダを送信しない場合にボディを即座に送信するかどうかを判断するロジックが複数箇所に存在していました。この重複を解消し、コードの可読性と保守性を向上させることを目指しています。

また、TCPのRST（Reset）セグメントに関する挙動、特にサーバーがクライアントからの予期せぬボディ送信に対して接続を切断するシナリオにおいて、RFC 1122への参照を追加しています。これは、将来的に同様の問題が発生した際に、関連する標準ドキュメントを素早く参照できるようにするための配慮です。

前提知識の解説

HTTP 100-continue

HTTP/1.1では、クライアントが大きなリクエストボディを送信する前に、サーバーがそのリクエストを受け入れる準備ができているかを確認するために Expect: 100-continue ヘッダを送信することができます。サーバーは、リクエストを受け入れる準備ができていれば 100 Continue ステータスコードを返し、クライアントはその後ボディの送信を開始します。もしサーバーがリクエストを受け入れられない場合（例: 認証エラー、リクエストが大きすぎるなど）、すぐにエラーレスポンスを返し、クライアントは無駄なボディ送信を避けることができます。

このメカニズムは、特に大きなファイルをアップロードする際などに帯域幅の無駄を減らすのに役立ちます。しかし、すべてのクライアントがこのメカニズムを使用するわけではなく、また、サーバーも常に100-continueをサポートするわけではありません。

TCP RST (Reset) セグメント

TCP (Transmission Control Protocol) は、インターネット上で信頼性の高いデータ転送を提供するプロトコルです。TCP接続は通常、FIN (Finish) フラグを使ったハンドシェイクによって正常に終了します。しかし、何らかの理由で接続が突然終了する必要がある場合、RST (Reset) フラグが設定されたTCPセグメントが送信されます。

RSTセグメントは、以下のような状況で送信されることがあります。

  • 接続拒否: 存在しないポートへの接続試行など。
  • 異常終了: アプリケーションが突然クラッシュしたり、接続を強制的に終了させたりする場合。
  • 半二重クローズ後のデータ受信: RFC 1122で言及されているように、TCP接続をクローズした後に、まだ受信バッファにデータが残っていたり、新しいデータが到着したりした場合、そのデータが失われたことを示すためにRSTが送信されることがあります。これは、特にクライアントがサーバーの許可なくボディを送信し、サーバーがそれを拒否して接続を切断した場合に発生しうる競合状態です。

RFC 1122

RFC 1122は「Requirements for Internet Hosts -- Communication Layers」というタイトルのインターネット標準ドキュメントです。これは、インターネットホストがTCP/IPプロトコルスタックをどのように実装すべきかに関する基本的な要件を定義しています。

特に、ページ88（およびその前のページ87）のセクション4.2.2.13「Closing a Connection」では、TCP接続の終了方法について議論されています。ここで、ホストが「半二重」TCPクローズシーケンスを実装している場合（つまり、CLOSEを呼び出したアプリケーションが接続からデータを読み続けられない場合）、そしてCLOSEが呼び出された後に受信データがまだTCPに残っているか、新しいデータが受信された場合、そのTCPはデータが失われたことを示すためにRSTを送信すべきであると述べられています。

このコミットでは、クライアントがサーバーの許可なくボディを送信し、サーバーが接続を切断した際に発生する可能性のあるTCP競合状態（クライアントがまだデータを書き込んでいる最中にサーバーがRSTを送信する）について、このRFCの記述が関連していることを示唆しています。

技術的詳細

このコミットは、src/pkg/net/http/serve_test.go 内の TestServerExpect 関数におけるHTTPリクエストボディの送信ロジックを簡素化しています。

変更前は、serverExpectTest 構造体に forcedBadBody() というメソッドがあり、これが「サーバーの許可なくボディを送信し、サーバーがそれを拒否することが分かっているテスト」を識別するために使用されていました。このメソッドは、contentLength > 0 かつ readBody が false かつ expectation が 100-continue でない場合に true を返していました。

この forcedBadBody() メソッドは、リクエストボディを書き込むべきかどうかを判断する if 文の条件と、エラーハンドリングの if 文の条件の両方で使われており、ロジックの重複がありました。

今回の変更では、以下の点が改善されています。

  1. forcedBadBody() メソッドの削除: このヘルパー関数は削除されました。
  2. writeBody 変数の導入: リクエストボディを送信すべきかどうかを判断する新しいブール変数 writeBody が導入されました。この変数は、test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue" という条件で初期化されます。これにより、ボディ送信の条件が明確に一箇所で定義され、重複が解消されました。
  3. エラーハンドリングの簡素化: 以前は test.forcedBadBody() を使っていたエラーハンドリングの条件が、writeBody && !test.readBody に変更されました。これは、ボディを送信しようとしたが、サーバーがそれを読み取らなかった（そしておそらく接続を切断した）場合に発生するエラーを捕捉するためのものです。
  4. RFC 1122への参照追加: TCP競合状態によるエラー（クライアントが書き込み中にサーバーが接続を切断する）に関するコメントに、RFC 1122のページ88への参照が追加されました。これにより、この特定の挙動の根拠となる標準ドキュメントが明示されます。

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

--- a/src/pkg/net/http/serve_test.go
+++ b/src/pkg/net/http/serve_test.go
@@ -618,13 +618,6 @@ 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"},
@@ -668,6 +661,11 @@ func TestServerExpect(t *testing.T) {
 			t.Fatalf("Dial: %v", err)
 		}
 		defer conn.Close()
+
+		// Only send the body immediately if we're acting like an HTTP client
+		// that doesn't send 100-continue expectations.
+		writeBody := test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue"
+
 		go func() {
 			_, err := fmt.Fprintf(conn, "POST /?readbody=%v HTTP/1.1\\r\\n"+
 				"Connection: close\\r\\n"+
@@ -678,13 +676,11 @@ func TestServerExpect(t *testing.T) {
 				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" {
+			if writeBody {
 				body := strings.Repeat("A", test.contentLength)
 				_, err = fmt.Fprint(conn, body)
 				if err != nil {
-					if test.forcedBadBody() {
+					if !test.readBody {
 						// Server likely already hung up on us.
 						// See larger comment below.
 						t.Logf("On test %#v, acceptable error writing request body: %v", test, err)
@@ -697,11 +693,12 @@ func TestServerExpect(t *testing.T) {
 		bufr := bufio.NewReader(conn)
 		line, err := bufr.ReadString('\n')
 		if err != nil {
-			if test.forcedBadBody() {
+			if writeBody && !test.readBody {
 				// 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.
+				// See RFC 1122 page 88.
 				t.Logf("On test %#v, acceptable error from ReadString: %v", test, err)
 				return
 			}

コアとなるコードの解説

削除されたコード

  • forcedBadBody() メソッドが完全に削除されました。このメソッドは、特定のテストケースでサーバーがボディを拒否する状況を識別するために使用されていましたが、そのロジックはインライン化され、より汎用的な変数に置き換えられました。

追加・変更されたコード

  1. writeBody 変数の導入:

    		writeBody := test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue"

    この行は、リクエストボディをすぐに送信すべきかどうかを決定する新しいブール変数 writeBody を定義しています。条件は以下の通りです。

    • test.contentLength > 0: リクエストボディの長さが0より大きい場合。
    • strings.ToLower(test.expectation) != "100-continue": クライアントが Expect: 100-continue ヘッダを送信していない場合。 この writeBody 変数により、ボディ送信の条件が明確に一箇所に集約され、コードの重複が解消されました。

  2. ボディ送信の条件変更:

    -			if test.contentLength > 0 && strings.ToLower(test.expectation) != "100-continue" {
+			if writeBody {

    以前はインラインで記述されていたボディ送信の条件が、新しく定義された writeBody 変数に置き換えられました。これにより、コードがより簡潔になり、意図が明確になりました。

  3. エラーハンドリングの条件変更:

    -					if test.forcedBadBody() {
+					if !test.readBody {

    リクエストボディの書き込み中にエラーが発生した場合のハンドリングにおいて、以前は削除された test.forcedBadBody() を使用していましたが、これが !test.readBody に変更されました。これは、テストケースがサーバーにボディを読み取らせることを期待していない場合に、書き込みエラーが許容されることを意味します。これは、サーバーがボディを拒否して接続を切断したために発生する可能性のあるTCP競合状態を考慮したものです。

  4. ReadString エラーハンドリングの条件変更とRFC 1122への参照追加:

    -			if test.forcedBadBody() {
+			if writeBody && !test.readBody {
				// 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.
+				// See RFC 1122 page 88.
				t.Logf("On test %#v, acceptable error from ReadString: %v", test, err)
				return
			}

    bufr.ReadString('\n') でエラーが発生した場合のハンドリングも変更されました。以前は test.forcedBadBody() を使用していましたが、これが writeBody && !test.readBody に変更されました。これは、ボディを送信しようとしたがサーバーがそれを読み取らなかった場合に、TCP競合状態（クライアントが書き込み中にサーバーがRSTを送信する）によって読み取りエラーが発生する可能性があることを示しています。 そして、この挙動の根拠として「See RFC 1122 page 88.」というコメントが追加されました。これは、TCPの実装が、リクエストボディデータが失われたことが判明した場合にRSTを送信し、それがクライアント側の読み取り失敗を引き起こす可能性があるというRFC 1122の記述を指しています。

これらの変更により、テストコードの重複が排除され、ボディ送信とエラーハンドリングのロジックがより明確かつ簡潔になりました。また、特定のネットワーク挙動に関する標準ドキュメントへの参照が追加され、コードの理解が深まりました。

関連リンク

参考にした情報源リンク