Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correctly set ExtensionRegistry when parsing with MessageParser, but using an already existing CodedInputStream #7246

Merged
merged 2 commits into from Oct 13, 2021
Merged

Correctly set ExtensionRegistry when parsing with MessageParser, but using an already existing CodedInputStream #7246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca123ec
Swap registry from input when merging from existing input
ObsidianMinor Feb 23, 2020
d36d84c
Add test for parsing using coded input
ObsidianMinor Feb 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions csharp/src/Google.Protobuf.Test/GeneratedMessageTest.Proto2.cs
View file
Open in desktop
Expand Up @@ -376,5 +376,18 @@ public void RoundTrip_NestedExtensionGroup()
TestGroupExtension extendable_parsed = TestGroupExtension.Parser.WithExtensionRegistry(new ExtensionRegistry() { TestNestedExtension.Extensions.OptionalGroupExtension }).ParseFrom(bytes);
Assert.AreEqual(message, extendable_parsed);
}

[Test]
public void RoundTrip_ParseUsingCodedInput()
{
var message = new TestAllExtensions();
message.SetExtension(UnittestExtensions.OptionalBoolExtension, true);
byte[] bytes = message.ToByteArray();
using (CodedInputStream input = new CodedInputStream(bytes))
{
var parsed = TestAllExtensions.Parser.WithExtensionRegistry(new ExtensionRegistry() { UnittestExtensions.OptionalBoolExtension }).ParseFrom(input);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

If at all possible, can there also be a way to pass an extension registry to ParseFrom/MergeFrom? It would be useful to be able to use the static MessageParser that's already present for a message and not have to construct a new one, or to be able to construct a new message and use MergeFrom with some stream and an extension registry. The current behavior of having WithExtensionRegistry create a new MessageParser each time is not ideal when parsing many different types of messages very quickly e.g. in an RPC system.

Assert.AreEqual(message, parsed);
}
}
}
}
3 changes: 3 additions & 0 deletions csharp/src/Google.Protobuf/MessageParser.cs
View file
Open in desktop
Expand Up @@ -159,14 +159,17 @@ public IMessage ParseJson(string json)
internal void MergeFrom(IMessage message, CodedInputStream codedInput)
{
bool originalDiscard = codedInput.DiscardUnknownFields;
ExtensionRegistry originalRegistry = codedInput.ExtensionRegistry;
try
{
codedInput.DiscardUnknownFields = DiscardUnknownFields;
codedInput.ExtensionRegistry = Extensions;
message.MergeFrom(codedInput);
}
finally
{
codedInput.DiscardUnknownFields = originalDiscard;
codedInput.ExtensionRegistry = originalRegistry;
}
}

Expand Down