Skip to content

Commit

Permalink
transport: allow setting max_header_list_size (#1835)
Browse files Browse the repository at this point in the history
There is a bug such that if the client sends a response with a header
value that exceeds the max_header_list_size, then RPCs just hang
(#1834). When tonic upgraded to hyper 1, it picked up [hyper#3622] which
changed the default from 16MiB to 16KiB for this configuration value.
Error messages in gRPC use headers. That means that services which ever
sent error messages in excess of 16KiB (including in their error
details!) will just hang.

This commit adds the ability for the client to configure this value to
something larger (perhaps the old default of 16MiB) to mitigate the
above-referenced bug.

[hyper#3622]: hyperium/hyper#3622
  • Loading branch information
ajwerner authored Aug 3, 2024
1 parent aff5eed commit decbf61
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 0 deletions.
70 changes: 70 additions & 0 deletions tests/integration_tests/tests/http2_max_header_list_size.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use std::time::Duration;

use integration_tests::pb::{test_client, test_server, Input, Output};
use tokio::sync::oneshot;
use tonic::{
transport::{Endpoint, Server},
Request, Response, Status,
};

/// This test checks that the max header list size is respected, and that
/// it allows for error messages up to that size.
#[tokio::test]
async fn test_http_max_header_list_size_and_long_errors() {
struct Svc;

// The default value is 16k.
const N: usize = 20_000;

fn long_message() -> String {
"a".repeat(N)
}

#[tonic::async_trait]
impl test_server::Test for Svc {
async fn unary_call(&self, _: Request<Input>) -> Result<Response<Output>, Status> {
Err(Status::internal(long_message()))
}
}

let svc = test_server::TestServer::new(Svc);

let (tx, rx) = oneshot::channel::<()>();
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = format!("http://{}", listener.local_addr().unwrap());

let jh = tokio::spawn(async move {
let (nodelay, keepalive) = (true, Some(Duration::from_secs(1)));
let listener =
tonic::transport::server::TcpIncoming::from_listener(listener, nodelay, keepalive)
.unwrap();
Server::builder()
.http2_max_pending_accept_reset_streams(Some(0))
.add_service(svc)
.serve_with_incoming_shutdown(listener, async { drop(rx.await) })
.await
.unwrap();
});

tokio::time::sleep(Duration::from_millis(100)).await;

let channel = Endpoint::from_shared(addr)
.unwrap()
// Increase the max header list size to 2x the default value. If this is
// not set, this test will hang. See
// <https://github.com/hyperium/tonic/issues/1834>.
.http2_max_header_list_size(u32::try_from(N * 2).unwrap())
.connect()
.await
.unwrap();

let mut client = test_client::TestClient::new(channel);

let err = client.unary_call(Request::new(Input {})).await.unwrap_err();

assert_eq!(err.message(), long_message());

tx.send(()).unwrap();

jh.await.unwrap();
}
10 changes: 10 additions & 0 deletions tonic/src/transport/channel/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub struct Endpoint {
pub(crate) http2_keep_alive_interval: Option<Duration>,
pub(crate) http2_keep_alive_timeout: Option<Duration>,
pub(crate) http2_keep_alive_while_idle: Option<bool>,
pub(crate) http2_max_header_list_size: Option<u32>,
pub(crate) connect_timeout: Option<Duration>,
pub(crate) http2_adaptive_window: Option<bool>,
pub(crate) executor: SharedExec,
Expand Down Expand Up @@ -290,6 +291,14 @@ impl Endpoint {
}
}

/// Sets the max header list size. Uses `hyper`'s default otherwise.
pub fn http2_max_header_list_size(self, size: u32) -> Self {
Endpoint {
http2_max_header_list_size: Some(size),
..self
}
}

/// Sets the executor used to spawn async tasks.
///
/// Uses `tokio::spawn` by default.
Expand Down Expand Up @@ -420,6 +429,7 @@ impl From<Uri> for Endpoint {
http2_keep_alive_interval: None,
http2_keep_alive_timeout: None,
http2_keep_alive_while_idle: None,
http2_max_header_list_size: None,
connect_timeout: None,
http2_adaptive_window: None,
executor: SharedExec::tokio(),
Expand Down
4 changes: 4 additions & 0 deletions tonic/src/transport/channel/service/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ impl Connection {
settings.adaptive_window(val);
}

if let Some(val) = endpoint.http2_max_header_list_size {
settings.max_header_list_size(val);
}

let stack = ServiceBuilder::new()
.layer_fn(|s| {
let origin = endpoint.origin.as_ref().unwrap_or(&endpoint.uri).clone();
Expand Down

0 comments on commit decbf61

Please sign in to comment.