feat(server): implement server fill #86

Merged
Lochnair merged 3 commits from feat/server-fill-policy into main 2026-08-14 06:54:16 +00:00
Lochnair commented 2026-08-14 06:01:00 +00:00 (Migrated from github.com)

Summary

Implements effective ServerFill negotiation and payload generation for
irtt-server, replacing the previous unconditional zero-filled reply payload.

  • Recognizes and negotiates the three valid descriptor families: none, rand
    and pattern:HH… (a repeating hexadecimal pattern).
  • Uses pattern:69727474 — the bytes 69 72 74 74, ASCII irtt — as the
    server default and as the fallback for an explicit descriptor it cannot parse.
  • Fills the actual ECHO payload region from the session's effective fill, sized
    from irtt-proto's own layout and echo_packet_len rather than from
    params.length.
  • Keeps the request payload entirely unrelated to the reply payload: the inbound
    tail stays opaque and is never echoed, mixed in, or used to seed anything.
  • Preserves safe zero behavior for no-fill. A none session hands the encoder an
    empty payload and the encoder's existing zero-fill stands, so a reply can never
    disclose residue from another request or client.

The descriptor string remains wire data carried verbatim by irtt-proto; what it
means is server policy and lives in a new crate-private irtt-server::fill
module. FillMode is stored on the Session and parsed exactly once, at open.

Policy choices

  • All valid descriptors are accepted; there is no upstream-style glob
    allow-list.
    The reference server's configurable allow-list (default: rand
    only) is upstream policy rather than an interoperability requirement, and
    refusing valid modes would create avoidable strict-negotiation failures while
    preventing nothing: the payload carries no protocol meaning, a descriptor is
    bounded to 32 wire bytes, a pattern is tiny, a random fill is bounded by the
    existing packet-length policy, and none is zero-filled here.
  • An absent or empty preference keeps its Params representation — absent
    stays absent, empty stays empty — while the session uses the server default
    internally. Writing the default descriptor into the reply would manufacture a
    restriction the client never asked for, which a strict client rightly rejects.
  • An unknown or malformed explicit descriptor is negotiated to the default
    descriptor
    and reported as such in the open reply, because the server really
    did change what was asked for. Clients model this as
    NegotiationRestriction::ServerFillChanged; client negotiation is unchanged.
  • The pattern phase resets per packet rather than reproducing upstream's
    observed cross-packet, cross-session and cross-listener continuity. The clean
    evidence states that payload phase has no protocol significance and that a
    conforming client must not depend on it, so this is interoperability-equivalent
    and buys determinism: no global mutable fill state and no session or listener
    coupling.
  • Both the allow-list and phase choices are server policy, not compatibility
    claims, and are recorded as such in crates/irtt-server/AGENTS.md.
  • A random-source failure zero-fills that one payload and is nonfatal. It
    never becomes a ServerError, never terminates Server::run and never drops
    the session; the reply stays structurally valid and interoperable. The bytes
    exist to vary the payload and are not security state — session token generation
    is untouched and keeps its own draw.

Fill is generated before the reply is encoded, so the packet's HMAC covers the
fill bytes through the ordinary encoder path and no session state commits before
encoding succeeds.

Tests

  • Default/absent preference: reply keeps server_fill == None, payload repeats
    irtt.
  • Explicit empty descriptor (built with the production encoder, which
    ClientConfig correctly refuses to produce): preserved on the wire, default
    fill used.
  • none: returned unchanged, payload region all zero, proven against a request
    carrying nonzero payload bytes.
  • rand: returned unchanged, correct datagram and payload-region length, with
    and without authentication. Random behavior is tested deterministically through
    a small injected draw — propagation of the whole region, the empty-region case,
    and the zero fallback on failure. No probabilistic assertion anywhere.
  • Patterns: table-driven pattern:00, pattern:ff00, pattern:AaBb,
    pattern:69727474 at a payload length that is not a multiple of the pattern.
  • Malformed/unknown fallback table: bogus, RAND, None, Pattern:aabb,
    pattern:, pattern:f, pattern:abc, pattern:zz, pattern:0g and a
    32-byte non-descriptor.
  • Valid descriptors are returned byte-for-byte, including pattern:AaBb rather
    than a case-normalized form.
  • Phase reset: two successive replies on a pattern:010203 session with a
    5-byte region both read 01 02 03 01 02.
  • Session independence: two patterned sessions interleaved on one core.
  • HMAC payload space: one negotiated length, authenticated and unauthenticated;
    the authenticated reply's region is 16 bytes shorter, every byte is filled, and
    the MAC verifies.
  • Server close: the maximum-duration close reply carries the same patterned
    payload as the session's ordinary replies.
  • No-test negotiation for valid, invalid and absent descriptors, creating no
    session and drawing no token.
  • Real UDP: a pattern:deadbeef session driven with the production codecs, and a
    strict/loose client pair showing the fallback surfacing as ServerFillChanged.
  • Default-client regression: the existing AsyncClient integration test now
    asserts the open reply carries no fill descriptor, so strict negotiation keeps
    succeeding for an ordinary client.

