Skip to content

connection: interpolate json.RawMessage as string#1058

Merged
julienschmidt merged 1 commit intogo-sql-driver:masterfrom
alexsn:interpolate_jsonrawmsg
Feb 9, 2020
Merged

connection: interpolate json.RawMessage as string#1058
julienschmidt merged 1 commit intogo-sql-driver:masterfrom
alexsn:interpolate_jsonrawmsg

Conversation

@alexsn
Copy link
Copy Markdown
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

@alexsn
connection: interpolate json.RawMessage as string (go-sql-driver#819)
5fb27bb 
json encoded data is represented as bytes however it should be
interpolated as a string.

Signed-off-by: Alex Snast <alexsn@fb.com>
julienschmidt
julienschmidt approved these changes Feb 9, 2020
View reviewed changes
Copy link
Copy Markdown
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
@a8m
stmt: add json.RawMessage for converter and prepared statement
29530f3 
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
@a8m
stmt: add json.RawMessage for converter and prepared statement
490a653 
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
@a8m
stmt: add json.RawMessage for converter and prepared statement
5e9c396 
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 a8m mentioned this pull request Feb 10, 2020
Merged
5 tasks
a8m added a commit to a8m/mysql that referenced this pull request Feb 14, 2020
@a8m
stmt: add json.RawMessage for converter and prepared statement
a9d6280 
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
@bitcoinbarron2
Revert "connection: interpolate json.RawMessage as string (go-sql-dri…
39baf7c 
…ver#1058)"

This reverts commit c4f1976.
a8m added a commit to a8m/mysql that referenced this pull request Feb 18, 2020
@a8m
stmt: add json.RawMessage for converter and prepared statement
b0a4d1f 
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
@a8m
stmt: add json.RawMessage for converter and prepared statement
f0d5b11 
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
@a8m
stmt: add json.RawMessage for converter and prepared statement (#1059)
3d8a029 
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
Copy Markdown
Contributor

dolmen commented Mar 26, 2020 •
edited
Loading

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
Copy Markdown
Contributor Author

alexsn commented Mar 26, 2020 •
edited
Loading

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
@alexsn @tz70s
connection: interpolate json.RawMessage as string (go-sql-driver#1058)
411d701 
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
@a8m @tz70s
stmt: add json.RawMessage for converter and prepared statement (go-sq…
fc12094 
…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
@alexsn @tz70s
connection: interpolate json.RawMessage as string (go-sql-driver#1058)
c5008b7 
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
@a8m @tz70s
stmt: add json.RawMessage for converter and prepared statement (go-sq…
c395bfb 
…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
@Mr-Dai Mr-Dai mentioned this pull request Sep 19, 2025
Closed
Copilot AI mentioned this pull request Sep 19, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@julienschmidt julienschmidt julienschmidt approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alexsn @dolmen @julienschmidt