From 51c130c3b2a9a666839e8027eef3bc6a9f3f59e9 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Sun, 26 Jul 2026 21:10:24 -0400 Subject: [PATCH 1/3] fix(xlsx): normalise reversed dimension refs in Dimensions::new Reversed refs like C5:A1 underflowed the u32 extent arithmetic in get_dimension and Dimensions::len. The constructor now stores the corners ordered so start <= end on both axes. --- src/lib.rs | 11 +++++++-- src/xlsx/mod.rs | 65 +++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e869c490..72697446 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -170,8 +170,14 @@ pub struct Dimensions { #[allow(clippy::len_without_is_empty)] impl Dimensions { /// create dimensions info with start position and end position + /// + /// The corners may be given in either order; they are stored so that + /// `start <= end` on both axes. 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 pub fn contains(&self, row: u32, col: u32) -> bool { @@ -179,7 +185,8 @@ impl Dimensions { } /// len 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..f0884481 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!( + "xlsx has more than maximum 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!( + "xlsx has more than maximum 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); + // A single cell is still one cell, and a full-width axis does not + // overflow the `+ 1`. + assert_eq!(Dimensions::new((7, 7), (7, 7)).len(), 1); + assert_eq!(Dimensions::new((0, 0), (u32::MAX, 0)).len(), 4_294_967_296); + } + #[test] fn test_parse_error() { assert_eq!( From 87e44c6602722f80e2f038c834344dfeb3885a7f Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Sun, 2 Aug 2026 09:16:13 -0400 Subject: [PATCH 2/3] xlsx: apply review wording for dimension warnings and Dimensions docs --- src/lib.rs | 9 +++------ src/xlsx/mod.rs | 7 +++---- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 72697446..9333ec5a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -169,21 +169,18 @@ pub struct Dimensions { #[allow(clippy::len_without_is_empty)] impl Dimensions { - /// create dimensions info with start position and end position - /// - /// The corners may be given in either order; they are stored so that - /// `start <= end` on both axes. + /// Create a `Dimensions` instance with start and end cell positions. pub fn new(start: (u32, u32), end: (u32, u32)) -> Self { 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 { // 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 f0884481..14e2ecf0 100644 --- a/src/xlsx/mod.rs +++ b/src/xlsx/mod.rs @@ -2793,13 +2793,13 @@ pub(crate) fn get_dimension(dimension: &[u8]) -> Result { let dim = Dimensions::new(parts[0], parts[1]); if dim.end.0 > MAX_ROWS { warn!( - "xlsx has more than maximum number of rows ({} > {MAX_ROWS})", + "file has more than maximum supported number of rows ({} > {MAX_ROWS})", dim.end.0 ); } if dim.end.1 > MAX_COLUMNS { warn!( - "xlsx has more than maximum number of columns ({} > {MAX_COLUMNS})", + "file has more than maximum supported number of columns ({} > {MAX_COLUMNS})", dim.end.1 ); } @@ -4049,8 +4049,7 @@ mod tests { } ); assert_eq!(dim.len(), 15); - // A single cell is still one cell, and a full-width axis does not - // overflow the `+ 1`. + // A single cell is one cell; a full-width axis does not overflow. assert_eq!(Dimensions::new((7, 7), (7, 7)).len(), 1); assert_eq!(Dimensions::new((0, 0), (u32::MAX, 0)).len(), 4_294_967_296); } From 9f2e90688b3000cdaa27c47de6b37657416c7065 Mon Sep 17 00:00:00 2001 From: Kristian Rickert Date: Tue, 11 Aug 2026 23:05:28 -0400 Subject: [PATCH 3/3] xlsx: tidy a test comment --- src/xlsx/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/xlsx/mod.rs b/src/xlsx/mod.rs index 14e2ecf0..8acbb632 100644 --- a/src/xlsx/mod.rs +++ b/src/xlsx/mod.rs @@ -4049,8 +4049,9 @@ mod tests { } ); assert_eq!(dim.len(), 15); - // A single cell is one cell; a full-width axis does not overflow. 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); }