code review 5616063: gob: fuzz testing, plus a fix for very large type names.

gob: fuzz testing, plus a fix for very large type names.

Fixes issue 2689.

[dsymonds, 2012-02-06 01:44:04]

Hello r (cc: golang-dev@googlegroups.com),

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

[r, 2012-02-06 01:53:59]

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/codec_te...
File src/pkg/encoding/gob/codec_test.go (right):

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/codec_te...
src/pkg/encoding/gob/codec_test.go:1437: func TestFuzz(t *testing.T) {
// comment

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/codec_te...
src/pkg/encoding/gob/codec_test.go:1457: // An instance triggering a type name
of ~102 GB.
s/of/of length/

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/codec_te...
src/pkg/encoding/gob/codec_test.go:1463: // testing package handles panics
correctly.
how about we fix this problem before committing this CL?

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/decode.go
File src/pkg/encoding/gob/decode.go (right):

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/decode.g...
src/pkg/encoding/gob/decode.go:695: errorf("invalid type name length %d", nr)
is zero a valid length?

[dsymonds, 2012-02-06 02:06:01]

There's also a change to error.go, which improves the failure message when the
panic is not a gobError.

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/codec_te...
File src/pkg/encoding/gob/codec_test.go (right):

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/codec_te...
src/pkg/encoding/gob/codec_test.go:1437: func TestFuzz(t *testing.T) {
On 2012/02/06 01:53:59, r wrote:
> // comment

Done.

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/codec_te...
src/pkg/encoding/gob/codec_test.go:1457: // An instance triggering a type name
of ~102 GB.
On 2012/02/06 01:53:59, r wrote:
> s/of/of length/

Done.

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/codec_te...
src/pkg/encoding/gob/codec_test.go:1463: // testing package handles panics
correctly.
On 2012/02/06 01:53:59, r wrote:
> how about we fix this problem before committing this CL?

Sure. Looking into it now. I'll make this t.Logf now.

[r, 2012-02-06 02:52:34]

LGTM

http://codereview.appspot.com/5616063/diff/3/src/pkg/encoding/gob/decode.go
File src/pkg/encoding/gob/decode.go (right):

http://codereview.appspot.com/5616063/diff/3/src/pkg/encoding/gob/decode.go#n...
src/pkg/encoding/gob/decode.go:694: if nr < 0 || nr > 1<<31 {
<= i think

[dsymonds, 2012-02-06 02:52:59]

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/decode.go
File src/pkg/encoding/gob/decode.go (right):

http://codereview.appspot.com/5616063/diff/4001/src/pkg/encoding/gob/decode.g...
src/pkg/encoding/gob/decode.go:695: errorf("invalid type name length %d", nr)
On 2012/02/06 01:53:59, r wrote:
> is zero a valid length?

Done.

[dsymonds, 2012-02-06 03:02:18]

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

gob: fuzz testing, plus a fix for very large type names.

Fixes issue 2689.

R=r
CC=golang-dev
http://codereview.appspot.com/5616063