jsonpb: does not handle lower case string enums

[bbassingthwaite]

We've been relying on the c++ proto library for json transcoding and are now starting to shim in a golang piece that should provide the same functionality. We've run into an issue where we send a lowercase string of an enum that the c++ library handles, but the golang library does not.

For example:

Given an enum

enum Category {
    NOT_SPECIFIED = 0;
    LISTINGS = 1;
    REPUTATION = 2;
}

If I try unmarshall the json {"category": "listings"} it works with the c++ library, but fails on the golang library.

[dsnet]

The jsonpb package follows the JSON specification, which does not mention casing with respect to enumeration names. I don't think what Go is currently doing is entirely correct, but I don't think the PR you are proposing is correct either.

The spec does say:

Proto3 JSON parsers are required to accept both the converted lowerCamelCase name and the proto field name.

And it doesn't seem that we respect that properly.

[bbassingthwaite]

What isn't correct about the proposed PR? The implementation or the logic?

It may not match the spec but it does match the logic of the c++ library.

[dsnet]

My point is that correctness is not defined by what C++ does, but what the specification say.

[bbassingthwaite]

Agreed, so either c++ needs to be fixed to meet the spec? or the spec needs to be updated?

[dsnet]

@xfxyjwf, do you know if C++ is correct in that it seems to ignore casing with regard to JSON deserialization? The JSON specification doesn't seem to address this, which I interpret as being disallowed.

[dsnet]

I'm inclined to believe this is a C++ bug. The section I quoted earlier is about field names, and not enum names. I'm fairly certain enums can only either be an integer or a string with the exact ENUM_NAME.

[bbassingthwaite]

@dsnet This is affecting our service is a large way and we've had to fork this library and now have to maintain our own version of protobuf. While it may be a c++ bug, it's now a part of the API and will remain like that forever unless they release a breaking change to the library.

Shouldn't the protobuf libraries across all languages support JSON consistently? Given that we've depended on this c++ functionality, there is no portability between c++ and golang now. I appreciate you are trying to follow the spec but uncertainties in your response make me feel that more can be done to resolve an important issue.

Thanks!

[puellanivis]

Shouldn't the protobuf libraries across all languages support JSON consistently?

Yes. That is why the standard was written.

Given that we've depended on this c++ functionality…

The C++ implementation does not define the consistent implementation of protobuf libraries across all languages.

The standard is the authority, not what the C++ library does. If one is depending upon behavior in the C++ library that goes beyond the standard, then one is depending upon undefined behavior.

:( The spec does not declare what should be done here, and C++ has gone beyond the specification. While we could implement this feature, there is no mandate for doing so, because the spec does not say one way or the other.

One could just as easily write a bug/issue into the C++ library complaining that they are generating JSON out-of-spec, and need to fix their code.

The matter needs to be settled in the right place: the specification.

[koorgoo]

May be a better solution is to add flags?

I have problems with converting JSON to Protobuf for next communication. These flags could solve my conversion issues.

package jsonpb

type Unmarshaler {
    // Whether to allow parse enum values from lowercase string values.
    EnumFromLower bool 
}

type Marshaler {
    // Whether to convert enum value to lowercase string values.
    EnumToLower bool 
}

[cybrcodr]

https://developers.google.com/protocol-buffers/docs/proto3 states

The name of the enum value as specified in proto is used. Parsers accept both enum names and integer values.

Not sure about C++ library in JSON parsing, but I do know that its textproto parsing will produce error if enum does not match the enum value in proto file.

If your service really needs to produce enum values in lowercase, then perhaps define them in lowercase in the proto file.