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

Call to sqlx.In on driver.Value with nil value panics #953

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func In(query string, args ...interface{}) (string, []interface{}, error) {
for i, arg := range args {
if a, ok := arg.(driver.Valuer); ok {
var err error
arg, err = a.Value()
arg, err = callValuerValue(a)
if err != nil {
return "", nil, err
}
Expand Down Expand Up @@ -263,3 +263,24 @@ func appendReflectSlice(args []interface{}, v reflect.Value, vlen int) []interfa

return args
}

// callValuerValue returns vr.Value(), with one exception:
// If vr.Value is an auto-generated method on a pointer type and the
// pointer is nil, it would panic at runtime in the panicwrap
// method. Treat it like nil instead.
// Issue 8415.
//
// This is so people can implement driver.Value on value types and
// still use nil pointers to those types to mean nil/NULL, just like
// string/*string.
//
// This function is copied from database/sql/driver package
// and mirrored in the database/sql package.
func callValuerValue(vr driver.Valuer) (v driver.Value, err error) {
if rv := reflect.ValueOf(vr); rv.Kind() == reflect.Pointer &&
rv.IsNil() &&
rv.Type().Elem().Implements(reflect.TypeOf((*driver.Valuer)(nil)).Elem()) {
return nil, nil
}
return vr.Value()
}
26 changes: 26 additions & 0 deletions sqlx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1504,6 +1504,14 @@ func TestIssue197(t *testing.T) {
})
}

type Valuer struct {
Val string
}

func (v Valuer) Value() (driver.Value, error) {
return v.Val, nil
}

func TestIn(t *testing.T) {
// some quite normal situations
type tr struct {
Expand Down Expand Up @@ -1538,6 +1546,24 @@ func TestIn(t *testing.T) {
}
}

// nil driver.Valuer
t.Run("with nil driver.Valuer", func(t *testing.T) {
query := `SELECT * FROM foo WHERE x = ? or y IN (?)`
_, _, err := In(query,
(*Valuer)(nil), // a non-inited pointer to valuer
[]interface{}{
"a", // a usual value
nil, // a nil value
Valuer{Val: "foo"}, // a Valuer
&Valuer{Val: "foo"}, // a pointer to valuer
(*Valuer)(nil), // a non-inited pointer to valuer
},
)
if err != nil {
t.Error(err)
}
})

// too many bindVars, but no slices, so short circuits parsing
// i'm not sure if this is the right behavior; this query/arg combo
// might not work, but we shouldn't parse if we don't need to
Expand Down