protobufjs / protobuf.js

Protocol Buffers for JavaScript & TypeScript.
Other
9.77k stars 1.4k forks source link

google.protobuf.Timestamp deserialization incompatible with canonical JSON representation #893

Open shylent opened 6 years ago

shylent commented 6 years ago

I am consuming a HTTP API, that is generated by grpc-gateway.

In protobuf3 docs (https://developers.google.com/protocol-buffers/docs/proto3#json) it says, that timestamps are encoded as RFC 3339 strings.

So, any Timestamp fields end up as strings when encoded to JSON. However, generated xxx.fromObject methods expect Timestamp fields to be objects with seconds and nanos fields, so messages, that contain non-null Timestamp fields are never deserialized.

I am using generated static-code.

Is there any additional info, that I could provide? Maybe I am just misunderstanding something?

dcodeIO commented 6 years ago

For reference: https://github.com/google/protobuf/blob/master/src/google/protobuf/timestamp.proto

protobuf.js uses so called wrappers to provide additional functionality on top of the raw data types (seconds, nanos). There's one for any, but none for timestamp yet.

shylent commented 6 years ago

Hello and thanks so much for such a quick reply!

Could you please expand a bit on the topic of wrappers (or, alternatively, point me to a comment / document) ? Am I correct in thinking, that if there was a wrapper for 'google.protobuf.Timestamp', it would've been used instead of default "fromObject" functionality, whenever timestamp is encountered?

Now that I think of it, may it be that I am doing something completely wrong? At the moment I am receiving JSON representations of protobuf messages from a HTTP API and I am using MessageName.fromObject on them to convert them from plain objects to messages. Maybe there is some other, correct way of decoding JSON representations of protobuf messages? I am using typescript, if that matters.

elwaxoro commented 6 years ago

Running into this now, was there any resolution?

ntindall commented 6 years ago

I think we need to revive this PR https://github.com/dcodeIO/protobuf.js/pull/929 (and the implementation present in the fork: https://github.com/zia-ai/protobuf.js/tree/feature/struct)

elwaxoro commented 6 years ago

protobuf3 writes Timestamp as a plain string in the RFC 3339 format (2018-06-19T18:27:29.350Z). Based on zia-ai's branch, I put together a hack to get things working. Should be an easy fix in zia-ai's branch, but I don't know enough about the rest to verify it.

wrappers.js

// Custom wrapper for Timestamp
wrappers[".google.protobuf.Timestamp"] = {
    fromObject: function(object) {
        //Convert ISO-8601 to epoch millis
        var dt = Date.parse(object);
        return this.create({
            seconds: Math.floor(dt/1000),
            nanos: (dt % 1000) * 1000
        })
    },

    toObject: function(message, options) {
        return new Date(message.seconds*1000 + message.nanos/1000).toISOString();
    }
};

converter.js genValuePartial_fromObject

...
        if (field.resolvedType instanceof Enum) { gen
...
        } else if(field.resolvedType.name === "Timestamp") {
            //Custom handler for Timestamp as a string
            gen
            ("if(typeof d%s!==\"string\")", prop)
                ("throw TypeError(%j)", field.fullName + ": string expected")
            ("m%s=types[%i].fromObject(d%s)", prop, fieldIndex, prop);
        } else gen
...

Update: I'm seeing the wrapper getting called from the unit test, but when I compile a .proto file into javascript, the wrapper is not called. Not sure what I'm doing wrong there, but the only way I can get it to "work" is to include the wrapper code in the genValuePartial_fromObject and genValuePartial_toObject:

function genValuePartial_fromObject(gen, field, fieldIndex, prop) {
    /* eslint-disable no-unexpected-multiline, block-scoped-var, no-redeclare */
    if (field.resolvedType) {
        if (field.resolvedType instanceof Enum) { gen
...
        } else if(field.resolvedType.name === "Timestamp") {
            //Custom handler for Timestamp as a string
            gen
            ("if(typeof d%s!==\"string\")", prop)
                ("throw TypeError(%j)", field.fullName + ": string expected")
            ("var dt = Date.parse(d%s)", prop)
            ("m%s=types[%i].fromObject(d%s)", prop, fieldIndex, prop)
            ("m%s.seconds=Math.floor(dt/1000)", prop)
            ("m%s.nanos=(dt\%1000)*1000", prop);
        } else gen
...
function genValuePartial_toObject(gen, field, fieldIndex, prop) {
    /* eslint-disable no-unexpected-multiline, block-scoped-var, no-redeclare */
    if (field.resolvedType) {
        if (field.resolvedType instanceof Enum) gen
            ("d%s=o.enums===String?types[%i].values[m%s]:m%s", prop, fieldIndex, prop, prop);
        else if(field.resolvedType.name === "Timestamp") {
            //Custom handler for Timestamp as a string
            gen
            ("d%s=new Date(m%s.seconds*1000+m%s.nanos/1000).toISOString()", prop, prop, prop);
        } else gen
            ("d%s=types[%i].toObject(m%s,o)", prop, fieldIndex, prop);
...

edit: nanos factor is 1000000, not 1000 (if anyone wants to use the above)

nof20 commented 5 years ago

+1

vordimous commented 5 years ago

+1

ntindall commented 5 years ago

https://github.com/protobufjs/protobuf.js/pull/1258 addresses this

chambs15 commented 5 years ago

Any news on this? I am seeing something similar on C#. When trying to use the Jsonparser I get the following exception using it like var temp = JsonParser.Default.Parse<myclass>(json);

Google.Protobuf.InvalidProtocolBufferException: Expected string value for Timestamp at Google.Protobuf.JsonParser.MergeTimestamp (Google.Protobuf.IMessage message, Google.Protobuf.JsonToken token) [0x00009] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:792 at Google.Protobuf.JsonParser+<>c.<.cctor>b__39_0 (Google.Protobuf.JsonParser parser, Google.Protobuf.IMessage message, Google.Protobuf.JsonTokenizer tokenizer) [0x00000] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:74 at Google.Protobuf.JsonParser.Merge (Google.Protobuf.IMessage message, Google.Protobuf.JsonTokenizer tokenizer) [0x0003f] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:160 at Google.Protobuf.JsonParser.ParseSingleValue (Google.Protobuf.Reflection.FieldDescriptor field, Google.Protobuf.JsonTokenizer tokenizer) [0x00062] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:346 at Google.Protobuf.JsonParser.MergeField (Google.Protobuf.IMessage message, Google.Protobuf.Reflection.FieldDescriptor field, Google.Protobuf.JsonTokenizer tokenizer) [0x0005f] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:245 at Google.Protobuf.JsonParser.Merge (Google.Protobuf.IMessage message, Google.Protobuf.JsonTokenizer tokenizer) [0x000fd] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:202 at Google.Protobuf.JsonParser.ParseSingleValue (Google.Protobuf.Reflection.FieldDescriptor field, Google.Protobuf.JsonTokenizer tokenizer) [0x00062] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:346 at Google.Protobuf.JsonParser.MergeRepeatedField (Google.Protobuf.IMessage message, Google.Protobuf.Reflection.FieldDescriptor field, Google.Protobuf.JsonTokenizer tokenizer) [0x00056] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:267 at Google.Protobuf.JsonParser.MergeField (Google.Protobuf.IMessage message, Google.Protobuf.Reflection.FieldDescriptor field, Google.Protobuf.JsonTokenizer tokenizer) [0x00055] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:241 at Google.Protobuf.JsonParser.Merge (Google.Protobuf.IMessage message, Google.Protobuf.JsonTokenizer tokenizer) [0x000fd] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:202 at Google.Protobuf.JsonParser.ParseSingleValue (Google.Protobuf.Reflection.FieldDescriptor field, Google.Protobuf.JsonTokenizer tokenizer) [0x00062] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:346 at Google.Protobuf.JsonParser.MergeRepeatedField (Google.Protobuf.IMessage message, Google.Protobuf.Reflection.FieldDescriptor field, Google.Protobuf.JsonTokenizer tokenizer) [0x00056] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:267 at Google.Protobuf.JsonParser.MergeField (Google.Protobuf.IMessage message, Google.Protobuf.Reflection.FieldDescriptor field, Google.Protobuf.JsonTokenizer tokenizer) [0x00055] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:241 at Google.Protobuf.JsonParser.Merge (Google.Protobuf.IMessage message, Google.Protobuf.JsonTokenizer tokenizer) [0x000fd] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:202 at Google.Protobuf.JsonParser.Merge (Google.Protobuf.IMessage message, System.IO.TextReader jsonReader) [0x00007] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:134 at Google.Protobuf.JsonParser.Parse[T] (System.IO.TextReader jsonReader) [0x00012] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:398 at Google.Protobuf.JsonParser.Parse[T] (System.String json) [0x0000c] in T:\src\github\protobuf\csharp\src\Google.Protobuf\JsonParser.cs:384