diff --git a/src/crates/monolith-commerce/src/shipping.rs b/src/crates/monolith-commerce/src/shipping.rs index 989d567..7c850cb 100644 --- a/src/crates/monolith-commerce/src/shipping.rs +++ b/src/crates/monolith-commerce/src/shipping.rs @@ -170,7 +170,7 @@ pub fn calculate_rates( length_mm: item.length_mm, width_mm: item.width_mm, height_mm: item.height_mm, - volume_mm3: item.length_mm as i64 * item.width_mm as i64 * item.height_mm as i64, + volume_mm3: volume_mm3_saturating(item.length_mm, item.width_mm, item.height_mm), }); } } @@ -287,7 +287,7 @@ fn calculate_rate_for_method( .templates .iter() .max_by_key(|t| { - t.max_length_mm as i64 * t.max_width_mm as i64 * t.max_height_mm as i64 + volume_mm3_saturating(t.max_length_mm, t.max_width_mm, t.max_height_mm) }) .unwrap(); // safe: checked !is_empty() above for unit in units { @@ -398,7 +398,7 @@ fn pack_into_templates( // Check each individual item fits in at least one template let largest_template = templates .iter() - .max_by_key(|t| t.max_length_mm as i64 * t.max_width_mm as i64 * t.max_height_mm as i64) + .max_by_key(|t| volume_mm3_saturating(t.max_length_mm, t.max_width_mm, t.max_height_mm)) .unwrap(); for unit in units { @@ -450,9 +450,11 @@ fn pack_into_templates( let mut result = Vec::new(); for parcel in &parcels { - let vol_weight = - (parcel.max_length as i64 * parcel.max_width as i64 * parcel.stacked_height as i64) - / divisor as i64; + let vol_weight = volume_mm3_saturating( + parcel.max_length, + parcel.max_width, + parcel.stacked_height, + ) / divisor as i64; let chargeable = parcel.total_weight.max(vol_weight as i32); // Parcel was built with stacking: max_length × max_width footprint, stacked_height. @@ -553,8 +555,8 @@ fn parcel_can_accept( let new_width = parcel.max_width.max(unit.width_mm); let new_height = parcel.stacked_height.saturating_add(unit.height_mm); - let vol_weight = - (new_length as i64 * new_width as i64 * new_height as i64) / vol_divisor.max(1) as i64; + let vol_weight = volume_mm3_saturating(new_length, new_width, new_height) + / vol_divisor.max(1) as i64; let chargeable = new_weight.max(vol_weight.min(i32::MAX as i64) as i32); new_length <= max_template.max_length_mm @@ -629,6 +631,17 @@ fn format_price(pence: i64) -> String { format!("£{:.2}", pence as f64 / 100.0) } +/// Multiply three i32 dimensions into an i64 volume, saturating at i64::MAX +/// instead of wrapping. Three i32::MAX values multiplied as i64 would otherwise +/// overflow silently. Saturated values preserve correct ordering for max_by_key +/// / sort and force oversized items to fail downstream fit checks rather than +/// pack into a parcel with a wrapped (often negative) volume. +fn volume_mm3_saturating(length: i32, width: i32, height: i32) -> i64 { + (length as i64) + .saturating_mul(width as i64) + .saturating_mul(height as i64) +} + // ── Tests ─────────────────────────────────────────────────────────────────── #[cfg(test)] @@ -670,7 +683,7 @@ mod tests { length_mm: l, width_mm: w, height_mm: h, - volume_mm3: l as i64 * w as i64 * h as i64, + volume_mm3: volume_mm3_saturating(l, w, h), } } @@ -785,4 +798,29 @@ mod tests { assert!(!surcharge_condition_met(&s, Some("SW1A 1AA"), &rate)); assert!(!surcharge_condition_met(&s, None, &rate)); } + + #[test] + fn volume_mm3_saturating_caps_at_i64_max() { + // Three i32::MAX values would mathematically be ~9.3e27, far past + // i64::MAX (~9.2e18). Saturating must clamp, not wrap. + let v = volume_mm3_saturating(i32::MAX, i32::MAX, i32::MAX); + assert_eq!(v, i64::MAX, "saturating mul should clamp at i64::MAX"); + assert!(v > 0, "must NOT be a wrapped negative value"); + } + + #[test] + fn volume_mm3_saturating_normal_case() { + // Normal cart-item dimensions multiply exactly. + assert_eq!(volume_mm3_saturating(100, 200, 300), 6_000_000); + } + + #[test] + fn oversized_dimensions_packing_rejected() { + // i32::MAX dimensions should fail to fit any sane template, not panic + // or pack with a wrapped negative volume. + let templates = royal_mail_templates(); + let units = vec![test_item(i32::MAX, i32::MAX, i32::MAX, 1)]; + let result = pack_into_templates(&units, &templates, 5000, &[]); + assert!(result.is_err(), "oversized item must be rejected, not packed"); + } }