Inputs to MOVE, SCALE, ROTATE, ARRAY, OFFSET and the rest are checked for being finite, but finite inputs can still overflow: moving by 1e308 twice put a circle at infinity, after which inspect showed null coordinates, the extents were null and a DXF save wrote it at 0,0. After a command changes the drawing (JSON `execute`, grip edits, and an interactive command when it ends), the objects it changed are checked: every number in their fields and their extents must be finite. If not, the drawing and selection are put back, no undo step is recorded and the command fails with "the result is too large: object … would get infinite or undefined coordinates. Nothing was changed". Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Valentin Klein
GitHub
parent
0fef879509
commit
2e3da66380
|
|
@ -0,0 +1,263 @@
|
|||
//! Edits whose result has coordinates too large to represent (NaN or infinite) are refused.
|
||||
//!
|
||||
//! Finite inputs can still overflow: moving by `[1e308, 1e308]` twice, scaling by a huge factor or
|
||||
//! offsetting by a huge distance. Infinite coordinates break extents, inspection and rendering, and
|
||||
//! are silently changed by a DXF save and reopen, so the command fails and the drawing stays as it
|
||||
//! was. Only objects the command changed are checked: every number in their serialized fields, and
|
||||
//! their extents.
|
||||
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
use cadcraft_doc::{Drawing, Entity, EntityStore, Handle, entity_bounds};
|
||||
use serde::Serialize;
|
||||
use serde::ser;
|
||||
|
||||
use crate::{EngineError, Session};
|
||||
|
||||
impl Session {
|
||||
/// After command `cmd` changed the drawing from `before` (the document `uid`, or the active
|
||||
/// one): when an object it changed has NaN or infinite numbers, put the drawing and selection
|
||||
/// back and return the error to report.
|
||||
pub(crate) fn refuse_non_finite(&mut self, uid: Option<u64>, before: &Arc<Drawing>, selection: &[Handle], cmd: &str) -> crate::Result<()> {
|
||||
let st = match uid {
|
||||
Some(uid) => self.docs.iter_mut().find(|d| d.uid == uid),
|
||||
None => self.state_mut().ok(),
|
||||
};
|
||||
let Some(st) = st.filter(|st| !Arc::ptr_eq(before, &st.doc)) else { return Ok(()) };
|
||||
let Some(h) = first_non_finite(before, &st.doc) else { return Ok(()) };
|
||||
st.doc = before.clone();
|
||||
st.selection = selection.to_vec();
|
||||
Err(EngineError::BadParams {
|
||||
cmd: cmd.into(),
|
||||
msg: format!("the result is too large: object {} would get infinite or undefined coordinates. Nothing was changed", h.hex()),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
/// The first object changed between `before` and `after` that has a NaN or infinite number, or
|
||||
/// whose extents overflow.
|
||||
pub(crate) fn first_non_finite(before: &Drawing, after: &Drawing) -> Option<Handle> {
|
||||
let layouts = after.layouts.iter().map(|l| (&l.entities, before.layout(&l.name).map(|b| &b.entities)));
|
||||
let blocks = after
|
||||
.blocks
|
||||
.iter()
|
||||
.filter(|(name, b)| before.blocks.get(*name).is_none_or(|old| !Arc::ptr_eq(old, b)))
|
||||
.map(|(name, b)| (&b.entities, before.blocks.get(name).map(|old| &old.entities)));
|
||||
std::iter::once((&after.model, Some(&before.model))).chain(layouts).chain(blocks).find_map(|(a, b)| changed_non_finite(after, b, a))
|
||||
}
|
||||
|
||||
fn changed_non_finite(d: &Drawing, before: Option<&EntityStore>, after: &EntityStore) -> Option<Handle> {
|
||||
if before.is_some_and(|b| b.same_as(after)) {
|
||||
return None;
|
||||
}
|
||||
// Chunks shared with `before` hold only unchanged objects.
|
||||
let shared: HashSet<usize> = before.map(|b| b.chunk_slices().map(|(key, _)| key).collect()).unwrap_or_default();
|
||||
let finite = |e: &Entity| {
|
||||
let b = entity_bounds(d, e, 0);
|
||||
is_finite(e) && (b.is_empty() || (b.min.is_finite() && b.max.is_finite()))
|
||||
};
|
||||
after
|
||||
.chunk_slices()
|
||||
.filter(|(key, _)| !shared.contains(key))
|
||||
.flat_map(|(_, items)| items.iter())
|
||||
.filter(|e| before.and_then(|b| b.get(e.handle)).is_none_or(|old| !Arc::ptr_eq(old, e)))
|
||||
.find(|e| !finite(e))
|
||||
.map(|e| e.handle)
|
||||
}
|
||||
|
||||
/// Whether every number in `v` is finite.
|
||||
pub(crate) fn is_finite<T: Serialize + ?Sized>(v: &T) -> bool {
|
||||
!matches!(v.serialize(Check), Err(Stop::NonFinite))
|
||||
}
|
||||
|
||||
/// Why the walk stopped: a non-finite number, or a value that can't be serialized (not ours to judge).
|
||||
#[derive(Debug)]
|
||||
enum Stop {
|
||||
NonFinite,
|
||||
Other,
|
||||
}
|
||||
|
||||
impl std::fmt::Display for Stop {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.write_str(match self {
|
||||
Stop::NonFinite => "non-finite number",
|
||||
Stop::Other => "not serializable",
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for Stop {}
|
||||
|
||||
impl ser::Error for Stop {
|
||||
fn custom<T: std::fmt::Display>(_: T) -> Self {
|
||||
Stop::Other
|
||||
}
|
||||
}
|
||||
|
||||
/// A serializer that only looks at floats.
|
||||
#[derive(Clone, Copy)]
|
||||
struct Check;
|
||||
|
||||
type R = Result<(), Stop>;
|
||||
|
||||
macro_rules! ignore {
|
||||
($($name:ident: $t:ty),*) => { $(fn $name(self, _: $t) -> R { Ok(()) })* };
|
||||
}
|
||||
|
||||
impl ser::Serializer for Check {
|
||||
type Ok = ();
|
||||
type Error = Stop;
|
||||
type SerializeSeq = Self;
|
||||
type SerializeTuple = Self;
|
||||
type SerializeTupleStruct = Self;
|
||||
type SerializeTupleVariant = Self;
|
||||
type SerializeMap = Self;
|
||||
type SerializeStruct = Self;
|
||||
type SerializeStructVariant = Self;
|
||||
|
||||
ignore!(serialize_bool: bool, serialize_i8: i8, serialize_i16: i16, serialize_i32: i32, serialize_i64: i64, serialize_i128: i128,
|
||||
serialize_u8: u8, serialize_u16: u16, serialize_u32: u32, serialize_u64: u64, serialize_u128: u128, serialize_char: char,
|
||||
serialize_str: &str, serialize_bytes: &[u8], serialize_unit_struct: &'static str);
|
||||
|
||||
fn serialize_f32(self, v: f32) -> R {
|
||||
if v.is_finite() { Ok(()) } else { Err(Stop::NonFinite) }
|
||||
}
|
||||
fn serialize_f64(self, v: f64) -> R {
|
||||
if v.is_finite() { Ok(()) } else { Err(Stop::NonFinite) }
|
||||
}
|
||||
fn serialize_none(self) -> R {
|
||||
Ok(())
|
||||
}
|
||||
fn serialize_some<T: Serialize + ?Sized>(self, v: &T) -> R {
|
||||
v.serialize(self)
|
||||
}
|
||||
fn serialize_unit(self) -> R {
|
||||
Ok(())
|
||||
}
|
||||
fn serialize_unit_variant(self, _: &'static str, _: u32, _: &'static str) -> R {
|
||||
Ok(())
|
||||
}
|
||||
fn serialize_newtype_struct<T: Serialize + ?Sized>(self, _: &'static str, v: &T) -> R {
|
||||
v.serialize(self)
|
||||
}
|
||||
fn serialize_newtype_variant<T: Serialize + ?Sized>(self, _: &'static str, _: u32, _: &'static str, v: &T) -> R {
|
||||
v.serialize(self)
|
||||
}
|
||||
fn serialize_seq(self, _: Option<usize>) -> Result<Self, Stop> {
|
||||
Ok(self)
|
||||
}
|
||||
fn serialize_tuple(self, _: usize) -> Result<Self, Stop> {
|
||||
Ok(self)
|
||||
}
|
||||
fn serialize_tuple_struct(self, _: &'static str, _: usize) -> Result<Self, Stop> {
|
||||
Ok(self)
|
||||
}
|
||||
fn serialize_tuple_variant(self, _: &'static str, _: u32, _: &'static str, _: usize) -> Result<Self, Stop> {
|
||||
Ok(self)
|
||||
}
|
||||
fn serialize_map(self, _: Option<usize>) -> Result<Self, Stop> {
|
||||
Ok(self)
|
||||
}
|
||||
fn serialize_struct(self, _: &'static str, _: usize) -> Result<Self, Stop> {
|
||||
Ok(self)
|
||||
}
|
||||
fn serialize_struct_variant(self, _: &'static str, _: u32, _: &'static str, _: usize) -> Result<Self, Stop> {
|
||||
Ok(self)
|
||||
}
|
||||
}
|
||||
|
||||
impl ser::SerializeSeq for Check {
|
||||
type Ok = ();
|
||||
type Error = Stop;
|
||||
fn serialize_element<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
|
||||
v.serialize(*self)
|
||||
}
|
||||
fn end(self) -> R {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl ser::SerializeTuple for Check {
|
||||
type Ok = ();
|
||||
type Error = Stop;
|
||||
fn serialize_element<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
|
||||
v.serialize(*self)
|
||||
}
|
||||
fn end(self) -> R {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl ser::SerializeTupleStruct for Check {
|
||||
type Ok = ();
|
||||
type Error = Stop;
|
||||
fn serialize_field<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
|
||||
v.serialize(*self)
|
||||
}
|
||||
fn end(self) -> R {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl ser::SerializeTupleVariant for Check {
|
||||
type Ok = ();
|
||||
type Error = Stop;
|
||||
fn serialize_field<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
|
||||
v.serialize(*self)
|
||||
}
|
||||
fn end(self) -> R {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl ser::SerializeMap for Check {
|
||||
type Ok = ();
|
||||
type Error = Stop;
|
||||
fn serialize_key<T: Serialize + ?Sized>(&mut self, k: &T) -> R {
|
||||
k.serialize(*self)
|
||||
}
|
||||
fn serialize_value<T: Serialize + ?Sized>(&mut self, v: &T) -> R {
|
||||
v.serialize(*self)
|
||||
}
|
||||
fn end(self) -> R {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl ser::SerializeStruct for Check {
|
||||
type Ok = ();
|
||||
type Error = Stop;
|
||||
fn serialize_field<T: Serialize + ?Sized>(&mut self, _: &'static str, v: &T) -> R {
|
||||
v.serialize(*self)
|
||||
}
|
||||
fn end(self) -> R {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl ser::SerializeStructVariant for Check {
|
||||
type Ok = ();
|
||||
type Error = Stop;
|
||||
fn serialize_field<T: Serialize + ?Sized>(&mut self, _: &'static str, v: &T) -> R {
|
||||
v.serialize(*self)
|
||||
}
|
||||
fn end(self) -> R {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn finds_nan_and_infinity_anywhere() {
|
||||
assert!(is_finite(&(1.0, vec![Some(2.0f32)], "inf")));
|
||||
assert!(!is_finite(&vec![(0.0, f64::INFINITY)]));
|
||||
assert!(!is_finite(&Some(f32::NAN)));
|
||||
let mut m = std::collections::BTreeMap::new();
|
||||
m.insert("x", f64::NEG_INFINITY);
|
||||
assert!(!is_finite(&m));
|
||||
}
|
||||
}
|
||||
|
|
@ -10,6 +10,7 @@
|
|||
|
||||
pub mod assoc;
|
||||
pub mod cmd;
|
||||
mod finite;
|
||||
pub mod grips;
|
||||
mod guard;
|
||||
pub mod prompt;
|
||||
|
|
@ -478,6 +479,10 @@ impl Session {
|
|||
};
|
||||
cmd::constraints::after_command(self, before.as_ref().map(|b| &b.0), spec.undoable && result.is_ok());
|
||||
assoc::after_command(self, before.as_ref().map(|b| &b.0), before.as_ref().map(|b| b.2));
|
||||
let result = match (result, &before) {
|
||||
(Ok(v), Some((doc, sel, uid))) if spec.undoable => self.refuse_non_finite(Some(*uid), doc, sel, spec.id).map(|()| v),
|
||||
(r, _) => r,
|
||||
};
|
||||
if spec.undoable
|
||||
&& let Some((doc, sel, uid)) = before
|
||||
&& let Some(st) = self.docs.iter_mut().find(|d| d.uid == uid)
|
||||
|
|
@ -679,6 +684,11 @@ impl Session {
|
|||
}
|
||||
|
||||
fn finish(&mut self, run: Running, cancelled: bool) {
|
||||
if find_command(&run.id).is_some_and(|c| c.undoable)
|
||||
&& let Err(e) = self.refuse_non_finite(None, &run.before, &run.selection_before, &run.id)
|
||||
{
|
||||
self.echo(e.to_string());
|
||||
}
|
||||
// A command outside undo that changed the drawing (UNDO, which swapped it for an earlier
|
||||
// one) records no undo step, as in `execute`.
|
||||
if find_command(&run.id).is_some_and(|c| !c.undoable) && self.state().is_ok_and(|st| !Arc::ptr_eq(&run.before, &st.doc)) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,56 @@
|
|||
//! Edits whose result would have infinite or undefined coordinates fail and leave the drawing as it
|
||||
//! was, whether they come as JSON (MCP, control channel) or are typed at the command line.
|
||||
|
||||
use cadcraft_engine::Session;
|
||||
use serde_json::{Value, json};
|
||||
|
||||
fn circle(s: &mut Session) -> String {
|
||||
s.execute("circle", &json!({"center": [0, 0], "radius": 1})).unwrap()["handle"].as_str().unwrap().to_string()
|
||||
}
|
||||
|
||||
fn snapshot(s: &mut Session) -> (Value, usize) {
|
||||
let i = s.execute("drawing.inspect", &json!({})).unwrap();
|
||||
let undo = i["undo"].as_array().map_or(0, Vec::len);
|
||||
(i["entities"].clone(), undo)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn json_transforms_that_overflow_are_refused() {
|
||||
let huge = [1e308, 1e308];
|
||||
let cases: Vec<(&str, Value)> = vec![
|
||||
("move", json!({"delta": huge})),
|
||||
("copy", json!({"delta": huge})),
|
||||
("scale", json!({"base": [0, 0], "factor": 1e308})),
|
||||
("rotate", json!({"base": [-1e308, -1e308], "angle": 180})),
|
||||
("mirror", json!({"p1": [-1e308, -1e308], "p2": [-1e308, 0]})),
|
||||
("arrayrect", json!({"rows": 3, "cols": 1, "rowSpacing": 1e308, "colSpacing": 1})),
|
||||
("stretch", json!({"window": [[-2, -2], [1.7e308, 1.7e308]], "delta": huge})),
|
||||
("offset", json!({"distance": 1e308, "side": [1.7e308, 1.7e308]})),
|
||||
];
|
||||
for (cmd, p) in cases {
|
||||
let mut s = Session::new();
|
||||
let h = circle(&mut s);
|
||||
// Put it where the edit overflows: far out, then once more.
|
||||
s.execute("move", &json!({"handles": [h], "delta": huge})).unwrap();
|
||||
let before = snapshot(&mut s);
|
||||
let mut p = p;
|
||||
p["handles"] = json!([h]);
|
||||
p["handle"] = json!(h);
|
||||
let r = s.execute(cmd, &p);
|
||||
let err = r.expect_err(&format!("{cmd} {p} must fail"));
|
||||
assert!(err.to_string().contains("too large"), "{cmd}: {err}");
|
||||
assert_eq!(snapshot(&mut s), before, "{cmd} must leave the drawing and undo history unchanged");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn typed_move_that_overflows_is_refused() {
|
||||
let mut s = Session::new();
|
||||
circle(&mut s);
|
||||
s.script("MOVE ALL\n\n0,0 1e308,1e308\n").unwrap();
|
||||
let before = snapshot(&mut s);
|
||||
s.script("MOVE ALL\n\n0,0 1e308,1e308\n").unwrap();
|
||||
assert_eq!(snapshot(&mut s), before);
|
||||
assert!(s.log.iter().any(|l| l.contains("too large")), "{:?}", s.log);
|
||||
assert!(s.running.is_none());
|
||||
}
|
||||
Loading…
Reference in New Issue