fix(client): avoid lossy timing casts
This commit is contained in:
5 files changed
+64
-21
No files matched your search
@@ -1,7 +1,7 @@
|
|||||||
use std::{
|
use std::{
|
||||||
io,
|
io,
|
||||||
net::{SocketAddr, UdpSocket},
|
net::{SocketAddr, UdpSocket},
|
||||||
time::{Duration, Instant},
|
time::{Duration, Instant, SystemTime, UNIX_EPOCH},
|
||||||
};
|
};
|
||||||
|
|
||||||
use irtt_proto::{
|
use irtt_proto::{
|
||||||
@@ -260,7 +260,8 @@ impl Client {
|
|||||||
.negotiated
|
.negotiated
|
||||||
.as_ref()
|
.as_ref()
|
||||||
.expect("negotiated must exist when Open");
|
.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
|
let session = self
|
||||||
.session
|
.session
|
||||||
.as_mut()
|
.as_mut()
|
||||||
@@ -817,16 +818,8 @@ fn compute_one_way(
|
|||||||
let server_recv_wall = ts.recv_wall.or(ts.midpoint_wall);
|
let server_recv_wall = ts.recv_wall.or(ts.midpoint_wall);
|
||||||
let server_send_wall = ts.send_wall.or(ts.midpoint_wall);
|
let server_send_wall = ts.send_wall.or(ts.midpoint_wall);
|
||||||
|
|
||||||
let client_send_ns = sent_at
|
let client_send_ns = unix_epoch_ns_i64(sent_at.wall);
|
||||||
.wall
|
let client_recv_ns = unix_epoch_ns_i64(received_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 c2s = server_recv_wall
|
let c2s = server_recv_wall
|
||||||
.zip(client_send_ns)
|
.zip(client_send_ns)
|
||||||
@@ -847,6 +840,12 @@ fn compute_one_way(
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn unix_epoch_ns_i64(time: SystemTime) -> Option<i64> {
|
||||||
|
time.duration_since(UNIX_EPOCH)
|
||||||
|
.ok()
|
||||||
|
.and_then(|duration| i64::try_from(duration.as_nanos()).ok())
|
||||||
|
}
|
||||||
|
|
||||||
fn build_received_stats(reply: &EchoReply) -> Option<ReceivedStatsSample> {
|
fn build_received_stats(reply: &EchoReply) -> Option<ReceivedStatsSample> {
|
||||||
if reply.recv_count.is_none() && reply.recv_window.is_none() {
|
if reply.recv_count.is_none() && reply.recv_window.is_none() {
|
||||||
return None;
|
return None;
|
||||||
@@ -922,8 +921,9 @@ fn config_duration_to_ns(field: &str, duration: Duration) -> Result<i64, ClientE
|
|||||||
|
|
||||||
fn negotiated_end_mono(start: Instant, duration_ns: i64) -> Result<Instant, ClientError> {
|
fn negotiated_end_mono(start: Instant, duration_ns: i64) -> Result<Instant, ClientError> {
|
||||||
debug_assert!(duration_ns > 0);
|
debug_assert!(duration_ns > 0);
|
||||||
|
let duration_ns = u64::try_from(duration_ns).expect("validated positive negotiated duration");
|
||||||
start
|
start
|
||||||
.checked_add(Duration::from_nanos(duration_ns as u64))
|
.checked_add(Duration::from_nanos(duration_ns))
|
||||||
.ok_or_else(|| ClientError::NegotiationRejected {
|
.ok_or_else(|| ClientError::NegotiationRejected {
|
||||||
reason: "duration is too large to schedule".to_owned(),
|
reason: "duration is too large to schedule".to_owned(),
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -112,8 +112,9 @@ fn params_from_config_rejects_zero_interval() {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn params_from_config_rejects_nanosecond_encoding_overflow_as_invalid_config() {
|
fn params_from_config_rejects_nanosecond_encoding_overflow_as_invalid_config() {
|
||||||
|
let i64_max_ns = u64::try_from(i64::MAX).unwrap();
|
||||||
let config = ClientConfig {
|
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()
|
..ClientConfig::default()
|
||||||
};
|
};
|
||||||
assert!(matches!(
|
assert!(matches!(
|
||||||
@@ -123,7 +124,7 @@ fn params_from_config_rejects_nanosecond_encoding_overflow_as_invalid_config() {
|
|||||||
));
|
));
|
||||||
|
|
||||||
let config = ClientConfig {
|
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()
|
..ClientConfig::default()
|
||||||
};
|
};
|
||||||
assert!(matches!(
|
assert!(matches!(
|
||||||
|
|||||||
@@ -1479,6 +1479,38 @@ fn compute_one_way_returns_none_when_both_directions_fail() {
|
|||||||
assert!(result.is_none());
|
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]
|
#[test]
|
||||||
fn matched_reply_with_reversed_monotonic_time_still_emits_event() {
|
fn matched_reply_with_reversed_monotonic_time_still_emits_event() {
|
||||||
let params = default_params();
|
let params = default_params();
|
||||||
|
|||||||
@@ -343,8 +343,8 @@ mod tests {
|
|||||||
fn test_params(duration: Option<Duration>, interval: Duration) -> Params {
|
fn test_params(duration: Option<Duration>, interval: Duration) -> Params {
|
||||||
Params {
|
Params {
|
||||||
protocol_version: PROTOCOL_VERSION,
|
protocol_version: PROTOCOL_VERSION,
|
||||||
duration_ns: duration.map_or(0, |d| d.as_nanos() as i64),
|
duration_ns: duration.map_or(0, test_duration_ns_i64),
|
||||||
interval_ns: interval.as_nanos() as i64,
|
interval_ns: test_duration_ns_i64(interval),
|
||||||
length: 0,
|
length: 0,
|
||||||
received_stats: ReceivedStats::Both,
|
received_stats: ReceivedStats::Both,
|
||||||
stamp_at: StampAt::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<Duration>) -> ClientConfig {
|
fn config(addr: SocketAddr, duration: Option<Duration>) -> ClientConfig {
|
||||||
ClientConfig {
|
ClientConfig {
|
||||||
server_addr: addr.to_string(),
|
server_addr: addr.to_string(),
|
||||||
|
|||||||
@@ -165,9 +165,13 @@ pub fn config_for_params(addr: SocketAddr, params: &Params) -> ClientConfig {
|
|||||||
duration: if params.duration_ns == 0 {
|
duration: if params.duration_ns == 0 {
|
||||||
None
|
None
|
||||||
} else {
|
} 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(),
|
length: u32::try_from(params.length).unwrap(),
|
||||||
received_stats: params.received_stats,
|
received_stats: params.received_stats,
|
||||||
stamp_at: params.stamp_at,
|
stamp_at: params.stamp_at,
|
||||||
@@ -571,10 +575,12 @@ fn echo_reply_packet(
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn materialize_wall_timestamps(mut timestamps: TimestampFields) -> TimestampFields {
|
fn materialize_wall_timestamps(mut timestamps: TimestampFields) -> TimestampFields {
|
||||||
let now_ns = SystemTime::now()
|
let now_ns: i64 = SystemTime::now()
|
||||||
.duration_since(SystemTime::UNIX_EPOCH)
|
.duration_since(SystemTime::UNIX_EPOCH)
|
||||||
.unwrap()
|
.unwrap()
|
||||||
.as_nanos() as i64;
|
.as_nanos()
|
||||||
|
.try_into()
|
||||||
|
.expect("current wall-clock time fits i64 nanoseconds");
|
||||||
if let Some(offset) = timestamps.recv_wall {
|
if let Some(offset) = timestamps.recv_wall {
|
||||||
timestamps.recv_wall = Some(now_ns + offset);
|
timestamps.recv_wall = Some(now_ns + offset);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user