feat(server): handle client close #75
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Lochnair-Labs/irtt-rs#75
Reference in new issue
Block a user
Release the session a client close names. A close is bound to a session only when the token matches and the datagram arrived from that session's exact endpoint, so holding a token is not by itself authority to tear the session down: a close from a foreign source port, address, family or IPv6 scope leaves the session live, and one close cannot take down a sibling session the same peer opened separately. A token that is unknown, already closed or zero matches nothing and leaves the table untouched; zero is reserved for no-test replies and never issued.
Nothing is sent either way. Protocol version 1 defines no acknowledgement for a client close, so a successful close returns the same
Ok(None)as a rejected one and a delivered close is indistinguishable to the client from a lost one. A standaloneReply|Closedatagram is tolerated by clients but has no demonstrated use, so none is encoded.Authentication still runs before the session is looked up. That keeps a bad MAC on a live token indistinguishable from an unknown token, and means a close that fails authentication — missing, wrongly keyed, corrupted, or unexpected on a server with no key — cannot remove a session. Trailing bytes after the token stay acceptable: the structural decoder already tolerates them and the server does not re-parse the request.
The session table remains the only record of how full the server is, so a close frees capacity immediately and a refused open can succeed once one is released. Close draws no token and touches no other state; the endpoint lookup borrows the table only until the comparison is made, so the removal needs no clone.
Echo remains ignored. The test that pinned close as a no-op is replaced by
tests/close.rs, which covers removal, silence, repeated close, echo after close, endpoint binding, unknown/zero/stale tokens, authentication failure, trailing bytes, sibling sessions and capacity reuse;unsupported.rskeeps echo and structurally invalid traffic.💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:
d269f77c87ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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".
@@ -191,6 +195,30 @@ impl ServerCore {Ok(Some(packet))When an IPv6 open and close have the same address, port, scope ID, and family but their
SocketAddrV6::flowinfo()values differ, this equality check rejects the close because Rust'sSocketAddrV6equality includesflowinfo. Flow information is not one of the session-identity components defined by the local server specification, so the publicServerCorecan leave a valid session live solely because callers supplied different ancillary flow information; compare only the specified endpoint fields or normalize flowinfo before storing/matching.AGENTS.md reference: crates/irtt-server/AGENTS.md:L7-L16
Useful? React with 👍 / 👎.
@@ -191,6 +195,30 @@ impl ServerCore {Ok(Some(packet))Fixed in
e79ea94— endpoint identity now compares family, address, port and IPv6 scope explicitly instead of usingSocketAddrV6equality, so the flow label no longer counts; scope still does, and the address is still stored verbatim for replies. Not reachable through a default socket (a receiver reports the flow label only when asked, and there is no runtime slice yet), but it is reachable through the public core, and a stranded session has nothing to reclaim it while expiry is unimplemented. Covered byipv6_flow_information_is_not_part_of_endpoint_identity.🤖 Addressed by Claude Code
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:
e79ea94318ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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".
For scoped IPv6 peers, this comparison makes the zone index part of session identity, so a close carrying a different scope representation leaves the session live. However,
IRTT_SERVER_PROTOCOL_SPEC.md§23.3 explicitly records whether and how scopes participate in identity as unresolved; the new implementation and tests turn that ambiguity into normative behavior without evidence. Preserve the ambiguity or document this as an explicit project-specific policy rather than presenting it as verified compatibility behavior.AGENTS.md reference: crates/irtt-server/AGENTS.md:L15-L16
Useful? React with 👍 / 👎.
Right that §23.3 leaves this unsettled — fixed the framing in
6c34fd3rather than the behavior.same_endpointnow separates the established components (family, address, port) from the two decided here, and documents the included zone as explicitirtt-rspolicy taken under a recorded unknown, with the same treatment for the excluded flow label; the test comment andcrates/irtt-server/AGENTS.mdsay the same. I kept the zone in identity deliberately: dropping it would be an equally unverified assumption in the unsafe direction, since two peers sharing a link-local address in different zones are genuinely different peers, and the failure it risks is a scoped client reopening rather than one peer closing another's session.🤖 Addressed by Claude Code