C# JsonParser fails to parse group

[CommonGuy]

What version of protobuf and what language are you using?
Version: 3.27.0
Language: C#

What operating system (Linux, Windows, ...) and version?
Windows, but doesn't matter

What runtime / compiler are you using (e.g., python version or gcc version)
Doesn't matter

What did you do?
Steps to reproduce the behavior:

1. Create a message containing a group (message encoding DELIMITED in edition 2023)
2. Serialize that message to JSON (that works)
3. Try to deserialize the message again (this fails)

What did you expect to see
The JSON should be parsed successfully.

What did you see instead?

Google.Protobuf.InvalidProtocolBufferException
Unsupported JSON token type StartObject for field type Group
   at Google.Protobuf.JsonParser.TryParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer, Object& value)
   at Google.Protobuf.JsonParser.MergeField(IMessage message, FieldDescriptor field, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.Merge(IMessage message, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.Merge(IMessage message, TextReader jsonReader)
   at Google.Protobuf.JsonParser.Parse[T](TextReader jsonReader)
   at Google.Protobuf.JsonParser.Parse[T](String json)

This happens at https://github.com/protocolbuffers/protobuf/blob/main/csharp/src/Google.Protobuf/JsonParser.cs#L380, since the code does not handle groups at all. I think this was due to version 3.26.0 having workarounds that reported the field type of a group as "message".

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

Proto2 test message:

message Proto2TestMessage {
  optional group Test = 1 {
    optional int32 number = 1;
    map<string, string> map = 2;
  }
  repeated group RepeatedGroup = 2 {
    optional bool entry = 1;
  }
}

Edition 2023 test message:

message Edition2023TestMessage {
  message Test {
    bool my_bool = 1;
  }

  Test test = 1 [features.message_encoding = DELIMITED];
}

[jskeet]

Thanks for reporting this.

When you say:

I think this was due to version 3.26.0 having workarounds that reported the field type of a group as "message".

... do you mean that it was parsed successfully in 3.26.0?

[jskeet]

(Logged internally as b/343354710)

[CommonGuy]

Yes, it worked in 3.26.1. But as far as I know, that relied on the (weird IMO) behavior where "group fields" were reported as "message fields". More specifially, FieldDescriptor.FieldType now (correctly) reports FieldType.Group, whereas in 3.26.1 it always returned FieldType.Message.

Probably introduced in this change (included in 3.27.0, but not 3.26.1): 450022d#diff-675b90652e1eebe500f6c60eaea34d328f3113d80e17d8375d5f0a01a53aa520R256

This check https://github.com/protocolbuffers/protobuf/blob/main/csharp/src/Google.Protobuf/JsonParser.cs#L333 probably needs to include checking for FieldType.Group. Maybe that is already enough to fix this bug.

[jskeet]

Okay, I'll look when I get a chance. I don't know when I'll be able to get to this though - hopefully in the next couple of weeks.

[CommonGuy]

I went ahead and created a PR for this

[CommonGuy]

Thanks for accepting the PR this quick!
Just a question, as I would be really happy to use the fixed version: On https://www.nuget.org/packages/google.protobuf/, version 3.27.1 does not show up (yet). Does this take a while or was this forgotten?

[jskeet]

@CommonGuy: It's now been published.