diff --git a/src/archive/ba2.rs b/src/archive/ba2.rs index 23cf8480..e2b56ef0 100644 --- a/src/archive/ba2.rs +++ b/src/archive/ba2.rs @@ -4,7 +4,7 @@ use std::{ io::{BufRead, Seek}, }; -use super::error::ArchiveParsingError; +use super::error::{ArchiveParsingError, slice_too_small}; use super::parse::{to_u32, to_u64}; @@ -28,11 +28,10 @@ impl TryFrom<[u8; HEADER_SIZE - TYPE_ID.len()]> for Header { fn try_from(value: [u8; HEADER_SIZE - TYPE_ID.len()]) -> Result { let header = Self { type_id: TYPE_ID, - version: to_u32(&value), - archive_type: <[u8; 4]>::try_from(&value[4..8]) - .expect("Bytes slice is large enough to hold a 4-byte array"), - file_count: to_u32(&value[8..]), - file_paths_offset: to_u64(&value[12..]), + version: to_u32(&value)?, + archive_type: to_archive_type(&value)?, + file_count: to_u32(&value[8..])?, + file_paths_offset: to_u64(&value[12..])?, }; // The header version is 1, 7 or 8 for Fallout 4 and 2 or 3 for Starfield. @@ -52,6 +51,17 @@ impl TryFrom<[u8; HEADER_SIZE - TYPE_ID.len()]> for Header { } } +fn to_archive_type( + array: &[u8; HEADER_SIZE - TYPE_ID.len()], +) -> Result<[u8; 4], ArchiveParsingError> { + let slice = &array[4..8]; + + slice + .try_into() + // This should be impossible, but it can't be asserted at compile time. + .map_err(|_e| slice_too_small(slice, 4)) +} + pub(super) fn read_assets( mut reader: T, ) -> Result>, ArchiveParsingError> { diff --git a/src/archive/bsa.rs b/src/archive/bsa.rs index dd4cea36..a4fc0e93 100644 --- a/src/archive/bsa.rs +++ b/src/archive/bsa.rs @@ -30,21 +30,17 @@ impl TryFrom<[u8; HEADER_SIZE - TYPE_ID.len()]> for Header { fn try_from(value: [u8; HEADER_SIZE - TYPE_ID.len()]) -> Result { let header = Self { type_id: TYPE_ID, - version: to_u32(&value), - records_offset: to_u32(&value[4..]), - archive_flags: to_u32(&value[8..]), - folder_count: to_u32(&value[12..]), - total_file_count: to_u32(&value[16..]), - total_folder_names_length: to_u32(&value[20..]), - total_file_names_length: to_u32(&value[24..]), - content_type_flags: to_u32(&value[28..]), + version: to_u32(&value)?, + records_offset: to_u32(&value[4..])?, + archive_flags: to_u32(&value[8..])?, + folder_count: to_u32(&value[12..])?, + total_file_count: to_u32(&value[16..])?, + total_folder_names_length: to_u32(&value[20..])?, + total_file_names_length: to_u32(&value[24..])?, + content_type_flags: to_u32(&value[28..])?, }; - if header.records_offset - != HEADER_SIZE - .try_into() - .expect("header size can fit in a u32") - { + if to_usize(header.records_offset) != HEADER_SIZE { return Err(ArchiveParsingError::InvalidRecordsOffset( header.records_offset, )); @@ -66,46 +62,54 @@ struct FolderRecord { // Also used for v104 BSAs. mod v103 { - use crate::archive::parse::{to_u32, to_u64}; + use crate::archive::{ + error::ArchiveParsingError, + parse::{to_u32, to_u64}, + }; use super::FolderRecord; pub(super) const FOLDER_RECORD_SIZE: usize = 16; - pub(super) fn read_folder_record(value: &[u8]) -> FolderRecord { - assert!( - value.len() >= FOLDER_RECORD_SIZE, - "Folder record byte slice is too small, expected {FOLDER_RECORD_SIZE} bytes, got {}", - value.len() - ); - - FolderRecord { - name_hash: to_u64(value), - file_count: to_u32(&value[8..]), - file_records_offset: to_u32(&value[12..]), + pub(super) fn read_folder_record(value: &[u8]) -> Result { + if value.len() < FOLDER_RECORD_SIZE { + return Err(ArchiveParsingError::SliceTooSmall { + expected: FOLDER_RECORD_SIZE, + actual: value.len(), + }); } + + Ok(FolderRecord { + name_hash: to_u64(value)?, + file_count: to_u32(&value[8..])?, + file_records_offset: to_u32(&value[12..])?, + }) } } mod v105 { - use crate::archive::parse::{to_u32, to_u64}; + use crate::archive::{ + error::ArchiveParsingError, + parse::{to_u32, to_u64}, + }; use super::FolderRecord; pub(super) const FOLDER_RECORD_SIZE: usize = 24; - pub(super) fn read_folder_record(value: &[u8]) -> FolderRecord { - assert!( - value.len() >= FOLDER_RECORD_SIZE, - "Folder record byte slice is too small, expected {FOLDER_RECORD_SIZE} bytes, got {}", - value.len() - ); - - FolderRecord { - name_hash: to_u64(value), - file_count: to_u32(&value[8..]), - file_records_offset: to_u32(&value[16..]), + pub(super) fn read_folder_record(value: &[u8]) -> Result { + if value.len() < FOLDER_RECORD_SIZE { + return Err(ArchiveParsingError::SliceTooSmall { + expected: FOLDER_RECORD_SIZE, + actual: value.len(), + }); } + + Ok(FolderRecord { + name_hash: to_u64(value)?, + file_count: to_u32(&value[8..])?, + file_records_offset: to_u32(&value[16..])?, + }) } } @@ -138,7 +142,7 @@ pub(super) fn read_assets( fn read_assets_with_header( mut reader: T, header: &Header, - read_folder_record: impl Fn(&[u8]) -> FolderRecord, + read_folder_record: impl Fn(&[u8]) -> Result, ) -> Result>, ArchiveParsingError> { let mut folders_buffer: Vec = vec![0; U * to_usize(header.folder_count)]; @@ -157,7 +161,7 @@ fn read_assets_with_header( let mut assets = BTreeMap::new(); for chunk in folders_buffer.chunks_exact(U) { - let folder_record = read_folder_record(chunk); + let folder_record = read_folder_record(chunk)?; let entry = assets.entry(folder_record.name_hash); if let Entry::Occupied(_) = entry { @@ -193,7 +197,7 @@ fn read_assets_with_header( .chunks_exact(FILE_RECORD_SIZE) .take(to_usize(folder_record.file_count)) { - let file_hash = to_u64(file_chunk); + let file_hash = to_u64(file_chunk)?; if !file_hashes.insert(file_hash) { return Err(ArchiveParsingError::HashCollision { diff --git a/src/archive/error.rs b/src/archive/error.rs index 379f6628..a07d320e 100644 --- a/src/archive/error.rs +++ b/src/archive/error.rs @@ -47,6 +47,7 @@ pub(crate) enum ArchiveParsingError { UsesBigEndianNumbers, FolderHashCollision(u64), HashCollision { folder_hash: u64, file_hash: u64 }, + SliceTooSmall { expected: usize, actual: usize }, } impl std::fmt::Display for ArchiveParsingError { @@ -74,6 +75,10 @@ impl std::fmt::Display for ArchiveParsingError { f, "unexpected collision for file name hash {file_hash:x} in set for folder name hash {folder_hash:x}" ), + Self::SliceTooSmall { expected, actual } => write!( + f, + "byte slice was unexpectedly too small: expected {expected} bytes, got {actual} bytes" + ), } } } @@ -92,3 +97,10 @@ impl From for ArchiveParsingError { ArchiveParsingError::IoError(value) } } + +pub(super) fn slice_too_small(slice: &[u8], expected_size: usize) -> ArchiveParsingError { + ArchiveParsingError::SliceTooSmall { + expected: expected_size, + actual: slice.len(), + } +} diff --git a/src/archive/parse.rs b/src/archive/parse.rs index c28cbc0f..c4d1c6ae 100644 --- a/src/archive/parse.rs +++ b/src/archive/parse.rs @@ -7,6 +7,7 @@ use std::{ use super::error::{ArchiveParsingError, ArchivePathParsingError}; use crate::{ + archive::error::slice_too_small, logging::{self, format_details}, plugin::has_ascii_extension, }; @@ -93,20 +94,30 @@ fn get_assets_in_archive( } } -pub(super) fn to_u32(bytes: &[u8]) -> u32 { - let array = - <[u8; 4]>::try_from(&bytes[..4]).expect("Bytes slice is large enough to hold a u32"); - u32::from_le_bytes(array) +pub(super) fn to_u32(bytes: &[u8]) -> Result { + const ARRAY_SIZE: usize = to_usize(u32::BITS >> 3); + + <[u8; ARRAY_SIZE]>::try_from(&bytes[..ARRAY_SIZE]) + .map(u32::from_le_bytes) + .map_err(|_e| slice_too_small(bytes, ARRAY_SIZE)) } -pub(super) fn to_u64(bytes: &[u8]) -> u64 { - let array = - <[u8; 8]>::try_from(&bytes[..8]).expect("Bytes slice is large enough to hold a u64"); - u64::from_le_bytes(array) +pub(super) fn to_u64(bytes: &[u8]) -> Result { + const ARRAY_SIZE: usize = to_usize(u64::BITS >> 3); + + <[u8; ARRAY_SIZE]>::try_from(&bytes[..ARRAY_SIZE]) + .map(u64::from_le_bytes) + .map_err(|_e| slice_too_small(bytes, ARRAY_SIZE)) } -pub(super) fn to_usize(size: u32) -> usize { - usize::try_from(size).expect("usize can hold a u32") +#[expect( + clippy::as_conversions, + reason = "Made safe by a compile-time assertion" +)] +pub(super) const fn to_usize(value: u32) -> usize { + // Error at compile time if this conversion isn't lossless. + const _: () = assert!(u32::BITS <= usize::BITS, "cannot fit a u32 into a usize!"); + value as usize } #[cfg(test)] diff --git a/src/lib.rs b/src/lib.rs index ef3e790b..724bead7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -37,7 +37,7 @@ clippy::error_impl_error, clippy::exit, // clippy::exhaustive_enums, - // clippy::expect_used, + clippy::expect_used, // clippy::filetype_is_file, clippy::float_cmp_const, clippy::fn_to_numeric_cast_any, @@ -91,7 +91,7 @@ clippy::unneeded_field_pattern, clippy::unreachable, clippy::unused_result_ok, - // clippy::unwrap_in_result, + clippy::unwrap_in_result, clippy::unwrap_used, // clippy::use_debug, clippy::verbose_file_reads, diff --git a/src/metadata/message.rs b/src/metadata/message.rs index c976a758..1069274c 100644 --- a/src/metadata/message.rs +++ b/src/metadata/message.rs @@ -312,12 +312,20 @@ impl TryFromYaml for Message { let subs = get_strings_vec_value(mapping, "subs", YamlObjectType::Message)?; if !subs.is_empty() { + #[expect( + clippy::expect_used, + reason = "Only panics if the hardcoded regex string is invalid" + )] static FMT_REGEX: LazyLock = LazyLock::new(|| { Regex::new(r"\{(\d+)\}").expect("hardcoded fmt placeholder regex should be valid") }); for mc in &mut content { if mc.text.contains("%1%") { + #[expect( + clippy::expect_used, + reason = "Only panics if the hardcoded regex string is invalid" + )] static BOOST_REGEX: LazyLock = LazyLock::new(|| { Regex::new(r"%(\d+)%") .expect("hardcoded Boost placeholder regex should be valid") diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 02f5f2a2..26b02773 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -404,6 +404,10 @@ fn extract_bash_tags(description: &str) -> Vec { } fn extract_version(description: &str) -> Result, Box> { + #[expect( + clippy::expect_used, + reason = "Only panics if a hardcoded regex string is invalid" + )] static VERSION_REGEXES: LazyLock> = LazyLock::new(|| { // The string below matches the range of version strings supported by // Pseudosem v1.0.1, excluding space separators, as they make version diff --git a/src/sorting/groups.rs b/src/sorting/groups.rs index dc380545..f3e5c432 100644 --- a/src/sorting/groups.rs +++ b/src/sorting/groups.rs @@ -86,9 +86,13 @@ fn add_groups<'a>( ); } - let node_index = group_nodes - .get(group.name()) - .expect("Group node should have just been added"); + let Some(node_index) = group_nodes.get(group.name()) else { + logging::error!( + "Unexpectedly couldn't find node for group {}: it should have just been added to the graph", + group.name() + ); + return Err(UndefinedGroupError::new(group.name().to_string())); + }; for other_group_name in sorted_clone(group.after_groups()) { if let Some(other_index) = group_nodes.get(other_group_name) {