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

このコミットは、Go言語の標準ライブラリであるnet/httpパッケージ内のTransportコンポーネントにおける、接続終了に関する警告の精度を向上させるための変更です。具体的には、アイドル状態のHTTPチャネルで予期せぬレスポンスが受信された際の警告ロジックが改善され、以前の脆弱で移植性の低い、そしてos.EINVALの削除によってスパム的になっていた挙動が修正されています。

コミット

commit 130e2943a320f012757518787b0c9dbf182ecb3a
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Tue Dec 6 16:38:02 2011 -0800

    http: make Transport warning about connections closing more accurate
    
    It was fragile and non-portable, and then became spammy with
    the os.EINVAL removal.  Now it just uses the length of the
    Peek return value instead.
    
    R=golang-dev, alex.brainman
    CC=golang-dev
    https://golang.org/cl/5453065

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

https://github.com/golang/go/commit/130e2943a320f012757518787b0c9dbf182ecb3a

元コミット内容

http: make Transport warning about connections closing more accurate

It was fragile and non-portable, and then became spammy with
the os.EINVAL removal. Now it just uses the length of the
Peek return value instead.

変更の背景

このコミットの主な背景には、net/httpパッケージのTransportが、アイドル状態のHTTP接続がリモート側によって閉じられた際に生成する警告メッセージの信頼性と移植性の問題がありました。

以前の実装では、接続が閉じられたことを検出するためにremoteSideClosedというヘルパー関数が使用されていました。この関数は、特定のOS固有のエラーコード（特にWindowsにおけるsyscall.Errno(10058)、つまりWSAECONNRESETに相当）をチェックすることで、リモート側からの接続リセットを判断していました。しかし、このアプローチは以下の問題点を抱えていました。

脆弱性 (Fragile): エラーコードに直接依存するため、OSやGoのバージョンアップによってエラーの挙動が変わると、予期せぬ動作を引き起こす可能性がありました。
非移植性 (Non-portable): syscall.Errno(10058)のような特定のエラーコードはWindowsに固有のものであり、他のOS（Linux, macOSなど）では異なるエラーコードが返されるか、そもそも同様の概念が存在しないため、コードの移植性が損なわれていました。
os.EINVALの削除によるスパム化: Go言語の進化の過程で、http.Transportにおけるos.EINVAL（無効な引数）エラーの扱いが見直され、より汎用的なエラーハンドリングから除外される変更がありました。これにより、以前はos.EINVALが返されていた状況で、remoteSideClosedが正しく機能しなくなり、結果として不要な警告メッセージが大量にログに出力される「スパム的」な挙動が発生するようになりました。これは、開発者にとってノイズとなり、真に重要な警告を見落とす原因となっていました。

これらの問題を解決するため、より堅牢で移植性の高い方法で接続終了を検出する必要がありました。新しいアプローチでは、Peekメソッドの戻り値の長さを利用することで、実際にデータが読み取れるかどうかを判断し、それに基づいて警告を出すかどうかのロジックを決定しています。これにより、OS固有のエラーコードに依存することなく、より正確かつ汎用的に接続状態を判断できるようになりました。また、Windows固有のtransport_windows.goファイルが削除されたことも、この変更の背景にあります。

前提知識の解説

このコミットを理解するためには、以下のGo言語のnet/httpパッケージに関する知識が必要です。

net/httpパッケージ: Go言語の標準ライブラリで、HTTPクライアントとサーバーの実装を提供します。WebアプリケーションやAPIクライアントを構築する際に広く利用されます。
http.Transport: net/httpパッケージの重要なコンポーネントの一つで、HTTPリクエストの実際の送信とHTTPレスポンスの受信を担当します。TCP接続の確立、TLSハンドシェイク、HTTP/1.1のキープアライブ（持続的接続）管理、プロキシの利用などを抽象化します。クライアントが複数のリクエストを同じサーバーに送信する際に、既存のTCP接続を再利用することでパフォーマンスを向上させます。
persistConn: http.Transportの内部で、単一のHTTP/1.1持続的接続（キープアライブ接続）を管理する構造体です。この構造体は、TCP接続のライフサイクル、リクエストとレスポンスの送受信、および接続のアイドル状態の管理を担当します。
bufio.Reader.Peek(n int) ([]byte, error): bufioパッケージのReader型が提供するメソッドです。これは、入力ストリームから最大nバイトを読み込まずに（つまり、内部バッファから）「覗き見」します。Peekは、実際に読み込まれたバイトのスライスと、エラーを返します。エラーがio.EOFの場合、ストリームの終端に達したことを意味します。このコミットでは、Peek(1)を使用して、接続が閉じられたかどうかを判断するために1バイトを覗き見しています。
os.EINVAL: syscallパッケージで定義されているエラーコードの一つで、"invalid argument"（無効な引数）を意味します。システムコールに渡された引数が無効である場合に返されることがあります。Go言語の進化の過程で、特定のコンテキストでのos.EINVALの扱いがより厳密になり、汎用的なエラーハンドリングから除外されることがありました。これが、このコミットで警告が「スパム的」になった原因の一つです。
Unsolicited Response (予期せぬレスポンス): HTTPの文脈では、クライアントがリクエストを送信していないにもかかわらず、サーバーからレスポンスが送られてくる状況を指します。これは通常、プロトコル違反や、接続がアイドル状態であるにもかかわらずサーバーがデータを送信してきた場合に発生します。net/httpのTransportは、このような予期せぬレスポンスを検出した場合に警告をログに出力します。
remoteSideClosed関数: 以前のtransport.goに存在した内部ヘルパー関数で、与えられたエラーがリモート側からの接続終了を示すものかどうかを判断するために使用されていました。この関数は、特にWindows環境での特定のエラーコード（syscall.Errno(10058)）をチェックしていました。

