Skip to content

Commit

Permalink
Feat: Add a feature flag to ignore connection headers
Browse files Browse the repository at this point in the history
According to the RFC, when encountering connection headers, H2 should
treat them as protocol errors. However, in reality there are servers
just setting these headers but only for informational purpose.

This feature allow the receiving end to just ignore these headers
without bailing the entire stream.
  • Loading branch information
eaufavor committed Sep 19, 2023
1 parent da38b1c commit 9dbf011
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 20 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ rust-version = "1.63"
[features]
# Enables `futures::Stream` implementations for various types.
stream = []
ignore_connection_header = []

# Enables **unstable** APIs. Any API exposed by this feature has no backwards
# compatibility guarantees. In other words, you should not use this feature for
Expand Down
19 changes: 14 additions & 5 deletions src/frame/headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -871,16 +871,25 @@ impl HeaderBlock {
match header {
Field { name, value } => {
// Connection level header fields are not supported and must
// result in a protocol error.
// result in a protocol error. However in reality there are
// servers setting these headers. Some of these headers are
// purely informational and can be simply ignored without
// affecting the integrity of the stream.

if name == header::CONNECTION
|| name == header::TRANSFER_ENCODING
if name == header::TRANSFER_ENCODING {
tracing::trace!("load_hpack; connection level header");
malformed = true;
} else if name == header::CONNECTION
|| name == header::UPGRADE
|| name == "keep-alive"
|| name == "proxy-connection"
{
tracing::trace!("load_hpack; connection level header");
malformed = true;
if cfg!(feature = "ignore_connection_header") {
tracing::trace!("load_hpack; connection level header ignored");
} else {
tracing::trace!("load_hpack; connection level header");
malformed = true;
}
} else if name == header::TE && value != "trailers" {
tracing::trace!(
"load_hpack; TE header not set to trailers; val={:?}",
Expand Down
3 changes: 3 additions & 0 deletions tests/h2-support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ version = "0.1.0"
authors = ["Carl Lerche <[email protected]>"]
edition = "2018"

[features]
ignore_connection_header = ["h2/ignore_connection_header"]

[dependencies]
h2 = { path = "../..", features = ["stream", "unstable"] }

Expand Down
3 changes: 3 additions & 0 deletions tests/h2-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ authors = ["Carl Lerche <[email protected]>"]
publish = false
edition = "2018"

[features]
ignore_connection_header = ["h2-support/ignore_connection_header"]

[dependencies]

[dev-dependencies]
Expand Down
89 changes: 74 additions & 15 deletions tests/h2-tests/tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,25 +528,84 @@ async fn recv_connection_header() {
};

let client = async move {
let settings = client.assert_server_handshake().await;
assert_default_settings!(settings);
client.send_frame(req(1, "connection", "foo")).await;
client.send_frame(req(3, "keep-alive", "5")).await;
client.send_frame(req(5, "proxy-connection", "bar")).await;
client
.send_frame(req(7, "transfer-encoding", "chunked"))
.await;
client.send_frame(req(9, "upgrade", "HTTP/2")).await;
client.recv_frame(frames::reset(1).protocol_error()).await;
client.recv_frame(frames::reset(3).protocol_error()).await;
client.recv_frame(frames::reset(5).protocol_error()).await;
client.recv_frame(frames::reset(7).protocol_error()).await;
client.recv_frame(frames::reset(9).protocol_error()).await;
if cfg!(feature = "ignore_connection_header") {
let settings = client.assert_server_handshake().await;
assert_default_settings!(settings);
client
.send_frame(
frames::headers(1)
.request("GET", "https://example.com/")
.field("connection", "foo")
.eos(),
)
.await;
client
.send_frame(
frames::headers(3)
.request("GET", "https://example.com/")
.field("keep-alive", "5")
.eos(),
)
.await;
client
.send_frame(
frames::headers(5)
.request("GET", "https://example.com/")
.field("proxy-connection", "bar")
.eos(),
)
.await;
client
.send_frame(
frames::headers(7)
.request("GET", "https://example.com/")
.field("upgrade", "HTTP/2")
.eos(),
)
.await;
client
.recv_frame(frames::headers(1).response(200).eos())
.await;
client
.recv_frame(frames::headers(3).response(200).eos())
.await;
client
.recv_frame(frames::headers(5).response(200).eos())
.await;
client
.recv_frame(frames::headers(7).response(200).eos())
.await;
} else {
let settings = client.assert_server_handshake().await;
assert_default_settings!(settings);
client.send_frame(req(1, "connection", "foo")).await;
client.send_frame(req(3, "keep-alive", "5")).await;
client.send_frame(req(5, "proxy-connection", "bar")).await;
client
.send_frame(req(7, "transfer-encoding", "chunked"))
.await;
client.send_frame(req(9, "upgrade", "HTTP/2")).await;
client.recv_frame(frames::reset(1).protocol_error()).await;
client.recv_frame(frames::reset(3).protocol_error()).await;
client.recv_frame(frames::reset(5).protocol_error()).await;
client.recv_frame(frames::reset(7).protocol_error()).await;
client.recv_frame(frames::reset(9).protocol_error()).await;
}
};

let srv = async move {
let mut srv = server::handshake(io).await.expect("handshake");
assert!(srv.next().await.is_none());
if cfg!(feature = "ignore_connection_header") {
while let Some(stream) = srv.next().await {
let (req, mut stream) = stream.unwrap();
// the headers are ignored
assert_eq!(req.headers().len(), 0);
let rsp = http::Response::builder().status(200).body(()).unwrap();
stream.send_response(rsp, true).unwrap();
}
} else {
assert!(srv.next().await.is_none());
}
};

join(client, srv).await;
Expand Down

0 comments on commit 9dbf011

Please sign in to comment.