fix(client): accept upstream midpoint echo replies #66

2026-08-11T08:15:57Z

Lochnair commented

2026-08-11 08:15:57 +00:00

(Migrated from github.com)

Upstream irtt 0.9.1 always emits both midpoint timestamp fields (wall,
then monotonic) in an ECHO reply for single-clock StampAt::Midpoint
negotiations, even though only one clock was negotiated, so its header is
one timestamp longer than the negotiated header.

The resulting datagram length is not simply negotiated_len + 8. Both
forms are max(negotiated_length, header), so the extra field lengthens
the datagram only while the larger header still exceeds the negotiated
length, and displaces payload beyond that; the difference is 8, then
7..1, then 0.

decode_echo_reply now accepts the compatibility length wherever it is
strictly longer than the negotiated length, parsing both wire fields and
exposing only the negotiated clock's value. Where the two lengths
coincide the forms are indistinguishable from the packet alone, so the
reply is accepted with normal negotiated parsing rather than a heuristic

a monotonic-only midpoint negotiation therefore cannot always be
corrected, which is documented and tested. Every other length stays
strictly rejected.

Echo request decoding and encode_echo_reply are unchanged: the encoder
must keep emitting the sane negotiated layout.

Upstream irtt 0.9.1 always emits both midpoint timestamp fields (wall, then monotonic) in an ECHO reply for single-clock StampAt::Midpoint negotiations, even though only one clock was negotiated, so its header is one timestamp longer than the negotiated header. The resulting datagram length is not simply negotiated_len + 8. Both forms are max(negotiated_length, header), so the extra field lengthens the datagram only while the larger header still exceeds the negotiated length, and displaces payload beyond that; the difference is 8, then 7..1, then 0. decode_echo_reply now accepts the compatibility length wherever it is strictly longer than the negotiated length, parsing both wire fields and exposing only the negotiated clock's value. Where the two lengths coincide the forms are indistinguishable from the packet alone, so the reply is accepted with normal negotiated parsing rather than a heuristic - a monotonic-only midpoint negotiation therefore cannot always be corrected, which is documented and tested. Every other length stays strictly rejected. Echo request decoding and encode_echo_reply are unchanged: the encoder must keep emitting the sane negotiated layout.

👍 1

chatgpt-codex-connector[bot] (Migrated from github.com) reviewed

2026-08-11 08:20:54 +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: 4738027598

ℹ️ 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:** `4738027598` <details> <summary>ℹ️ About Codex in GitHub</summary> [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>

crates/irtt-proto/src/echo.rs

						
				@@ -2,10 +2,38 @@ use crate::{

				    envelope::{self, FlagRule},

chatgpt-codex-connector[bot] (Migrated from github.com) commented

2026-08-11 08:20:54 +00:00

Guard compatibility-length addition on 32-bit targets

On 32-bit targets, a decoded or directly constructed single-clock midpoint Params with length in usize::MAX - 7..=usize::MAX makes this expected_len + 8 overflow even though echo_packet_len accepted the value, causing a debug panic or a wrapped maximum length in release builds. The same unchecked addition in validate_echo_length can panic while decoding a small malformed reply under those parameters; use checked addition and return a protocol error instead.

AGENTS.md reference: crates/irtt-proto/AGENTS.md:L13-L13

Useful? React with 👍 / 👎.

**![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat) Guard compatibility-length addition on 32-bit targets** On 32-bit targets, a decoded or directly constructed single-clock midpoint `Params` with `length` in `usize::MAX - 7..=usize::MAX` makes this `expected_len + 8` overflow even though `echo_packet_len` accepted the value, causing a debug panic or a wrapped maximum length in release builds. The same unchecked addition in `validate_echo_length` can panic while decoding a small malformed reply under those parameters; use checked addition and return a protocol error instead. AGENTS.md reference: [crates/irtt-proto/AGENTS.md:L13-L13](https://github.com/Lochnair/irtt-rs/blob/4738027598ccda791af18fffa647d0f6dc213a23/crates/irtt-proto/AGENTS.md#L13-L13) Useful? React with 👍 / 👎.

Lochnair (Migrated from github.com) reviewed

2026-08-11 08:35:24 +00:00

crates/irtt-proto/src/echo.rs

						
				@@ -2,10 +2,38 @@ use crate::{

				    envelope::{self, FlagRule},

Lochnair (Migrated from github.com) commented

2026-08-11 08:35:24 +00:00

Fixed in 8d84008: switched to checked_add (returns a new ProtoError::PacketLengthOverflow instead of wrapping/saturating), centralized in one helper (midpoint_compat_len) shared by echo_reply_max_len and the decoder's length check so they can't diverge, and added a unit test asserting no panic/wrap at usize::MAX.

🤖 Addressed by Claude Code

Fixed in 8d84008: switched to checked_add (returns a new ProtoError::PacketLengthOverflow instead of wrapping/saturating), centralized in one helper (`midpoint_compat_len`) shared by `echo_reply_max_len` and the decoder's length check so they can't diverge, and added a unit test asserting no panic/wrap at `usize::MAX`. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_

Lochnair (Migrated from github.com) reviewed

2026-08-11 09:24:08 +00:00

crates/irtt-proto/src/echo.rs

						
				@@ -2,10 +2,38 @@ use crate::{

				    envelope::{self, FlagRule},

Lochnair (Migrated from github.com) commented

2026-08-11 09:24:08 +00:00

Superseded in 79efa99: new black-box evidence corrected the model — the compat form is max(negotiated_length, normal_header + 8), not normal_len + 8, so the remaining addition is on a small bounded header size and can no longer overflow. ProtoError::PacketLengthOverflow and the checked_add from 8d84008 were removed as unnecessary; the original 32-bit concern is now structurally impossible rather than handled.

🤖 Addressed by Claude Code

Superseded in 79efa99: new black-box evidence corrected the model — the compat form is `max(negotiated_length, normal_header + 8)`, not `normal_len + 8`, so the remaining addition is on a small bounded *header* size and can no longer overflow. `ProtoError::PacketLengthOverflow` and the `checked_add` from 8d84008 were removed as unnecessary; the original 32-bit concern is now structurally impossible rather than handled. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Lochnair-Labs/irtt-rs#66

				`@@ -2,10 +2,38 @@ use crate::{`
				`envelope::{self, FlagRule},`