Server executes RPC handler despite protobuf deserialization failure, returning a generic INTERNAL error
- Dominant language
- Rust
- Stars
- 12.5k
- Forks
- 1.3k
- Avg merge
- 4d 7h
- Merged PRs (30d)
- 24
Description
## Bug Report
### Version
```
├── tonic v0.13.1
└── tonic-build v0.13.1
```
and
```toml
prost = "0.13.1"
prost-types = "0.13.1"
```
and Postman: Version 11.34.5 (11.34.5)
### Platform
`Darwin Kernel Version 23.4.0`
### Description
There appears to be an issue where the `tonic` server executes an RPC handler even if the incoming request cannot be successfully deserialized due to a protobuf schema mismatch. This was observed when sending a gRPC request from Postman (which was configured with a `.proto` file) to a `tonic`-based server.
The expected behavior would be for the server to detect the deserialization failure, reject the request with a specific status code like `INVALID_ARGUMENT`, and never call the handler code.
Instead, the handler is executed, and the client receives a generic `status: Internal, message: "Error decoding request"`. This is problematic because it can lead to unintended side effects, as the handler logic runs on what is effectively a malformed or empty request.
#### How to Reproduce
This scenario can be reproduced using a `tonic` server and a gRPC client like Postman.
1. **Define a `game.proto` for the server:** This is the schema the `tonic` server will be built with.
```proto
syntax = "proto3";
package game;
service Game {
rpc PlayerJoin(PlayerJoinRequest) returns (PlayerJoinReply);
}
message PlayerJoinRequest { string name = 1; }
message PlayerJoinReply { string message = 1; }
```
2. **Create a `tonic` server implementation** based on the `game.proto` above. The handler should log a message to indicate it has been executed.
```rust
use tonic::{transport::Server, Request, Response, Status};
use game::{game_server::{Game, GameServer}, PlayerJoinRequest, PlayerJoinReply};
pub mod game {
tonic::include_proto!("game"); // Corresponds to server's proto
}
#[derive(Debug, Default)]
pub struct MyGame {}
#[tonic::async_trait]
impl Game for MyGame {
async fn player_join(&self, request: Request) -> Result, Status> {
// This log should not appear if the request is malformed
println!("BUG: PlayerJoin RPC handler was executed!");
println!("Request payload: {:?}", request.into_inner());
let reply = PlayerJoinReply {
message: "Welcome!".into(),
};
Ok(Response::new(reply))
}
}
// ... main function to run the server
```
3. **Define a different `client.proto` for the client**, where the field type in the request is intentionally different to simulate a protocol mismatch.
```proto
syntax = "proto3";
package game;
service Game {
rpc PlayerJoin(PlayerJoinRequest) returns (PlayerJoinReply);
}
message PlayerJoinRequest { int32 name = 1; } // Type mismatch here
message PlayerJoinReply { string message = 1; }
```
4. **Use Postman as the client.** Import the `client.proto` (with the `int32` field) into Postman to define the service. Use it to send a `PlayerJoin` request to the running `tonic` server.
(Actually, I don't have a second file, but I have Postman load the `game.proto` file and not change the imported file, probably Postman will store it elsewhere.)
#### Expected Behavior
When Postman sends the `player_join` RPC call:
1. The `tonic` server should fail to deserialize the request body because the `name` field is expected to be a `string`, but an `int32` was sent.
2. The `player_join` handler on the server should **NOT** be executed. The "BUG: PlayerJoin RPC handler was executed!" message should not be printed to the console.
#### Actual Behavior
The `player_join` handler on the server **IS** executed. The "BUG: PlayerJoin RPC handler was executed!" message appears in the server's logs.
After many attempts, the OK status may appear, but the field name of the message is in the old version, and the data is incorrect.
Postman message examples:
- Response message parsing error: invalid wire type 4 at offset 3
- Response message parsing error: index out of range: 4 + 50 > 11
---
I'm not sure if this is the intended behavior that needs to be handled by the server. In my opinion, this should be considered a bad request. Since we cannot make assumptions about the user's input, shouldn't the server have the capability to intercept such malformed requests before they reach the business logic?
Contributor guide
Assessment
This issue has not been assessed yet.