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

このコミットは、Go言語の標準ライブラリ bufio パッケージにおける潜在的な無限ループのバグを修正するものです。具体的には、bufio.Reader の ReadByte メソッドが、基盤となる io.Reader がバイトを返さない場合に無限ループに陥る問題を解決します。

コミット

bufio: fix potential endless loop in ReadByte

Fixes #7745.

LGTM=bradfitz, r
R=r, bradfitz
CC=golang-codereviews
https://golang.org/cl/86220044

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

https://github.com/golang/go/commit/b38fba21f01558ac4834137f35af3b2a42aef4d1

元コミット内容

commit b38fba21f01558ac4834137f35af3b2a42aef4d1
Author: Robert Griesemer <gri@golang.org>
Date:   Wed Apr 9 17:53:09 2014 -0700

    bufio: fix potential endless loop in ReadByte
    
    Fixes #7745.
    
    LGTM=bradfitz, r
    R=r, bradfitz
    CC=golang-codereviews
    https://golang.org/cl/86220044
---
 src/pkg/bufio/bufio.go      | 26 ++++++++++++++++++--------
 src/pkg/bufio/bufio_test.go | 29 +++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 8 deletions(-)

変更の背景

このコミットは、Go言語の bufio パッケージにおける重要なバグ、具体的には Issue #7745 で報告された問題を修正するために行われました。この問題は、bufio.Reader の ReadByte() メソッドが、ラップしている基盤の io.Reader がバイトを返さず、かつエラーも返さない場合に、無限ループに陥るというものでした。

通常、io.Reader の Read メソッドは、データを読み込めない場合に (0, io.EOF) または (0, someError) を返すことが期待されます。しかし、一部の不正な、または特殊な io.Reader の実装では、Read が (0, nil) を返し続けることがありました。bufio.Reader は内部バッファが空になった際に fill() メソッドを呼び出して基盤のリーダーからデータを読み込もうとしますが、この (0, nil) のケースでは fill() が成功したと判断し、バッファが満たされないまま無限に Read を呼び出し続ける状態に陥っていました。

この無限ループは、プログラムがハングアップし、応答しなくなる原因となり、非常に深刻な問題でした。この修正は、このような不正な io.Reader の振る舞いに対処し、bufio.Reader の堅牢性を向上させることを目的としています。

前提知識の解説

bufio パッケージ: Go言語の標準ライブラリの一部で、I/O操作をバッファリングすることで効率を向上させるための機能を提供します。bufio.Reader は、io.Reader インターフェースを実装する任意のデータソース（ファイル、ネットワーク接続など）からデータをバッファリングして読み込むための構造体です。
io.Reader インターフェース: Go言語における基本的なI/Oインターフェースの一つです。Read(p []byte) (n int, err error) メソッドを定義しており、p に最大 len(p) バイトのデータを読み込み、読み込んだバイト数 n とエラー err を返します。
- n > 0: データを読み込んだ場合。
- n == 0, err == nil: データを読み込めなかったが、エラーも発生していない場合。これは通常、一時的な状況（例: ノンブロッキングI/Oでデータがまだ利用可能でない）を示しますが、不正なリーダーでは無限に続く可能性があります。
- n == 0, err == io.EOF: ストリームの終端に達した場合。
- n == 0, err != nil: エラーが発生した場合。
bufio.Reader.ReadByte() メソッド: bufio.Reader のメソッドで、バッファから1バイトを読み込みます。バッファが空の場合、内部的に fill() メソッドを呼び出して基盤の io.Reader からデータを補充しようとします。
無限ループ (Endless Loop): プログラムの実行フローが、終了条件を満たさないために同じコードブロックを繰り返し実行し続ける状態です。この場合、bufio.Reader がデータを読み込めないにもかかわらず、エラーも発生しないために、永遠にデータを読み込もうとし続ける状態を指します。
io.ErrNoProgress: io パッケージで定義されているエラー変数です。これは、io.Reader が Read 呼び出しでバイトを読み込まず、かつエラーも返さない状態が一定回数続いた場合に、進捗がないことを示すために使用されます。このエラーは、無限ループを防ぐためのメカニズムとして導入されました。

技術的詳細

このバグは、bufio.Reader の fill() メソッドの動作に起因していました。fill() メソッドは、bufio.Reader の内部バッファが空になったときに、基盤の io.Reader からデータを読み込んでバッファを補充する役割を担っています。

修正前の fill() メソッドは、基盤の io.Reader の Read メソッドが n=0, err=nil を返した場合、それを「成功したがデータはなかった」と解釈し、特にエラー処理を行いませんでした。このため、Read が常に (0, nil) を返すような不正なリーダーに対しては、fill() は無限に Read を呼び出し続け、結果として ReadByte() などのメソッドがハングアップする原因となっていました。

