grpc / grpc/grpc-rust

Server executes RPC handler despite protobuf deserialization failure, returning a generic INTERNAL error

Open
#2,356 0 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

```
├── 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

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.