Skip to content

Commit 86ea93f

Browse files
eaufavoreaufavor
committed
Feat: Add a feature flag to ignore connection headers
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.
1 parent 633116e commit 86ea93f

File tree

5 files changed

+95
-20
lines changed

5 files changed

+95
-20
lines changed

‎Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ rust-version = "1.56"
2424
[features]
2525
# Enables `futures::Stream` implementations for various types.
2626
stream = []
27+
ignore_connection_header = []
2728

2829
# Enables **unstable** APIs. Any API exposed by this feature has no backwards
2930
# compatibility guarantees. In other words, you should not use this feature for

‎src/frame/headers.rs

+14-5
Original file line numberDiff line numberDiff line change
@@ -871,16 +871,25 @@ impl HeaderBlock {
871871
match header {
872872
Field { name, value } => {
873873
// Connection level header fields are not supported and must
874-
// result in a protocol error.
874+
// result in a protocol error. However in reality there are
875+
// servers setting these headers. Some of these headers are
876+
// purely informational and can be simply ignored without
877+
// affecting the integrity of the stream.
875878

876-
if name == header::CONNECTION
877-
|| name == header::TRANSFER_ENCODING
879+
if name == header::TRANSFER_ENCODING {
880+
tracing::trace!("load_hpack; connection level header");
881+
malformed = true;
882+
} else if name == header::CONNECTION
878883
|| name == header::UPGRADE
879884
|| name == "keep-alive"
880885
|| name == "proxy-connection"
881886
{
882-
tracing::trace!("load_hpack; connection level header");
883-
malformed = true;
887+
if cfg!(feature = "ignore_connection_header") {
888+
tracing::trace!("load_hpack; connection level header ignored");
889+
} else {
890+
tracing::trace!("load_hpack; connection level header");
891+
malformed = true;
892+
}
884893
} else if name == header::TE && value != "trailers" {
885894
tracing::trace!(
886895
"load_hpack; TE header not set to trailers; val={:?}",

‎tests/h2-support/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ version = "0.1.0"
44
authors = ["Carl Lerche <[email protected]>"]
55
edition = "2018"
66

7+
[features]
8+
ignore_connection_header = ["h2/ignore_connection_header"]
9+
710
[dependencies]
811
h2 = { path = "../..", features = ["stream", "unstable"] }
912

‎tests/h2-tests/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ authors = ["Carl Lerche <[email protected]>"]
55
publish = false
66
edition = "2018"
77

8+
[features]
9+
ignore_connection_header = ["h2-support/ignore_connection_header"]
10+
811
[dependencies]
912

1013
[dev-dependencies]

‎tests/h2-tests/tests/server.rs

+74-15
Original file line numberDiff line numberDiff line change
@@ -528,25 +528,84 @@ async fn recv_connection_header() {
528528
};
529529

530530
let client = async move {
531-
let settings = client.assert_server_handshake().await;
532-
assert_default_settings!(settings);
533-
client.send_frame(req(1, "connection", "foo")).await;
534-
client.send_frame(req(3, "keep-alive", "5")).await;
535-
client.send_frame(req(5, "proxy-connection", "bar")).await;
536-
client
537-
.send_frame(req(7, "transfer-encoding", "chunked"))
538-
.await;
539-
client.send_frame(req(9, "upgrade", "HTTP/2")).await;
540-
client.recv_frame(frames::reset(1).protocol_error()).await;
541-
client.recv_frame(frames::reset(3).protocol_error()).await;
542-
client.recv_frame(frames::reset(5).protocol_error()).await;
543-
client.recv_frame(frames::reset(7).protocol_error()).await;
544-
client.recv_frame(frames::reset(9).protocol_error()).await;
531+
if cfg!(feature = "ignore_connection_header") {
532+
let settings = client.assert_server_handshake().await;
533+
assert_default_settings!(settings);
534+
client
535+
.send_frame(
536+
frames::headers(1)
537+
.request("GET", "https://example.com/")
538+
.field("connection", "foo")
539+
.eos(),
540+
)
541+
.await;
542+
client
543+
.send_frame(
544+
frames::headers(3)
545+
.request("GET", "https://example.com/")
546+
.field("keep-alive", "5")
547+
.eos(),
548+
)
549+
.await;
550+
client
551+
.send_frame(
552+
frames::headers(5)
553+
.request("GET", "https://example.com/")
554+
.field("proxy-connection", "bar")
555+
.eos(),
556+
)
557+
.await;
558+
client
559+
.send_frame(
560+
frames::headers(7)
561+
.request("GET", "https://example.com/")
562+
.field("upgrade", "HTTP/2")
563+
.eos(),
564+
)
565+
.await;
566+
client
567+
.recv_frame(frames::headers(1).response(200).eos())
568+
.await;
569+
client
570+
.recv_frame(frames::headers(3).response(200).eos())
571+
.await;
572+
client
573+
.recv_frame(frames::headers(5).response(200).eos())
574+
.await;
575+
client
576+
.recv_frame(frames::headers(7).response(200).eos())
577+
.await;
578+
} else {
579+
let settings = client.assert_server_handshake().await;
580+
assert_default_settings!(settings);
581+
client.send_frame(req(1, "connection", "foo")).await;
582+
client.send_frame(req(3, "keep-alive", "5")).await;
583+
client.send_frame(req(5, "proxy-connection", "bar")).await;
584+
client
585+
.send_frame(req(7, "transfer-encoding", "chunked"))
586+
.await;
587+
client.send_frame(req(9, "upgrade", "HTTP/2")).await;
588+
client.recv_frame(frames::reset(1).protocol_error()).await;
589+
client.recv_frame(frames::reset(3).protocol_error()).await;
590+
client.recv_frame(frames::reset(5).protocol_error()).await;
591+
client.recv_frame(frames::reset(7).protocol_error()).await;
592+
client.recv_frame(frames::reset(9).protocol_error()).await;
593+
}
545594
};
546595

547596
let srv = async move {
548597
let mut srv = server::handshake(io).await.expect("handshake");
549-
assert!(srv.next().await.is_none());
598+
if cfg!(feature = "ignore_connection_header") {
599+
while let Some(stream) = srv.next().await {
600+
let (req, mut stream) = stream.unwrap();
601+
// the headers are ignored
602+
assert_eq!(req.headers().len(), 0);
603+
let rsp = http::Response::builder().status(200).body(()).unwrap();
604+
stream.send_response(rsp, true).unwrap();
605+
}
606+
} else {
607+
assert!(srv.next().await.is_none());
608+
}
550609
};
551610

552611
join(client, srv).await;

0 commit comments

Comments
 (0)