diff --git a/CHANGELOG.md b/CHANGELOG.md index 23ea4abcb..2fa4a36bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,17 @@ All notable changes to this project will be documented in this file. built. See `docs/v0-3-0-to-v0-4-0-migration-guide.md` for migration examples. (#642) +- Breaking: `MessageAssemblyState` now enforces + `FirstFrameHeader::total_body_len` as an authoritative declaration. A first + frame whose body exceeds its declared total is rejected with + `DeclaredTotalTooSmall`; a continuation that would overshoot the declaration + is rejected with `DeclaredTotalExceeded`; and a completed body whose length + differs from the declaration is rejected with `DeclaredTotalMismatch`. + Protocol implementations must supply exact totals or leave `total_body_len` as + `None`. Downstream exhaustive matches on `MessageAssemblyError` need arms + for the three new variants. See + [ADR 015](docs/adr-015-declared-total-body-length-contract.md). + ([#544](https://github.com/leynos/wireframe/issues/544)) - Added scheduler and slot poison-recovery coverage, and extracted `scheduler_tests.rs` to keep `scheduler.rs` below 400 lines. (#539) - Added client-pool lifecycle and slot-rotation coverage that kills the diff --git a/docs/adr-015-declared-total-body-length-contract.md b/docs/adr-015-declared-total-body-length-contract.md new file mode 100644 index 000000000..52f4c9023 --- /dev/null +++ b/docs/adr-015-declared-total-body-length-contract.md @@ -0,0 +1,188 @@ +# Architectural decision record (ADR) 015: declared total body length is authoritative + +## Status + +Accepted. + +`FirstFrameHeader::total_body_len` is authoritative: the assembler rejects a +declaration it can already contradict, stops a continuation from overshooting +it, and requires the completed body to match it exactly. The contract is +implemented by issue [#544](https://github.com/leynos/wireframe/issues/544). + +## Date + +2026-10-18. + +## Context and problem statement + +`FirstFrameHeader::total_body_len` lets a protocol declare the size of a +logical message on its first frame. `MessageSeries` stores the declaration as +`expected_total`, and `MessageAssemblyState` uses it to reject messages that +cannot fit the configured size limit. + +The declaration was only a budget input. The completion path removed the +partial assembly and dispatched its body without comparing the accumulated +length to the declaration, and a single-frame message was returned before any +comparison at all. A peer could therefore declare any total that passed the +size check, send a different number of bytes, and have the body dispatched +unchanged. The name `total_body_len` promised a guarantee the runtime did not +make, which is a validation weakness (CWE-20) and an inconsistent length +interpretation (CWE-130). + +The formal verification guide frames the choice between an authoritative +declaration and an advisory hint in its section on +[whether `total_body_len` is authoritative or advisory](formal-verification-methods-in-wireframe.md#2-is-total_body_len-authoritative-or-advisory). + +## Decision drivers + +- Every caller in this repository that supplies a total already expects it to + be exact. +- The size limit is only meaningful if the declared value bounds the bytes that + are actually buffered and dispatched. +- The domain, not the inbound adapter, owns message-assembly invariants. +- An over- or under-declaration must be rejected before an unbounded amount of + peer-controlled data is retained. + +## Options considered + +### Option A: authoritative declaration + +Reject any message whose assembled body length differs from the declaration. +Enforce before buffering as well as at completion. + +### Option B: advisory hint + +Rename the field and tolerate mismatch. + +### Option C: enforce only at completion + +Keep buffering unrestricted and compare once the final frame arrives. + +| Topic | Option A | Option B | Option C | +| ------------------ | ----------------------- | ------------- | ---------------------- | +| Preserves the name | Yes | No (renamed) | Yes | +| Bounds buffering | Yes, before each append | No | No | +| Bypass of limit | Closed | Remains open | Closed after buffering | +| Caller impact | Exact totals required | Silent change | Exact totals required | + +_Table 1: Trade-offs between the considered options._ + +## Decision outcome + +Option A is adopted. `total_body_len` is a protocol commitment. Four +enforcement points apply, all in domain code: + +| Stage | Rejected condition | Error | +| ----------------------- | --------------------------------------------- | ----------------------- | +| First frame | `total_body_len < body_len` | `DeclaredTotalTooSmall` | +| First frame, final | `total_body_len != body_len` | `DeclaredTotalMismatch` | +| Continuation | accumulated body would exceed the declaration | `DeclaredTotalExceeded` | +| Completing continuation | accumulated body differs from the declaration | `DeclaredTotalMismatch` | + +_Table 2: Declared-total enforcement points._ + +- The first-frame check runs before the size-limit check. A declaration that + the first frame already contradicts is rejected as malformed rather than + measured against a limit that cannot make it valid. +- A continuation is checked before its bytes are buffered, so an oversized + frame is never retained, even briefly. +- A rejected continuation or completion removes the partial assembly. The + failure is reported through `MessageAssemblyError`, which the inbound adapter + maps to `io::ErrorKind::InvalidData` through its existing failure tracker. +- Protocols that leave `total_body_len` as `None` are unaffected; they retain + the previous behaviour with no equality requirement. + +For screen readers: the following flow chart traces a first frame through its +declaration check, a continuation through its admission check, and a completed +message through the final comparison, showing where each rejection frees the +partial assembly. + +```mermaid +flowchart TD + A[Receive first frame] --> B[validate_first_frame] + B -->|declared total smaller than body| E1[Reject: DeclaredTotalTooSmall] + B -->|single-frame total differs| E2[Reject: DeclaredTotalMismatch] + B -->|valid multi-frame declaration| C[Buffer first-frame body] + C --> D[Receive continuation] + D --> F[validate_continuation] + F -->|would exceed declaration or overflow| E3[Reject: DeclaredTotalExceeded] + F -->|within declaration| G[Buffer continuation before completion] + G --> H{Message complete?} + H -->|no| D + H -->|yes| I[validate_completion] + I -->|assembled length differs| E4[Reject: DeclaredTotalMismatch] + I -->|exact declared length| J[Dispatch assembled message] + E1 --> K[Free partial assembly] + E2 --> K + E3 --> K + E4 --> K +``` + +_Figure 1: Declared-total enforcement from the first frame to dispatch._ + +The checks live in `src/message_assembler/validation.rs`, next to the state +machine that applies them. `PartialAssembly` owns the completion check so the +buffered length and the declaration are read together. + +## Consequences + +### Positive + +- When the first frame declares a total, the dispatched body length always + equals that declaration. A frame that leaves `total_body_len` as `None` opts + out, so equality is guaranteed only for declared messages. +- An under-declared message can no longer bypass an application-configured size + limit. +- Failures are typed per enforcement point, so diagnostics distinguish an + unsatisfiable first frame from a completion mismatch. + +### Negative + +- A protocol that declared an approximate total will now see its messages + rejected. No protocol in this repository does so. +- `MessageAssemblyError` gains three variants. The enum is not + `#[non_exhaustive]`, so downstream exhaustive matches need updating. + +## Rejected shortcuts + +- Do not rename `total_body_len` to make the current behaviour sound: the + validation weakness would remain. +- Do not enforce only in the inbound adapter: another adapter would still see + the unsafe domain contract. +- Do not compare only at completion: a peer could force the runtime to buffer + a body up to the configured message-size limit before the mismatch is + detected, rather than rejecting the overshooting frame when it arrives. +- Do not return the mismatched body with a warning: dispatch is the security + boundary. + +## Migration plan + +1. Supply an exact `total_body_len`, or leave it as `None` to opt out of the + equality requirement. +2. Update exhaustive matches on `MessageAssemblyError` with the new variants. +3. Update behavioural expectations that asserted a mismatched body was + dispatched. + +## Verification + +- Unit tests cover under-declared single-frame totals, over-declared + single-frame totals, an over-declared continuation, a final frame short of + the declaration, exact equality, and the `None` opt-out. +- Property tests in `src/message_assembler/declared_total_property_tests.rs` + quantify over arbitrary `usize` lengths. They pin acceptance against the + remaining room, reject unrepresentable sums near `usize::MAX` as + `DeclaredTotalExceeded`, and drive the public state machine across generated + multi-frame partitions, asserting that an overshooting frame is never + buffered and that only an exact final total dispatches. +- `tests/ui/message_assembly_error_exhaustive_match.rs` pins the compile-time + break caused by the three new variants: a match naming only the pre-change + variants fails to compile, as it does for `ClientError`. +- Behavioural tests in `tests/features/message_assembly.feature` exercise the + same paths through the public API. +- `make check-fmt`, `make lint`, and `make test` pass. + +## References + +- [Issue #544](https://github.com/leynos/wireframe/issues/544) +- [ADR 002: streaming requests and shared message assembly](adr-002-streaming-requests-and-shared-message-assembly.md) +- [Formal verification methods in Wireframe](formal-verification-methods-in-wireframe.md) diff --git a/docs/contents.md b/docs/contents.md index 7fd811bb4..0a864db26 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -65,6 +65,10 @@ reference when navigating the project's design and architecture material. client pool. - [ADR 014: server shutdown ownership and terminal observation](adr-014-server-shutdown-ownership.md) Proposed awaitable server shutdown and supervisor-failure observation. +- [ADR 015: declared total body length is authoritative][adr-015] Accepted + contract for enforcing `total_body_len` during message assembly. + +[adr-015]: adr-015-declared-total-body-length-contract.md ## Execution plans diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 2cfcfa9ec..1c63f5e0f 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -614,6 +614,51 @@ duplicate/gap/out-of-order validation; and `advance_sequence_or_overflow()` owns the overflow decision. Each piece is independently testable and the decision tree stays flat and readable during future refactors. +### Declared total body length architecture + +`FirstFrameHeader::total_body_len` is authoritative, following ADR 015. The +contract is enforced in domain code so every adapter inherits it, and the work +is split across four modules in `src/message_assembler/`: + +- `validation.rs` holds the pure checks. `validate_first_frame`, + `validate_continuation`, and `validate_completion` take lengths and return a + `MessageAssemblyError`; they hold no state and perform no I/O, so each + boundary is testable without an assembly. +- `partial.rs` owns an in-flight `PartialAssembly`. It keeps the completion + check next to the buffered bytes it compares, so the accumulated length and + the declaration are read together. +- `state.rs` owns admission and routing. It stops growing past its module size + budget by delegating the length arithmetic to `validation`. +- `budget.rs` holds `ContinuationLimits`, which bundles the per-message cap + with the aggregate `AggregateBudgets` so a caller cannot pair the cap with + the wrong budgets. + +The four enforcement points: + +| Boundary | Rejected condition | Variant | +| ----------------------- | --------------------------------------------- | ----------------------- | +| First frame | declaration smaller than the frame's body | `DeclaredTotalTooSmall` | +| First frame, final | body differs from the declaration | `DeclaredTotalMismatch` | +| Continuation | accumulated body would exceed the declaration | `DeclaredTotalExceeded` | +| Completing continuation | accumulated body differs from the declaration | `DeclaredTotalMismatch` | + +`validate_continuation` runs before the bytes are buffered, so a frame that +would overshoot is never retained, even briefly. A rejected continuation or +completion removes the partial assembly, so a failed message cannot linger +under its key. The inbound adapter in `src/app/frame_handling/assembly.rs` +translates the resulting `MessageAssemblyError` into +`io::ErrorKind::InvalidData` through `DeserFailureTracker`; the domain never +sees `io::Error`. + +Overflow is part of the contract, not an incidental guard. The accumulator uses +`usize::checked_add`, and an unrepresentable sum is reported as +`DeclaredTotalExceeded` with `attempted: usize::MAX` rather than wrapping to a +small value that would appear to fit. `declared_total_property_tests.rs` pins +that boundary over values near `usize::MAX`, and +`tests/ui/message_assembly_error_exhaustive_match.rs` pins the compile-time +break caused by the three new variants, matching the existing `ClientError` +fixture. + ### `fill_buf_with_prefix` The private helper `fill_buf_with_prefix(buf, prefix, endianness)` in diff --git a/docs/formal-verification-methods-in-wireframe.md b/docs/formal-verification-methods-in-wireframe.md index 39456a604..a48daf867 100644 --- a/docs/formal-verification-methods-in-wireframe.md +++ b/docs/formal-verification-methods-in-wireframe.md @@ -131,19 +131,15 @@ This is the best candidate for the **strongest** verification investment. `expected_total`, and `MessageAssemblyState` uses the declared total for the initial size-limit check.[^22][^23][^24] -However, on completion, the current `accept_continuation_frame_at` path removes -the partial assembly and returns the assembled message without an explicit -final equality check between the accumulated body length and -`expected_total`.[^24] +The completion path now enforces an explicit final equality check between the +accumulated body length and `expected_total`, and the first-frame and +continuation paths reject declarations that cannot be honoured.[^24] The +contract is recorded as authoritative in +[ADR 015](adr-015-declared-total-body-length-contract.md), following Position A +below. -That is either: - -- a deliberate design choice, in which case the name `total_body_len` - overstates the guarantee, or -- a missing invariant, in which case it deserves both runtime enforcement and - verification. - -This is where Verus becomes plausible, but only **after** a contract decision. +This is where Verus becomes plausible, now that the contract decision is +settled. ### Fourth priority: `src/connection/*` @@ -200,18 +196,20 @@ verify, and closer to what the implementation already does. ### 2. Is `total_body_len` authoritative or advisory? -The current name strongly implies authority, but the completion path does not -enforce it.[^22][^24] +**Resolved:** authoritative, recorded in +[ADR 015](adr-015-declared-total-body-length-contract.md). Completion fails if +the actual body length differs, and the first-frame and continuation paths +reject declarations that cannot be honoured.[^22][^24] -Choose one of these positions: +The alternatives were: - **Position A:** It is authoritative. Completion must fail if the actual body length differs. - **Position B:** It is advisory. Rename it accordingly and document that mismatch is tolerated. -**Position A** is the stronger choice unless an existing protocol reason -requires treating it as a hint. +**Position A** was chosen because no existing protocol requires treating the +declaration as a hint. ### 3. What fairness guarantee does `ConnectionActor` actually make? diff --git a/docs/roadmap.md b/docs/roadmap.md index 4d1f11c39..ac164c22e 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -655,13 +655,15 @@ cited section names the specific question it answers. 15.1.1. See §"What widths does Wireframe actually support for length prefixes?". Success criteria: an ADR records the decision, constructors enforce the chosen set, and existing tests cover rejected widths. -- [ ] 15.2.2. Treat `total_body_len` as either authoritative or advisory and +- [x] 15.2.2. Treat `total_body_len` as either authoritative or advisory and enforce or rename it consistently across the message assembly path; record the decision in an ADR and add tests for both conforming and violating inputs. Requires 15.1.1. See §"Is `total_body_len` authoritative or advisory?". Success criteria: an ADR records the decision, runtime code enforces the chosen semantics, and tests verify both conforming and violating - inputs. + inputs. See [ADR 015](adr-015-declared-total-body-length-contract.md); issue + [#544](https://github.com/leynos/wireframe/issues/544) added conformance and + violation coverage. - [ ] 15.2.3. Publish named fairness and priority guarantees for `ConnectionActor` and encode them as model properties for Stateright checks. Requires 15.1.1. See §"What fairness guarantee does `ConnectionActor` @@ -717,13 +719,14 @@ cited section names the specific question it answers. ### 15.5. Verus proofs for message assembly -- [ ] 15.5.1. Enforce the chosen `total_body_len` contract in runtime code +- [x] 15.5.1. Enforce the chosen `total_body_len` contract in runtime code before relying on proofs. See [formal verification guide §"Is `total_body_len` authoritative or advisory?"](formal-verification-methods-in-wireframe.md#2-is-total_body_len-authoritative-or-advisory) and [§"What Verus should prove in Wireframe"](formal-verification-methods-in-wireframe.md#what-verus-should-prove-in-wireframe). Requires 15.2.2. Success criteria: runtime assertions or checks enforce the - contract, and existing tests confirm the enforcement. + contract, and existing tests confirm the enforcement. See + [ADR 015](adr-015-declared-total-body-length-contract.md). - [ ] 15.5.2. Add proof-only modules under `verus/` for declared-total and buffered-byte accounting invariants. See [formal verification guide §Proof style recommendation](formal-verification-methods-in-wireframe.md#proof-style-recommendation) diff --git a/docs/users-guide.md b/docs/users-guide.md index 5006bc74f..36c7ca6cd 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -1282,6 +1282,33 @@ assert!(matches!( )); ``` +#### Declared total body length + +`FirstFrameHeader::total_body_len` is an authoritative declaration, not a hint. +When a protocol supplies it, the assembler enforces it at every boundary: + +- a first frame that carries more body bytes than it declares is rejected with + `MessageAssemblyError::DeclaredTotalTooSmall`, because its declaration can + never be satisfied; +- a single-frame message whose body differs from its declaration is rejected + with `MessageAssemblyError::DeclaredTotalMismatch`; +- a continuation that would push the accumulated body past the declaration is + rejected with `MessageAssemblyError::DeclaredTotalExceeded` before its bytes + are buffered; and +- the frame that completes the message must bring the accumulated body to + exactly the declared total, or `MessageAssemblyError::DeclaredTotalMismatch` + is returned. + +A rejected continuation or a completion mismatch also removes its partial +assembly, so a mismatched message never remains buffered and is never +dispatched. A rejected first frame is refused before any assembly exists, so +there is nothing to remove. Protocols that leave `total_body_len` as `None` opt +out of the equality requirement and keep the previous behaviour. + +The declaration is also used for the size-limit check, so an exact total is the +cheapest way to fail early on an oversized message. The decision is recorded in +[ADR 015](adr-015-declared-total-body-length-contract.md). + ### Streaming request body consumption Handlers can opt into streaming request bodies using the `StreamingBody` diff --git a/docs/v0-3-0-to-v0-4-0-migration-guide.md b/docs/v0-3-0-to-v0-4-0-migration-guide.md index 68710c289..193f8579a 100644 --- a/docs/v0-3-0-to-v0-4-0-migration-guide.md +++ b/docs/v0-3-0-to-v0-4-0-migration-guide.md @@ -49,6 +49,42 @@ that inspect rejection rates can use the optional `wireframe_decode_rejections_total` counter, labelled by the bounded decode path and rejection reason. +## Declared total body length is authoritative + +`FirstFrameHeader::total_body_len` is now a protocol commitment. Message +assembly rejects a first frame that already contradicts its declaration, stops +a continuation from overshooting it, and requires the completed body to match +it exactly. See [ADR 015](adr-015-declared-total-body-length-contract.md) for +the decision. + +Applications that declare a total must supply the exact body length. A protocol +that declares an approximate size will now see its messages rejected because +the length actually assembled can no longer differ from the length promised. +Leaving `total_body_len` as `None` opts out of the equality requirement and +keeps the previous behaviour, so that is the migration path for a protocol +whose totals were only ever advisory. + +`MessageAssemblyError` gains three variants: `DeclaredTotalTooSmall`, +`DeclaredTotalExceeded`, and `DeclaredTotalMismatch`. The enum is not +`#[non_exhaustive]`, so an exhaustive match written against the previous +variants fails to compile. Add the new arms: + +```rust +use wireframe::message_assembler::MessageAssemblyError; + +fn is_retryable(error: MessageAssemblyError) -> bool { + match error { + MessageAssemblyError::DeclaredTotalTooSmall { .. } + | MessageAssemblyError::DeclaredTotalExceeded { .. } + | MessageAssemblyError::DeclaredTotalMismatch { .. } => false, + _ => true, + } +} +``` + +The inbound adapter maps all three to `io::ErrorKind::InvalidData` and counts a +deserialization failure, so a rejected message never reaches a handler. + ## Prepared application transition `WireframeApp` remains the mutable builder. Register routes, middleware, diff --git a/src/app/frame_handling/assembly_tests/declared_total.rs b/src/app/frame_handling/assembly_tests/declared_total.rs new file mode 100644 index 000000000..ceb59c8fc --- /dev/null +++ b/src/app/frame_handling/assembly_tests/declared_total.rs @@ -0,0 +1,109 @@ +//! Inbound-adapter coverage for declared-total enforcement (issue #544). +//! +//! The domain owns the declared-total checks, but the observable contract is +//! that the inbound adapter converts a rejection into `InvalidData`, counts a +//! deserialization failure, and never dispatches the message. These tests pin +//! that end-to-end behaviour. + +use std::sync::Arc; + +use rstest::rstest; + +use super::{ + inbound_envelope, + load_message_assembly_state, + message_assembler, + message_assembly_state, + ok, + process_assembly_frame, +}; +use crate::{ + message_assembler::{FrameSequence, MessageAssemblyState, MessageKey}, + test_helpers, +}; + +#[rstest] +fn inbound_assembly_rejects_under_declared_single_frame( + message_assembler: Arc, + message_assembly_state: Result, +) { + let mut state = Some(load_message_assembly_state(message_assembly_state)); + let mut deser_failures = 0_u32; + // Declares 3 bytes but carries 5 on the only frame. + let frame = inbound_envelope( + 1, + ok( + test_helpers::first_frame_payload(MessageKey::from(5), b"hello", true, Some(3)), + "valid test payload", + ), + ); + + let result = ok( + process_assembly_frame(&message_assembler, &mut state, &mut deser_failures, frame), + "malformed declaration should be handled", + ); + + assert!( + result.is_none(), + "a declaration the first frame contradicts must not dispatch" + ); + assert_eq!( + deser_failures, 1, + "declared-total rejection should count as one failure" + ); +} + +#[rstest] +fn inbound_assembly_rejects_continuation_overshooting_declared_total( + message_assembler: Arc, + message_assembly_state: Result, +) { + let mut state = Some(load_message_assembly_state(message_assembly_state)); + let mut deser_failures = 0_u32; + let first = inbound_envelope( + 1, + ok( + test_helpers::first_frame_payload(MessageKey::from(5), b"hello", false, Some(8)), + "valid test payload", + ), + ); + let result = ok( + process_assembly_frame(&message_assembler, &mut state, &mut deser_failures, first), + "first frame should process", + ); + assert!(result.is_none(), "first frame should not complete"); + + // 5 + 4 exceeds the declared 8. + let overrun = inbound_envelope( + 1, + ok( + test_helpers::continuation_frame_payload( + MessageKey::from(5), + FrameSequence::from(1), + b"more", + false, + ), + "valid test payload", + ), + ); + let result = ok( + process_assembly_frame(&message_assembler, &mut state, &mut deser_failures, overrun), + "overshooting continuation should be handled", + ); + + assert!( + result.is_none(), + "an overshooting continuation must not dispatch" + ); + assert_eq!( + deser_failures, 1, + "declared-total rejection should count as one failure" + ); + assert_eq!( + state + .as_ref() + .map_or(usize::MAX, MessageAssemblyState::buffered_count), + 0, + "the rejected assembly must be freed" + ); +} diff --git a/src/app/frame_handling/assembly_tests/mod.rs b/src/app/frame_handling/assembly_tests/mod.rs index 33dad6b46..0fbfaae82 100644 --- a/src/app/frame_handling/assembly_tests/mod.rs +++ b/src/app/frame_handling/assembly_tests/mod.rs @@ -354,5 +354,6 @@ fn inbound_assembly_timeout_purges_partial_state( ); } +mod declared_total; mod pass_through; mod routing_metadata; diff --git a/src/message_assembler/budget.rs b/src/message_assembler/budget.rs index 876845869..28a7b6a5a 100644 --- a/src/message_assembler/budget.rs +++ b/src/message_assembler/budget.rs @@ -32,6 +32,19 @@ impl AggregateBudgets { } } +/// Every limit that governs whether a continuation body may be buffered. +/// +/// Bundled so admission receives one value rather than a growing list of +/// parameters, and so a caller cannot pair the per-message cap with the wrong +/// aggregate budgets. +#[derive(Clone, Copy, Debug)] +pub(super) struct ContinuationLimits { + /// Per-message body cap. + pub(super) max_message_size: NonZeroUsize, + /// Aggregate budgets across in-flight assemblies. + pub(super) budgets: AggregateBudgets, +} + /// Check whether accepting `additional_bytes` for `key` would exceed /// the connection budget or in-flight budget. /// diff --git a/src/message_assembler/declared_total_property_tests.rs b/src/message_assembler/declared_total_property_tests.rs new file mode 100644 index 000000000..631a7166a --- /dev/null +++ b/src/message_assembler/declared_total_property_tests.rs @@ -0,0 +1,306 @@ +//! Property tests for declared total body length enforcement (issue #544). +//! +//! The example-based cases in `declared_total_tests.rs` name the protocol +//! outcomes that matter. These properties cover the open-ended space between +//! them: every validator is a pure function of `usize` values, so the boundary +//! around `usize::MAX` — where a naive sum wraps — is reachable by values +//! rather than only by hand-picked examples. +//! +//! The model-based property drives the public state machine and compares every +//! outcome against an independent reimplementation of the contract, so a +//! regression in admission, buffering, or cleanup fails with the partition +//! that provoked it. + +use std::{num::NonZeroUsize, time::Duration}; + +use proptest::prelude::*; + +use crate::message_assembler::{ + AssembledMessage, + ContinuationFrameHeader, + EnvelopeRouting, + FirstFrameHeader, + FirstFrameInput, + FrameSequence, + MessageAssemblyError, + MessageAssemblyState, + MessageKey, + validation, +}; + +/// Values clustered at the top of the `usize` range, where addition overflows. +/// +/// A uniform generator almost never lands within a few bytes of `usize::MAX`, +/// which is exactly where `accumulated + body_len` stops being representable. +fn near_max() -> impl Strategy { + (0usize..=3).prop_map(|offset| usize::MAX - offset) +} + +/// A first-frame header for key `1` declaring `total`. +fn header_with_total(total: usize, body_len: usize) -> FirstFrameHeader { + FirstFrameHeader { + message_key: MessageKey(1), + metadata_len: 0, + body_len, + total_body_len: Some(total), + is_last: false, + } +} + +/// State with a limit large enough that the declaration is decisive. +fn subject() -> MessageAssemblyState { + MessageAssemblyState::new( + NonZeroUsize::new(1_024).unwrap_or(NonZeroUsize::MIN), + Duration::from_secs(30), + ) +} + +/// The outcome the contract requires for one continuation. +#[derive(Debug)] +enum Expected { + /// The frame fits and does not complete the message. + Buffered, + /// The frame fits, completes the message, and dispatches the body. + Dispatched, + /// The frame would overrun the declaration. + Exceeded, + /// The frame completes the message at the wrong length. + Mismatch, +} + +/// Reimplement the contract independently of the production helpers. +fn expected_outcome( + declared: usize, + accumulated: usize, + body_len: usize, + is_last: bool, +) -> Expected { + let Some(attempted) = accumulated.checked_add(body_len) else { + return Expected::Exceeded; + }; + if attempted > declared { + return Expected::Exceeded; + } + if is_last { + return if attempted == declared { + Expected::Dispatched + } else { + Expected::Mismatch + }; + } + Expected::Buffered +} + +proptest! { + /// A first frame is accepted exactly when its declaration is satisfiable. + /// + /// Satisfiable means the declaration covers the frame's own body, and — + /// when the frame also completes the message — equals that body exactly. + /// The error variant distinguishes the two ways of failing. + #[test] + fn first_frame_accepts_only_satisfiable_declarations( + body_len in 0usize..=1_000, + total in 0usize..=1_000, + is_last in any::(), + ) { + let header = FirstFrameHeader { + is_last, + ..header_with_total(total, body_len) + }; + + let outcome = validation::validate_first_frame(&header, body_len); + + let is_satisfiable = total >= body_len && (!is_last || total == body_len); + prop_assert_eq!(outcome.is_ok(), is_satisfiable); + + match outcome { + Ok(()) => {} + Err(MessageAssemblyError::DeclaredTotalTooSmall { + declared, + body_len: carried, + .. + }) => { + prop_assert!(total < body_len); + prop_assert_eq!(declared, total); + prop_assert_eq!(carried, body_len); + } + Err(MessageAssemblyError::DeclaredTotalMismatch { declared, actual, .. }) => { + prop_assert!(is_last); + prop_assert!(total >= body_len); + prop_assert_eq!(declared, total); + prop_assert_eq!(actual, body_len); + } + Err(other) => prop_assert!(false, "unexpected first-frame error: {other:?}"), + } + } + + /// A continuation is admitted exactly when its bytes fit the remaining room. + /// + /// On rejection the attempted length is the true sum, except that an + /// unrepresentable sum is reported as `usize::MAX` rather than wrapping to + /// a small value that would appear to fit. + #[test] + fn continuation_admission_matches_available_room( + declared in 0usize..=1_000, + accumulated in 0usize..=1_000, + body_len in 0usize..=1_000, + ) { + let key = MessageKey(1); + let outcome = validation::validate_continuation(key, declared, accumulated, body_len); + + let fits = match accumulated.checked_add(body_len) { + Some(attempted) => attempted <= declared, + None => false, + }; + prop_assert_eq!(outcome.is_ok(), fits); + + if let Err(MessageAssemblyError::DeclaredTotalExceeded { attempted, .. }) = outcome { + prop_assert_eq!(attempted, accumulated + body_len); + prop_assert!(attempted > declared); + } + } + + /// Addition that `usize` cannot represent is rejected, never wrapped. + /// + /// This is the region a uniform generator misses and the only place where + /// a wrapping implementation would silently accept an overrun. + #[test] + fn unrepresentable_sums_are_rejected_as_exceeded( + declared in near_max(), + accumulated in near_max(), + body_len in near_max(), + ) { + let outcome = + validation::validate_continuation(MessageKey(1), declared, accumulated, body_len); + + match accumulated.checked_add(body_len) { + Some(attempted) => prop_assert_eq!(outcome.is_ok(), attempted <= declared), + None => { + prop_assert_eq!( + outcome, + Err(MessageAssemblyError::DeclaredTotalExceeded { + key: MessageKey(1), + declared, + attempted: usize::MAX, + }) + ); + } + } + } + + /// Completion accepts exactly the declared length. + /// + /// Both directions matter: a short body is the under-declared message the + /// contract exists to stop, and a long body is an overrun that reached + /// completion despite continuation admission. + #[test] + fn completion_accepts_only_an_exact_match( + declared in any::(), + actual in any::(), + ) { + prop_assert_eq!( + validation::validate_completion(MessageKey(1), declared, actual).is_ok(), + declared == actual, + ); + } +} + +/// Feed one continuation and report what the state machine did. +fn feed_continuation( + state: &mut MessageAssemblyState, + sequence: u32, + body: &[u8], + is_last: bool, +) -> Result, MessageAssemblyError> { + let header = ContinuationFrameHeader { + message_key: MessageKey(1), + sequence: Some(FrameSequence(sequence)), + body_len: body.len(), + is_last, + }; + state.accept_continuation_frame(&header, body) +} + +proptest! { + #![proptest_config(ProptestConfig { + cases: 256, + ..ProptestConfig::default() + })] + + /// The state machine matches an independent model of the contract for an + /// arbitrary partition of a message across a first frame and up to three + /// continuations, including partitions that fall short or overshoot. + #[test] + fn partitioned_frames_match_the_declared_total_model( + declared in 0usize..=64, + first_body in 0usize..=64, + continuations in proptest::collection::vec(0usize..=64, 0..3), + ) { + prop_assume!(first_body <= declared); + + let first_bytes = vec![b'a'; first_body]; + let mut state = subject(); + + let header = header_with_total(declared, first_body); + let input = FirstFrameInput::new( + &header, + EnvelopeRouting::default(), + vec![], + &first_bytes, + ) + .expect("header lengths match the body"); + state.accept_first_frame(input).expect("satisfiable first frame"); + + let mut assembled = first_bytes; + let mut terminated = false; + for (index, body_len) in continuations.iter().copied().enumerate() { + let is_last = index + 1 == continuations.len(); + let body = vec![b'b'; body_len]; + let expected = expected_outcome(declared, assembled.len(), body_len, is_last); + + let result = feed_continuation( + &mut state, + u32::try_from(index).expect("fewer than u32::MAX frames"), + &body, + is_last, + ); + + match expected { + Expected::Buffered => { + prop_assert_eq!(result, Ok(None)); + assembled.extend_from_slice(&body); + continue; + } + Expected::Dispatched => { + assembled.extend_from_slice(&body); + let message = result + .expect("exact completion is accepted") + .expect("a completing frame dispatches"); + prop_assert_eq!(message.body(), assembled.as_slice()); + } + Expected::Exceeded => { + let was_exceeded = + matches!(result, Err(MessageAssemblyError::DeclaredTotalExceeded { .. })); + prop_assert!(was_exceeded, "expected DeclaredTotalExceeded, got {result:?}"); + } + Expected::Mismatch => { + let was_mismatch = + matches!(result, Err(MessageAssemblyError::DeclaredTotalMismatch { .. })); + prop_assert!(was_mismatch, "expected DeclaredTotalMismatch, got {result:?}"); + } + } + + terminated = true; + break; + } + + // A rejected continuation or completion frees the assembly; otherwise + // the message is still in flight under its key. + let expected_buffered = usize::from(!terminated); + prop_assert_eq!( + state.buffered_count(), + expected_buffered, + "the partial assembly must be freed exactly when a frame terminated it" + ); + } +} diff --git a/src/message_assembler/declared_total_tests.rs b/src/message_assembler/declared_total_tests.rs new file mode 100644 index 000000000..4006b1a6d --- /dev/null +++ b/src/message_assembler/declared_total_tests.rs @@ -0,0 +1,327 @@ +//! Regression tests for declared total body length enforcement (issue #544). +//! +//! A first frame may declare the total body length of its logical message. +//! That declaration is a protocol commitment: it must agree with the bytes +//! that are actually assembled, and a message that cannot honour it must be +//! rejected rather than dispatched. These tests pin every enforcement point. + +use std::{num::NonZeroUsize, time::Duration}; + +use rstest::{fixture, rstest}; + +use crate::message_assembler::{ + AssembledMessage, + ContinuationFrameHeader, + EnvelopeRouting, + FirstFrameHeader, + FirstFrameInput, + FrameSequence, + MessageAssemblyError, + MessageAssemblyState, + MessageKey, + test_helpers::{first_header, first_header_with_total}, + validation, +}; + +/// State with a limit large enough that declared-total checks are decisive. +#[fixture] +fn subject() -> MessageAssemblyState { + let size = NonZeroUsize::new(1024).unwrap_or(NonZeroUsize::MIN); + MessageAssemblyState::new(size, Duration::from_secs(30)) +} + +/// Build a first-frame header carrying `total` and the requested final flag. +fn first_with_total(key: u64, body_len: usize, total: usize, is_last: bool) -> FirstFrameHeader { + FirstFrameHeader { + is_last, + ..first_header_with_total(key, body_len, total) + } +} + +/// Feed a first frame and return the state-machine result. +fn accept_first( + state: &mut MessageAssemblyState, + header: &FirstFrameHeader, + body: &[u8], +) -> Result, MessageAssemblyError> { + let input = FirstFrameInput::new(header, EnvelopeRouting::default(), vec![], body) + .expect("header lengths match body"); + state.accept_first_frame(input) +} + +/// A sequenced continuation frame to feed to the state machine. +#[derive(Clone, Copy, Debug)] +struct Continuation<'a> { + key: u64, + sequence: u32, + body: &'a [u8], + is_last: bool, +} + +/// Feed a continuation frame and return the state-machine result. +fn accept_continuation( + state: &mut MessageAssemblyState, + frame: Continuation<'_>, +) -> Result, MessageAssemblyError> { + let header = ContinuationFrameHeader { + message_key: MessageKey(frame.key), + sequence: Some(FrameSequence(frame.sequence)), + body_len: frame.body.len(), + is_last: frame.is_last, + }; + state.accept_continuation_frame(&header, frame.body) +} + +// ============================================================================= +// Single-frame declarations +// ============================================================================= + +/// Every way a first frame can contradict its own declaration. +/// +/// The three cases differ only in the declaration, the final-frame flag, and +/// the error the contradiction produces, so they share one body and one set of +/// post-conditions: rejection with the expected error and no buffered state. +#[rstest] +#[case::under_declared_final(3, true, MessageAssemblyError::DeclaredTotalTooSmall { + key: MessageKey(1), + declared: 3, + body_len: 5, +})] +#[case::over_declared_final(10, true, MessageAssemblyError::DeclaredTotalMismatch { + key: MessageKey(1), + declared: 10, + actual: 5, +})] +#[case::under_declared_non_final(4, false, MessageAssemblyError::DeclaredTotalTooSmall { + key: MessageKey(1), + declared: 4, + body_len: 5, +})] +fn first_frame_contradicting_declaration_is_rejected( + #[from(subject)] mut subject: MessageAssemblyState, + #[case] total: usize, + #[case] is_last: bool, + #[case] expected: MessageAssemblyError, +) { + let header = first_with_total(1, 5, total, is_last); + + let err = accept_first(&mut subject, &header, b"hello").expect_err("declaration rejected"); + + assert_eq!(err, expected); + assert_eq!(subject.buffered_count(), 0); +} + +#[rstest] +fn single_frame_exact_declaration_is_dispatched( + #[from(subject)] mut subject: MessageAssemblyState, +) { + let header = first_with_total(1, 5, 5, true); + + let msg = accept_first(&mut subject, &header, b"hello") + .expect("exact declaration accepted") + .expect("single frame completes immediately"); + + assert_eq!(msg.body(), b"hello"); + assert_eq!(subject.buffered_count(), 0); +} + +// ============================================================================= +// Multi-frame declarations +// ============================================================================= + +#[rstest] +fn continuation_exceeding_declared_total_is_rejected( + #[from(subject)] mut subject: MessageAssemblyState, +) { + let header = first_with_total(1, 5, 8, false); + accept_first(&mut subject, &header, b"hello").expect("first frame accepted"); + assert_eq!(subject.buffered_count(), 1); + + // 5 + 4 = 9 exceeds the declared 8. + let frame = Continuation { + key: 1, + sequence: 1, + body: b"more", + is_last: false, + }; + let err = accept_continuation(&mut subject, frame).expect_err("declaration exceeded"); + + assert_eq!( + err, + MessageAssemblyError::DeclaredTotalExceeded { + key: MessageKey(1), + declared: 8, + attempted: 9, + } + ); + assert_eq!( + subject.buffered_count(), + 0, + "the assembly must be freed after overshooting its declaration" + ); +} + +#[rstest] +fn final_frame_short_of_declared_total_is_rejected( + #[from(subject)] mut subject: MessageAssemblyState, +) { + let header = first_with_total(1, 5, 10, false); + accept_first(&mut subject, &header, b"hello").expect("first frame accepted"); + + // 5 + 4 = 9, within the declaration, but the series completes here. + let err = accept_continuation( + &mut subject, + Continuation { + key: 1, + sequence: 1, + body: b"more", + is_last: true, + }, + ) + .expect_err("final frame mismatch"); + + assert_eq!( + err, + MessageAssemblyError::DeclaredTotalMismatch { + key: MessageKey(1), + declared: 10, + actual: 9, + } + ); + assert_eq!( + subject.buffered_count(), + 0, + "the assembly must be freed after a completion mismatch" + ); +} + +/// A completing frame dispatches the assembled body whether the protocol +/// declared an exact total or declared nothing at all. +/// +/// The two cases share one body: the only difference is whether the +/// declaration exists for the completion check to enforce. +#[rstest] +#[case::exact_declaration(Some(10), "world", "helloworld")] +#[case::undeclared_total(None, "worlds", "helloworlds")] +fn completing_frame_dispatches_the_assembled_body( + #[from(subject)] mut subject: MessageAssemblyState, + #[case] declared: Option, + #[case] final_body: &str, + #[case] expected: &str, +) { + let header = FirstFrameHeader { + total_body_len: declared, + ..first_header(1, 5, false) + }; + accept_first(&mut subject, &header, b"hello").expect("first frame accepted"); + + let msg = accept_continuation( + &mut subject, + Continuation { + key: 1, + sequence: 1, + body: final_body.as_bytes(), + is_last: true, + }, + ) + .expect("body honours the declaration") + .expect("message completes"); + + assert_eq!(msg.body(), expected.as_bytes()); + assert_eq!(subject.buffered_count(), 0); +} + +#[rstest] +fn non_final_frame_may_exactly_reach_the_declaration( + #[from(subject)] mut subject: MessageAssemblyState, +) { + let header = first_with_total(1, 5, 10, false); + accept_first(&mut subject, &header, b"hello").expect("first frame accepted"); + + // Exactly reaches the declaration without completing. + let msg = accept_continuation( + &mut subject, + Continuation { + key: 1, + sequence: 1, + body: b"world", + is_last: false, + }, + ) + .expect("equals the declaration, so accepted"); + assert!(msg.is_none()); + assert_eq!(subject.buffered_count(), 1); + + // No bytes remain, so the completing frame must be rejected. + let err = accept_continuation( + &mut subject, + Continuation { + key: 1, + sequence: 2, + body: b"!", + is_last: true, + }, + ) + .expect_err("no room remains within the declaration"); + + assert_eq!( + err, + MessageAssemblyError::DeclaredTotalExceeded { + key: MessageKey(1), + declared: 10, + attempted: 11, + } + ); + assert_eq!(subject.buffered_count(), 0); +} + +#[rstest] +fn declared_total_equal_to_first_frame_body_leaves_no_room( + #[from(subject)] mut subject: MessageAssemblyState, +) { + // The boundary case: declared == body_len with more frames expected. + let header = first_with_total(1, 5, 5, false); + accept_first(&mut subject, &header, b"hello").expect("boundary declaration accepted"); + + let err = accept_continuation( + &mut subject, + Continuation { + key: 1, + sequence: 1, + body: b"!", + is_last: true, + }, + ) + .expect_err("no bytes remain within the declaration"); + + assert_eq!( + err, + MessageAssemblyError::DeclaredTotalExceeded { + key: MessageKey(1), + declared: 5, + attempted: 6, + } + ); + assert_eq!(subject.buffered_count(), 0); +} + +// ============================================================================= +// Pure helper boundaries +// ============================================================================= + +/// The accumulator helper must report overflow as an exceeded declaration +/// rather than wrapping to a small value that appears to fit. +#[test] +fn continuation_accumulation_overflow_is_reported_as_exceeded() { + let err = validation::validate_continuation(MessageKey(1), usize::MAX, usize::MAX, 1) + .expect_err("sum overflows usize"); + + assert_eq!( + err, + MessageAssemblyError::DeclaredTotalExceeded { + key: MessageKey(1), + declared: usize::MAX, + attempted: usize::MAX, + } + ); +} diff --git a/src/message_assembler/error.rs b/src/message_assembler/error.rs index 86b5c33db..4c0fe7fbc 100644 --- a/src/message_assembler/error.rs +++ b/src/message_assembler/error.rs @@ -129,4 +129,50 @@ pub enum MessageAssemblyError { /// Configured in-flight byte cap. limit: NonZeroUsize, }, + + /// The first frame declared fewer total body bytes than it carried. + /// + /// A declaration smaller than the first frame's own body can never be + /// satisfied, and accepting it would let a peer understate the size of the + /// message that is actually dispatched. + #[error( + "declared total body length too small for key {key}: declared {declared} bytes, first \ + frame carries {body_len}" + )] + DeclaredTotalTooSmall { + /// Message key whose declaration is unsatisfiable. + key: MessageKey, + /// Total body length declared by the first frame. + declared: usize, + /// Body bytes actually carried by the first frame. + body_len: usize, + }, + + /// Appending this frame would carry the body past its declared total. + #[error( + "declared total body length exceeded for key {key}: declared {declared} bytes, \ + accumulated {attempted}" + )] + DeclaredTotalExceeded { + /// Message key whose declaration was exceeded. + key: MessageKey, + /// Total body length declared by the first frame. + declared: usize, + /// Body bytes that would result from accepting the frame. + attempted: usize, + }, + + /// The completed message's body length differed from its declared total. + #[error( + "declared total body length mismatch for key {key}: declared {declared} bytes, assembled \ + {actual}" + )] + DeclaredTotalMismatch { + /// Message key whose declaration was not honoured. + key: MessageKey, + /// Total body length declared by the first frame. + declared: usize, + /// Body bytes actually assembled. + actual: usize, + }, } diff --git a/src/message_assembler/mod.rs b/src/message_assembler/mod.rs index d6362f27b..ce5198bd9 100644 --- a/src/message_assembler/mod.rs +++ b/src/message_assembler/mod.rs @@ -23,9 +23,11 @@ mod budget; pub mod error; mod header; +mod partial; pub mod series; pub mod state; pub mod types; +mod validation; use std::io; diff --git a/src/message_assembler/partial.rs b/src/message_assembler/partial.rs new file mode 100644 index 000000000..0879ce5f5 --- /dev/null +++ b/src/message_assembler/partial.rs @@ -0,0 +1,127 @@ +//! In-flight message assembly state. +//! +//! [`PartialAssembly`] holds the bytes and routing metadata accumulated +//! between a first frame and the frame that completes the message. Keeping +//! this state in its own module lets [`MessageAssemblyState`] concentrate on +//! admission decisions while the completion check lives next to the data it +//! validates. +//! +//! [`MessageAssemblyState`]: super::state::MessageAssemblyState + +use std::time::Instant; + +use super::{ + ContinuationFrameHeader, + MessageKey, + budget::{ContinuationLimits, check_aggregate_budgets, check_size_limit}, + error::{MessageAssemblyError, MessageSeriesError, MessageSeriesStatus}, + series::MessageSeries, + types::{AssembledMessage, EnvelopeRouting, FirstFrameInput}, + validation, +}; + +/// Partial message assembly in progress. +#[derive(Debug)] +pub(super) struct PartialAssembly { + /// Ordering state that rejects gaps, duplicates, and post-completion data. + series: MessageSeries, + /// Envelope routing retained until the assembled message is emitted. + routing: EnvelopeRouting, + /// First-frame metadata kept separate from the body for reconstruction. + metadata: Vec, + /// Ordered body bytes accumulated from first and continuation frames. + body: Vec, + /// Start time used to discard incomplete peer-controlled state. + pub(super) started_at: Instant, +} + +impl PartialAssembly { + /// Start a new assembly from an admitted first frame. + /// + /// The input's header has already been validated against its payloads and + /// its declared total, so this only stores the components. + pub(super) fn from_first_frame(input: FirstFrameInput<'_>, started_at: Instant) -> Self { + let series = MessageSeries::from_first_frame(input.header); + Self { + series, + routing: input.routing, + metadata: input.metadata, + body: input.body.to_vec(), + started_at, + } + } + + /// Body bytes buffered so far, excluding metadata. + pub(super) const fn body_len(&self) -> usize { self.body.len() } + + /// Declared total body length, if the first frame supplied one. + pub(super) fn declared_total(&self) -> Option { self.series.expected_total() } + + /// Validate continuity and report whether the series is now complete. + pub(super) fn accept_continuation( + &mut self, + header: &ContinuationFrameHeader, + ) -> Result { + self.series.accept_continuation(header) + } + + /// Apply size, budget, and declared-total admission checks to a + /// continuation body before it is buffered. + /// + /// # Errors + /// + /// Returns [`MessageAssemblyError::MessageTooLarge`] or an aggregate + /// budget error when the frame cannot be retained, and + /// [`MessageAssemblyError::DeclaredTotalExceeded`] when it would push the + /// accumulated body past the declared total. The caller frees the + /// assembly on any error. + pub(super) fn admit_continuation( + &self, + body_len: usize, + buffered_total: usize, + limits: ContinuationLimits, + ) -> Result<(), MessageAssemblyError> { + let key = self.series.message_key(); + let accumulated = self.body_len(); + check_size_limit(limits.max_message_size, key, accumulated, body_len)?; + check_aggregate_budgets(key, buffered_total, body_len, &limits.budgets)?; + + // Enforce the declared total before buffering so an oversized + // continuation cannot be retained even briefly. + if let Some(declared) = self.declared_total() { + validation::validate_continuation(key, declared, accumulated, body_len)?; + } + Ok(()) + } + + /// Append an admitted continuation body. + pub(super) fn push_body(&mut self, body: &[u8]) { self.body.extend_from_slice(body); } + + /// Bytes retained for this assembly, including metadata. + pub(super) const fn buffered_bytes(&self) -> usize { + self.metadata.len().saturating_add(self.body.len()) + } + + /// Consume the assembly, enforcing any declared total. + /// + /// The partial state is always freed. A mismatch is reported instead of + /// dispatching a body whose length contradicts the peer's declaration. + /// + /// # Errors + /// + /// Returns [`MessageAssemblyError::DeclaredTotalMismatch`] when the + /// assembled body length differs from the declared total. + pub(super) fn into_message(self) -> Result, MessageAssemblyError> { + let key: MessageKey = self.series.message_key(); + if let Some(declared) = self.series.expected_total() { + validation::validate_completion(key, declared, self.body.len())?; + } + + Ok(Some(AssembledMessage::new( + key, + self.routing, + self.metadata, + self.body, + ))) + } +} diff --git a/src/message_assembler/state.rs b/src/message_assembler/state.rs index ff68d8e02..7128b549e 100644 --- a/src/message_assembler/state.rs +++ b/src/message_assembler/state.rs @@ -13,46 +13,13 @@ use std::{ use super::{ ContinuationFrameHeader, MessageKey, - budget::{AggregateBudgets, check_aggregate_budgets, check_size_limit}, + budget::{AggregateBudgets, ContinuationLimits, check_aggregate_budgets}, error::{MessageAssemblyError, MessageSeriesError, MessageSeriesStatus}, - series::MessageSeries, - types::{AssembledMessage, EnvelopeRouting, FirstFrameInput}, + partial::PartialAssembly, + types::{AssembledMessage, FirstFrameInput}, + validation, }; -/// Partial message assembly in progress. -#[derive(Debug)] -struct PartialAssembly { - /// Ordering state that rejects gaps, duplicates, and post-completion data. - series: MessageSeries, - /// Envelope routing retained until the assembled message is emitted. - routing: EnvelopeRouting, - /// First-frame metadata kept separate from the body for reconstruction. - metadata: Vec, - /// Ordered body bytes accumulated from first and continuation frames. - body: Vec, - /// Start time used to discard incomplete peer-controlled state. - started_at: Instant, -} -impl PartialAssembly { - /// Create empty assembly storage anchored at the first frame's timestamp. - const fn new(series: MessageSeries, routing: EnvelopeRouting, started_at: Instant) -> Self { - Self { - series, - routing, - metadata: Vec::new(), - body: Vec::new(), - started_at, - } - } - /// Append body bytes after the caller has validated size and ordering. - fn push_body(&mut self, data: &[u8]) { self.body.extend_from_slice(data); } - /// Replace the first-frame metadata retained for the completed message. - fn set_metadata(&mut self, data: Vec) { self.metadata = data; } - /// Return body bytes accumulated so far for per-message budget checks. - const fn accumulated_len(&self) -> usize { self.body.len() } - /// Logical payload length represented by the body and metadata buffers. - const fn buffered_bytes(&self) -> usize { self.body.len().saturating_add(self.metadata.len()) } -} /// Stateful manager for multiple concurrent message assemblies. /// /// Tracks in-flight assemblies keyed by [`MessageKey`], applying continuity @@ -165,8 +132,10 @@ impl MessageAssemblyState { /// # Errors /// /// Returns [`MessageAssemblyError::DuplicateFirstFrame`] if an assembly - /// for this key is already in progress, or [`MessageAssemblyError::MessageTooLarge`] - /// if the body exceeds the configured limit. + /// for this key is already in progress, + /// [`MessageAssemblyError::MessageTooLarge`] if the body exceeds the + /// configured limit, or a declared-total error when the declaration is + /// unsatisfiable or not honoured. pub fn accept_first_frame( &mut self, input: FirstFrameInput<'_>, @@ -180,8 +149,10 @@ impl MessageAssemblyState { /// # Errors /// /// Returns [`MessageAssemblyError::DuplicateFirstFrame`] if an assembly - /// for this key is already in progress, or [`MessageAssemblyError::MessageTooLarge`] - /// if the body exceeds the configured limit. + /// for this key is already in progress, + /// [`MessageAssemblyError::MessageTooLarge`] if the body exceeds the + /// configured limit, or a declared-total error when the declaration is + /// unsatisfiable or not honoured. pub fn accept_first_frame_at( &mut self, input: FirstFrameInput<'_>, @@ -195,6 +166,10 @@ impl MessageAssemblyState { return Err(MessageAssemblyError::DuplicateFirstFrame { key }); } + // A declared total is a protocol commitment: reject declarations that + // the first frame already contradicts before consulting size limits. + validation::validate_first_frame(input.header, input.body.len())?; + // Validate message size (prefer declared total body length, include metadata) let declared_body_len = input.header.total_body_len.unwrap_or(input.body.len()); let total_message_size = declared_body_len.saturating_add(input.metadata.len()); @@ -207,8 +182,6 @@ impl MessageAssemblyState { }); } - let series = MessageSeries::from_first_frame(input.header); - // If this is a single-frame message, return immediately if input.header.is_last { return Ok(Some(AssembledMessage::new( @@ -234,9 +207,7 @@ impl MessageAssemblyState { // Start new assembly, preserving envelope routing metadata from the // first frame so the completed message is dispatched correctly. - let mut partial = PartialAssembly::new(series, input.routing, now); - partial.set_metadata(input.metadata); - partial.push_body(input.body); + let partial = PartialAssembly::from_first_frame(input, now); self.assemblies.insert(key, partial); Ok(None) @@ -249,7 +220,11 @@ impl MessageAssemblyState { /// # Errors /// /// Returns an error if no assembly exists for this key, if continuity - /// validation fails, or if the size limit would be exceeded. + /// validation fails, or if the size limit would be exceeded. Returns + /// [`MessageAssemblyError::DeclaredTotalExceeded`] when the frame would + /// carry the body past its declared total, or + /// [`MessageAssemblyError::DeclaredTotalMismatch`] when the completing + /// frame leaves the body short of the declaration. pub fn accept_continuation_frame( &mut self, header: &ContinuationFrameHeader, @@ -282,7 +257,11 @@ impl MessageAssemblyState { /// # Errors /// /// Returns an error if no assembly exists for this key, if continuity - /// validation fails, or if the size limit would be exceeded. + /// validation fails, or if the size limit would be exceeded. Returns + /// [`MessageAssemblyError::DeclaredTotalExceeded`] when the frame would + /// carry the body past its declared total, or + /// [`MessageAssemblyError::DeclaredTotalMismatch`] when the completing + /// frame leaves the body short of the declaration. pub fn accept_continuation_frame_at( &mut self, header: &ContinuationFrameHeader, @@ -306,9 +285,11 @@ impl MessageAssemblyState { // Snapshot budget state before the mutable entry borrow. The O(n) // scan is gated on is_enabled() so it's skipped when budgets are absent. - let max_message_size = self.max_message_size; - let budgets = self.budgets; - let buffered_total = if budgets.is_enabled() { + let limits = ContinuationLimits { + max_message_size: self.max_message_size, + budgets: self.budgets, + }; + let buffered_total = if limits.budgets.is_enabled() { self.total_buffered_bytes() } else { 0 @@ -321,7 +302,7 @@ impl MessageAssemblyState { }; // Validate continuity - let status = match entry.get_mut().series.accept_continuation(header) { + let status = match entry.get_mut().accept_continuation(header) { Ok(s) => s, Err(e) => { if Self::is_unrecoverable_continuity_error(&e) { @@ -331,33 +312,17 @@ impl MessageAssemblyState { } }; - // Check size limit - let accumulated = entry.get().accumulated_len(); - if let Err(e) = check_size_limit(max_message_size, key, accumulated, body.len()) { + let partial = entry.get_mut(); + if let Err(e) = partial.admit_continuation(body.len(), buffered_total, limits) { entry.remove(); return Err(e); } + partial.push_body(body); - // Check aggregate budgets - if let Err(e) = check_aggregate_budgets(key, buffered_total, body.len(), &budgets) { - entry.remove(); - return Err(e); - } - - entry.get_mut().push_body(body); - - match status { - MessageSeriesStatus::Incomplete => Ok(None), - MessageSeriesStatus::Complete => { - let partial = entry.remove(); - Ok(Some(AssembledMessage::new( - key, - partial.routing, - partial.metadata, - partial.body, - ))) - } - } + let MessageSeriesStatus::Complete = status else { + return Ok(None); + }; + entry.remove().into_message() } /// Remove any partial assemblies that exceeded the configured timeout. diff --git a/src/message_assembler/tests.rs b/src/message_assembler/tests.rs index a045bc5c3..57508d9d1 100644 --- a/src/message_assembler/tests.rs +++ b/src/message_assembler/tests.rs @@ -206,3 +206,17 @@ mod state_tests; #[path = "budget_tests.rs"] mod budget_tests; + +// ============================================================================= +// Declared total enforcement tests (issue #544) - see declared_total_tests.rs +// ============================================================================= + +#[path = "declared_total_tests.rs"] +mod declared_total_tests; + +// ============================================================================= +// Declared total properties (issue #544) - see declared_total_property_tests.rs +// ============================================================================= + +#[path = "declared_total_property_tests.rs"] +mod declared_total_property_tests; diff --git a/src/message_assembler/validation.rs b/src/message_assembler/validation.rs new file mode 100644 index 000000000..8314fc887 --- /dev/null +++ b/src/message_assembler/validation.rs @@ -0,0 +1,125 @@ +//! Declared-total validation for message assembly. +//! +//! A protocol may declare the total body length of a logical message on its +//! first frame. That declaration is a commitment, not merely a hint: it is used +//! to reject messages that cannot fit the configured size limit, and it is +//! enforced against the bytes that are actually accumulated. Treating it as +//! advisory would let a peer understate the size of the message that is +//! dispatched (`CWE-20`, `CWE-130`). +//! +//! The helpers here are pure so that the state machine in +//! [`super::state`] can apply them at each frame boundary without growing that +//! module past its size budget. +//! +//! Contract, in full: +//! +//! - a declaration smaller than the first frame's own body can never be satisfied, so the first +//! frame is rejected; +//! - when the first frame is also the last, its body must equal the declaration exactly; +//! - each continuation must keep the accumulated body within the declaration; overshooting is +//! rejected as soon as it is detected; and +//! - the final frame must bring the accumulated body to exactly the declared total. +//! +//! Protocols that supply `None` are unaffected. + +use super::{FirstFrameHeader, MessageKey, error::MessageAssemblyError}; + +/// Validate the declaration carried by a first frame. +/// +/// Checks that the declaration is satisfiable by the first frame alone, and +/// that a single-frame message matches its declaration exactly. +/// +/// # Errors +/// +/// Returns [`MessageAssemblyError::DeclaredTotalTooSmall`] when the declaration +/// is smaller than the first frame's body, or +/// [`MessageAssemblyError::DeclaredTotalMismatch`] when the first frame is also +/// the final frame and its body does not equal the declaration. +pub(super) fn validate_first_frame( + header: &FirstFrameHeader, + body_len: usize, +) -> Result<(), MessageAssemblyError> { + let Some(declared) = header.total_body_len else { + return Ok(()); + }; + + let key = header.message_key; + if declared < body_len { + return Err(MessageAssemblyError::DeclaredTotalTooSmall { + key, + declared, + body_len, + }); + } + + if header.is_last && declared != body_len { + return Err(MessageAssemblyError::DeclaredTotalMismatch { + key, + declared, + actual: body_len, + }); + } + + Ok(()) +} + +/// Validate that appending `body_len` bytes keeps the assembly within its +/// declared total. +/// +/// Called before the bytes are buffered, so a peer cannot use a single +/// oversized continuation to overrun the declaration and only be detected at +/// completion. +/// +/// # Errors +/// +/// Returns [`MessageAssemblyError::DeclaredTotalExceeded`] when the accumulated +/// body would exceed the declared total, including when the addition would +/// overflow `usize`. +pub(super) fn validate_continuation( + key: MessageKey, + declared: usize, + accumulated: usize, + body_len: usize, +) -> Result<(), MessageAssemblyError> { + let Some(attempted) = accumulated.checked_add(body_len) else { + return Err(MessageAssemblyError::DeclaredTotalExceeded { + key, + declared, + attempted: usize::MAX, + }); + }; + + if attempted > declared { + return Err(MessageAssemblyError::DeclaredTotalExceeded { + key, + declared, + attempted, + }); + } + + Ok(()) +} + +/// Validate that a completed assembly matches its declared total exactly. +/// +/// # Errors +/// +/// Returns [`MessageAssemblyError::DeclaredTotalMismatch`] when the assembled +/// body length differs from the declaration. This catches an under-declared +/// multi-frame message, whose shorter-than-declared body would otherwise be +/// dispatched unchanged. +pub(super) fn validate_completion( + key: MessageKey, + declared: usize, + actual: usize, +) -> Result<(), MessageAssemblyError> { + if actual != declared { + return Err(MessageAssemblyError::DeclaredTotalMismatch { + key, + declared, + actual, + }); + } + + Ok(()) +} diff --git a/src/testkit/reassembly/message_error.rs b/src/testkit/reassembly/message_error.rs index a81f35dbe..b52de2d0f 100644 --- a/src/testkit/reassembly/message_error.rs +++ b/src/testkit/reassembly/message_error.rs @@ -49,6 +49,21 @@ pub enum MessageAssemblyErrorExpectation { /// Message key whose frame triggered the budget guard. key: MessageKey, }, + /// Expect a declaration smaller than the first frame's own body. + DeclaredTotalTooSmall { + /// Message key whose declaration is unsatisfiable. + key: MessageKey, + }, + /// Expect a continuation that would overshoot the declared total. + DeclaredTotalExceeded { + /// Message key whose declaration was exceeded. + key: MessageKey, + }, + /// Expect a completed body that differs from the declared total. + DeclaredTotalMismatch { + /// Message key whose declaration was not honoured. + key: MessageKey, + }, } /// Match a structured assembly error against the expected protocol failure. @@ -78,6 +93,15 @@ pub(super) fn matches_message_error( MessageAssemblyErrorExpectation::InFlightBudgetExceeded { key } => { matches_in_flight_budget_exceeded(err, key) } + MessageAssemblyErrorExpectation::DeclaredTotalTooSmall { key } => { + matches_declared_total_too_small(err, key) + } + MessageAssemblyErrorExpectation::DeclaredTotalExceeded { key } => { + matches_declared_total_exceeded(err, key) + } + MessageAssemblyErrorExpectation::DeclaredTotalMismatch { key } => { + matches_declared_total_mismatch(err, key) + } } } @@ -156,3 +180,30 @@ fn matches_in_flight_budget_exceeded(err: &MessageAssemblyError, key: MessageKey if *actual_key == key ) } + +/// Match a first-frame declaration smaller than the frame's own body. +fn matches_declared_total_too_small(err: &MessageAssemblyError, key: MessageKey) -> bool { + matches!( + err, + MessageAssemblyError::DeclaredTotalTooSmall { key: actual_key, .. } + if *actual_key == key + ) +} + +/// Match a continuation that would overshoot the declared total. +fn matches_declared_total_exceeded(err: &MessageAssemblyError, key: MessageKey) -> bool { + matches!( + err, + MessageAssemblyError::DeclaredTotalExceeded { key: actual_key, .. } + if *actual_key == key + ) +} + +/// Match a completed body whose length differs from the declared total. +fn matches_declared_total_mismatch(err: &MessageAssemblyError, key: MessageKey) -> bool { + matches!( + err, + MessageAssemblyError::DeclaredTotalMismatch { key: actual_key, .. } + if *actual_key == key + ) +} diff --git a/tests/compile_error.rs b/tests/compile_error.rs index 4a0eafa9b..81fbc2df4 100644 --- a/tests/compile_error.rs +++ b/tests/compile_error.rs @@ -19,6 +19,7 @@ fn compile_tests() { t.compile_fail("tests/ui/zero_test_budget.rs"); t.pass("tests/ui/client_error_wildcard_match.rs"); t.compile_fail("tests/ui/client_error_exhaustive_match.rs"); + t.compile_fail("tests/ui/message_assembly_error_exhaustive_match.rs"); t.pass("tests/ui/wireframe_testing_exports.rs"); // Trybuild forwards this build's features to the fixture, so the pool // accessors only exist here when `test-support` is enabled. `make test` diff --git a/tests/features/message_assembly.feature b/tests/features/message_assembly.feature index 5c91febe1..2d220314b 100644 --- a/tests/features/message_assembly.feature +++ b/tests/features/message_assembly.feature @@ -69,6 +69,39 @@ Feature: Message assembly multiplexing and continuity validation Then the error is message too large for key 1 And the buffered count is 0 + Scenario: Under-declared single-frame total is rejected + Given a final first frame for key 1 with body "hello" declaring total 3 + When the first frame is accepted + Then the error is declared total too small for key 1 + And the buffered count is 0 + + Scenario: Over-declared single-frame total is rejected + Given a final first frame for key 1 with body "hello" declaring total 9 + When the first frame is accepted + Then the error is declared total mismatch for key 1 + And the buffered count is 0 + + Scenario: Continuation exceeding the declared total is rejected + Given a first frame for key 1 with body "hello" declaring total 8 + When the first frame is accepted + And a continuation for key 1 with body "more" arrives + Then the error is declared total exceeded for key 1 + And the buffered count is 0 + + Scenario: Final frame short of the declared total is rejected + Given a first frame for key 1 with body "hello" declaring total 10 + When the first frame is accepted + And a final continuation for key 1 with sequence 1 and body "more" arrives + Then the error is declared total mismatch for key 1 + And the buffered count is 0 + + Scenario: Exactly declared multi-frame message completes + Given a first frame for key 1 with body "hello" declaring total 10 + When the first frame is accepted + And a final continuation for key 1 with sequence 1 and body "world" arrives + Then the assembly completes with body "helloworld" + And the buffered count is 0 + Scenario: Expired assemblies are purged Given a first frame for key 1 with body "partial" When the first frame is accepted at time T diff --git a/tests/fixtures/budget_enforcement.rs b/tests/fixtures/budget_enforcement.rs index 753699c7b..64288add0 100644 --- a/tests/fixtures/budget_enforcement.rs +++ b/tests/fixtures/budget_enforcement.rs @@ -262,6 +262,9 @@ impl BudgetEnforcementWorld { MessageAssemblyError::InFlightBudgetExceeded { .. } => "InFlightBudgetExceeded", MessageAssemblyError::MessageTooLarge { .. } => "MessageTooLarge", MessageAssemblyError::DuplicateFirstFrame { .. } => "DuplicateFirstFrame", + MessageAssemblyError::DeclaredTotalTooSmall { .. } => "DeclaredTotalTooSmall", + MessageAssemblyError::DeclaredTotalExceeded { .. } => "DeclaredTotalExceeded", + MessageAssemblyError::DeclaredTotalMismatch { .. } => "DeclaredTotalMismatch", MessageAssemblyError::Series(_) => "Series", }; if actual != expected { diff --git a/tests/fixtures/message_assembly.rs b/tests/fixtures/message_assembly.rs index ffeae4609..4e0c42564 100644 --- a/tests/fixtures/message_assembly.rs +++ b/tests/fixtures/message_assembly.rs @@ -140,7 +140,7 @@ impl MessageAssemblyWorld { message_key: params.key, metadata_len: params.metadata.len(), body_len: params.body.len(), - total_body_len: None, + total_body_len: params.total_body_len, is_last: params.is_last, }, metadata: params.metadata, diff --git a/tests/fixtures/message_assembly_params.rs b/tests/fixtures/message_assembly_params.rs index 246b8d783..257669016 100644 --- a/tests/fixtures/message_assembly_params.rs +++ b/tests/fixtures/message_assembly_params.rs @@ -13,6 +13,8 @@ pub struct FirstFrameParams { pub body: Vec, /// Whether this is the final frame. pub is_last: bool, + /// Total body length declared for the logical message, if any. + pub total_body_len: Option, } impl FirstFrameParams { @@ -24,6 +26,7 @@ impl FirstFrameParams { metadata: vec![], body, is_last: false, + total_body_len: None, } } @@ -34,6 +37,13 @@ impl FirstFrameParams { self } + /// Declare the total body length of the logical message. + #[must_use] + pub fn with_total(mut self, total_body_len: usize) -> Self { + self.total_body_len = Some(total_body_len); + self + } + /// Mark as the final frame. #[must_use] pub fn final_frame(mut self) -> Self { diff --git a/tests/reassembly_assertion_helpers.rs b/tests/reassembly_assertion_helpers.rs index 376f8b34b..79cf58ddf 100644 --- a/tests/reassembly_assertion_helpers.rs +++ b/tests/reassembly_assertion_helpers.rs @@ -135,6 +135,36 @@ fn message_assertion_helpers_accept_completed_bodies() { key: MessageKey(8), } )] +#[case( + MessageAssemblyError::DeclaredTotalTooSmall { + key: MessageKey(13), + declared: 2, + body_len: 6, + }, + MessageAssemblyErrorExpectation::DeclaredTotalTooSmall { + key: MessageKey(13), + } +)] +#[case( + MessageAssemblyError::DeclaredTotalExceeded { + key: MessageKey(14), + declared: 10, + attempted: 12, + }, + MessageAssemblyErrorExpectation::DeclaredTotalExceeded { + key: MessageKey(14), + } +)] +#[case( + MessageAssemblyError::DeclaredTotalMismatch { + key: MessageKey(15), + declared: 10, + actual: 8, + }, + MessageAssemblyErrorExpectation::DeclaredTotalMismatch { + key: MessageKey(15), + } +)] fn message_assertion_helpers_match_errors( #[case] error: MessageAssemblyError, #[case] expected: MessageAssemblyErrorExpectation, diff --git a/tests/scenarios/message_assembly_scenarios.rs b/tests/scenarios/message_assembly_scenarios.rs index c475f9177..9e2313b01 100644 --- a/tests/scenarios/message_assembly_scenarios.rs +++ b/tests/scenarios/message_assembly_scenarios.rs @@ -68,6 +68,46 @@ fn message_assembly_too_large(message_assembly_world: MessageAssemblyWorld) { drop(message_assembly_world); } +#[scenario( + path = "tests/features/message_assembly.feature", + name = "Under-declared single-frame total is rejected" +)] +fn message_assembly_under_declared_single_frame(message_assembly_world: MessageAssemblyWorld) { + drop(message_assembly_world); +} + +#[scenario( + path = "tests/features/message_assembly.feature", + name = "Over-declared single-frame total is rejected" +)] +fn message_assembly_over_declared_single_frame(message_assembly_world: MessageAssemblyWorld) { + drop(message_assembly_world); +} + +#[scenario( + path = "tests/features/message_assembly.feature", + name = "Continuation exceeding the declared total is rejected" +)] +fn message_assembly_continuation_over_declared(message_assembly_world: MessageAssemblyWorld) { + drop(message_assembly_world); +} + +#[scenario( + path = "tests/features/message_assembly.feature", + name = "Final frame short of the declared total is rejected" +)] +fn message_assembly_final_frame_short(message_assembly_world: MessageAssemblyWorld) { + drop(message_assembly_world); +} + +#[scenario( + path = "tests/features/message_assembly.feature", + name = "Exactly declared multi-frame message completes" +)] +fn message_assembly_exact_declaration(message_assembly_world: MessageAssemblyWorld) { + drop(message_assembly_world); +} + #[scenario( path = "tests/features/message_assembly.feature", name = "Expired assemblies are purged" diff --git a/tests/steps/message_assembly_steps.rs b/tests/steps/message_assembly_steps.rs index 2ab5028b2..4eeb54510 100644 --- a/tests/steps/message_assembly_steps.rs +++ b/tests/steps/message_assembly_steps.rs @@ -77,6 +77,70 @@ fn given_state( message_assembly_world.create_state(config); } +/// The frame attributes a Gherkin phrase may add beyond key and body. +#[derive(Clone, Debug, Default)] +struct FrameOptions { + metadata: Option>, + total: Option, + is_last: bool, +} + +impl FrameOptions { + /// Attach `metadata` to the frame. + fn metadata(self, metadata: Vec) -> Self { + Self { + metadata: Some(metadata), + ..self + } + } + + /// Declare a total body length. + fn total(self, total: CountParam) -> Self { + Self { + total: Some(total.0), + ..self + } + } + + /// Mark the frame as completing its message. + fn final_frame(self) -> Self { + Self { + is_last: true, + ..self + } + } + + /// Apply these attributes to `params`. + fn apply_to(self, params: FirstFrameParams) -> FirstFrameParams { + let with_metadata = match self.metadata { + Some(metadata) => params.with_metadata(metadata), + None => params, + }; + let with_total = match self.total { + Some(total) => with_metadata.with_total(total), + None => with_metadata, + }; + if self.is_last { + with_total.final_frame() + } else { + with_total + } + } +} + +/// Record the single first frame the scenario is currently describing. +fn record_first_frame( + world: &mut MessageAssemblyWorld, + key: MessageKeyParam, + body: &str, + options: FrameOptions, +) { + world.add_first_frame(options.apply_to(FirstFrameParams::new( + key.to_key(), + body.as_bytes().to_vec(), + ))); +} + #[rustfmt::skip] #[given("a first frame for key {key:MessageKeyParam} with metadata {metadata:string} and body {body:string}")] fn given_first_frame_with_metadata( @@ -85,9 +149,8 @@ fn given_first_frame_with_metadata( metadata: String, body: String, ) { - message_assembly_world.add_first_frame( - FirstFrameParams::new(key.to_key(), body.into_bytes()).with_metadata(metadata.into_bytes()), - ); + let options = FrameOptions::default().metadata(metadata.into_bytes()); + record_first_frame(message_assembly_world, key, &body, options); } #[given("a first frame for key {key:MessageKeyParam} with body {body:string}")] @@ -96,7 +159,31 @@ fn given_first_frame( key: MessageKeyParam, body: String, ) { - message_assembly_world.add_first_frame(FirstFrameParams::new(key.to_key(), body.into_bytes())); + record_first_frame(message_assembly_world, key, &body, FrameOptions::default()); +} + +#[rustfmt::skip] +#[given("a first frame for key {key:MessageKeyParam} with body {body:string} declaring total {total:CountParam}")] +fn given_first_frame_with_total( + message_assembly_world: &mut MessageAssemblyWorld, + key: MessageKeyParam, + body: String, + total: CountParam, +) { + let options = FrameOptions::default().total(total); + record_first_frame(message_assembly_world, key, &body, options); +} + +#[rustfmt::skip] +#[given("a final first frame for key {key:MessageKeyParam} with body {body:string} declaring total {total:CountParam}")] +fn given_final_first_frame_with_total( + message_assembly_world: &mut MessageAssemblyWorld, + key: MessageKeyParam, + body: String, + total: CountParam, +) { + let options = FrameOptions::default().total(total).final_frame(); + record_first_frame(message_assembly_world, key, &body, options); } #[given("a final first frame for key {key:MessageKeyParam} with body {body:string}")] @@ -105,8 +192,8 @@ fn given_final_first_frame( key: MessageKeyParam, body: String, ) { - message_assembly_world - .add_first_frame(FirstFrameParams::new(key.to_key(), body.into_bytes()).final_frame()); + let options = FrameOptions::default().final_frame(); + record_first_frame(message_assembly_world, key, &body, options); } // ============================================================================= @@ -276,6 +363,33 @@ fn then_error_message_too_large( .assert_error(MessageAssemblyErrorExpectation::MessageTooLarge { key: key.to_key() }) } +#[then("the error is declared total too small for key {key:MessageKeyParam}")] +fn then_error_declared_total_too_small( + message_assembly_world: &mut MessageAssemblyWorld, + key: MessageKeyParam, +) -> TestResult { + message_assembly_world + .assert_error(MessageAssemblyErrorExpectation::DeclaredTotalTooSmall { key: key.to_key() }) +} + +#[then("the error is declared total exceeded for key {key:MessageKeyParam}")] +fn then_error_declared_total_exceeded( + message_assembly_world: &mut MessageAssemblyWorld, + key: MessageKeyParam, +) -> TestResult { + message_assembly_world + .assert_error(MessageAssemblyErrorExpectation::DeclaredTotalExceeded { key: key.to_key() }) +} + +#[then("the error is declared total mismatch for key {key:MessageKeyParam}")] +fn then_error_declared_total_mismatch( + message_assembly_world: &mut MessageAssemblyWorld, + key: MessageKeyParam, +) -> TestResult { + message_assembly_world + .assert_error(MessageAssemblyErrorExpectation::DeclaredTotalMismatch { key: key.to_key() }) +} + #[then("key {key:MessageKeyParam} was evicted")] fn then_key_evicted( message_assembly_world: &mut MessageAssemblyWorld, diff --git a/tests/ui/message_assembly_error_exhaustive_match.rs b/tests/ui/message_assembly_error_exhaustive_match.rs new file mode 100644 index 000000000..c30e2bbbd --- /dev/null +++ b/tests/ui/message_assembly_error_exhaustive_match.rs @@ -0,0 +1,22 @@ +//! A downstream match written before ADR 015 no longer compiles. +//! +//! `MessageAssemblyError` is not `#[non_exhaustive]`, so adding the three +//! declared-total variants is a compile-time break. This fixture pins that +//! break: a match that names every variant from before the change fails to +//! compile until the new variants are handled. + +use wireframe::message_assembler::MessageAssemblyError; + +fn describe(error: MessageAssemblyError) -> &'static str { + match error { + MessageAssemblyError::Series(_) => "series", + MessageAssemblyError::DuplicateFirstFrame { .. } => "duplicate first frame", + MessageAssemblyError::MessageTooLarge { .. } => "too large", + MessageAssemblyError::ConnectionBudgetExceeded { .. } => "connection budget", + MessageAssemblyError::InFlightBudgetExceeded { .. } => "in-flight budget", + } +} + +fn main() { + let _ = describe as fn(MessageAssemblyError) -> &'static str; +} diff --git a/tests/ui/message_assembly_error_exhaustive_match.stderr b/tests/ui/message_assembly_error_exhaustive_match.stderr new file mode 100644 index 000000000..ea62065a2 --- /dev/null +++ b/tests/ui/message_assembly_error_exhaustive_match.stderr @@ -0,0 +1,26 @@ +error[E0004]: non-exhaustive patterns: `MessageAssemblyError::DeclaredTotalTooSmall { .. }`, `MessageAssemblyError::DeclaredTotalExceeded { .. }` and `MessageAssemblyError::DeclaredTotalMismatch { .. }` not covered + --> tests/ui/message_assembly_error_exhaustive_match.rs:11:11 + | +11 | match error { + | ^^^^^ patterns `MessageAssemblyError::DeclaredTotalTooSmall { .. }`, `MessageAssemblyError::DeclaredTotalExceeded { .. }` and `MessageAssemblyError::DeclaredTotalMismatch { .. }` not covered + | +note: `MessageAssemblyError` defined here + --> src/message_assembler/error.rs + | + | pub enum MessageAssemblyError { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +... + | DeclaredTotalTooSmall { + | --------------------- not covered +... + | DeclaredTotalExceeded { + | --------------------- not covered +... + | DeclaredTotalMismatch { + | --------------------- not covered + = note: the matched value is of type `MessageAssemblyError` +help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms + | +16 ~ MessageAssemblyError::InFlightBudgetExceeded { .. } => "in-flight budget", +17 ~ MessageAssemblyError::DeclaredTotalTooSmall { .. } | MessageAssemblyError::DeclaredTotalExceeded { .. } | MessageAssemblyError::DeclaredTotalMismatch { .. } => todo!(), + |