From c3f2a524a31ce6098f6a0ae941af56ce15ede31d Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Fri, 8 May 2026 12:19:52 +0700 Subject: [PATCH] fix(client): avoid lossy timing casts --- crates/irtt-client/src/client.rs | 26 +++++++-------- crates/irtt-client/src/client/tests/config.rs | 5 +-- .../src/client/tests/probes_replies.rs | 32 +++++++++++++++++++ crates/irtt-client/src/managed/runner.rs | 8 +++-- crates/irtt-client/tests/support/mod.rs | 14 +++++--- 5 files changed, 64 insertions(+), 21 deletions(-) diff --git a/crates/irtt-client/src/client.rs b/crates/irtt-client/src/client.rs index 0d76d48..bd7734a 100644 --- a/crates/irtt-client/src/client.rs +++ b/crates/irtt-client/src/client.rs @@ -1,7 +1,7 @@ use std::{ io, net::{SocketAddr, UdpSocket}, - time::{Duration, Instant}, + time::{Duration, Instant, SystemTime, UNIX_EPOCH}, }; use irtt_proto::{ @@ -260,7 +260,8 @@ impl Client { .negotiated .as_ref() .expect("negotiated must exist when Open"); - let interval_ns = negotiated.params.interval_ns as u64; + let interval_ns = u64::try_from(negotiated.params.interval_ns) + .expect("validated positive negotiated interval"); let session = self .session .as_mut() @@ -817,16 +818,8 @@ fn compute_one_way( let server_recv_wall = ts.recv_wall.or(ts.midpoint_wall); let server_send_wall = ts.send_wall.or(ts.midpoint_wall); - let client_send_ns = sent_at - .wall - .duration_since(std::time::UNIX_EPOCH) - .ok() - .map(|d| d.as_nanos() as i64); - let client_recv_ns = received_at - .wall - .duration_since(std::time::UNIX_EPOCH) - .ok() - .map(|d| d.as_nanos() as i64); + let client_send_ns = unix_epoch_ns_i64(sent_at.wall); + let client_recv_ns = unix_epoch_ns_i64(received_at.wall); let c2s = server_recv_wall .zip(client_send_ns) @@ -847,6 +840,12 @@ fn compute_one_way( }) } +fn unix_epoch_ns_i64(time: SystemTime) -> Option { + time.duration_since(UNIX_EPOCH) + .ok() + .and_then(|duration| i64::try_from(duration.as_nanos()).ok()) +} + fn build_received_stats(reply: &EchoReply) -> Option { if reply.recv_count.is_none() && reply.recv_window.is_none() { return None; @@ -922,8 +921,9 @@ fn config_duration_to_ns(field: &str, duration: Duration) -> Result Result { debug_assert!(duration_ns > 0); + let duration_ns = u64::try_from(duration_ns).expect("validated positive negotiated duration"); start - .checked_add(Duration::from_nanos(duration_ns as u64)) + .checked_add(Duration::from_nanos(duration_ns)) .ok_or_else(|| ClientError::NegotiationRejected { reason: "duration is too large to schedule".to_owned(), }) diff --git a/crates/irtt-client/src/client/tests/config.rs b/crates/irtt-client/src/client/tests/config.rs index f7560e6..d9c1991 100644 --- a/crates/irtt-client/src/client/tests/config.rs +++ b/crates/irtt-client/src/client/tests/config.rs @@ -112,8 +112,9 @@ fn params_from_config_rejects_zero_interval() { #[test] fn params_from_config_rejects_nanosecond_encoding_overflow_as_invalid_config() { + let i64_max_ns = u64::try_from(i64::MAX).unwrap(); let config = ClientConfig { - duration: Some(Duration::from_nanos(i64::MAX as u64) + Duration::from_nanos(1)), + duration: Some(Duration::from_nanos(i64_max_ns) + Duration::from_nanos(1)), ..ClientConfig::default() }; assert!(matches!( @@ -123,7 +124,7 @@ fn params_from_config_rejects_nanosecond_encoding_overflow_as_invalid_config() { )); let config = ClientConfig { - interval: Duration::from_nanos(i64::MAX as u64) + Duration::from_nanos(1), + interval: Duration::from_nanos(i64_max_ns) + Duration::from_nanos(1), ..ClientConfig::default() }; assert!(matches!( diff --git a/crates/irtt-client/src/client/tests/probes_replies.rs b/crates/irtt-client/src/client/tests/probes_replies.rs index c46656b..4be8741 100644 --- a/crates/irtt-client/src/client/tests/probes_replies.rs +++ b/crates/irtt-client/src/client/tests/probes_replies.rs @@ -1479,6 +1479,38 @@ fn compute_one_way_returns_none_when_both_directions_fail() { assert!(result.is_none()); } +#[test] +fn unix_epoch_ns_i64_rejects_out_of_range_wall_time() { + let i64_max_ns = u64::try_from(i64::MAX).unwrap(); + let max = SystemTime::UNIX_EPOCH + Duration::from_nanos(i64_max_ns); + let overflow = max + Duration::from_nanos(1); + + assert_eq!(unix_epoch_ns_i64(max), Some(i64::MAX)); + assert_eq!(unix_epoch_ns_i64(overflow), None); +} + +#[test] +fn compute_one_way_omits_samples_when_client_wall_time_overflows_i64() { + let i64_max_ns = u64::try_from(i64::MAX).unwrap(); + let overflow_wall = + SystemTime::UNIX_EPOCH + Duration::from_nanos(i64_max_ns) + Duration::from_nanos(1); + let sent_at = ClientTimestamp { + mono: Instant::now(), + wall: overflow_wall, + }; + let received_at = ClientTimestamp { + mono: sent_at.mono + Duration::from_millis(40), + wall: overflow_wall + Duration::from_millis(40), + }; + let ts = TimestampFields { + recv_wall: Some(i64::MAX), + send_wall: Some(i64::MAX), + ..Default::default() + }; + + assert_eq!(compute_one_way(&sent_at, &received_at, &ts), None); +} + #[test] fn matched_reply_with_reversed_monotonic_time_still_emits_event() { let params = default_params(); diff --git a/crates/irtt-client/src/managed/runner.rs b/crates/irtt-client/src/managed/runner.rs index b79014d..8daf702 100644 --- a/crates/irtt-client/src/managed/runner.rs +++ b/crates/irtt-client/src/managed/runner.rs @@ -343,8 +343,8 @@ mod tests { fn test_params(duration: Option, interval: Duration) -> Params { Params { protocol_version: PROTOCOL_VERSION, - duration_ns: duration.map_or(0, |d| d.as_nanos() as i64), - interval_ns: interval.as_nanos() as i64, + duration_ns: duration.map_or(0, test_duration_ns_i64), + interval_ns: test_duration_ns_i64(interval), length: 0, received_stats: ReceivedStats::Both, stamp_at: StampAt::Both, @@ -354,6 +354,10 @@ mod tests { } } + fn test_duration_ns_i64(duration: Duration) -> i64 { + i64::try_from(duration.as_nanos()).expect("test duration fits i64 nanoseconds") + } + fn config(addr: SocketAddr, duration: Option) -> ClientConfig { ClientConfig { server_addr: addr.to_string(), diff --git a/crates/irtt-client/tests/support/mod.rs b/crates/irtt-client/tests/support/mod.rs index 42715a3..fb49e35 100644 --- a/crates/irtt-client/tests/support/mod.rs +++ b/crates/irtt-client/tests/support/mod.rs @@ -165,9 +165,13 @@ pub fn config_for_params(addr: SocketAddr, params: &Params) -> ClientConfig { duration: if params.duration_ns == 0 { None } else { - Some(Duration::from_nanos(params.duration_ns as u64)) + Some(Duration::from_nanos( + u64::try_from(params.duration_ns).expect("test duration must be non-negative"), + )) }, - interval: Duration::from_nanos(params.interval_ns as u64), + interval: Duration::from_nanos( + u64::try_from(params.interval_ns).expect("test interval must be non-negative"), + ), length: u32::try_from(params.length).unwrap(), received_stats: params.received_stats, stamp_at: params.stamp_at, @@ -571,10 +575,12 @@ fn echo_reply_packet( } fn materialize_wall_timestamps(mut timestamps: TimestampFields) -> TimestampFields { - let now_ns = SystemTime::now() + let now_ns: i64 = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) .unwrap() - .as_nanos() as i64; + .as_nanos() + .try_into() + .expect("current wall-clock time fits i64 nanoseconds"); if let Some(offset) = timestamps.recv_wall { timestamps.recv_wall = Some(now_ns + offset); }