Encode and decode empty bytes fields correctly.

R=r, dsymonds, rsc
CC=golang-dev
http://codereview.appspot.com/3274043

proto/all_test.go

@@ -772,6 +772,59 @@ func TestEncodeDecode5(t *testing.T) {
 
 }
 
+// Test that we can encode empty bytes fields.
+func TestEncodeDecodeBytes1(t *testing.T) {
+	pb := initGoTest(false)
+
+	// Create our bytes
+	pb.F_BytesRequired = []byte{}
+	pb.F_BytesRepeated = [][]byte{ []byte{} }
+	pb.F_BytesOptional = []byte{}
+
+	d, err := Marshal(pb)
+	if err != nil {
+		t.Errorf(err.String())
+	}
+
+	pbd := new(GoTest)
+	if err := Unmarshal(d, pbd); err != nil {
+		t.Errorf(err.String())
+	}
+
+	if pbd.F_BytesRequired == nil || len(pbd.F_BytesRequired) != 0 {
+		t.Errorf("required empty bytes field is incorrect")
+	}
+	if pbd.F_BytesRepeated == nil || len(pbd.F_BytesRepeated) == 1 && pbd.F_BytesRepeated[0] == nil {
+		t.Errorf("repeated empty bytes field is incorrect")
+	}
+	if pbd.F_BytesOptional == nil || len(pbd.F_BytesOptional) != 0 {
+		t.Errorf("optional empty bytes field is incorrect")
+	}
+}
+
+// Test that we encode nil-valued fields of a repeated bytes field correctly.
+// Since entries in a repeated field cannot be nil, nil must mean empty value.
+func TestEncodeDecodeBytes2(t *testing.T) {
+	pb := initGoTest(false)
+
+	// Create our bytes
+	pb.F_BytesRepeated = [][]byte{ nil }
+
+	d, err := Marshal(pb)
+	if err != nil  {
+		t.Errorf(err.String())
+	}
+
+	pbd := new(GoTest)
+	if err := Unmarshal(d, pbd); err != nil {
+		t.Errorf(err.String())
+	}
+
+	if len(pbd.F_BytesRepeated) != 1 || pbd.F_BytesRepeated[0] == nil {
+		t.Errorf("Unexpected value for repeated bytes field")
+	}
+}
+
 // All required fields set, defaults provided, all repeated fields given two values.
 func TestSkippingUnrecognizedFields(t *testing.T) {
 	o := old()

proto/decode.go

@@ -479,10 +479,7 @@ func (o *Buffer) dec_slice_byte(p *Properties, base uintptr, sbase uintptr) os.E
 	if err != nil {
 		return err
 	}
-	lb := len(b)
-	if lb == 0 {
-		return nil
-	}
+
 	x := (*[]uint8)(unsafe.Pointer(base + p.offset))
 
 	y := *x
@@ -494,6 +491,7 @@ func (o *Buffer) dec_slice_byte(p *Properties, base uintptr, sbase uintptr) os.E
 	}
 
 	l := len(y)
+	lb := len(b)
 	if l+lb > c {
 		// incremental growth is max(len(slice)*1.5, len(slice)+len(bytes))
 		g := l * 3 / 2

proto/encode.go

@@ -364,14 +364,14 @@ func (o *Buffer) enc_slice_bool(p *Properties, base uintptr) os.Error {
 // Encode a slice of bytes ([]byte).
 func (o *Buffer) enc_slice_byte(p *Properties, base uintptr) os.Error {
 	s := *(*[]uint8)(unsafe.Pointer(base + p.offset))
-	// if the field is required, we must send something, even if it's an empty array.
+	if s == nil {
+		return ErrNil
+	}
+	// TODO: remove this block and fix tests
 	if !p.Required {
 		l := len(s)
-		if l == 0 {
-			return ErrNil
-		}
 		// check default
-		if l == len(p.Default) {
+		if l > 0 && l == len(p.Default) {
 			same := true
 			for i := 0; i < len(p.Default); i++ {
 				if p.Default[i] != s[i] {