grpc / grpc/grpc-rust

`max_decoding_message_size` not applied in some cases

Open
#1,525 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
12.5k
Forks
1.3k
Avg merge
4d 7h
Merged PRs (30d)
24

Description

## Bug Report

### Version

Reproduced on HEAD, see test in linked diff.

### Platform

N/A

### Description

Hey,

I'm working with a slightly unorthodox setup involving FFI, and have a custom `tower::Service` to pass the FFI barrier. After upgrading to 0.10, I noticed some of our bulky messages still fail to decode despite setting `MyClient::max_decoding_message_size` to a significantly higher limit.

After some digging, I found #1353. I reduced the repro from my repository to (approximately) what's now in the `max_message_size_blob` test case, which fails with the code as is. My analysis is that due to how our FFI code is implemented, we end up consuming the trailers into headers immediately (not supporting streaming). This triggers the `Streaming::empty` case, which as noted in the previous issue does not pass the `max_decoding_message_size`.

I *think* this is actually a bug in how we re-construct the message because there's no easy way to construct a Body with Trailers from "owned" data. It seems like the gRPC spec doesn't allow passing the trailers in the headers, and the issue goes away if I create a body with two frames of data. However; I'd argue there's still a bug here insofar that deserialization works *as expected* up to 4MB even with our potentially malformed responses; and beyond 4MB if I ensure the [`max_decoding_message_size` is forwarded](https://github.com/hyperium/tonic/compare/master...tgolsson:tonic:ts/max-message-size). I'd thus argue for still passing it to `Streaming::empty()`, as proposed in the original issue and in my eventual PR. This would lead to a more consistent behavior.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.