diff --git a/Cargo.toml b/Cargo.toml index 8443c99..5ecf949 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,3 +26,7 @@ unimplemented = "deny" unreachable = "deny" unwrap_in_result = "deny" unwrap_used = "deny" + +cast_possible_truncation = "deny" +cast_possible_wrap = "deny" +cast_sign_loss = "deny" diff --git a/src/core/error/internal.rs b/src/core/error/internal.rs index f9a0923..f9faec7 100644 --- a/src/core/error/internal.rs +++ b/src/core/error/internal.rs @@ -54,6 +54,8 @@ pub enum InvariantViolation { EmptyLeafSplit, #[error("leaf split target key is missing")] LeafSplitTargetMissing, + #[error("leaf split target slot {slot_index} exceeds the slot-id range")] + LeafSplitTargetSlotOutOfRange { slot_index: usize }, #[error("page {page_id} pin count overflowed")] PagePinCountOverflow { page_id: PageId }, } diff --git a/src/core/mod.rs b/src/core/mod.rs index 3fdac2d..919f31e 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -29,4 +29,5 @@ pub use types::{ }; pub(crate) use types::{Lsn, SlotId, TxnId}; -pub(crate) const PAGE_SIZE: usize = 4096; +pub(crate) const PAGE_SIZE_U16: u16 = 4096; +pub(crate) const PAGE_SIZE: usize = PAGE_SIZE_U16 as usize; diff --git a/src/executor/expression.rs b/src/executor/expression.rs index 776dd7f..c4ea3e1 100644 --- a/src/executor/expression.rs +++ b/src/executor/expression.rs @@ -25,8 +25,10 @@ pub fn evaluate_expression( /// Each values row is evaluated against an empty synthetic record. The row's /// position in the `VALUES` list becomes the result table key. pub(super) fn execute_values(rows: Vec>) -> ExecutorResult { - let rows = rows.into_iter().enumerate().map(|(table_key, expressions)| { - let input = empty_record(table_key as TableKey)?; + let rows = rows.into_iter().enumerate().map(|(row_index, expressions)| { + let table_key = TableKey::try_from(row_index) + .map_err(|_| ExecutorError::ValuesRowIndexOutOfRange { row_index })?; + let input = empty_record(table_key)?; evaluate_expressions(&expressions, &input) }); Ok(ExecutionOutput::Rows { rows: Box::new(rows) }) diff --git a/src/executor/mod.rs b/src/executor/mod.rs index 3fc1993..e0f1270 100644 --- a/src/executor/mod.rs +++ b/src/executor/mod.rs @@ -102,6 +102,12 @@ pub enum ExecutorError { /// Operand value rejected by the operator. value: Value, }, + /// A synthetic row index for a `VALUES` result exceeded the table-key range. + #[error("VALUES row index {row_index} does not fit in a table key")] + ValuesRowIndexOutOfRange { + /// Zero-based position of the row in the `VALUES` list. + row_index: usize, + }, /// Integer arithmetic overflowed. #[error("integer overflow while evaluating operator {op}")] IntegerOverflow { diff --git a/src/planner/planning.rs b/src/planner/planning.rs index 67a6cea..c7acdf6 100644 --- a/src/planner/planning.rs +++ b/src/planner/planning.rs @@ -738,7 +738,7 @@ fn exact_single_column_index( ) -> bool { index.table_id == table.table_id && index.columns.len() == 1 - && index.columns[0].source_column_ordinal as usize == column.ordinal + && index.columns[0].source_column_ordinal == column.ordinal } fn value_matches_data_type(value: &Value, data_type: DataType) -> bool { diff --git a/src/relational/catalog.rs b/src/relational/catalog.rs index 2c64e83..ebc50df 100644 --- a/src/relational/catalog.rs +++ b/src/relational/catalog.rs @@ -142,7 +142,7 @@ pub struct TableSchema { #[derive(Debug, Clone, PartialEq, Eq)] pub struct IndexColumnSchema { /// Ordinal of the source column in the indexed table schema. - pub source_column_ordinal: u64, + pub source_column_ordinal: usize, /// Column metadata copied into the index key schema. pub column: ColumnSchema, } @@ -384,10 +384,7 @@ impl IndexSchema { table: table.name.clone(), column: (*column_name).to_owned(), })?; - columns.push(IndexColumnSchema { - source_column_ordinal: source_column_ordinal as u64, - column: column.clone(), - }); + columns.push(IndexColumnSchema { source_column_ordinal, column: column.clone() }); } Ok(Self { diff --git a/src/relational/catalog_manager.rs b/src/relational/catalog_manager.rs index 8b120a8..2fba650 100644 --- a/src/relational/catalog_manager.rs +++ b/src/relational/catalog_manager.rs @@ -115,10 +115,8 @@ impl CatalogManager { column: (*column_name).to_owned(), }) })?; - index_columns.push(IndexColumnSchema { - source_column_ordinal: source_column_ordinal as u64, - column: source_column.clone(), - }); + index_columns + .push(IndexColumnSchema { source_column_ordinal, column: source_column.clone() }); catalog_columns.push(ColumnCatalogRow { column_id, object_kind: CatalogObjectKind::Index, @@ -276,7 +274,7 @@ impl CatalogManager { }) .collect(); - Ok(index_schema_from_rows(index, columns)) + index_schema_from_rows(index, columns) } pub(crate) fn index_schemas_for_table( @@ -302,7 +300,7 @@ impl CatalogManager { }) .cloned() .collect(); - Ok(index_schema_from_rows(index, columns)) + index_schema_from_rows(index, columns) }) .collect() } @@ -465,23 +463,26 @@ fn column_schema_from_row(row: ColumnCatalogRow) -> ColumnSchema { fn index_schema_from_rows( index: IndexCatalogRow, mut columns: Vec, -) -> IndexSchema { +) -> StorageResult { columns.sort_by_key(|row| row.ordinal); + let columns = columns + .into_iter() + .map(|row| { + let source_column_ordinal = row.source_column_ordinal.unwrap_or(row.ordinal); + let source_column_ordinal = usize::try_from(source_column_ordinal) + .map_err(|error| invalid_catalog_row("sys_columns", error))?; + Ok(IndexColumnSchema { source_column_ordinal, column: column_schema_from_row(row) }) + }) + .collect::>()?; - IndexSchema { + Ok(IndexSchema { index_id: index.index_id, name: index.name, table_id: index.table_id, root_page_id: index.root_page_id, unique: index.unique, - columns: columns - .into_iter() - .map(|row| IndexColumnSchema { - source_column_ordinal: row.source_column_ordinal.unwrap_or(row.ordinal), - column: column_schema_from_row(row), - }) - .collect(), - } + columns, + }) } #[cfg(test)] @@ -608,7 +609,7 @@ mod tests { let mut tables = manager.table_cursor(SYS_TABLES_ROOT_PAGE_ID); assert_table_catalog_row(&mut tables, table.table_id, "users", table.root_page_id); - let first_user_column_id = system_column_rows().len() as CatalogId + 1; + let first_user_column_id = CatalogId::try_from(system_column_rows().len()).unwrap() + 1; let mut columns = manager.table_cursor(SYS_COLUMNS_ROOT_PAGE_ID); assert_column_catalog_row( &mut columns, @@ -706,8 +707,9 @@ mod tests { }, ); - let index_column_id = - system_column_rows().len() as CatalogId + table.row.columns.len() as CatalogId + 1; + let index_column_id = CatalogId::try_from(system_column_rows().len()).unwrap() + + CatalogId::try_from(table.row.columns.len()).unwrap() + + 1; let mut columns = manager.table_cursor(SYS_COLUMNS_ROOT_PAGE_ID); assert_column_catalog_row( &mut columns, diff --git a/src/relational/index_manager.rs b/src/relational/index_manager.rs index 4c756a2..f67ce30 100644 --- a/src/relational/index_manager.rs +++ b/src/relational/index_manager.rs @@ -101,7 +101,7 @@ fn index_key_from_record_bytes( let mut values = Vec::with_capacity(index.columns.len()); for column in &index.columns { - let ordinal = column.source_column_ordinal as usize; + let ordinal = column.source_column_ordinal; let value = tuple.values().nth(ordinal).ok_or_else(|| { invalid_table_record( table, diff --git a/src/relational/tuple.rs b/src/relational/tuple.rs index 7012c6b..2c500f8 100644 --- a/src/relational/tuple.rs +++ b/src/relational/tuple.rs @@ -53,9 +53,9 @@ const TAG_UNSIGNED_INTEGER: u8 = 0x06; const NULL_LENGTH: u32 = 0; const BOOL_LENGTH: u32 = 1; -const I32_LENGTH: u32 = size_of::() as u32; -const F32_LENGTH: u32 = size_of::() as u32; -const U64_LENGTH: u32 = size_of::() as u32; +const I32_LENGTH: u32 = i32::BITS / 8; +const F32_LENGTH: u32 = 4; +const U64_LENGTH: u32 = u64::BITS / 8; /// A single typed value stored in a [`Tuple`]. #[derive(Debug, Clone, PartialEq)] @@ -539,13 +539,13 @@ fn read_value(reader: &mut R, tag: u8, len: u32) -> io::Result { fn validate_value_payload(tag: u8, payload: &[u8]) -> io::Result<()> { match tag { - TAG_NULL => validate_len(tag, payload.len() as u32, NULL_LENGTH), + TAG_NULL => validate_payload_len(tag, payload.len(), 0), TAG_STRING => { std::str::from_utf8(payload).map_err(invalid_data)?; Ok(()) } TAG_BOOLEAN => { - validate_len(tag, payload.len() as u32, BOOL_LENGTH)?; + validate_payload_len(tag, payload.len(), 1)?; match payload[0] { 0 | 1 => Ok(()), actual => Err(io::Error::new( @@ -554,13 +554,13 @@ fn validate_value_payload(tag: u8, payload: &[u8]) -> io::Result<()> { )), } } - TAG_INTEGER => validate_len(tag, payload.len() as u32, I32_LENGTH), + TAG_INTEGER => validate_payload_len(tag, payload.len(), size_of::()), TAG_FLOAT => { - validate_len(tag, payload.len() as u32, F32_LENGTH)?; + validate_payload_len(tag, payload.len(), size_of::())?; let bytes = payload.try_into().map_err(invalid_data)?; validate_float(decode_ordered_f32(bytes)) } - TAG_UNSIGNED_INTEGER => validate_len(tag, payload.len() as u32, U64_LENGTH), + TAG_UNSIGNED_INTEGER => validate_payload_len(tag, payload.len(), size_of::()), actual => Err(io::Error::new( io::ErrorKind::InvalidData, format!("unknown tuple value tag: {actual}"), @@ -568,6 +568,17 @@ fn validate_value_payload(tag: u8, payload: &[u8]) -> io::Result<()> { } } +fn validate_payload_len(tag: u8, actual: usize, expected: usize) -> io::Result<()> { + if actual == expected { + Ok(()) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("invalid length {actual} for tuple value tag {tag}; expected {expected}"), + )) + } +} + fn validate_len(tag: u8, actual: u32, expected: u32) -> io::Result<()> { if actual == expected { Ok(()) @@ -625,11 +636,11 @@ fn read_value_len_from_slice(bytes: &[u8], offset: usize) -> io::Result<(u32, us } fn encode_ordered_i32(value: i32) -> [u8; size_of::()] { - ((value as u32) ^ 0x8000_0000).to_be_bytes() + (value.cast_unsigned() ^ 0x8000_0000).to_be_bytes() } fn decode_ordered_i32(bytes: [u8; size_of::()]) -> i32 { - (u32::from_be_bytes(bytes) ^ 0x8000_0000) as i32 + (u32::from_be_bytes(bytes) ^ 0x8000_0000).cast_signed() } fn encode_ordered_f32(value: f32) -> io::Result<[u8; size_of::()]> { diff --git a/src/storage/btree/rebalance.rs b/src/storage/btree/rebalance.rs index 3001b16..9d1760f 100644 --- a/src/storage/btree/rebalance.rs +++ b/src/storage/btree/rebalance.rs @@ -144,15 +144,16 @@ impl TreeCursor { let page = pin.read()?; let interior = page.open::()?; let slot_count = interior.slot_count(); - if child_index > slot_count as usize { - let slot_index = child_index.try_into().unwrap_or(u16::MAX); - return Err(PageError::InvalidSlotIndex { slot_index, slot_count }.into()); - } - if child_index == slot_count as usize { + if child_index == usize::from(slot_count) { return Ok(interior.rightmost_child()); } + let slot_index = u16::try_from(child_index) + .map_err(|_| PageError::InvalidSlotIndex { slot_index: u16::MAX, slot_count })?; + if slot_index > slot_count { + return Err(PageError::InvalidSlotIndex { slot_index, slot_count }.into()); + } - let (left_child, _, _, _) = interior.cell_payload_parts(child_index as u16)?; + let (left_child, _, _, _) = interior.cell_payload_parts(slot_index)?; Ok(left_child) } @@ -389,12 +390,12 @@ impl TreeCursor { ) -> StorageResult { let child_page_ids = self.read_interior_child_page_ids(page_id)?; for (slot_index, &child_page_id) in - child_page_ids[..child_page_ids.len() - 1].iter().enumerate() + (0u16..).zip(&child_page_ids[..child_page_ids.len() - 1]) { let matches = self.with_subtree_max_key(child_page_id, |expected_key| { let expected_key = expected_key.ok_or_else(|| Self::missing_child_max_key_error(page_id))?; - self.compare_interior_key(page_id, slot_index as u16, expected_key) + self.compare_interior_key(page_id, slot_index, expected_key) .map(|ordering| ordering == Ordering::Equal) })?; if !matches { diff --git a/src/storage/btree/rebalance_repair.rs b/src/storage/btree/rebalance_repair.rs index ff21561..5a370af 100644 --- a/src/storage/btree/rebalance_repair.rs +++ b/src/storage/btree/rebalance_repair.rs @@ -112,7 +112,8 @@ impl TreeCursor { let mut child_index = 0; loop { - let child_count = self.raw_interior_slot_count(page_id)? as usize + 1; + let slot_count = self.raw_interior_slot_count(page_id)?; + let child_count = usize::from(slot_count) + 1; if child_index >= child_count { break; } @@ -122,7 +123,10 @@ impl TreeCursor { let child_ref = if child_index + 1 == child_count { ChildSlotRef::Rightmost } else { - ChildSlotRef::Slot(child_index as u16) + let slot_index = u16::try_from(child_index).map_err(|_| { + PageError::InvalidSlotIndex { slot_index: u16::MAX, slot_count } + })?; + ChildSlotRef::Slot(slot_index) }; let parent_frame = PathFrame { page_id, child_ref }; if let Some(parent_pending) = self.insert_into_parent(parent_frame, pending)? { diff --git a/src/storage/btree/split.rs b/src/storage/btree/split.rs index 7393d3a..f05b7d5 100644 --- a/src/storage/btree/split.rs +++ b/src/storage/btree/split.rs @@ -316,7 +316,12 @@ impl TreeCursor { .position(|cell| cell.key() == target_key) .ok_or(StorageError::Internal(InternalError::InvariantViolation( InvariantViolation::LeafSplitTargetMissing, - )))? as u16; + )))?; + let target_slot_index = u16::try_from(target_slot_index).map_err(|_| { + StorageError::Internal(InternalError::InvariantViolation( + InvariantViolation::LeafSplitTargetSlotOutOfRange { slot_index: target_slot_index }, + )) + })?; self.set_positioned_state(target_page_id, target_slot_index); Ok(PendingSplit { separator, left_page_id: leaf_page_id, right_page_id }) diff --git a/src/storage/btree/tests.rs b/src/storage/btree/tests.rs index b1c64cf..2c0b0f7 100644 --- a/src/storage/btree/tests.rs +++ b/src/storage/btree/tests.rs @@ -478,10 +478,10 @@ fn failed_interior_rewrite_leaves_page_unchanged() { let page = pin.read().unwrap(); *page.page() }; - let children: Vec<_> = (0..16) + let children: Vec<_> = (0u8..16) .map(|index| ChildEntry { - page_id: 100 + index, - max_key: Some(vec![index as u8; PAGE_SIZE]), + page_id: 100 + u64::from(index), + max_key: Some(vec![index; PAGE_SIZE]), }) .collect(); diff --git a/src/storage/database_header.rs b/src/storage/database_header.rs index 2d44d57..c8b9e2d 100644 --- a/src/storage/database_header.rs +++ b/src/storage/database_header.rs @@ -1,5 +1,5 @@ use crate::core::{ - PAGE_SIZE, PageId, + PAGE_SIZE, PAGE_SIZE_U16, PageId, error::{CorruptionComponent, CorruptionError, CorruptionKind, StorageError, StorageResult}, }; @@ -17,7 +17,7 @@ impl DatabaseHeader { let mut page = [0u8; PAGE_SIZE]; page[0..8].copy_from_slice(MAGIC); page[8..10].copy_from_slice(&FORMAT_VERSION.to_le_bytes()); - page[10..12].copy_from_slice(&(PAGE_SIZE as u16).to_le_bytes()); + page[10..12].copy_from_slice(&PAGE_SIZE_U16.to_le_bytes()); page } diff --git a/src/storage/log_manager/frame.rs b/src/storage/log_manager/frame.rs index 5dd5a1b..221a390 100644 --- a/src/storage/log_manager/frame.rs +++ b/src/storage/log_manager/frame.rs @@ -108,10 +108,10 @@ pub(super) fn deserialize_transaction( records.push(deserialize_log_record(&mut payload_cursor, txn_id)?); } - if records.len() != entry_count as usize { + if u32::try_from(records.len()) != Ok(entry_count) { return Err(LogManagerError::RecordCountMismatch { expected: entry_count, - actual: records.len() as u32, + actual: records.len(), }); } @@ -167,16 +167,19 @@ pub(super) fn scan_transaction_frame( .map_err(|_| LogManagerError::PayloadLengthTooLarge { payload_len: header.payload_len })?; let mut remaining = payload_len; let mut digest = CRC32.digest(); - let mut actual_record_count = 0u32; + let mut actual_record_count = 0usize; while remaining > 0 { scan_log_record_payload(reader, &mut digest, &mut remaining)?; actual_record_count = actual_record_count.checked_add(1).ok_or({ - LogManagerError::RecordCountMismatch { expected: header.entry_count, actual: u32::MAX } + LogManagerError::RecordCountMismatch { + expected: header.entry_count, + actual: usize::MAX, + } })?; } - if actual_record_count != header.entry_count { + if u32::try_from(actual_record_count) != Ok(header.entry_count) { return Err(LogManagerError::RecordCountMismatch { expected: header.entry_count, actual: actual_record_count, @@ -186,7 +189,7 @@ pub(super) fn scan_transaction_frame( let footer_bytes = read_exact_or_eof::<_, FOOTER_LEN>(reader)? .ok_or(LogManagerError::TruncatedFrame { needed: FOOTER_LEN, remaining: 0 })?; validate_footer(&footer_bytes, header.txn_id, digest.finalize())?; - Ok(Some(ScannedWalFrame { txn_id: header.txn_id, record_count: actual_record_count })) + Ok(Some(ScannedWalFrame { txn_id: header.txn_id, record_count: header.entry_count })) } fn scan_log_record_payload( diff --git a/src/storage/log_manager/mod.rs b/src/storage/log_manager/mod.rs index 19f04cb..21867af 100644 --- a/src/storage/log_manager/mod.rs +++ b/src/storage/log_manager/mod.rs @@ -122,7 +122,7 @@ pub enum LogManagerError { RecordTxnIdMismatch { expected: TxnId, actual: TxnId }, /// The decoded payload record count differed from the header count. #[error("WAL record count mismatch: expected {expected}, got {actual}")] - RecordCountMismatch { expected: u32, actual: u32 }, + RecordCountMismatch { expected: u32, actual: usize }, /// A transaction frame contains more records than the format can encode. #[error("too many WAL records in transaction: {count}")] TooManyRecords { count: usize }, @@ -1292,11 +1292,12 @@ mod tests { fn rewrite_crc(buf: &mut [u8]) { let payload_len_start = 8 + 2 + 8 + 4; - let payload_len = u64::from_le_bytes( + let payload_len = usize::try_from(u64::from_le_bytes( buf[payload_len_start..payload_len_start + 8] .try_into() .expect("payload length slice has fixed width"), - ) as usize; + )) + .unwrap(); let payload_start = HEADER_LEN; let payload_end = payload_start + payload_len; let crc = CRC32.checksum(&buf[payload_start..payload_end]); diff --git a/src/storage/page/core.rs b/src/storage/page/core.rs index 74e13ad..a0b2a88 100644 --- a/src/storage/page/core.rs +++ b/src/storage/page/core.rs @@ -10,7 +10,7 @@ use super::{ self, CELL_LENGTH_SIZE, CONTENT_START_OFFSET, FIRST_FREEBLOCK_OFFSET, FORMAT_VERSION, FRAGMENTED_FREE_BYTES_OFFSET, FREEBLOCK_HEADER_SIZE, KIND_OFFSET, LSN_OFFSET, MAX_FRAGMENTED_FREE_BYTES, NEXT_PAGE_ID_OFFSET, PREV_PAGE_ID_OFFSET, SLOT_COUNT_OFFSET, - USABLE_SPACE_END, VERSION_OFFSET, + USABLE_SPACE_END, USABLE_SPACE_END_U16, VERSION_OFFSET, }, }; @@ -119,6 +119,10 @@ impl Freeblock { } } +fn encode_page_u16(value: usize) -> PageResult { + u16::try_from(value).map_err(|_| PageError::CellTooLarge { len: value, max: u16::MAX as usize }) +} + #[derive(Debug, Clone, Copy)] pub(crate) struct FreeblockIter<'a> { bytes: &'a [u8; PAGE_SIZE], @@ -155,6 +159,7 @@ fn read_freeblock( content_start: u16, offset: u16, ) -> PageResult { + let encoded_offset = offset; let offset = offset as usize; if offset < content_start as usize || offset + FREEBLOCK_HEADER_SIZE > USABLE_SPACE_END { return Err(PageError::MalformedPage(PageCorruption::FreeblockOffsetOutOfBounds)); @@ -168,7 +173,7 @@ fn read_freeblock( return Err(PageError::MalformedPage(PageCorruption::FreeblockOutOfBounds)); } - Ok(Freeblock { offset: offset as u16, size, next: format::read_optional_u16(bytes, offset) }) + Ok(Freeblock { offset: encoded_offset, size, next: format::read_optional_u16(bytes, offset) }) } fn page_kind() -> format::PageKind @@ -397,10 +402,10 @@ where format::write_u16( &mut packed, format::slot_entry_offset(header_size, slot_index), - cursor as u16, + encode_page_u16(cursor)?, ); } - format::write_u16(&mut packed, CONTENT_START_OFFSET, cursor as u16); + format::write_u16(&mut packed, CONTENT_START_OFFSET, encode_page_u16(cursor)?); format::write_optional_u16(&mut packed, FIRST_FREEBLOCK_OFFSET, None); format::write_u16(&mut packed, FRAGMENTED_FREE_BYTES_OFFSET, 0); @@ -429,7 +434,7 @@ where } if self.free_space() >= needed { - return Ok(self.allocate_from_gap(cell_len)); + return self.allocate_from_gap(cell_len); } let available = self.total_reclaimable_space()?; @@ -443,7 +448,7 @@ where return Err(PageError::PageFull { needed, available }); } - Ok(self.allocate_from_gap(cell_len)) + self.allocate_from_gap(cell_len) } fn ensure_cell_fits(&self, cell_len: usize) -> PageResult<()> { @@ -469,10 +474,11 @@ where Ok(None) } - fn allocate_from_gap(&mut self, cell_len: usize) -> u16 { + fn allocate_from_gap(&mut self, cell_len: usize) -> PageResult { let new_content_start = self.content_start() as usize - cell_len; - self.set_content_start(new_content_start as u16); - new_content_start as u16 + let new_content_start = encode_page_u16(new_content_start)?; + self.set_content_start(new_content_start); + Ok(new_content_start) } fn can_store_fragmented_bytes(&self, extra: usize) -> bool { @@ -515,14 +521,15 @@ where return Ok(freeblock.offset); } + let encoded_remainder = encode_page_u16(remainder)?; if remainder >= FREEBLOCK_HEADER_SIZE { - self.write_freeblock(freeblock.offset, freeblock.next, remainder as u16); - return Ok(freeblock.offset + remainder as u16); + self.write_freeblock(freeblock.offset, freeblock.next, encoded_remainder); + return Ok(freeblock.offset + encoded_remainder); } self.set_chain_link(previous, freeblock.next); - self.add_fragmented_bytes(remainder as u16)?; - Ok(freeblock.offset + remainder as u16) + self.add_fragmented_bytes(encoded_remainder)?; + Ok(freeblock.offset + encoded_remainder) } fn absorb_freeblocks_into_gap(&mut self) -> PageResult<()> { @@ -533,7 +540,7 @@ where let freeblock = read_freeblock(self.bytes(), self.content_start(), first_freeblock)?; self.set_first_freeblock(freeblock.next); - self.set_content_start(freeblock.end() as u16); + self.set_content_start(encode_page_u16(freeblock.end())?); } Ok(()) } @@ -546,7 +553,7 @@ where let reclaim_start = cell_offset as usize; if reclaim_start == self.content_start() as usize { - self.set_content_start((reclaim_start + cell_len) as u16); + self.set_content_start(encode_page_u16(reclaim_start + cell_len)?); self.absorb_freeblocks_into_gap()?; return Ok(()); } @@ -575,7 +582,7 @@ where self.write_freeblock( previous.offset, next_link, - (merged_end - previous.offset as usize) as u16, + encode_page_u16(merged_end - previous.offset as usize)?, ); return Ok(()); } @@ -584,20 +591,20 @@ where let merged_end = merged_with_next.map_or(reclaim_end, Freeblock::end); let merged_size = merged_end - merged_start as usize; if merged_size < FREEBLOCK_HEADER_SIZE { - return self.add_fragmented_bytes(merged_size as u16); + return self.add_fragmented_bytes(encode_page_u16(merged_size)?); } let next_link = match merged_with_next { Some(freeblock) => freeblock.next, None => next.map(|freeblock| freeblock.offset), }; - self.write_freeblock(merged_start, next_link, merged_size as u16); + self.write_freeblock(merged_start, next_link, encode_page_u16(merged_size)?); self.set_chain_link(previous, Some(merged_start)); Ok(()) } fn reset_empty_page(&mut self) { - self.set_content_start(USABLE_SPACE_END as u16); + self.set_content_start(USABLE_SPACE_END_U16); self.set_first_freeblock(None); self.set_fragmented_free_bytes(0); } @@ -627,7 +634,7 @@ where bytes[KIND_OFFSET] = page_kind::() as u8; bytes[VERSION_OFFSET] = FORMAT_VERSION; format::write_u16(bytes, SLOT_COUNT_OFFSET, 0); - format::write_u16(bytes, CONTENT_START_OFFSET, USABLE_SPACE_END as u16); + format::write_u16(bytes, CONTENT_START_OFFSET, USABLE_SPACE_END_U16); format::write_optional_u16(bytes, FIRST_FREEBLOCK_OFFSET, None); format::write_u16(bytes, FRAGMENTED_FREE_BYTES_OFFSET, 0); format::write_optional_u64(bytes, PREV_PAGE_ID_OFFSET, None); @@ -690,13 +697,15 @@ fn validate_page(bytes: &[u8; PAGE_SIZE], expected_kind: format::PageKind) -> Pa } let header_size = expected_kind.header_size(); - let slot_count = format::read_u16(bytes, SLOT_COUNT_OFFSET) as usize; + let encoded_slot_count = format::read_u16(bytes, SLOT_COUNT_OFFSET); + let slot_count = usize::from(encoded_slot_count); let slot_directory_end = header_size + slot_count * format::SLOT_ENTRY_SIZE; if slot_directory_end > USABLE_SPACE_END { return Err(PageError::MalformedPage(PageCorruption::SlotDirectoryExceedsUsableSpace)); } - let content_start = format::read_u16(bytes, CONTENT_START_OFFSET) as usize; + let encoded_content_start = format::read_u16(bytes, CONTENT_START_OFFSET); + let content_start = usize::from(encoded_content_start); if !(slot_directory_end..=USABLE_SPACE_END).contains(&content_start) { return Err(PageError::MalformedPage(if content_start > USABLE_SPACE_END { PageCorruption::ContentStartOutOfBounds @@ -711,12 +720,12 @@ fn validate_page(bytes: &[u8; PAGE_SIZE], expected_kind: format::PageKind) -> Pa let first_freeblock = format::read_optional_u16(bytes, FIRST_FREEBLOCK_OFFSET); let max_freeblocks = USABLE_SPACE_END / FREEBLOCK_HEADER_SIZE; for freeblock in - FreeblockIter::new(bytes, content_start as u16, first_freeblock).take(max_freeblocks) + FreeblockIter::new(bytes, encoded_content_start, first_freeblock).take(max_freeblocks) { let _ = freeblock?; } - for slot_index in 0..slot_count as SlotId { + for slot_index in 0..encoded_slot_count { let slot_offset = format::read_u16(bytes, format::slot_entry_offset(header_size, slot_index)) as usize; if slot_offset < content_start || slot_offset >= USABLE_SPACE_END { diff --git a/src/storage/page/format.rs b/src/storage/page/format.rs index dca14c1..9b4c157 100644 --- a/src/storage/page/format.rs +++ b/src/storage/page/format.rs @@ -4,15 +4,17 @@ //! upward from the header, a packed cell-content region that grows downward //! from the end of usable space, and a zeroed reserved footer. -use crate::core::{PAGE_SIZE, PageId, SlotId}; +use crate::core::{PAGE_SIZE, PAGE_SIZE_U16, PageId, SlotId}; use crate::storage::log_manager::Lsn; /// Current on-disk page format version. pub(crate) const FORMAT_VERSION: u8 = 5; /// Number of bytes reserved at the end of every page. -pub(crate) const RESERVED_FOOTER_SIZE: usize = 4; +pub(crate) const RESERVED_FOOTER_SIZE_U16: u16 = 4; /// Exclusive end offset of the usable region within a page buffer. -pub(crate) const USABLE_SPACE_END: usize = PAGE_SIZE - RESERVED_FOOTER_SIZE; +pub(crate) const USABLE_SPACE_END_U16: u16 = PAGE_SIZE_U16 - RESERVED_FOOTER_SIZE_U16; +/// Exclusive end offset of the usable region within a page buffer. +pub(crate) const USABLE_SPACE_END: usize = USABLE_SPACE_END_U16 as usize; /// Width in bytes of a single slot directory entry. pub(crate) const SLOT_ENTRY_SIZE: usize = 2; /// Width in bytes of the length prefix at the start of every cell. diff --git a/src/storage/page/interior.rs b/src/storage/page/interior.rs index 9bb9779..8072fa0 100644 --- a/src/storage/page/interior.rs +++ b/src/storage/page/interior.rs @@ -95,11 +95,11 @@ pub(crate) fn write_cell_with_payload( bytes: &mut [u8; PAGE_SIZE], cell_offset: usize, left_child: PageId, - key_len: usize, + key_len: u16, first_overflow_page_id: Option, inline_payload: &[u8], ) { - format::write_u16(bytes, cell_offset, key_len as u16); + format::write_u16(bytes, cell_offset, key_len); format::write_optional_u64( bytes, cell_offset + FIRST_OVERFLOW_PAGE_ID_OFFSET, @@ -107,7 +107,7 @@ pub(crate) fn write_cell_with_payload( ); let cell_len = INTERIOR_CELL_PREFIX_SIZE + inline_payload.len(); write_left_child(&mut bytes[cell_offset..cell_offset + cell_len], left_child); - format::write_u16(bytes, cell_offset + KEY_LENGTH_OFFSET, key_len as u16); + format::write_u16(bytes, cell_offset + KEY_LENGTH_OFFSET, key_len); let payload_start = cell_offset + INTERIOR_CELL_PREFIX_SIZE; bytes[payload_start..payload_start + inline_payload.len()].copy_from_slice(inline_payload); } @@ -177,9 +177,8 @@ where first_overflow_page_id: Option, inline_payload: &[u8], ) -> PageResult { - if key_len > u16::MAX as usize { - return Err(PageError::CellTooLarge { len: key_len, max: u16::MAX as usize }); - } + let encoded_key_len = u16::try_from(key_len) + .map_err(|_| PageError::CellTooLarge { len: key_len, max: u16::MAX as usize })?; let Some(expected_inline_len) = format::inline_payload_len(key_len, first_overflow_page_id) else { return Err(PageError::CellTooLarge { len: key_len, max: u16::MAX as usize }); @@ -200,7 +199,7 @@ where self.bytes_mut(), cell_offset as usize, left_child, - key_len, + encoded_key_len, first_overflow_page_id, inline_payload, ); diff --git a/src/storage/page/leaf.rs b/src/storage/page/leaf.rs index e6ccc14..6884503 100644 --- a/src/storage/page/leaf.rs +++ b/src/storage/page/leaf.rs @@ -91,38 +91,40 @@ where pub(crate) fn write_cell_with_payload( bytes: &mut [u8; PAGE_SIZE], cell_offset: usize, - key_len: usize, - value_len: usize, + payload_len: u16, + key_len: u16, + value_len: u16, first_overflow_page_id: Option, inline_payload: &[u8], ) { - let payload_len = key_len + value_len; - format::write_u16(bytes, cell_offset, payload_len as u16); + format::write_u16(bytes, cell_offset, payload_len); format::write_optional_u64( bytes, cell_offset + FIRST_OVERFLOW_PAGE_ID_OFFSET, first_overflow_page_id, ); - format::write_u16(bytes, cell_offset + KEY_LENGTH_OFFSET, key_len as u16); - format::write_u16(bytes, cell_offset + VALUE_LENGTH_OFFSET, value_len as u16); + format::write_u16(bytes, cell_offset + KEY_LENGTH_OFFSET, key_len); + format::write_u16(bytes, cell_offset + VALUE_LENGTH_OFFSET, value_len); let payload_start = cell_offset + LEAF_CELL_PREFIX_SIZE; bytes[payload_start..payload_start + inline_payload.len()].copy_from_slice(inline_payload); } -/// Validates raw leaf payload lengths and returns the total logical payload length. +/// Validates raw leaf payload lengths and returns their encoded representations. fn validate_payload_parts( key_len: usize, value_len: usize, first_overflow_page_id: Option, inline_payload: &[u8], -) -> PageResult { - let payload_len = key_len + value_len; - if key_len > u16::MAX as usize - || value_len > u16::MAX as usize - || payload_len > u16::MAX as usize - { - return Err(PageError::CellTooLarge { len: payload_len, max: u16::MAX as usize }); - } +) -> PageResult<(u16, u16, u16)> { + let payload_len = key_len + .checked_add(value_len) + .ok_or(PageError::CellTooLarge { len: usize::MAX, max: u16::MAX as usize })?; + let encoded_key_len = u16::try_from(key_len) + .map_err(|_| PageError::CellTooLarge { len: payload_len, max: u16::MAX as usize })?; + let encoded_value_len = u16::try_from(value_len) + .map_err(|_| PageError::CellTooLarge { len: payload_len, max: u16::MAX as usize })?; + let encoded_payload_len = u16::try_from(payload_len) + .map_err(|_| PageError::CellTooLarge { len: payload_len, max: u16::MAX as usize })?; let Some(expected_inline_len) = format::inline_payload_len(payload_len, first_overflow_page_id) else { return Err(PageError::CellTooLarge { len: payload_len, max: u16::MAX as usize }); @@ -133,7 +135,7 @@ fn validate_payload_parts( max: LEAF_CELL_PREFIX_SIZE + expected_inline_len, }); } - Ok(payload_len) + Ok((encoded_payload_len, encoded_key_len, encoded_value_len)) } fn compare_key( @@ -188,7 +190,8 @@ where first_overflow_page_id: Option, inline_payload: &[u8], ) -> PageResult { - validate_payload_parts(key_len, value_len, first_overflow_page_id, inline_payload)?; + let (payload_len, key_len, value_len) = + validate_payload_parts(key_len, value_len, first_overflow_page_id, inline_payload)?; if slot_index > self.slot_count() { return Err(PageError::InvalidSlotIndex { slot_index, slot_count: self.slot_count() }); } @@ -198,6 +201,7 @@ where write_cell_with_payload( self.bytes_mut(), cell_offset as usize, + payload_len, key_len, value_len, first_overflow_page_id, @@ -216,7 +220,8 @@ where first_overflow_page_id: Option, inline_payload: &[u8], ) -> PageResult { - validate_payload_parts(key_len, value_len, first_overflow_page_id, inline_payload)?; + let (payload_len, encoded_key_len, encoded_value_len) = + validate_payload_parts(key_len, value_len, first_overflow_page_id, inline_payload)?; self.validate_slot_index(slot_index)?; let cell_len = LEAF_CELL_PREFIX_SIZE + inline_payload.len(); @@ -226,8 +231,9 @@ where write_cell_with_payload( self.bytes_mut(), old_offset as usize, - key_len, - value_len, + payload_len, + encoded_key_len, + encoded_value_len, first_overflow_page_id, inline_payload, ); diff --git a/src/storage/page_cache.rs b/src/storage/page_cache.rs index 35976d2..49a8883 100644 --- a/src/storage/page_cache.rs +++ b/src/storage/page_cache.rs @@ -513,8 +513,8 @@ mod tests { /// Generates a deterministic page payload from a seed byte. fn page_with_pattern(seed: u8) -> [u8; PAGE_SIZE] { let mut page = [0u8; PAGE_SIZE]; - for (index, byte) in page.iter_mut().enumerate() { - *byte = seed.wrapping_add(index as u8); + for (index, byte) in (0u8..=u8::MAX).cycle().zip(&mut page) { + *byte = seed.wrapping_add(index); } page::format::write_u64(&mut page, page::format::LSN_OFFSET, ZERO_LSN); page diff --git a/src/storage/recovery.rs b/src/storage/recovery.rs index 7f68b00..cd849e1 100644 --- a/src/storage/recovery.rs +++ b/src/storage/recovery.rs @@ -148,7 +148,7 @@ mod tests { page::format::write_u16( &mut page, page::format::CONTENT_START_OFFSET, - page::format::USABLE_SPACE_END as u16, + page::format::USABLE_SPACE_END_U16, ); page::format::write_u64(&mut page, page::format::LSN_OFFSET, lsn); page