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

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

このコミットは、Go言語の標準ライブラリである net/textproto パッケージにおけるデータ破損のバグを修正するものです。具体的には、bufio.Reader から読み取ったデータが、その後の読み取り操作によって上書きされ、不正なデータとして扱われる可能性があった問題を解決します。この修正は、bufio.Reader の内部バッファから読み取ったデータを常にコピーすることで、データの整合性を保証します。

コミット

commit e955a3cca2ad600666f2f814aad6075a42a88d4d
Author: Andrew Gerrand <adg@golang.org>
Date:   Thu Jan 12 14:15:58 2012 +1100

    net/textproto: always copy the data from bufio to avoid corruption

    Fixes #2621.

    R=rsc, rsc
    CC=golang-dev
    https://golang.org/cl/5498104

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

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

元コミット内容

net/textproto: always copy the data from bufio to avoid corruption

Fixes #2621.

R=rsc, rsc CC=golang-dev https://golang.org/cl/5498104

変更の背景

この変更は、net/textproto パッケージが bufio.Reader からデータを読み取る際に発生する可能性のあるデータ破損の問題を修正するために行われました。bufio.ReaderReadSlice メソッドは、内部バッファへのスライスを返します。このスライスは、bufio.Reader が次に読み取り操作を行うと、その内部バッファが再利用されるため、無効になる可能性があります。

net/textprotoreadContinuedLineSlice 関数は、この ReadSlice の戻り値を直接使用し、その後の読み取り操作で同じスライスを参照し続けることで、データが上書きされてしまうという暗黙の仮定に依存していました。この仮定が破られると、特に継続行(HTTPヘッダーなどで見られる、次の行に続く行)を処理する際に、以前に読み取ったデータが破損する可能性がありました。

この問題は、GoのIssue #2621として報告されており、このコミットはその問題を解決することを目的としています。コードレビューの過程で、bufio のAPI保証に関する暗黙の仮定に依存しないようにコードを修正する必要があることが強調されました。

前提知識の解説

  • net/textproto パッケージ: このパッケージは、HTTP、NNTP、SMTPなどのテキストベースのネットワークプロトコルを実装するための汎用的なテキストプロトコルパーサーを提供します。ヘッダーの解析やメッセージの読み取りなど、プロトコルに共通する処理を抽象化します。
  • bufio.Reader: bufio パッケージは、I/O操作をバッファリングすることで効率を向上させます。bufio.Reader は、基になる io.Reader からデータを読み取り、内部バッファに格納します。
    • ReadSlice メソッド: bufio.ReaderReadSlice メソッドは、区切り文字まで(またはバッファの終わりまで)のデータを内部バッファへのスライスとして返します。重要な点として、この返されたスライスは bufio.Reader が次に読み取り操作を行うと無効になる可能性があります。 これは、bufio.Reader が内部バッファを再利用するためです。
  • Goにおけるスライスとバッファの再利用: Goのスライスは、基になる配列の一部を参照するビューです。bufio.Reader のように内部バッファを再利用するコンポーネントからスライスを受け取る場合、そのスライスが指すデータは、元のバッファが上書きされると変更される可能性があります。そのため、データの永続的なコピーが必要な場合は、明示的に新しいスライスにデータをコピーする必要があります。

技術的詳細

このコミットの核心は、net/textproto.Readerbufio.Reader から読み取ったデータを扱う方法の変更にあります。以前の実装では、bufio.Reader.ReadSlice が返したスライスを直接使用し、そのスライスが指すデータがその後の読み取り操作によって変更されないという暗黙の仮定がありました。しかし、これは bufio.Reader の設計上保証されていません。

修正では、この問題を解決するために以下の変更が導入されました。

  1. buf フィールドの追加: net/textproto.Reader 構造体に buf []byte という新しいフィールドが追加されました。これは、bufio.Reader から読み取ったデータをコピーして格納するための再利用可能なバッファとして機能します。
  2. データの強制コピー: readContinuedLineSlice メソッド内で、readLineSlice (内部で bufio.Reader.ReadSlice を使用) から取得した行データを、r.buf に常にコピーするように変更されました。
    • r.buf = append(r.buf[:0], trim(line)...): この行が重要です。r.buf[:0] は、既存の r.buf スライスの容量を維持しつつ長さをゼロにリセットします。これにより、新しいメモリ割り当てを避けてバッファを再利用できます。そして、trim(line) の結果(行の先頭と末尾の空白を削除したもの)をこのバッファにコピーします。
  3. 継続行の処理の変更: 継続行を読み取る際も、readLineSlice で読み取ったデータを r.buf に追加するように変更されました。これにより、すべての行データが r.buf にコピーされ、bufio.Reader の内部バッファの再利用によるデータ破損を防ぎます。
  4. skipSpace ヘルパー関数の導入: 継続行の先頭の空白をスキップするための新しいヘルパー関数 skipSpace が導入され、コードの可読性と保守性が向上しました。

