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
Milestone

Comments

@matt-duch
Copy link

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
Copy link
Contributor

Comment 1:

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

@gopherbot
Copy link

Comment 2:

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

@rsc
Copy link
Contributor

rsc commented Oct 7, 2014

Comment 3:

This issue was closed by revision 7b2b8ed.

Status changed to Fixed.

@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 pushed a commit to wheatman/go-akaros that referenced this issue Jun 25, 2018
Fixes golang#8587.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/152270044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jun 26, 2018
Fixes golang#8587.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/152270044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 9, 2018
Fixes golang#8587.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews, iant, r
https://golang.org/cl/152270044
wheatman pushed a commit to wheatman/go-akaros that referenced this issue Jul 30, 2018
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.
Projects
None yet
Development

No branches or pull requests

4 participants