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

Protobuf-net.Grpc fails when using protobuf-net 3.0 alpha #100

Closed
RichardVogelij opened this issue Jun 22, 2020 · 6 comments · Fixed by #102
Closed

Protobuf-net.Grpc fails when using protobuf-net 3.0 alpha #100

RichardVogelij opened this issue Jun 22, 2020 · 6 comments · Fixed by #102

Comments

@RichardVogelij
Copy link

RichardVogelij commented Jun 22, 2020 •
edited
Loading

Hi, when I try to send over a slightly complex object which uses ProtoInclude (which works fine on protobuf-net 2.*) it fails in 3.0-alpha when I update protobuf-net.grpc from 1.0.81 to 1.0.90.

So in essence: this issue rises when using protobuf.net 3.0+ and protobuf-net.grpc 1.0.90 (not on 1.0.81)

There are no issues when I serialize/deserialize/deepclone my object in protobuf-net-3.0.0-alpha - but protobuf-net.grpc fails server side in protobuf-net.grpc.

This is the object in which this issue is reproduced.

[ProtoContract]
public class TestObject
{
	[ProtoMember(1)]
	public TestThingy Test {get;set;}
}

[ProtoContract]
[ProtoInclude(1, typeof(TestThingy))]
public abstract class TestBase
{
	[ProtoMember(2)]
	public string SomeText { get; set; }

	public abstract string SomeText2 { get; set; }
}

[ProtoContract]
public class TestThingy : TestBase
{
	[ProtoMember(1)]
	public override string SomeText2 { get; set; }
}

public interface ITest { ValueTask<TestObject> GetTestInstance(); }

As soon as my server-side implementation of ITest returns the TestObject i get:

