*Breaking* types change: JsonText -> JSONText, also document the []byte dest memory corruption bug and discourage JSONText and GzippedText both in package documentation and in the readme

jmoiron · jmoiron · commit 4301d72cf919 · 2016-01-12T00:48:10.000-05:00
diff --git a/README.md b/README.md
@@ -20,26 +20,15 @@ explains how to use `database/sql` along with sqlx.
 
 ## Recent Changes
 
-The addition of `sqlx.In` and `sqlx.Named`, which can be used to bind IN style
-queries and named queries respectively.
+* sqlx/types.JsonText has been renamed to JSONText to follow Go naming conventions.
 
-```go
-ids := []int{1, 2, 3}
-query, args, err := sqlx.In("SELECT * FROM person WHERE id IN(?);", ids)
-
-chris := Person{First: "Christian", Last: "Cullen"}
-query, args, err := sqlx.Named("INSERT INTO person VALUES (:first, :last);", chris)
+This breaks backwards compatibility, but it's in a way that is trivially fixable
+(`s/JsonText/JSONText/g`).  The `types` package is both experimental and not in
+active development currently.
 
-// these can be combined:
-arg := map[string]interface{}{
-    "published": true,
-    "authors": []{8, 19, 32, 44},
-}
-query, args, err := sqlx.Named("SELECT * FROM articles WHERE published=:published AND author_id IN (:authors)", arg)
-query, args, err := sqlx.In(query, args...)
-// finally, if you're using eg. pg, you can rebind the query:
-query = db.Rebind(query)
-```
+More importantly, [golang bug #13905](https://github.com/golang/go/issues/13905)
+makes `types.JSONText` and `types.GzippedText` _potentially unsafe_, **especially**
+when used with common auto-scan sqlx idioms like `Select` and `Get`.
 
 ### Backwards Compatibility
 
diff --git a/types/types.go b/types/types.go
@@ -12,6 +12,10 @@ import (
 
 // GzippedText is a []byte which transparently gzips data being submitted to
 // a database and ungzips data being Scanned from a database.
+// WARNING: due to go issue https://github.com/golang/go/issues/13905 it is
+// potentially unsafe to use any []byte alias (like GzippedText) unless you
+// use it as if it had the behavior of sql.RawBytes (ie. it ceases to be
+// valid at the next scan)
 type GzippedText []byte
 
 // Value implements the driver.Valuer interface, gzipping the raw value of
@@ -48,21 +52,25 @@ func (g *GzippedText) Scan(src interface{}) error {
 	return nil
 }
 
-// JsonText is a json.RawMessage, which is a []byte underneath.
+// JSONText is a json.RawMessage, which is a []byte underneath.
 // Value() validates the json format in the source, and returns an error if
-// the json is not valid.  Scan does no validation.  JsonText additionally
+// the json is not valid.  Scan does no validation.  JSONText additionally
 // implements `Unmarshal`, which unmarshals the json within to an interface{}
-type JsonText json.RawMessage
+// WARNING: due to go issue https://github.com/golang/go/issues/13905 it is
+// potentially unsafe to use any []byte alias (like JSONText) unless you
+// use it as if it had the behavior of sql.RawBytes (ie. it ceases to be
+// valid at the next scan)
+type JSONText json.RawMessage
 
 // MarshalJSON returns the *j as the JSON encoding of j.
-func (j *JsonText) MarshalJSON() ([]byte, error) {
+func (j *JSONText) MarshalJSON() ([]byte, error) {
 	return *j, nil
 }
 
 // UnmarshalJSON sets *j to a copy of data
-func (j *JsonText) UnmarshalJSON(data []byte) error {
+func (j *JSONText) UnmarshalJSON(data []byte) error {
 	if j == nil {
-		return errors.New("JsonText: UnmarshalJSON on nil pointer")
+		return errors.New("JSONText: UnmarshalJSON on nil pointer")
 	}
 	*j = append((*j)[0:0], data...)
 	return nil
@@ -71,7 +79,7 @@ func (j *JsonText) UnmarshalJSON(data []byte) error {
 
 // Value returns j as a value.  This does a validating unmarshal into another
 // RawMessage.  If j is invalid json, it returns an error.
-func (j JsonText) Value() (driver.Value, error) {
+func (j JSONText) Value() (driver.Value, error) {
 	var m json.RawMessage
 	var err = j.Unmarshal(&m)
 	if err != nil {
@@ -81,26 +89,26 @@ func (j JsonText) Value() (driver.Value, error) {
 }
 
 // Scan stores the src in *j.  No validation is done.
-func (j *JsonText) Scan(src interface{}) error {
+func (j *JSONText) Scan(src interface{}) error {
 	var source []byte
 	switch src.(type) {
 	case string:
 		source = []byte(src.(string))
 	case []byte:
 		source = src.([]byte)
 	default:
-		return errors.New("Incompatible type for JsonText")
+		return errors.New("Incompatible type for JSONText")
 	}
-	*j = JsonText(append((*j)[0:0], source...))
+	*j = JSONText(append((*j)[0:0], source...))
 	return nil
 }
 
 // Unmarshal unmarshal's the json in j to v, as in json.Unmarshal.
-func (j *JsonText) Unmarshal(v interface{}) error {
+func (j *JSONText) Unmarshal(v interface{}) error {
 	return json.Unmarshal([]byte(*j), v)
 }
 
-// Pretty printing for JsonText types
-func (j JsonText) String() string {
+// Pretty printing for JSONText types
+func (j JSONText) String() string {
 	return string(j)
 }
diff --git a/types/types_test.go b/types/types_test.go
@@ -17,8 +17,8 @@ func TestGzipText(t *testing.T) {
 	}
 }
 
-func TestJsonText(t *testing.T) {
-	j := JsonText(`{"foo": 1, "bar": 2}`)
+func TestJSONText(t *testing.T) {
+	j := JSONText(`{"foo": 1, "bar": 2}`)
 	v, err := j.Value()
 	if err != nil {
 		t.Errorf("Was not expecting an error")
@@ -34,7 +34,7 @@ func TestJsonText(t *testing.T) {
 		t.Errorf("Expected valid json but got some garbage instead? %#v", m)
 	}
 
-	j = JsonText(`{"foo": 1, invalid, false}`)
+	j = JSONText(`{"foo": 1, invalid, false}`)
 	v, err = j.Value()
 	if err == nil {
 		t.Errorf("Was expecting invalid json to fail!")

Original file line number	Diff line number	Diff line change
`@@ -17,8 +17,8 @@ func TestGzipText(t *testing.T) {`
`17`	`17`	`}`
`18`	`18`	`}`
`19`	`19`
`20`		`-func TestJsonText(t *testing.T) {`
`21`		- j := JsonText(`{"foo": 1, "bar": 2}`)
	`20`	`+func TestJSONText(t *testing.T) {`
	`21`	+ j := JSONText(`{"foo": 1, "bar": 2}`)
`22`	`22`	`v, err := j.Value()`
`23`	`23`	`if err != nil {`
`24`	`24`	`t.Errorf("Was not expecting an error")`
`@@ -34,7 +34,7 @@ func TestJsonText(t *testing.T) {`
`34`	`34`	`t.Errorf("Expected valid json but got some garbage instead? %#v", m)`
`35`	`35`	`}`
`36`	`36`
`37`		- j = JsonText(`{"foo": 1, invalid, false}`)
	`37`	+ j = JSONText(`{"foo": 1, invalid, false}`)
`38`	`38`	`v, err = j.Value()`
`39`	`39`	`if err == nil {`
`40`	`40`	`t.Errorf("Was expecting invalid json to fail!")`