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>
diff --git a/internal/encoding/messageset/messageset.go b/internal/encoding/messageset/messageset.go
index 4bd0e4e..67c6173 100644
--- a/internal/encoding/messageset/messageset.go
+++ b/internal/encoding/messageset/messageset.go
@@ -159,9 +159,9 @@
}
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 @@
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...)
diff --git a/proto/decode_test.go b/proto/decode_test.go
index 9cfe395..a6b7ce8 100644
--- a/proto/decode_test.go
+++ b/proto/decode_test.go
@@ -5,6 +5,7 @@
package proto_test
import (
+ "bytes"
"fmt"
"reflect"
"testing"
@@ -34,8 +35,12 @@
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
}
