-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
Custom codec #461
Custom codec #461
Conversation
This allows the underlying codec to be configured by a build script, useful when using a codec wrapper.
We have custom codec support already via https://docs.rs/tonic-build/0.3.1/tonic_build/server/fn.generate.html and https://docs.rs/tonic-build/0.3.1/tonic_build/client/fn.generate.html that use https://docs.rs/tonic-build/0.3.1/tonic_build/trait.Service.html. You should be able to override the current prost one and just replace the codec path? |
While this does work, it means I'd have to copy paste the whole prost file in my build.rs, just to make a very minor modification to the My use-case, in particular, is to allow encrypting the gRPC payload. I still want to use the underlying prost types and serializer, but I want to add a small layer on top that encrypts it (using an AEAD) before writing it to the wire. AFAIK, the codec is the only place in tonic I can hook into to do this. |
Yeah, I see your point, I specifically didn't include this because I wanted to push people to use the trait rather than a method with a string (which I think is a finky and bad way to expose things personally). I think what you want is compression for your prost messages and I think this is a different problem all together with how we incorporate it. I have not had much time to think about it but we could possibly provide hooks for this. Maybe, when you create the client you can pass some encoding config or something like that? In the end this feels like a hack that we could avoid if we just implemented the proper solution for compression. |
I mean, that really depends. I'm not well-versed in how compression works in gRPC. My middleware needs to have two information at hand to work:
FWIW, I originally thought this whole logic could run in an Interceptor, but it turns out tonic interceptors are a lot less powerful than their Python counterpart. In Python, interceptors can wrap the underlying serialiser, deserialiser, and callback. For example: class CryptoInterceptor(grpc.aio.ServerInterceptor):
async def intercept_service(self, continuation, handler_call_details):
# Returns the RpcMethodHandler associated with this req
inner = await continuation(handler_call_details)
# Creates a wrapper handler
handler = grpc.RpcMethodHandler()
handler.request_streaming = inner.request_streaming
handler.response_streaming = inner.response_streaming
handler.unary_unary = inner.unary_unary
handler.unary_stream = inner.unary_stream
handler.stream_unary = inner.stream_unary
handler.stream_stream = inner.stream_stream
# Injects aes crypto in the wrapper handler
handler.request_deserializer = wrap_deserializer(inner)
handler.response_serializer = wrap_serializer(inner)
return handler In Tonic, as far as I understand, Interceptors can only inspect the headers and optionally return an error to stop the request from being processed any further. I suppose this is meant to be used for authentication? But it prevents us from adding our crypto layer here. |
Yeah, there are two main ways to do compression: 1) Compressing the body like any other http request (using |
Motivation
Allow the user to specify a custom prost codec implementation.
My use-case is to provide application-layer encryption, by (en/de)crypting the messages at serialization time, before writing them to the wire. Other use-cases include serializing as JSON instead of ProtoBuf (once support for it lands in prost).
Solution
The idea is to allow the user to specify a path to their codec implementation, e.g.
This is technically a breaking change, as it requires moving
Method::CODEC_PATH
to be a method instead. It's probably possible to avoid the breaking change (by leavingCODEC_PATH
, and makingcodec_path
have a default implementation returningCODEC_PATH
), but since a 0.7 version seems to be in the work, I assume a breaking change isn't a big problem.