From 2795a4ab0930fd743fab52c2dd51cadc362d2dd7 Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 20 Jun 2025 15:40:07 +0100 Subject: [PATCH 1/5] Add configurable length format for frames --- AGENTS.md | 11 +- README.md | 15 ++- docs/roadmap.md | 7 +- docs/rust-binary-router-library-design.md | 1 - src/app.rs | 4 +- src/frame.rs | 153 +++++++++++++++++++--- tests/response.rs | 35 ++++- 7 files changed, 185 insertions(+), 41 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 394b057a..66d48317 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -95,11 +95,10 @@ management. Contributors should follow these best practices when working on the project: - Run `make fmt`, `make lint`, and `make test` before committing. These targets - wrap `cargo fmt`, `cargo clippy`, and `cargo test` with the appropriate - flags. + wrap `cargo fmt`, `cargo clippy`, and `cargo test` with the appropriate flags. - Clippy warnings MUST be disallowed. -- Fix any warnings emitted during tests in the code itself rather than - silencing them. +- Fix any warnings emitted during tests in the code itself rather than silencing + them. - Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS. - Where a function has too many parameters, group related parameters in @@ -121,8 +120,8 @@ project: - Validate Markdown files using `markdownlint`. - Run `mdformat-all` after any documentation changes to format all Markdown files and fix table markup. -- Validate Markdown Mermaid diagrams using the `nixie` CLI. The tool is - already installed; run `nixie` directly instead of using `npx`. +- Validate Markdown Mermaid diagrams using the `nixie` CLI. The tool is already + installed; run `nixie` directly instead of using `npx`. ### Key Takeaway diff --git a/README.md b/README.md index f1f1c3ec..6ebddd76 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,8 @@ encoded using the application's configured serializer and written back through the `FrameProcessor`【F:docs/rust-binary-router-library-design.md†L718-L724】. The included `LengthPrefixedProcessor` illustrates a simple framing strategy -based on a big‑endian length +that prefixes each frame with its length. The format is configurable (prefix +size and endianness) and defaults to a 4‑byte big‑endian length prefix【F:docs/rust-binary-router-library-design.md†L1076-L1117】. ## Connection Lifecycle @@ -113,9 +114,9 @@ when the connection ends. Extractors are types that implement `FromMessageRequest`. When a handler lists an extractor as a parameter, `wireframe` automatically constructs it using the -incoming \[`MessageRequest`\] and remaining \[`Payload`\]. Built‑in extractors like -`Message`, `SharedState` and `ConnectionInfo` decode the payload, access -app state or expose peer information. +incoming \[`MessageRequest`\] and remaining \[`Payload`\]. Built‑in extractors +like `Message`, `SharedState` and `ConnectionInfo` decode the payload, +access app state or expose peer information. Custom extractors let you centralize parsing and validation logic that would otherwise be duplicated across handlers. A session token parser, for example, @@ -168,9 +169,9 @@ let logging = from_fn(|req, next| async move { ## Current Limitations -Connection handling now processes frames and routes messages, but the -server is still experimental. Release builds fail to compile, so the -library cannot be used accidentally in production. +Connection handling now processes frames and routes messages, but the server is +still experimental. Release builds fail to compile, so the library cannot be +used accidentally in production. ## Roadmap diff --git a/docs/roadmap.md b/docs/roadmap.md index 0006b588..bd92613e 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -88,9 +88,10 @@ after formatting. Line numbers below refer to that file. - [ ] Implement middleware using `Transform`/`Service` traits. - [x] Implement `ServiceRequest` and `ServiceResponse` wrappers (lines - 866-899) and introduce a `Next` helper to build the asynchronous call - chain. Trait definitions live in - [`src/middleware.rs`](../src/middleware.rs#L71-L84). + 866-899) and introduce a `Next` helper to build the asynchronous call chain. + Trait definitions live in + [`src/middleware.rs`](../src/middleware.rs#L71-L84). + - [ ] Provide a `from_fn` helper for functional middleware. - [x] Add tests verifying middleware can modify requests and observe responses. diff --git a/docs/rust-binary-router-library-design.md b/docs/rust-binary-router-library-design.md index fc0387ab..6c7f51d5 100644 --- a/docs/rust-binary-router-library-design.md +++ b/docs/rust-binary-router-library-design.md @@ -784,7 +784,6 @@ instance of each type can exist; later registrations overwrite earlier ones. a specific field in all messages, validate it, and provide a `UserSession` object to the handler. - This extractor system, backed by Rust's strong type system, ensures that handlers receive correctly typed and validated data, significantly reducing the likelihood of runtime errors and boilerplate parsing code within the handler diff --git a/src/app.rs b/src/app.rs index 1195275c..1d6a308a 100644 --- a/src/app.rs +++ b/src/app.rs @@ -17,7 +17,7 @@ use bytes::BytesMut; use tokio::io::{self, AsyncWrite, AsyncWriteExt}; use crate::{ - frame::{FrameProcessor, LengthPrefixedProcessor}, + frame::{FrameProcessor, LengthFormat, LengthPrefixedProcessor}, message::Message, serializer::{BincodeSerializer, Serializer}, }; @@ -152,7 +152,7 @@ where routes: HashMap::new(), services: Vec::new(), middleware: Vec::new(), - frame_processor: Box::new(LengthPrefixedProcessor), + frame_processor: Box::new(LengthPrefixedProcessor::new(LengthFormat::default())), serializer: S::default(), app_data: HashMap::new(), on_connect: None, diff --git a/src/frame.rs b/src/frame.rs index e4add771..aaa0dfab 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -6,7 +6,122 @@ use std::io; -use bytes::{Buf, BytesMut}; +use bytes::{Buf, BufMut, BytesMut}; + +/// Byte order used for encoding and decoding length prefixes. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Endianness { + /// Most significant byte first. + Big, + /// Least significant byte first. + Little, +} + +/// Format of the length prefix preceding each frame. +#[derive(Clone, Copy, Debug)] +pub struct LengthFormat { + bytes: usize, + endianness: Endianness, +} + +impl LengthFormat { + /// Create a new [`LengthFormat`]. + #[must_use] + pub const fn new(bytes: usize, endianness: Endianness) -> Self { Self { bytes, endianness } } + + /// Two byte big-endian prefix. + #[must_use] + pub const fn u16_be() -> Self { Self::new(2, Endianness::Big) } + + /// Two byte little-endian prefix. + #[must_use] + pub const fn u16_le() -> Self { Self::new(2, Endianness::Little) } + + /// Four byte big-endian prefix. + #[must_use] + pub const fn u32_be() -> Self { Self::new(4, Endianness::Big) } + + /// Four byte little-endian prefix. + #[must_use] + pub const fn u32_le() -> Self { Self::new(4, Endianness::Little) } + + fn read_len(&self, bytes: &[u8]) -> io::Result { + let len = match (self.bytes, self.endianness) { + (1, _) => u64::from(u8::from_ne_bytes([bytes[0]])), + (2, Endianness::Big) => u64::from(u16::from_be_bytes([bytes[0], bytes[1]])), + (2, Endianness::Little) => u64::from(u16::from_le_bytes([bytes[0], bytes[1]])), + (4, Endianness::Big) => { + u64::from(u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) + } + (4, Endianness::Little) => { + u64::from(u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) + } + (8, Endianness::Big) => u64::from_be_bytes([ + bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7], + ]), + (8, Endianness::Little) => u64::from_le_bytes([ + bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7], + ]), + _ => { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "unsupported length prefix size", + )); + } + }; + usize::try_from(len).map_err(|_| io::Error::other("frame too large")) + } + + fn write_len(&self, len: usize, dst: &mut BytesMut) -> io::Result<()> { + match (self.bytes, self.endianness) { + (1, _) => dst.put_u8( + u8::try_from(len) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))?, + ), + (2, Endianness::Big) => dst.put_slice( + &u16::try_from(len) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))? + .to_be_bytes(), + ), + (2, Endianness::Little) => dst.put_slice( + &u16::try_from(len) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))? + .to_le_bytes(), + ), + (4, Endianness::Big) => dst.put_slice( + &u32::try_from(len) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))? + .to_be_bytes(), + ), + (4, Endianness::Little) => dst.put_slice( + &u32::try_from(len) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))? + .to_le_bytes(), + ), + (8, Endianness::Big) => dst.put_slice( + &u64::try_from(len) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))? + .to_be_bytes(), + ), + (8, Endianness::Little) => dst.put_slice( + &u64::try_from(len) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))? + .to_le_bytes(), + ), + _ => { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "unsupported length prefix size", + )); + } + } + Ok(()) + } +} + +impl Default for LengthFormat { + fn default() -> Self { Self::u32_be() } +} /// Trait defining how raw bytes are decoded into frames and how frames are /// encoded back into bytes for transmission. @@ -38,34 +153,40 @@ pub trait FrameProcessor: Send + Sync { fn encode(&self, frame: &Self::Frame, dst: &mut BytesMut) -> Result<(), Self::Error>; } -/// Simple length-prefixed framing using big-endian u32 lengths. -pub struct LengthPrefixedProcessor; +/// Simple length-prefixed framing using a configurable length prefix. +pub struct LengthPrefixedProcessor { + format: LengthFormat, +} + +impl LengthPrefixedProcessor { + /// Construct a processor with the provided [`LengthFormat`]. + #[must_use] + pub const fn new(format: LengthFormat) -> Self { Self { format } } +} + +impl Default for LengthPrefixedProcessor { + fn default() -> Self { Self::new(LengthFormat::default()) } +} impl FrameProcessor for LengthPrefixedProcessor { type Frame = Vec; type Error = std::io::Error; fn decode(&self, src: &mut BytesMut) -> Result, Self::Error> { - if src.len() < 4 { + if src.len() < self.format.bytes { return Ok(None); } - let mut len_bytes = [0u8; 4]; - len_bytes.copy_from_slice(&src[..4]); - let len = u32::from_be_bytes(len_bytes); - let len_usize = usize::try_from(len).map_err(|_| io::Error::other("frame too large"))?; - if src.len() < 4 + len_usize { + let len = self.format.read_len(&src[..self.format.bytes])?; + if src.len() < self.format.bytes + len { return Ok(None); } - src.advance(4); - Ok(Some(src.split_to(len_usize).to_vec())) + src.advance(self.format.bytes); + Ok(Some(src.split_to(len).to_vec())) } fn encode(&self, frame: &Self::Frame, dst: &mut BytesMut) -> Result<(), Self::Error> { - use bytes::BufMut; - dst.reserve(4 + frame.len()); - let len = u32::try_from(frame.len()) - .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))?; - dst.put_u32(len); + dst.reserve(self.format.bytes + frame.len()); + self.format.write_len(frame.len(), dst)?; dst.extend_from_slice(frame); Ok(()) } diff --git a/tests/response.rs b/tests/response.rs index 277820cf..b90f6988 100644 --- a/tests/response.rs +++ b/tests/response.rs @@ -6,7 +6,7 @@ use bytes::BytesMut; use wireframe::{ app::WireframeApp, - frame::{FrameProcessor, LengthPrefixedProcessor}, + frame::{FrameProcessor, LengthFormat, LengthPrefixedProcessor}, message::Message, serializer::BincodeSerializer, }; @@ -38,13 +38,13 @@ impl<'de> bincode::BorrowDecode<'de, ()> for FailingResp { async fn send_response_encodes_and_frames() { let app = WireframeApp::new() .unwrap() - .frame_processor(LengthPrefixedProcessor) + .frame_processor(LengthPrefixedProcessor::default()) .serializer(BincodeSerializer); let mut out = Vec::new(); app.send_response(&mut out, &TestResp(7)).await.unwrap(); - let processor = LengthPrefixedProcessor; + let processor = LengthPrefixedProcessor::default(); let mut buf = BytesMut::from(&out[..]); let frame = processor.decode(&mut buf).unwrap().unwrap(); let (decoded, _) = TestResp::from_bytes(&frame).unwrap(); @@ -53,7 +53,7 @@ async fn send_response_encodes_and_frames() { #[tokio::test] async fn length_prefixed_decode_requires_complete_header() { - let processor = LengthPrefixedProcessor; + let processor = LengthPrefixedProcessor::default(); let mut buf = BytesMut::from(&[0x00, 0x00, 0x00][..]); // only 3 bytes assert!(processor.decode(&mut buf).unwrap().is_none()); assert_eq!(buf.len(), 3); // nothing consumed @@ -61,7 +61,7 @@ async fn length_prefixed_decode_requires_complete_header() { #[tokio::test] async fn length_prefixed_decode_requires_full_frame() { - let processor = LengthPrefixedProcessor; + let processor = LengthPrefixedProcessor::default(); let mut buf = BytesMut::from(&[0x00, 0x00, 0x00, 0x05, 0x01, 0x02][..]); assert!(processor.decode(&mut buf).unwrap().is_none()); // buffer should retain bytes since frame isn't complete @@ -98,7 +98,7 @@ impl tokio::io::AsyncWrite for FailingWriter { async fn send_response_propagates_write_error() { let app = WireframeApp::new() .unwrap() - .frame_processor(LengthPrefixedProcessor); + .frame_processor(LengthPrefixedProcessor::default()); let mut writer = FailingWriter; let err = app @@ -117,3 +117,26 @@ async fn send_response_returns_encode_error() { .expect_err("expected error"); assert!(matches!(err, wireframe::app::SendError::Serialize(_))); } + +#[test] +fn custom_two_byte_big_endian_roundtrip() { + let fmt = LengthFormat::u16_be(); + let processor = LengthPrefixedProcessor::new(fmt); + let frame = vec![1, 2, 3, 4]; + let mut buf = BytesMut::new(); + processor.encode(&frame, &mut buf).unwrap(); + let decoded = processor.decode(&mut buf).unwrap().unwrap(); + assert_eq!(decoded, frame); +} + +#[test] +fn custom_four_byte_little_endian_roundtrip() { + let fmt = LengthFormat::u32_le(); + let processor = LengthPrefixedProcessor::new(fmt); + let frame = vec![9, 8, 7]; + let mut buf = BytesMut::new(); + processor.encode(&frame, &mut buf).unwrap(); + assert_eq!(&buf[..4], u32::try_from(frame.len()).unwrap().to_le_bytes()); + let decoded = processor.decode(&mut buf).unwrap().unwrap(); + assert_eq!(decoded, frame); +} From 2cb4d3edb9f66d0943939d3ea6937395b459166b Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Fri, 20 Jun 2025 16:02:12 +0100 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`cod?= =?UTF-8?q?ex/define-endianness-enum-and-refactor-lengthprefixedprocessor`?= =?UTF-8?q?=20(#89)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 📝 Add docstrings to `codex/define-endianness-enum-and-refactor-lengthprefixedprocessor` Docstrings generation was requested by @leynos. * https://github.com/leynos/wireframe/pull/87#issuecomment-2991876730 The following files were modified: * `src/app.rs` * `src/frame.rs` * `tests/response.rs` * Move attributes below doc comments (#90) --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Leynos --- src/app.rs | 4 +++ src/frame.rs | 71 +++++++++++++++++++++++++++++++++++++++++++---- tests/response.rs | 25 +++++++++++++++++ 3 files changed, 94 insertions(+), 6 deletions(-) diff --git a/src/app.rs b/src/app.rs index 1d6a308a..a9749cc0 100644 --- a/src/app.rs +++ b/src/app.rs @@ -147,6 +147,10 @@ where S: Serializer + Default, C: Send + 'static, { + /// Creates a new `WireframeApp` instance with default configuration. + /// + /// Initialises empty routes, services, middleware, and application data. Sets the + /// default frame processor and serializer, with no connection lifecycle hooks. fn default() -> Self { Self { routes: HashMap::new(), diff --git a/src/frame.rs b/src/frame.rs index aaa0dfab..97e31dc5 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -25,26 +25,47 @@ pub struct LengthFormat { } impl LengthFormat { - /// Create a new [`LengthFormat`]. + /// Creates a new `LengthFormat` with the specified number of bytes and + /// endianness for the length prefix. + /// + /// # Parameters + /// - `bytes`: The number of bytes used for the length prefix. + /// - `endianness`: The byte order for encoding and decoding the length prefix. + /// + /// # Returns + /// A `LengthFormat` configured with the given size and endianness. #[must_use] pub const fn new(bytes: usize, endianness: Endianness) -> Self { Self { bytes, endianness } } - /// Two byte big-endian prefix. + /// Creates a `LengthFormat` for a 2-byte big-endian length prefix. #[must_use] pub const fn u16_be() -> Self { Self::new(2, Endianness::Big) } - /// Two byte little-endian prefix. + /// Creates a `LengthFormat` for a 2-byte little-endian length prefix. #[must_use] pub const fn u16_le() -> Self { Self::new(2, Endianness::Little) } - /// Four byte big-endian prefix. + /// Creates a `LengthFormat` for a 4-byte big-endian length prefix. #[must_use] pub const fn u32_be() -> Self { Self::new(4, Endianness::Big) } - /// Four byte little-endian prefix. + /// Creates a `LengthFormat` for a 4-byte little-endian length prefix. #[must_use] pub const fn u32_le() -> Self { Self::new(4, Endianness::Little) } + /// Reads a length prefix from a byte slice according to the configured prefix size and + /// endianness. + /// + /// # Parameters + /// - `bytes`: The byte slice containing the length prefix. Must be at least as long as the + /// configured prefix size. + /// + /// # Returns + /// The decoded length as a `usize` if successful. + /// + /// # Errors + /// Returns an error if the prefix size is unsupported or if the decoded length does not fit in + /// a `usize`. fn read_len(&self, bytes: &[u8]) -> io::Result { let len = match (self.bytes, self.endianness) { (1, _) => u64::from(u8::from_ne_bytes([bytes[0]])), @@ -72,6 +93,18 @@ impl LengthFormat { usize::try_from(len).map_err(|_| io::Error::other("frame too large")) } + /// Writes a length prefix to the destination buffer using the configured size and endianness. + /// + /// Returns an error if the length is too large to fit in the configured prefix size or if the + /// prefix size is unsupported. + /// + /// # Parameters + /// - `len`: The length value to encode and write. + /// - `dst`: The buffer to which the encoded length prefix will be appended. + /// + /// # Errors + /// Returns an error if `len` exceeds the maximum value for the configured prefix size or if the + /// prefix size is not supported. fn write_len(&self, len: usize, dst: &mut BytesMut) -> io::Result<()> { match (self.bytes, self.endianness) { (1, _) => dst.put_u8( @@ -120,6 +153,9 @@ impl LengthFormat { } impl Default for LengthFormat { + /// Returns a `LengthFormat` using a 4-byte big-endian length prefix. + /// + /// This is the default format for length-prefixed framing. fn default() -> Self { Self::u32_be() } } @@ -159,12 +195,24 @@ pub struct LengthPrefixedProcessor { } impl LengthPrefixedProcessor { - /// Construct a processor with the provided [`LengthFormat`]. + /// Creates a new `LengthPrefixedProcessor` with the specified length prefix + /// format. + /// + /// # Parameters + /// - `format`: The length prefix format to use for framing. + /// + /// # Returns + /// A `LengthPrefixedProcessor` configured with the given length format. #[must_use] pub const fn new(format: LengthFormat) -> Self { Self { format } } } impl Default for LengthPrefixedProcessor { + /// Creates a `LengthPrefixedProcessor` using the default length format (4-byte big-endian + /// prefix). + /// + /// # Returns + /// A processor configured for 4-byte big-endian length-prefixed framing. fn default() -> Self { Self::new(LengthFormat::default()) } } @@ -172,6 +220,13 @@ impl FrameProcessor for LengthPrefixedProcessor { type Frame = Vec; type Error = std::io::Error; + /// Attempts to decode a single length-prefixed frame from the source buffer. + /// + /// Returns `Ok(Some(frame))` if a complete frame is available, `Ok(None)` if + /// more data is needed, or an error if the length prefix is invalid or cannot + /// be read according to the configured format. + /// + /// The source buffer is advanced past the decoded frame and its length prefix. fn decode(&self, src: &mut BytesMut) -> Result, Self::Error> { if src.len() < self.format.bytes { return Ok(None); @@ -184,6 +239,10 @@ impl FrameProcessor for LengthPrefixedProcessor { Ok(Some(src.split_to(len).to_vec())) } + /// Encodes a frame by prefixing it with its length and appending it to the destination buffer. + /// + /// The length prefix format is determined by the processor's configuration. Returns an error + /// if the frame length cannot be represented in the configured format. fn encode(&self, frame: &Self::Frame, dst: &mut BytesMut) -> Result<(), Self::Error> { dst.reserve(self.format.bytes + frame.len()); self.format.write_len(frame.len(), dst)?; diff --git a/tests/response.rs b/tests/response.rs index b90f6988..7b024e81 100644 --- a/tests/response.rs +++ b/tests/response.rs @@ -35,6 +35,8 @@ impl<'de> bincode::BorrowDecode<'de, ()> for FailingResp { } #[tokio::test] +/// Tests that sending a response serialises and frames the data correctly, +/// and that the response can be decoded and deserialised back to its original value asynchronously. async fn send_response_encodes_and_frames() { let app = WireframeApp::new() .unwrap() @@ -52,6 +54,9 @@ async fn send_response_encodes_and_frames() { } #[tokio::test] +/// Tests that decoding with an incomplete length prefix header returns `None` and does not consume any bytes from the buffer. +/// +/// This ensures that the decoder waits for the full header before attempting to decode a frame. async fn length_prefixed_decode_requires_complete_header() { let processor = LengthPrefixedProcessor::default(); let mut buf = BytesMut::from(&[0x00, 0x00, 0x00][..]); // only 3 bytes @@ -60,6 +65,10 @@ async fn length_prefixed_decode_requires_complete_header() { } #[tokio::test] +/// Tests that decoding with a complete length prefix but incomplete frame data returns `None` +/// and retains all bytes in the buffer. +/// +/// Ensures that the decoder does not consume any bytes when the full frame is not yet available. async fn length_prefixed_decode_requires_full_frame() { let processor = LengthPrefixedProcessor::default(); let mut buf = BytesMut::from(&[0x00, 0x00, 0x00, 0x05, 0x01, 0x02][..]); @@ -95,6 +104,10 @@ impl tokio::io::AsyncWrite for FailingWriter { } #[tokio::test] +/// Tests that `send_response` correctly propagates I/O errors encountered during writing. +/// +/// This test uses a writer that always fails on write operations and asserts that +/// the resulting error is of the `Io` variant. async fn send_response_propagates_write_error() { let app = WireframeApp::new() .unwrap() @@ -109,6 +122,10 @@ async fn send_response_propagates_write_error() { } #[tokio::test] +/// Tests that `send_response` returns a serialization error when encoding fails. +/// +/// This test sends a `FailingResp` using `send_response` and asserts that the resulting +/// error is of the `Serialize` variant, indicating a failure during response encoding. async fn send_response_returns_encode_error() { let app = WireframeApp::new().unwrap(); let err = app @@ -119,6 +136,10 @@ async fn send_response_returns_encode_error() { } #[test] +/// Tests roundtrip encoding and decoding of a frame using a two-byte big-endian length prefix. +/// +/// Verifies that a frame encoded with a `LengthPrefixedProcessor` configured for a 2-byte +/// big-endian length format can be correctly decoded back to its original contents. fn custom_two_byte_big_endian_roundtrip() { let fmt = LengthFormat::u16_be(); let processor = LengthPrefixedProcessor::new(fmt); @@ -130,6 +151,10 @@ fn custom_two_byte_big_endian_roundtrip() { } #[test] +/// Tests roundtrip encoding and decoding of a frame using a four-byte little-endian length prefix. +/// +/// Verifies that the encoded buffer contains the correct little-endian length prefix and that +/// decoding restores the original frame. fn custom_four_byte_little_endian_roundtrip() { let fmt = LengthFormat::u32_le(); let processor = LengthPrefixedProcessor::new(fmt); From e0a7c0868cde8ad1bdc0600cd7b807104bd0860d Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 20 Jun 2025 16:02:54 +0100 Subject: [PATCH 3/5] Refine length prefix handling --- docs/roadmap.md | 4 +-- src/app.rs | 4 +-- src/frame.rs | 88 +++++++++++++++-------------------------------- tests/response.rs | 22 +++++++++++- 4 files changed, 52 insertions(+), 66 deletions(-) diff --git a/docs/roadmap.md b/docs/roadmap.md index bd92613e..a0acefd1 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -88,8 +88,8 @@ after formatting. Line numbers below refer to that file. - [ ] Implement middleware using `Transform`/`Service` traits. - [x] Implement `ServiceRequest` and `ServiceResponse` wrappers (lines - 866-899) and introduce a `Next` helper to build the asynchronous call chain. - Trait definitions live in + 866-899) and introduce a `Next` helper to build the asynchronous call + chain. Trait definitions live in [`src/middleware.rs`](../src/middleware.rs#L71-L84). - [ ] Provide a `from_fn` helper for functional middleware. diff --git a/src/app.rs b/src/app.rs index a9749cc0..abb674ea 100644 --- a/src/app.rs +++ b/src/app.rs @@ -17,7 +17,7 @@ use bytes::BytesMut; use tokio::io::{self, AsyncWrite, AsyncWriteExt}; use crate::{ - frame::{FrameProcessor, LengthFormat, LengthPrefixedProcessor}, + frame::{FrameProcessor, LengthPrefixedProcessor}, message::Message, serializer::{BincodeSerializer, Serializer}, }; @@ -147,7 +147,7 @@ where S: Serializer + Default, C: Send + 'static, { - /// Creates a new `WireframeApp` instance with default configuration. + frame_processor: Box::new(LengthPrefixedProcessor::default()), /// /// Initialises empty routes, services, middleware, and application data. Sets the /// default frame processor and serializer, with no connection lifecycle hooks. diff --git a/src/frame.rs b/src/frame.rs index 97e31dc5..e8b531f4 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -25,69 +25,35 @@ pub struct LengthFormat { } impl LengthFormat { - /// Creates a new `LengthFormat` with the specified number of bytes and - /// endianness for the length prefix. - /// - /// # Parameters - /// - `bytes`: The number of bytes used for the length prefix. - /// - `endianness`: The byte order for encoding and decoding the length prefix. - /// - /// # Returns - /// A `LengthFormat` configured with the given size and endianness. - #[must_use] - pub const fn new(bytes: usize, endianness: Endianness) -> Self { Self { bytes, endianness } } - - /// Creates a `LengthFormat` for a 2-byte big-endian length prefix. - #[must_use] - pub const fn u16_be() -> Self { Self::new(2, Endianness::Big) } - - /// Creates a `LengthFormat` for a 2-byte little-endian length prefix. - #[must_use] - pub const fn u16_le() -> Self { Self::new(2, Endianness::Little) } - - /// Creates a `LengthFormat` for a 4-byte big-endian length prefix. - #[must_use] - pub const fn u32_be() -> Self { Self::new(4, Endianness::Big) } + if bytes.len() < self.bytes { + return Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + "length prefix truncated", + )); + } + if !matches!(self.bytes, 1 | 2 | 4 | 8) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "unsupported length prefix size", + )); + } - /// Creates a `LengthFormat` for a 4-byte little-endian length prefix. - #[must_use] - pub const fn u32_le() -> Self { Self::new(4, Endianness::Little) } + let mut slice = &bytes[..self.bytes]; + let len = match self.endianness { + Endianness::Big => slice.get_uint(self.bytes), + Endianness::Little => slice.get_uint_le(self.bytes), + if !matches!(self.bytes, 1 | 2 | 4 | 8) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "unsupported length prefix size", + )); + } - /// Reads a length prefix from a byte slice according to the configured prefix size and - /// endianness. - /// - /// # Parameters - /// - `bytes`: The byte slice containing the length prefix. Must be at least as long as the - /// configured prefix size. - /// - /// # Returns - /// The decoded length as a `usize` if successful. - /// - /// # Errors - /// Returns an error if the prefix size is unsupported or if the decoded length does not fit in - /// a `usize`. - fn read_len(&self, bytes: &[u8]) -> io::Result { - let len = match (self.bytes, self.endianness) { - (1, _) => u64::from(u8::from_ne_bytes([bytes[0]])), - (2, Endianness::Big) => u64::from(u16::from_be_bytes([bytes[0], bytes[1]])), - (2, Endianness::Little) => u64::from(u16::from_le_bytes([bytes[0], bytes[1]])), - (4, Endianness::Big) => { - u64::from(u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) - } - (4, Endianness::Little) => { - u64::from(u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) - } - (8, Endianness::Big) => u64::from_be_bytes([ - bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7], - ]), - (8, Endianness::Little) => u64::from_le_bytes([ - bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7], - ]), - _ => { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "unsupported length prefix size", - )); + let len_u64 = u64::try_from(len) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))?; + match self.endianness { + Endianness::Big => dst.put_uint(len_u64, self.bytes), + Endianness::Little => dst.put_uint_le(len_u64, self.bytes), } }; usize::try_from(len).map_err(|_| io::Error::other("frame too large")) diff --git a/tests/response.rs b/tests/response.rs index 7b024e81..65f28f7c 100644 --- a/tests/response.rs +++ b/tests/response.rs @@ -6,7 +6,7 @@ use bytes::BytesMut; use wireframe::{ app::WireframeApp, - frame::{FrameProcessor, LengthFormat, LengthPrefixedProcessor}, + frame::{Endianness, FrameProcessor, LengthFormat, LengthPrefixedProcessor}, message::Message, serializer::BincodeSerializer, }; @@ -115,6 +115,26 @@ async fn send_response_propagates_write_error() { let mut writer = FailingWriter; let err = app + +#[test] +fn encode_fails_for_unsupported_prefix_size() { + let fmt = LengthFormat::new(3, Endianness::Big); + let processor = LengthPrefixedProcessor::new(fmt); + let mut buf = BytesMut::new(); + let err = processor + .encode(&vec![1, 2], &mut buf) + .expect_err("expected error"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); +} + +#[test] +fn decode_fails_for_unsupported_prefix_size() { + let fmt = LengthFormat::new(3, Endianness::Little); + let processor = LengthPrefixedProcessor::new(fmt); + let mut buf = BytesMut::from(&[0x00, 0x01, 0x02][..]); + let err = processor.decode(&mut buf).expect_err("expected error"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); +} .send_response(&mut writer, &TestResp(3)) .await .expect_err("expected error"); From f9a55491afc8c7386e013949e9ccc7a36abfe75d Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 20 Jun 2025 16:22:59 +0100 Subject: [PATCH 4/5] Use rstest for length format tests --- docs/roadmap.md | 4 ++-- tests/response.rs | 24 ++++++++++-------------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/docs/roadmap.md b/docs/roadmap.md index a0acefd1..bd92613e 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -88,8 +88,8 @@ after formatting. Line numbers below refer to that file. - [ ] Implement middleware using `Transform`/`Service` traits. - [x] Implement `ServiceRequest` and `ServiceResponse` wrappers (lines - 866-899) and introduce a `Next` helper to build the asynchronous call - chain. Trait definitions live in + 866-899) and introduce a `Next` helper to build the asynchronous call chain. + Trait definitions live in [`src/middleware.rs`](../src/middleware.rs#L71-L84). - [ ] Provide a `from_fn` helper for functional middleware. diff --git a/tests/response.rs b/tests/response.rs index 65f28f7c..7d4fa0dc 100644 --- a/tests/response.rs +++ b/tests/response.rs @@ -4,6 +4,7 @@ //! write failures and encode errors. use bytes::BytesMut; +use rstest::rstest; use wireframe::{ app::WireframeApp, frame::{Endianness, FrameProcessor, LengthFormat, LengthPrefixedProcessor}, @@ -93,21 +94,16 @@ impl tokio::io::AsyncWrite for FailingWriter { _: &mut std::task::Context<'_>, ) -> std::task::Poll> { std::task::Poll::Ready(Ok(())) - } - - fn poll_shutdown( - self: std::pin::Pin<&mut Self>, - _: &mut std::task::Context<'_>, - ) -> std::task::Poll> { - std::task::Poll::Ready(Ok(())) - } -} +#[rstest] +#[case(LengthFormat::u16_be(), vec![1, 2, 3, 4], vec![0x00, 0x04])] +#[case(LengthFormat::u32_le(), vec![9, 8, 7], vec![3, 0, 0, 0])] +fn custom_length_roundtrip( + #[case] fmt: LengthFormat, + #[case] frame: Vec, + #[case] prefix: Vec, +) { + assert_eq!(&buf[..prefix.len()], &prefix[..]); -#[tokio::test] -/// Tests that `send_response` correctly propagates I/O errors encountered during writing. -/// -/// This test uses a writer that always fails on write operations and asserts that -/// the resulting error is of the `Io` variant. async fn send_response_propagates_write_error() { let app = WireframeApp::new() .unwrap() From 63995ff77e261f8f1811c9eca9e9676d51939651 Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 20 Jun 2025 17:09:19 +0100 Subject: [PATCH 5/5] Fix syntax errors and update tests (#91) --- src/app.rs | 8 ++--- src/frame.rs | 88 ++++++++++++++++++++++++++++++++--------------- tests/response.rs | 61 +++++++++++++------------------- 3 files changed, 89 insertions(+), 68 deletions(-) diff --git a/src/app.rs b/src/app.rs index abb674ea..fd7cd6fb 100644 --- a/src/app.rs +++ b/src/app.rs @@ -17,7 +17,7 @@ use bytes::BytesMut; use tokio::io::{self, AsyncWrite, AsyncWriteExt}; use crate::{ - frame::{FrameProcessor, LengthPrefixedProcessor}, + frame::{FrameProcessor, LengthFormat, LengthPrefixedProcessor}, message::Message, serializer::{BincodeSerializer, Serializer}, }; @@ -147,10 +147,10 @@ where S: Serializer + Default, C: Send + 'static, { - frame_processor: Box::new(LengthPrefixedProcessor::default()), /// - /// Initialises empty routes, services, middleware, and application data. Sets the - /// default frame processor and serializer, with no connection lifecycle hooks. + /// Initialises empty routes, services, middleware, and application data. + /// Sets the default frame processor and serializer, with no connection + /// lifecycle hooks. fn default() -> Self { Self { routes: HashMap::new(), diff --git a/src/frame.rs b/src/frame.rs index e8b531f4..97e31dc5 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -25,35 +25,69 @@ pub struct LengthFormat { } impl LengthFormat { - if bytes.len() < self.bytes { - return Err(io::Error::new( - io::ErrorKind::UnexpectedEof, - "length prefix truncated", - )); - } - if !matches!(self.bytes, 1 | 2 | 4 | 8) { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "unsupported length prefix size", - )); - } + /// Creates a new `LengthFormat` with the specified number of bytes and + /// endianness for the length prefix. + /// + /// # Parameters + /// - `bytes`: The number of bytes used for the length prefix. + /// - `endianness`: The byte order for encoding and decoding the length prefix. + /// + /// # Returns + /// A `LengthFormat` configured with the given size and endianness. + #[must_use] + pub const fn new(bytes: usize, endianness: Endianness) -> Self { Self { bytes, endianness } } - let mut slice = &bytes[..self.bytes]; - let len = match self.endianness { - Endianness::Big => slice.get_uint(self.bytes), - Endianness::Little => slice.get_uint_le(self.bytes), - if !matches!(self.bytes, 1 | 2 | 4 | 8) { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "unsupported length prefix size", - )); - } + /// Creates a `LengthFormat` for a 2-byte big-endian length prefix. + #[must_use] + pub const fn u16_be() -> Self { Self::new(2, Endianness::Big) } + + /// Creates a `LengthFormat` for a 2-byte little-endian length prefix. + #[must_use] + pub const fn u16_le() -> Self { Self::new(2, Endianness::Little) } + + /// Creates a `LengthFormat` for a 4-byte big-endian length prefix. + #[must_use] + pub const fn u32_be() -> Self { Self::new(4, Endianness::Big) } + + /// Creates a `LengthFormat` for a 4-byte little-endian length prefix. + #[must_use] + pub const fn u32_le() -> Self { Self::new(4, Endianness::Little) } - let len_u64 = u64::try_from(len) - .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "frame too large"))?; - match self.endianness { - Endianness::Big => dst.put_uint(len_u64, self.bytes), - Endianness::Little => dst.put_uint_le(len_u64, self.bytes), + /// Reads a length prefix from a byte slice according to the configured prefix size and + /// endianness. + /// + /// # Parameters + /// - `bytes`: The byte slice containing the length prefix. Must be at least as long as the + /// configured prefix size. + /// + /// # Returns + /// The decoded length as a `usize` if successful. + /// + /// # Errors + /// Returns an error if the prefix size is unsupported or if the decoded length does not fit in + /// a `usize`. + fn read_len(&self, bytes: &[u8]) -> io::Result { + let len = match (self.bytes, self.endianness) { + (1, _) => u64::from(u8::from_ne_bytes([bytes[0]])), + (2, Endianness::Big) => u64::from(u16::from_be_bytes([bytes[0], bytes[1]])), + (2, Endianness::Little) => u64::from(u16::from_le_bytes([bytes[0], bytes[1]])), + (4, Endianness::Big) => { + u64::from(u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) + } + (4, Endianness::Little) => { + u64::from(u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) + } + (8, Endianness::Big) => u64::from_be_bytes([ + bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7], + ]), + (8, Endianness::Little) => u64::from_le_bytes([ + bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7], + ]), + _ => { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "unsupported length prefix size", + )); } }; usize::try_from(len).map_err(|_| io::Error::other("frame too large")) diff --git a/tests/response.rs b/tests/response.rs index 7d4fa0dc..069334d0 100644 --- a/tests/response.rs +++ b/tests/response.rs @@ -55,7 +55,8 @@ async fn send_response_encodes_and_frames() { } #[tokio::test] -/// Tests that decoding with an incomplete length prefix header returns `None` and does not consume any bytes from the buffer. +/// Tests that decoding with an incomplete length prefix header returns `None` and does not consume +/// any bytes from the buffer. /// /// This ensures that the decoder waits for the full header before attempting to decode a frame. async fn length_prefixed_decode_requires_complete_header() { @@ -94,6 +95,16 @@ impl tokio::io::AsyncWrite for FailingWriter { _: &mut std::task::Context<'_>, ) -> std::task::Poll> { std::task::Poll::Ready(Ok(())) + } + + fn poll_shutdown( + self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + std::task::Poll::Ready(Ok(())) + } +} + #[rstest] #[case(LengthFormat::u16_be(), vec![1, 2, 3, 4], vec![0x00, 0x04])] #[case(LengthFormat::u32_le(), vec![9, 8, 7], vec![3, 0, 0, 0])] @@ -102,8 +113,15 @@ fn custom_length_roundtrip( #[case] frame: Vec, #[case] prefix: Vec, ) { + let processor = LengthPrefixedProcessor::new(fmt); + let mut buf = BytesMut::new(); + processor.encode(&frame, &mut buf).unwrap(); assert_eq!(&buf[..prefix.len()], &prefix[..]); + let decoded = processor.decode(&mut buf).unwrap().unwrap(); + assert_eq!(decoded, frame); +} +#[tokio::test] async fn send_response_propagates_write_error() { let app = WireframeApp::new() .unwrap() @@ -111,6 +129,11 @@ async fn send_response_propagates_write_error() { let mut writer = FailingWriter; let err = app + .send_response(&mut writer, &TestResp(3)) + .await + .expect_err("expected error"); + assert!(matches!(err, wireframe::app::SendError::Io(_))); +} #[test] fn encode_fails_for_unsupported_prefix_size() { @@ -131,11 +154,6 @@ fn decode_fails_for_unsupported_prefix_size() { let err = processor.decode(&mut buf).expect_err("expected error"); assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); } - .send_response(&mut writer, &TestResp(3)) - .await - .expect_err("expected error"); - assert!(matches!(err, wireframe::app::SendError::Io(_))); -} #[tokio::test] /// Tests that `send_response` returns a serialization error when encoding fails. @@ -150,34 +168,3 @@ async fn send_response_returns_encode_error() { .expect_err("expected error"); assert!(matches!(err, wireframe::app::SendError::Serialize(_))); } - -#[test] -/// Tests roundtrip encoding and decoding of a frame using a two-byte big-endian length prefix. -/// -/// Verifies that a frame encoded with a `LengthPrefixedProcessor` configured for a 2-byte -/// big-endian length format can be correctly decoded back to its original contents. -fn custom_two_byte_big_endian_roundtrip() { - let fmt = LengthFormat::u16_be(); - let processor = LengthPrefixedProcessor::new(fmt); - let frame = vec![1, 2, 3, 4]; - let mut buf = BytesMut::new(); - processor.encode(&frame, &mut buf).unwrap(); - let decoded = processor.decode(&mut buf).unwrap().unwrap(); - assert_eq!(decoded, frame); -} - -#[test] -/// Tests roundtrip encoding and decoding of a frame using a four-byte little-endian length prefix. -/// -/// Verifies that the encoded buffer contains the correct little-endian length prefix and that -/// decoding restores the original frame. -fn custom_four_byte_little_endian_roundtrip() { - let fmt = LengthFormat::u32_le(); - let processor = LengthPrefixedProcessor::new(fmt); - let frame = vec![9, 8, 7]; - let mut buf = BytesMut::new(); - processor.encode(&frame, &mut buf).unwrap(); - assert_eq!(&buf[..4], u32::try_from(frame.len()).unwrap().to_le_bytes()); - let decoded = processor.decode(&mut buf).unwrap().unwrap(); - assert_eq!(decoded, frame); -}