Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changesets/feat_http2_header_size_limits_for_tcp_uds.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
### Enables HTTP/2 header size limits for TCP and UDS (unix sockets)

The router config's HTTP/2 header size limit option is now respected by requests using TCP and UDS. Previously it would only work for TLS connections.

By [@aaronArinder](https://github.com/aaronArinder) in https://github.com/apollographql/router/pull/8673
142 changes: 71 additions & 71 deletions apollo-router/src/axum_factory/listeners.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ use std::time::Duration;

use axum::Router;
use axum::response::*;
use bytesize::ByteSize;
use futures::channel::oneshot;
use futures::prelude::*;
use hyper_util::rt::TokioExecutor;
use hyper_util::rt::TokioIo;
use hyper_util::rt::TokioTimer;
use hyper_util::server::conn::auto::Builder;
use hyper_util::server::conn::auto::Http1Builder;
use multimap::MultiMap;
#[cfg(unix)]
use tokio::net::UnixListener;
Expand Down Expand Up @@ -354,6 +356,24 @@ pub(super) fn serve_router_on_listen_addr(
let _connection_stop_signal = connection_stop_signal;
let connection_handle = ConnectionHandle::new(pipeline_ref, address);

// Development note: the following describes the different network
// streams and how to think about them when modifying the logic
// below. In general, a change to one stream (eg, TLS) should also
// have similar changes to the others unless there's a very good
// reason why it shouldn't be applied more broadly. Any changes to
// how the connections are configured should be centralized in
// configure_connections()
//
// Here are some pratical examples of each network stream below:
// - TCP :: after TLS termination (eg, reverse proxy), dev
// servers, or testing environments; ie, whenever https isn't
// used
// - Unix :: unix sockets, which are used widely in sidecars or
// when commnicating on the same machine, but can also be used
// by reverse proxies
// - TLS :: encrypted requests, often when the router is used to
// expose its graph to the public internet (ie, not behind a proxy)

match res {
NetworkStream::Tcp(stream) => {
let received_first_request = Arc::new(AtomicBool::new(false));
Expand All @@ -374,19 +394,8 @@ pub(super) fn serve_router_on_listen_addr(
});

let mut builder = Builder::new(TokioExecutor::new());
let mut http_connection = builder.http1();
let http_config = http_connection
.keep_alive(true)
.timer(TokioTimer::new())
.header_read_timeout(header_read_timeout);
if let Some(max_headers) = opt_max_http1_headers {
http_config.max_headers(max_headers);
}

if let Some(max_buf_size) = opt_max_http1_buf_size{
http_config.max_buf_size(max_buf_size.as_u64() as usize);
}
let connection = http_config.serve_connection_with_upgrades(tokio_stream, hyper_service);
let config = configure_connection(&mut builder, header_read_timeout, opt_max_http1_headers, opt_max_http1_buf_size, opt_max_http2_headers_list_bytes);
let connection = config.serve_connection_with_upgrades(tokio_stream, hyper_service);
handle_connection!(connection, connection_handle, connection_shutdown, connection_shutdown_timeout, received_first_request);
}
#[cfg(unix)]
Expand All @@ -398,19 +407,8 @@ pub(super) fn serve_router_on_listen_addr(
app.clone().call(request)
});
let mut builder = Builder::new(TokioExecutor::new());
let mut http_connection = builder.http1();
let http_config = http_connection
.keep_alive(true)
.timer(TokioTimer::new())
.header_read_timeout(header_read_timeout);
if let Some(max_headers) = opt_max_http1_headers {
http_config.max_headers(max_headers);
}

if let Some(max_buf_size) = opt_max_http1_buf_size {
http_config.max_buf_size(max_buf_size.as_u64() as usize);
}
let connection = http_config.serve_connection_with_upgrades(tokio_stream, hyper_service);
let config = configure_connection(&mut builder, header_read_timeout, opt_max_http1_headers, opt_max_http1_buf_size, opt_max_http2_headers_list_bytes);
let connection = config.serve_connection_with_upgrades(tokio_stream, hyper_service);
handle_connection!(connection, connection_handle, connection_shutdown, connection_shutdown_timeout, received_first_request);
},
NetworkStream::Tls(stream) => {
Expand All @@ -427,52 +425,11 @@ pub(super) fn serve_router_on_listen_addr(
app.clone().call(request)
});

// the builder can toggle between http/2 and http/1, which
// makes for somewhat confusing code below
let mut builder = Builder::new(TokioExecutor::new());

// ALPN (application-layer protocol negotiation) is an
// extension of TLS that lets the client/server agree on a
// protocol during the TLS handshake
if stream.get_ref().1.alpn_protocol() == Some(&b"h2"[..]) {
// if we see "h2", we're going to use only HTTP/2
// WARN: from the docs, this doesn't do anything when
// used with serve_connection_with_upgrades(), which we
// use below
builder = builder.http2_only();

}

