Proto2 required fields that contain default values are not serialized correctly

[sujithkris]

My proto is as follows

message Update {
    repeated Prop prop = 1;
    message Prop {
        oneof prop_update_type {
            StandardProp standard_prop = 2;
            RegularProp regular_prop = 5;
        }
    message StandardProp {
        optional string id = 1;
        optional string title = 2;
    }

    message RegularProp {
        required string name = 1;
        required bool is_array = 2;
        optional Value value = 3;
    }
}
message Value {
    oneof value {
        string string_value = 1;
        bool boolean_value = 2;
        int32 integer_value = 3;
        int64 long_value = 4;
        float float_value = 5;
        double double_value = 6;
    }
}

The Update object protoMarshal() is sent across Kafka and on the consumer side, I am getting exceptions which are on java platform. The exceptions are different in different projects.

Note: I am getting exception only when Prop contains RegularProp.

Below listed bullet points are the exception messages I am getting in java projects which deserialize using Update.parseFrom(data)

Project - 1
Value cannot cast to Update
Project - 2
prop[0].regular_prop.is_array not present

Is this due to the presence of Value in RegularProp?

The same proto is used by other Kafka producers in different projects (Javascript, java, etc.) and working fine. So to repeat the exception is only for Kafka message sent from Kotlin to other platform and only when using RegularProp

[sujithkris]

Json representation of Update

• Kotlin updateObject.jsonMarshal()

{"prop":[{"standardProp":null, "regularProp":{"name":"string-key","isArray":false,"value":{"stringValue":"string-value","booleanValue":null,"integerValue":null,"longValue":null,"floatValue":null,"doubleValue":null}}}]}

• Java JsonFormat.printToString(updateObject)

{"prop": [{"regular_prop": {"name": "string-key", "is_array": false, "value": {"string_value": "string-value"}}}]}

[garyp]

I suspect the RegularProp.is_array field is being excluded from the binary serialization because its value is false. This would be the right thing to do in proto3, but not for a required field in proto2. @sujithkris Any chance you can provide the binary data that's being generated by the Kotlin code when this error happens?

[sujithkris]

I suspect the RegularProp.is_array field is being excluded from the binary serialization because its value is false. This would be the right thing to do in proto3, but not for a required field in proto2. @sujithkris Any chance you can provide the binary data that's being generated by the Kotlin code when this error happens?

Yes @garyp, Thank you very much for the pointer. It worked with is_array set to true. So you recommend a migration to proto3 or any other approach?

Would you prefer to look into binary data still? I can try to share it.

Just wondering how this works fine between java producer & java consumer and we noted this issue only now when migrating to Kotlin producer! Or I think I need to verify java to java once again whether this false condition really working :)

[garyp]

@sujithkris This seems like a bug in pbandk. That's why it works in Java 🙂 pbandk isn't doing the correct thing for proto2 required fields that contain default values.

At least that's my hypothesis. If you could share the binary data, that'd help me confirm the hypothesis. But I should be able to also write a quick unit test to test this scenario.

[sujithkris]

@garyp - Good to address the presence of default null in JSON as well, which increases the size of the payload. We can see the null fields does not present in the output of java JsonFormat.printToString(protoObject)

I did not get a chance to get the binary data. Let me know the best way you recommend to share the same.

[garyp]

Good to address the presence of default null in JSON as well, which increases the size of the payload.

@sujithkris The JSON format is technically defined only for proto3 files. The Javadoc for the Java JsonFormat class says (emphasis added):

The JSON format follows Proto3 JSON specification and only proto3 features are supported. Proto2 only features (e.g., extensions and unknown fields) will be discarded in the conversion.

While the JSON format will mostly work fine with proto2 files, there are going to be proto2-specific corner cases with undefined behavior. I wouldn't recommend using JSON with proto2 files until Google officially supports it.

---

I did not get a chance to get the binary data. Let me know the best way you recommend to share the same.

It should be pretty small. So you could just base64-encode it and paste it into a comment here.

[sujithkris]

@sujithkris This seems like a bug in pbandk. That's why it works in Java 🙂 pbandk isn't doing the correct thing for proto2 required fields that contain default values.

@garyp Hi, Do we have a fix ready for pbandk default values in Kotlin?

[garyp]

@sujithkris I was able to reproduce the bug with a unit test. Unfortunately it doesn't look like pbandk ever properly supported required proto2 fields (despite the documentation), so this is going to be a bit more involved to fix than I expected. It won't make the 0.9.0 release (which should be this or next week), but we'll try to get it in for the 0.10.0 release.

Also, to clarify, the bug only impacts proto2 required fields that contain default values (as you have in your example up above). proto2 optional fields with default values and proto3 fields with default values should be working correctly.