From 555e140057b3c53971baac08960335c21ea2a78f Mon Sep 17 00:00:00 2001 From: tottoto Date: Wed, 17 Apr 2024 11:24:35 +0900 Subject: [PATCH 1/3] refactor(proto): replace map and unwrap_or combination with map_or --- src/proto/h1/conn.rs | 6 ++---- src/proto/h2/mod.rs | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index f880f97dcb..c6b464d2d3 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -276,8 +276,7 @@ where .head .headers .get(TE) - .map(|te_header| te_header == "trailers") - .unwrap_or(false); + .map_or(false, |te_header| te_header == "trailers"); Poll::Ready(Some(Ok((msg.head, msg.decode, wants)))) } @@ -594,8 +593,7 @@ where let outgoing_is_keep_alive = head .headers .get(CONNECTION) - .map(connection_keep_alive) - .unwrap_or(false); + .map_or(false, connection_keep_alive); if !outgoing_is_keep_alive { match head.version { diff --git a/src/proto/h2/mod.rs b/src/proto/h2/mod.rs index d880502b91..201bbd1396 100644 --- a/src/proto/h2/mod.rs +++ b/src/proto/h2/mod.rs @@ -54,8 +54,7 @@ fn strip_connection_headers(headers: &mut HeaderMap, is_request: bool) { if is_request { if headers .get(TE) - .map(|te_header| te_header != "trailers") - .unwrap_or(false) + .map_or(false, |te_header| te_header != "trailers") { warn!("TE headers not set to \"trailers\" are illegal in HTTP/2 requests"); headers.remove(TE); From e2d451e27bc598a0b30b0f76acef23f6e9f724fc Mon Sep 17 00:00:00 2001 From: tottoto Date: Wed, 17 Apr 2024 11:14:01 +0900 Subject: [PATCH 2/3] refactor(h1): resolve deprecated std::usize --- src/proto/h1/io.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/proto/h1/io.rs b/src/proto/h1/io.rs index 6fc0920eb2..34eb477fb9 100644 --- a/src/proto/h1/io.rs +++ b/src/proto/h1/io.rs @@ -450,7 +450,7 @@ fn prev_power_of_two(n: usize) -> usize { // Only way this shift can underflow is if n is less than 4. // (Which would means `usize::MAX >> 64` and underflowed!) debug_assert!(n >= 4); - (::std::usize::MAX >> (n.leading_zeros() + 2)) + 1 + (usize::MAX >> (n.leading_zeros() + 2)) + 1 } impl Default for ReadStrategy { @@ -763,7 +763,7 @@ mod tests { assert_eq!(strategy.next(), 32768); // Enormous records still increment at same rate - strategy.record(::std::usize::MAX); + strategy.record(usize::MAX); assert_eq!(strategy.next(), 65536); let max = strategy.max(); @@ -833,7 +833,7 @@ mod tests { fn fuzz(max: usize) { let mut strategy = ReadStrategy::with_max(max); while strategy.next() < max { - strategy.record(::std::usize::MAX); + strategy.record(usize::MAX); } let mut next = strategy.next(); while next > 8192 { @@ -854,7 +854,7 @@ mod tests { fuzz(max); max = (max / 2).saturating_mul(3); } - fuzz(::std::usize::MAX); + fuzz(usize::MAX); } #[test] From 226305d0fc78ab780aa5a1084e013a3b0a39e4d8 Mon Sep 17 00:00:00 2001 From: "Herman J. Radtke III" Date: Sat, 20 Apr 2024 08:23:23 -0400 Subject: [PATCH 3/3] fix(http1): improve debug messages when sending trailers When the "trailer" header was not found in the response, the debug message would say "attempted to encode trailers for non-chunked response". This was quite misleading as the response was chunked. We now include a better debug message that hints to the user that the "trailer" header was not specified. When a chunked response contained a trailer header that did not match the header names specified in the "trailer" response header, there was no debug message to the user. We now include debug messages that tell the user if the header name is not allowed and if the header name was not specified in the "trailer" response header. --- src/proto/h1/encode.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/proto/h1/encode.rs b/src/proto/h1/encode.rs index fe582901ea..ff0a03a3dd 100644 --- a/src/proto/h1/encode.rs +++ b/src/proto/h1/encode.rs @@ -165,6 +165,7 @@ impl Encoder { trailers: HeaderMap, title_case_headers: bool, ) -> Option> { + trace!("encoding trailers"); match &self.kind { Kind::Chunked(Some(ref allowed_trailer_fields)) => { let allowed_trailer_field_map = allowed_trailer_field_map(&allowed_trailer_fields); @@ -178,10 +179,14 @@ impl Encoder { } let name = cur_name.as_ref().expect("current header name"); - if allowed_trailer_field_map.contains_key(name.as_str()) - && is_valid_trailer_field(name) - { - allowed_trailers.insert(name, value); + if allowed_trailer_field_map.contains_key(name.as_str()) { + if is_valid_trailer_field(name) { + allowed_trailers.insert(name, value); + } else { + debug!("trailer field is not valid: {}", &name); + } + } else { + debug!("trailer header name not found in trailer header: {}", &name); } } @@ -200,6 +205,10 @@ impl Encoder { kind: BufKind::Trailers(b"0\r\n".chain(Bytes::from(buf)).chain(b"\r\n")), }) } + Kind::Chunked(None) => { + debug!("attempted to encode trailers, but the trailer header is not set"); + None + } _ => { debug!("attempted to encode trailers for non-chunked response"); None