この修正により、net/textproto.Readerbufio.Reader の内部動作に依存することなく、常にデータの安全なコピーを保持するようになり、データ破損の問題が根本的に解決されました。

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

src/pkg/net/textproto/reader.goReader 構造体と readContinuedLineSlice メソッド、および新しい skipSpace メソッドが変更されました。

--- a/src/pkg/net/textproto/reader.go
+++ b/src/pkg/net/textproto/reader.go
@@ -22,6 +22,7 @@ import (
 type Reader struct {
 	R   *bufio.Reader
 	dot *dotReader
+	buf []byte // a re-usable buffer for readContinuedLineSlice
 }

 // NewReader returns a new Reader reading from r.
@@ -121,74 +122,44 @@ func (r *Reader) readContinuedLineSlice() ([]byte, error) {
 	// Read the first line.
 	line, err := r.readLineSlice()
 	if err != nil {
-		return line, err
+		return nil, err
 	}
 	if len(line) == 0 { // blank line - no continuation
 		return line, nil
 	}
-	line = trim(line)

-	copied := false
-	if r.R.Buffered() < 1 {
-		// ReadByte will flush the buffer; make a copy of the slice.
-		copied = true
-		line = append([]byte(nil), line...)
-	}
-
-	// Look for a continuation line.
-	c, err := r.R.ReadByte()
-	if err != nil {
-		// Delay err until we read the byte next time.
-		return line, nil
-	}
-	if c != ' ' && c != '\t' {
-		// Not a continuation.
-		r.R.UnreadByte()
-		return line, nil
-	}
-
-	if !copied {
-		// The next readLineSlice will invalidate the previous one.
-		line = append(make([]byte, 0, len(line)*2), line...)
-	}
+	// ReadByte or the next readLineSlice will flush the read buffer;
+	// copy the slice into buf.
+	r.buf = append(r.buf[:0], trim(line)...)

 	// Read continuation lines.
-	for {
-		// Consume leading spaces; one already gone.
-		for {
-			c, err = r.R.ReadByte()
-			if err != nil {
-				break
-			}
-			if c != ' ' && c != '\t' {
-				r.R.UnreadByte()
-				break
-			}
-		}
-		var cont []byte
-		cont, err = r.readLineSlice()
-		cont = trim(cont)
-		line = append(line, ' ')
-		line = append(line, cont...)
+	for r.skipSpace() > 0 {
+		line, err := r.readLineSlice()
 		if err != nil {
 			break
 		}
+		r.buf = append(r.buf, ' ')
+		r.buf = append(r.buf, line...)
+	}
+	return r.buf, nil
+}

-		// Check for leading space on next line.
-		if c, err = r.R.ReadByte(); err != nil {
-			break
-		}
-		if c != ' ' && c != '\t' {
-			r.R.UnreadByte()
-			break
-		}
+// skipSpace skips R over all spaces and returns the number of bytes skipped.
+func (r *Reader) skipSpace() int {
+	n := 0
+	for {
+		c, err := r.R.ReadByte()
+		if err != nil {
+			// Bufio will keep err until next read.
+			break
+		}
+		if c != ' ' && c != '\t' {
+			r.R.UnreadByte()
+			break
+		}
+		n++
 	}
-
-	// Delay error until next call.
-	if len(line) > 0 {
-		err = nil
-	}
-	return line, err
+	return n
 }

 func (r *Reader) readCodeLine(expectCode int) (code int, continued bool, message string, err error) {

src/pkg/net/textproto/reader_test.go には、新しいテストケース TestReadMIMEHeaderSingle が追加されました。

--- a/src/pkg/net/textproto/reader_test.go
+++ b/src/pkg/net/textproto/reader_test.go
@@ -138,6 +138,15 @@ func TestReadMIMEHeader(t *testing.T) {
 	}\n}\n \n+func TestReadMIMEHeaderSingle(t *testing.T) {\n+\tr := reader(\"Foo: bar\\n\\n\")\n+\tm, err := r.ReadMIMEHeader()\n+\twant := MIMEHeader{\"Foo\": {\"bar\"}}\n+\tif !reflect.DeepEqual(m, want) || err != nil {\n+\t\tt.Fatalf(\"ReadMIMEHeader: %v, %v; want %v\", m, err, want)\n+\t}\n+}\n+\n func TestLargeReadMIMEHeader(t *testing.T) {\n \tdata := make([]byte, 16*1024)\n \tfor i := 0; i < len(data); i++ {\n```

## コアとなるコードの解説

### `src/pkg/net/textproto/reader.go`

*   **`Reader` 構造体への `buf` フィールド追加**:
    ```go
    type Reader struct {
    	R   *bufio.Reader
    	dot *dotReader
    	buf []byte // a re-usable buffer for readContinuedLineSlice
    }
    ```
    `buf` は、`readContinuedLineSlice` メソッド内で `bufio.Reader` から読み取ったデータを一時的に格納し、再利用するためのバイトスライスです。これにより、`bufio.Reader` の内部バッファが再利用されても、`net/textproto` が保持するデータが破損しないようにします。

*   **`readContinuedLineSlice` メソッドの変更**:
    このメソッドは、継続行を含む行を読み取るためのものです。
    ```go
    	line, err := r.readLineSlice()
    	if err != nil {
    		return nil, err // 以前は line を返していたが、エラーの場合は nil を返すように変更
    	}
    	if len(line) == 0 {
    		return line, nil
    	}

    	// ReadByte or the next readLineSlice will flush the read buffer;
    	// copy the slice into buf.
    	r.buf = append(r.buf[:0], trim(line)...)
    ```
    ここで、`r.buf = append(r.buf[:0], trim(line)...)` が最も重要な変更点です。
    *   `r.buf[:0]` は、`r.buf` スライスの長さをゼロにリセットしますが、基になる配列の容量は保持します。これにより、新しいメモリ割り当てを最小限に抑えつつ、バッファを再利用できます。
    *   `trim(line)` は、`readLineSlice` で読み取った行の先頭と末尾の空白を削除します。
    *   `append(...)` は、`trim(line)` の結果を `r.buf` にコピーします。これにより、`line` スライスが `bufio.Reader` の内部バッファを指していても、そのデータが `r.buf` に安全にコピーされ、その後の `bufio` の操作によって上書きされる心配がなくなります。

    ```go
    	for r.skipSpace() > 0 { // 新しい skipSpace 関数を使用
    		line, err := r.readLineSlice()
    		if err != nil {
    			break
    		}
    		r.buf = append(r.buf, ' ') // 継続行の前にスペースを追加
    		r.buf = append(r.buf, line...) // 継続行のデータを r.buf に追加
    	}
    	return r.buf, nil
    ```
    継続行を読み取るループも変更され、`skipSpace` 関数を使用して先頭の空白をスキップし、読み取った継続行のデータを `r.buf` に追加しています。これにより、すべての継続行データも安全に `r.buf` にコピーされます。最終的に、`r.buf` に格納された完全な行データが返されます。

*   **`skipSpace` ヘルパー関数の追加**:
    ```go
    func (r *Reader) skipSpace() int {
    	n := 0
    	for {
    		c, err := r.R.ReadByte()
    		if err != nil {
    			// Bufio will keep err until next read.
    			break
    		}
    		if c != ' ' && c != '\t' {
    			r.R.UnreadByte()
    			break
    		}
    		n++
    	}
    	return n
    }
    ```
    この関数は、`bufio.Reader` からバイトを1つずつ読み取り、それがスペースまたはタブである限りスキップします。スペースまたはタブでない文字に遭遇するか、エラーが発生するとループを終了し、スキップしたバイト数を返します。これにより、`readContinuedLineSlice` 内の継続行の先頭の空白を処理するロジックが簡潔になりました。

### `src/pkg/net/textproto/reader_test.go`

*   **`TestReadMIMEHeaderSingle` テストケースの追加**:
    ```go
    func TestReadMIMEHeaderSingle(t *testing.T) {
    	r := reader("Foo: bar\n\n")
    	m, err := r.ReadMIMEHeader()
    	want := MIMEHeader{"Foo": {"bar"}}
    	if !reflect.DeepEqual(m, want) || err != nil {
    		t.Fatalf("ReadMIMEHeader: %v, %v; want %v", m, err, want)
    	}
    }
    ```
    この新しいテストケースは、単一のMIMEヘッダーが正しく解析されることを確認します。これは、`readContinuedLineSlice` の変更が `ReadMIMEHeader` のような高レベルの関数に悪影響を与えないことを保証するものです。

## 関連リンク

*   **Go CL (Change List)**: [https://golang.org/cl/5498104](https://golang.org/cl/5498104)
*   **Go Issue #2621**: コミットメッセージに `Fixes #2621` とありますが、これはGoの内部的なバグトラッカーのIDである可能性が高く、公開されているGoのGitHubリポジトリのIssueとは直接対応しない場合があります。しかし、CLの議論から、`bufio.Reader` の `ReadSlice` の挙動に関する暗黙の仮定が問題であったことが確認できます。

## 参考にした情報源リンク

*   **Go CL 5498104 のレビューコメント**: `https://golang.org/cl/5498104` のレビューコメントは、この変更の背景、技術的な議論、および `bufio.Reader` の `ReadSlice` の保証に関するRuss Cox氏のコメントなど、非常に詳細な情報を提供しています。
*   **Go言語の `bufio` パッケージのドキュメント**: `https://pkg.go.dev/bufio`
*   **Go言語のスライスに関するドキュメント**: `https://go.dev/blog/slices` (Goブログのスライスに関する記事など)