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

fix(json): improve handling of omitempty with embedded pointers #89

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion json/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,9 @@ func appendStructFields(fields []structField, t reflect.Type, offset uintptr, se

if embfield.pointer {
subfield.codec = constructEmbeddedStructPointerCodec(embfield.subtype.typ, embfield.unexported, subfield.offset, subfield.codec)
subfield.ptrOffset += subfield.offset
subfield.offset = embfield.offset
subfield.ptr = true
} else {
subfield.offset += embfield.offset
}
Expand Down Expand Up @@ -931,7 +933,6 @@ type structType struct {
fieldsIndex map[string]*structField
ficaseIndex map[string]*structField
typ reflect.Type
inlined bool
}

type structField struct {
Expand All @@ -946,6 +947,8 @@ type structField struct {
typ reflect.Type
zero reflect.Value
index int
ptr bool
ptrOffset uintptr
}

func unmarshalTypeError(b []byte, t reflect.Type) error {
Expand Down
13 changes: 11 additions & 2 deletions json/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -767,8 +767,17 @@ func (e encoder) encodeStruct(b []byte, p unsafe.Pointer, st *structType) ([]byt
f := &st.fields[i]
v := unsafe.Pointer(uintptr(p) + f.offset)

if f.omitempty && f.empty(v) {
continue
if f.omitempty {
if f.ptr {
v2 := *(*unsafe.Pointer)(v)
v3 := unsafe.Pointer(uintptr(v2) + f.ptrOffset)

if f.empty(v3) {
continue
}
} else if f.empty(v) {
continue
}
}

if escapeHTML {
Expand Down
100 changes: 99 additions & 1 deletion json/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1578,7 +1578,6 @@ func TestGithubIssue41(t *testing.T) {
"expected: ", expectedString,
)
}

}

func TestGithubIssue44(t *testing.T) {
Expand All @@ -1602,6 +1601,105 @@ func (r *rawJsonString) UnmarshalJSON(b []byte) error {
return nil
}

// See https://github.com/segmentio/encoding/issues/63
// In short, embedding a struct pointer resulted in an incorrect memory address
// as we were still looking to the parent struct to start and only using offsets
// which resulted in the wrong values being extracted.
func TestGithubIssue63(t *testing.T) {
spec := []struct {
description string
input func() interface{}
expected string
}{
{
description: "original",
expected: `{"my_field":"test","code":0}`,
input: func() interface{} {
type MyStruct struct {
MyField string `json:"my_field,omitempty"`
}

type MyStruct2 struct {
*MyStruct
Code int `json:"code"`
}

return MyStruct2{
MyStruct: &MyStruct{
MyField: "test",
},
Code: 0,
}
},
},
{
description: "additional fields",
expected: `{"my_field":"test","my_other_field":"testing","code":1}`,
input: func() interface{} {
type MyStruct struct {
MyField string `json:"my_field,omitempty"`
MyOtherField string `json:"my_other_field"`
MyEmptyField string `json:"my_empty_field,omitempty"`
}

type MyStruct2 struct {
*MyStruct
Code int `json:"code"`
}

return MyStruct2{
MyStruct: &MyStruct{
MyField: "test",
MyOtherField: "testing",
},
Code: 1,
}
},
},
{
description: "multiple embed levels",
expected: `{"my_field":"test","my_other_field":"testing","code":1}`,
input: func() interface{} {
type MyStruct struct {
MyField string `json:"my_field,omitempty"`
MyOtherField string `json:"my_other_field"`
MyEmptyField string `json:"my_empty_field,omitempty"`
}

type MyStruct2 struct {
*MyStruct
Code int `json:"code"`
}

type MyStruct3 struct {
*MyStruct2
}

return MyStruct3{
MyStruct2: &MyStruct2{
MyStruct: &MyStruct{
MyField: "test",
MyOtherField: "testing",
},
Code: 1,
},
}
},
},
}

for _, test := range spec {
t.Run(test.description, func(t *testing.T) {
if b, err := Marshal(test.input()); err != nil {
t.Error(err)
} else if string(b) != test.expected {
t.Errorf("got: %s", string(b))
t.Errorf("expected: %s", test.expected)
}
})
}
}

func TestSetTrustRawMessage(t *testing.T) {
buf := &bytes.Buffer{}
enc := NewEncoder(buf)
Expand Down