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

コミット

commit 701f70abf6ac76fbd28c640ec49609090882f05a
Author: Brad Fitzpatrick <bradfitz@golang.org>
Date:   Thu Jan 12 11:23:33 2012 -0800

    sql: fix potential corruption in QueryRow.Scan into a *[]byte
    
    Fixes #2622
    
    R=golang-dev, adg
    CC=golang-dev
    https://golang.org/cl/5533077

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

https://github.com/golang/go/commit/701f70abf6ac76fbd28c640ec49609090882f05a

元コミット内容

このコミットは、Go言語の標準ライブラリdatabase/sqlパッケージにおける潜在的なデータ破損の問題を修正するものです。具体的には、QueryRow().Scan()メソッドを使用してデータベースから[]byte型のデータを読み込む際に、そのデータが後で破損する可能性があったバグに対処しています。

コミットメッセージは以下の通りです。 "sql: fix potential corruption in QueryRow.Scan into a *[]byte" "Fixes #2622"

これは、QueryRow().Scan()が*[]byte（バイトスライスのポインタ）にスキャンする際に発生する可能性のあるデータ破損を修正することを示しています。#2622は、この問題が追跡されていた内部または以前のイシュートラッカーの番号であると考えられます。

変更の背景

Goのdatabase/sqlパッケージは、データベースドライバとアプリケーションコードの間の抽象化レイヤーを提供します。ドライバは、データベースから取得したデータをGoの型に変換して返します。特に[]byte型の場合、ドライバは内部的な一時バッファへの参照を返すことがあります。

問題は、QueryRow().Scan()メソッドの動作にありました。このメソッドは、単一の行をクエリし、その結果をすぐにスキャンした後、内部的にRowsオブジェクトを閉じます。この「閉じる」操作が、ドライバが返した[]byteスライスが参照していた内部バッファを解放したり、再利用したりする可能性がありました。

もしアプリケーションコードがQueryRow().Scan()から返された[]byteスライスを保持し、後でその内容にアクセスしようとした場合、そのスライスが指すメモリがすでに変更されているか、無効になっている可能性があるため、データ破損が発生していました。これは、[]byteが参照型であり、その内容が基盤となる配列によって決定されるためです。ドライバが一時的なメモリを指すスライスを返した場合、そのメモリが解放されると、スライスは無効なデータを指すことになります。

この問題は、特にバイナリデータ（画像、ファイル内容など）を[]byteとしてデータベースから取得する際に顕在化し、アプリケーションの信頼性を損なう可能性がありました。

前提知識の解説

1. Go言語のdatabase/sqlパッケージ

database/sqlパッケージは、GoアプリケーションからSQLデータベースにアクセスするための汎用的なインターフェースを提供します。このパッケージ自体は特定のデータベースドライバを含まず、database/sql/driverインターフェースを実装する外部ドライバを介してデータベースと通信します。

• DB: データベースへの接続プールを表します。
• QueryRow(): 単一の行を返すことが期待されるクエリを実行するためのメソッドです。結果セットから最初の行を読み込んだ後、自動的にRowsオブジェクトを閉じます。
• Scan(): クエリ結果の列の値をGoの変数に読み込むためのメソッドです。Scanに渡される引数は、データベースの列の型と互換性のあるGoの型のポインタである必要があります。

2. Go言語におけるスライス（[]byte）の挙動

Goのスライスは、配列への参照（ビュー）です。スライスは以下の3つの要素で構成されます。

• ポインタ: スライスが参照する基盤となる配列の最初の要素へのポインタ。
• 長さ（Length）: スライスに含まれる要素の数。
• 容量（Capacity）: スライスの最初の要素から基盤となる配列の末尾までの要素の数。

重要なのは、複数のスライスが同じ基盤となる配列の一部または全体を参照できる点です。この特性は効率的ですが、注意が必要です。あるスライスを介して基盤となる配列の要素を変更すると、同じ配列を参照している他のすべてのスライスにもその変更が反映されます。

