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

このコミットは、Go言語の標準ライブラリである net/http パッケージにおける変更の取り消し（リバート）に関するものです。具体的には、以前のコミット 97d027b3aa68 で導入された、クライアントがKeep-Aliveを無効にできるようにする変更が、Windows 64環境でのテスト失敗を引き起こしたため、その変更を元に戻すことが目的です。

コミット

コミットハッシュ: 733b51d996a2b270c2ccfcee149db0583fade879
作者: Gustavo Niemeyer gustavo@niemeyer.net
コミット日時: 2012年4月25日（水）02:32:51 -0300

コミットメッセージ:

net/http: revert 97d027b3aa68

Revert the following change set:

        changeset:   13018:97d027b3aa68
        user:        Gustavo Niemeyer <gustavo@niemeyer.net>
        date:        Mon Apr 23 22:00:16 2012 -0300
        summary:     net/http: allow clients to disable keep-alive

This broke a test on Windows 64 and somebody else
will have to check.

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/6112054

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

https://github.com/golang/go/commit/733b51d996a2b270c2ccfcee149db0583fade879

元コミット内容

このコミットによってリバートされた元のコミット 97d027b3aa68 の内容は、「net/http: クライアントがKeep-Aliveを無効にできるようにする」というものでした。これは、HTTPクライアントが明示的にコネクションを閉じることを要求できるようにするための変更であったと推測されます。

元のコミット 97d027b3aa68 が導入された後、Windows 64ビット環境でのテストが失敗する問題が発生しました。この問題は、特定のプラットフォームでの予期せぬ挙動や、新しい変更が既存のテストスイートと互換性がないことを示唆していました。問題の原因究明と修正に時間がかかるため、一時的に元の状態に戻す（リバートする）ことが決定されました。これは、開発の安定性を保ち、他の開発者がその問題に時間を費やすことなく作業を継続できるようにするための一般的なプラクティスです。コミットメッセージには「somebody else will have to check.」とあり、この問題の調査と修正は別の担当者に委ねられることが示唆されています。

前提知識の解説

HTTP/1.0 と HTTP/1.1

HTTP/1.0: デフォルトでは、各リクエスト/レスポンスのペアごとに新しいTCPコネクションが確立され、レスポンスが完了するとコネクションは閉じられます。これは効率が悪く、特に多数の小さなリソースをロードするウェブページではパフォーマンスのボトルネックとなります。
HTTP/1.1: デフォルトで「持続的接続（Persistent Connection）」または「Keep-Alive」が有効になっています。これにより、単一のTCPコネクション上で複数のリクエスト/レスポンスのやり取りが可能になり、コネクションの確立と切断のオーバーヘッドが削減され、パフォーマンスが向上します。

Keep-Alive と `Connection` ヘッダー

HTTP/1.1では、クライアントとサーバーは Connection ヘッダーを使用してコネクションの振る舞いを制御できます。

Connection: keep-alive: コネクションを持続させることを示します（HTTP/1.0で明示的に指定する場合や、HTTP/1.1でデフォルトの振る舞いを強調する場合）。
Connection: close: 現在のリクエスト/レスポンスの後にコネクションを閉じることを明示的に要求します。

`Expect` ヘッダー

Expect ヘッダーは、クライアントがサーバーに特定のリクエストの処理を期待していることを示すために使用されます。最も一般的な使用例は Expect: 100-continue です。これは、クライアントが大きなリクエストボディを送信する前に、サーバーがそのリクエストを受け入れる準備ができているかどうかを確認するために使用されます。サーバーが 100 Continue レスポンスを返した場合、クライアントはボディの送信を開始します。

Go言語の `net/http` パッケージ

net/http パッケージは、Go言語でHTTPクライアントとサーバーを実装するための基本的な機能を提供します。このパッケージは、HTTPプロトコルの詳細を抽象化し、開発者が簡単にウェブアプリケーションを構築できるようにします。http.Request 構造体は受信したHTTPリクエストを表し、http.ResponseWriter インターフェースはHTTPレスポンスを構築するために使用されます。

技術的詳細

このリバートコミットは、主に net/http パッケージ内のHTTPヘッダーの解析ロジックと、それに関連するコネクション管理の振る舞いを元に戻しています。

元のコミット 97d027b3aa68 では、Request 構造体のメソッドである expectsContinue()、wantsHttp10KeepAlive()、そして新しく追加された wantsClose() の実装が変更されました。これらのメソッドは、HTTPリクエストヘッダー（特に Expect と Connection）を解析し、コネクションの振る舞いを決定するために使用されます。

元の変更では、hasToken というヘルパー関数が導入され、Connection ヘッダーや Expect ヘッダーの値に特定のトークン（例: "keep-alive", "close", "100-continue"）が含まれているかをより汎用的にチェックしようとしました。しかし、コミットメッセージのコメント // TODO This is a poor implementation of the RFC. See http://golang.org/issue/3535 が示すように、この hasToken 関数の実装はRFC（Request for Comments）の仕様に完全に準拠していなかった可能性があります。HTTPヘッダーの解析は、カンマ区切りのリストや引用符で囲まれた文字列など、複雑なルールを持つため、単純な strings.Contains では不十分な場合があります。

