Fix wrong interpretation of root delay and dispersion to match spec

ntp-proto/src/packet/v5/mod.rs

@@ -272,8 +272,8 @@ impl NtpHeaderV5 {
                 timescale: NtpTimescale::from_bits(data[4])?,
                 era: NtpEra(data[5]),
                 flags: NtpFlags::from_bits(data[6..8].try_into().unwrap())?,
-                root_delay: NtpDuration::from_bits_short(data[8..12].try_into().unwrap()),
-                root_dispersion: NtpDuration::from_bits_short(data[12..16].try_into().unwrap()),
+                root_delay: NtpDuration::from_bits_time32(data[8..12].try_into().unwrap()),
+                root_dispersion: NtpDuration::from_bits_time32(data[12..16].try_into().unwrap()),
                 server_cookie: NtpServerCookie(data[16..24].try_into().unwrap()),
                 client_cookie: NtpClientCookie(data[24..32].try_into().unwrap()),
                 receive_timestamp: NtpTimestamp::from_bits(data[32..40].try_into().unwrap()),
@@ -290,8 +290,8 @@ impl NtpHeaderV5 {
         w.write_all(&[self.timescale.to_bits()])?;
         w.write_all(&[self.era.0])?;
         w.write_all(&self.flags.as_bits())?;
-        w.write_all(&self.root_delay.to_bits_short())?;
-        w.write_all(&self.root_dispersion.to_bits_short())?;
+        w.write_all(&self.root_delay.to_bits_time32())?;
+        w.write_all(&self.root_dispersion.to_bits_time32())?;
         w.write_all(&self.server_cookie.0)?;
         w.write_all(&self.client_cookie.0)?;
         w.write_all(&self.receive_timestamp.to_bits())?;
@@ -421,7 +421,7 @@ mod tests {
     }
 
     #[test]
-    fn parse_resonse() {
+    fn parse_response() {
         #[allow(clippy::unusual_byte_groupings)] // Bits are grouped by fields
         #[rustfmt::skip]
         let data = [
@@ -441,9 +441,9 @@ mod tests {
             0x00,
             0b0000_00_1_0,
             // Root Delay
-            0x00, 0x00, 0x02, 0x3f,
+            0x10, 0x00, 0x00, 0x00,
             // Root Dispersion
-            0x00, 0x00, 0x00, 0x42,
+            0x20, 0x00, 0x00, 0x00,
             // Server Cookie
             0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
             // Client Cookie
@@ -467,14 +467,8 @@ mod tests {
         assert!(parsed.flags.interleaved_mode);
         assert!(!parsed.flags.unknown_leap);
         assert!(parsed.flags.interleaved_mode);
-        assert_eq!(
-            parsed.root_delay,
-            NtpDuration::from_seconds(0.00877380371298031)
-        );
-        assert_eq!(
-            parsed.root_dispersion,
-            NtpDuration::from_seconds(0.001007080078359479)
-        );
+        assert_eq!(parsed.root_delay, NtpDuration::from_seconds(1.0));
+        assert_eq!(parsed.root_dispersion, NtpDuration::from_seconds(2.0));
         assert_eq!(
             parsed.server_cookie,
             NtpServerCookie([0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08])
@@ -515,8 +509,8 @@ mod tests {
                     interleaved_mode: i % 4 == 0,
                     status_message: i % 5 == 0,
                 },
-                root_delay: NtpDuration::from_bits_short([i; 4]),
-                root_dispersion: NtpDuration::from_bits_short([i.wrapping_add(1); 4]),
+                root_delay: NtpDuration::from_bits_time32([i; 4]),
+                root_dispersion: NtpDuration::from_bits_time32([i.wrapping_add(1); 4]),
                 server_cookie: NtpServerCookie([i.wrapping_add(2); 8]),
                 client_cookie: NtpClientCookie([i.wrapping_add(3); 8]),
                 receive_timestamp: NtpTimestamp::from_bits([i.wrapping_add(4); 8]),

ntp-proto/src/time_types.rs

@@ -235,6 +235,27 @@ impl NtpDuration {
         .to_be_bytes()
     }
 
+    #[cfg(feature = "ntpv5")]
+    pub(crate) const fn from_bits_time32(bits: [u8; 4]) -> Self {
+        NtpDuration {
+            duration: (u32::from_be_bytes(bits) as i64) << 4,
+        }
+    }
+
+    #[cfg(feature = "ntpv5")]
+    pub(crate) fn to_bits_time32(self) -> [u8; 4] {
+        // serializing negative durations should never happen
+        // and indicates a programming error elsewhere.
+        // as for duration that are too large, saturating is
+        // the safe option.
+        assert!(self.duration >= 0);
+
+        // On overflow we just saturate to the maximum 16s
+        u32::try_from(self.duration >> 4)
+            .unwrap_or(u32::MAX)
+            .to_be_bytes()
+    }
+
     /// Convert to an f64; required for statistical calculations
     /// (e.g. in clock filtering)
     pub fn to_seconds(self) -> f64 {
@@ -843,4 +864,33 @@ mod tests {
             NtpDuration::from_seconds(1.0) * FrequencyTolerance::ppm(1_000_000),
         );
     }
+
+    #[test]
+    #[cfg(feature = "ntpv5")]
+    fn time32() {
+        type D = NtpDuration;
+        assert_eq!(D::from_bits_time32([0, 0, 0, 0]), D::ZERO);
+        assert_eq!(D::from_bits_time32([0x10, 0, 0, 0]), D::from_seconds(1.0));
+        assert_eq!(D::from_bits_time32([0, 0, 0, 1]).as_seconds_nanos(), (0, 3));
+        assert_eq!(
+            D::from_bits_time32([0, 0, 0, 10]).as_seconds_nanos(),
+            (0, 37)
+        );
+
+        assert_eq!(D::from_seconds(16.0).to_bits_time32(), [0xFF; 4]);
+        assert_eq!(D { duration: 0xF }.to_bits_time32(), [0; 4]);
+        assert_eq!(D { duration: 0x1F }.to_bits_time32(), [0, 0, 0, 1]);
+
+        for i in 0..u8::MAX {
+            let mut bits = [i, i, i, i];
+            for (idx, b) in bits.iter_mut().enumerate() {
+                *b = b.wrapping_add(idx as u8);
+            }
+
+            let d = D::from_bits_time32(bits);
+            let out_bits = d.to_bits_time32();
+
+            assert_eq!(bits, out_bits);
+        }
+    }
 }