internal/encoding/messageset: don't modify input data when unmarshaling

When combining multiple message fields in a MessageSet item (a case
which should never happen in practice), unmarshal could modify the input
data. Fix it to not do so. Add a general check to ensure that unmarshal
operations don't modify the input.

Change-Id: Idde46e6132a1dc96c374f9146efff81783c3bef3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/223818
Reviewed-by: Joe Tsai <joetsai@google.com>

internal/encoding/messageset/messageset.go

@@ -159,9 +159,9 @@ func ConsumeFieldValue(b []byte, wantLen bool) (typeid wire.Number, message []by
 			}
 			if message == nil {
 				if wantLen {
-					message = b[:n]
+					message = b[:n:n]
 				} else {
-					message = m
+					message = m[:len(m):len(m)]
 				}
 			} else {
 				// This case should never happen in practice, but handle it for
@@ -174,7 +174,7 @@ func ConsumeFieldValue(b []byte, wantLen bool) (typeid wire.Number, message []by
 				if wantLen {
 					_, nn := wire.ConsumeVarint(message)
 					m0 := message[nn:]
-					message = message[:0]
+					message = nil
 					message = wire.AppendVarint(message, uint64(len(m0)+len(m)))
 					message = append(message, m0...)
 					message = append(message, m...)

proto/decode_test.go

@@ -5,6 +5,7 @@
 package proto_test
 
 import (
+	"bytes"
 	"fmt"
 	"reflect"
 	"testing"
@@ -34,8 +35,12 @@ func TestDecode(t *testing.T) {
 					return
 				}
 
-				// Aliasing check: Modifying the original wire bytes shouldn't
-				// affect the unmarshaled message.
+				// Aliasing check: Unmarshal shouldn't modify the original wire
+				// bytes, and modifying the original wire bytes shouldn't affect
+				// the unmarshaled message.
+				if !bytes.Equal(test.wire, wire) {
+					t.Errorf("Unmarshal unexpectedly modified its input")
+				}
 				for i := range wire {
 					wire[i] = 0
 				}