diff --git a/src/extension/writer.rs b/src/extension/writer.rs index 6a5e890..5f82228 100644 --- a/src/extension/writer.rs +++ b/src/extension/writer.rs @@ -3,15 +3,16 @@ use crate::database::column::Column; use crate::database::column::ColumnType; use crate::error::RustySheetError; +use crate::spreadsheet::SpreadsheetError; use crate::spreadsheet::cell::Cell; use crate::spreadsheet::cell::CellType; +use crate::spreadsheet::resolve_loaded_shared_string; +use crate::spreadsheet::sheet::Sheet; use duckdb::core::FlatVector; use duckdb::core::Inserter; use libduckdb_sys::duckdb_date; use libduckdb_sys::duckdb_time; use libduckdb_sys::duckdb_timestamp; -use crate::spreadsheet::sheet::Sheet; -use crate::spreadsheet::SpreadsheetError; /// Writes a cell value to a DuckDB vector based on column type. /// Handles type conversion and error mapping for different data types. @@ -25,8 +26,9 @@ pub(super) fn write_to_vector(sheet: &Sheet, column: &Column, cell: &Cell, vecto ) }; let cell = if cell.kind == CellType::SharedString { - let index = cell.value.parse::()?; - if let Some(shared_string) = &shared_strings[index] { + if let Some(shared_string) = + resolve_loaded_shared_string(shared_strings, &sheet.file_name, &sheet.name, cell)? + { &Cell { row: cell.row, col: cell.col, @@ -41,7 +43,7 @@ pub(super) fn write_to_vector(sheet: &Sheet, column: &Column, cell: &Cell, vecto cell }; match column.kind { - ColumnType::Varchar => vector.insert(row, &cell.to_string()), + ColumnType::Varchar => vector.insert(row, &cell.to_display_string().map_err(mapper)?), ColumnType::Boolean => write_primitive(vector, row, cell.to_boolean()), ColumnType::BigInt => write_primitive(vector, row, cell.to_bigint().map_err(mapper)?), ColumnType::Double => write_primitive(vector, row, cell.to_double().map_err(mapper)?), diff --git a/src/helpers/biff12.rs b/src/helpers/biff12.rs index c0cc814..3bab2a7 100644 --- a/src/helpers/biff12.rs +++ b/src/helpers/biff12.rs @@ -42,15 +42,14 @@ impl Biff12Reader { &'_ self, at: usize, ) -> Result<(Cow<'_, str>, usize), RustySheetError> { - let lower_bound = at + 4usize; - let size = to_usize(&self.buffer[at..at + lower_bound]); - let upper_bound = lower_bound + size * 2; - if self.buffer.len() >= upper_bound { - let (value, _, _) = UTF_16LE.decode(&self.buffer[lower_bound..upper_bound]); - Ok((value, upper_bound)) - } else { - Err(Biff12Error::NoEnoughData(upper_bound, self.buffer.len()))? - } + let size = self.get_usize(at)?; + let lower_bound = checked_offset(at, 4, self.buffer.len())?; + let byte_length = size + .checked_mul(2) + .ok_or(Biff12Error::NoEnoughData(usize::MAX, self.buffer.len()))?; + let upper_bound = checked_offset(lower_bound, byte_length, self.buffer.len())?; + let (value, _, _) = UTF_16LE.decode(self.slice(lower_bound, byte_length)?); + Ok((value, upper_bound)) } /// Reads a UTF-16 string from the specified position @@ -60,33 +59,39 @@ impl Biff12Reader { } /// Reads a usize value from the specified position - pub(crate) fn get_usize(&'_ self, at: usize) -> usize { - to_usize(&self.buffer[at..at + 4]) + pub(crate) fn get_usize(&'_ self, at: usize) -> Result { + Ok(to_usize(self.slice(at, 4)?)) } /// Reads a u16 value from the specified position - pub(crate) fn get_u16(&'_ self, at: usize) -> u16 { - to_u16(&self.buffer[at..at + 2]) + pub(crate) fn get_u16(&'_ self, at: usize) -> Result { + Ok(to_u16(self.slice(at, 2)?)) } /// Reads a u32 value from the specified position - pub(crate) fn get_u32(&'_ self, at: usize) -> u32 { - to_u32(&self.buffer[at..at + 4]) + pub(crate) fn get_u32(&'_ self, at: usize) -> Result { + Ok(to_u32(self.slice(at, 4)?)) } /// Reads an i32 value from the specified position - pub(crate) fn get_i32(&'_ self, at: usize) -> i32 { - to_i32(&self.buffer[at..at + 4]) + pub(crate) fn get_i32(&'_ self, at: usize) -> Result { + Ok(to_i32(self.slice(at, 4)?)) } /// Reads an f64 value from the specified position - pub(crate) fn get_f64(&'_ self, at: usize) -> f64 { - to_f64(&self.buffer[at..at + 8]) + pub(crate) fn get_f64(&'_ self, at: usize) -> Result { + Ok(to_f64(self.slice(at, 8)?)) } /// Reads a style index from the specified position (3 bytes, padded to 4) - pub(crate) fn get_style(&'_ self, at: usize) -> usize { - to_usize(&[self.buffer[at], self.buffer[at + 1], self.buffer[at + 2], 0]) + pub(crate) fn get_style(&'_ self, at: usize) -> Result { + let bytes = self.slice(at, 3)?; + Ok(to_usize(&[bytes[0], bytes[1], bytes[2], 0])) + } + + /// Reads a byte from the specified position + pub(crate) fn get_u8(&'_ self, at: usize) -> Result { + Ok(self.slice(at, 1)?[0]) } /// Reads a 7-bit continuation integer with the specified byte limit @@ -150,6 +155,46 @@ impl Biff12Reader { pub(crate) fn find(&mut self, target: u16) -> Result { self.find_with(target, &[]) } + + fn slice(&self, at: usize, length: usize) -> Result<&[u8], RustySheetError> { + let upper_bound = checked_offset(at, length, self.buffer.len())?; + Ok(&self.buffer[at..upper_bound]) + } +} + +fn checked_offset(at: usize, length: usize, actual: usize) -> Result { + let upper_bound = at + .checked_add(length) + .ok_or(Biff12Error::NoEnoughData(usize::MAX, actual))?; + if upper_bound <= actual { + Ok(upper_bound) + } else { + Err(Biff12Error::NoEnoughData(upper_bound, actual)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Cursor; + + #[test] + fn fixed_width_reads_return_errors_for_short_records() { + let mut reader = Biff12Reader::new(Cursor::new(Vec::new())); + reader.buffer = vec![0; 8]; + + assert!(reader.get_f64(1).unwrap_err().to_string().contains("No enough data")); + assert!(reader.get_style(6).unwrap_err().to_string().contains("No enough data")); + } + + #[test] + fn string_reads_return_errors_for_short_records() { + let mut reader = Biff12Reader::new(Cursor::new(Vec::new())); + reader.buffer = vec![2, 0, 0, 0, b'a']; + + assert!(reader.get_str(0).unwrap_err().to_string().contains("No enough data")); + assert!(reader.get_str(3).unwrap_err().to_string().contains("No enough data")); + } } #[macro_export] diff --git a/src/spreadsheet/cell.rs b/src/spreadsheet/cell.rs index 8f4e5c5..715b471 100644 --- a/src/spreadsheet/cell.rs +++ b/src/spreadsheet/cell.rs @@ -243,76 +243,36 @@ impl Cell { _ => Err(format!("parse '{}' to datetime failed", self.value))?, } } -} -impl Display for Cell { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let value = match self.kind { - CellType::Boolean => if self.value == "1" { "true" } else { "false" }.to_owned(), - CellType::NumberDateTime1900 => { - if let Ok(value) = to_datetime_string(&self.value, false) { - value - } else { - panic!( - "Parse cell value '{}' at {} to DateTime(1900) failed", - self.value, - self.reference() - ); - } - } - CellType::NumberDate1900 => { - if let Ok(value) = to_date_string(&self.value, false) { - value - } else { - panic!( - "Parse cell value '{}' at {} to Date(1900) failed", - self.value, - self.reference() - ); - } - } - CellType::NumberDateTime1904 => { - if let Ok(value) = to_datetime_string(&self.value, true) { - value - } else { - panic!( - "Parse cell value '{}' at {} to DateTime(1904) failed", - self.value, - self.reference() - ); - } - } - CellType::NumberDate1904 => { - if let Ok(value) = to_date_string(&self.value, true) { - value - } else { - panic!( - "Parse cell value '{}' at {} to Date(1904) failed", - self.value, - self.reference() - ); - } - } - CellType::NumberTime1900 | CellType::NumberTime1904 => { - if let Ok(value) = to_time_string(&self.value) { - value - } else { - panic!( - "Parse cell value '{}' at {} to Time failed", - self.value, - self.reference() - ); - } - } - CellType::IsoDateTime => self.value.replace("T", " "), - CellType::IsoDuration => self + /// Converts the cell to the user-visible string representation. + pub(crate) fn to_display_string(&self) -> Result { + match self.kind { + CellType::Boolean => Ok(if self.value == "1" { "true" } else { "false" }.to_owned()), + CellType::NumberDateTime1900 => to_datetime_string(&self.value, false) + .map_err(|_| format!("parse '{}' to DateTime(1900) failed", self.value)), + CellType::NumberDate1900 => to_date_string(&self.value, false) + .map_err(|_| format!("parse '{}' to Date(1900) failed", self.value)), + CellType::NumberDateTime1904 => to_datetime_string(&self.value, true) + .map_err(|_| format!("parse '{}' to DateTime(1904) failed", self.value)), + CellType::NumberDate1904 => to_date_string(&self.value, true) + .map_err(|_| format!("parse '{}' to Date(1904) failed", self.value)), + CellType::NumberTime1900 | CellType::NumberTime1904 => to_time_string(&self.value) + .map_err(|_| format!("parse '{}' to Time failed", self.value)), + CellType::IsoDateTime => Ok(self.value.replace("T", " ")), + CellType::IsoDuration => Ok(self .value .replace("PT", "") .replace("H", ":") .replace("M", ":") - .replace("S", ""), - _ => self.value.to_owned(), - }; + .replace("S", "")), + _ => Ok(self.value.to_owned()), + } + } +} + +impl Display for Cell { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let value = self.to_display_string().unwrap_or_else(|_| self.value.to_owned()); write!(f, "{}", value) } } diff --git a/src/spreadsheet/mod.rs b/src/spreadsheet/mod.rs index 6ab6f4d..2a49881 100644 --- a/src/spreadsheet/mod.rs +++ b/src/spreadsheet/mod.rs @@ -27,6 +27,100 @@ pub(crate) mod xlsx; pub(crate) mod criteria; pub(crate) mod sheet; +pub(crate) fn resolve_number_format( + number_formats: &[CellType], + file_name: &str, + sheet_name: &str, + row: usize, + col: usize, + style_index: usize, +) -> Result { + number_formats.get(style_index).copied().ok_or_else(|| { + SpreadsheetError::CellStyleIndexError( + file_name.to_owned(), + sheet_name.to_owned(), + crate::spreadsheet::reference::index_to_reference(row, col), + style_index, + number_formats.len(), + ) + }) +} + +#[cfg(test)] +mod number_format_tests { + use super::*; + + #[test] + fn invalid_style_with_out_of_range_column_returns_error() { + let error = resolve_number_format( + &[CellType::Number], + "workbook.xlsb", + "Sheet1", + 10, + 16_384, + 999, + ) + .unwrap_err(); + + assert_eq!( + error.to_string(), + "Cell '[workbook.xlsb]Sheet1!R11C16385': invalid style index 999; workbook defines 1 styles" + ); + } +} + +pub(crate) fn resolve_shared_string<'a>( + shared_strings: &'a [String], + mappings: &HashMap, + file_name: &str, + sheet_name: &str, + cell: &Cell, +) -> Result<&'a str, RustySheetError> { + let shared_string_index = cell.value.parse::()?; + let mapped_index = mappings.get(&shared_string_index).copied().ok_or_else(|| { + SpreadsheetError::CellSharedStringIndexError( + file_name.to_owned(), + sheet_name.to_owned(), + cell.reference(), + shared_string_index, + ) + })?; + + shared_strings + .get(mapped_index) + .map(String::as_str) + .ok_or_else(|| { + SpreadsheetError::CellSharedStringIndexError( + file_name.to_owned(), + sheet_name.to_owned(), + cell.reference(), + shared_string_index, + ) + .into() + }) +} + +pub(crate) fn resolve_loaded_shared_string<'a>( + shared_strings: &'a [Option], + file_name: &str, + sheet_name: &str, + cell: &Cell, +) -> Result, RustySheetError> { + let shared_string_index = cell.value.parse::()?; + shared_strings + .get(shared_string_index) + .map(Option::as_deref) + .ok_or_else(|| { + SpreadsheetError::CellSharedStringIndexError( + file_name.to_owned(), + sheet_name.to_owned(), + cell.reference(), + shared_string_index, + ) + .into() + }) +} + #[derive(Error, Debug)] pub(crate) enum SpreadsheetError { /// Error indicating the spreadsheet format is not supported @@ -48,6 +142,18 @@ pub(crate) enum SpreadsheetError { /// Error indicating a specific cell value is invalid #[error("Cell '[{0}]{1}!{2}': {3}")] CellValueError(String, String, String, String), + + /// Error indicating a cell style index is outside the workbook's style table. + #[error("Cell '[{0}]{1}!{2}': invalid style index {3}; workbook defines {4} styles")] + CellStyleIndexError(String, String, String, usize, usize), + + /// Error indicating a shared string index is outside the workbook's shared string table. + #[error("Cell '[{0}]{1}!{2}': invalid shared string index {3}")] + CellSharedStringIndexError(String, String, String, usize), + + /// Error indicating a worksheet cell reference is outside the supported spreadsheet bounds. + #[error("Sheet '[{0}]{1}': invalid cell reference '{2}'")] + CellReferenceError(String, String, String), } pub(crate) trait Spreadsheet { @@ -109,56 +215,78 @@ pub(crate) trait Spreadsheet { let (shared_strings, mappings) = self.load_shared_strings(Some(shared_indexes))?; let mut tables = Vec::::new(); - for (name, header, data, row_lower_bound, col_lower_bound, col_upper_bound) in sheets.into_iter() { - let names = (col_lower_bound..=col_upper_bound).map(|col| { - let index = col - col_lower_bound; - if let Some(cell) = &header[index] { - let value = if cell.kind == CellType::SharedString { - let id = cell.value.parse::().expect("Shared string index"); - let index = mappings[&id]; - shared_strings[index].to_owned() - } else { - cell.to_string() - }; - if !criteria.nulls.contains(&value) { - value + let file_name = self.name(); + for (sheet_name, header, data, row_lower_bound, col_lower_bound, col_upper_bound) in sheets.into_iter() { + let names = (col_lower_bound..=col_upper_bound) + .map(|col| -> Result { + let index = col - col_lower_bound; + if let Some(cell) = &header[index] { + let value = if cell.kind == CellType::SharedString { + resolve_shared_string( + &shared_strings, + &mappings, + &file_name, + &sheet_name, + cell, + )?.to_owned() + } else { + cell.to_display_string().map_err(|message| { + SpreadsheetError::CellValueError( + file_name.to_owned(), + sheet_name.to_owned(), + cell.reference(), + message, + ) + })? + }; + Ok(if !criteria.nulls.contains(&value) { + value + } else { + index_to_col(col).to_owned() + }) } else { - index_to_col(col).to_owned() + Ok(index_to_col(col).to_owned()) } - } else { - index_to_col(col).to_owned() - } - }).collect::>(); + }) + .collect::, _>>()?; let columns = names.iter().zip(data) - .map(|(name, cells)| { + .map(|(column_name, cells)| -> Result { let types = cells.iter() - .map(|cell| { + .map(|cell| -> Result, RustySheetError> { if cell.kind == CellType::SharedString { - let id = cell.value.parse::().expect("Shared string index"); - let index = mappings[&id]; - let value = shared_strings[index].as_str(); - ColumnType::from(if criteria.nulls.contains(value) { - &CellType::Empty - } else { - &cell.kind - }, value) + let value = resolve_shared_string( + &shared_strings, + &mappings, + &file_name, + &sheet_name, + cell, + )?; + Ok(ColumnType::from( + if criteria.nulls.contains(value) { + &CellType::Empty + } else { + &cell.kind + }, + value, + )) } else { - ColumnType::from(&cell.kind, &cell.value) + Ok(ColumnType::from(&cell.kind, &cell.value)) } }) - .collect::>(); - Column { - name: name.to_owned(), - kind: presets.iter() - .find(|(pattern, _)| pattern.matches(name)) + .collect::, _>>()?; + Ok(Column { + name: column_name.to_owned(), + kind: presets + .iter() + .find(|(pattern, _)| pattern.matches(column_name)) .map(|(_, kind)| kind.to_owned()) .unwrap_or(ColumnType::detect(types)), - } + }) }) - .collect::>(); + .collect::, _>>()?; tables.push(Table { - name, + name: sheet_name, columns, row_lower_bound, col_lower_bound, diff --git a/src/spreadsheet/reference.rs b/src/spreadsheet/reference.rs index ce9a7f6..c409022 100644 --- a/src/spreadsheet/reference.rs +++ b/src/spreadsheet/reference.rs @@ -1,3 +1,6 @@ +const MAX_COL_INDEX: usize = 16_383; +const MAX_ROW_NUMBER: usize = 1_048_576; + /// Converts a zero-based column index to Excel column letter notation /// /// Uses a precomputed lookup table for optimal performance. Supports @@ -44,40 +47,12 @@ pub(crate) fn col_to_index(letters: &str) -> Option { let mut column = 0usize; for char in letters.chars() { if 'A' <= char && char <= 'Z' { - column = column * 26 + char as usize - 64; + column = column.checked_mul(26)?.checked_add(char as usize - 64)?; } else { break; } } - if column > 0 { - Some(column - 1) - } else { - None - } -} - -/// Converts a zero-based row index to Excel row number string -/// -/// Transforms internal zero-based row indexing into Excel's 1-based row numbering system. -/// This conversion bridges the gap between internal storage (0-based) and Excel's -/// user-facing representation (1-based). -/// -/// The conversion is straightforward: adds 1 to the zero-based index and converts -/// the result to a string. This maintains compatibility with Excel's row numbering -/// convention while using efficient zero-based indexing internally. -/// -/// # Arguments -/// * `index` - Zero-based row index (0 = row 1, 1 = row 2, etc.) -/// -/// # Returns -/// Excel-style row number as a string (1-based) -/// -/// # Examples -/// - 0 -> "1" // First row -/// - 1 -> "2" // Second row -/// - 99 -> "100" // 100th row -pub(crate) fn index_to_row(index: usize) -> String { - (index + 1).to_string() + zero_based_column(column) } /// Converts an Excel row number string to a zero-based row index @@ -89,22 +64,25 @@ pub(crate) fn index_to_row(index: usize) -> String { /// `Some(usize)` containing the zero-based row index if valid, /// `None` if the input cannot be parsed as a valid row number pub(crate) fn row_to_index(row: &str) -> Option { - row.parse::().ok().map(|row| row - 1) + row.parse::().ok().and_then(zero_based_row) } -/// Converts zero-based row and column indices to an Excel-style cell reference +/// Converts zero-based row and column indices to a cell reference /// /// # Arguments /// * `row_index` - Zero-based row index (0 = row 1) /// * `col_index` - Zero-based column index (0 = column "A") /// /// # Returns -/// Excel-style cell reference as a string (e.g., "A1", "B2", "AB100") +/// A1 notation for valid Excel coordinates, or R1C1 notation for out-of-range +/// coordinates so malformed-file error reporting remains panic-free. pub(crate) fn index_to_reference(row_index: usize, col_index: usize) -> String { - let mut reference = String::new(); - reference.push_str(index_to_col(col_index)); - reference.push_str(&index_to_row(row_index)); - reference + let row_number = row_index.saturating_add(1); + if let Some(column) = INDEXES_TO_COLUMNS.get(col_index) { + format!("{column}{row_number}") + } else { + format!("R{row_number}C{}", col_index.saturating_add(1)) + } } /// Converts an Excel-style cell reference to zero-based row and column indices @@ -135,17 +113,32 @@ pub(crate) fn index_to_reference(row_index: usize, col_index: usize) -> String { pub(crate) fn reference_to_index(letters: &str) -> Option<(usize, usize)> { let mut column = 0usize; let mut row = 0usize; + let mut row_started = false; for char in letters.chars() { - if 'A' <= char && char <= 'Z' { - column = column * 26 + char as usize - 64; + if 'A' <= char && char <= 'Z' && !row_started { + column = column.checked_mul(26)?.checked_add(char as usize - 64)?; } else if '0' <= char && char <= '9' { - row = row * 10 + char as usize - 48; + row_started = true; + row = row.checked_mul(10)?.checked_add(char as usize - 48)?; } else { - break; + return None; } } - if column > 0 { - Some((row - 1, column - 1)) + Some((zero_based_row(row)?, zero_based_column(column)?)) +} + +fn zero_based_column(column: usize) -> Option { + let index = column.checked_sub(1)?; + if index <= MAX_COL_INDEX { + Some(index) + } else { + None + } +} + +fn zero_based_row(row: usize) -> Option { + if (1..=MAX_ROW_NUMBER).contains(&row) { + Some(row - 1) } else { None } @@ -1418,4 +1411,26 @@ const INDEXES_TO_COLUMNS: [&'static str; 16384] = [ "XDS", "XDT", "XDU", "XDV", "XDW", "XDX", "XDY", "XDZ", "XEA", "XEB", "XEC", "XED", "XEE", "XEF", "XEG", "XEH", "XEI", "XEJ", "XEK", "XEL", "XEM", "XEN", "XEO", "XEP", "XEQ", "XER", "XES", "XET", "XEU", "XEV", "XEW", "XEX", "XEY", "XEZ", "XFA", "XFB", "XFC", "XFD", -]; \ No newline at end of file +]; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_reference_bounds() { + assert_eq!(col_to_index("XFD"), Some(16_383)); + assert_eq!(row_to_index("1048576"), Some(1_048_575)); + assert_eq!(reference_to_index("XFD1048576"), Some((1_048_575, 16_383))); + } + + #[test] + fn rejects_out_of_bounds_references() { + assert_eq!(col_to_index("XFE"), None); + assert_eq!(row_to_index("0"), None); + assert_eq!(row_to_index("1048577"), None); + assert_eq!(reference_to_index("A0"), None); + assert_eq!(reference_to_index("XFE1"), None); + assert_eq!(reference_to_index("A1048577"), None); + } +} diff --git a/src/spreadsheet/xls.rs b/src/spreadsheet/xls.rs index 0e68ef1..02119c4 100644 --- a/src/spreadsheet/xls.rs +++ b/src/spreadsheet/xls.rs @@ -10,6 +10,7 @@ use crate::spreadsheet::cell::CellType; use crate::spreadsheet::criteria::Criteria; use crate::spreadsheet::excel::load_number_formats; use crate::spreadsheet::reference::index_to_reference; +use crate::spreadsheet::resolve_number_format; use crate::spreadsheet::sheet::Sheet; use crate::spreadsheet::Spreadsheet; use crate::spreadsheet::SpreadsheetError; @@ -201,7 +202,14 @@ impl Spreadsheet for XlsSpreadsheet { } last_row = Some(row); let index = self.reader.read_u16()? as usize; - let kind = self.number_formats[index]; + let kind = resolve_number_format( + &self.number_formats, + &sheet.file_name, + &sheet.name, + row, + col, + index, + )?; let value = self.reader.read_rk_number()?; sheet.push(Cell { row, @@ -234,7 +242,14 @@ impl Spreadsheet for XlsSpreadsheet { }; let kind = match either { Either::Left(kind) => kind, - Either::Right(index) => self.number_formats[index], + Either::Right(index) => resolve_number_format( + &self.number_formats, + &sheet.file_name, + &sheet.name, + row, + col, + index, + )?, }; if kind != CellType::Error { if !criteria.nulls.contains(&value) { diff --git a/src/spreadsheet/xlsb.rs b/src/spreadsheet/xlsb.rs index bb1231e..11396ef 100644 --- a/src/spreadsheet/xlsb.rs +++ b/src/spreadsheet/xlsb.rs @@ -10,6 +10,7 @@ use crate::spreadsheet::criteria::Criteria; use crate::spreadsheet::excel; use crate::spreadsheet::excel::load_relationships; use crate::spreadsheet::reference::index_to_reference; +use crate::spreadsheet::resolve_number_format; use crate::spreadsheet::sheet::Sheet; use crate::spreadsheet::Spreadsheet; use crate::spreadsheet::SpreadsheetError; @@ -134,7 +135,7 @@ impl Spreadsheet for XlsbSpreadsheet { }; reader.find(BRT_BEGIN_SST)?; - for id in 0..reader.get_usize(4) { + for id in 0..reader.get_usize(4)? { reader.find_with(BRT_SST_ITEM, &[(BRT_FRT_BEGIN, BRT_FRT_END)])?; if let Some(keys) = &mut indexes { if keys.contains(&id) { @@ -189,7 +190,7 @@ impl Spreadsheet for XlsbSpreadsheet { match tag { BRT_END_SHEET_DATA => break, BRT_ROW_HDR => { - row = reader.get_usize(0); + row = reader.get_usize(0)?; if sheet.after_row_upper_bound(row) { break; } @@ -202,7 +203,7 @@ impl Spreadsheet for XlsbSpreadsheet { | BRT_CELL_ISST | BRT_CELL_ERROR | BRT_FMLA_ERROR if !sheet.before_row_lower_bound(row) => { - let col = reader.get_usize(0); + let col = reader.get_usize(0)?; if sheet.contains(row, col) { if let Some(last_row) = last_row { if criteria.end_at_empty_row && ((sheet.is_empty() && last_row != row) || (!sheet.is_empty() && last_row + 1 < row)) { @@ -211,17 +212,24 @@ impl Spreadsheet for XlsbSpreadsheet { } last_row = Some(row); let (either, value) = match tag { - BRT_CELL_BOOL | BRT_FMLA_BOOL => read_bool_cell(&mut reader), - BRT_CELL_REAL | BRT_FMLA_NUM => read_real_cell(&mut reader), + BRT_CELL_BOOL | BRT_FMLA_BOOL => read_bool_cell(&mut reader)?, + BRT_CELL_REAL | BRT_FMLA_NUM => read_real_cell(&mut reader)?, BRT_CELL_ST | BRT_FMLA_STRING => read_st_cell(&mut reader)?, BRT_CELL_R_STRING => read_rich_string_cell(&mut reader)?, - BRT_CELL_ISST => read_shared_string_cell(&mut reader), - BRT_CELL_ERROR | BRT_FMLA_ERROR => read_error_cell(&mut reader), - _ => read_rk_cell(&mut reader), + BRT_CELL_ISST => read_shared_string_cell(&mut reader)?, + BRT_CELL_ERROR | BRT_FMLA_ERROR => read_error_cell(&mut reader)?, + _ => read_rk_cell(&mut reader)?, }; let kind = match either { Either::Left(kind) => kind, - Either::Right(index) => (self.number_formats)[index], + Either::Right(index) => resolve_number_format( + &self.number_formats, + &sheet.file_name, + &sheet.name, + row, + col, + index, + )?, }; if kind != CellType::Error { if !criteria.nulls.contains(&value) { @@ -282,7 +290,7 @@ fn load_workbook(zip: &mut ZipArchive) -> Result<(Vec<(String, St } } BRT_WB_PROP => { - is_1904 = (&reader.buffer[0] & 0x1) != 0; + is_1904 = (reader.get_u8(0)? & 0x1) != 0; } }); Ok((sheets, is_1904)) @@ -309,9 +317,9 @@ fn load_number_formats(zip: &mut ZipArchive, is_1904: bool) -> Re let mut format_indexes: Vec = Vec::new(); match_biff12_record!(reader => { BRT_BEGIN_FMTS => { - for _ in 0..reader.get_usize(0) { + for _ in 0..reader.get_usize(0)? { reader.find(BRT_FMT)?; - let id = reader.get_u16(0); + let id = reader.get_u16(0)?; let format = reader.get_str(2)?; custom_formats.insert( id.to_string(), @@ -320,9 +328,9 @@ fn load_number_formats(zip: &mut ZipArchive, is_1904: bool) -> Re } } BRT_BEGIN_CELL_XFS => { - for _ in 0..reader.get_usize(0) { + for _ in 0..reader.get_usize(0)? { reader.find(BRT_XF)?; - let id = reader.get_u16(2); + let id = reader.get_u16(2)?; format_indexes.push(id.to_string()); } break; @@ -341,9 +349,11 @@ fn load_number_formats(zip: &mut ZipArchive, is_1904: bool) -> Re /// * `(Either, String)` - Tuple containing: /// - Cell type (boolean) and format index /// - String representation of boolean value ("1" or "0") -fn read_bool_cell(reader: &mut Biff12Reader>>) -> (Either, String) { - let value = if reader.buffer[8] != 0 { "1" } else { "0" }; - (Either::Left(CellType::Boolean), value.to_owned()) +fn read_bool_cell( + reader: &mut Biff12Reader>>, +) -> Result<(Either, String), RustySheetError> { + let value = if reader.get_u8(8)? != 0 { "1" } else { "0" }; + Ok((Either::Left(CellType::Boolean), value.to_owned())) } /// Reads a real number (double precision) cell value from BIFF12 data @@ -355,10 +365,12 @@ fn read_bool_cell(reader: &mut Biff12Reader>>) /// * `(Either, String)` - Tuple containing: /// - Format index reference and cell type /// - String representation of numeric value -fn read_real_cell(reader: &mut Biff12Reader>>) -> (Either, String) { - let index = reader.get_style(4); - let value = reader.get_f64(8).to_string(); - (Either::Right(index), value) +fn read_real_cell( + reader: &mut Biff12Reader>>, +) -> Result<(Either, String), RustySheetError> { + let index = reader.get_style(4)?; + let value = reader.get_f64(8)?.to_string(); + Ok((Either::Right(index), value)) } /// Reads an inline string cell value from BIFF12 data @@ -398,9 +410,11 @@ fn read_rich_string_cell(reader: &mut Biff12Reader, String)` - Tuple containing: /// - Cell type (shared string) and format index /// - String representation of shared string index -fn read_shared_string_cell(reader: &mut Biff12Reader>>) -> (Either, String) { - let value = reader.get_usize(8).to_string(); - (Either::Left(CellType::SharedString), value) +fn read_shared_string_cell( + reader: &mut Biff12Reader>>, +) -> Result<(Either, String), RustySheetError> { + let value = reader.get_usize(8)?.to_string(); + Ok((Either::Left(CellType::SharedString), value)) } /// Reads an error cell value from BIFF12 data @@ -412,9 +426,11 @@ fn read_shared_string_cell(reader: &mut Biff12Reader, String)` - Tuple containing: /// - Cell type (error) and format index /// - String representation of error value -fn read_error_cell(reader: &mut Biff12Reader>>) -> (Either, String) { - let value = to_error_value(reader.buffer[8]).to_owned(); - (Either::Left(CellType::Error), value) +fn read_error_cell( + reader: &mut Biff12Reader>>, +) -> Result<(Either, String), RustySheetError> { + let value = to_error_value(reader.get_u8(8)?).to_owned(); + Ok((Either::Left(CellType::Error), value)) } /// Reads an RK (compressed floating point) cell value from BIFF12 data @@ -429,16 +445,19 @@ fn read_error_cell(reader: &mut Biff12Reader>>) /// * `(Either, String)` - Tuple containing: /// - Format index reference and cell type /// - String representation of decompressed numeric value -fn read_rk_cell(reader: &mut Biff12Reader>>) -> (Either, String) { - let index = reader.get_style(4); - let is_percentage = (reader.buffer[8] & 0x01) != 0; - let is_integer = (reader.buffer[8] & 0x02) != 0; - reader.buffer[8] &= 0xFC; // Clear A and B flag bits +fn read_rk_cell( + reader: &mut Biff12Reader>>, +) -> Result<(Either, String), RustySheetError> { + let index = reader.get_style(4)?; + let flags = reader.get_u8(8)?; + let is_percentage = (flags & 0x01) != 0; + let is_integer = (flags & 0x02) != 0; + reader.buffer[8] = flags & 0xFC; // Clear A and B flag bits let mut value = if is_integer { - (reader.get_i32(8) >> 2) as f64 + (reader.get_i32(8)? >> 2) as f64 } else { - let value = (reader.get_u32(8) >> 2) as u64; + let value = (reader.get_u32(8)? >> 2) as u64; f64::from_bits(value << 34) }; if is_percentage { @@ -450,5 +469,5 @@ fn read_rk_cell(reader: &mut Biff12Reader>>) -> value.to_string() }; - (Either::Right(index), value) - } \ No newline at end of file + Ok((Either::Right(index), value)) +} diff --git a/src/spreadsheet/xlsx.rs b/src/spreadsheet/xlsx.rs index 026954b..b0fe368 100644 --- a/src/spreadsheet/xlsx.rs +++ b/src/spreadsheet/xlsx.rs @@ -15,6 +15,7 @@ use crate::spreadsheet::excel; use crate::spreadsheet::excel::load_relationships; use crate::spreadsheet::reference::index_to_reference; use crate::spreadsheet::reference::reference_to_index; +use crate::spreadsheet::resolve_number_format; use crate::spreadsheet::sheet::Sheet; use quick_xml::events::Event; use quick_xml::name::QName; @@ -207,9 +208,17 @@ impl Spreadsheet for XlsxSpreadsheet { col_count = 0; } Event::Start(event) if event.name() == TAG_CELL => { - (row, col) = event.get_attribute_value("r")? - .and_then(|reference| reference_to_index(&reference)) - .unwrap_or((row_count, col_count)); + if let Some(reference) = event.get_attribute_value("r")? { + (row, col) = reference_to_index(&reference).ok_or_else(|| { + SpreadsheetError::CellReferenceError( + sheet.file_name.to_owned(), + sheet.name.to_owned(), + reference.to_string(), + ) + })?; + } else { + (row, col) = (row_count, col_count); + } col_count += 1; if sheet.after_row_upper_bound(row) { break; @@ -231,7 +240,14 @@ impl Spreadsheet for XlsxSpreadsheet { if let Some(format_id) = event.get_attribute_value("s")? { if kind == CellType::Number && !format_id.is_empty() { let index = format_id.parse::()?; - kind = self.number_formats[index]; + kind = resolve_number_format( + &self.number_formats, + &sheet.file_name, + &sheet.name, + row, + col, + index, + )?; } } } else { @@ -519,3 +535,347 @@ fn read_string_value( }); Ok(text) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::spreadsheet::criteria::Criteria; + use std::collections::HashSet; + use std::fs::File; + use std::io::Write; + use std::path::Path; + use std::path::PathBuf; + use std::time::SystemTime; + use std::time::UNIX_EPOCH; + use zip::write::SimpleFileOptions; + use zip::ZipWriter; + + #[test] + fn invalid_style_index_returns_error() { + let path = invalid_workbook_path("style"); + write_invalid_style_workbook(&path); + + let mut spreadsheet = XlsxSpreadsheet::open(path.to_str().unwrap()).unwrap(); + let result = spreadsheet.read_sheets(&default_criteria()); + + std::fs::remove_file(path).unwrap(); + let error = match result { + Ok(_) => panic!("expected invalid style index error"), + Err(error) => error.to_string(), + }; + assert!(error.contains("Sheet1!A11")); + assert!(error.contains("invalid style index 999")); + assert!(error.contains("workbook defines 1 styles")); + } + + #[test] + fn invalid_shared_string_index_returns_error() { + let path = invalid_workbook_path("shared-string"); + write_invalid_shared_string_workbook(&path); + + let mut spreadsheet = XlsxSpreadsheet::open(path.to_str().unwrap()).unwrap(); + let result = spreadsheet.analyze_sheets(true, &default_criteria(), &Vec::new()); + + std::fs::remove_file(path).unwrap(); + let error = match result { + Ok(_) => panic!("expected invalid shared string index error"), + Err(error) => error.to_string(), + }; + assert!(error.contains("Sheet1!A1")); + assert!(error.contains("invalid shared string index 999")); + } + + #[test] + fn invalid_cell_reference_returns_error() { + let path = invalid_workbook_path("cell-reference"); + write_invalid_cell_reference_workbook(&path); + + let mut spreadsheet = XlsxSpreadsheet::open(path.to_str().unwrap()).unwrap(); + let result = spreadsheet.read_sheets(&default_criteria()); + + std::fs::remove_file(path).unwrap(); + let error = match result { + Ok(_) => panic!("expected invalid cell reference error"), + Err(error) => error.to_string(), + }; + assert!(error.contains("Sheet1")); + assert!(error.contains("invalid cell reference 'XFE1'")); + } + + #[test] + fn invalid_date_header_value_returns_error() { + let path = invalid_workbook_path("date-header"); + write_invalid_date_header_workbook(&path); + + let mut spreadsheet = XlsxSpreadsheet::open(path.to_str().unwrap()).unwrap(); + let result = spreadsheet.analyze_sheets(true, &default_criteria(), &Vec::new()); + + std::fs::remove_file(path).unwrap(); + let error = match result { + Ok(_) => panic!("expected invalid date header value error"), + Err(error) => error.to_string(), + }; + assert!(error.contains("Sheet1!A1")); + assert!(error.contains("parse 'not-a-number' to Date(1900) failed")); + } + + fn invalid_workbook_path(kind: &str) -> PathBuf { + let id = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + std::env::temp_dir().join(format!("rusty-sheet-invalid-{kind}-{id}.xlsx")) + } + + fn default_criteria() -> Criteria { + Criteria { + sheet_name_patterns: None, + sheet_limit: None, + range: None, + rows_limit: None, + nulls: HashSet::from(["".to_string()]), + error_as_null: false, + skip_empty_rows: false, + end_at_empty_row: false, + spread_merged_cells: false, + } + } + + fn write_invalid_style_workbook(path: &Path) { + let file = File::create(path).unwrap(); + let mut zip = ZipWriter::new(file); + let options = + SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated); + let rows = (2..11) + .map(|row| format!("{row}")) + .collect::(); + let sheet = format!( + r#" + + value{rows}11 +"# + ); + + for (name, content) in [ + ( + "[Content_Types].xml", + r#" + + + + + + +"#.to_string(), + ), + ( + "_rels/.rels", + r#" + + +"#.to_string(), + ), + ( + "xl/workbook.xml", + r#" + + +"#.to_string(), + ), + ( + "xl/_rels/workbook.xml.rels", + r#" + + + +"#.to_string(), + ), + ( + "xl/styles.xml", + r#" + + +"#.to_string(), + ), + ("xl/worksheets/sheet1.xml", sheet), + ] { + zip.start_file(name, options).unwrap(); + zip.write_all(content.as_bytes()).unwrap(); + } + zip.finish().unwrap(); + } + + fn write_invalid_shared_string_workbook(path: &Path) { + let file = File::create(path).unwrap(); + let mut zip = ZipWriter::new(file); + let options = + SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated); + + for (name, content) in [ + ( + "[Content_Types].xml", + r#" + + + + + + +"#.to_string(), + ), + ( + "_rels/.rels", + r#" + + +"#.to_string(), + ), + ( + "xl/workbook.xml", + r#" + + +"#.to_string(), + ), + ( + "xl/_rels/workbook.xml.rels", + r#" + + + +"#.to_string(), + ), + ( + "xl/sharedStrings.xml", + r#" + + name +"#.to_string(), + ), + ( + "xl/worksheets/sheet1.xml", + r#" + + 999 +"#.to_string(), + ), + ] { + zip.start_file(name, options).unwrap(); + zip.write_all(content.as_bytes()).unwrap(); + } + zip.finish().unwrap(); + } + + fn write_invalid_cell_reference_workbook(path: &Path) { + let file = File::create(path).unwrap(); + let mut zip = ZipWriter::new(file); + let options = + SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated); + + for (name, content) in [ + ( + "[Content_Types].xml", + r#" + + + + + +"#.to_string(), + ), + ( + "_rels/.rels", + r#" + + +"#.to_string(), + ), + ( + "xl/workbook.xml", + r#" + + +"#.to_string(), + ), + ( + "xl/_rels/workbook.xml.rels", + r#" + + +"#.to_string(), + ), + ( + "xl/worksheets/sheet1.xml", + r#" + + value +"#.to_string(), + ), + ] { + zip.start_file(name, options).unwrap(); + zip.write_all(content.as_bytes()).unwrap(); + } + zip.finish().unwrap(); + } + + fn write_invalid_date_header_workbook(path: &Path) { + let file = File::create(path).unwrap(); + let mut zip = ZipWriter::new(file); + let options = + SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated); + + for (name, content) in [ + ( + "[Content_Types].xml", + r#" + + + + + + +"#.to_string(), + ), + ( + "_rels/.rels", + r#" + + +"#.to_string(), + ), + ( + "xl/workbook.xml", + r#" + + +"#.to_string(), + ), + ( + "xl/_rels/workbook.xml.rels", + r#" + + + +"#.to_string(), + ), + ( + "xl/styles.xml", + r#" + + +"#.to_string(), + ), + ( + "xl/worksheets/sheet1.xml", + r#" + + not-a-number +"#.to_string(), + ), + ] { + zip.start_file(name, options).unwrap(); + zip.write_all(content.as_bytes()).unwrap(); + } + zip.finish().unwrap(); + } +}