util: assert compatibility between LengthDelimitedCodec options

tokio-util/src/codec/length_delimited.rs

@@ -87,6 +87,7 @@
 //!     .length_field_type::<u16>()
 //!     .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::<u16>()
 //!     .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::<u16>()
 //!     .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::<u16>()
 //!     .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::<u16>()
 //!     .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::<u16>()
+//!     .max_frame_length(255)
 //!     .new_write(io);
 //! # }
 //! # pub fn main() {}
@@ -386,6 +394,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 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
 #[derive(Debug, Clone, Copy)]
 pub struct Builder {
     // Maximum frame length
@@ -921,6 +950,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 +964,13 @@ impl Builder {
     ///     .length_field_type::<u16>()
     ///     .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 +979,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 +995,7 @@ impl Builder {
     ///     .length_field_type::<u16>()
     ///     .length_adjustment(0)
     ///     .num_skip(0)
+    ///     .max_frame_length(65535)
     ///     .new_read(io);
     /// # }
     /// # pub fn main() {}
@@ -968,6 +1009,10 @@ 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
     ///
     /// ```
@@ -976,6 +1021,7 @@ impl Builder {
     /// # fn write_frame<T: AsyncWrite>(io: T) {
     /// LengthDelimitedCodec::builder()
     ///     .length_field_type::<u16>()
+    ///     .max_frame_length(65535)
     ///     .new_write(io);
     /// # }
     /// # pub fn main() {}
@@ -989,6 +1035,10 @@ 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
     ///
     /// ```
@@ -998,6 +1048,7 @@ impl Builder {
     /// # let _ =
     /// LengthDelimitedCodec::builder()
     ///     .length_field_type::<u16>()
+    ///     .max_frame_length(65535)
     ///     .new_framed(io);
     /// # }
     /// # pub fn main() {}
@@ -1009,6 +1060,36 @@ 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 {
+        // 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
+            }
+        }
+
+        // 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);
+
+        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))
@@ -1018,6 +1099,19 @@ 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) {
+        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
+            )
+        }
+    }
 }
 
 impl Default for Builder {

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,43 @@ fn encode_overflow() {
     codec.encode(Bytes::from("hello"), &mut buf).unwrap();
 }
 
+#[test]
+#[should_panic(expected = "max frame length exceeds the allowed amount: 256 > 255")]
+fn 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 allowed amount: 255 > 254")]
+fn 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 allowed amount: 257 > 256")]
+fn pos_adjusted_frame_does_not_fit() {
+    LengthDelimitedCodec::builder()
+        .length_field_length(1)
+        .length_adjustment(1)
+        .max_frame_length(257)
+        .new_codec();
+}
+
+#[test]
+fn max_allowed_frame_fits() {
+    LengthDelimitedCodec::builder()
+        .length_field_length(std::mem::size_of::<usize>())
+        .max_frame_length(usize::MAX)
+        .new_codec();
+}
+
 // ===== Test utils =====
 
 struct Mock {