protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Empty 'Any' type field values cannot be correctly serialized/deserialized to JSON text in the C# library

Open tltsaia opened this issue 1 year ago • 3 comments

What version of protobuf and what language are you using? Version: main/v29.2 Language: C#

What operating system (Linux, Windows, ...) and version? Windows 10 22H2 What runtime / compiler are you using (e.g., python version or gcc version) Visual Studio 2022 What did you do? Steps to reproduce the behavior:

The "Any" type fields do not accept null values for packing; only an empty object instantiated with new Any() can be passed to represent a null value. An empty "Any" object is represented as {"@type": "", "@value": ""}. The current C# JsonFormatter and JsonParser both do not accept this input value, and both throw an error: Type registry has no descriptor for type name ''.

For example:

        var rep = new MsgFromProtoFile(); // IMessage instance
        rep.SomeAnyField = Any.Pack(new MyAnyField{ strField= "string", 
            anyField= new Any() // empty Any object, this will be {"@type": "", "@value": ""}
        });

        var tr = TypeRegistry.FromMessages(MyAnyField.Descriptor);
        var json_parser = new JsonParser(JsonParser.Settings.Default.WithTypeRegistry(tr));
        var formater = new JsonFormatter(new JsonFormatter.Settings(true).WithTypeRegistry(tr));

        var json_text = formater.Format(rep); // throw exception as above
        var rep0 = json_parser.Parse<MsgFromProtoFile>(json_text); // throw exception as above

What did you expect to see formater return json text with null value, ex: {"anyField": null} parser return empty Any object as expect, even if {"@type": "", "@value": ""}

What did you see instead? throw an error: Type registry has no descriptor for type name ''.

Make sure you include information that can help us debug (full error message, exception listing, stack trace, logs).

Anything else we should know about your project / environment In the Golang library, an empty "Any" object represented as {"@type": "", "@value": ""} can be processed normally.

It is recommended to handle this in the function MergeAny() of Google.Protobuf.JsonParser

        string typeUrl = token.StringValue;
        string typeName = Any.GetTypeName(typeUrl);

        if (string.IsNullOrEmpty(typeName))
        {
            tokenizer.Next(); // @value
            tokenizer.Next(); // string
            tokenizer.Next(); // EndObject
            return;
        }

to handle this in the function WriteAny() of Google.Protobuf.JsonFormatter

  string typeName = Any.GetTypeName(typeUrl);

        if (string.IsNullOrEmpty(typeName))
        {
            WriteNull(writer);
            return;
        }

tltsaia avatar Dec 20 '24 06:12 tltsaia

I will look at this in January.

jskeet avatar Dec 20 '24 15:12 jskeet

Fundamentally, an Any without a populated type isn't valid. That said, I'd rather be able to deserialize it to the same value than it just fail.

Will consult internally to find out what other languages do.

jskeet avatar Jan 02 '25 08:01 jskeet

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.

github-actions[bot] avatar Apr 02 '25 10:04 github-actions[bot]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please reopen it.

This issue was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

github-actions[bot] avatar Apr 17 '25 10:04 github-actions[bot]

This issue remains unresolved. Do you have any further suggestions? Thank you.

tltsaia avatar Apr 17 '25 10:04 tltsaia

Hey, sorry for slow updates here.

We had checked the other language's behavior on this case and they pretty much were all matching in behavior on this case, so we also updated the conformance test suite to match the behavior that ~all other impls already had.

The behavior that everyone else had that is now in the conformance tests is:

  • null mean that the Any field unset
  • Any with no type_url should round trip as {}
  • {"@type_url": "", "value":""} is malformed and should parse fail

csharp diverging from this is reflected by these lines in the conformance test known failures: https://github.com/protocolbuffers/protobuf/blob/main/conformance/failure_list_csharp.txt#L36-L38

So the right step forward is basically to match that behavior in a way that makes those lines of the conformance test known failures can be removed.

esrauchg avatar Apr 17 '25 13:04 esrauchg

https://github.com/protocolbuffers/protobuf/commit/dc4e4295a4515c593cd8b96e63b3581cc99ac97f implements the change described above. I'll close this issue now, as although it isn't exactly as requested, it's where we want to end up (across all languages).

jskeet avatar Apr 30 '25 13:04 jskeet