From 24f11a421d8422714bf023a602d7718b885a39a0 Mon Sep 17 00:00:00 2001 From: Theodore Cipicchio Date: Sat, 25 Aug 2018 00:55:53 -0700 Subject: [PATCH] fix(http2): allow TE "trailers" request headers The HTTP/2 spec allows TE headers in requests if the value is "trailers". Other TE headers are still stripped. Closes #1642 --- src/proto/h2/client.rs | 2 +- src/proto/h2/mod.rs | 17 +++++++++++++++-- src/proto/h2/server.rs | 2 +- tests/integration.rs | 24 ++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/proto/h2/client.rs b/src/proto/h2/client.rs index 1570d2ee0a..b285574f47 100644 --- a/src/proto/h2/client.rs +++ b/src/proto/h2/client.rs @@ -108,7 +108,7 @@ where } let (head, body) = req.into_parts(); let mut req = ::http::Request::from_parts(head, ()); - super::strip_connection_headers(req.headers_mut()); + super::strip_connection_headers(req.headers_mut(), true); if let Some(len) = body.content_length() { headers::set_content_length_if_missing(req.headers_mut(), len); } diff --git a/src/proto/h2/mod.rs b/src/proto/h2/mod.rs index de877f0bc7..6620877210 100644 --- a/src/proto/h2/mod.rs +++ b/src/proto/h2/mod.rs @@ -15,15 +15,17 @@ mod server; pub(crate) use self::client::Client; pub(crate) use self::server::Server; -fn strip_connection_headers(headers: &mut HeaderMap) { +fn strip_connection_headers(headers: &mut HeaderMap, is_request: bool) { // List of connection headers from: // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Connection + // + // TE headers are allowed in HTTP/2 requests as long as the value is "trailers", so they're + // tested separately. let connection_headers = [ HeaderName::from_lowercase(b"keep-alive").unwrap(), HeaderName::from_lowercase(b"proxy-connection").unwrap(), PROXY_AUTHENTICATE, PROXY_AUTHORIZATION, - TE, TRAILER, TRANSFER_ENCODING, UPGRADE, @@ -35,6 +37,17 @@ fn strip_connection_headers(headers: &mut HeaderMap) { } } + if is_request { + if headers.get(TE).map(|te_header| te_header != "trailers").unwrap_or(false) { + warn!("TE headers not set to \"trailers\" are illegal in HTTP/2 requests"); + headers.remove(TE); + } + } else { + if headers.remove(TE).is_some() { + warn!("TE headers illegal in HTTP/2 responses"); + } + } + if let Some(header) = headers.remove(CONNECTION) { warn!( "Connection header illegal in HTTP/2: {}", diff --git a/src/proto/h2/server.rs b/src/proto/h2/server.rs index d78d4a254e..1ded63b33e 100644 --- a/src/proto/h2/server.rs +++ b/src/proto/h2/server.rs @@ -192,7 +192,7 @@ where let (head, body) = res.into_parts(); let mut res = ::http::Response::from_parts(head, ()); - super::strip_connection_headers(res.headers_mut()); + super::strip_connection_headers(res.headers_mut(), false); if let Some(len) = body.content_length() { headers::set_content_length_if_missing(res.headers_mut(), len); } diff --git a/tests/integration.rs b/tests/integration.rs index e593284bba..9e7450b685 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -184,6 +184,30 @@ t! { ; } +t! { + get_allow_te_trailers_header, + client: + request: + uri: "/", + headers: { + // http2 strips connection headers other than TE "trailers" + "te" => "trailers", + }, + ; + response: + status: 200, + ; + server: + request: + uri: "/", + headers: { + "te" => "trailers", + }, + ; + response: + ; +} + t! { get_body_chunked, client: