| name | defensive-programming |
| description | Defensive Programming -- Zero-Panic Production Code guidance for Fortress Rollback. Use when Implementing error handling, ensuring zero-panic compliance, validating inputs. |
Defensive Programming -- Zero-Panic Production Code
Zero-Panic Policy (CRITICAL)
Forbidden Patterns
panic!(); value.unwrap(); value.expect("..."); array[index]; todo!(); unimplemented!();
assert!(cond);
unreachable!();
Required Patterns
value.ok_or(FortressError::MissingValue)?;
array.get(index).ok_or(FortressError::IndexOutOfBounds { index, len: array.len() })?;
a.checked_add(b).ok_or(FortressError::ArithmeticOverflow)?;
operation().map_err(|e| FortressError::OperationFailed { cause: e.to_string() })?;
Doc Examples Must Also Follow Zero-Panic
Use ? with # Ok::<(), FortressError>(()) pattern in doc tests.
| Scenario | Pattern |
|---|
| Teaching defensive handling | if let Some(s) = cell.load() { state = s; } |
| Happy path with proven state | .expect("just saved") (with justification) |
| Error propagation | .ok_or(Error::Missing)? |
| General fallible ops | ? operator |
Doc Example Verification
Always verify error variants, struct names, method names exist in source. Match on #[non_exhaustive] enums must include _ => arm.
Never Swallow Errors
let _ = fallible_operation();
let value = operation().unwrap_or_default();
fallible_operation()?;
match fallible_operation() {
Ok(value) => process(value),
Err(Error::NotFound) => DEFAULT,
Err(e) => return Err(e.into()),
}
No Silent Skips on Internal-Invariant Lookups
Internal-invariant collections (input_queues, local_connect_status,
handles, etc.) are sized by construction to cover every valid index. If a
lookup misses, the invariant is broken — that is a library bug, not a
recoverable condition. The fix depends on the function shape:
| Function shape | Required handling |
|---|
Returns Result | .ok_or(FortressError::InternalErrorStructured { kind: ... })? (e.g. IndexOutOfBounds, ConnectionStatusIndexOutOfBounds) |
&mut self, returns () / Frame / etc. | report_violation!(ViolationSeverity::Error, ViolationKind::InputQueue | ::NetworkProtocol, "..."); then take a safe fallback |
Loops for i in 0..self.num_players { if let Some(q) = self.input_queues.get(i) {...} } | Restructure to for q in self.input_queues.iter() { ... } — eliminates the silent-skip pattern by construction |
if let Some(...) = self.collection.get(...) followed by no else arm is a
red flag for invariant-backed collections: it converts a bug into a no-op.
The format string in report_violation! should include the index, the
collection name, and collection.len() so the diagnostic is actionable.
Severity rubric (use the highest applicable tier):
ViolationSeverity::Critical — construction-time configuration breakage
that leaves the structure in a permanently broken state (e.g., the
per-player queue count is short of num_players after a constructor
fallback fails — every subsequent operation that indexes by handle is
unsafe by construction).ViolationSeverity::Error — runtime invariant violation where the call
took a safe fallback or returned a structured error to its caller; the
session as a whole keeps running, but this specific operation reflects a
bug somewhere in the library.ViolationSeverity::Warning — recoverable conditions where the system
corrected itself (clamping, automatic retries, transient gaps that the
caller can ignore).
Input Validation
All public APIs must validate inputs at the boundary:
pub fn set_player_count(&mut self, count: usize) -> Result<(), FortressError> {
if count == 0 || count > MAX_PLAYERS {
return Err(FortressError::InvalidPlayerCount { count, reason: "out of range" });
}
self.players = vec![Player::default(); count];
Ok(())
}
State Consistency
Operations must succeed completely or leave state unchanged (prepare-then-commit or rollback).
Error Categorization
| Question | Category |
|---|
| Is invalid value from caller's argument? | InvalidRequestStructured |
| Is it from internal library state? | InternalErrorStructured |
Quick test: Can a user following docs correctly trigger this? YES -> InvalidRequest. NO -> InternalError.
Unknown Fallback Variants
Include Unknown variant in error reason enums for safe fallback in mapping functions. Never use existing variants with placeholder values.
Fallible / Infallible Constructor Pairs
When a fallible try_* constructor is wrapped by an infallible new/with_* that reports a violation on error, the try_* layer returns the structured error silently — only the wrapper calls report_violation!. Logging in both layers emits duplicate telemetry for one invalid input. The infallible wrapper should also document its degraded fallback (the exact shape it returns on error) so the behavioral contract is explicit. Pattern source: SavedStates, TimeSync, SyncLayer, InputQueue. Also: when one error path must be preserved (e.g. propagating AllocationFailed), prefer propagating the original FortressError over collapsing every cause into a single opaque variant.
Safe Collection Access
for item in &items { process(item); }
let first = items.first().ok_or(Error::Empty)?;
match items.as_slice() {
[] => Err(Error::Empty),
[only] => process_single(only),
[first, rest @ ..] => { }
}
Advanced Patterns
TryFrom over From for Fallible Conversions
From must never panic. Use TryFrom if conversion can fail.
Safe Numeric Conversions
Never as for lossy conversions. Use i8::try_from(big)?.into() or infallible widening .into().
Avoid ..Default::default()
New fields silently get defaults. Prefer explicit field initialization or destructure-then-override.
Exhaustive Destructuring in Trait Impls
impl PartialEq for PlayerState {
fn eq(&self, other: &Self) -> bool {
let Self { frame, input, checksum } = self;
*frame == other.frame && *input == other.input && *checksum == other.checksum
}
}
#[must_use] on Important Types
#[must_use = "frame advance result contains requests that must be processed"]
pub struct FrameAdvanceResult { }
Temporary Mutability
Shadow to freeze: let config = config; after setup.
Parameter Structs
Replace many params with a config struct for self-documenting call sites.
Recommended Clippy Lints
[lints.clippy]
unwrap_used = "deny"
expect_used = "deny"
panic = "deny"
indexing_slicing = "deny"
arithmetic_side_effects = "deny"
cast_possible_truncation = "deny"
cast_sign_loss = "deny"
fallible_impl_from = "deny"
must_use_candidate = "warn"
Bounded Allocation
The default allocator aborts the process on allocation failure (uncatchable, cf. RUSTSEC-2022-0035). So any Vec::with_capacity(n), vec![x; n], or reserve(n) whose n comes from an unbounded source (a length read from the wire, a user-configurable size) is a DoS/abort vector unless the allocation is made fallible first.
Two rules:
- Never trust a length from the wire. Bound any size derived from network input against a caller-configured limit or explicit default before allocating, and use
checked_add while accumulating so a crafted packet cannot overflow usize. Return a structured error on the bomb path instead of allocating. When a wire length is decoded into a wider type (e.g. a u64 varint), range-check it against the limit while it is still u64, before any as usize narrowing: on a 32-bit target (usize == u32) the cast truncates the high bits and could turn a huge declared length into a small one that slips past the check (see rle::checked_run_len). Example: protocol input decode derives its RLE decoded-length limit from ProtocolConfig::pending_output_limit and the reference input size; standalone rle::decode uses rle::DEFAULT_MAX_DECODED_LEN. Avoid generic serde/bincode container decoding for untrusted bytes when the schema has Vec, String, or similar length-prefixed fields; parse those lengths explicitly, validate them against the actual input/config, reserve fallibly, and then copy/decode the bounded payload.
When bounded bincode decoding still needs a consumed-byte count, use network::codec::decode_bounded_with_consumed; do not point peer-controlled callers at generic codec::decode.
- Do not invent arbitrary caps for user configuration. Validate true semantic invariants at the boundary (for example
queue_length >= 2 or stream_delay < buffer_size), then use try_reserve*/checked arithmetic at construction so huge but user-requested sizes return a structured allocation error instead of aborting the process.
- A zero size is non-functional, not just abort-prone. A size that is too small can silently break an object without any allocation failure: a zero-length socket receive buffer reads zero bytes forever (silently dropping every datagram and able to spin), and a zero-length send buffer can never encode. Reject a zero size at the boundary with
io::ErrorKind::InvalidInput (or the structured equivalent), and centralize the check in one constructor helper so every entry point inherits it (e.g. network::buffer::zeroed_buffer, used by every socket-buffer constructor). Where an infallible constructor must degrade rather than fail, fall back to a non-empty buffer and guard the consumer against an empty one — never leave it to spin.
Bound the length BEFORE reserving, even when the reserve is fallible
try_reserve / try_reserve_exact prevent the allocator abort, but they do not prevent a huge speculative allocation that succeeds: a try_reserve_exact(len) driven by an attacker-chosen len can still exhaust memory (a DoS) even though it never aborts. So an untrusted length must be bounded against the remaining input bytes BEFORE the reserve, never relying on fallibility alone. The canonical primitive is network::codec::ensure_length_within_remaining(bytes, cursor, len, min_encoded_len, field), which rejects a length prefix whose minimum wire footprint (len * min_encoded_len, via checked_mul) exceeds the unread bytes.
let len = read_len(bytes, &mut cursor)?;
ensure_length_within_remaining(bytes, cursor, len, min_encoded_len, "frames")?;
let mut out = Vec::new();
out.try_reserve_exact(len)
.map_err(|_| allocation_failed("frames", len))?;
Every dynamically-sized infallible allocation in src/ must carry an // alloc-bound: <why> comment (same line or the line above) stating why its size is concretely bounded. Pure integer literals (with_capacity(4), vec![0u8; 16]), exact .len()/.count() sizes, and fallible try_reserve* reservations are exempt. Weak markers such as "trusted local config" or "no numeric cap" are rejected by the hook. Tests/proofs in trailing #[cfg(test)] / #[cfg(kani)] modules are skipped.
let mut queues = Vec::new();
queues.try_reserve_exact(num_players)?;
for player in 0..num_players {
queues.push(InputQueue::try_with_queue_length(player, queue_length)?);
}
Bulk-reserving from an untrusted size_hint
Iterator::size_hint is a performance hint only -- a buggy/adversarial iterator may report an upper bound that is too low, too high, or None. Never trust it for correctness. Use error::try_reserve_hint(&mut vec, upper, per_item) (the canonical helper) to bulk-reserve ONCE before a loop. It is best-effort and infallible: it multiplies with saturating arithmetic, reserves via try_reserve, and silently ignores failure (an over-large or dishonest hint simply leaves the vector unreserved). It never aborts and never returns an error.
Because a failed reservation here is a no-op, it is a pure optimization that cannot change behavior for any iterator (honest or adversarial). The load-bearing, panic-free growth path is the caller's per-iteration fallible try_reserve, which always runs and covers the None/under-reporting/over-large cases identically to pure incremental growth. The reservation is upper * per_item additional capacity (Vec::try_reserve semantics: reserves for len + additional, not an absolute target), so call it on an empty/near-empty vector before the loop — on a partially-filled vector it would under-reserve.
let (_lower, upper) = pending_input.size_hint();
try_reserve_hint(&mut bytes, upper, ref_bytes.len());
for input in pending_input {
let requested = bytes.len().saturating_add(input.len());
let reserved = bytes.try_reserve(input.len());
reserved.map_err(|_| allocation_failed("compression.delta_encode", requested))?;
}
The // reserve-in-loop: marker
A try_reserve / try_reserve_exact whose nearest enclosing block is a loop body (for/while/loop) is flagged by check-unbounded-alloc unless it carries a // reserve-in-loop: <why> justification (same line or the line above). Per-iteration fallible reserves are sometimes correct -- guarding an untrusted/absent size_hint, or allocating one fresh bounded buffer per iteration -- but must be a CONSCIOUS choice, not an accident: prefer a single bulk pre-reservation before the loop where the count is known. A closure or match arm LEXICALLY NESTED within a for/while/loop still counts as in-loop; a fn defined inside a loop body shields its own reserves. impl Trait for Type { is not treated as a loop. Known tested limitations (all defense-in-depth only, all false negative, never false positive; none occurs in src/): (1) a loop whose HEADER embeds a brace block (for x in match v {} {, for x in vec![Foo { a: 1 }] {) is classified as a non-loop body; (2) a closure whose signature embeds a bare fn-pointer TYPE (|cb: fn()| { ... }) is conservatively classified as a fn body, so an in-loop reserve inside it is shielded; (3) the iteration-IS-the-closure idiom (items.iter().for_each(|x| { ... }), .map(|x| { ... }).collect()) is NOT flagged — "closure inside a loop counts as in-loop" means a closure lexically nested in a for/while/loop, NOT an iterator-adapter closure that performs the iteration itself.
Symmetric teardown and single-send sites
When the same logical operation runs from more than one path, the set of steps must not diverge between paths — the classic bug is one abort path that cleans up less than another (e.g. a hot-join serve aborted on timeout cleared the joiner endpoint's pending_output, but the abort-on-disconnect path forgot to). Funnel every abort/teardown path through one helper that performs the complete cleanup, so a new caller cannot under-clean by omission (P2PSession::abort_hot_join_serve is the canonical example; both the Phase-4 timeout and the reserved-slot disconnect call it). The success path is deliberately separate when it must keep state the abort path discards.
Mirror this for retransmits: a "send once now, then resend each poll" shape double-sends on the triggering poll if the just-opened item is already in the resend set. Keep a single send site (the per-poll resend loop) and let the open step only enqueue — then each item is transmitted exactly once per poll, and poll-count/timeout accounting (>=, not >, for an inclusive "max N polls" bound) stays exact. The same applies to ack resends on the receiving side: one ack-send site per poll, never a resend loop plus a duplicate-arrival handler that both fire.
Make these bounds (serve timeout, ack-resend budget) configurable builder DEFAULTS, validated at the boundary (reject a 0 timeout that would abort before any handshake completes). Test per-poll send counts deterministically with a counting socket that tallies sends around each poll_remote_clients and asserts the delta is <= 1 (see tests/sessions/hot_join.rs CountingSocket).
Checklist