Scope exclusions

  • No glob allow-list configuration and no ServerConfig fill fields.
  • No server-side fill CLI knobs; the existing client --sfill option is
    unchanged.
  • No multi-listener work.
  • No socket or runtime changes.
  • No irtt-proto semantics change, no irtt-client/irtt-cli production change.
  • No new upstream research, no upstream execution, and no protocol-evidence edits.

Verification

  • cargo fmt --all -- --check
  • cargo check --workspace --all-targets --all-features --locked
  • cargo test --workspace --all-targets --all-features --locked
  • cargo clippy --workspace --all-targets --all-features --locked -- -D warnings
  • RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps
  • cargo test -p irtt-client --all-features --locked

Manual smoke, in-repo binaries only, against irtt-server --bind 127.0.0.1:2112:

client run result
default (no fill requested), strict completed, 5/5 packets, exit 0
--sfill pattern:aabb, strict completed, exit 0
--sfill rand, strict completed, 3/3 packets
--sfill bogus, strict negotiation refused, exit 1 (expected)
--sfill bogus --loose completed, 3/3 packets

The server stayed alive through all five and was still listening afterwards.

## Summary Implements effective `ServerFill` negotiation and payload generation for `irtt-server`, replacing the previous unconditional zero-filled reply payload. - Recognizes and negotiates the three valid descriptor families: `none`, `rand` and `pattern:HH…` (a repeating hexadecimal pattern). - Uses `pattern:69727474` — the bytes `69 72 74 74`, ASCII `irtt` — as the server default and as the fallback for an explicit descriptor it cannot parse. - Fills the actual ECHO payload region from the session's effective fill, sized from `irtt-proto`'s own layout and `echo_packet_len` rather than from `params.length`. - Keeps the request payload entirely unrelated to the reply payload: the inbound tail stays opaque and is never echoed, mixed in, or used to seed anything. - Preserves safe zero behavior for no-fill. A `none` session hands the encoder an empty payload and the encoder's existing zero-fill stands, so a reply can never disclose residue from another request or client. The descriptor string remains wire data carried verbatim by `irtt-proto`; what it *means* is server policy and lives in a new crate-private `irtt-server::fill` module. `FillMode` is stored on the `Session` and parsed exactly once, at open. ## Policy choices - **All valid descriptors are accepted; there is no upstream-style glob allow-list.** The reference server's configurable allow-list (default: `rand` only) is upstream policy rather than an interoperability requirement, and refusing valid modes would create avoidable strict-negotiation failures while preventing nothing: the payload carries no protocol meaning, a descriptor is bounded to 32 wire bytes, a pattern is tiny, a random fill is bounded by the existing packet-length policy, and `none` is zero-filled here. - **An absent or empty preference keeps its `Params` representation** — absent stays absent, empty stays empty — while the session uses the server default internally. Writing the default descriptor into the reply would manufacture a restriction the client never asked for, which a strict client rightly rejects. - **An unknown or malformed explicit descriptor is negotiated to the default descriptor** and reported as such in the open reply, because the server really did change what was asked for. Clients model this as `NegotiationRestriction::ServerFillChanged`; client negotiation is unchanged. - **The pattern phase resets per packet** rather than reproducing upstream's observed cross-packet, cross-session and cross-listener continuity. The clean evidence states that payload phase has no protocol significance and that a conforming client must not depend on it, so this is interoperability-equivalent and buys determinism: no global mutable fill state and no session or listener coupling. - Both the allow-list and phase choices are server policy, not compatibility claims, and are recorded as such in `crates/irtt-server/AGENTS.md`. - **A random-source failure zero-fills that one payload and is nonfatal.** It never becomes a `ServerError`, never terminates `Server::run` and never drops the session; the reply stays structurally valid and interoperable. The bytes exist to vary the payload and are not security state — session token generation is untouched and keeps its own draw. Fill is generated before the reply is encoded, so the packet's HMAC covers the fill bytes through the ordinary encoder path and no session state commits before encoding succeeds. ## Tests - Default/absent preference: reply keeps `server_fill == None`, payload repeats `irtt`. - Explicit empty descriptor (built with the production encoder, which `ClientConfig` correctly refuses to produce): preserved on the wire, default fill used. - `none`: returned unchanged, payload region all zero, proven against a request carrying nonzero payload bytes. - `rand`: returned unchanged, correct datagram and payload-region length, with and without authentication. Random behavior is tested deterministically through a small injected draw — propagation of the whole region, the empty-region case, and the zero fallback on failure. No probabilistic assertion anywhere. - Patterns: table-driven `pattern:00`, `pattern:ff00`, `pattern:AaBb`, `pattern:69727474` at a payload length that is not a multiple of the pattern. - Malformed/unknown fallback table: `bogus`, `RAND`, `None`, `Pattern:aabb`, `pattern:`, `pattern:f`, `pattern:abc`, `pattern:zz`, `pattern:0g` and a 32-byte non-descriptor. - Valid descriptors are returned byte-for-byte, including `pattern:AaBb` rather than a case-normalized form. - Phase reset: two successive replies on a `pattern:010203` session with a 5-byte region both read `01 02 03 01 02`. - Session independence: two patterned sessions interleaved on one core. - HMAC payload space: one negotiated length, authenticated and unauthenticated; the authenticated reply's region is 16 bytes shorter, every byte is filled, and the MAC verifies. - Server close: the maximum-duration close reply carries the same patterned payload as the session's ordinary replies. - No-test negotiation for valid, invalid and absent descriptors, creating no session and drawing no token. - Real UDP: a `pattern:deadbeef` session driven with the production codecs, and a strict/loose client pair showing the fallback surfacing as `ServerFillChanged`. - Default-client regression: the existing `AsyncClient` integration test now asserts the open reply carries no fill descriptor, so strict negotiation keeps succeeding for an ordinary client. ## Scope exclusions - No glob allow-list configuration and no `ServerConfig` fill fields. - No server-side fill CLI knobs; the existing client `--sfill` option is unchanged. - No multi-listener work. - No socket or runtime changes. - No `irtt-proto` semantics change, no `irtt-client`/`irtt-cli` production change. - No new upstream research, no upstream execution, and no protocol-evidence edits. ## Verification - `cargo fmt --all -- --check` - `cargo check --workspace --all-targets --all-features --locked` - `cargo test --workspace --all-targets --all-features --locked` - `cargo clippy --workspace --all-targets --all-features --locked -- -D warnings` - `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps` - `cargo test -p irtt-client --all-features --locked` Manual smoke, in-repo binaries only, against `irtt-server --bind 127.0.0.1:2112`: | client run | result | |---|---| | default (no fill requested), strict | completed, 5/5 packets, exit 0 | | `--sfill pattern:aabb`, strict | completed, exit 0 | | `--sfill rand`, strict | completed, 3/3 packets | | `--sfill bogus`, strict | negotiation refused, exit 1 (expected) | | `--sfill bogus --loose` | completed, 3/3 packets | The server stayed alive through all five and was still listening afterwards.
chatgpt-codex-connector[bot] (Migrated from github.com) reviewed 2026-08-14 06:06:19 +00:00
chatgpt-codex-connector[bot] (Migrated from github.com) left a comment

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e4239f45c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

