Provide a way to configure message marshallers

[jtattermusch]

In C#, the raw message payload are being deserialized/serialized using contextual marshallers (which are instantiated directly from the generated code of protobuf services). Currently there is no way to provide any additional configuration to these marshallers, which causes problems such as https://github.com/grpc/grpc/issues/22682.

Since being able to configure marshallers (e.g. providing recursionLimit for protobuf deserialization) is a very legitimate request, we should looks into ways how we could make marshallers configurable.

[jtattermusch]

CC @JunTaoLuo @JamesNK

[jtattermusch]

The static generated code for instantiating marshallers is e.g. here: https://github.com/grpc/grpc/blob/b49b29b7eaebfb76478e27c4e40d848448be18f3/src/csharp/Grpc.Examples/MathGrpc.cs#L60

For clients, the Marshaller is being passed to the call invocation logic here: https://github.com/grpc/grpc/blob/b49b29b7eaebfb76478e27c4e40d848448be18f3/src/csharp/Grpc.Examples/MathGrpc.cs#L69

And for server the handlers are created like this: https://github.com/grpc/grpc/blob/b49b29b7eaebfb76478e27c4e40d848448be18f3/src/csharp/Grpc.Examples/MathGrpc.cs#L327

[mikhailshilkov]

Thank you @jtattermusch for filing this!

Can anyone give me a rough idea of when this could be implemented so that I could plan my downstream mitigation accordingly?

[jtattermusch]

@mikhailshilkov it looks like this would take some careful design work first (I don't currently have a straightforward way to implement this and it's probably going to be a bit tricky), so it might take a while. Ideas for how this could be implemented are welcome.

[JamesNK]

What does Java/Go do here?

[jtattermusch]

@ejona86 I took a look and it seemed that Java also doesn't have a good way of configuring (e.g. setting recursionLimit etc.) the CodedInputStream that is used for (un)marshalling protobuf messages in the generated grpc stubs. Can you confirm that? If so, is there any plans for providing such feature? https://github.com/grpc/grpc-java/blob/master/protobuf-lite/src/main/java/io/grpc/protobuf/lite/ProtoLiteUtils.java

[ejona86]

Yes, gRPC Java does not support setting recursion limit. We have a global for setting extension registry, although I can't say I'm particularly fond of that solution.

There's only two settings on CodedInputStream: setRecursionLimit and setSizeLimit. We unconditionally setSizeLimit to MAX_INT, as gRPC manages that limit. So that means the only configuration missing is setRecursionLimit, and nobody's complained about that.

If Java needed to provide a way to configure that, I think I'd add a new ProtoUtil API to create a Marshaller with a specified limit and then have users manually create the Marshaller and replace the one that's in the generated MethodDescriptor. That's annoying (mainly on server-side), but feasible for users. Since this is clearly rarely needed, that seems fine.

[stale[bot]]

This issue/PR has been automatically marked as stale because it has not had any update (including commits, comments, labels, milestones, etc) for 30 days. It will be closed automatically if no further update occurs in 7 day. Thank you for your contributions!

[mikhailshilkov]

Shall I come here and leave a "we still need this" comment every 30 days? 🤔

[jtattermusch]

Since Grpc.Core.Api now lives in this repository, I transferred the issue from grpc/grpc and marked it with a new "api" label.

[pepone]

Would be nice to have a way to configure these limits when using other overloads for decoding data, as opposed to only offering this functionally with the CodedInputStream API.

[EronWright]

I opened a PR to simply increase the default recursion limit, as a reasonable workaround: https://github.com/protocolbuffers/protobuf/pull/17881