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

connection: interpolate json.RawMessage as string #1058

Merged
merged 1 commit into from Feb 9, 2020

Conversation

alexsn
Copy link
Contributor

@alexsn alexsn commented Feb 8, 2020

json encoded data is represented as bytes however it should be
interpolated as a string.

Signed-off-by: Alex Snast alexsn@fb.com

Description

On argument interpolation treat json.RawMessage as a string instead of []bytes to prevents _binary statement injection.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Added myself / the copyright holder to the AUTHORS file

json encoded data is represented as bytes however it should be
interpolated as a string.

Signed-off-by: Alex Snast <alexsn@fb.com>
Copy link
Member

@julienschmidt julienschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@julienschmidt julienschmidt changed the title connection: interpolate json.RawMessage as string (#819) connection: interpolate json.RawMessage as string Feb 9, 2020
@julienschmidt julienschmidt merged commit c4f1976 into go-sql-driver:master Feb 9, 2020
@alexsn alexsn deleted the interpolate_jsonrawmsg branch February 9, 2020 15:00
a8m added a commit to a8m/mysql that referenced this pull request Feb 10, 2020
Following go-sql-driver#1058, in order for the driver.Value to get as a json.RawMessage,
the converter should accept it as a valid value, and handle it as bytes in
case where interpolation is disabled
a8m added a commit to a8m/mysql that referenced this pull request Feb 10, 2020
Following go-sql-driver#1058, in order for the driver.Value to get as a json.RawMessage,
the converter should accept it as a valid value, and handle it as bytes in
case where interpolation is disabled
a8m added a commit to a8m/mysql that referenced this pull request Feb 10, 2020
Following go-sql-driver#1058, in order for the driver.Value to get as a json.RawMessage,
the converter should accept it as a valid value, and handle it as bytes in
case where interpolation is disabled
a8m added a commit to a8m/mysql that referenced this pull request Feb 14, 2020
Following go-sql-driver#1058, in order for the driver.Value to get as a json.RawMessage,
the converter should accept it as a valid value, and handle it as bytes in
case where interpolation is disabled
bitcoinbarron2 added a commit to bitcoinbarron2/mysql that referenced this pull request Feb 18, 2020
a8m added a commit to a8m/mysql that referenced this pull request Feb 18, 2020
Following go-sql-driver#1058, in order for the driver.Value to get as a json.RawMessage,
the converter should accept it as a valid value, and handle it as bytes in
case where interpolation is disabled
a8m added a commit to a8m/mysql that referenced this pull request Feb 18, 2020
Following go-sql-driver#1058, in order for the driver.Value to get as a json.RawMessage,
the converter should accept it as a valid value, and handle it as bytes in
case where interpolation is disabled
methane pushed a commit that referenced this pull request Feb 18, 2020
Following #1058, in order for the driver.Value to get as a json.RawMessage,
the converter should accept it as a valid value, and handle it as bytes in
case where interpolation is disabled
@dolmen
Copy link
Contributor

dolmen commented Mar 26, 2020

I'm sad this has been merged.
The mysql driver now has a strong dependency on a particular implementation of JSON encoding/decoding (this implementation being in the stdlib is not an excuse). How the growth of dependencies will stop if such patches are accepted?

This is just a user error to pass a json.RawMessage to the driver as this is not a driver.Value. The user should just not pass json.RawMessage. Use instead a type that wraps json.RawMessage and implements driver.Valuer and sql.Scanner, but not in the driver itself.

I wrote this function that allows to wrap a json.RawMessage (or any other type that is is serialized as JSON in a DB column), and this doesn't require to patch any SQL drivers.

var errInvalidJSONWrapperValue = errors.New("invalid target value for sqlutil.JSON wrapper")
var errInvalidJSONValue = errors.New("invalid value for sqlutil.JSON target")

// JSON is an helper to wrap a value for I/O to a database column as a JSON string
func JSON(p interface{}) driver.Value {
	if p == nil {
		panic(errInvalidJSONWrapperValue)
	}
	v := reflect.ValueOf(p)
	if v.Kind() == reflect.Ptr && !v.IsNil() && v.Elem().CanSet() {
		return jsonScanner{jsonValuer{p}}
	} else {
		return jsonValuer{p}
	}
}

type jsonValuer struct {
	target interface{}
}

type jsonScanner struct {
	jsonValuer
}

// Value implements interface driver.Valuer for both jsonValuer and jsonScanner
func (j jsonValuer) Value() (driver.Value, error) {
	return json.Marshal(j.target)
}

// Scan implements interface sql.Scanner
func (j jsonScanner) Scan(value interface{}) error {
	switch value := value.(type) {
	case []byte:
		return json.Unmarshal(value, j.target)
	case string:
		return json.Unmarshal([]byte(value), j.target)
	default:
		target := reflect.ValueOf(j.target)
		source := reflect.ValueOf(value)
		if value == nil {
			source = reflect.Zero(target.Elem().Type())
		}
		if !source.Type().AssignableTo(target.Elem().Type()) {
			return errInvalidJSONValue
		}
		target.Elem().Set(source)
		return nil
	}
}

@alexsn
Copy link
Contributor Author

alexsn commented Mar 26, 2020

The fact that we use types from stdlib doesn't add any dependency so I'm not sure what you mean by saying "How the growth of dependencies will stop ...".

tz70s pushed a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
json encoded data is represented as bytes however it should be interpolated as a string

Fixes go-sql-driver#819
tz70s pushed a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
…l-driver#1059)

Following go-sql-driver#1058, in order for the driver.Value to get as a json.RawMessage,
the converter should accept it as a valid value, and handle it as bytes in
case where interpolation is disabled
tz70s pushed a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
json encoded data is represented as bytes however it should be interpolated as a string

Fixes go-sql-driver#819
tz70s pushed a commit to tz70s/mysql that referenced this pull request Sep 5, 2020
…l-driver#1059)

Following go-sql-driver#1058, in order for the driver.Value to get as a json.RawMessage,
the converter should accept it as a valid value, and handle it as bytes in
case where interpolation is disabled
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

Successfully merging this pull request may close these issues.

None yet

3 participants