このリバートは、hasToken 関数を削除し、以前のより単純な strings.ToLower と == または strings.Contains を使用した比較に戻しています。これにより、元のコミットで導入された潜在的なヘッダー解析の不正確さが解消され、Windows 64でのテスト失敗の原因となっていた問題が回避されたと考えられます。

特に、wantsClose() メソッドの削除は重要です。このメソッドは、クライアントが Connection: close ヘッダーを送信した場合にコネクションを閉じることを意図していましたが、その実装がテスト失敗の原因となった可能性があります。リバートにより、サーバー側のコネクション管理ロジックは、以前の安定した状態に戻されました。

また、テストファイル serve_test.go では、testClientCanClose というテストケースが削除されています。これは、クライアントが Connection: close ヘッダーを送信してコネクションを強制的に閉じることができることを検証するテストでした。このテストの削除は、元のコミットで導入されたクライアントによるKeep-Alive無効化の機能がリバートされたことに伴うものです。

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

このコミットでは、以下の3つのファイルが変更されています。

src/pkg/net/http/request.go
src/pkg/net/http/serve_test.go
src/pkg/net/http/server.go

`src/pkg/net/http/request.go`

--- a/src/pkg/net/http/request.go
+++ b/src/pkg/net/http/request.go
@@ -732,24 +732,12 @@ func (r *Request) FormFile(key string) (multipart.File, *multipart.FileHeader, e
 }

 func (r *Request) expectsContinue() bool {
-	return hasToken(r.Header.Get("Expect"), "100-continue")
+	return strings.ToLower(r.Header.Get("Expect")) == "100-continue"
 }

 func (r *Request) wantsHttp10KeepAlive() bool {
 	if r.ProtoMajor != 1 || r.ProtoMinor != 0 {
 		return false
 	}
-	return hasToken(r.Header.Get("Connection"), "keep-alive")
-}
-
-func (r *Request) wantsClose() bool {
-	return hasToken(r.Header.Get("Connection"), "close")
-}
-
-func hasToken(s, token string) bool {
-	if s == "" {
-		return false
-	}
-	// TODO This is a poor implementation of the RFC. See http://golang.org/issue/3535
-	return strings.Contains(strings.ToLower(s), token)
+	return strings.Contains(strings.ToLower(r.Header.Get("Connection")), "keep-alive")
 }

`src/pkg/net/http/serve_test.go`

--- a/src/pkg/net/http/serve_test.go
+++ b/src/pkg/net/http/serve_test.go
@@ -370,7 +370,7 @@ func TestIdentityResponse(t *testing.T) {
 	})
 }

