Skip to content

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

@umoho

Description

@umoho

Bug Report

Version

├── tonic v0.13.1
└── tonic-build v0.13.1

and

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.

    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.

    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<PlayerJoinRequest>) -> Result<Response<PlayerJoinReply>, 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.

    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?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions