forked from 1b-agent/opencadstudio-upstream
Bound typed counts and values that could crash the editor
- ARRAYRECT, ARRAYPOLAR, ARRAYPATH, ARRAY3D and COPY Array reject item counts above 100,000 (checked as a product of every dimension) and keep the prompt open. Unbounded counts allocated one transform per item and aborted on allocation failure. Stored defaults are clamped as well. - EXPLODE leaves a MINSERT above the same limit intact instead of materializing every cell. - Typed lengths and angles reject non-finite values and magnitudes above 1e15; SCALE rejects a non-finite factor. - Zero-radius FILLET compares candidate intersections with total_cmp and rejects non-finite results; partial_cmp().unwrap() panicked on NaN. - SOLID falls back to a minimal kernel tolerance when its corners' extent overflows, since Tolerance::new panics on infinity. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
1 parent
38aa2ee34d
commit
a3d65e2fc8
7 files changed
+223
-15
No files matched your search
@@ -283,6 +283,20 @@ fn surveyor(value_rad: f64, prec: usize) -> String {
|
||||
/// A leading `-` is a sign, but the `-` inside `5'-9"` is a separator — after
|
||||
/// feet there is nothing left to subtract from.
|
||||
pub fn parse_length(text: &str) -> Option<f64> {
|
||||
parse_length_unbounded(text).filter(|value| typed_value_in_range(*value))
|
||||
}
|
||||
|
||||
/// Largest magnitude a typed length or angle may have. Rust's float parser
|
||||
/// accepts `inf`, `nan` and overflowing exponents; geometry built from those,
|
||||
/// or from values whose differences overflow, breaks downstream tolerance and
|
||||
/// intersection math.
|
||||
pub const MAX_TYPED_MAGNITUDE: f64 = 1e15;
|
||||
|
||||
pub fn typed_value_in_range(value: f64) -> bool {
|
||||
value.is_finite() && value.abs() <= MAX_TYPED_MAGNITUDE
|
||||
}
|
||||
|
||||
fn parse_length_unbounded(text: &str) -> Option<f64> {
|
||||
let text = text.trim();
|
||||
let (sign, rest) = match text.strip_prefix('-') {
|
||||
Some(rest) => (-1.0, rest.trim_start()),
|
||||
@@ -339,6 +353,10 @@ fn parse_inches(text: &str) -> Option<f64> {
|
||||
/// number is read in whatever convention the drawing is set to, so what the
|
||||
/// readout shows can be typed straight back.
|
||||
pub fn parse_angle(text: &str) -> Option<f64> {
|
||||
parse_angle_unbounded(text).filter(|value| typed_value_in_range(*value))
|
||||
}
|
||||
|
||||
fn parse_angle_unbounded(text: &str) -> Option<f64> {
|
||||
let text = text.trim();
|
||||
if text.is_empty() {
|
||||
return None;
|
||||
@@ -679,11 +697,16 @@ pub fn parse_f64(value: &str) -> Option<f64> {
|
||||
.ok()
|
||||
.or_else(|| parse_length(t))
|
||||
.or_else(|| parse_angle_deg(t))
|
||||
.filter(|value| typed_value_in_range(*value))
|
||||
}
|
||||
|
||||
/// Parse an angle string the panel displayed via AUNITS back to DEGREES:
|
||||
/// "30", "30°"/"30d", DMS "30°15'20.5\"", grads "33.33g", radians "0.52r".
|
||||
pub fn parse_angle_deg(value: &str) -> Option<f64> {
|
||||
parse_angle_deg_unbounded(value).filter(|value| typed_value_in_range(*value))
|
||||
}
|
||||
|
||||
fn parse_angle_deg_unbounded(value: &str) -> Option<f64> {
|
||||
let s = value.trim();
|
||||
if s.is_empty() {
|
||||
return None;
|
||||
@@ -1161,6 +1184,24 @@ pub(crate) fn polyline_segment_fill(
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod typed_range_tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn non_finite_and_huge_typed_values_are_rejected() {
|
||||
for text in ["inf", "-inf", "nan", "1e400", "1e308"] {
|
||||
assert_eq!(parse_length(text), None, "length {text}");
|
||||
assert_eq!(parse_angle(text), None, "angle {text}");
|
||||
assert_eq!(parse_f64(text), None, "property {text}");
|
||||
assert_eq!(parse_angle_deg(text), None, "angle degrees {text}");
|
||||
}
|
||||
assert_eq!(parse_length("1e16"), None);
|
||||
assert_eq!(parse_length("1e15"), Some(1e15));
|
||||
assert_eq!(parse_length("-5'-6\""), Some(-66.0));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod length_format_tests {
|
||||
use super::*;
|
||||
|
||||
+28
-1
@@ -80,7 +80,14 @@ fn boundary_edges(points: &[[f64; 3]]) -> Vec<Edge> {
|
||||
}
|
||||
|
||||
fn kernel_tolerance(points: &[[f64; 3]]) -> f64 {
|
||||
cadkernel::space::coplanarity_tolerance(points).max(1.0e-12)
|
||||
let tolerance = cadkernel::space::coplanarity_tolerance(points);
|
||||
// Corners whose extent overflows f64 give an infinite tolerance, which
|
||||
// `Tolerance::new` rejects by panicking.
|
||||
if tolerance.is_finite() {
|
||||
tolerance.max(1.0e-12)
|
||||
} else {
|
||||
1.0e-12
|
||||
}
|
||||
}
|
||||
|
||||
fn extrusion_body(
|
||||
@@ -366,3 +373,23 @@ impl Transformable for Solid {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod overflow_tests {
|
||||
use super::*;
|
||||
use acadrust::types::Vector3;
|
||||
|
||||
/// Corners 1e308 apart overflow the coplanarity tolerance to infinity,
|
||||
/// which the kernel's `Tolerance::new` rejects by panicking.
|
||||
#[test]
|
||||
fn corners_with_overflowing_extent_do_not_panic() {
|
||||
let solid = Solid::new(
|
||||
Vector3::new(0.0, 0.0, 0.0),
|
||||
Vector3::new(1e308, 0.0, 0.0),
|
||||
Vector3::new(0.0, 1e308, 0.0),
|
||||
Vector3::new(1e308, 1e308, 0.0),
|
||||
);
|
||||
let _ = wcs_bounds(&solid);
|
||||
let _ = solid_geometry(&solid);
|
||||
}
|
||||
}
|
||||
@@ -45,6 +45,27 @@ pub const DROPDOWN_ITEMS: &[(&str, &str, IconKind)] = &[
|
||||
),
|
||||
];
|
||||
|
||||
// ── Item limit ─────────────────────────────────────────────────────────────
|
||||
|
||||
/// Most items one array may create, including the source. Every item becomes
|
||||
/// a transform and then a copy of each selected object, so an unbounded typed
|
||||
/// count exhausts memory before anything is drawn.
|
||||
pub(crate) const MAX_ARRAY_ITEMS: u64 = 100_000;
|
||||
|
||||
/// Whether an array of `counts` (rows, columns, levels, ...) stays in bounds.
|
||||
pub(crate) fn array_items_within_limit(counts: &[u64]) -> bool {
|
||||
counts
|
||||
.iter()
|
||||
.try_fold(1u64, |total, &count| total.checked_mul(count))
|
||||
.is_some_and(|total| total <= MAX_ARRAY_ITEMS)
|
||||
}
|
||||
|
||||
pub(crate) fn array_limit_error() -> CmdResult {
|
||||
CmdResult::ReportError(
|
||||
crate::tf!("Array size exceeds the limit of {} items.", MAX_ARRAY_ITEMS).into_owned(),
|
||||
)
|
||||
}
|
||||
|
||||
// ── Rectangular Array ──────────────────────────────────────────────────────
|
||||
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
@@ -72,8 +93,8 @@ impl ArrayRectCommand {
|
||||
handles,
|
||||
wire_models,
|
||||
step: RectStep::Rows,
|
||||
default_rows: defaults::get_array_rows() as u32,
|
||||
default_cols: defaults::get_array_cols() as u32,
|
||||
default_rows: (defaults::get_array_rows() as u32).clamp(1, MAX_ARRAY_ITEMS as u32),
|
||||
default_cols: (defaults::get_array_cols() as u32).clamp(1, MAX_ARRAY_ITEMS as u32),
|
||||
default_row_sp: defaults::get_array_row_sp(),
|
||||
default_col_sp: defaults::get_array_col_sp(),
|
||||
plane: WorkingPlane::default(),
|
||||
@@ -154,6 +175,9 @@ impl CadCommand for ArrayRectCommand {
|
||||
self.default_rows
|
||||
} else {
|
||||
let v = t.parse::<u32>().unwrap_or(self.default_rows).max(1);
|
||||
if !array_items_within_limit(&[v as u64]) {
|
||||
return Some(array_limit_error());
|
||||
}
|
||||
defaults::set_array_rows(v as f64);
|
||||
self.default_rows = v;
|
||||
v
|
||||
@@ -165,11 +189,15 @@ impl CadCommand for ArrayRectCommand {
|
||||
let cols = if t.is_empty() {
|
||||
self.default_cols
|
||||
} else {
|
||||
let v = t.parse::<u32>().unwrap_or(self.default_cols).max(1);
|
||||
defaults::set_array_cols(v as f64);
|
||||
self.default_cols = v;
|
||||
v
|
||||
t.parse::<u32>().unwrap_or(self.default_cols).max(1)
|
||||
};
|
||||
if !array_items_within_limit(&[rows as u64, cols as u64]) {
|
||||
return Some(array_limit_error());
|
||||
}
|
||||
if !t.is_empty() {
|
||||
defaults::set_array_cols(cols as f64);
|
||||
self.default_cols = cols;
|
||||
}
|
||||
self.step = RectStep::RowSp { rows, cols };
|
||||
None
|
||||
}
|
||||
@@ -222,6 +250,10 @@ impl CadCommand for ArrayRectCommand {
|
||||
}
|
||||
RectStep::ColSp { rows, cols, row_sp } => (rows, cols, row_sp, self.default_col_sp),
|
||||
};
|
||||
// Defaults are only bounded one count at a time.
|
||||
if !array_items_within_limit(&[rows as u64, cols as u64]) {
|
||||
return vec![];
|
||||
}
|
||||
Self::build_transforms(rows, cols, row_sp, col_sp, self.plane)
|
||||
.iter()
|
||||
.flat_map(|t| {
|
||||
@@ -274,7 +306,7 @@ impl ArrayPolarCommand {
|
||||
handles,
|
||||
wire_models,
|
||||
step: PolarStep::Center,
|
||||
default_count: defaults::get_array_p_count() as u32,
|
||||
default_count: (defaults::get_array_p_count() as u32).clamp(2, MAX_ARRAY_ITEMS as u32),
|
||||
default_angle: defaults::get_array_p_angle(),
|
||||
plane: WorkingPlane::default(),
|
||||
}
|
||||
@@ -330,6 +362,9 @@ impl CadCommand for ArrayPolarCommand {
|
||||
self.default_count
|
||||
} else {
|
||||
let v = t.parse::<u32>().unwrap_or(self.default_count).max(2);
|
||||
if !array_items_within_limit(&[v as u64]) {
|
||||
return Some(array_limit_error());
|
||||
}
|
||||
defaults::set_array_p_count(v as f64);
|
||||
self.default_count = v;
|
||||
v
|
||||
@@ -461,7 +496,7 @@ impl ArrayPathCommand {
|
||||
wire_models,
|
||||
all_entities,
|
||||
step: PathStep::SelectPath,
|
||||
default_count: defaults::get_array_path_count() as u32,
|
||||
default_count: (defaults::get_array_path_count() as u32).clamp(2, MAX_ARRAY_ITEMS as u32),
|
||||
pick_pt: DVec3::ZERO,
|
||||
}
|
||||
}
|
||||
@@ -686,6 +721,9 @@ impl CadCommand for ArrayPathCommand {
|
||||
self.default_count
|
||||
} else {
|
||||
let v = t.parse::<u32>().unwrap_or(self.default_count).max(2);
|
||||
if !array_items_within_limit(&[v as u64]) {
|
||||
return Some(array_limit_error());
|
||||
}
|
||||
defaults::set_array_path_count(v as f64);
|
||||
self.default_count = v;
|
||||
v
|
||||
@@ -891,6 +929,9 @@ impl CadCommand for Array3DCommand {
|
||||
} else {
|
||||
t.parse::<u32>().unwrap_or(2).max(1)
|
||||
};
|
||||
if !array_items_within_limit(&[v as u64]) {
|
||||
return Some(array_limit_error());
|
||||
}
|
||||
self.step = Array3DStep::Cols { rows: v };
|
||||
Some(CmdResult::NeedPoint)
|
||||
}
|
||||
@@ -900,6 +941,9 @@ impl CadCommand for Array3DCommand {
|
||||
} else {
|
||||
t.parse::<u32>().unwrap_or(2).max(1)
|
||||
};
|
||||
if !array_items_within_limit(&[rows as u64, v as u64]) {
|
||||
return Some(array_limit_error());
|
||||
}
|
||||
self.step = Array3DStep::Levels { rows, cols: v };
|
||||
Some(CmdResult::NeedPoint)
|
||||
}
|
||||
@@ -909,6 +953,9 @@ impl CadCommand for Array3DCommand {
|
||||
} else {
|
||||
t.parse::<u32>().unwrap_or(2).max(1)
|
||||
};
|
||||
if !array_items_within_limit(&[rows as u64, cols as u64, v as u64]) {
|
||||
return Some(array_limit_error());
|
||||
}
|
||||
self.step = Array3DStep::RowSp {
|
||||
rows,
|
||||
cols,
|
||||
@@ -987,3 +1034,33 @@ impl CadCommand for Array3DCommand {
|
||||
|
||||
// ── Autocomplete registry ─────────────────────────────────
|
||||
inventory::submit!(crate::command::CommandRegistration { names: &["ARRAY3D", "3DARRAY"] });
|
||||
|
||||
#[cfg(test)]
|
||||
mod item_limit_tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn item_limit_counts_every_dimension_without_overflow() {
|
||||
assert!(array_items_within_limit(&[MAX_ARRAY_ITEMS]));
|
||||
assert!(!array_items_within_limit(&[MAX_ARRAY_ITEMS + 1]));
|
||||
assert!(!array_items_within_limit(&[1_000, 1_000]));
|
||||
assert!(!array_items_within_limit(&[u32::MAX as u64, u32::MAX as u64, u32::MAX as u64]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn oversized_counts_are_rejected_at_the_prompt() {
|
||||
let mut rect = ArrayRectCommand::new(vec![Handle::new(1)], Vec::new());
|
||||
assert!(rect.on_text_input("100000").is_none());
|
||||
assert!(matches!(rect.on_text_input("100000"), Some(CmdResult::ReportError(_))));
|
||||
assert!(matches!(rect.step, RectStep::Cols { rows: 100_000 }));
|
||||
|
||||
let mut polar = ArrayPolarCommand::new(vec![Handle::new(1)], Vec::new());
|
||||
polar.step = PolarStep::Count { center: DVec3::ZERO };
|
||||
assert!(matches!(polar.on_text_input("4294967295"), Some(CmdResult::ReportError(_))));
|
||||
|
||||
let mut cube = Array3DCommand::new(vec![Handle::new(1)]);
|
||||
assert!(matches!(cube.on_text_input("300"), Some(CmdResult::NeedPoint)));
|
||||
assert!(matches!(cube.on_text_input("300"), Some(CmdResult::NeedPoint)));
|
||||
assert!(matches!(cube.on_text_input("2"), Some(CmdResult::ReportError(_))));
|
||||
}
|
||||
}
|
||||
@@ -199,6 +199,9 @@ impl CadCommand for CopyCommand {
|
||||
fn on_text_input(&mut self, text: &str) -> Option<CmdResult> {
|
||||
if self.awaiting_count {
|
||||
if let Ok(n) = text.trim().parse::<usize>() {
|
||||
if !super::array::array_items_within_limit(&[n as u64]) {
|
||||
return Some(super::array::array_limit_error());
|
||||
}
|
||||
if n >= 2 {
|
||||
self.array_count = Some(n);
|
||||
}
|
||||
@@ -277,6 +280,18 @@ mod tests {
|
||||
cmd.options().into_iter().map(|o| o.keyword).collect()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn array_count_above_the_item_limit_is_rejected() {
|
||||
let mut cmd = CopyCommand::new(vec![Handle::new(1)], vec![]);
|
||||
cmd.awaiting_count = true;
|
||||
assert!(matches!(
|
||||
cmd.on_text_input("18446744073709551615"),
|
||||
Some(CmdResult::ReportError(_))
|
||||
));
|
||||
assert!(cmd.awaiting_count, "the count prompt stays up");
|
||||
assert_eq!(cmd.array_count, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mode_single_exits_after_one_copy() {
|
||||
let mut cmd = CopyCommand::new(vec![Handle::new(1)], vec![]);
|
||||
|
||||
@@ -88,6 +88,16 @@ pub fn explode_entity(entity: &EntityType, document: &CadDocument) -> Vec<Entity
|
||||
EntityType::Polyline2D(p) => explode_polyline2d(p),
|
||||
EntityType::Polyline(p) => explode_polyline(p),
|
||||
EntityType::Polyline3D(p) => explode_polyline3d(p),
|
||||
// A MINSERT expands to one copy of the block per cell; leave arrays
|
||||
// too large to materialize intact instead of exhausting memory.
|
||||
EntityType::Insert(ins)
|
||||
if !super::array::array_items_within_limit(&[
|
||||
ins.row_count as u64,
|
||||
ins.column_count as u64,
|
||||
]) =>
|
||||
{
|
||||
vec![]
|
||||
}
|
||||
EntityType::Insert(ins) => ins
|
||||
.explode_from_document(document)
|
||||
.into_iter()
|
||||
@@ -1478,6 +1488,18 @@ mod tests {
|
||||
use super::*;
|
||||
use acadrust::entities::DimensionLinear;
|
||||
|
||||
#[test]
|
||||
fn minsert_too_large_to_materialize_is_left_intact() {
|
||||
let mut doc = CadDocument::new();
|
||||
let mut block = acadrust::tables::BlockRecord::new("B1");
|
||||
block.handle = doc.allocate_handle();
|
||||
doc.block_records.add(block).unwrap();
|
||||
let mut insert = acadrust::entities::Insert::new("B1", Vector3::new(0.0, 0.0, 0.0));
|
||||
insert.row_count = u16::MAX;
|
||||
insert.column_count = u16::MAX;
|
||||
assert!(explode_entity(&EntityType::Insert(insert), &doc).is_empty());
|
||||
}
|
||||
|
||||
/// A dimension created without a geometry block gets a real `*D` block on
|
||||
/// bake, its `block_name` resolves to that block, and a second bake is a
|
||||
/// no-op (already has a valid block).
|
||||
|
||||
@@ -1062,10 +1062,13 @@ fn fillet_line_arc(
|
||||
+ (p1[1] + a * u[1] - click_line[1]).powi(2);
|
||||
let db = (p1[0] + b * u[0] - click_line[0]).powi(2)
|
||||
+ (p1[1] + b * u[1] - click_line[1]).powi(2);
|
||||
da.partial_cmp(&db).unwrap()
|
||||
da.total_cmp(&db)
|
||||
})?;
|
||||
let ix = p1[0] + t_best * u[0];
|
||||
let iy = p1[1] + t_best * u[1];
|
||||
if !(ix.is_finite() && iy.is_finite()) {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Trim line to intersection
|
||||
let new_line = trim_line_to_point(line, [ix, iy], click_line)?;
|
||||
@@ -1154,9 +1157,11 @@ fn fillet_arc_arc(
|
||||
let ip = *pts.iter().min_by(|a, b| {
|
||||
(a[0] - cx)
|
||||
.hypot(a[1] - cy)
|
||||
.partial_cmp(&(b[0] - cx).hypot(b[1] - cy))
|
||||
.unwrap()
|
||||
.total_cmp(&(b[0] - cx).hypot(b[1] - cy))
|
||||
})?;
|
||||
if !(ip[0].is_finite() && ip[1].is_finite()) {
|
||||
return None;
|
||||
}
|
||||
|
||||
let ia1 = arc_angle_at(c1, ip);
|
||||
let ia2 = arc_angle_at(c2, ip);
|
||||
@@ -2368,6 +2373,26 @@ mod tests {
|
||||
cmd.options().into_iter().map(|o| o.keyword).collect()
|
||||
}
|
||||
|
||||
/// The upper half of a radius-5 circle at (5, 0), scaled about the origin
|
||||
/// by an infinite factor: the center becomes (inf, NaN).
|
||||
fn overflowed_arc(sweep_sign: f64) -> ArcEnt {
|
||||
let mut arc = ArcEnt::new();
|
||||
arc.center = acadrust::types::Vector3::new(f64::INFINITY, f64::NAN, 0.0);
|
||||
arc.radius = f64::INFINITY;
|
||||
arc.start_angle = 0.0;
|
||||
arc.end_angle = sweep_sign * std::f64::consts::PI;
|
||||
arc
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn zero_radius_fillet_rejects_non_finite_geometry() {
|
||||
let line = LineEnt::from_coords(f64::NAN, f64::NAN, 0.0, f64::INFINITY, f64::INFINITY, 0.0);
|
||||
let arc = overflowed_arc(1.0);
|
||||
assert!(fillet_line_arc(&line, [5.0, 5.0], &arc, [5.0, 5.0], 0.0, 0.0).is_none());
|
||||
let other = overflowed_arc(-1.0);
|
||||
assert!(fillet_arc_arc(&arc, [5.0, 5.0], &other, [5.0, -5.0], 0.0, 0.0).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fillet_undo_option_appears_after_a_fillet_and_refreshes_from_document() {
|
||||
let mut command = FilletCommand::new(
|
||||
|
||||
@@ -179,12 +179,12 @@ impl CadCommand for ScaleCommand {
|
||||
return Some(CmdResult::NeedPoint);
|
||||
}
|
||||
let factor: f64 = t.replace(',', ".").parse().ok()?;
|
||||
(factor > 0.0).then(|| self.commit(base, factor))
|
||||
(factor.is_finite() && factor > 0.0).then(|| self.commit(base, factor))
|
||||
}
|
||||
Step::RefFirst { base } => {
|
||||
let base = *base;
|
||||
let ref_dist: f64 = t.replace(',', ".").parse().ok()?;
|
||||
if ref_dist > 0.0 {
|
||||
if ref_dist.is_finite() && ref_dist > 0.0 {
|
||||
self.step = Step::RefNew { base, ref_dist };
|
||||
return Some(CmdResult::NeedPoint);
|
||||
}
|
||||
@@ -194,7 +194,8 @@ impl CadCommand for ScaleCommand {
|
||||
Step::RefNew { base, ref_dist } => {
|
||||
let (base, ref_dist) = (*base, *ref_dist);
|
||||
let new_len: f64 = t.replace(',', ".").parse().ok()?;
|
||||
(new_len > 0.0).then(|| self.commit(base, new_len / ref_dist))
|
||||
let factor = new_len / ref_dist;
|
||||
(factor.is_finite() && factor > 0.0).then(|| self.commit(base, factor))
|
||||
}
|
||||
Step::Base => None,
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user