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

encoding/json: error when unmarshaling empty string to int #22182

Closed
xtudouh opened this issue Oct 9, 2017 · 12 comments
Closed

encoding/json: error when unmarshaling empty string to int #22182

xtudouh opened this issue Oct 9, 2017 · 12 comments
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@xtudouh
Copy link

xtudouh commented Oct 9, 2017

Please answer these questions before submitting your issue. Thanks!

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

go1.8

Does this issue reproduce with the latest release?

yes

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

amd64

What did you do?

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

What did you expect to see?

json.Unmarshal should return nil

What did you see instead?

If try to unmarshal an empty string, it should return 0, instead of an error

@ianlancetaylor ianlancetaylor changed the title encoding/json unmarshal empty string to int return error encoding/json: error when unmarshaling empty string to int Oct 9, 2017
@ianlancetaylor
Copy link
Contributor

The JSON decoder has acted this way since https://golang.org/cl/6035050 in 2012. I don't know what the correct behavior is for unmarshaling an empty string into an int field.

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 9, 2017
@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Oct 9, 2017
@xtudouh
Copy link
Author

xtudouh commented Oct 9, 2017

OK. I have defined a new type to implement my behavior. I just think the behavior now is strange, anyway for the object to receive result will return ZERO value, why doesn't it return 0 directly.

@ianlancetaylor
Copy link
Contributor

To the extent that there is a JSON specification, we should follow it. However, I do not know what the JSON specification says for an empty string.

@bmerrill42
Copy link
Contributor

bmerrill42 commented Oct 9, 2017

json.org says "A string is a sequence of zero or more Unicode characters, wrapped in double quotes, using backslash escapes." so "" is valid go just has trouble translating it gracefully.
changing data := []byte(`{"a":""}`) to data := []byte(`{"a":null}`) or data := []byte(`{"a":"null"}`) works just fine though.

@kevinburke
Copy link
Contributor

kevinburke commented Oct 10, 2017

It's a valid JSON value, sure, but I think it should probably be an error to unmarshal any string into an int. I'm not sure the specification covers how values should be deserialized into native code.

JSON is so weakly typed and unexpected type conversion errors are a cause of so many problems I think it's better to try to enforce type errors when we can. Better to error when a type is confusing than to silently try to convert it, I think.

I'm open to changing my mind if you can demonstrate the existing Go behavior allows {"a": 0} to be unmarshaled into a struct of type string, or {"a": false} unmarshallable into an int, or any other conversion of native types besides null.

When you say data := []byte(`{"a":"null"}`) "works", what do you mean? You can destructure that object into an int field?

@xtudouh
Copy link
Author

xtudouh commented Oct 10, 2017

I think we can extend behavior of go-JSON-package, as annotation is supported, why don't we add one more, e.g: default value for different types conversion

@theckman
Copy link
Contributor

theckman commented Oct 12, 2017

I agree with @kevinburke.

I don't think this is a bug or something that should be changed. As a Go developer, who consumes JSON, if I define my field as an int I expect Go to error when trying to parse the JSON if the type is wrong. I want Go to enforce types, as best it can, with the JSON data I'm giving it.

Edit: I wanted to elaborate further. I think if you want to have behavior like this inside of Go, you should go down the path @xtudouh went and implement your own type. That allows you to have complete control over what is valid, or an error, without the Go standard library needing to make potentially unsafe assumptions.

@ghost
Copy link

ghost commented Oct 12, 2017

@kevinburke

It's a valid JSON value, sure, but I think it should probably be an error to unmarshal any string into an int.

But Go already allows this. That's what this means:

type Demo struct {
A int json:"a,string"
}

Parse a string as an Integer, erroring if the string does not contain a valid Integer representation.

JSON is so weakly typed and unexpected type conversion errors are a cause of so many problems I think it's better to try to enforce type errors when we can.

And it does. In this example it requires the JSON value to be a string, AND it requires the string to contain a valid Integer representation. Try this and see what happens, even though the target type is an int:

func main() {
data := []byte({"a":0})

This works fine:

func main() {
data := []byte({"a":"42"})

As does this:

func main() {
data := []byte({"a":"0"})

I guess the question is what should happen to this?

func main() {
data := []byte({"a":""})

@theckman

I don't think this is a bug or something that should be changed. As a Go developer, who consumes JSON, if I define my field as an int I expect Go to error when trying to parse the JSON if the type is wrong.

But it already allows for that as described above. Type conversion already occurs when you describe a struct field as: A int json:"a,string". In fact type conversion is required - the input MUST be a string, and the string MUST be parsable into an int.

The only question we are left with is should "" be parsed as "0"? I would say no, as "" is not a valid integer representation.

@theckman
Copy link
Contributor

@fcntl I think I"m of the opinion that "" should fail to parse in to an integer type, as should "bacon".

@JohnRoesler
Copy link

I think an argument could be made for parsing "" to 0 given the situation described above:

type Demo struct {
A int json:"a,string"
}

as the "zero" value of a string is "" and the "zero" value of an Int is 0. However, I would rather do data validation and determine how to handle a string value of "" as it may be different based on the use case. So, in summary I prefer having a conversion of json string "" to Int result in an error.

@leighmcculloch
Copy link
Contributor

leighmcculloch commented Oct 21, 2017

I think there's value in retaining consistency when encoding/decoding between different types in the stdlib, otherwise the behavior of parsing integers becomes unintuitive. The strconv package treats an empty string as an error, and the encoders/decoders should probably continue to do the same.

Example:

_, err := strconv.Atoi("")
fmt.Println(err)

Outputs:

strconv.Atoi: parsing "": invalid syntax

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

@rsc
Copy link
Contributor

rsc commented Nov 22, 2017

Since the empty string is not the string form of any integer and the code has worked this way since 2012, we're not going to change this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

9 participants