技術的詳細

このコミットの技術的な核心は、net/httpパッケージのTransportがアイドル状態のHTTP接続で予期せぬデータを受信した際の警告ロジックの変更にあります。

変更前は、persistConnのreadLoop内で、pc.br.Peek(1)（バッファリングされたリーダーから1バイトを覗き見する）の呼び出し後にエラーが発生した場合、そのエラーがremoteSideClosed(err)によってリモート側からの接続終了を示すものかどうかを判断していました。もしそうであれば、接続を閉じ、readLoopを終了していました。

// 変更前のコード (抜粋)
		if err != nil {
			if remoteSideClosed(err) && !pc.expectingResponse() {
				// Remote side closed on us.  (We probably hit their
				// max idle timeout)
				pc.close()
				return
			}
		}
		if !pc.expectingResponse() {
			log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
				string(pb), err)
			pc.close()
			return
		}

このremoteSideClosed関数は、特にWindows環境でsyscall.Errno(10058)（WSAECONNRESET）のようなOS固有のエラーをチェックしていました。しかし、この方法は前述の通り、脆弱で移植性が低く、Goの内部変更（特にos.EINVALの扱い）によって、不要な警告が多発する問題を引き起こしていました。

このコミットでは、この脆弱なエラーチェックのロジックが完全に削除されました。代わりに、Peek(1)の戻り値であるバイトスライスpbの長さを直接チェックするようになりました。

// 変更後のコード (抜粋)
		if !pc.expectingResponse() {
			if len(pb) > 0 { // ここが変更点
				log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
					string(pb), err)
			}
			pc.close()
			return
		}

新しいロジックでは、pc.expectingResponse()がfalse（つまり、クライアントが現在レスポンスを期待していないアイドル状態）である場合に、len(pb) > 0という条件が追加されました。これは、Peek(1)が実際に1バイト以上のデータを読み込めた（つまり、バッファにデータが存在した）場合にのみ、予期せぬレスポンスの警告をログに出力するという意味です。

なぜlen(pb) > 0が機能するのか？
- Peek(1)は、ストリームの終端に達した場合や、読み取るべきデータがない場合には、空のスライス（len(pb) == 0）を返します。
- 以前はエラーオブジェクトを解析して接続終了を判断していましたが、新しいアプローチでは、Peekが実際にデータを返したかどうか（len(pb) > 0）を直接確認することで、アイドル接続で予期せぬデータが到着したことをより堅牢に検出します。
- これにより、OS固有のエラーコードに依存することなく、Goのio.Readerインターフェースのセマンティクスに沿った、より汎用的で移植性の高い方法で接続状態を判断できるようになりました。

さらに、この変更に伴い、Windows固有のtransport_windows.goファイルが削除されました。このファイルは、remoteSideClosedFuncという関数を定義しており、Windows特有のエラーハンドリングを提供していました。このコミットでremoteSideClosedロジックが不要になったため、このファイルも削除され、コードベースの簡素化とクロスプラットフォーム対応の強化が図られました。

Makefileの変更は、このtransport_windows.goファイルの削除を反映したもので、GOFILES_windows変数からtransport_windows.goが削除され、それに関連する条件付きコンパイルのロジックも不要になったため削除されています。

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

このコミットによる主要なコード変更は以下の3つのファイルにわたります。

src/pkg/net/http/Makefile
src/pkg/net/http/transport.go
src/pkg/net/http/transport_windows.go (削除)

diff --git a/src/pkg/net/http/Makefile b/src/pkg/net/http/Makefile
index 4bf33a629d..807bc32447 100644
--- a/src/pkg/net/http/Makefile
+++ b/src/pkg/net/http/Makefile
@@ -21,9 +21,4 @@ GOFILES=\
 	transfer.go\
 	transport.go\
 
-GOFILES_windows=\
-	transport_windows.go\
-
-GOFILES+=$(GOFILES_$(GOOS))\
-
 include ../../../Make.pkg
diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go
index e622e41f0a..dc70be43f2 100644
--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -519,17 +519,11 @@ func (pc *persistConn) readLoop() {
 
 	for alive {
 		pb, err := pc.br.Peek(1)
-		if err != nil {
-			if remoteSideClosed(err) && !pc.expectingResponse() {
-				// Remote side closed on us.  (We probably hit their
-				// max idle timeout)
-				pc.close()
-				return
-			}
-		}
 		if !pc.expectingResponse() {
-			log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
-				string(pb), err)
+			if len(pb) > 0 {
+				log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
+					string(pb), err)
+			}
 			pc.close()
 			return
 		}
diff --git a/src/pkg/net/http/transport_windows.go b/src/pkg/net/http/transport_windows.go
deleted file mode 100644
index c9ef2c2ab6..0000000000
--- a/src/pkg/net/http/transport_windows.go
+++ /dev/null
@@ -1,21 +0,0 @@
-// Copyright 2011 The Go Authors. All rights reserved.\n-// Use of this source code is governed by a BSD-style\n-// license that can be found in the LICENSE file.\n-\n-package http\n-\n-import (\n-\t\"net\"\n-\t\"syscall\"\n-)\n-\n-func init() {\n-\tremoteSideClosedFunc = func(err error) (out bool) {\n-\t\top, ok := err.(*net.OpError)\n-\t\tif ok && op.Op == \"WSARecv\" && op.Net == \"tcp\" && op.Err == syscall.Errno(10058) {\n-\t\t\t// TODO(brainman,rsc): Fix whatever is generating this.\n-\t\t\treturn true\n-\t\t}\n-\t\treturn false\n-\t}\n-}

コアとなるコードの解説

`src/pkg/net/http/transport.go` の変更

transport.goのpersistConn構造体のreadLoopメソッドが変更されています。このメソッドは、持続的接続上でレスポンスを読み取るためのループです。

変更前は、pc.br.Peek(1)（バッファから1バイトを覗き見する）の呼び出し後にエラーが発生した場合、そのエラーがremoteSideClosed(err)関数によってリモート側からの接続終了を示すものかどうかをチェックしていました。もしそうであれば、接続を閉じ、readLoopを終了していました。

// 変更前のコード
		pb, err := pc.br.Peek(1)
		if err != nil { // Peekでエラーが発生した場合
			if remoteSideClosed(err) && !pc.expectingResponse() { // remoteSideClosedで接続終了と判断され、かつレスポンスを期待していない場合
				pc.close() // 接続を閉じる
				return     // readLoopを終了
			}
		}
		if !pc.expectingResponse() { // レスポンスを期待していないアイドル状態の場合
			log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
				string(pb), err) // 予期せぬレスポンスの警告をログに出力
			pc.close()
			return
		}

変更後では、このif err != nilブロック全体が削除されました。これにより、remoteSideClosed関数への依存がなくなりました。

// 変更後のコード
		pb, err := pc.br.Peek(1) // Peekの呼び出しは変わらず
		// 以前の if err != nil ブロックが削除された
		if !pc.expectingResponse() { // レスポンスを期待していないアイドル状態の場合
			if len(pb) > 0 { // ここが新しい条件: Peekが実際にデータを返した場合のみ
				log.Printf("Unsolicited response received on idle HTTP channel starting with %q; err=%v",
					string(pb), err) // 予期せぬレスポンスの警告をログに出力
			}
			pc.close()
			return
		}

新しいロジックでは、pc.expectingResponse()がfalse（アイドル状態）である場合に、Peek(1)の戻り値であるpbスライスのlen(pb) > 0という条件が追加されました。これは、Peekが実際に1バイト以上のデータを読み込めた場合にのみ、予期せぬレスポンスの警告をログに出力するという意味です。これにより、エラーオブジェクトの解析に依存せず、より直接的にデータ受信の有無を判断できるようになりました。

`src/pkg/net/http/Makefile` の変更

Makefileの変更は、transport_windows.goファイルの削除を反映したものです。

-GOFILES_windows=\
-	transport_windows.go\
-
-GOFILES+=$(GOFILES_$(GOOS))\

上記の行が削除されました。これは、Windows固有のソースファイルtransport_windows.goをGOFILES変数に追加するための設定でした。このファイルが削除されたため、関連するMakefileのエントリも不要となり、削除されました。これにより、ビルドプロセスが簡素化され、クロスプラットフォーム対応がよりクリーンになりました。

`src/pkg/net/http/transport_windows.go` の削除

このファイルは完全に削除されました。

deleted file mode 100644

このファイルには、Windows固有のremoteSideClosedFuncの実装が含まれていました。この関数は、net.OpErrorをチェックし、特にWSARecv操作でsyscall.Errno(10058)（WSAECONNRESET）エラーが発生した場合にtrueを返すことで、リモート側からの接続リセットを検出していました。

transport.goの変更により、remoteSideClosed関数（およびそのWindows固有の実装）が不要になったため、このファイルは削除されました。これにより、Goのnet/httpパッケージは、OS固有のエラーハンドリングから解放され、より汎用的で移植性の高いコードベースになりました。

comemo