-func testTCPConnectionCloses(t *testing.T, req string, h Handler) {
+func testTcpConnectionCloses(t *testing.T, req string, h Handler) {
 	s := httptest.NewServer(h)
 	defer s.Close()

@@ -410,28 +410,21 @@ func testTCPConnectionCloses(t *testing.T, req string, h Handler) {

 // TestServeHTTP10Close verifies that HTTP/1.0 requests won't be kept alive.
 func TestServeHTTP10Close(t *testing.T) {
-	testTCPConnectionCloses(t, "GET / HTTP/1.0\\r\\n\\r\\n", HandlerFunc(func(w ResponseWriter, r *Request) {
+	testTcpConnectionCloses(t, "GET / HTTP/1.0\\r\\n\\r\\n", HandlerFunc(func(w ResponseWriter, r *Request) {
 		ServeFile(w, r, "testdata/file")
 	}))
 }

-// TestClientCanClose verifies that clients can also force a connection to close.
-func TestClientCanClose(t *testing.T) {
-	testTCPConnectionCloses(t, "GET / HTTP/1.1\\r\\nConnection: close\\r\\n\\r\\n", HandlerFunc(func(w ResponseWriter, r *Request) {
-		// Nothing.
-	}))
-}
-
 // TestHandlersCanSetConnectionClose verifies that handlers can force a connection to close,
 // even for HTTP/1.1 requests.
 func TestHandlersCanSetConnectionClose11(t *testing.T) {
-	testTCPConnectionCloses(t, "GET / HTTP/1.1\\r\\n\\r\\n", HandlerFunc(func(w ResponseWriter, r *Request) {
+	testTcpConnectionCloses(t, "GET / HTTP/1.1\\r\\n\\r\\n", HandlerFunc(func(w ResponseWriter, r *Request) {
 		w.Header().Set("Connection", "close")
 	}))
 }

 func TestHandlersCanSetConnectionClose10(t *G) {
-	testTCPConnectionCloses(t, "GET / HTTP/1.0\\r\\nConnection: keep-alive\\r\\n\\r\\n", HandlerFunc(func(w ResponseWriter, r *Request) {
+	testTcpConnectionCloses(t, "GET / HTTP/1.0\\r\\nConnection: keep-alive\\r\\n\\r\\n", HandlerFunc(func(w ResponseWriter, r *Request) {
 		w.Header().Set("Connection", "close")
 	}))
 }

`src/pkg/net/http/server.go`

--- a/src/pkg/net/http/server.go
+++ b/src/pkg/net/http/server.go
@@ -303,7 +303,8 @@ func (w *response) WriteHeader(code int) {
 		if !connectionHeaderSet {
 			w.header.Set("Connection", "keep-alive")
 		}
-	} else if !w.req.ProtoAtLeast(1, 1) || w.req.wantsClose() {
+	} else if !w.req.ProtoAtLeast(1, 1) {
+		// Client did not ask to keep connection alive.
 		w.closeAfterReply = true
 	}

コアとなるコードの解説

`src/pkg/net/http/request.go`

expectsContinue() メソッド:
- 変更前: hasToken(r.Header.Get("Expect"), "100-continue") を使用。
- 変更後: strings.ToLower(r.Header.Get("Expect")) == "100-continue" に戻されました。これは、Expect ヘッダーの値が正確に "100-continue" であるかを、大文字小文字を区別せずに比較する以前のロジックです。
wantsHttp10KeepAlive() メソッド:
- 変更前: hasToken(r.Header.Get("Connection"), "keep-alive") を使用。
- 変更後: strings.Contains(strings.ToLower(r.Header.Get("Connection")), "keep-alive") に戻されました。これは、Connection ヘッダーの値に "keep-alive" という文字列が含まれているかを、大文字小文字を区別せずにチェックする以前のロジックです。
wantsClose() メソッドの削除:
- このメソッドは、元のコミットで追加され、Connection: close ヘッダーをチェックするために使用されていましたが、このリバートで完全に削除されました。
hasToken() ヘルパー関数の削除:
- この関数は、元のコミットで導入されましたが、RFCの不完全な実装であるというコメントと共に、このリバートで削除されました。

これらの変更は、HTTPヘッダーの解析ロジックを、よりシンプルで以前の安定した状態に戻すことを目的としています。特に hasToken の削除は、ヘッダー解析の正確性に関する懸念を解消するためと考えられます。

`src/pkg/net/http/serve_test.go`

testTCPConnectionCloses から testTcpConnectionCloses への関数名変更:
- これは、Goの命名規約（エクスポートされない関数は小文字で始まる）に合わせた変更であり、機能的な変更ではありません。
TestClientCanClose テストケースの削除:
- このテストは、クライアントが Connection: close ヘッダーを送信することでコネクションを強制的に閉じることができるかを検証するものでした。元のコミットで導入された「クライアントがKeep-Aliveを無効にできるようにする」機能がリバートされたため、このテストケースも不要となり削除されました。
他のテストケースでの関数名更新:
- TestServeHTTP10Close, TestHandlersCanSetConnectionClose11, TestHandlersCanSetConnectionClose10 の各テストケース内で呼び出されているヘルパー関数名が、testTCPConnectionCloses から testTcpConnectionCloses に更新されています。

`src/pkg/net/http/server.go`

response.WriteHeader メソッド内のコネクション管理ロジックの変更:
- 変更前: !w.req.ProtoAtLeast(1, 1) || w.req.wantsClose()
- 変更後: !w.req.ProtoAtLeast(1, 1)
- この変更により、HTTP/1.1未満のプロトコルを使用している場合にのみ w.closeAfterReply = true が設定されるようになりました。以前は、HTTP/1.1以上であっても w.req.wantsClose() が真であればコネクションが閉じられていましたが、wantsClose() メソッドが削除されたため、この条件も削除されました。これにより、サーバーがコネクションを閉じるかどうかの判断ロジックが、元のシンプルな状態に戻されました。コメント // Client did not ask to keep connection alive. が追加され、この条件がHTTP/1.0クライアント（またはKeep-Aliveを要求しないクライアント）に対するものであることが明確化されています。

参考にした情報源リンク

HTTP/1.0 と HTTP/1.1 の違いに関する一般的な情報源
Go言語の net/http パッケージのドキュメント
RFC 2616 (HTTP/1.1) - 特にヘッダーフィールドの定義に関するセクション
Go issue 3535 (hasTokenに関するTODOコメントで言及されている可能性のあるissue) - このコミットメッセージで言及されている http://golang.org/issue/3535 は、当時のGoのIssueトラッカーのURL形式であり、現在は https://github.com/golang/go/issues/3535 にリダイレクトされる可能性があります。このIssueは、hasToken 関数の実装がRFCに準拠していない可能性について議論しているものと推測されます。

comemo

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

コミット

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

元コミット内容

変更の背景

前提知識の解説

HTTP/1.0 と HTTP/1.1

Keep-Alive と `Connection` ヘッダー

`Expect` ヘッダー

Go言語の `net/http` パッケージ

技術的詳細

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

`src/pkg/net/http/request.go`

`src/pkg/net/http/serve_test.go`

`src/pkg/net/http/server.go`

コアとなるコードの解説

`src/pkg/net/http/request.go`

`src/pkg/net/http/serve_test.go`

`src/pkg/net/http/server.go`

関連リンク

参考にした情報源リンク

Keyboard shortcuts

comemo