Skip to content

Commit 51c130c

Browse files
committed
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.
1 parent f059beb commit 51c130c

2 files changed

Lines changed: 64 additions & 12 deletions

File tree

src/lib.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,16 +170,23 @@ pub struct Dimensions {
170170
#[allow(clippy::len_without_is_empty)]
171171
impl Dimensions {
172172
/// create dimensions info with start position and end position
173+
///
174+
/// The corners may be given in either order; they are stored so that
175+
/// `start <= end` on both axes.
173176
pub fn new(start: (u32, u32), end: (u32, u32)) -> Self {
174-
Self { start, end }
177+
Self {
178+
start: (start.0.min(end.0), start.1.min(end.1)),
179+
end: (start.0.max(end.0), start.1.max(end.1)),
180+
}
175181
}
176182
/// check if a position is in it
177183
pub fn contains(&self, row: u32, col: u32) -> bool {
178184
row >= self.start.0 && row <= self.end.0 && col >= self.start.1 && col <= self.end.1
179185
}
180186
/// len
181187
pub fn len(&self) -> u64 {
182-
(self.end.0 - self.start.0 + 1) as u64 * (self.end.1 - self.start.1 + 1) as u64
188+
// Widened before the `+ 1` so a full-width axis cannot overflow.
189+
(u64::from(self.end.0 - self.start.0) + 1) * (u64::from(self.end.1 - self.start.1) + 1)
183190
}
184191
}
185192

src/xlsx/mod.rs

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2789,18 +2789,21 @@ pub(crate) fn get_dimension(dimension: &[u8]) -> Result<Dimensions, XlsxError> {
27892789
end: parts[0],
27902790
}),
27912791
2 => {
2792-
let rows = parts[1].0 - parts[0].0;
2793-
let columns = parts[1].1 - parts[0].1;
2794-
if rows > MAX_ROWS {
2795-
warn!("xlsx has more than maximum number of rows ({rows} > {MAX_ROWS})");
2792+
// The `ref` may be in reversed order like `C5:A1`.
2793+
let dim = Dimensions::new(parts[0], parts[1]);
2794+
if dim.end.0 > MAX_ROWS {
2795+
warn!(
2796+
"xlsx has more than maximum number of rows ({} > {MAX_ROWS})",
2797+
dim.end.0
2798+
);
27962799
}
2797-
if columns > MAX_COLUMNS {
2798-
warn!("xlsx has more than maximum number of columns ({columns} > {MAX_COLUMNS})");
2800+
if dim.end.1 > MAX_COLUMNS {
2801+
warn!(
2802+
"xlsx has more than maximum number of columns ({} > {MAX_COLUMNS})",
2803+
dim.end.1
2804+
);
27992805
}
2800-
Ok(Dimensions {
2801-
start: parts[0],
2802-
end: parts[1],
2803-
})
2806+
Ok(dim)
28042807
}
28052808
len => Err(XlsxError::DimensionCount(len)),
28062809
}
@@ -4010,6 +4013,48 @@ mod tests {
40104013
);
40114014
}
40124015

4016+
#[test]
4017+
fn test_reversed_dimension_is_normalised() {
4018+
// A reversed `ref` such as `C5:A1` gives the same `Dimensions` as `A1:C5`.
4019+
let reversed = get_dimension(b"C5:A1").unwrap();
4020+
assert_eq!(
4021+
reversed,
4022+
Dimensions {
4023+
start: (0, 0),
4024+
end: (4, 2),
4025+
}
4026+
);
4027+
assert_eq!(reversed, get_dimension(b"A1:C5").unwrap());
4028+
assert_eq!(reversed.len(), 15);
4029+
4030+
// Reversed on one axis only.
4031+
assert_eq!(
4032+
get_dimension(b"A5:C1").unwrap(),
4033+
get_dimension(b"A1:C5").unwrap()
4034+
);
4035+
assert_eq!(
4036+
get_dimension(b"C1:A5").unwrap(),
4037+
get_dimension(b"A1:C5").unwrap()
4038+
);
4039+
}
4040+
4041+
#[test]
4042+
fn test_dimensions_new_normalises_order() {
4043+
let dim = Dimensions::new((4, 2), (0, 0));
4044+
assert_eq!(
4045+
dim,
4046+
Dimensions {
4047+
start: (0, 0),
4048+
end: (4, 2),
4049+
}
4050+
);
4051+
assert_eq!(dim.len(), 15);
4052+
// A single cell is still one cell, and a full-width axis does not
4053+
// overflow the `+ 1`.
4054+
assert_eq!(Dimensions::new((7, 7), (7, 7)).len(), 1);
4055+
assert_eq!(Dimensions::new((0, 0), (u32::MAX, 0)).len(), 4_294_967_296);
4056+
}
4057+
40134058
#[test]
40144059
fn test_parse_error() {
40154060
assert_eq!(

0 commit comments

Comments
 (0)