[インデックス 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.Reader
の ReadSlice
メソッドは、内部バッファへのスライスを返します。このスライスは、bufio.Reader
が次に読み取り操作を行うと、その内部バッファが再利用されるため、無効になる可能性があります。
net/textproto
の readContinuedLineSlice
関数は、この ReadSlice
の戻り値を直接使用し、その後の読み取り操作で同じスライスを参照し続けることで、データが上書きされてしまうという暗黙の仮定に依存していました。この仮定が破られると、特に継続行(HTTPヘッダーなどで見られる、次の行に続く行)を処理する際に、以前に読み取ったデータが破損する可能性がありました。
この問題は、GoのIssue #2621として報告されており、このコミットはその問題を解決することを目的としています。コードレビューの過程で、bufio
のAPI保証に関する暗黙の仮定に依存しないようにコードを修正する必要があることが強調されました。
前提知識の解説
net/textproto
パッケージ: このパッケージは、HTTP、NNTP、SMTPなどのテキストベースのネットワークプロトコルを実装するための汎用的なテキストプロトコルパーサーを提供します。ヘッダーの解析やメッセージの読み取りなど、プロトコルに共通する処理を抽象化します。bufio.Reader
:bufio
パッケージは、I/O操作をバッファリングすることで効率を向上させます。bufio.Reader
は、基になるio.Reader
からデータを読み取り、内部バッファに格納します。ReadSlice
メソッド:bufio.Reader
のReadSlice
メソッドは、区切り文字まで(またはバッファの終わりまで)のデータを内部バッファへのスライスとして返します。重要な点として、この返されたスライスはbufio.Reader
が次に読み取り操作を行うと無効になる可能性があります。 これは、bufio.Reader
が内部バッファを再利用するためです。
- Goにおけるスライスとバッファの再利用: Goのスライスは、基になる配列の一部を参照するビューです。
bufio.Reader
のように内部バッファを再利用するコンポーネントからスライスを受け取る場合、そのスライスが指すデータは、元のバッファが上書きされると変更される可能性があります。そのため、データの永続的なコピーが必要な場合は、明示的に新しいスライスにデータをコピーする必要があります。
技術的詳細
このコミットの核心は、net/textproto.Reader
が bufio.Reader
から読み取ったデータを扱う方法の変更にあります。以前の実装では、bufio.Reader.ReadSlice
が返したスライスを直接使用し、そのスライスが指すデータがその後の読み取り操作によって変更されないという暗黙の仮定がありました。しかし、これは bufio.Reader
の設計上保証されていません。
修正では、この問題を解決するために以下の変更が導入されました。
buf
フィールドの追加:net/textproto.Reader
構造体にbuf []byte
という新しいフィールドが追加されました。これは、bufio.Reader
から読み取ったデータをコピーして格納するための再利用可能なバッファとして機能します。- データの強制コピー:
readContinuedLineSlice
メソッド内で、readLineSlice
(内部でbufio.Reader.ReadSlice
を使用) から取得した行データを、r.buf
に常にコピーするように変更されました。r.buf = append(r.buf[:0], trim(line)...)
: この行が重要です。r.buf[:0]
は、既存のr.buf
スライスの容量を維持しつつ長さをゼロにリセットします。これにより、新しいメモリ割り当てを避けてバッファを再利用できます。そして、trim(line)
の結果(行の先頭と末尾の空白を削除したもの)をこのバッファにコピーします。
- 継続行の処理の変更: 継続行を読み取る際も、
readLineSlice
で読み取ったデータをr.buf
に追加するように変更されました。これにより、すべての行データがr.buf
にコピーされ、bufio.Reader
の内部バッファの再利用によるデータ破損を防ぎます。 skipSpace
ヘルパー関数の導入: 継続行の先頭の空白をスキップするための新しいヘルパー関数skipSpace
が導入され、コードの可読性と保守性が向上しました。
この修正により、net/textproto.Reader
は bufio.Reader
の内部動作に依存することなく、常にデータの安全なコピーを保持するようになり、データ破損の問題が根本的に解決されました。
コアとなるコードの変更箇所
src/pkg/net/textproto/reader.go
の Reader
構造体と 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ブログのスライスに関する記事など)