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: inconsistent behavior in *(numeric type), string tag option, and nil values #8587

Closed
matt-duch opened this issue Aug 25, 2014 · 3 comments

Comments

Projects
None yet
4 participants
@matt-duch
Copy link

commented Aug 25, 2014

What does 'go version' print?
go version go1.3.1 linux/amd64

What steps reproduce the problem?
http://play.golang.org/p/s_AebxMbdg

1. Define a struct with a field of type *int64, with the json ,string tag option
2. Create a new instance of that struct, leaving the field nil
3. Marshal with json.Marshal
4. Unmarshal with json.Unmarshal

What happened?
Error during unmarshalling: err unmarshalling json: invalid use of ,string struct tag,
trying to unmarshal "" into *int64

What should have happened instead?
Successfully decoded the json string from encoding the zero value of the struct

Please provide any additional information below.
In encoding/json/decode.go:
func (d *decodeState) object(v reflect.Value):
566         if destring {
   567              d.value(reflect.ValueOf(&d.tempstr))
   568              d.literalStore([]byte(d.tempstr), subv, true)
   569              d.tempstr = "" // Zero scratch space for successive values.
   570          } else {
   571              d.value(subv)
   572          }

After a call to d.value, the null literal is handled and the d.tempstr field is left
empty. The call to d.literalStore then assumes it received an empty string
(""), as this is what d.value would do in the case that the json value was an
empty string, and results in an error being returned in func (d *decodeState)
literalStore, L631. At the moment there doesn't seem to be an easy way to differentiate
the two cases. 
One note (not sure if it's relevant or not) is that it seems tempstr is only used with a
,string tag option. 

I've worked through a few ideas, but they end up requiring a wrapper around the
d.tempstr field (converted to an []byte or *string) to determine whether or not it is
nil or an empty string, or a new field to track that difference. 

Not sure if it's worth it (or desirable), but a final comment is that as this logic
assumes the ,string case:
 629        if len(item) == 0 {
   630          //Empty string given
   631          d.saveError(fmt.Errorf("json: invalid use of ,string struct tag, trying to unmarshal %q into %v", item, v.Type()))
   632          return
   633      }
and is therefore only reached in the if destring {} block (L567), it should be possible
to move it out of the literalStore func and into the if destring {} block.
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2014

Comment 1:

Labels changed: added repo-main, release-go1.4.

@gopherbot

This comment has been minimized.

Copy link

commented Oct 7, 2014

Comment 2:

CL https://golang.org/cl/152270044 mentions this issue.
@rsc

This comment has been minimized.

Copy link
Contributor

commented Oct 7, 2014

Comment 3:

This issue was closed by revision 7b2b8ed.

Status changed to Fixed.

@matt-duch matt-duch added fixed labels Oct 7, 2014

@rsc rsc added this to the Go1.4 milestone Apr 14, 2015

@rsc rsc removed the release-go1.4 label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 25, 2016

wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018

encoding/json: fix handling of null with ,string fields
Fixes golang#8587.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/152270044

wheatman added a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018

encoding/json: fix handling of null with ,string fields
Fixes golang#8587.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/152270044

wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018

encoding/json: fix handling of null with ,string fields
Fixes golang#8587.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/152270044

wheatman added a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018

encoding/json: fix handling of null with ,string fields
Fixes golang#8587.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/152270044

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.