From a00cc20afc597cb55cbc62c70b0b25b46c82a0a6 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Thu, 17 Dec 2020 02:49:21 +0300 Subject: [PATCH 1/3] fix(http1): ignore chunked trailers (#2363) Previously, hyper returned an "Invalid chunk end CR" error on chunked responses with trailers, as described in RFC 7230 Section 4.1.2. This commit adds code to ignore the trailers. Closes #2171 Co-authored-by: Alex Rebert --- src/proto/h1/decode.rs | 39 ++++++++++++++++++++++++-- tests/client.rs | 63 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/src/proto/h1/decode.rs b/src/proto/h1/decode.rs index beaf9aff7a..7612880b42 100644 --- a/src/proto/h1/decode.rs +++ b/src/proto/h1/decode.rs @@ -55,6 +55,8 @@ enum ChunkedState { Body, BodyCr, BodyLf, + Trailer, + TrailerLf, EndCr, EndLf, End, @@ -196,6 +198,8 @@ impl ChunkedState { Body => ChunkedState::read_body(cx, body, size, buf), BodyCr => ChunkedState::read_body_cr(cx, body), BodyLf => ChunkedState::read_body_lf(cx, body), + Trailer => ChunkedState::read_trailer(cx, body), + TrailerLf => ChunkedState::read_trailer_lf(cx, body), EndCr => ChunkedState::read_end_cr(cx, body), EndLf => ChunkedState::read_end_lf(cx, body), End => Poll::Ready(Ok(ChunkedState::End)), @@ -340,18 +344,38 @@ impl ChunkedState { } } - fn read_end_cr( + fn read_trailer( cx: &mut task::Context<'_>, rdr: &mut R, ) -> Poll> { + trace!("read_trailer"); match byte!(rdr, cx) { - b'\r' => Poll::Ready(Ok(ChunkedState::EndLf)), + b'\r' => Poll::Ready(Ok(ChunkedState::TrailerLf)), + _ => Poll::Ready(Ok(ChunkedState::Trailer)), + } + } + fn read_trailer_lf( + cx: &mut task::Context<'_>, + rdr: &mut R, + ) -> Poll> { + match byte!(rdr, cx) { + b'\n' => Poll::Ready(Ok(ChunkedState::EndCr)), _ => Poll::Ready(Err(io::Error::new( io::ErrorKind::InvalidInput, - "Invalid chunk end CR", + "Invalid trailer end LF", ))), } } + + fn read_end_cr( + cx: &mut task::Context<'_>, + rdr: &mut R, + ) -> Poll> { + match byte!(rdr, cx) { + b'\r' => Poll::Ready(Ok(ChunkedState::EndLf)), + _ => Poll::Ready(Ok(ChunkedState::Trailer)), + } + } fn read_end_lf( cx: &mut task::Context<'_>, rdr: &mut R, @@ -537,6 +561,15 @@ mod tests { assert_eq!("1234567890abcdef", &result); } + #[tokio::test] + async fn test_read_chunked_trailer_with_missing_lf() { + let mut mock_buf = &b"10\r\n1234567890abcdef\r\n0\r\nbad\r\r\n"[..]; + let mut decoder = Decoder::chunked(); + decoder.decode_fut(&mut mock_buf).await.expect("decode"); + let e = decoder.decode_fut(&mut mock_buf).await.unwrap_err(); + assert_eq!(e.kind(), io::ErrorKind::InvalidInput); + } + #[tokio::test] async fn test_read_chunked_after_eof() { let mut mock_buf = &b"10\r\n1234567890abcdef\r\n0\r\n\r\n"[..]; diff --git a/tests/client.rs b/tests/client.rs index 576423768f..8654d1a382 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -430,6 +430,69 @@ test! { body: None, } +test! { + name: client_get_req_body_chunked_with_trailer, + + server: + expected: "\ + GET / HTTP/1.1\r\n\ + host: {addr}\r\n\ + \r\n\ + ", + reply: "\ + HTTP/1.1 200 OK\r\n\ + Transfer-Encoding: chunked\r\n\ + \r\n\ + 5\r\n\ + hello\r\n\ + 0\r\n\ + Trailer: value\r\n\ + \r\n\ + ", + + client: + request: { + method: GET, + url: "http://{addr}/", + }, + response: + status: OK, + headers: {}, + body: &b"hello"[..], +} + +test! { + name: client_get_req_body_chunked_with_multiple_trailers, + + server: + expected: "\ + GET / HTTP/1.1\r\n\ + host: {addr}\r\n\ + \r\n\ + ", + reply: "\ + HTTP/1.1 200 OK\r\n\ + Transfer-Encoding: chunked\r\n\ + \r\n\ + 5\r\n\ + hello\r\n\ + 0\r\n\ + Trailer: value\r\n\ + another-trainer: another-value\r\n\ + \r\n\ + ", + + client: + request: { + method: GET, + url: "http://{addr}/", + }, + response: + status: OK, + headers: {}, + body: &b"hello"[..], +} + test! { name: client_get_req_body_sized, From 6d9e5f9fd1691a0befe70b5b5be3393e45cac66a Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Fri, 5 Feb 2021 13:27:30 -0800 Subject: [PATCH 2/3] fix(http1): fix server misinterpretting multiple Transfer-Encoding headers When a request arrived with multiple `Transfer-Encoding` headers, hyper would check each if they ended with `chunked`. It should have only checked if the *last* header ended with `chunked`. See https://github.com/hyperium/hyper/security/advisories/GHSA-6hfq-h8hq-87mf --- src/proto/h1/role.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/proto/h1/role.rs b/src/proto/h1/role.rs index f188efa5a5..ca8e52c30d 100644 --- a/src/proto/h1/role.rs +++ b/src/proto/h1/role.rs @@ -204,6 +204,8 @@ impl Http1Transaction for Server { if headers::is_chunked_(&value) { is_te_chunked = true; decoder = DecodedLength::CHUNKED; + } else { + is_te_chunked = false; } } header::CONTENT_LENGTH => { @@ -1334,6 +1336,16 @@ mod tests { "transfer-encoding doesn't end in chunked", ); + parse_err( + "\ + POST / HTTP/1.1\r\n\ + transfer-encoding: chunked\r\n\ + transfer-encoding: afterlol\r\n\ + \r\n\ + ", + "transfer-encoding multiple lines doesn't end in chunked", + ); + // http/1.0 assert_eq!( From 17f54264ac28b84208ccbc4119aba0573d2f7c1f Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Fri, 5 Feb 2021 14:05:42 -0800 Subject: [PATCH 3/3] v0.13.10 --- CHANGELOG.md | 8 ++++++++ Cargo.toml | 2 +- src/lib.rs | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 791012e138..043cc1d3d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +### v0.13.10 (2021-02-05) + + +#### Bug Fixes + +* **http1:** fix server misinterpretting multiple Transfer-Encoding headers ([6d9e5f9f](https://github.com/hyperium/hyper/commit/6d9e5f9fd1691a0befe70b5b5be3393e45cac66a)) + + ### v0.13.9 (2020-11-02) diff --git a/Cargo.toml b/Cargo.toml index 0034f1deb5..ac1141b3ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hyper" -version = "0.13.9" # don't forget to update html_root_url +version = "0.13.10" # don't forget to update html_root_url description = "A fast and correct HTTP library." readme = "README.md" homepage = "https://hyper.rs/" diff --git a/src/lib.rs b/src/lib.rs index 0897c25c34..054e5c8aa8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -#![doc(html_root_url = "https://docs.rs/hyper/0.13.9")] +#![doc(html_root_url = "https://docs.rs/hyper/0.13.10")] #![deny(missing_docs)] #![deny(missing_debug_implementations)] #![cfg_attr(test, deny(rust_2018_idioms))]