fix(client): avoid lossy timing casts
This commit is contained in:
1 parent
a41a1e319d
commit
c3f2a524a3
5 files changed
+64
-21
No files matched your search
@@ -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<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> {
|
||||
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<i64, ClientE
|
||||
|
||||
fn negotiated_end_mono(start: Instant, duration_ns: i64) -> Result<Instant, ClientError> {
|
||||
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(),
|
||||
})
|
||||
|
||||
@@ -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!(
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -343,8 +343,8 @@ mod tests {
|
||||
fn test_params(duration: Option<Duration>, 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<Duration>) -> ClientConfig {
|
||||
ClientConfig {
|
||||
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 {
|
||||
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);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user