transport: allow setting max_header_list_size (#1835)

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

Author: ajwerner

tests/integration_tests/tests/http2_max_header_list_size.rs

@@ -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();
+}

tonic/src/transport/channel/endpoint.rs

@@ -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,
@@ -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.
@@ -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(),

tonic/src/transport/channel/service/connection.rs

@@ -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();