goprotobuf: Fix Clone to deep copy []byte fields.
LGTM=nigeltao
R=nigeltao
CC=golang-codereviews
https://codereview.appspot.com/165140043
diff --git a/proto/clone.go b/proto/clone.go
index 8b94d9e..16e464e 100644
--- a/proto/clone.go
+++ b/proto/clone.go
@@ -125,6 +125,12 @@
if in.IsNil() {
return
}
+ if in.Type().Elem().Kind() == reflect.Uint8 {
+ // []byte is a scalar bytes field, not a repeated field.
+ // Make a deep copy.
+ out.SetBytes(append([]byte(nil), in.Bytes()...))
+ return
+ }
n := in.Len()
if out.IsNil() {
out.Set(reflect.MakeSlice(in.Type(), 0, n))
@@ -133,9 +139,6 @@
case reflect.Bool, reflect.Float32, reflect.Float64, reflect.Int32, reflect.Int64,
reflect.String, reflect.Uint32, reflect.Uint64:
out.Set(reflect.AppendSlice(out, in))
- case reflect.Uint8:
- // []byte is a scalar bytes field.
- out.Set(in)
default:
for i := 0; i < n; i++ {
x := reflect.Indirect(reflect.New(in.Type().Elem()))
diff --git a/proto/clone_test.go b/proto/clone_test.go
index 39aaaf7..522d40e 100644
--- a/proto/clone_test.go
+++ b/proto/clone_test.go
@@ -79,6 +79,22 @@
if proto.Equal(m, cloneTestMessage) {
t.Error("Mutating clone changed the original")
}
+ // Byte fields and repeated fields should be copied.
+ if &m.Pet[0] == &cloneTestMessage.Pet[0] {
+ t.Error("Pet: repeated field not copied")
+ }
+ if &m.Others[0] == &cloneTestMessage.Others[0] {
+ t.Error("Others: repeated field not copied")
+ }
+ if &m.Others[0].Value[0] == &cloneTestMessage.Others[0].Value[0] {
+ t.Error("Others[0].Value: bytes field not copied")
+ }
+ if &m.RepBytes[0] == &cloneTestMessage.RepBytes[0] {
+ t.Error("RepBytes: repeated field not copied")
+ }
+ if &m.RepBytes[0][0] == &cloneTestMessage.RepBytes[0][0] {
+ t.Error("RepBytes[0]: bytes field not copied")
+ }
}
func TestCloneNil(t *testing.T) {
diff --git a/proto/text_parser.go b/proto/text_parser.go
index 9e4c7b1..1799e1b 100644
--- a/proto/text_parser.go
+++ b/proto/text_parser.go
@@ -430,6 +430,7 @@
st := sv.Type()
reqCount := GetProperties(st).reqCount
var reqFieldErr error
+ fieldSet := make(map[string]bool)
// A struct is a sequence of "name: value", terminated by one of
// '>' or '}', or the end of the input. A name may also be
// "[extension]".
@@ -511,17 +512,17 @@
}
} else {
// This is a normal, non-extension field.
- fi, props, ok := structFieldByName(st, tok.value)
+ name := tok.value
+ fi, props, ok := structFieldByName(st, name)
if !ok {
- return p.errorf("unknown field name %q in %v", tok.value, st)
+ return p.errorf("unknown field name %q in %v", name, st)
}
dst := sv.Field(fi)
- isDstNil := isNil(dst)
// Check that it's not already set if it's not a repeated field.
- if !props.Repeated && !isDstNil {
- return p.errorf("non-repeated field %q was repeated", tok.value)
+ if !props.Repeated && fieldSet[name] {
+ return p.errorf("non-repeated field %q was repeated", name)
}
if err := p.checkForColon(props, st.Field(fi).Type); err != nil {
@@ -529,6 +530,7 @@
}
// Parse into the field.
+ fieldSet[name] = true
if err := p.readAny(dst, props); err != nil {
if _, ok := err.(*RequiredNotSetError); !ok {
return err
