diff --git a/src/lib.rs b/src/lib.rs index e869c490..9333ec5a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -169,17 +169,21 @@ pub struct Dimensions { #[allow(clippy::len_without_is_empty)] impl Dimensions { - /// create dimensions info with start position and end position + /// Create a `Dimensions` instance with start and end cell positions. pub fn new(start: (u32, u32), end: (u32, u32)) -> Self { - Self { start, end } + Self { + start: (start.0.min(end.0), start.1.min(end.1)), + end: (start.0.max(end.0), start.1.max(end.1)), + } } - /// check if a position is in it + /// Check if a cell position is within the `Dimensions` range. pub fn contains(&self, row: u32, col: u32) -> bool { row >= self.start.0 && row <= self.end.0 && col >= self.start.1 && col <= self.end.1 } - /// len + /// Get the `len()` of the `Dimensions` range, which is equivalent to the cell area. pub fn len(&self) -> u64 { - (self.end.0 - self.start.0 + 1) as u64 * (self.end.1 - self.start.1 + 1) as u64 + // Widened before the `+ 1` so a full-width axis cannot overflow. + (u64::from(self.end.0 - self.start.0) + 1) * (u64::from(self.end.1 - self.start.1) + 1) } } diff --git a/src/xlsx/mod.rs b/src/xlsx/mod.rs index bd3854ec..8acbb632 100644 --- a/src/xlsx/mod.rs +++ b/src/xlsx/mod.rs @@ -2789,18 +2789,21 @@ pub(crate) fn get_dimension(dimension: &[u8]) -> Result { end: parts[0], }), 2 => { - let rows = parts[1].0 - parts[0].0; - let columns = parts[1].1 - parts[0].1; - if rows > MAX_ROWS { - warn!("xlsx has more than maximum number of rows ({rows} > {MAX_ROWS})"); + // The `ref` may be in reversed order like `C5:A1`. + let dim = Dimensions::new(parts[0], parts[1]); + if dim.end.0 > MAX_ROWS { + warn!( + "file has more than maximum supported number of rows ({} > {MAX_ROWS})", + dim.end.0 + ); } - if columns > MAX_COLUMNS { - warn!("xlsx has more than maximum number of columns ({columns} > {MAX_COLUMNS})"); + if dim.end.1 > MAX_COLUMNS { + warn!( + "file has more than maximum supported number of columns ({} > {MAX_COLUMNS})", + dim.end.1 + ); } - Ok(Dimensions { - start: parts[0], - end: parts[1], - }) + Ok(dim) } len => Err(XlsxError::DimensionCount(len)), } @@ -4010,6 +4013,48 @@ mod tests { ); } + #[test] + fn test_reversed_dimension_is_normalised() { + // A reversed `ref` such as `C5:A1` gives the same `Dimensions` as `A1:C5`. + let reversed = get_dimension(b"C5:A1").unwrap(); + assert_eq!( + reversed, + Dimensions { + start: (0, 0), + end: (4, 2), + } + ); + assert_eq!(reversed, get_dimension(b"A1:C5").unwrap()); + assert_eq!(reversed.len(), 15); + + // Reversed on one axis only. + assert_eq!( + get_dimension(b"A5:C1").unwrap(), + get_dimension(b"A1:C5").unwrap() + ); + assert_eq!( + get_dimension(b"C1:A5").unwrap(), + get_dimension(b"A1:C5").unwrap() + ); + } + + #[test] + fn test_dimensions_new_normalises_order() { + let dim = Dimensions::new((4, 2), (0, 0)); + assert_eq!( + dim, + Dimensions { + start: (0, 0), + end: (4, 2), + } + ); + assert_eq!(dim.len(), 15); + assert_eq!(Dimensions::new((7, 7), (7, 7)).len(), 1); + + // A full axis must not overflow the `+ 1` in `len()`. + assert_eq!(Dimensions::new((0, 0), (u32::MAX, 0)).len(), 4_294_967_296); + } + #[test] fn test_parse_error() { assert_eq!(