Encoding an empty types.Value has surprising and dangerous result

[aboodman]

Repo:

package main

import (
	"fmt"

	"github.com/attic-labs/noms/go/chunks"
	//"github.com/attic-labs/noms/go/marshal"
	"github.com/attic-labs/noms/go/types"
)

type Foo struct {
	Bar types.Ref
	Baz types.String
}

func main() {
	ts := &chunks.TestStorage{}
	vs := types.NewValueStore(ts.NewView())

	// This is how I discovered this bug - very easy mistake to make
	// note missing `noms:",omitempty"` on `Bar` field.
	//
	// nf := marshal.MustMarshal(vs, Foo{Baz:"monkey"}).(types.Struct)
	// fmt.Println(nf.Get("bar"))
	//
	// ... but you can also get the bug directly via the types API:

	foo := types.NewStruct("", types.StructData{
		"bar": types.Ref{},
	})
	c := types.EncodeValue(foo)
	v := types.DecodeValue(c, vs).(types.Struct)
	fmt.Println(v.Get("bar"))
}

yields:

panic: runtime error: index out of range

... deep inside serialization. The issue is that in this case valueImpl contains a completely empty buffer, which is what ends up being serialized, which completely confuses the deserializer.

I'm not sure if the right thing is to panic inside valueImpl serialization if we're uninitialized? That would violate the Go philosophy of making zero values useful. Or if we can instead serialize an empty value instead... in that case it seems like for consistency we should make sure that all the zero types.Values are useful.