jsonpb: allow invalid enums to be unmarshalled to zero value int32

[marwan-at-work]

What language does this apply to?
Go, C++

This issue is ported from the Go implementation of protobuf/jsonpb because the implementation of json marshaling/unmarshaling is faifthful to the C++ one according to golang/protobuf#893 (comment)

JSONPB allows a JSON string to be unmarshalled into a proto struct. However, the JSON string cannot enforce that the enum string will always be correct. Bad user input is always expected.

For example, for the following enum proto declaration:

message Input {
  TrafficLight light = 1;
} 

enum TrafficLight {
  UNKNOWN = 0;
  RED = 1;
  YELLOW = 2;
  GREEN = 3;
}

The user can send a json that looks like this:

{ "light": "purple" } 

In Go, if we used jsonpb.Unmarshaler{} to process the above json, we would get an error saying that "purple" is not a known enum.

One thing that's worth noting is that if the JSON did not have a string representation of the enum and instead had a number representation such as { "light": 33983 }, then the Unmarshaler does not care about validating the enum actually exists and whatever number you give it, it's happy. I find that quite odd.

Therefore, I think the Unmarshaler struct should take AllowInvalidEnums boolean as an option so that if there's bad user input, the unmarshaller would just set the default value of in32 and continue processing.

This should be a backwards-compatible change since the default remains the same.

[TeBoring]

For binary format, unknown enum can be kept using their original value.
However, for json, when it's unknown, we have no way to convert its name to its value. Thus, we cannot properly stored it in memory (memory storage is in favor of binary instead of json).
Your proposal to use default value for unknown enum has some semantic problem. For a round trip, the original sender may end up receive a different value from it sent.
I don't think we want this behavior.

[jjhuff]

@TeBoring I raised a similar issue in golang/protobuf. In my case, I used protobufs all over -- but need to convert to json for storage in ElasticSearch. When we add a new enum value, currently, we're required to rebuild and deploy all code that reads from ES -- even if they don't care about the new enum value. This seems like a similar situation that caused the existing 'discard unknown' option to be created.

While I agree that keeping the encoding round-trip-safe is an important consideration (and should remain the default), graceful degradation is also a useful attribute.

[hypnoglow]

For binary format, unknown enum can be kept using their original value. However, for json, when it's unknown, we have no way to convert its name to its value. Thus, we cannot properly stored it in memory (memory storage is in favor of binary instead of json). Your proposal to use default value for unknown enum has some semantic problem. For a round trip, the original sender may end up receive a different value from it sent. I don't think we want this behavior.

We have a problem that protojson does not guarantee the same compatibility as protobuf binary format. I don't know if it should, but this causes a lot of pain.

The original issue causes the nightmare of having to update all clients that consume API when the producer added new Enum value, even if consumers never use the enum.

Any recommended workarounds?