collective
Browse / github.com/storytold/cadcraft / patch
Reject out-of-range numeric colours in properties.set
Reject out-of-range numeric colours in properties.set Found while testing `crates/engine/src/cmd/props.rs:155-157 (parse_color), with crates/color/src/lib.rs:60-66 (Color::from_aci)` (no existing issue): properties.set accepts out-of-range numeric colours and silently resets the object to ByLayer. ## Cause `parse_color` converted any JSON integer that fits an i16 with `Color::from_aci`, which maps every value outside 0..=255 to ByLayer. So `properties.set` with `color` 300 or -5 silently replaced the object's colour with ByLayer. Only values that don't fit an i16, such as 70000, got the 'bad colour' error. ## Fix `parse_color` now accepts integ
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Silvertree2010 (imported from pull request #2286, open)
- Base
d864a08b013353143086bd8bb568c26cb6256796- Size
- +21 −1 in 1 file
- Review
- pending
- Shared
- 2026-10-28
- Id
09f0f2e118bca2d3825894e1
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/props.rs
@@ -153,7 +153,9 @@ fn run_get(s: &mut Session, p: &Value) -> Result<Value> { } fn parse_color(v: &Value) -> Option<Color> {- v.as_str().and_then(Color::parse).or_else(|| v.as_i64().and_then(|i| i16::try_from(i).ok()).map(Color::from_aci))+ v.as_str()+ .and_then(Color::parse)+ .or_else(|| v.as_i64().filter(|i| (0..=256).contains(i)).and_then(|i| i16::try_from(i).ok()).map(Color::from_aci)) } /// A transparency as JSON, in the form `properties.set` takes: "ByLayer", "ByBlock" or a percentage.@@ -1181,6 +1183,24 @@ mod tests { use super::*; use serde_json::json; + #[test]+ fn properties_set_rejects_out_of_range_colour() {+ let mut s = Session::new();+ s.execute("line", &json!({ "points": [[0, 0], [50, 0]] })).unwrap();+ let h = s.doc().unwrap().model.last().unwrap().handle.hex();+ s.execute("properties.set", &json!({ "handles": [h], "color": 1 })).unwrap();+ let col = |s: &Session| s.doc().unwrap().model.last().unwrap().common.color;+ for bad in [300, -5, 257] {+ let e = s.execute("properties.set", &json!({ "handles": [h], "color": bad })).unwrap_err();+ assert!(e.to_string().contains("bad colour"), "{e}");+ assert_eq!(col(&s), Color::Index(1));+ }+ s.execute("properties.set", &json!({ "handles": [h], "color": 256 })).unwrap();+ assert_eq!(col(&s), Color::ByLayer);+ s.execute("properties.set", &json!({ "handles": [h], "color": 0 })).unwrap();+ assert_eq!(col(&s), Color::ByBlock);+ }+ #[test] fn properties_set_rejects_unloaded_linetype() { let mut s = Session::new();