diff options
author | Joe Tsai <joetsai@digital-static.net> | 2017-07-21 16:53:54 -0700 |
---|---|---|
committer | Joe Tsai <joetsai@google.com> | 2017-07-22 01:29:58 +0000 |
commit | b81735924936291303559fd71dabaa1aa88f57c5 (patch) | |
tree | f429ab3c9ed7612f1781bc1305635cb622230904 | |
parent | 5f7a03e148b9a37f2c61f7d428abc6b360897a0c (diff) | |
download | go-b81735924936291303559fd71dabaa1aa88f57c5.tar.gz go-b81735924936291303559fd71dabaa1aa88f57c5.zip |
encoding/json: ignore embedded fields of pointers to unexported non-structs
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>
-rw-r--r-- | src/encoding/json/encode.go | 23 | ||||
-rw-r--r-- | src/encoding/json/encode_test.go | 229 |
2 files changed, 182 insertions, 70 deletions
diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go index 6fcea4735f..0371f0a24d 100644 --- a/src/encoding/json/encode.go +++ b/src/encoding/json/encode.go @@ -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 diff --git a/src/encoding/json/encode_test.go b/src/encoding/json/encode_test.go index d5f5f0a691..3fda6a0c71 100644 --- a/src/encoding/json/encode_test.go +++ b/src/encoding/json/encode_test.go @@ -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) + } + }) } } |