このコミットによる修正は、この問題を解決するために以下の主要な変更を導入しました。

maxConsecutiveEmptyReads 定数の導入: bufio パッケージ内に maxConsecutiveEmptyReads という新しい内部定数（値は998）が導入されました。これは、基盤の io.Reader が連続して (0, nil) を返すことを許容する最大回数を定義します。
fill() メソッド内のループ: fill() メソッド内で、基盤の io.Reader からデータを読み込む処理が for ループで囲まれ、maxConsecutiveEmptyReads の回数だけ試行されるようになりました。
- ループ内で n, err := b.rd.Read(b.buf[b.w:]) を呼び出します。
- n < 0 の場合は panic(errNegativeRead) でパニックを起こします（これは不正なリーダーの振る舞いです）。
- err != nil の場合は、そのエラーを b.err に設定して fill() を終了します。
- n > 0 の場合は、データが読み込まれたことを意味するため、ループを終了して fill() を正常に完了します。
- 重要な変更点: n == 0 かつ err == nil の場合、ループは継続し、i の値がデクリメントされます。これにより、連続して (0, nil) が返される回数がカウントされます。
io.ErrNoProgress の導入: for ループが maxConsecutiveEmptyReads 回の試行を使い果たしても、n > 0 とならず、かつエラーも発生しなかった場合（つまり、基盤のリーダーが (0, nil) を返し続けた場合）、b.err に io.ErrNoProgress が設定されます。これにより、bufio.Reader は無限ループに陥ることなく、進捗がないことを呼び出し元にエラーとして通知できるようになります。
Read メソッドの修正: bufio.Reader の Read メソッド内でも、直接 b.rd.Read(p) を呼び出す箇所で n < 0 のチェックが追加され、パニックを発生させるようになりました。これは fill() と同様に、不正なリーダーの振る舞いに対する防御的なチェックです。
テストケースの追加: bufio_test.go に TestZeroReader という新しいテストケースが追加されました。このテストは、常に (0, nil) を返す zeroReader というダミーの io.Reader を作成し、bufio.Reader がこれを使用した場合に無限ループに陥らず、io.ErrNoProgress を返すことを検証します。タイムアウト機構も組み込まれており、無限ループが発生した場合にはテストが失敗するように設計されています。

これらの変更により、bufio.Reader は、基盤の io.Reader が不正な振る舞い（Read が常に (0, nil) を返す）をした場合でも、無限ループに陥ることなく、適切なエラー (io.ErrNoProgress) を返すようになりました。

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

このコミットによる主要なコード変更は以下のファイルで行われました。

src/pkg/bufio/bufio.go:
- fill() メソッドのロジックが大幅に変更されました。
  - maxConsecutiveEmptyReads 定数（const maxConsecutiveEmptyReads = 998）が追加されました。
  - データを読み込む b.rd.Read の呼び出しが for ループで囲まれ、maxConsecutiveEmptyReads 回の試行制限が導入されました。
  - ループが終了してもデータが読み込まれなかった場合、b.err = io.ErrNoProgress が設定されるようになりました。
- Read メソッド内で、直接 b.rd.Read(p) を呼び出す箇所に if n < 0 { panic(errNegativeRead) } のチェックが追加されました。
src/pkg/bufio/bufio_test.go:
- TestZeroReader という新しいテスト関数が追加されました。
  - このテストは、常に (0, nil) を返す zeroReader というカスタム io.Reader を定義します。
  - bufio.Reader が zeroReader をラップした場合に ReadByte() が io.ErrNoProgress を返すことを検証します。
  - テストがタイムアウトしないように、time.After を使用して無限ループを検出するメカニズムが組み込まれています。

コアとなるコードの解説

`src/pkg/bufio/bufio.go` の変更点

// bufio.go の fill() メソッドの変更箇所
@@ -88,15 +88,22 @@ func (b *Reader) fill() {
 		b.r = 0
 	}

