sql: fix potential corruption in QueryRow.Scan into a *[]byte
Fixes #2622
R=golang-dev, adg
CC=golang-dev
https://golang.org/cl/5533077
diff --git a/src/pkg/exp/sql/sql.go b/src/pkg/exp/sql/sql.go
index f53691b..a076fcd 100644
--- a/src/pkg/exp/sql/sql.go
+++ b/src/pkg/exp/sql/sql.go
@@ -803,10 +803,6 @@
// 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 @@
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.
