internal/impl: avoid preserving presence on proto3 bytes field
When setting a proto3 bytes field to an empty, non-nil bytes slice,
just store a nil slice in the underderlying storage.
This is done to avoid presenting the illusion to the user that
presence is preserved for proto3.
Updates golang/protobuf#896
Change-Id: I1b97bedd547d336863c65d9418d8f07edf69ccd6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185577
Reviewed-by: Herbie Ong <herbie@google.com>
diff --git a/internal/impl/message_field.go b/internal/impl/message_field.go
index f3b4f8a..7cba71d 100644
--- a/internal/impl/message_field.go
+++ b/internal/impl/message_field.go
@@ -204,11 +204,15 @@
}
}
-var emptyBytes = reflect.ValueOf([]byte{})
+var (
+ nilBytes = reflect.ValueOf([]byte(nil))
+ emptyBytes = reflect.ValueOf([]byte{})
+)
func fieldInfoForScalar(fd pref.FieldDescriptor, fs reflect.StructField, x exporter) fieldInfo {
ft := fs.Type
nullable := fd.Syntax() == pref.Proto2
+ isBytes := ft.Kind() == reflect.Slice && ft.Elem().Kind() == reflect.Uint8
if nullable {
if ft.Kind() != reflect.Ptr && ft.Kind() != reflect.Slice {
panic(fmt.Sprintf("invalid type: got %v, want pointer", ft))
@@ -274,8 +278,12 @@
rv = rv.Elem()
}
rv.Set(conv.GoValueOf(v))
- if nullable && rv.Kind() == reflect.Slice && rv.IsNil() {
- rv.Set(emptyBytes)
+ if isBytes && rv.Len() == 0 {
+ if nullable {
+ rv.Set(emptyBytes) // preserve presence in proto2
+ } else {
+ rv.Set(nilBytes) // do not preserve presence in proto3
+ }
}
},
}
diff --git a/internal/impl/message_test.go b/internal/impl/message_test.go
index f2fd290..4132fd1 100644
--- a/internal/impl/message_test.go
+++ b/internal/impl/message_test.go
@@ -270,6 +270,11 @@
}).ProtoReflect()},
clearFields{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22},
equalMessage{new(ScalarProto2).ProtoReflect()},
+
+ // Setting a bytes field nil empty bytes should preserve presence.
+ setFields{10: V([]byte(nil)), 11: V([]byte(nil)), 21: V([]byte(nil)), 22: V([]byte(nil))},
+ getFields{10: V([]byte{}), 11: V([]byte(nil)), 21: V([]byte{}), 22: V([]byte(nil))},
+ hasFields{10: true, 11: true, 21: true, 22: true},
})
// Test read-only operations on nil message.
@@ -391,6 +396,11 @@
hasFields{6: false, 7: false},
setFields{6: V(float32(math.Copysign(0, -1))), 7: V(float64(math.Copysign(0, -1)))},
hasFields{6: true, 7: true},
+
+ // Setting a bytes field to non-nil empty bytes should not preserve presence.
+ setFields{10: V([]byte{}), 11: V([]byte{}), 21: V([]byte{}), 22: V([]byte{})},
+ getFields{10: V([]byte(nil)), 11: V([]byte(nil)), 21: V([]byte(nil)), 22: V([]byte(nil))},
+ hasFields{10: false, 11: false, 21: false, 22: false},
})
// Test read-only operations on nil message.
@@ -1200,7 +1210,6 @@
}
}),
cmpopts.EquateNaNs(),
- cmpopts.EquateEmpty(),
}
func testMessage(t *testing.T, p path, m pref.Message, tt messageOps) {
