From 87bee5e8bab34e113e98157b9300c475d6d75496 Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Tue, 19 Mar 2024 23:26:39 +0330 Subject: [PATCH 01/12] assert compatibility between LengthDelimitedCodec options --- tokio-util/src/codec/length_delimited.rs | 64 ++++++++++++ tokio-util/tests/length_delimited.rs | 121 +++++++++++++++++++++++ 2 files changed, 185 insertions(+) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index a182dcaec0c..57da4f775bf 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -921,6 +921,10 @@ impl Builder { /// Create a configured length delimited `LengthDelimitedCodec` /// + /// # Panics + /// This method panics if the combination of `length_field_len`, `length_adjustment`, + /// and `max_frame_len` are incompatible with each other. + /// /// # Examples /// /// ``` @@ -931,10 +935,13 @@ impl Builder { /// .length_field_type::() /// .length_adjustment(0) /// .num_skip(0) + /// .max_frame_length(65535) /// .new_codec(); /// # } /// ``` pub fn new_codec(&self) -> LengthDelimitedCodec { + self.assert_compatiblity(); + LengthDelimitedCodec { builder: *self, state: DecodeState::Head, @@ -943,6 +950,10 @@ impl Builder { /// Create a configured length delimited `FramedRead` /// + /// # Panics + /// This method panics if the combination of `length_field_len`, `length_adjustment`, + /// and `max_frame_len` are incompatible with each other. + /// /// # Examples /// /// ``` @@ -955,6 +966,7 @@ impl Builder { /// .length_field_type::() /// .length_adjustment(0) /// .num_skip(0) + /// .max_frame_length(65535) /// .new_read(io); /// # } /// # pub fn main() {} @@ -963,11 +975,17 @@ impl Builder { where T: AsyncRead, { + self.assert_compatiblity(); + FramedRead::new(upstream, self.new_codec()) } /// Create a configured length delimited `FramedWrite` /// + /// # Panics + /// This method panics if the combination of `length_field_len`, `length_adjustment`, + /// and `max_frame_len` are incompatible with each other. + /// /// # Examples /// /// ``` @@ -976,6 +994,7 @@ impl Builder { /// # fn write_frame(io: T) { /// LengthDelimitedCodec::builder() /// .length_field_type::() + /// .max_frame_length(65535) /// .new_write(io); /// # } /// # pub fn main() {} @@ -984,11 +1003,17 @@ impl Builder { where T: AsyncWrite, { + self.assert_compatiblity(); + FramedWrite::new(inner, self.new_codec()) } /// Create a configured length delimited `Framed` /// + /// # Panics + /// This method panics if the combination of `length_field_len`, `length_adjustment`, + /// and `max_frame_len` are incompatible with each other. + /// /// # Examples /// /// ``` @@ -998,6 +1023,7 @@ impl Builder { /// # let _ = /// LengthDelimitedCodec::builder() /// .length_field_type::() + /// .max_frame_length(65535) /// .new_framed(io); /// # } /// # pub fn main() {} @@ -1006,6 +1032,8 @@ impl Builder { where T: AsyncRead + AsyncWrite, { + self.assert_compatiblity(); + Framed::new(inner, self.new_codec()) } @@ -1018,6 +1046,42 @@ impl Builder { self.num_skip .unwrap_or(self.length_field_offset + self.length_field_len) } + + // Panics if the combination of `length_field_len`, `length_adjustment`, and `max_frame_len` + // are incompatible with each other. + fn assert_compatiblity(&self) { + // This function is basically `std::usize::saturating_add_signed`. Since it + // requires MSRV 1.66, its implementation is copied here. + // + // TODO: use the method from std when MSRV becomes >= 1.66 + fn saturating_add_signed(num: usize, rhs: isize) -> usize { + let (res, overflow) = num.overflowing_add(rhs as usize); + if overflow == (rhs < 0) { + res + } else if overflow { + usize::MAX + } else { + 0 + } + } + + // The smallest number that cannot be represneted using `length_field_len` bytes. + // For example for one byte, `max_number` would be 256. + let max_number = 1 << (8 * self.length_field_len); + + // From the maximum length of bytes that `max_number` can represent, some could be taken away if + // `length_adjustment` is set. For example, if `length_field_len` is one byte, but its value also + // contains the length of the header, then `length_adjustment` would be one byte which consequently + // would make the allowed `max_frame_len`, 254 instead of 255. + let ajusted_max_number = saturating_add_signed(max_number, self.length_adjustment); + + if self.max_frame_len >= ajusted_max_number { + panic!( + "max frame length exceeds the possible amount: {} >= {}", + self.max_frame_len, ajusted_max_number + ) + } + } } impl Default for Builder { diff --git a/tokio-util/tests/length_delimited.rs b/tokio-util/tests/length_delimited.rs index ed5590f9644..a490eb056d2 100644 --- a/tokio-util/tests/length_delimited.rs +++ b/tokio-util/tests/length_delimited.rs @@ -325,6 +325,7 @@ fn read_update_max_frame_len_in_flight() { fn read_one_byte_length_field() { let io = length_delimited::Builder::new() .length_field_length(1) + .max_frame_length(255) .new_read(mock! { data(b"\x09abcdefghi"), }); @@ -339,6 +340,7 @@ fn read_header_offset() { let io = length_delimited::Builder::new() .length_field_length(2) .length_field_offset(4) + .max_frame_length(65535) .new_read(mock! { data(b"zzzz\x00\x09abcdefghi"), }); @@ -360,6 +362,7 @@ fn read_single_multi_frame_one_packet_skip_none_adjusted() { .length_field_offset(2) .num_skip(0) .length_adjustment(4) + .max_frame_length(65539) .new_read(mock! { data(&d), }); @@ -400,6 +403,7 @@ fn read_single_multi_frame_one_packet_length_includes_head() { let io = length_delimited::Builder::new() .length_field_length(2) .length_adjustment(-2) + .max_frame_length(65533) .new_read(mock! { data(&d), }); @@ -579,6 +583,7 @@ fn write_single_frame_little_endian() { fn write_single_frame_with_short_length_field() { let io = length_delimited::Builder::new() .length_field_length(1) + .max_frame_length(255) .new_write(mock! { data(b"\x09"), data(b"abcdefghi"), @@ -689,6 +694,122 @@ fn encode_overflow() { codec.encode(Bytes::from("hello"), &mut buf).unwrap(); } +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 256 >= 256")] +fn new_codec_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .max_frame_length(256) + .new_codec(); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 255 >= 255")] +fn new_codec_neg_adjusted_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .length_adjustment(-1) + .max_frame_length(255) + .new_codec(); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 257 >= 257")] +fn new_codec_pos_adjusted_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .length_adjustment(1) + .max_frame_length(257) + .new_codec(); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 256 >= 256")] +fn new_framed_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .max_frame_length(256) + .new_framed(mock!()); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 255 >= 255")] +fn new_framed_neg_adjusted_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .length_adjustment(-1) + .max_frame_length(255) + .new_framed(mock!()); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 257 >= 257")] +fn new_framed_pos_adjusted_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .length_adjustment(1) + .max_frame_length(257) + .new_framed(mock!()); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 256 >= 256")] +fn new_read_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .max_frame_length(256) + .new_read(mock!()); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 255 >= 255")] +fn new_read_neg_adjusted_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .length_adjustment(-1) + .max_frame_length(255) + .new_read(mock!()); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 257 >= 257")] +fn new_read_pos_adjusted_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .length_adjustment(1) + .max_frame_length(257) + .new_read(mock!()); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 256 >= 256")] +fn new_write_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .max_frame_length(256) + .new_write(mock!()); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 255 >= 255")] +fn new_write_neg_adjusted_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .length_adjustment(-1) + .max_frame_length(255) + .new_write(mock!()); +} + +#[test] +#[should_panic(expected = "max frame length exceeds the possible amount: 257 >= 257")] +fn new_write_pos_adjusted_frame_does_not_fit() { + LengthDelimitedCodec::builder() + .length_field_length(1) + .length_adjustment(1) + .max_frame_length(257) + .new_write(mock!()); +} + // ===== Test utils ===== struct Mock { From 3ce97f71058b622976383dbc4aad38973bc427fa Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Wed, 20 Mar 2024 00:42:28 +0330 Subject: [PATCH 02/12] fix issue with overflow --- tokio-util/src/codec/length_delimited.rs | 19 +---- tokio-util/tests/length_delimited.rs | 99 +++--------------------- 2 files changed, 14 insertions(+), 104 deletions(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index 57da4f775bf..b1210450fc5 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -975,8 +975,6 @@ impl Builder { where T: AsyncRead, { - self.assert_compatiblity(); - FramedRead::new(upstream, self.new_codec()) } @@ -1003,8 +1001,6 @@ impl Builder { where T: AsyncWrite, { - self.assert_compatiblity(); - FramedWrite::new(inner, self.new_codec()) } @@ -1032,8 +1028,6 @@ impl Builder { where T: AsyncRead + AsyncWrite, { - self.assert_compatiblity(); - Framed::new(inner, self.new_codec()) } @@ -1065,19 +1059,14 @@ impl Builder { } } - // The smallest number that cannot be represneted using `length_field_len` bytes. - // For example for one byte, `max_number` would be 256. - let max_number = 1 << (8 * self.length_field_len); + let max_number = 2usize.saturating_pow((8 * self.length_field_len - 1) as u32); + let max_number = max_number + (max_number - 1); - // From the maximum length of bytes that `max_number` can represent, some could be taken away if - // `length_adjustment` is set. For example, if `length_field_len` is one byte, but its value also - // contains the length of the header, then `length_adjustment` would be one byte which consequently - // would make the allowed `max_frame_len`, 254 instead of 255. let ajusted_max_number = saturating_add_signed(max_number, self.length_adjustment); - if self.max_frame_len >= ajusted_max_number { + if self.max_frame_len > ajusted_max_number { panic!( - "max frame length exceeds the possible amount: {} >= {}", + "max frame length exceeds the possible amount: {} > {}", self.max_frame_len, ajusted_max_number ) } diff --git a/tokio-util/tests/length_delimited.rs b/tokio-util/tests/length_delimited.rs index a490eb056d2..3d51fe5033a 100644 --- a/tokio-util/tests/length_delimited.rs +++ b/tokio-util/tests/length_delimited.rs @@ -695,8 +695,8 @@ fn encode_overflow() { } #[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 256 >= 256")] -fn new_codec_frame_does_not_fit() { +#[should_panic(expected = "max frame length exceeds the possible amount: 256 > 255")] +fn frame_does_not_fit() { LengthDelimitedCodec::builder() .length_field_length(1) .max_frame_length(256) @@ -704,8 +704,8 @@ fn new_codec_frame_does_not_fit() { } #[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 255 >= 255")] -fn new_codec_neg_adjusted_frame_does_not_fit() { +#[should_panic(expected = "max frame length exceeds the possible amount: 255 > 254")] +fn neg_adjusted_frame_does_not_fit() { LengthDelimitedCodec::builder() .length_field_length(1) .length_adjustment(-1) @@ -714,8 +714,8 @@ fn new_codec_neg_adjusted_frame_does_not_fit() { } #[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 257 >= 257")] -fn new_codec_pos_adjusted_frame_does_not_fit() { +#[should_panic(expected = "max frame length exceeds the possible amount: 257 > 256")] +fn pos_adjusted_frame_does_not_fit() { LengthDelimitedCodec::builder() .length_field_length(1) .length_adjustment(1) @@ -724,90 +724,11 @@ fn new_codec_pos_adjusted_frame_does_not_fit() { } #[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 256 >= 256")] -fn new_framed_frame_does_not_fit() { +fn max_allowed_frame_fits() { LengthDelimitedCodec::builder() - .length_field_length(1) - .max_frame_length(256) - .new_framed(mock!()); -} - -#[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 255 >= 255")] -fn new_framed_neg_adjusted_frame_does_not_fit() { - LengthDelimitedCodec::builder() - .length_field_length(1) - .length_adjustment(-1) - .max_frame_length(255) - .new_framed(mock!()); -} - -#[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 257 >= 257")] -fn new_framed_pos_adjusted_frame_does_not_fit() { - LengthDelimitedCodec::builder() - .length_field_length(1) - .length_adjustment(1) - .max_frame_length(257) - .new_framed(mock!()); -} - -#[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 256 >= 256")] -fn new_read_frame_does_not_fit() { - LengthDelimitedCodec::builder() - .length_field_length(1) - .max_frame_length(256) - .new_read(mock!()); -} - -#[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 255 >= 255")] -fn new_read_neg_adjusted_frame_does_not_fit() { - LengthDelimitedCodec::builder() - .length_field_length(1) - .length_adjustment(-1) - .max_frame_length(255) - .new_read(mock!()); -} - -#[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 257 >= 257")] -fn new_read_pos_adjusted_frame_does_not_fit() { - LengthDelimitedCodec::builder() - .length_field_length(1) - .length_adjustment(1) - .max_frame_length(257) - .new_read(mock!()); -} - -#[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 256 >= 256")] -fn new_write_frame_does_not_fit() { - LengthDelimitedCodec::builder() - .length_field_length(1) - .max_frame_length(256) - .new_write(mock!()); -} - -#[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 255 >= 255")] -fn new_write_neg_adjusted_frame_does_not_fit() { - LengthDelimitedCodec::builder() - .length_field_length(1) - .length_adjustment(-1) - .max_frame_length(255) - .new_write(mock!()); -} - -#[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 257 >= 257")] -fn new_write_pos_adjusted_frame_does_not_fit() { - LengthDelimitedCodec::builder() - .length_field_length(1) - .length_adjustment(1) - .max_frame_length(257) - .new_write(mock!()); + .length_field_length(8) + .max_frame_length(usize::MAX) + .new_codec(); } // ===== Test utils ===== From 4155c8f632ef2c949524763230f81561c88b933b Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Wed, 20 Mar 2024 01:42:53 +0330 Subject: [PATCH 03/12] add explanation, helper, and fix wasi --- tokio-util/src/codec/length_delimited.rs | 67 ++++++++++++++++++------ tokio-util/tests/length_delimited.rs | 8 +-- 2 files changed, 55 insertions(+), 20 deletions(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index b1210450fc5..2a70ef1e0df 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -386,6 +386,27 @@ use std::{cmp, fmt, mem}; /// `Builder` enables constructing configured length delimited codecs. Note /// that not all configuration settings apply to both encoding and decoding. See /// the documentation for specific methods for more detail. +/// +/// # Configuration Compatibility +/// Not all combinations of [`max_frame_length`], [`length_field_length`], and +/// [`length_adjustment`] is valid. For a combination of these values to be valid, +/// the following must be true: +/// ```text +/// max_frame_length < 2^(length_field_length * 8) + length_adjustment +/// ``` +/// For example in the simplest case where `length_adjustment` is `0` and `length_field_length` +/// is one byte, the smallest number that cannot be represented by it is 256. So `max_frame_length` +/// must be smaller than 256 for the configuration to be valid. +/// +/// `length_adjustment` can offset the maximum allowed value. For example if `length_adjustment` +/// is `-1`, this means the length of the payload is going to be reported `1` byte more than its +/// true value. So a payload of `255` bytes will be framed with a length of `256`. But `256` cannot +/// be stored in a single byte `length_field_length`. So the previosly valid `max_frame_length` of +/// `255` is now invalid. The case for a positive `length_adjustment` can be infered accordingly. +/// +/// [`max_frame_length`]: Builder::max_frame_length +/// [`length_field_length`]: Builder::length_field_length +/// [`length_adjustment`]: Builder::length_adjustment #[derive(Debug, Clone, Copy)] pub struct Builder { // Maximum frame length @@ -1031,19 +1052,13 @@ impl Builder { Framed::new(inner, self.new_codec()) } - fn num_head_bytes(&self) -> usize { - let num = self.length_field_offset + self.length_field_len; - cmp::max(num, self.num_skip.unwrap_or(0)) - } - - fn get_num_skip(&self) -> usize { - self.num_skip - .unwrap_or(self.length_field_offset + self.length_field_len) - } - - // Panics if the combination of `length_field_len`, `length_adjustment`, and `max_frame_len` - // are incompatible with each other. - fn assert_compatiblity(&self) { + /// Returns the maximum frame length that is allowed based on the values configured + /// by [`length_field_length`] and [`length_adjustment`]. For info about how it is + /// computed, refer to the [`Builder`] documentation. + /// + /// [`length_field_length`]: Builder::length_field_length + /// [`length_adjustment`]: Builder::length_adjustment + pub fn max_allowed_frame_length(&self) -> usize { // This function is basically `std::usize::saturating_add_signed`. Since it // requires MSRV 1.66, its implementation is copied here. // @@ -1059,15 +1074,35 @@ impl Builder { } } + // Calculate the maximum number that can be represented using `length_field_len` bytes. let max_number = 2usize.saturating_pow((8 * self.length_field_len - 1) as u32); + // In order to prevent an overflow, we do the last part manually. let max_number = max_number + (max_number - 1); let ajusted_max_number = saturating_add_signed(max_number, self.length_adjustment); - if self.max_frame_len > ajusted_max_number { + ajusted_max_number + } + + fn num_head_bytes(&self) -> usize { + let num = self.length_field_offset + self.length_field_len; + cmp::max(num, self.num_skip.unwrap_or(0)) + } + + fn get_num_skip(&self) -> usize { + self.num_skip + .unwrap_or(self.length_field_offset + self.length_field_len) + } + + // Panics if the combination of `length_field_len`, `length_adjustment`, and `max_frame_len` + // are incompatible with each other. + fn assert_compatiblity(&self) { + let max_allowed_frame_length = self.max_allowed_frame_length(); + + if self.max_frame_len > max_allowed_frame_length { panic!( - "max frame length exceeds the possible amount: {} > {}", - self.max_frame_len, ajusted_max_number + "max frame length exceeds the allowed amount: {} > {}", + self.max_frame_len, max_allowed_frame_length ) } } diff --git a/tokio-util/tests/length_delimited.rs b/tokio-util/tests/length_delimited.rs index 3d51fe5033a..11f18739e78 100644 --- a/tokio-util/tests/length_delimited.rs +++ b/tokio-util/tests/length_delimited.rs @@ -695,7 +695,7 @@ fn encode_overflow() { } #[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 256 > 255")] +#[should_panic(expected = "max frame length exceeds the allowed amount: 256 > 255")] fn frame_does_not_fit() { LengthDelimitedCodec::builder() .length_field_length(1) @@ -704,7 +704,7 @@ fn frame_does_not_fit() { } #[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 255 > 254")] +#[should_panic(expected = "max frame length exceeds the allowed amount: 255 > 254")] fn neg_adjusted_frame_does_not_fit() { LengthDelimitedCodec::builder() .length_field_length(1) @@ -714,7 +714,7 @@ fn neg_adjusted_frame_does_not_fit() { } #[test] -#[should_panic(expected = "max frame length exceeds the possible amount: 257 > 256")] +#[should_panic(expected = "max frame length exceeds the allowed amount: 257 > 256")] fn pos_adjusted_frame_does_not_fit() { LengthDelimitedCodec::builder() .length_field_length(1) @@ -726,7 +726,7 @@ fn pos_adjusted_frame_does_not_fit() { #[test] fn max_allowed_frame_fits() { LengthDelimitedCodec::builder() - .length_field_length(8) + .length_field_length(0usize.to_ne_bytes().len()) .max_frame_length(usize::MAX) .new_codec(); } From 37de038924e33725d956f2ac57715d41e94c421a Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Wed, 20 Mar 2024 01:45:28 +0330 Subject: [PATCH 04/12] fix clippy --- tokio-util/src/codec/length_delimited.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index 2a70ef1e0df..52055b016f6 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -1079,9 +1079,7 @@ impl Builder { // In order to prevent an overflow, we do the last part manually. let max_number = max_number + (max_number - 1); - let ajusted_max_number = saturating_add_signed(max_number, self.length_adjustment); - - ajusted_max_number + saturating_add_signed(max_number, self.length_adjustment) } fn num_head_bytes(&self) -> usize { From 5334253248517aea0ba9d3dd480c747d5a008a56 Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Wed, 20 Mar 2024 01:52:00 +0330 Subject: [PATCH 05/12] fix spellcheck --- tokio-util/src/codec/length_delimited.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index 52055b016f6..de53a1a0658 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -401,8 +401,8 @@ use std::{cmp, fmt, mem}; /// `length_adjustment` can offset the maximum allowed value. For example if `length_adjustment` /// is `-1`, this means the length of the payload is going to be reported `1` byte more than its /// true value. So a payload of `255` bytes will be framed with a length of `256`. But `256` cannot -/// be stored in a single byte `length_field_length`. So the previosly valid `max_frame_length` of -/// `255` is now invalid. The case for a positive `length_adjustment` can be infered accordingly. +/// be stored in a single byte `length_field_length`. So the previously valid `max_frame_length` of +/// `255` is now invalid. The case for a positive `length_adjustment` can be inferred accordingly. /// /// [`max_frame_length`]: Builder::max_frame_length /// [`length_field_length`]: Builder::length_field_length From 311d9007ca68643170b0cb71794844d1101ec9dc Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Wed, 20 Mar 2024 02:12:47 +0330 Subject: [PATCH 06/12] fix examples --- tokio-util/src/codec/length_delimited.rs | 8 ++++++++ tokio-util/tests/length_delimited.rs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index de53a1a0658..e4cefd1f1d4 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -87,6 +87,7 @@ //! .length_field_type::() //! .length_adjustment(0) // default value //! .num_skip(0) // Do not strip frame header +//! .max_frame_length(255) //! .new_read(io); //! # } //! # pub fn main() {} @@ -120,6 +121,7 @@ //! .length_field_offset(0) // default value //! .length_field_type::() //! .length_adjustment(0) // default value +//! .max_frame_length(255) //! // `num_skip` is not needed, the default is to skip //! .new_read(io); //! # } @@ -153,6 +155,7 @@ //! .length_field_type::() //! .length_adjustment(-2) // size of head //! .num_skip(0) +//! .max_frame_length(253) //! .new_read(io); //! # } //! # pub fn main() {} @@ -187,6 +190,7 @@ //! .length_field_length(3) //! .length_adjustment(2) // remaining head //! .num_skip(0) +//! .max_frame_length(257) //! .new_read(io); //! # } //! # pub fn main() {} @@ -231,6 +235,7 @@ //! .length_field_type::() //! .length_adjustment(1) // length of hdr2 //! .num_skip(3) // length of hdr1 + LEN +//! .max_frame_length(256) //! .new_read(io); //! # } //! # pub fn main() {} @@ -277,6 +282,7 @@ //! .length_field_type::() //! .length_adjustment(-3) // length of hdr1 + LEN, negative //! .num_skip(3) +//! .max_frame_length(252) //! .new_read(io); //! # } //! ``` @@ -316,6 +322,7 @@ //! .length_field_length(3) //! .length_adjustment(0) // default value //! .num_skip(4) // skip the first 4 bytes +//! .max_frame_length(255) //! .new_read(io); //! # } //! # pub fn main() {} @@ -351,6 +358,7 @@ //! # let _ = //! LengthDelimitedCodec::builder() //! .length_field_type::() +//! .max_frame_length(255) //! .new_write(io); //! # } //! # pub fn main() {} diff --git a/tokio-util/tests/length_delimited.rs b/tokio-util/tests/length_delimited.rs index 11f18739e78..de7f56c54e6 100644 --- a/tokio-util/tests/length_delimited.rs +++ b/tokio-util/tests/length_delimited.rs @@ -726,7 +726,7 @@ fn pos_adjusted_frame_does_not_fit() { #[test] fn max_allowed_frame_fits() { LengthDelimitedCodec::builder() - .length_field_length(0usize.to_ne_bytes().len()) + .length_field_length(std::mem::size_of::()) .max_frame_length(usize::MAX) .new_codec(); } From 5958b07354638bf513601f7b1a4717d70fcf4d30 Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Thu, 21 Mar 2024 01:23:54 +0330 Subject: [PATCH 07/12] adjust max_frame_len instead of panic --- tokio-util/src/codec/length_delimited.rs | 100 +++++++++-------------- tokio-util/tests/length_delimited.rs | 36 ++++---- 2 files changed, 59 insertions(+), 77 deletions(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index e4cefd1f1d4..e95c48707db 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -87,7 +87,6 @@ //! .length_field_type::() //! .length_adjustment(0) // default value //! .num_skip(0) // Do not strip frame header -//! .max_frame_length(255) //! .new_read(io); //! # } //! # pub fn main() {} @@ -121,7 +120,6 @@ //! .length_field_offset(0) // default value //! .length_field_type::() //! .length_adjustment(0) // default value -//! .max_frame_length(255) //! // `num_skip` is not needed, the default is to skip //! .new_read(io); //! # } @@ -155,7 +153,6 @@ //! .length_field_type::() //! .length_adjustment(-2) // size of head //! .num_skip(0) -//! .max_frame_length(253) //! .new_read(io); //! # } //! # pub fn main() {} @@ -190,7 +187,6 @@ //! .length_field_length(3) //! .length_adjustment(2) // remaining head //! .num_skip(0) -//! .max_frame_length(257) //! .new_read(io); //! # } //! # pub fn main() {} @@ -235,7 +231,6 @@ //! .length_field_type::() //! .length_adjustment(1) // length of hdr2 //! .num_skip(3) // length of hdr1 + LEN -//! .max_frame_length(256) //! .new_read(io); //! # } //! # pub fn main() {} @@ -282,7 +277,6 @@ //! .length_field_type::() //! .length_adjustment(-3) // length of hdr1 + LEN, negative //! .num_skip(3) -//! .max_frame_length(252) //! .new_read(io); //! # } //! ``` @@ -322,7 +316,6 @@ //! .length_field_length(3) //! .length_adjustment(0) // default value //! .num_skip(4) // skip the first 4 bytes -//! .max_frame_length(255) //! .new_read(io); //! # } //! # pub fn main() {} @@ -358,7 +351,6 @@ //! # let _ = //! LengthDelimitedCodec::builder() //! .length_field_type::() -//! .max_frame_length(255) //! .new_write(io); //! # } //! # pub fn main() {} @@ -395,28 +387,14 @@ use std::{cmp, fmt, mem}; /// that not all configuration settings apply to both encoding and decoding. See /// the documentation for specific methods for more detail. /// -/// # Configuration Compatibility -/// Not all combinations of [`max_frame_length`], [`length_field_length`], and -/// [`length_adjustment`] is valid. For a combination of these values to be valid, -/// the following must be true: -/// ```text -/// max_frame_length < 2^(length_field_length * 8) + length_adjustment -/// ``` -/// For example in the simplest case where `length_adjustment` is `0` and `length_field_length` -/// is one byte, the smallest number that cannot be represented by it is 256. So `max_frame_length` -/// must be smaller than 256 for the configuration to be valid. -/// -/// `length_adjustment` can offset the maximum allowed value. For example if `length_adjustment` -/// is `-1`, this means the length of the payload is going to be reported `1` byte more than its -/// true value. So a payload of `255` bytes will be framed with a length of `256`. But `256` cannot -/// be stored in a single byte `length_field_length`. So the previously valid `max_frame_length` of -/// `255` is now invalid. The case for a positive `length_adjustment` can be inferred accordingly. -/// -/// [`max_frame_length`]: Builder::max_frame_length -/// [`length_field_length`]: Builder::length_field_length -/// [`length_adjustment`]: Builder::length_adjustment +/// Note that the if the value of [`Builder::max_frame_length`] becomes larger than +/// what can actually fit in [`Builder::length_field_length`], it will be clipped to +/// the maximum value that can fit. #[derive(Debug, Clone, Copy)] pub struct Builder { + // Indicates whether the value of `max_frame_len` is specified by the user or is adjusted + max_frame_len_adjusted: bool, + // Maximum frame length max_frame_len: usize, @@ -694,6 +672,8 @@ impl Builder { /// ``` pub fn new() -> Builder { Builder { + max_frame_len_adjusted: false, + // Default max frame length of 8MB max_frame_len: 8 * 1_024 * 1_024, @@ -817,6 +797,10 @@ impl Builder { /// ``` pub fn max_frame_length(&mut self, val: usize) -> &mut Self { self.max_frame_len = val; + self.max_frame_len_adjusted = false; + + self.adjust_max_frame_len(false); + self } @@ -878,6 +862,9 @@ impl Builder { pub fn length_field_length(&mut self, val: usize) -> &mut Self { assert!(val > 0 && val <= 8, "invalid length field length"); self.length_field_len = val; + + self.adjust_max_frame_len(true); + self } @@ -921,6 +908,9 @@ impl Builder { /// ``` pub fn length_adjustment(&mut self, val: isize) -> &mut Self { self.length_adjustment = val; + + self.adjust_max_frame_len(true); + self } @@ -964,13 +954,10 @@ impl Builder { /// .length_field_type::() /// .length_adjustment(0) /// .num_skip(0) - /// .max_frame_length(65535) /// .new_codec(); /// # } /// ``` pub fn new_codec(&self) -> LengthDelimitedCodec { - self.assert_compatiblity(); - LengthDelimitedCodec { builder: *self, state: DecodeState::Head, @@ -995,7 +982,6 @@ impl Builder { /// .length_field_type::() /// .length_adjustment(0) /// .num_skip(0) - /// .max_frame_length(65535) /// .new_read(io); /// # } /// # pub fn main() {} @@ -1021,7 +1007,6 @@ impl Builder { /// # fn write_frame(io: T) { /// LengthDelimitedCodec::builder() /// .length_field_type::() - /// .max_frame_length(65535) /// .new_write(io); /// # } /// # pub fn main() {} @@ -1048,7 +1033,6 @@ impl Builder { /// # let _ = /// LengthDelimitedCodec::builder() /// .length_field_type::() - /// .max_frame_length(65535) /// .new_framed(io); /// # } /// # pub fn main() {} @@ -1060,13 +1044,17 @@ impl Builder { Framed::new(inner, self.new_codec()) } - /// Returns the maximum frame length that is allowed based on the values configured - /// by [`length_field_length`] and [`length_adjustment`]. For info about how it is - /// computed, refer to the [`Builder`] documentation. - /// - /// [`length_field_length`]: Builder::length_field_length - /// [`length_adjustment`]: Builder::length_adjustment - pub fn max_allowed_frame_length(&self) -> usize { + fn num_head_bytes(&self) -> usize { + let num = self.length_field_offset + self.length_field_len; + cmp::max(num, self.num_skip.unwrap_or(0)) + } + + fn get_num_skip(&self) -> usize { + self.num_skip + .unwrap_or(self.length_field_offset + self.length_field_len) + } + + fn adjust_max_frame_len(&mut self, adjust_to_max: bool) { // This function is basically `std::usize::saturating_add_signed`. Since it // requires MSRV 1.66, its implementation is copied here. // @@ -1087,29 +1075,15 @@ impl Builder { // In order to prevent an overflow, we do the last part manually. let max_number = max_number + (max_number - 1); - saturating_add_signed(max_number, self.length_adjustment) - } + let max_allowed_len = saturating_add_signed(max_number, self.length_adjustment); - fn num_head_bytes(&self) -> usize { - let num = self.length_field_offset + self.length_field_len; - cmp::max(num, self.num_skip.unwrap_or(0)) - } - - fn get_num_skip(&self) -> usize { - self.num_skip - .unwrap_or(self.length_field_offset + self.length_field_len) - } - - // Panics if the combination of `length_field_len`, `length_adjustment`, and `max_frame_len` - // are incompatible with each other. - fn assert_compatiblity(&self) { - let max_allowed_frame_length = self.max_allowed_frame_length(); - - if self.max_frame_len > max_allowed_frame_length { - panic!( - "max frame length exceeds the allowed amount: {} > {}", - self.max_frame_len, max_allowed_frame_length - ) + // We update the value of `max_frame_len` in two cases + // - Current value does not fit in `length_field_len` bytes + // - We have a larger `max_allowed_len` but only if the value has been adjusted before. + // This is to prevent overriding `max_frame_len` provided by the user. + if self.max_frame_len > max_allowed_len || (adjust_to_max && self.max_frame_len_adjusted) { + self.max_frame_len = max_allowed_len; + self.max_frame_len_adjusted = true; } } } diff --git a/tokio-util/tests/length_delimited.rs b/tokio-util/tests/length_delimited.rs index de7f56c54e6..12ec549ec93 100644 --- a/tokio-util/tests/length_delimited.rs +++ b/tokio-util/tests/length_delimited.rs @@ -325,7 +325,6 @@ fn read_update_max_frame_len_in_flight() { fn read_one_byte_length_field() { let io = length_delimited::Builder::new() .length_field_length(1) - .max_frame_length(255) .new_read(mock! { data(b"\x09abcdefghi"), }); @@ -340,7 +339,6 @@ fn read_header_offset() { let io = length_delimited::Builder::new() .length_field_length(2) .length_field_offset(4) - .max_frame_length(65535) .new_read(mock! { data(b"zzzz\x00\x09abcdefghi"), }); @@ -362,7 +360,6 @@ fn read_single_multi_frame_one_packet_skip_none_adjusted() { .length_field_offset(2) .num_skip(0) .length_adjustment(4) - .max_frame_length(65539) .new_read(mock! { data(&d), }); @@ -403,7 +400,6 @@ fn read_single_multi_frame_one_packet_length_includes_head() { let io = length_delimited::Builder::new() .length_field_length(2) .length_adjustment(-2) - .max_frame_length(65533) .new_read(mock! { data(&d), }); @@ -583,7 +579,6 @@ fn write_single_frame_little_endian() { fn write_single_frame_with_short_length_field() { let io = length_delimited::Builder::new() .length_field_length(1) - .max_frame_length(255) .new_write(mock! { data(b"\x09"), data(b"abcdefghi"), @@ -695,40 +690,53 @@ fn encode_overflow() { } #[test] -#[should_panic(expected = "max frame length exceeds the allowed amount: 256 > 255")] fn frame_does_not_fit() { - LengthDelimitedCodec::builder() + let codec = LengthDelimitedCodec::builder() .length_field_length(1) .max_frame_length(256) .new_codec(); + + assert_eq!(codec.max_frame_length(), 255); } #[test] -#[should_panic(expected = "max frame length exceeds the allowed amount: 255 > 254")] fn neg_adjusted_frame_does_not_fit() { - LengthDelimitedCodec::builder() + let codec = LengthDelimitedCodec::builder() .length_field_length(1) .length_adjustment(-1) - .max_frame_length(255) .new_codec(); + + assert_eq!(codec.max_frame_length(), 254); } #[test] -#[should_panic(expected = "max frame length exceeds the allowed amount: 257 > 256")] fn pos_adjusted_frame_does_not_fit() { - LengthDelimitedCodec::builder() + let codec = LengthDelimitedCodec::builder() .length_field_length(1) .length_adjustment(1) - .max_frame_length(257) .new_codec(); + + assert_eq!(codec.max_frame_length(), 256); } #[test] fn max_allowed_frame_fits() { - LengthDelimitedCodec::builder() + let codec = LengthDelimitedCodec::builder() .length_field_length(std::mem::size_of::()) .max_frame_length(usize::MAX) .new_codec(); + + assert_eq!(codec.max_frame_length(), usize::MAX); +} + +#[test] +fn smaller_frame_len_not_adjusted() { + let codec = LengthDelimitedCodec::builder() + .max_frame_length(10) + .length_field_length(std::mem::size_of::()) + .new_codec(); + + assert_eq!(codec.max_frame_length(), 10); } // ===== Test utils ===== From 08eeefec275d25611e95c1c160ce3885db62ff7a Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Thu, 21 Mar 2024 01:26:14 +0330 Subject: [PATCH 08/12] remove leftover docs --- tokio-util/src/codec/length_delimited.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index e95c48707db..c79bb775e2f 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -940,10 +940,6 @@ impl Builder { /// Create a configured length delimited `LengthDelimitedCodec` /// - /// # Panics - /// This method panics if the combination of `length_field_len`, `length_adjustment`, - /// and `max_frame_len` are incompatible with each other. - /// /// # Examples /// /// ``` @@ -966,10 +962,6 @@ impl Builder { /// Create a configured length delimited `FramedRead` /// - /// # Panics - /// This method panics if the combination of `length_field_len`, `length_adjustment`, - /// and `max_frame_len` are incompatible with each other. - /// /// # Examples /// /// ``` @@ -995,10 +987,6 @@ impl Builder { /// Create a configured length delimited `FramedWrite` /// - /// # Panics - /// This method panics if the combination of `length_field_len`, `length_adjustment`, - /// and `max_frame_len` are incompatible with each other. - /// /// # Examples /// /// ``` @@ -1020,10 +1008,6 @@ impl Builder { /// Create a configured length delimited `Framed` /// - /// # Panics - /// This method panics if the combination of `length_field_len`, `length_adjustment`, - /// and `max_frame_len` are incompatible with each other. - /// /// # Examples /// /// ``` From 541aaf597d5c722fc2c4c012c71955757a76ef50 Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Sat, 23 Mar 2024 03:11:50 +0330 Subject: [PATCH 09/12] add test for max length field --- tokio-util/src/codec/length_delimited.rs | 40 +++++++----------------- tokio-util/tests/length_delimited.rs | 10 ++++++ 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index c79bb775e2f..5552cdad2a8 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -392,9 +392,6 @@ use std::{cmp, fmt, mem}; /// the maximum value that can fit. #[derive(Debug, Clone, Copy)] pub struct Builder { - // Indicates whether the value of `max_frame_len` is specified by the user or is adjusted - max_frame_len_adjusted: bool, - // Maximum frame length max_frame_len: usize, @@ -672,8 +669,6 @@ impl Builder { /// ``` pub fn new() -> Builder { Builder { - max_frame_len_adjusted: false, - // Default max frame length of 8MB max_frame_len: 8 * 1_024 * 1_024, @@ -797,10 +792,6 @@ impl Builder { /// ``` pub fn max_frame_length(&mut self, val: usize) -> &mut Self { self.max_frame_len = val; - self.max_frame_len_adjusted = false; - - self.adjust_max_frame_len(false); - self } @@ -862,9 +853,6 @@ impl Builder { pub fn length_field_length(&mut self, val: usize) -> &mut Self { assert!(val > 0 && val <= 8, "invalid length field length"); self.length_field_len = val; - - self.adjust_max_frame_len(true); - self } @@ -908,9 +896,6 @@ impl Builder { /// ``` pub fn length_adjustment(&mut self, val: isize) -> &mut Self { self.length_adjustment = val; - - self.adjust_max_frame_len(true); - self } @@ -954,8 +939,12 @@ impl Builder { /// # } /// ``` pub fn new_codec(&self) -> LengthDelimitedCodec { + let mut builder = *self; + + builder.adjust_max_frame_len(); + LengthDelimitedCodec { - builder: *self, + builder, state: DecodeState::Head, } } @@ -1038,36 +1027,31 @@ impl Builder { .unwrap_or(self.length_field_offset + self.length_field_len) } - fn adjust_max_frame_len(&mut self, adjust_to_max: bool) { + fn adjust_max_frame_len(&mut self) { // This function is basically `std::usize::saturating_add_signed`. Since it // requires MSRV 1.66, its implementation is copied here. // // TODO: use the method from std when MSRV becomes >= 1.66 - fn saturating_add_signed(num: usize, rhs: isize) -> usize { - let (res, overflow) = num.overflowing_add(rhs as usize); + fn saturating_add_signed(num: u64, rhs: isize) -> u64 { + let (res, overflow) = num.overflowing_add(rhs as u64); if overflow == (rhs < 0) { res } else if overflow { - usize::MAX + u64::MAX } else { 0 } } // Calculate the maximum number that can be represented using `length_field_len` bytes. - let max_number = 2usize.saturating_pow((8 * self.length_field_len - 1) as u32); + let max_number = 2u64.saturating_pow((8 * self.length_field_len - 1) as u32); // In order to prevent an overflow, we do the last part manually. let max_number = max_number + (max_number - 1); let max_allowed_len = saturating_add_signed(max_number, self.length_adjustment); - // We update the value of `max_frame_len` in two cases - // - Current value does not fit in `length_field_len` bytes - // - We have a larger `max_allowed_len` but only if the value has been adjusted before. - // This is to prevent overriding `max_frame_len` provided by the user. - if self.max_frame_len > max_allowed_len || (adjust_to_max && self.max_frame_len_adjusted) { - self.max_frame_len = max_allowed_len; - self.max_frame_len_adjusted = true; + if self.max_frame_len as u64 > max_allowed_len { + self.max_frame_len = usize::try_from(max_allowed_len).unwrap_or(usize::MAX); } } } diff --git a/tokio-util/tests/length_delimited.rs b/tokio-util/tests/length_delimited.rs index 12ec549ec93..091a5b449e4 100644 --- a/tokio-util/tests/length_delimited.rs +++ b/tokio-util/tests/length_delimited.rs @@ -739,6 +739,16 @@ fn smaller_frame_len_not_adjusted() { assert_eq!(codec.max_frame_length(), 10); } +#[test] +fn max_allowed_length_field() { + let codec = LengthDelimitedCodec::builder() + .length_field_length(8) + .max_frame_length(usize::MAX) + .new_codec(); + + assert_eq!(codec.max_frame_length(), usize::MAX); +} + // ===== Test utils ===== struct Mock { From e064eeef89bf1400394805c65ab3fc3eee00f191 Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Sat, 23 Mar 2024 03:24:53 +0330 Subject: [PATCH 10/12] follow the impl for u64 more closely --- tokio-util/src/codec/length_delimited.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index 5552cdad2a8..bd5c9ced407 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -1028,11 +1028,11 @@ impl Builder { } fn adjust_max_frame_len(&mut self) { - // This function is basically `std::usize::saturating_add_signed`. Since it + // This function is basically `std::u64::saturating_add_signed`. Since it // requires MSRV 1.66, its implementation is copied here. // // TODO: use the method from std when MSRV becomes >= 1.66 - fn saturating_add_signed(num: u64, rhs: isize) -> u64 { + fn saturating_add_signed(num: u64, rhs: i64) -> u64 { let (res, overflow) = num.overflowing_add(rhs as u64); if overflow == (rhs < 0) { res @@ -1048,7 +1048,7 @@ impl Builder { // In order to prevent an overflow, we do the last part manually. let max_number = max_number + (max_number - 1); - let max_allowed_len = saturating_add_signed(max_number, self.length_adjustment); + let max_allowed_len = saturating_add_signed(max_number, self.length_adjustment as i64); if self.max_frame_len as u64 > max_allowed_len { self.max_frame_len = usize::try_from(max_allowed_len).unwrap_or(usize::MAX); From 52ca017ed2a668083bb8a1c758770182e5950408 Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Sat, 23 Mar 2024 14:14:28 +0330 Subject: [PATCH 11/12] Update tokio-util/src/codec/length_delimited.rs Co-authored-by: Alice Ryhl --- tokio-util/src/codec/length_delimited.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index bd5c9ced407..07a7cceba81 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -1044,9 +1044,10 @@ impl Builder { } // Calculate the maximum number that can be represented using `length_field_len` bytes. - let max_number = 2u64.saturating_pow((8 * self.length_field_len - 1) as u32); - // In order to prevent an overflow, we do the last part manually. - let max_number = max_number + (max_number - 1); + let max_number = match 1.checked_shl(8 * self.length_field_len) { + Some(shl) => shl - 1, + None => u64::MAX, + }; let max_allowed_len = saturating_add_signed(max_number, self.length_adjustment as i64); From 2fec6fbd48d0dd7b08d1b9b260e7ba8b724a9abe Mon Sep 17 00:00:00 2001 From: "M.Amin Rayej" Date: Sat, 23 Mar 2024 14:18:34 +0330 Subject: [PATCH 12/12] fix type issue --- tokio-util/src/codec/length_delimited.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokio-util/src/codec/length_delimited.rs b/tokio-util/src/codec/length_delimited.rs index 07a7cceba81..92d76b2cd28 100644 --- a/tokio-util/src/codec/length_delimited.rs +++ b/tokio-util/src/codec/length_delimited.rs @@ -1044,7 +1044,7 @@ impl Builder { } // Calculate the maximum number that can be represented using `length_field_len` bytes. - let max_number = match 1.checked_shl(8 * self.length_field_len) { + let max_number = match 1u64.checked_shl((8 * self.length_field_len) as u32) { Some(shl) => shl - 1, None => u64::MAX, };