Navigation Menu

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

Jump to bottom

encoding/json: marshal of custom type not quoted when using ",string" tag field #20651

Open
aronatkins opened this issue Jun 12, 2017 · 5 comments
Open

encoding/json: marshal of custom type not quoted when using ",string" tag field #20651

aronatkins opened this issue Jun 12, 2017 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. v2 A language change or incompatible library change
Milestone
Go2

Comments

@aronatkins
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.8.3 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/var/lib/jenkins/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build418778517=/tmp/go-build"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

We receive JSON which encodes booleans as numeric strings. Parsing that JSON works just fine with a ",string" tag and a type with its own UnmarshalJSON definition. The quoting is removed and we get the proper boolean value.

Quoting is not added when serializing this type, which means that JSON IN>decode>encode>OUT produces OUT different than IN.

type parsedBool bool

func (basi *parsedBool) UnmarshalJSON(data []byte) error {
	v, err := strconv.ParseBool(string(data))
	if err != nil {
		return err
	}
	*basi = parsedBool(v)
	return nil
}

func (basi parsedBool) MarshalJSON() ([]byte, error) {
	if basi {
		return []byte(`1`), nil
	} else {
		return []byte(`0`), nil
	}
}

https://play.golang.org/p/ccIw4E6UJF

What did you expect to see?

Equivalent support for ",string" for this custom type when both encoding and decoding. I was surprised by the difference in behavior. Both cases should either err (because parsedBool is not bool) or properly handle the quotes.

The json docs state:

The "string" option signals that a field is stored as JSON inside a JSON-encoded string. It applies only to fields of string, floating point, integer, or boolean types.

What did you see instead?

Given:

	wrapped := struct {
		P parsedBool `json:",string"`
		B bool       `json:",string"`
	}{
		P: true,
		B: true,
	}

This is marshaled as:

{"P":1,"B":"true"}

This was unexpected because we properly parse:

{"P":"1","B":"true"}
@OneOfOne
Copy link
Contributor

You are using a custom marshal function, not sure why is that unexpected?

@mvdan
Copy link
Member

mvdan commented Jun 13, 2017

Perhaps the package docs should be improved to clarify that ,string gets ignored when using a Marshaler/Unmarshaler.

@aronatkins
Copy link
Author

@OneOfOne + @mvdan The primary concern is the inconsistency between reading/writing JSON. Note that the quoting is removed when parsing JSON but not added when creating JSON.

The inconsistency can be addressed either by supporting quoting when serializing ",string"-annotated custom types (which are still primitives under the hood) OR by removing support for quote-removal when deserializing ",string"-annotated custom types.

Both options have backwards-compatibility issues, as programs are likely to have already worked around the observed behavior.

@andybons
Copy link
Member

Marking for Go2 due to backwards-incompatible changes that would need to be made.

@andybons andybons added the v2 A language change or incompatible library change label Apr 11, 2018
@andybons andybons added this to the Go2 milestone Apr 11, 2018
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 17, 2018
@toby1991
Copy link

toby1991 commented Jun 13, 2019 •
edited

@OneOfOne + @mvdan The primary concern is the inconsistency between reading/writing JSON. Note that the quoting is removed when parsing JSON but not added when creating JSON.

The inconsistency can be addressed either by supporting quoting when serializing ",string"-annotated custom types (which are still primitives under the hood) OR by removing support for quote-removal when deserializing ",string"-annotated custom types.

Both options have backwards-compatibility issues, as programs are likely to have already worked around the observed behavior.

@aronatkins I've found the solution...

type BigInt struct {
	_bi big.Int
}
func (bi BigInt) String() string {
	return bi._bi.String()
}

// before
//func (bi BigInt) MarshalJSON() ([]byte, error) {
//	return []byte(bi._bi.String()), nil
//}

// now
func (bi BigInt) MarshalJSON() ([]byte, error) {
	return []byte(`"` + bi._bi.String() + `"`), nil
}

You jus need to add the " or ' to quote the string data.. I think that will make the json package consider that the data is a string type

I fixed this issue by using the tricks above. @totoval
https://github.com/totoval/framework/blob/7b3016a86bb1e41c56d90668d63ddd929e3f34a7/model/types/bigint/bigint.go#L67

For your example,
https://play.golang.org/p/7N0c1Q4_UOF

toby1991 referenced this issue in totoval/framework Jun 13, 2019
@toby1991
fix bigfloat, bigint marshalJSON result is not str
7b3016a 
Signed-off-by: Toby Yan <me@tobyan.com>
@lmittmann lmittmann mentioned this issue Nov 11, 2021
Closed
@aronatkins aronatkins mentioned this issue Feb 3, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. v2 A language change or incompatible library change
Projects
None yet
Milestone
Go2
Development

No branches or pull requests
6 participants
@aronatkins @andybons @OneOfOne @ianlancetaylor @mvdan @toby1991