code review 7490046: encoding/json: properly handle extra objects with fixed...

encoding/json: properly handle extra objects with fixed size arrays

If a fixed size array is passed in as the decode target and the JSON
to decode has extra array elements that are objects, then previously
the decoder would return a "data changing underfoot" error.

Fixes issue 3717.

[rick, 2013-03-12 02:02:15]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[rick, 2013-03-12 02:02:47]

https://codereview.appspot.com/7490046/diff/5001/src/pkg/encoding/json/decode.go
File src/pkg/encoding/json/decode.go (right):

https://codereview.appspot.com/7490046/diff/5001/src/pkg/encoding/json/decode...
src/pkg/encoding/json/decode.go:394: m := make(map[string]interface{})
I couldn't find a more efficient way of doing this. I tried d.next() but that
didn't work. Any ideas?

[rick, 2013-03-12 23:02:48]

PTAL

[adg, 2013-03-13 02:23:40]

https://codereview.appspot.com/7490046/diff/14001/src/pkg/encoding/json/decod...
File src/pkg/encoding/json/decode_test.go (right):

https://codereview.appspot.com/7490046/diff/14001/src/pkg/encoding/json/decod...
src/pkg/encoding/json/decode_test.go:1186: json := `["article",{"category":1},
{"type": "foo"}, 2, {"data": {"bar": "baz"}}, [1, 2, 3]]`
Can you please use the reduced test case, as described here:
http://play.golang.org/p/OYCGWFzZGp

[rick, 2013-03-13 02:50:26]

PTAL

https://codereview.appspot.com/7490046/diff/14001/src/pkg/encoding/json/decod...
File src/pkg/encoding/json/decode_test.go (right):

https://codereview.appspot.com/7490046/diff/14001/src/pkg/encoding/json/decod...
src/pkg/encoding/json/decode_test.go:1186: json := `["article",{"category":1},
{"type": "foo"}, 2, {"data": {"bar": "baz"}}, [1, 2, 3]]`
On 2013/03/13 02:23:40, adg wrote:
> Can you please use the reduced test case, as described here:
> http://play.golang.org/p/OYCGWFzZGp

Done.

[adg, 2013-03-13 05:38:58]

https://codereview.appspot.com/7490046/diff/22001/src/pkg/encoding/json/decod...
File src/pkg/encoding/json/decode.go (right):

https://codereview.appspot.com/7490046/diff/22001/src/pkg/encoding/json/decod...
src/pkg/encoding/json/decode.go:397: d.value(reflect.Value{})
I just don't see why this shouldn't work. It seems like we're papering over the
actual problem.

Shouldn't this case be treated just like decoding into a struct field that
doesn't exist?

[rick, 2013-03-13 13:59:34]

PTAL

https://codereview.appspot.com/7490046/diff/22001/src/pkg/encoding/json/decod...
File src/pkg/encoding/json/decode.go (right):

https://codereview.appspot.com/7490046/diff/22001/src/pkg/encoding/json/decod...
src/pkg/encoding/json/decode.go:397: d.value(reflect.Value{})
On 2013/03/13 05:38:58, adg wrote:
> I just don't see why this shouldn't work. It seems like we're papering over
the
> actual problem.
> 
> Shouldn't this case be treated just like decoding into a struct field that
> doesn't exist?

Sorry, my reasoning was that I should fix this special case locally instead of
the underlying issue to reduce risk close to Go 1.1. The latest patchset has the
complete fix.

[rsc, 2013-03-13 18:51:41]

LGTM

[rsc, 2013-03-13 18:53:07]

*** Submitted as https://code.google.com/p/go/source/detail?r=088249126434 ***

encoding/json: properly handle extra objects with fixed size arrays

If a fixed size array is passed in as the decode target and the JSON
to decode has extra array elements that are objects, then previously
the decoder would return a "data changing underfoot" error.

Fixes issue 3717.

R=golang-dev, adg, rsc
CC=golang-dev
https://codereview.appspot.com/7490046

Committer: Russ Cox <rsc@golang.org>