encoding/json: always ignore embedded pointers to unexported struct types

CL 60410 fixes a bug in reflect that allows assignments to an embedded
field of a pointer to an unexported struct type.
This breaks the json package because unmarshal is now unable to assign
a newly allocated struct to such fields.

In order to be consistent in the behavior for marshal and unmarshal,
this CL changes both marshal and unmarshal to always ignore
embedded pointers to unexported structs.

Fixes #21357

Change-Id: If62ea11155555e61115ebb9cfa5305caf101bde5
Reviewed-on: https://go-review.googlesource.com/76851
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/encoding/json/decode_test.go b/src/encoding/json/decode_test.go
index fc546bf2a7f634c422004eff51361ba92aad1389..27ceee471aec5c773e3b37eee7e3d6a97d0074fa 100644 (file)
--- a/src/encoding/json/decode_test.go
+++ b/src/encoding/json/decode_test.go
@@ -195,6 +195,11 @@ type embed struct {
        Q int
 }
 
+type Issue21357 struct {
+       *embed
+       R int
+}
+
 type Loop struct {
        Loop1 int `json:",omitempty"`
        Loop2 int `json:",omitempty"`
@@ -866,6 +871,20 @@ var unmarshalTests = []unmarshalTest{
                err: fmt.Errorf("json: unknown field \"extra\""),
                disallowUnknownFields: true,
        },
+
+       // Issue 21357.
+       // Ignore any embedded fields that are pointers to unexported structs.
+       {
+               in:  `{"Q":1,"R":2}`,
+               ptr: new(Issue21357),
+               out: Issue21357{R: 2},
+       },
+       {
+               in:  `{"Q":1,"R":2}`,
+               ptr: new(Issue21357),
+               err: fmt.Errorf("json: unknown field \"Q\""),
+               disallowUnknownFields: true,
+       },
 }
 
 func TestMarshal(t *testing.T) {

diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go
index 317a5a940d00770658a4f37bee4dce4b11e0cb39..66d1a183b0ee334498cd41a79c5faf110e3a9c20 100644 (file)
--- a/src/encoding/json/encode.go
+++ b/src/encoding/json/encode.go
@@ -1094,11 +1094,18 @@ func typeFields(t reflect.Type) []field {
                                isUnexported := sf.PkgPath != ""
                                if sf.Anonymous {
                                        t := sf.Type
-                                       if t.Kind() == reflect.Ptr {
+                                       isPointer := t.Kind() == reflect.Ptr
+                                       if isPointer {
                                                t = t.Elem()
                                        }
-                                       if isUnexported && t.Kind() != reflect.Struct {
-                                               // Ignore embedded fields of unexported non-struct types.
+                                       isStruct := t.Kind() == reflect.Struct
+                                       if isUnexported && (!isStruct || isPointer) {
+                                               // Ignore embedded fields of unexported non-struct types
+                                               // or pointers to unexported struct types.
+                                               //
+                                               // The latter is forbidden because unmarshal is unable
+                                               // to assign a new struct to the unexported field.
+                                               // See https://golang.org/issue/21357
                                                continue
                                        }
                                        // Do not ignore embedded fields of unexported struct types

diff --git a/src/encoding/json/encode_test.go b/src/encoding/json/encode_test.go
index 0f194e13d2681f33e8b3752ec276fd851640ceca..df7338c98d159abb6f2c540bdece2565ced925e1 100644 (file)
--- a/src/encoding/json/encode_test.go
+++ b/src/encoding/json/encode_test.go
@@ -364,8 +364,9 @@ func TestAnonymousFields(t *testing.T) {
                want: `{"X":2,"Y":4}`,
        }, {
                // Exported fields of pointers to embedded structs should have their
-               // exported fields be serialized regardless of whether the struct types
-               // themselves are exported.
+               // exported fields be serialized only for exported struct types.
+               // Pointers to unexported structs are not allowed since the decoder
+               // is unable to allocate a struct for that field
                label: "EmbeddedStructPointer",
                makeInput: func() interface{} {
                        type (
@@ -378,7 +379,7 @@ func TestAnonymousFields(t *testing.T) {
                        )
                        return S{&s1{1, 2}, &S2{3, 4}}
                },
-               want: `{"X":2,"Y":4}`,
+               want: `{"Y":4}`,
        }, {
                // Exported fields on embedded unexported structs at multiple levels
                // of nesting should still be serialized.