今回の問題では、データベースドライバが内部バッファ（基盤となる配列）の一部を指す[]byteスライスを返していました。QueryRow().Scan()が完了し、Rowsオブジェクトが閉じられると、ドライバはその内部バッファを再利用したり、解放したりする可能性がありました。その結果、アプリケーションが保持していた[]byteスライスは、もはや有効なデータを指さなくなり、データ破損につながりました。

3. driver.Next()とScan()の契約

database/sql/driverパッケージのdriver.RowsインターフェースにはNext(dest []driver.Value) errorメソッドがあります。このメソッドの契約では、ドライバが返す[]byteスライスは、次のNext()呼び出しまたはClose()呼び出しまでのみ有効な一時的なメモリを指す可能性があるとされています。これは、ドライバがメモリを効率的に再利用できるようにするためです。しかし、QueryRow().Scan()のようにRowsがすぐに閉じられる場合、この契約がアプリケーションレベルでのデータ破損を引き起こす原因となっていました。

技術的詳細

このコミットの技術的解決策は、QueryRow().Scan()が*[]byte型の引数を受け取る場合に、ドライバから返された[]byteデータを「防御的にクローン（複製）」することです。これにより、アプリケーションに渡される[]byteスライスが、ドライバの内部バッファのライフサイクルに依存しない、独立したコピーとなることが保証されます。

変更は主にsrc/pkg/exp/sql/sql.goのRow.Scanメソッドに集中しています。

1. 既存のコメントの削除: 以前はRow.Scanのドキュメントに「destが[]byteへのポインタを含む場合、スライスは変更されるべきではなく、次のNextまたはScan呼び出しまでのみ有効であると見なされるべきです」という注意書きがありました。このコミットでは、この注意書きが削除されました。これは、新しい実装がこの制約を緩和し、ユーザーが返された[]byteを安全に保持・使用できるようにするためです。

2. []byteの防御的クローン: Row.Scanメソッド内で、r.rows.Scan(dest...)が呼び出された後、destに渡された引数をイテレートし、*[]byte型の引数を見つけます。

   • b, ok := dp.(*[]byte): dest内の各ポインタdpが*[]byte型であるかをチェックします。
   • clone := make([]byte, len(*b)): 元のバイトスライスと同じ長さの新しいバイトスライスを作成します。これにより、新しい基盤となる配列が割り当てられます。
   • copy(clone, *b): 元のバイトスライスの内容を新しく作成したクローンにコピーします。
   • *b = clone: destに渡されたポインタが、新しく作成されたクローンを指すように更新されます。

このクローン処理により、QueryRow().Scan()から返された[]byteスライスは、ドライバが内部的に使用していた一時バッファとは完全に独立したものになります。したがって、Rowsオブジェクトが閉じられた後も、アプリケーションは安全にその[]byteスライスの内容にアクセスできます。

テストコード（src/pkg/exp/sql/fakedb_test.goとsrc/pkg/exp/sql/sql_test.go）も更新され、この修正が正しく機能することを検証しています。特にfakedb_test.goでは、rowsCursorにbytesCloneマップを追加し、Close()時にクローンされたバイトスライスの最初のバイトを意図的に破損させることで、Scanが防御的コピーを行わない場合にテストが失敗するようにしています。これにより、修正が実際にデータ破損を防ぐことを保証しています。

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

src/pkg/exp/sql/sql.go