-	// Read new data.
-	n, err := b.rd.Read(b.buf[b.w:])
-	if n < 0 {
-		panic(errNegativeRead)
-	}
-	b.w += n
-	if err != nil {
-		b.err = err
+	// Read new data: try a limited number of times.
+	for i := maxConsecutiveEmptyReads; i > 0; i-- {
+		n, err := b.rd.Read(b.buf[b.w:])
+		if n < 0 {
+			panic(errNegativeRead)
+		}
+		b.w += n
+		if err != nil {
+			b.err = err
+			return
+		}
+		if n > 0 {
+			return
+		}
 	}
+	b.err = io.ErrNoProgress
 }

 func (b *Reader) readErr() error {
@@ -151,6 +158,9 @@ func (b *Reader) Read(p []byte) (n int, err error) {
 			// Large read, empty buffer.
 			// Read directly into p to avoid copy.
 			n, b.err = b.rd.Read(p)
+		if n < 0 {
+			panic(errNegativeRead)
+		}
 		if n > 0 {
 			b.lastByte = int(p[n-1])
 			b.lastRuneSize = -1

fill() メソッド:
- for i := maxConsecutiveEmptyReads; i > 0; i-- ループが導入されました。maxConsecutiveEmptyReads は 998 に設定されており、これは基盤のリーダーが連続して (0, nil) を返すことを許容する最大回数です。
- ループ内で b.rd.Read を呼び出し、n (読み込んだバイト数) と err (エラー) を取得します。
- if n < 0 { panic(errNegativeRead) }: Read メソッドが負のバイト数を返すのは不正な振る舞いであるため、パニックを発生させます。
- b.w += n: 読み込んだバイト数をバッファの書き込みポインタ b.w に加算します。
- if err != nil { b.err = err; return }: エラーが発生した場合は、そのエラーを b.err に設定し、fill() を終了します。
- if n > 0 { return }: 1バイトでもデータが読み込まれた場合は、バッファが補充されたとみなし、fill() を正常に終了します。
- ループが maxConsecutiveEmptyReads 回繰り返されても n > 0 とならなかった場合（つまり、基盤のリーダーが (0, nil) を返し続けた場合）、ループを抜けた後に b.err = io.ErrNoProgress が設定されます。これにより、無限ループを防ぎ、進捗がないことを呼び出し元に通知します。
Read メソッド:
- n, b.err = b.rd.Read(p) の呼び出しの直後に if n < 0 { panic(errNegativeRead) } が追加されました。これは fill() と同様に、不正な io.Reader の振る舞いに対する防御的なチェックです。

`src/pkg/bufio/bufio_test.go` の変更点

// bufio_test.go の TestZeroReader の追加箇所
@@ -14,6 +14,7 @@ import (
 	"strings"
 	"testing"
 	"testing/iotest"
+	"time"
 	"unicode/utf8"
 )

@@ -174,6 +175,34 @@ func TestReader(t *testing.T) {
 	}
 }

+type zeroReader struct{}
+
+func (zeroReader) Read(p []byte) (int, error) {
+	return 0, nil
+}
+
+func TestZeroReader(t *testing.T) {
+	var z zeroReader
+	r := NewReader(z)
+
+	c := make(chan error)
+	go func() {
+		_, err := r.ReadByte()
+		c <- err
+	}()
+
+	select {
+	case err := <-c:
+		if err == nil {
+			t.Error("error expected")
+		} else if err != io.ErrNoProgress {
+			t.Error("unexpected error:", err)
+		}
+	case <-time.After(time.Second):
+		t.Error("test timed out (endless loop in ReadByte?)")
+	}
+}
+
 // A StringReader delivers its data one string segment at a time via Read.
 type StringReader struct {
 	data []string

zeroReader 構造体:
- Read(p []byte) (int, error) メソッドを実装するカスタム io.Reader です。
- 常に return 0, nil を返します。これは、データを読み込まず、かつエラーも返さない不正な io.Reader の振る舞いをシミュレートします。
TestZeroReader 関数:
- zeroReader のインスタンス z を作成し、それをラップする bufio.Reader r を作成します。
- ゴルーチン内で r.ReadByte() を呼び出し、その結果のエラーをチャネル c に送信します。
- select ステートメントを使用して、以下のいずれかのイベントを待ちます。
  - err := <-c: ReadByte() がエラーを返した場合。
    - if err == nil { t.Error("error expected") }: エラーが期待されるため、nil であればテスト失敗。
    - else if err != io.ErrNoProgress { t.Error("unexpected error:", err) }: 期待されるエラーは io.ErrNoProgress であるため、それ以外であればテスト失敗。
  - <-time.After(time.Second): 1秒後にタイムアウトした場合。
    - t.Error("test timed out (endless loop in ReadByte?)"): これは ReadByte() が無限ループに陥ったことを示し、テスト失敗となります。

このテストケースは、修正が正しく機能し、不正な io.Reader に対して bufio.Reader が無限ループに陥ることなく io.ErrNoProgress を返すことを効果的に検証しています。

参考にした情報源リンク

Google Web Search (query: "Go bufio ReadByte endless loop #7745")

comemo