Panic in TestFuzzBytes

[ghost]

If I bump up the number of iterations to 10,000,000 I reliably get panics in (*multiaddr).String().

[whyrusleeping]

@lgierth could you paste the stack trace from a panic?

[ghost]

Having it happen about 3/4th of the runs here.

> git diff && go test ./...
diff --git a/multiaddr_test.go b/multiaddr_test.go
index 5c3d256..358767e 100644
--- a/multiaddr_test.go
+++ b/multiaddr_test.go
@@ -373,7 +373,7 @@ func TestFuzzBytes(t *testing.T) {
        rand.Seed(time.Now().UnixNano())
        // Bump up these numbers if you want to stress this
        buf := make([]byte, 256)
-       for i := 0; i < 2000; i++ {
+       for i := 0; i < 10000000; i++ {
                l := rand.Intn(len(buf))
                rand.Read(buf[:l])
 
--- FAIL: TestFuzzBytes (4.37s)
panic: multiaddr failed to convert back to string. corrupted? [recovered]
	panic: multiaddr failed to convert back to string. corrupted?

goroutine 15 [running]:
panic(0x529180, 0xc4200b4cf0)
	/usr/local/go/src/runtime/panic.go:500 +0x1a1
testing.tRunner.func1(0xc42008f080)
	/usr/local/go/src/testing/testing.go:579 +0x25d
panic(0x529180, 0xc4200b4cf0)
	/usr/local/go/src/runtime/panic.go:458 +0x243
_/home/user/protocol/multiformats/go-multiaddr.(*multiaddr).String(0xc4200b3b40, 0x13, 0x100)
	/home/user/protocol/multiformats/go-multiaddr/multiaddr.go:64 +0xbc
_/home/user/protocol/multiformats/go-multiaddr.TestFuzzBytes(0xc42008f080)
	/home/user/protocol/multiformats/go-multiaddr/multiaddr_test.go:384 +0x15d
testing.tRunner(0xc42008f080, 0x56af60)
	/usr/local/go/src/testing/testing.go:610 +0x81
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:646 +0x2ec
FAIL	_/home/user/protocol/multiformats/go-multiaddr	4.379s

[hsanjuan]

I think I know what this is. Probably has to do with NewMultiaddrBytes uses the slice that is passed-in. When this slice is re-used it screws the multiaddress completely. It happens when deserializing anything with an optimized decoder that re-uses previously allocated slices.

The solution is a copy().

[Kubuxu]

I have a fuzzer running on it and at the moment it seems alright (155457102 iterations).
@hsanjuan why would it screw up multiaddress if it isn't used in future.
We do: 1. write to buffer, 2. decode, 3. check, 4. discard, 5. repeat. It shouldn't screw up anything as far as I can see.

[hsanjuan]

I know that the multiaddress mutates after https://godoc.org/github.com/ugorji/go/codec uses UnmarshalBinary() because (I guess) the underlying byte slice is re-used or altered and it gives exactly this problem. But you're right, I don't see how this would happen in this test.