database/sql: all named []byte types get driver memory like RawBytes

[jmoiron]

• Go version: 1.4.3, 1.5.2, and git
• OS: Linux x86_64

I added this test to src/database/sql/convert_test.go:

func TestUserDefinedBytes(t *testing.T) {
    type userDefinedBytes []byte
    var u userDefinedBytes
    v := []byte("foo")

    convertAssign(&u, v)
    if &u[0] == &v[0] {
        t.Fatal("userDefinedBytes got potentially dirty driver memory")
    }
}

• I expected this test to pass, but
• It fails

The problem is line 207 of convert.go; in this case, dest is assignable the src pointer, so it just assigns it instead of cloning it. The documentation doesn't actually make any promises about where memory is going to come from when you call Scan, but it seems obvious from the code that RawBytes was supposed to be treated specially as it gets driver memory via a different code path. Having that behavior inadvertently for non-RawBytes named []byte types seems like an oversight.

This patch fixes it, though it may potentially be present for other types or in line 217.

diff --git a/src/database/sql/convert.go b/src/database/sql/convert.go
index bba5a88..fda6293 100644
--- a/src/database/sql/convert.go
+++ b/src/database/sql/convert.go
@@ -204,7 +204,12 @@ func convertAssign(dest, src interface{}) error {

        dv := reflect.Indirect(dpv)
        if sv.IsValid() && sv.Type().AssignableTo(dv.Type()) {
-               dv.Set(sv)
+               switch b := src.(type) {
+               case []byte:
+                       dv.Set(reflect.ValueOf(cloneBytes(b)))
+               default:
+                       dv.Set(sv)
+               }
                return nil
        }

I signed the CLA and wouldn't mind going through the contribution process, though I've never done so.

[ianlancetaylor]

CC @bradfitz

[gofish]

A temporary workaround may be an explicit cast to *[]byte.

This passes the test:

convertAssign((*[]byte)(&u),v)

For json.RawMessage:

var msg json.RawMessage
row := db.QueryRow(`SELECT '{}'::json`)
err := row.Scan((*[]byte)(&msg))
...

[ianlancetaylor]

This is not a regression, and we are late in the release cycle, so postponing until 1.7.

[gopherbot]

CL https://golang.org/cl/22393 mentions this issue.