protobuf

protobuf copied to clipboard

Should there be a way for protobuf reflection to distinguish proto.Message values that are truly invalid?

Currently, we have protoreflect.Message.IsValid which indicates that the current Go value is functionally a read-only empty message. This implies that the current value has protobuf type information (i.e., Descriptor and Type methods are callable).

However, there are a number of Go types that possess a higher level of "invalid"-ness. For example, the zero value of dynamicpb.Message and ptypes.DynamicAny obviously cannot contain protobuf type information since they are both concrete Go types that may may represent any arbitrary protobuf message type.

The issue today is that a user of protobuf reflection cannot distinguish such messages and risk panicking whenever they call the methods on protoreflect.Message. For example, protocmp.Transform will choke on the zero value of dynamicpb.Message.

Some possibilities (some or all of these may be done) regarding truly invalid values:

• Document that calling proto.Message.ProtoReflect on returns nil.
• Document that calling protoreflect.Message.Type or protoreflect.Message.Descriptor returns nil.

(I define "truly invalid" as a concrete value that has no sensible protobuf message type associated with it at the present moment.)

\cc @neild

For dynamicpb.Message, we could define the zero value of the type as a read-only google.protobuf.Empty. Perhaps something similar for ptypes.DynamicAny.

😕 hm… defining special magic values for these otherwise–invalid values means that they would become part of the public API, and once anyone ends up relying on that behavior, if there is any attempt to change the semantics, then we would break those that depends upon the semantics we set now.

I’m all for setting it to be documented undefined behavior, or otherwise stating that it’s otherwise invalid, and that it returns a non–useful value, then we have much more leeway to change it to a useful secondary–semantic like google.protobuf.Empty later, when we know this would actually be a useful and appropriate semantic.

Removing block-v2 label. We discussed this more today.

• The various implementations of the v2 proto.Message today that exhibit this problem today panic for nil types. This makes it hard for people to depend on this behavior and gives us the flexibility to change it in the future.
• We still want to address this as spurious panics are not desired. The suggestion to treat truly invalid message values as google.protobuf.Empty has some compelling benefits, but will require more thought.