Skip to content

Commit

Permalink
fix(http2): allow TE "trailers" request headers
Browse files Browse the repository at this point in the history
The HTTP/2 spec allows TE headers in requests if the value is
"trailers". Other TE headers are still stripped.

Closes #1642
  • Loading branch information
okready authored and seanmonstar committed Aug 27, 2018
1 parent e3dc6c5 commit 24f11a4
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/proto/h2/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
17 changes: 15 additions & 2 deletions src/proto/h2/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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: {}",
Expand Down
2 changes: 1 addition & 1 deletion src/proto/h2/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
24 changes: 24 additions & 0 deletions tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 24f11a4

Please sign in to comment.