From 967e427cfbe8ded17fefb8c46dc4ad2dfeaaf0db Mon Sep 17 00:00:00 2001 From: Alex van de Sandt Date: Sun, 28 Dec 2025 11:14:36 -0600 Subject: [PATCH] Avoid some panics in raw telem conversion --- Cargo.lock | 28 ++++++++++ Cargo.toml | 2 + src/file.rs | 24 ++++++--- src/raw.rs | 40 ++++++++++----- src/telemetry/headers.rs | 108 +++++++++++++++++---------------------- src/telemetry/mod.rs | 2 +- 6 files changed, 123 insertions(+), 81 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e80073a..b41d15f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -87,6 +87,12 @@ dependencies = [ "windows-link", ] +[[package]] +name = "claims" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bba18ee93d577a8428902687bcc2b6b45a56b1981a1f6d779731c86cc4c5db18" + [[package]] name = "core-foundation-sys" version = "0.8.7" @@ -226,11 +232,13 @@ dependencies = [ "aligned-vec", "bytemuck", "chrono", + "claims", "csv", "indexmap", "num_enum", "saphyr", "serde", + "thiserror", ] [[package]] @@ -422,6 +430,26 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "thiserror" +version = "2.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f63587ca0f12b72a0600bcba1d40081f830876000bb46dd2337a3051618f4fc8" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ff15c8ecd7de3849db632e14d18d2571fa09dfc5ed93479bc4485c7a517c913" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "toml_datetime" version = "0.7.5+spec-1.1.0" diff --git a/Cargo.toml b/Cargo.toml index 6b917b1..4979c3d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,8 +11,10 @@ indexmap = "2.12.1" num_enum = "0.7.5" saphyr = "0.0.6" serde = { version = "1.0.228", features = ["derive"] } +thiserror = "2.0.17" [dev-dependencies] +claims = "0.8.0" csv = "1.4.0" [lints.clippy] diff --git a/src/file.rs b/src/file.rs index c3561f8..3df0092 100644 --- a/src/file.rs +++ b/src/file.rs @@ -5,9 +5,21 @@ use saphyr::LoadableYamlNode; use crate::{ raw, - telemetry::{DiskSubHeader, Header, Sample, VarBufInfo, VarHeader, VarSet}, + telemetry::{CastError, DiskSubHeader, Header, Sample, VarBufInfo, VarHeader, VarSet}, }; +#[derive(Debug, thiserror::Error)] +pub enum IbtFileError { + #[error(transparent)] + CastError(#[from] CastError), + + #[error(transparent)] + Io(#[from] std::io::Error), + + #[error(transparent)] + RawTelem(#[from] raw::RawTelemError), +} + #[derive(Clone, Debug)] pub struct IbtFile { data: AVec>, @@ -21,16 +33,16 @@ pub struct IbtFile { } impl IbtFile { - pub fn from_file>(path: P) -> Result { + pub fn from_file>(path: P) -> Result { let data = AVec::from_slice(raw::ALIGNMENT, &std::fs::read(&path)?); - let raw_header = raw::Header::from_raw_bytes(&data[..raw::HEADER_SIZE]); - let header = Header::from_raw(&raw_header); + let raw_header = raw::Header::from_raw_bytes(&data[..raw::HEADER_SIZE])?; + let header = Header::from_raw(&raw_header)?; let raw_sub_header = raw::DiskSubHeader::from_raw_bytes( &data[raw::HEADER_SIZE..raw::HEADER_SIZE + raw::SUB_HEADER_SIZE], ); - let sub_header = DiskSubHeader::from_raw(&raw_sub_header); + let sub_header = DiskSubHeader::from_raw(&raw_sub_header)?; let var_headers_offset = raw_header.var_header_offset as usize; let var_headers_len = raw::VAR_HEADER_SIZE * raw_header.num_vars as usize; @@ -41,7 +53,7 @@ impl IbtFile { .collect(); let vars = VarSet::new(var_headers); - let var_buf_info = VarBufInfo::from_raw(&raw_header.var_bufs[0]); + let var_buf_info = VarBufInfo::from_raw(&raw_header.var_bufs[0])?; Ok(Self { data, diff --git a/src/raw.rs b/src/raw.rs index 039efe9..a13780f 100644 --- a/src/raw.rs +++ b/src/raw.rs @@ -16,10 +16,15 @@ pub const HEADER_SIZE: usize = std::mem::size_of::
(); pub const SUB_HEADER_SIZE: usize = std::mem::size_of::(); pub const VAR_HEADER_SIZE: usize = std::mem::size_of::(); +#[derive(Clone, Copy, Debug, thiserror::Error)] +pub enum RawTelemError { + #[error("API version (first four bytes) should always be `2`, got `{0}`")] + InvalidApiVersion(c_int), +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, AnyBitPattern)] #[repr(C, align(16))] pub struct Header { - // TODO: add assertions on this field /// API header version, should always be 2 pub ver: c_int, /// Connected status, should always be 1 @@ -61,15 +66,15 @@ pub struct VarBuf { #[repr(C, align(16))] pub struct DiskSubHeader { /// Timestamp for the start of the session, seconds since epoch - pub session_start_date: time_t, + pub start_date: time_t, /// How long into the session the run started, in seconds - pub session_start_time: c_double, + pub start_time: c_double, /// How long into the session the run ended, in seconds - pub session_end_time: c_double, + pub end_time: c_double, /// Number of laps run in the session - pub session_lap_count: c_int, + pub lap_count: c_int, /// Number of records in the file - pub session_record_count: c_int, + pub record_count: c_int, } #[derive(Clone, Copy, Debug, Eq, AnyBitPattern)] @@ -86,8 +91,13 @@ pub struct VarHeader { } impl Header { - pub fn from_raw_bytes(bytes: &[u8]) -> Self { - *bytemuck::from_bytes(bytes) + pub fn from_raw_bytes(bytes: &[u8]) -> Result { + let header = *bytemuck::from_bytes::(bytes); + if header.ver != 2 { + return Err(RawTelemError::InvalidApiVersion(header.ver)); + } + + Ok(header) } } @@ -159,6 +169,8 @@ impl PartialEq for VarHeader { #[cfg(test)] mod tests { + use claims::assert_ok_eq; + use crate::{ include_bytes_aligned, raw::{DiskSubHeader, Header, VarBuf, VarHeader}, @@ -171,7 +183,7 @@ mod tests { let raw = include_bytes_aligned!("../test-data/raw_header"); let header = Header::from_raw_bytes(&raw); - assert_eq!( + assert_ok_eq!( header, Header { ver: 2, @@ -219,11 +231,11 @@ mod tests { assert_eq!( disk_sub_header, DiskSubHeader { - session_start_date: 1764642265, - session_start_time: 52.116666030881774, - session_end_time: 219.34999949144233, - session_lap_count: 3, - session_record_count: 9759, + start_date: 1764642265, + start_time: 52.116666030881774, + end_time: 219.34999949144233, + lap_count: 3, + record_count: 9759, } ); } diff --git a/src/telemetry/headers.rs b/src/telemetry/headers.rs index 39de4c2..9aed512 100644 --- a/src/telemetry/headers.rs +++ b/src/telemetry/headers.rs @@ -4,6 +4,12 @@ use chrono::{DateTime, Utc}; use crate::raw; +#[derive(Clone, Copy, Debug, thiserror::Error)] +#[error("field at struct offset `{offset}` could not be cast")] +pub struct CastError { + offset: usize, +} + #[derive(Clone, Debug, PartialEq, Eq)] pub struct Header { pub tick_rate: u32, @@ -43,68 +49,49 @@ pub struct VarBufInfo { pub buf_offset: usize, } +/// Calls `try_into` on `.field`, returning an error with the byte offset of +/// the field if the conversion fails. Relies on type inference to determine output type. +macro_rules! cast_field { + ($raw:ident, $field:ident, $raw_container:ty) => {{ + use crate::telemetry::CastError; + $raw.$field.try_into().map_err(|_| CastError { + offset: std::mem::offset_of!($raw_container, $field), + }) + }}; +} + impl Header { - pub fn from_raw(raw: &raw::Header) -> Self { - Self { - tick_rate: raw - .tick_rate - .try_into() - .expect("`tick_rate` should be positive"), - session_info_update: raw - .session_info_update - .try_into() - .expect("`session_info_update` should be positive"), - session_info_len: raw - .session_info_len - .try_into() - .expect("`session_info_len` should be positive"), - session_info_offset: raw - .session_info_offset - .try_into() - .expect("`session_info_offset` should be positive"), - num_vars: raw - .num_vars - .try_into() - .expect("`num_vars` should be positive"), - buf_len: raw - .buf_len - .try_into() - .expect("`buf_len` should be positive"), - } + pub fn from_raw(raw: &raw::Header) -> Result { + Ok(Self { + tick_rate: cast_field!(raw, tick_rate, raw::Header)?, + session_info_update: cast_field!(raw, session_info_update, raw::Header)?, + session_info_len: cast_field!(raw, session_info_len, raw::Header)?, + session_info_offset: cast_field!(raw, session_info_offset, raw::Header)?, + num_vars: cast_field!(raw, num_vars, raw::Header)?, + buf_len: cast_field!(raw, buf_len, raw::Header)?, + }) } } impl DiskSubHeader { - pub fn from_raw(raw: &raw::DiskSubHeader) -> Self { - Self { - date: DateTime::from_timestamp_secs(raw.session_start_date) + pub fn from_raw(raw: &raw::DiskSubHeader) -> Result { + Ok(Self { + date: DateTime::from_timestamp_secs(raw.start_date) .expect("`session_start_date` should be a valid timestamp"), - start_time: Duration::from_secs_f64(raw.session_start_time), - end_time: Duration::from_secs_f64(raw.session_end_time), - lap_count: raw - .session_lap_count - .try_into() - .expect("`session_lap_count` should be positive"), - record_count: raw - .session_record_count - .try_into() - .expect("`session_record_count` should be positive"), - } + start_time: Duration::from_secs_f64(raw.start_time), + end_time: Duration::from_secs_f64(raw.end_time), + lap_count: cast_field!(raw, lap_count, raw::DiskSubHeader)?, + record_count: cast_field!(raw, record_count, raw::DiskSubHeader)?, + }) } } impl VarBufInfo { - pub fn from_raw(raw: &raw::VarBuf) -> Self { - Self { - tick_count: raw - .tick_count - .try_into() - .expect("`tick_count` to be positive"), - buf_offset: raw - .buf_offset - .try_into() - .expect("`buf_offset` to be positive"), - } + pub fn from_raw(raw: &raw::VarBuf) -> Result { + Ok(Self { + tick_count: cast_field!(raw, tick_count, raw::VarBuf)?, + buf_offset: cast_field!(raw, buf_offset, raw::VarBuf)?, + }) } } @@ -113,6 +100,7 @@ mod tests { use std::time::Duration; use chrono::{DateTime, Utc}; + use claims::assert_ok_eq; use crate::{ raw, @@ -136,7 +124,7 @@ mod tests { }; let header = Header::from_raw(&raw); - assert_eq!( + assert_ok_eq!( header, Header { tick_rate: 60, @@ -154,7 +142,7 @@ mod tests { let raw = raw::VarBuf::new(1234, 5678); let info = VarBufInfo::from_raw(&raw); - assert_eq!( + assert_ok_eq!( info, VarBufInfo { tick_count: 1234, @@ -167,15 +155,15 @@ mod tests { fn decodes_disk_sub_header() { let now = Utc::now(); let raw = raw::DiskSubHeader { - session_start_date: now.timestamp(), - session_start_time: 100.0, - session_end_time: 200.0, - session_lap_count: 3, - session_record_count: 9759, + start_date: now.timestamp(), + start_time: 100.0, + end_time: 200.0, + lap_count: 3, + record_count: 9759, }; let sub_header = DiskSubHeader::from_raw(&raw); - assert_eq!( + assert_ok_eq!( sub_header, DiskSubHeader { date: DateTime::from_timestamp_secs(now.timestamp()).unwrap(), diff --git a/src/telemetry/mod.rs b/src/telemetry/mod.rs index 0542f17..a316644 100644 --- a/src/telemetry/mod.rs +++ b/src/telemetry/mod.rs @@ -7,6 +7,6 @@ pub use enums::{ CarLeftRight, Enum, PaceMode, PitServiceStatus, SessionState, TrackLocation, TrackSurface, TrackWetness, }; -pub use headers::{DiskSubHeader, Header, VarBufInfo}; +pub use headers::{CastError, DiskSubHeader, Header, VarBufInfo}; pub use sample::{Sample, Value}; pub use var::{VarHeader, VarSet, VarType}; -- 2.51.2