code review 6945065: encoding/json: don't panic marshaling anonymous non-str... (issue 6945065)

[tkap...@gmail.com]

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golan...@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
encoding/json: don't panic marshaling anonymous non-struct field

Add a check for this case and don't try to follow the anonymous
type's non-existent fields.

Fixes issue 4474.

Please review this at https://codereview.appspot.com/6945065/

Affected files:
M src/pkg/encoding/json/encode.go
M src/pkg/encoding/json/encode_test.go


Index: src/pkg/encoding/json/encode.go
===================================================================
--- a/src/pkg/encoding/json/encode.go
+++ b/src/pkg/encoding/json/encode.go
@@ -641,6 +641,13 @@
// Must be pointer.
ft = ft.Elem()
}
+
+ // If it's not a struct, simply record the field.
+ if ft.Kind() != reflect.Struct {
+ fields = append(fields, field{name: ft.Name(), index: index, typ: ft})
+ continue
+ }
+
nextCount[ft]++
if nextCount[ft] == 1 {
next = append(next, field{name: ft.Name(), index: index, typ: ft})
Index: src/pkg/encoding/json/encode_test.go
===================================================================
--- a/src/pkg/encoding/json/encode_test.go
+++ b/src/pkg/encoding/json/encode_test.go
@@ -186,3 +186,23 @@
t.Errorf("got %q, want %q", got, want)
}
}
+
+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)
+ }
+}

[r...@golang.org]

https://codereview.appspot.com/6945065/diff/4001/src/pkg/encoding/json/encode.go
File src/pkg/encoding/json/encode.go (right):

https://codereview.appspot.com/6945065/diff/4001/src/pkg/encoding/json/encode.go#newcode621
src/pkg/encoding/json/encode.go:621: if name != "" || !sf.Anonymous {
Can you do

|| sf.Type.Kind() != reflect.Struct

here?

https://codereview.appspot.com/6945065/

[tkap...@gmail.com]

On 2012/12/17 00:37:40, rsc wrote:

https://codereview.appspot.com/6945065/diff/4001/src/pkg/encoding/json/encode.go
> File src/pkg/encoding/json/encode.go (right):


https://codereview.appspot.com/6945065/diff/4001/src/pkg/encoding/json/encode.go#newcode621
> src/pkg/encoding/json/encode.go:621: if name != "" || !sf.Anonymous {
> Can you do

> || sf.Type.Kind() != reflect.Struct

> here?

Yep, that works. I like it better because it's one less branch. Also, I
forgot to handle tags, which comes now for free.

I just had to move up the code that follows pointers (ft = ft.Elem())
for anonymous fields, because in that case we want to check
sf.Type.Elem().Kind() != reflect.Struct.

https://codereview.appspot.com/6945065/

[tkap...@gmail.com]

Hello r...@golang.org (cc: golan...@googlegroups.com),

Please take another look.


https://codereview.appspot.com/6945065/