]> Cypherpunks repositories - gostls13.git/commitdiff
encoding/json: ignore embedded fields of pointers to unexported non-structs
authorJoe Tsai <joetsai@digital-static.net>
Fri, 21 Jul 2017 23:53:54 +0000 (16:53 -0700)
committerJoe Tsai <joetsai@google.com>
Sat, 22 Jul 2017 01:29:58 +0000 (01:29 +0000)
https://golang.org/cl/33773 fixes the JSON marshaler to avoid serializing
embedded fields on unexported types of non-struct types. However, Go allows
embedding pointer to types, so the check for whether the field is a non-struct
type must first dereference the pointer to get at the underlying type.

Furthermore, due to a edge-case in the behavior of StructField.PkgPath not
being a reliable indicator of whether the field is unexported (see #21122),
we use our own logic to determine whether the field is exported or not.

The logic in this CL may be simplified depending on what happens in #21122.

Fixes #21121
Updates #21122

Change-Id: I8dfd1cdfac8a87950df294a566fb96dfd04fd749
Reviewed-on: https://go-review.googlesource.com/50711
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/encoding/json/encode.go
src/encoding/json/encode_test.go

index 6fcea4735f931315425f2078b55faa2e2e65d8dd..0371f0a24dd3543f79402bc066fc3fae7779eeda 100644 (file)
@@ -1093,7 +1093,22 @@ func typeFields(t reflect.Type) []field {
                        // Scan f.typ for fields to include.
                        for i := 0; i < f.typ.NumField(); i++ {
                                sf := f.typ.Field(i)
-                               if sf.PkgPath != "" && (!sf.Anonymous || sf.Type.Kind() != reflect.Struct) { // unexported
+                               if sf.Anonymous {
+                                       t := sf.Type
+                                       if t.Kind() == reflect.Ptr {
+                                               t = t.Elem()
+                                       }
+                                       // If embedded, StructField.PkgPath is not a reliable
+                                       // indicator of whether the field is exported.
+                                       // See https://golang.org/issue/21122
+                                       if !isExported(t.Name()) && t.Kind() != reflect.Struct {
+                                               // Ignore embedded fields of unexported non-struct types.
+                                               // Do not ignore embedded fields of unexported struct types
+                                               // since they may have exported fields.
+                                               continue
+                                       }
+                               } else if sf.PkgPath != "" {
+                                       // Ignore unexported non-embedded fields.
                                        continue
                                }
                                tag := sf.Tag.Get("json")
@@ -1211,6 +1226,12 @@ func typeFields(t reflect.Type) []field {
        return fields
 }
 
+// isExported reports whether the identifier is exported.
+func isExported(id string) bool {
+       r, _ := utf8.DecodeRuneInString(id)
+       return unicode.IsUpper(r)
+}
+
 // dominantField looks through the fields, all of which are known to
 // have the same name, to find the single field that dominates the
 // others using Go's embedding rules, modified by the presence of
index d5f5f0a691d3587fe965fa3d18c9ff7284cebe08..3fda6a0c719d431af2f04fec90ba53bca652c80a 100644 (file)
@@ -253,76 +253,167 @@ func TestMarshalerEscaping(t *testing.T) {
        }
 }
 
-type IntType int
-
-type MyStruct struct {
-       IntType
-}
-
-func TestAnonymousNonstruct(t *testing.T) {
-       var i IntType = 11
-       a := MyStruct{i}
-       const want = `{"IntType":11}`
-
-       b, err := Marshal(a)
-       if err != nil {
-               t.Fatalf("Marshal: %v", err)
-       }
-       if got := string(b); got != want {
-               t.Errorf("got %q, want %q", got, want)
-       }
-}
-
-type unexportedIntType int
-
-type MyStructWithUnexportedIntType struct {
-       unexportedIntType
-}
-
-func TestAnonymousNonstructWithUnexportedType(t *testing.T) {
-       a := MyStructWithUnexportedIntType{11}
-       const want = `{}`
-
-       b, err := Marshal(a)
-       if err != nil {
-               t.Fatalf("Marshal: %v", err)
-       }
-       if got := string(b); got != want {
-               t.Errorf("got %q, want %q", got, want)
-       }
-}
-
-type MyStructContainingUnexportedStruct struct {
-       unexportedStructType1
-       unexportedIntType
-}
-
-type unexportedStructType1 struct {
-       ExportedIntType1
-       unexportedIntType
-       unexportedStructType2
-}
-
-type unexportedStructType2 struct {
-       ExportedIntType2
-       unexportedIntType
-}
-
-type ExportedIntType1 int
-type ExportedIntType2 int
-
-func TestUnexportedAnonymousStructWithExportedType(t *testing.T) {
-       s2 := unexportedStructType2{3, 4}
-       s1 := unexportedStructType1{1, 2, s2}
-       a := MyStructContainingUnexportedStruct{s1, 6}
-       const want = `{"ExportedIntType1":1,"ExportedIntType2":3}`
+func TestAnonymousFields(t *testing.T) {
+       tests := []struct {
+               label     string             // Test name
+               makeInput func() interface{} // Function to create input value
+               want      string             // Expected JSON output
+       }{{
+               // Both S1 and S2 have a field named X. From the perspective of S,
+               // it is ambiguous which one X refers to.
+               // This should not serialize either field.
+               label: "AmbiguousField",
+               makeInput: func() interface{} {
+                       type (
+                               S1 struct{ x, X int }
+                               S2 struct{ x, X int }
+                               S  struct {
+                                       S1
+                                       S2
+                               }
+                       )
+                       return S{S1{1, 2}, S2{3, 4}}
+               },
+               want: `{}`,
+       }, {
+               label: "DominantField",
+               // Both S1 and S2 have a field named X, but since S has an X field as
+               // well, it takes precedence over S1.X and S2.X.
+               makeInput: func() interface{} {
+                       type (
+                               S1 struct{ x, X int }
+                               S2 struct{ x, X int }
+                               S  struct {
+                                       S1
+                                       S2
+                                       x, X int
+                               }
+                       )
+                       return S{S1{1, 2}, S2{3, 4}, 5, 6}
+               },
+               want: `{"X":6}`,
+       }, {
+               // Unexported embedded field of non-struct type should not be serialized.
+               label: "UnexportedEmbeddedInt",
+               makeInput: func() interface{} {
+                       type (
+                               myInt int
+                               S     struct{ myInt }
+                       )
+                       return S{5}
+               },
+               want: `{}`,
+       }, {
+               // Exported embedded field of non-struct type should be serialized.
+               label: "ExportedEmbeddedInt",
+               makeInput: func() interface{} {
+                       type (
+                               MyInt int
+                               S     struct{ MyInt }
+                       )
+                       return S{5}
+               },
+               want: `{"MyInt":5}`,
+       }, {
+               // Unexported embedded field of pointer to non-struct type
+               // should not be serialized.
+               label: "UnexportedEmbeddedIntPointer",
+               makeInput: func() interface{} {
+                       type (
+                               myInt int
+                               S     struct{ *myInt }
+                       )
+                       s := S{new(myInt)}
+                       *s.myInt = 5
+                       return s
+               },
+               want: `{}`,
+       }, {
+               // Exported embedded field of pointer to non-struct type
+               // should be serialized.
+               label: "ExportedEmbeddedIntPointer",
+               makeInput: func() interface{} {
+                       type (
+                               MyInt int
+                               S     struct{ *MyInt }
+                       )
+                       s := S{new(MyInt)}
+                       *s.MyInt = 5
+                       return s
+               },
+               want: `{"MyInt":5}`,
+       }, {
+               // Exported fields of embedded structs should have their
+               // exported fields be serialized regardless of whether the struct types
+               // themselves are exported.
+               label: "EmbeddedStruct",
+               makeInput: func() interface{} {
+                       type (
+                               s1 struct{ x, X int }
+                               S2 struct{ y, Y int }
+                               S  struct {
+                                       s1
+                                       S2
+                               }
+                       )
+                       return S{s1{1, 2}, S2{3, 4}}
+               },
+               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.
+               label: "EmbeddedStructPointer",
+               makeInput: func() interface{} {
+                       type (
+                               s1 struct{ x, X int }
+                               S2 struct{ y, Y int }
+                               S  struct {
+                                       *s1
+                                       *S2
+                               }
+                       )
+                       return S{&s1{1, 2}, &S2{3, 4}}
+               },
+               want: `{"X":2,"Y":4}`,
+       }, {
+               // Exported fields on embedded unexported structs at multiple levels
+               // of nesting should still be serialized.
+               label: "NestedStructAndInts",
+               makeInput: func() interface{} {
+                       type (
+                               MyInt1 int
+                               MyInt2 int
+                               myInt  int
+                               s2     struct {
+                                       MyInt2
+                                       myInt
+                               }
+                               s1 struct {
+                                       MyInt1
+                                       myInt
+                                       s2
+                               }
+                               S struct {
+                                       s1
+                                       myInt
+                               }
+                       )
+                       return S{s1{1, 2, s2{3, 4}}, 6}
+               },
+               want: `{"MyInt1":1,"MyInt2":3}`,
+       }}
 
-       b, err := Marshal(a)
-       if err != nil {
-               t.Fatalf("Marshal: %v", err)
-       }
-       if got := string(b); got != want {
-               t.Errorf("got %q, want %q", got, want)
+       for _, tt := range tests {
+               t.Run(tt.label, func(t *testing.T) {
+                       b, err := Marshal(tt.makeInput())
+                       if err != nil {
+                               t.Fatalf("Marshal() = %v, want nil error", err)
+                       }
+                       if string(b) != tt.want {
+                               t.Fatalf("Marshal() = %q, want %q", b, tt.want)
+                       }
+               })
        }
 }