--- a/src/pkg/exp/sql/sql.go
+++ b/src/pkg/exp/sql/sql.go
@@ -803,10 +803,6 @@ type Row struct {
 // pointed at by dest.  If more than one row matches the query,
 // Scan uses the first row and discards the rest.  If no row matches
 // the query, Scan returns ErrNoRows.
-//
-// If dest contains pointers to []byte, the slices should not be
-// modified and should only be considered valid until the next call to
-// Next or Scan.
 func (r *Row) Scan(dest ...interface{}) error {
  	if r.err != nil {
  		return r.err
@@ -815,7 +811,33 @@ func (r *Row) Scan(dest ...interface{}) error {
  	if !r.rows.Next() {
  		return ErrNoRows
  	}
-	return r.rows.Scan(dest...)
+	err := r.rows.Scan(dest...)
+	if err != nil {
+		return err
+	}
+
+	// TODO(bradfitz): for now we need to defensively clone all
+	// []byte that the driver returned, since we're about to close
+	// the Rows in our defer, when we return from this function.
+	// the contract with the driver.Next(...) interface is that it
+	// can return slices into read-only temporary memory that's
+	// only valid until the next Scan/Close.  But the TODO is that
+	// for a lot of drivers, this copy will be unnecessary.  We
+	// should provide an optional interface for drivers to
+	// implement to say, "don't worry, the []bytes that I return
+	// from Next will not be modified again." (for instance, if
+	// they were obtained from the network anyway) But for now we
+	// don't care.
+	for _, dp := range dest {
+		b, ok := dp.(*[]byte)
+		if !ok {
+			continue
+		}
+		clone := make([]byte, len(*b))
+		copy(clone, *b)
+		*b = clone
+	}
+	return nil
 }
 
 // A Result summarizes an executed SQL command.

コアとなるコードの解説

上記の差分は、Row.Scanメソッドの変更を示しています。

1. 既存のコメントの削除: Row.Scanのドキュメントから、[]byteスライスが一時的なものであるという警告が削除されました。これは、この修正によってその制約がなくなったためです。

2. r.rows.Scan(dest...)の呼び出し: まず、内部のrowsオブジェクトのScanメソッドを呼び出し、データベースからデータを読み込みます。

3. エラーハンドリング: r.rows.Scanがエラーを返した場合、そのエラーをすぐに返します。

4. 防御的クローン処理の追加: ここがこのコミットの核心部分です。

   for _, dp := range dest {
       b, ok := dp.(*[]byte)
       if !ok {
           continue
       }
       clone := make([]byte, len(*b))
       copy(clone, *b)
       *b = clone
   }
   

   • for _, dp := range dest: Scanメソッドに渡された可変長引数dest（スキャン対象の変数へのポインタのリスト）をループで処理します。
   • b, ok := dp.(*[]byte): 各引数dpが*[]byte型（バイトスライスへのポインタ）であるかを型アサーションで確認します。もしそうでない場合（例えば、*stringや*intなど）、okはfalseとなり、continueで次の引数に移ります。
   • clone := make([]byte, len(*b)): *[]byte型であることが確認された場合、元のバイトスライス*bと同じ長さの新しいバイトスライスcloneを作成します。make関数は、新しい基盤となる配列を割り当てます。
   • copy(clone, *b): 元のバイトスライス*bの内容を、新しく作成したcloneスライスにコピーします。これにより、データが物理的に複製されます。
   • *b = clone: 最後に、destに渡された元のポインタdpが指す[]byte変数を、新しく作成されたcloneスライスに置き換えます。これにより、アプリケーションコードが受け取る[]byteスライスは、ドライバの内部バッファとは完全に独立した、安全なコピーとなります。

この変更により、QueryRow().Scan()が返された後でも、[]byteデータがドライバの内部的なメモリ管理の影響を受けずに、アプリケーションによって安全に利用できるようになります。

関連リンク

• Go言語の公式ドキュメント (database/sql): https://pkg.go.dev/database/sql
• Go言語の公式ドキュメント (database/sql/driver): https://pkg.go.dev/database/sql/driver
• Go言語の公式ドキュメント (スライス): https://go.dev/blog/slices-intro (スライスの紹介記事)

参考にした情報源リンク

• GitHub上のコミットページ: https://github.com/golang/go/commit/701f70abf6ac76fbd28c640ec49609090882f05a
• Go CL (Change List) 5533077: https://golang.org/cl/5533077 (このコミットに対応するGoのコードレビューシステム上の変更リスト)
• Go言語のdatabase/sqlパッケージのソースコード: この解説は、コミットの差分とGoのdatabase/sqlパッケージの一般的な動作に関する知識に基づいて作成されています。