Length mismatch; calculated '18', actual '8'


   at ProtoBuf.Internal.ThrowHelper.ThrowInvalidOperationException(String message, Exception innerException) in /_/src/protobuf-net.Core/Internal/ThrowHelper.cs:line 56
   at ProtoBuf.ProtoWriter.BufferWriterProtoWriter.WriteWithLengthPrefix[T](State& state, T value, ISubTypeSerializer`1 serializer) in /_/src/protobuf-net.Core/ProtoWriter.BufferWriter.cs:line 288
   at ProtoBuf.ProtoWriter.BufferWriterProtoWriter.WriteWithLengthPrefix[T](State& state, T value, ISerializer`1 serializer, PrefixStyle style) in /_/src/protobuf-net.Core/ProtoWriter.BufferWriter.cs:line 260
   at ProtoBuf.ProtoWriter.BufferWriterProtoWriter.WriteMessage[T](State& state, T value, ISerializer`1 serializer, PrefixStyle style, Boolean recursionCheck) in /_/src/protobuf-net.Core/ProtoWriter.BufferWriter.cs:line 209
   at ProtoBuf.ProtoWriter.State.WriteMessage[T](Int32 fieldNumber, SerializerFeatures features, T value, ISerializer`1 serializer) in /_/src/protobuf-net.Core/ProtoWriter.State.WriteMethods.cs:line 335
   at ProtoBuf.ProtoWriter.State.SerializeRoot[T](T value, ISerializer`1 serializer) in /_/src/protobuf-net.Core/ProtoWriter.State.WriteMethods.cs:line 542
   at ProtoBuf.MeasureState`1.SerializeCore(State state) in /_/src/protobuf-net.Core/MeasureState.cs:line 86
   at ProtoBuf.MeasureState`1.Serialize(IBufferWriter`1 writer) in /_/src/protobuf-net.Core/MeasureState.cs:line 106
   at ProtoBuf.Meta.TypeModel.ProtoBuf.IMeasuredProtoOutput<System.Buffers.IBufferWriter<System.Byte>>.Serialize[T](MeasureState`1 measured, IBufferWriter`1 destination) in /_/src/protobuf-net.Core/Meta/TypeModel.InputOutput.cs:line 47
   at ProtoBuf.Grpc.Configuration.ProtoBufMarshallerFactory.ContextualSerialize[T](T value, SerializationContext context) in ~\protobuf-src\protobuf-net.Grpc\src\protobuf-net.Grpc\Configuration\ProtoBufMarshallerFactory.cs:line 129
   at Grpc.Core.Internal.AsyncCallBase`2.UnsafeSerialize(TWrite msg, DefaultSerializationContext context)
   at Grpc.Core.Internal.AsyncCallServer`2.SendStatusFromServerAsync(Status status, Metadata trailers, Nullable`1 optionalWrite)
   at Grpc.Core.Internal.UnaryServerCallHandler`2.<HandleCall>d__4.MoveNext()

I attempted to use protobuf-net 3.0 in my project because it seems to be the only version that does not give all sorts of trouble regarding the dependency on System.Reflection.Emit in 2.0 which is not possible in a Xamarin ios project.

This issue started to appear when I updated protobuf-net.grpc from 1.0.81 to 1.0.90 - I already was running protobuf.net in 3.0 space some time.

I am fairly certain the bug is actually in protobuf-net 3.0.alpha - but since updating protobuf-grpc.net to the latest version started to cause this issue I felt i should create the issue here.

Thank you for the great work on protobuf-net.*!!!
@mgravell
Copy link
Member

mgravell commented Jun 22, 2020 via email

mgravell added a commit that referenced this issue Jun 23, 2020
@mgravell
add failing tests for #100
978c170
@mgravell
Copy link
Member

Failing test added; looks like a bug in protobuf-net - no need to relocate the issue, though - this is fine. Will get that fixed. A temp workaround may be to use a custom binder config that omits the contextual (measured) mode:

var binderConfig = BinderConfiguration.Create(new List<MarshallerFactory> {
    ProtoBufMarshallerFactory.Create(options: ProtoBufMarshallerFactory.Options.DisableContextualSerializer)
});
_server.Services.AddCodeFirst(this, binderConfig);

The above is for Google's server implementation; if you're using Kestrel, IIRC it is similar but you need to register the BinderConfiguration with the DI:

services.AddSingleton(binderConfig);

mgravell added a commit to protobuf-net/protobuf-net that referenced this issue Jun 23, 2020
@mgravell
investigate protobuf-net/protobuf-net.Grpc#100
a9eb54c 
- when considering a pre-measured object without sub-type awareness, we need to think in terms of the base type
- add new IObjectSerializer<T> which expresses this; without that: no measurements are trusted
- add test for the specific scenario raised in GRPC 100
- implement the new API is the 4 runtime wrappers
- implement the new API in generated types
- TODO: investigate PEVerify warnings
@mgravell mgravell mentioned this issue Jun 23, 2020
Merged
mgravell added a commit to protobuf-net/protobuf-net that referenced this issue Jun 23, 2020
@mgravell
Fix bug in pre-measured subtypes (#667)
80789c5 
* investigate protobuf-net/protobuf-net.Grpc#100

- when considering a pre-measured object without sub-type awareness, we need to think in terms of the base type
- add new IObjectSerializer<T> which expresses this; without that: no measurements are trusted
- add test for the specific scenario raised in GRPC 100
- implement the new API is the 4 runtime wrappers
- implement the new API in generated types
- TODO: investigate PEVerify warnings

* fix emit Type (we were returning the token, not the Type)
@mgravell
Copy link
Member

mgravell commented Jun 23, 2020 •
edited
Loading

Found and fixed. Waiting on CI, then I'll let you know the version number you need.

Edit: it'll be 3.0.1; waiting on CI, then deploying

mgravell added a commit that referenced this issue Jun 23, 2020
@mgravell
consume pb-net 3.0.1; fix #100
f6fcbd5
@mgravell mgravell mentioned this issue Jun 23, 2020
Merged
mgravell added a commit that referenced this issue Jun 23, 2020
@mgravell
Fix issue with sub-type length mismatch (#102)
bb1961b 
* add failing tests for #100

* consume pb-net 3.0.1; fix #100
@mgravell
Copy link
Member

Fix is:

<PackageReference Include="protobuf-net" Version="3.0.1" />

@RichardVogelij
Copy link
Author

Thank you Marc!

@mgravell
Copy link
Member

(note: 3.0.2 is a minor tweak to the implementation details of the same problem; I realized overnight that I was over-complicating things)

@mgravell mgravell mentioned this issue Apr 1, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix issue with sub-type length mismatch protobuf-net/protobuf-net.Grpc
2 participants
@mgravell @RichardVogelij