Skip to content

encoding/json: ignore mismatched null during unmarshal? #2540

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.

Sign up for GitHub

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
Closed
gopherbot opened this issue Dec 8, 2011 · 13 comments
Closed

encoding/json: ignore mismatched null during unmarshal? #2540

gopherbot opened this issue Dec 8, 2011 · 13 comments
Labels
FrozenDueToAge
Milestone
Go1.1

Comments

@gopherbot
Copy link
Contributor

by dahankzter:

Before filing a bug, please check whether it has been fixed since
the latest release: run "hg pull", "hg update default", rebuild, and
retry
what you did to
reproduce the problem.  Thanks.

What steps will reproduce the problem?
1. Create a struct with a string field
type T struct {
    S string
}

2. Create json with the nested string field null
{\"s\" : null}

3. Unmarshal it

What is the expected output?
I expect no error to be returned from the json.Unmarshall

What do you see instead?
Error message:
Unmarshal error: json: cannot unmarshal null into Go value of type string

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
Arch Linux x86_64

Which revision are you using?  (hg identify)
5190380293e8+ tip

Please provide any additional information below.
The patch:

diff -r 5190380293e8 src/pkg/encoding/json/decode.go
--- a/src/pkg/encoding/json/decode.go   Thu Dec 08 15:12:08 2011 +0900
+++ b/src/pkg/encoding/json/decode.go   Thu Dec 08 18:06:53 2011 +0100
@@ -592,7 +592,7 @@
        switch v.Kind() {
        default:
            d.saveError(&UnmarshalTypeError{"null", v.Type()})
-       case reflect.Interface, reflect.Ptr, reflect.Map, reflect.Slice:
+       case reflect.Interface, reflect.Ptr, reflect.Map, reflect.Slice, reflect.String:
            v.Set(reflect.Zero(v.Type()))
        }

Should be tentatively sufficient.
@rsc
Copy link
Contributor

rsc commented Dec 9, 2011

Comment 1:

Labels changed: added priority-later, removed priority-medium.

@rsc
Copy link
Contributor

rsc commented Dec 12, 2011

Comment 2:

I know there's a lot of bad JSON floating around, but null is not a string.
If you must deal with bad JSON like this, I suggest
defining your own type with a custom UnmarshalJSON method.
Russ

Status changed to WorkingAsIntended.

@gopherbot
Copy link
Contributor Author

Comment 3 by dahankzter:

I agree but isn't it so that the resulting struct is identical with or without the
applied change? The only difference seems to be that there is an error returned.
Upon checking the status of the returned error I would note that something went wrong
and act accordingly when in fact there was nothing wrong other than that the
unmarshalling noted a null string and decided not to set that struct element value.
I may have missed something crucial in the code but to me it seems at least wrong to
return an error in this case although I agree that null is not a string.
Henrik

@rsc
Copy link
Contributor

rsc commented Dec 14, 2011

Comment 4:

So should {"s": 42} not return an error either, silently doing nothing?

@gopherbot
Copy link
Contributor Author

Comment 5 by dahankzter:

No because the intent is clear. The creator of the json string means for this to be an
int. In the case of null it is just as undefined as leaving it out of the data.
You don't agree that null is undefined and different from 42?

@rsc
Copy link
Contributor

rsc commented Dec 14, 2011

Comment 6:

Both null and 42 are values that cannot be stored in a string.

@gopherbot
Copy link
Contributor Author

Comment 7 by dahankzter:

I know but my argument is just pragmatic. Any json null value should be handled as if
absent unless the target type allows nil then it should be set such.
I am not making any other point really.

@rsc
Copy link
Contributor

rsc commented Dec 15, 2011

Comment 8:

Status changed to Thinking.

@rsc
Copy link
Contributor

rsc commented Jan 29, 2012

Comment 9:

We should probably do this, but after Go 1.

Status changed to Accepted.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 10:

Labels changed: added go1.1.

@gopherbot
Copy link
Contributor Author

Comment 11 by rickarnoldjr:

Fixed by: https://golang.org/cl/6759043/

@rsc
Copy link
Contributor

rsc commented Nov 12, 2012

Comment 13:

This issue was closed by revision c90739e.

Status changed to Fixed.

@gopherbot
Copy link
Contributor Author

Comment 14:

CL https://golang.org/cl/100430043 mentions this issue.

@rsc rsc added this to the Go1.1 milestone Apr 14, 2015
@rsc rsc removed the go1.1 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.1
Development

No branches or pull requests
2 participants
@rsc @gopherbot