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

cmd/compile: commit d349fa25 breaks unmarshaling public fields of embedded private structs #22546

Closed
wardn opened this issue Nov 2, 2017 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@wardn
Copy link

wardn commented Nov 2, 2017

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

commit 812b34e: good
commit d349fa2: bad

Does this issue reproduce with the latest release?

the issue reproduces on master, it is not on the 1.9 release branch

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

freebsd/amd64
linux/amd64
darwin/amd64
windows/amd64

What did you do?

https://play.golang.org/p/6ABGl7lNJA

package main

import (
	"encoding/json"
)

func main() {
	b := []byte(`{"ID": "1"}`)
	var m Manager
	json.Unmarshal(b, &m)
	println(m.ID)
}

type Manager struct {
	*employee
}

type employee struct {
	ID string
}

What did you expect to see?

1

What did you see instead?

panic: reflect: reflect.Value.Set using value obtained using unexported field [recovered]
panic: interface conversion: string is not error: missing method Error

goroutine 1 [running]:
encoding/json.(*decodeState).unmarshal.func1(0xc42003bed8)
/usr/home/user/dev/golang/src/encoding/json/decode.go:175 +0x8f
panic(0x4b20c0, 0xc420072210)
/usr/home/user/dev/golang/src/runtime/panic.go:492 +0x266
reflect.flag.mustBeAssignable(0x1d6)
/usr/home/user/dev/golang/src/reflect/value.go:224 +0x200
reflect.Value.Set(0x4aca20, 0xc42008c018, 0x1d6, 0x4aca20, 0xc420072200, 0x16)
/usr/home/user/dev/golang/src/reflect/value.go:1344 +0x2f
encoding/json.(*decodeState).object(0xc42009a000, 0x4ac9e0, 0xc42008c018, 0x16)
/usr/home/user/dev/golang/src/encoding/json/decode.go:707 +0x3fa
encoding/json.(*decodeState).value(0xc42009a000, 0x4ac9e0, 0xc42008c018, 0x16)
/usr/home/user/dev/golang/src/encoding/json/decode.go:405 +0x2d8
encoding/json.(*decodeState).unmarshal(0xc42009a000, 0x4ac9e0, 0xc42008c018, 0x0, 0x0)
/usr/home/user/dev/golang/src/encoding/json/decode.go:187 +0x201
encoding/json.Unmarshal(0xc420086050, 0xb, 0x10, 0x4ac9e0, 0xc42008c018, 0x10, 0x49f939)
/usr/home/user/dev/golang/src/encoding/json/decode.go:107 +0x148
main.main()
/home/user/dev/go/src/serializer/main.go:10 +0xa

@odeke-em
Copy link
Member

odeke-em commented Nov 2, 2017

Thank you for the report @wardn!

/cc @mdempsky

@odeke-em odeke-em added this to the Go1.10 milestone Nov 2, 2017
@odeke-em odeke-em added the NeedsFix The path to resolution is known, but the work has not been done. label Nov 2, 2017
@dsnet
Copy link
Member

dsnet commented Nov 2, 2017

Duplicate of #21357.

This is (sort of) working as intended. The json package was relying on a bug in the reflect package. @mdempsky fixed that bug. The work to be done is to get json.Unmarshal to not panic.

Fundamentally, it is impossible for json to unmarshal into Manager.employee because it can't assign into an unexported field. The Go language spec forbids that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants