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: Unmarshal does not return error on ambiguous input #19194

Closed
sarathsp06 opened this issue Feb 20, 2017 · 5 comments
Closed

encoding/json: Unmarshal does not return error on ambiguous input #19194

sarathsp06 opened this issue Feb 20, 2017 · 5 comments
Labels
Documentation FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Go1.10

Comments

@sarathsp06
Copy link

Please answer these questions before submitting your issue. Thanks!

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

go version go1.8 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/sarath/go"
GORACE=""
GOROOT="/home/sarath/goroot"
GOTOOLDIR="/home/sarath/goroot/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build040972726=/tmp/go-build -gno-record-gcc-switches"
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?

  1. Created two structs with one field same in both and embedded both to a new struct
    Eg:

    type A struct{ X int }
type B struct{ X int }
type C struct {
       A
   	B
}

  2. Tried to assign C.X , it failed as expected with expected error ambiguous selector c.X

  3. Tried to unmarshal a JSON to C .It did not assign value to X but did not return any error ,which is unexpected

Here is the playground link : https://play.golang.org/p/SUlPSC2y3F

What did you expect to see?

Expected error to return on json.Unmarshal

What did you see instead?

No error returned and no field assignments either
@zegl
Copy link
Contributor

zegl commented Feb 20, 2017

Another interesting effect is that json.Marshal() of the C type in OPs playground example outputs nothing, and no error is returned.

Playground: https://play.golang.org/p/REonDIbRan

Is this the expected behaviour?

@bradfitz bradfitz changed the title encoding/json Unmarshal does not return error on ambigues input encoding/json: Unmarshal does not return error on ambiguous input Feb 24, 2017
@bradfitz bradfitz added this to the Go1.9Maybe milestone Feb 24, 2017
@bradfitz bradfitz added Documentation NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Feb 24, 2017
@bradfitz
Copy link
Contributor

There seems to at least be some documentation missing. Leaving for @rsc.

@odeke-em
Copy link
Member

Kindly requesting your input here @rsc.

@dmitshur
Copy link
Contributor

dmitshur commented Jul 24, 2017 •
edited

I'm not sure if I see a bug here. It seems to me that json.Unmarshal is working as documented.

Have you read this part of the encoding/json docs?

The Go visibility rules for struct fields are amended for JSON when deciding which field to marshal or unmarshal. If there are multiple fields at the same level, and that level is the least nested (and would therefore be the nesting level selected by the usual Go rules), the following extra rules apply:

  1. Of those fields, if any are JSON-tagged, only tagged fields are considered, even if there are multiple untagged fields that would otherwise conflict.

  2. If there is exactly one field (tagged or not according to the first rule), that is selected.

  3. Otherwise there are multiple fields, and all are ignored; no error occurs.

In the original sample, what's happening is that there are multiple conflicting JSON-tagged fields. Therefore, the last rule applies. All fields are ignored, no error occurs.

Do you see anything wrong with my analysis?

@ianlancetaylor ianlancetaylor added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Aug 3, 2017
@ianlancetaylor ianlancetaylor modified the milestones: Go1.10, Go1.9Maybe Aug 3, 2017
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@golang golang locked and limited conversation to collaborators Sep 3, 2018
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
8 participants
@bradfitz @zegl @rsc @sarathsp06 @dmitshur @ianlancetaylor @odeke-em @gopherbot