-
Notifications
You must be signed in to change notification settings - Fork 0
Pluggable protocol codecs #413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
f0e8f67
Fix ADR naming scheme
leynos e6ab704
Update documentation style guide
leynos 8080c3b
Add ADR and exec plan for codecs
leynos 38995e2
Add pluggable frame codecs
leynos e450315
Add codec examples and integration test
leynos f702371
Update codec roadmap and ADR findings
leynos d35d0dd
Add serializer abstraction ADR
leynos 4b5f0a2
Gate codec examples behind feature
leynos aba00b3
Gate remaining examples behind feature
leynos 231d678
Clamp frame length and propagate correlation
leynos c3e0548
Refactor builder rebuild helper
leynos 5ec1183
Split oversized modules
leynos ca8d93c
Document correlation placement
leynos 9366515
Add RESP codec example
leynos ae1a418
Harden pluggable codec integration
leynos 9d866cb
Address PR review comments
43ebf36
Refactor builder helpers and address documentation review comments
leynos 8fb4b30
Harden codec examples against DoS and reduce allocations
leynos 78dc96f
Add comma before conjunction in roadmap.md
leynos 8098fa0
Use zero-allocation itoa for RESP bulk string and array lengths
leynos 8b0ed4b
Add unit tests for codec-aware response forwarding
leynos 1547a82
Harden RESP codec and decompose into submodules
leynos 8aacc43
Address PR review comments for codec hardening
leynos 7d22507
Expand RESP parser documentation and support null arrays
leynos 593d452
Add RESP error frame support to example codec
leynos be01daf
Extract parse_text_frame helper to eliminate duplication
leynos 513e564
Introduce ParseContext to reduce argument count in parse helpers
leynos 283efac
Fix bulk frame length check to account for total frame size
leynos 1e1dd59
Fix off-by-one in parse_line length check
leynos File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
File renamed without changes.
File renamed without changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,256 @@ | ||
| # Architectural decision record (ADR) 004: pluggable protocol codecs | ||
|
|
||
| ## Status | ||
|
|
||
| Accepted - 2025-12-30: introduce a `FrameCodec` trait with a default | ||
| length-delimited implementation. | ||
|
|
||
| ## Date | ||
|
|
||
| 2025-12-30. | ||
|
|
||
| ## Context and Problem Statement | ||
|
|
||
| Wireframe currently hardcodes `tokio_util::codec::LengthDelimitedCodec` with a | ||
| 4-byte big-endian length prefix in the connection pipeline (notably in | ||
| `src/app/connection.rs` and `src/app/frame_handling.rs`). This makes framing | ||
| inflexible and prevents protocols with alternative framing rules (such as | ||
| Hotline, MySQL, and Redis Serialization Protocol (RESP) from reusing | ||
| Wireframe's routing, middleware, and serialization infrastructure. | ||
|
|
||
| A pluggable framing layer is required that can: | ||
|
|
||
| - Decode frames with protocol-specific headers. | ||
| - Preserve protocol metadata (transaction IDs, sequence numbers, etc.). | ||
| - Provide correlation identifiers when available. | ||
| - Remain backward compatible for existing users. | ||
|
|
||
| ## Decision Drivers | ||
|
|
||
| - Support non-length-delimited protocols (Hotline, MySQL, Redis Serialization | ||
| Protocol (RESP)). | ||
| - Preserve protocol-specific metadata by using protocol-native frame types. | ||
| - Maintain backward compatibility for current Wireframe users. | ||
| - Keep framing logic encapsulated and testable. | ||
| - Provide guardrails for maximum frame size to reduce denial-of-service risk. | ||
| - Avoid runtime indirection where zero-cost abstractions are feasible. | ||
|
|
||
| ## Requirements | ||
|
|
||
| ### Functional requirements | ||
|
|
||
| - Allow multiple wire protocols to plug in their own framing rules. | ||
| - Preserve protocol metadata in the frame type. | ||
| - Support protocols with and without request/response correlation. | ||
| - Continue to support the current length-delimited framing by default. | ||
|
|
||
| ### Technical requirements | ||
|
|
||
| - Use Tokio codecs for integration with `Framed` streams. | ||
| - Keep framing configuration owned by the codec, not `WireframeApp`. | ||
| - Avoid allocations or copies beyond what framing requires. | ||
| - Remain compatible with the current minimum supported Rust version. | ||
|
|
||
| ## Options Considered | ||
|
|
||
| ### Option A: retain fixed `LengthDelimitedCodec` | ||
|
|
||
| Keep the current hardcoded 4-byte big-endian length-delimited framing. | ||
|
|
||
| ### Option B: introduce a `FrameCodec` trait with a default length-delimited | ||
|
|
||
| implementation (preferred) | ||
|
|
||
| Define a new trait that supplies a decoder, encoder, payload extraction, frame | ||
| wrapping, correlation lookup, and maximum frame length. Provide a default | ||
| `LengthDelimitedFrameCodec` for backward compatibility. | ||
|
|
||
| ### Option C: make `WireframeApp` generic over raw `Decoder`/`Encoder` types or | ||
|
|
||
| use trait objects | ||
|
|
||
| Expose `tokio_util::codec::Decoder` and `Encoder` directly as type parameters | ||
| or boxed trait objects and have `WireframeApp` manage them. | ||
|
|
||
| | Topic | Option A: fixed codec | Option B: FrameCodec trait | Option C: raw decoder/encoder | | ||
| | ---------------------- | --------------------- | -------------------------- | ----------------------------- | | ||
| | Protocol support | None | Broad | Broad | | ||
| | Type safety | Strong | Strong | Varies (trait objects) | | ||
| | Ergonomics | Simple | Simple defaults | Complex API surface | | ||
| | Backward compatibility | Full | Full with defaults | Medium (API changes) | | ||
| | Performance | Stable | Stable (associated types) | Depends on implementation | | ||
|
|
||
| _Table 1: Comparison of framing options._ | ||
|
|
||
| ## Decision Outcome / Proposed Direction | ||
|
|
||
| Adopt Option B: introduce a `FrameCodec` trait with a default length-delimited | ||
| implementation, and make `WireframeApp` generic over a codec type parameter. | ||
|
|
||
| Key elements: | ||
|
|
||
| - `FrameCodec` owns the decoder/encoder configuration, payload extraction, and | ||
| maximum frame length. | ||
| - Frame types are protocol-specific to preserve metadata. | ||
| - The default `LengthDelimitedFrameCodec` uses `Bytes` frames to align with | ||
| `tokio_util::codec::LengthDelimitedCodec` and avoid extra copies. | ||
| - A default `LengthDelimitedFrameCodec` provides compatibility for existing | ||
| users without code changes. | ||
| - `WireframeApp` becomes generic over `F: FrameCodec` with a default type | ||
| parameter, and the builder gains `.with_codec()` to swap codecs. | ||
|
|
||
| For orientation, the trait shape is expected to look like this: | ||
|
|
||
| ```rust,no_run | ||
| use std::io; | ||
| use tokio_util::codec::{Decoder, Encoder}; | ||
|
|
||
| /// Trait for pluggable frame codecs supporting different wire protocols. | ||
| pub trait FrameCodec: Send + Sync + 'static { | ||
| /// Frame type produced by decoding. | ||
| type Frame: Send + Sync + 'static; | ||
| /// Decoder type for this codec. | ||
| type Decoder: Decoder<Item = Self::Frame, Error = io::Error> + Send; | ||
| /// Encoder type for this codec. | ||
| type Encoder: Encoder<Self::Frame, Error = io::Error> + Send; | ||
|
|
||
| /// Create a Tokio Decoder for this codec. | ||
| fn decoder(&self) -> Self::Decoder; | ||
|
|
||
| /// Create a Tokio Encoder for this codec. | ||
| fn encoder(&self) -> Self::Encoder; | ||
|
|
||
| /// Extract payload bytes from a frame. | ||
| fn frame_payload(frame: &Self::Frame) -> &[u8]; | ||
|
|
||
| /// Wrap payload bytes into a frame for sending. | ||
| fn wrap_payload(payload: Vec<u8>) -> Self::Frame; | ||
|
|
||
| /// Extract correlation ID for request/response matching. | ||
| fn correlation_id(_frame: &Self::Frame) -> Option<u64> { None } | ||
|
|
||
| /// Maximum frame length this codec will accept. | ||
| fn max_frame_length(&self) -> usize; | ||
| } | ||
| ``` | ||
|
|
||
| ## Goals and Non-Goals | ||
|
|
||
| ### Goals | ||
|
|
||
| - Allow Wireframe to support Hotline, MySQL, and Redis Serialization Protocol | ||
| (RESP) framing. | ||
| - Preserve protocol metadata in frame types. | ||
| - Maintain the existing API surface for users who rely on length-delimited | ||
| framing. | ||
| - Keep framing logic encapsulated and testable. | ||
|
|
||
| ### Non-Goals | ||
|
|
||
| - Protocol negotiation or runtime codec selection. | ||
| - Replacing the existing message serialization format. | ||
| - Introducing asynchronous codec initialization. | ||
|
|
||
| ## Migration Plan | ||
|
|
||
| ### Phase 1: Trait definition (non-breaking) | ||
|
|
||
| - Create `src/codec.rs` with the `FrameCodec` trait and | ||
| `LengthDelimitedFrameCodec` default. | ||
| - Add unit tests covering the default codec behaviour. | ||
|
|
||
| ### Phase 2: Parameterize `WireframeApp` (breaking change) | ||
|
|
||
| - Introduce `F: FrameCodec = LengthDelimitedFrameCodec` to `WireframeApp`. | ||
| - Add a `codec: F` field and update the default implementation. | ||
| - Provide `.with_codec()` for builder ergonomics. | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| ### Phase 3: Update connection handling | ||
|
|
||
| - Parameterize `FrameHandlingContext` and `ResponseContext` over the codec. | ||
| - Replace `LengthDelimitedCodec` usage with `FrameCodec` decoder/encoder calls. | ||
| - Use `max_frame_length()` for buffer sizing and fragmentation defaults. | ||
|
|
||
| ### Phase 4: Update `WireframeServer` | ||
|
|
||
| - Propagate the codec type parameter through server factory types. | ||
| - Update any connection handling signatures that assume the fixed codec. | ||
|
|
||
| ### Phase 5: Protocol examples | ||
|
|
||
| - Add an example codec for Hotline. | ||
| - Add an example codec for MySQL if time permits. | ||
|
|
||
| ## Implementation Findings | ||
|
|
||
| ### Codec ergonomics and stateful metadata | ||
|
|
||
| - `FrameCodec::wrap_payload` is a static constructor that takes `Vec<u8>`. The | ||
| Hotline and MySQL example codecs therefore stamp default metadata values | ||
| (transaction ID and sequence number set to `0`) unless the caller manually | ||
| constructs a frame and bypasses the default send path. | ||
| - Payload extraction now uses `Bytes` frames to avoid copying on decode, but | ||
| `wrap_payload` still takes `Vec<u8>`, so outbound framing remains | ||
| allocation-bound. | ||
|
|
||
| ### Error handling and recovery | ||
|
|
||
| - The codec surface uses `io::Error`, so protocol-specific framing errors are | ||
| collapsed into generic IO failures. Decode errors currently close the | ||
| connection, leaving no path to recover from malformed frames or to emit a | ||
| protocol-specific error response. | ||
|
|
||
| ### Connection pipeline alignment | ||
|
|
||
| - Codec integration lives in the app connection path, while the `Connection` | ||
| actor retains its own framing logic. This makes protocol hooks and streaming | ||
| behaviour inconsistent across read/write paths. | ||
|
|
||
| ### Fragmentation alignment | ||
|
|
||
| - Fragmentation defaults are tied to `max_frame_length`, but fragmentation is | ||
| still integrated directly into the connection pipeline rather than being a | ||
| pluggable `FragmentAdapter` as described in the fragmentation design. | ||
|
|
||
| ### Mock protocol handler experience (Hotline, MySQL) | ||
|
|
||
| - Both example codecs required manual header parsing and validation, including | ||
| explicit checks for payload length, total frame size, and header layout. | ||
| - Implementing MySQL’s 3-byte little-endian length header required custom | ||
| helpers; there is no shared utility for non-standard integer widths. | ||
| - Clippy’s indexing and truncation lints forced the examples to use `bytes::Buf` | ||
| and explicit `try_from` conversions, suggesting a need for shared parsing | ||
| helpers to reduce boilerplate. | ||
|
|
||
| ## Known Risks and Limitations | ||
|
|
||
| - Associated types avoid RPITIT for `FrameCodec`, but the overall design still | ||
| depends on Rust 1.75+ for return-position `impl Trait` elsewhere in the | ||
| connection stack. | ||
| - The new codec type parameter propagates through public APIs, increasing type | ||
| signatures and potentially affecting downstream type inference. | ||
| - `LengthDelimitedFrameCodec` now returns `Bytes`, so any code requiring owned | ||
| `Vec<u8>` payloads must convert explicitly, reintroducing copies. | ||
|
|
||
| ## Outstanding Decisions | ||
|
|
||
| - Decide whether `FrameCodec::wrap_payload` should accept `&self` (or other | ||
| state) to support codecs that maintain sequence counters or compression | ||
| dictionaries. | ||
| - Decide whether to introduce a `CodecError` taxonomy and recovery policy | ||
| (drop frame versus close connection) for protocol-specific failures. | ||
| - Decide how to realign fragmentation with the `FragmentAdapter` design so | ||
| opt-in behaviour and composition order are explicit. | ||
| - Decide whether to unify codec handling between the app router path and the | ||
| `Connection` actor to ensure protocol hooks run consistently. | ||
| - Future protocol adoption may still require revisiting how correlation | ||
| identifiers are surfaced for FIFO protocols (for example, RESP). | ||
|
|
||
| ## Architectural Rationale | ||
|
|
||
| A dedicated `FrameCodec` abstraction aligns framing with the protocol boundary | ||
| and keeps the routing/middleware pipeline agnostic to wire-level details. It | ||
| preserves existing behaviour through a default codec while enabling protocol | ||
| specificity and reusability. By tying buffer sizing and maximum frame length to | ||
| codec configuration, the design keeps transport-level constraints close to the | ||
| framing rules that define them. | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.