all: fix reflection behavior for empty lists and maps
The protoreflect documentation states that Get on an empty repeated or
map field returns an invalid (empty, read-only) List or Map. We weren't
doing this; fix it.
The documentation also states that an invalid List or Map may not be
assigned via Set. We were permitting Set with an invalid map; fix this.
Add tests for this behavior in prototest.
Change-Id: I4678af532e192210af0bde7c96a1439a4cd26efa
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/209019
Reviewed-by: Joe Tsai <joetsai@google.com>
diff --git a/internal/impl/message_reflect_field.go b/internal/impl/message_reflect_field.go
index 8d4e6ae..7b87d47 100644
--- a/internal/impl/message_reflect_field.go
+++ b/internal/impl/message_reflect_field.go
@@ -138,11 +138,18 @@
return conv.Zero()
}
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
+ if rv.Len() == 0 {
+ return conv.Zero()
+ }
return conv.PBValueOf(rv)
},
set: func(p pointer, v pref.Value) {
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
- rv.Set(conv.GoValueOf(v))
+ pv := conv.GoValueOf(v)
+ if pv.IsNil() {
+ panic(fmt.Sprintf("invalid value: setting map field to read-only value"))
+ }
+ rv.Set(pv)
},
mutable: func(p pointer) pref.Value {
v := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
@@ -184,11 +191,18 @@
return conv.Zero()
}
rv := p.Apply(fieldOffset).AsValueOf(fs.Type)
+ if rv.Elem().Len() == 0 {
+ return conv.Zero()
+ }
return conv.PBValueOf(rv)
},
set: func(p pointer, v pref.Value) {
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
- rv.Set(reflect.ValueOf(v.List().(unwrapper).protoUnwrap()).Elem())
+ pv := conv.GoValueOf(v)
+ if pv.IsNil() {
+ panic(fmt.Sprintf("invalid value: setting repeated field to read-only value"))
+ }
+ rv.Set(pv.Elem())
},
mutable: func(p pointer) pref.Value {
v := p.Apply(fieldOffset).AsValueOf(fs.Type)