Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get treats json.RawMessage like db.RawBytes #197

Closed
chowey opened this issue Jan 3, 2016 · 7 comments
Closed

Get treats json.RawMessage like db.RawBytes #197

chowey opened this issue Jan 3, 2016 · 7 comments

Comments

@chowey
Copy link

chowey commented Jan 3, 2016

I have a struct where one of the fields is a json.RawMessage. When I use DB.Get() to populate it, the field is volatile and changes with more calls to DB.Get().

Here is a simple program to reproduce it.

package main

import (
    "encoding/json"
    "fmt"

    _ "github.com/go-sql-driver/mysql"
    "github.com/jmoiron/sqlx"
)

type Var struct {
    Raw json.RawMessage
}

func main() {
    db, err := sqlx.Open("mysql", "root@/")
    if err != nil {
        panic(err)
    }
    defer db.Close()

    var v Var
    if err = db.Get(&v, `SELECT '{"a":"b"}' AS raw`); err != nil {
        panic(err)
    }
    // first time works great
    fmt.Printf("v: %s\n", v.Raw)

    var q Var
    if err = db.Get(&q, `SELECT 'null' AS raw`); err != nil {
        panic(err)
    }
    // "v" has changed???
    fmt.Printf("v: %s\n", v.Raw)
}

The output I get is:

v: {"a":"b"}
v: null�
@jmoiron
Copy link
Owner

jmoiron commented Jan 11, 2016

I believe this is a Go issue and I've submitted an issue for it. That ticket describes the problem in more detail; rest assured that this problem exists with db.QueryRow as well. Unfortunately, for now you'll have to use []byte to get a copy of that data, though it should not involve a copy to type-convert it into json.RawMessage.

Here's a test for sqlx_test.go that shows that this behavior is consistent across any named []byte type:

func TestIssue197(t *testing.T) {
    type mybyte []byte
    type Var struct{ Raw json.RawMessage }
    type Var2 struct{ Raw []byte }
    type Var3 struct{ Raw mybyte }
    RunWithSchema(defaultSchema, t, func(db *DB, t *testing.T) {
        var err error
        var v, q Var
        if err = db.Get(&v, `SELECT '{"a": "b"}' AS raw`); err != nil {
            t.Fatal(err)
        }
        fmt.Printf("%s: v %s\n", db.DriverName(), v.Raw)
        if err = db.Get(&q, `SELECT 'null' AS raw`); err != nil {
            t.Fatal(err)
        }
        fmt.Printf("%s: v %s\n", db.DriverName(), v.Raw)

        var v2, q2 Var2
        if err = db.Get(&v2, `SELECT '{"a": "b"}' AS raw`); err != nil {
            t.Fatal(err)
        }
        fmt.Printf("%s: v2 %s\n", db.DriverName(), v2.Raw)
        if err = db.Get(&q2, `SELECT 'null' AS raw`); err != nil {
            t.Fatal(err)
        }
        fmt.Printf("%s: v2 %s\n", db.DriverName(), v2.Raw)

        var v3, q3 Var3
        if err = db.QueryRow(`SELECT '{"a": "b"}' AS raw`).Scan(&v3.Raw); err != nil {
            t.Fatal(err)
        }
        fmt.Printf("v3 %s\n", v3.Raw)
        if err = db.QueryRow(`SELECT '{"c": "d"}' AS raw`).Scan(&q3.Raw); err != nil {
            t.Fatal(err)
        }
        fmt.Printf("v3 %s\n", v3.Raw)
        t.Fail()
    })
}

Output:

postgres: v  1": "b"}
postgres: v  1l�
postgres: v2 {"a": "b"}
postgres: v2 {"a": "b"}
v3  1": "b"}
v3  1": "d"}
sqlite3: v {"a": "b"}
sqlite3: v {"a": "b"}
sqlite3: v2 {"a": "b"}
sqlite3: v2 {"a": "b"}
v3 {"a": "b"}
v3 {"a": "b"}
mysql: v {"a": "b"}
mysql: v null�
mysql: v2 {"a": "b"}
mysql: v2 {"a": "b"}
v3 {"a": "b"}
v3 {"c": "d"}

@jmoiron
Copy link
Owner

jmoiron commented Jan 11, 2016

I'm going to close this issue because it's not an sqlx issue, however it does make some things in sqlx/types potentially dangerous to use, so I think I will document that this bug is there for certain Go versions.

@jmoiron jmoiron closed this as completed Jan 11, 2016
@chowey
Copy link
Author

chowey commented Jan 11, 2016

Okay thanks for shedding light on this. In the meanwhile I am explicitly using []byte and converting to a json.RawMessage afterwards, as you suggested.

jmoiron added a commit that referenced this issue Jan 12, 2016
@gofish
Copy link

gofish commented Jan 14, 2016

@jmoiron Are you sure the warnings are appropriate for types that implement Scanner? It would seem they are always passed driver memory and are expected to copy the []byte.

@jmoiron
Copy link
Owner

jmoiron commented Jan 14, 2016

You're right, the types versions are not vulnerable because they implement Scanner, and other types that implement scanner are also not vulnerable. Any type foo []byte that is not implementing Scanner is vulnerable.

@gofish
Copy link

gofish commented Jan 14, 2016

Cool. If you haven't, see my comment on golang/go#13905 for a workaround on scanning types that are convertible to []byte. Could sqlx detect non-scannable structs (edit: struct fields) that are so convertible, e.g. under fieldsByTraversal(), then convert the pointer value to *byte[] and pass that to Row.Scan(...)?

@kevinburke
Copy link

FYI - I submitted @jmoiron's patch, which has been included in Go 1.7, so you should be safe against this problem if you are using it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants