collective
Browse / github.com/storytold/cadcraft / patch
Refuse invalid count in JSON copy instead of clamping it
Refuse invalid count in JSON copy instead of clamping it Found while testing `crates/engine/src/cmd/modify/transform.rs:69 (run_copy)` (no existing issue): JSON copy silently clamps an invalid count (0, negative, non-integer or over 10000) instead of refusing it. ## Cause run_copy read `count` with `as_u64().unwrap_or(1).clamp(1, 10_000)`. Zero, negative, non-integer and non-numeric counts silently became 1, and counts over 10000 silently became 10000. The command reported success with a drawing different from the one requested. ## Fix run_copy now returns an invalid-parameters error (`count` must be an integer from 1 to 10000) when `count` is pre
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Silvertree2010 (imported from pull request #2294, open)
- Base
d864a08b013353143086bd8bb568c26cb6256796- Size
- +29 −1 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
b12039eefb0bab96d511cb97
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/modify/transform.rs crates/engine/tests/it/copy_count.rs crates/engine/tests/it/main.rs
@@ -66,7 +66,10 @@ pub(super) fn run_move(s: &mut Session, p: &Value) -> Result<Value> { pub(super) fn run_copy(s: &mut Session, p: &Value) -> Result<Value> { let hs = targets(s, p)?; let d = delta_of("copy", p)?;- let n = p.get("count").and_then(Value::as_u64).unwrap_or(1).clamp(1, 10_000);+ let n = match p.get("count") {+ None => 1,+ Some(v) => v.as_u64().filter(|n| (1..=10_000).contains(n)).ok_or_else(|| bad("copy", "`count` must be an integer from 1 to 10000"))?,+ }; let mut out = Vec::new(); for k in 1..=n { out.extend(transform_entities(s, &hs, &Mat3::translate(d * k as f64), true)?.into_iter().map(|h| h.hex()));new file mode 100644@@ -0,0 +1,24 @@+//! JSON COPY refuses a `count` that is not an integer from 1 to 10000 instead of clamping it.++use cadcraft_engine::Session;+use serde_json::{Value, json};++fn entities(s: &Session) -> usize {+ s.doc().map(|d| d.model.iter().count()).unwrap_or(0)+}++#[test]+fn invalid_copy_count_is_refused() {+ let mut s = Session::new();+ let r = s.execute("line", &json!({"points": [[0, 0], [10, 0]]})).unwrap();+ let h = r["handles"][0].clone();+ let before = entities(&s);+ let bad: [Value; 7] = [json!(0), json!(-3), json!(2.5), json!("abc"), json!(null), json!(10001), json!(50000)];+ for c in bad {+ let r = s.execute("copy", &json!({"handles": [h], "delta": [0, 5], "count": c}));+ assert!(r.is_err(), "count {c} should be refused");+ assert_eq!(entities(&s), before, "count {c} changed the drawing");+ }+ s.execute("copy", &json!({"handles": [h], "delta": [0, 5], "count": 3})).unwrap();+ assert_eq!(entities(&s), before + 3);+}@@ -20,6 +20,7 @@ mod burst; mod compare_export; mod constraint_geometry; mod constraint_infer;+mod copy_count; mod count; mod dash_block; mod data_extraction;