let mut http2_config = builder.http2();
if let Some(max_header_list_size) = opt_max_http2_headers_list_bytes {
// ByteSize as a .as_u64, but not as_u32, to explain
// this funky `as` conversion
http2_config.max_header_list_size(max_header_list_size.as_u64() as u32);
}

// access the http1 config via the http2 builder doesn't
// erase the http2 config, only extends the overall
// configuration for connections
let mut http1_config = http2_config.http1();
let http_config = http1_config
.keep_alive(true)
.timer(TokioTimer::new())
.header_read_timeout(header_read_timeout);


if let Some(max_headers) = opt_max_http1_headers {
// NOTE: max headers has no effect on http2
http_config.max_headers(max_headers);
}


if let Some(max_buf_size) = opt_max_http1_buf_size {
http_config.max_buf_size(max_buf_size.as_u64() as usize);
}


let tokio_stream = TokioIo::new(stream);
let connection = http_config

let mut builder = Builder::new(TokioExecutor::new());
let config = configure_connection(&mut builder, header_read_timeout, opt_max_http1_headers, opt_max_http1_buf_size, opt_max_http2_headers_list_bytes);
let connection = config
.serve_connection_with_upgrades(tokio_stream, hyper_service);

handle_connection!(connection, connection_handle, connection_shutdown, connection_shutdown_timeout, received_first_request);
Expand All @@ -496,6 +453,49 @@ pub(super) fn serve_router_on_listen_addr(
(server, shutdown_sender)
}

/// Configures a connection, no matter whether it's TLS, TCP, or UDS (ie, unix sockets) with
/// whatever parameters are configured by the end-user in their router config
///
/// NOTE: centralize all connection configuration changes here so that the behavior of the
/// connections remains in-step
fn configure_connection(
conn_builder: &mut Builder<TokioExecutor>,
header_read_timeout: Duration,
opt_max_http1_headers: Option<usize>,
opt_max_http1_buf_size: Option<ByteSize>,
opt_max_http2_headers_list_bytes: Option<ByteSize>,
) -> Http1Builder<'_, TokioExecutor> {
// NOTE: this is a builder that auto-detects http1/http2, so we can configure both and rely on
// it to figure out whether we have an http1 or http2 connection
let mut builder = conn_builder.http2();
if let Some(max_header_list_size) = opt_max_http2_headers_list_bytes {
let max_header_list_size = u32::try_from(max_header_list_size.as_u64())
.inspect_err(|e| tracing::warn!("attempted to use an HTTP/2 header size limit greater than is allowed by u32 (~4.3gb): {e}"))
.unwrap_or(u32::MAX);
builder.max_header_list_size(max_header_list_size);
}

let mut builder = conn_builder.http1();

builder
.keep_alive(true)
.timer(TokioTimer::new())
.header_read_timeout(header_read_timeout);

if let Some(max_headers) = opt_max_http1_headers {
builder.max_headers(max_headers);
}

if let Some(max_buf_size) = opt_max_http1_buf_size {
let max_buf_size = usize::try_from(max_buf_size.as_u64())
.inspect_err(|e| tracing::warn!("attempted to use an HTTP/2 header size limit greater than is allowed by an unsized integer (quite large, multiple GBs, but platform-specific): {e}"))
.unwrap_or(usize::MAX);
builder.max_buf_size(max_buf_size);
}

builder
}

#[derive(Clone)]
struct IdleConnectionChecker<S> {
received_request: Arc<AtomicBool>,
Expand Down
20 changes: 15 additions & 5 deletions apollo-router/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1727,7 +1727,7 @@ fn merge_overrides(
}

// Override the listening address always since we spawn the router on a
// random port.
// random port. However, don't override Unix socket paths.
match config
.as_object_mut()
.and_then(|o| o.get_mut("supergraph"))
Expand All @@ -1744,10 +1744,20 @@ fn merge_overrides(
}
}
Some(supergraph_conf) => {
supergraph_conf.insert(
"listen".to_string(),
serde_json::Value::String(bind_addr.to_string()),
);
// check if the listen address is a Unix socket path (ie, starts with /)
let is_unix_socket = supergraph_conf
.get("listen")
.and_then(|v| v.as_str())
.map(|s| s.starts_with('/'))
.unwrap_or(false);

// only override if it's not a Unix socket
if !is_unix_socket {
supergraph_conf.insert(
"listen".to_string(),
serde_json::Value::String(bind_addr.to_string()),
);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
supergraph:
listen: 127.0.0.1:0

limits:
http1_max_request_headers: 100
http1_max_request_buf_size: "16000"
http2_max_headers_list_bytes: "20Mib"
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
supergraph:
listen: /tmp/apollo_router_test_{{RANDOM}}.sock

limits:
http1_max_request_headers: 100
http1_max_request_buf_size: "16000"
http2_max_headers_list_bytes: "20Mib"
Loading