### 💡 Codex Review Here are some automated review suggestions for this pull request. **Reviewed commit:** `e4239f45c5` <details> <summary>ℹ️ About Codex in GitHub</summary> <br/> [Your team has set up Codex to review pull requests in this repo](https://chatgpt.com/codex/cloud/settings/general). Reviews are triggered when you - Open a pull request for review - Mark a draft as ready - Comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". </details>
@@ -308,6 +308,13 @@ The client, event stream, machine-readable output, multi-target execution, local
The server library and the server applet are implemented: open negotiation,
chatgpt-codex-connector[bot] (Migrated from github.com) commented 2026-08-14 06:06:19 +00:00

P2 Badge Distinguish no preference from the none descriptor

The phrase “a client that requests none” can be read as requesting the literal none descriptor, contradicting both the preceding sentence and the implementation: none is honored and produces zero-filled payloads, while only an absent or empty preference uses the default pattern. Reword this as “a client that requests no fill preference” so the README does not misstate the wire behavior.

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Distinguish no preference from the `none` descriptor** The phrase “a client that requests none” can be read as requesting the literal `none` descriptor, contradicting both the preceding sentence and the implementation: `none` is honored and produces zero-filled payloads, while only an absent or empty preference uses the default pattern. Reword this as “a client that requests no fill preference” so the README does not misstate the wire behavior. Useful? React with 👍 / 👎.
Lochnair (Migrated from github.com) reviewed 2026-08-14 06:07:50 +00:00
@@ -308,6 +308,13 @@ The client, event stream, machine-readable output, multi-target execution, local
The server library and the server applet are implemented: open negotiation,
Lochnair (Migrated from github.com) commented 2026-08-14 06:07:50 +00:00

Reworded to "a client that expresses no fill preference at all", so the sentence no longer reads as the literal none descriptor — which is honored and zero-fills.

🤖 Addressed by Claude Code

Reworded to "a client that expresses no fill preference at all", so the sentence no longer reads as the literal `none` descriptor — which is honored and zero-fills. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
Sign in to join this conversation.