Merge remote-tracking branch 'origin/main' into feat/glyph-alternates

crates/engine/src/cmd/menucmds.rs

@@ -115,7 +115,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Rasterize…",
             ["Object"],
             None,
-            "{ppi?, background?: transparent|white|black|\"#rrggbb\" (#rrggbbaa and #rgba are refused), antiAlias?: none|art|type (text snapped to pixels)|bool, padding?|addAround?: pt (0–1000), colorModel?: \"rgb\"|\"cmyk\"|\"grayscale\"|\"bitmap\", clippingMask?: bool (a clip group with the art's outline)} replace the selection with an embedded image (PNG; in a CMYK document with its own colour model, a CMYK image of the art's inks, its transparency as its opacity mask); each defaults to document.rasterEffectsSettings; a frame of threaded text that is rasterized leaves its thread with the text it shows → {id (the image, or its clip group), width, height, warning?: \"an image couldn't be decoded and is missing from the picture\" (as raster exports say)}",
+            "{ppi?, background?: transparent|white|black|\"#rrggbb\" (#rrggbbaa and #rgba are refused), antiAlias?: none|art|type (text snapped to pixels)|bool, padding?|addAround?: pt (0–1000), colorModel?: \"rgb\"|\"cmyk\"|\"grayscale\"|\"bitmap\", clippingMask?: bool (a clip group with the art's outline)} replace the selection with an embedded image (PNG; in a CMYK document with its own colour model, a CMYK image of the art's inks, its transparency as its opacity mask); each defaults to document.rasterEffectsSettings; an image larger than 64 Mpx or 32,768 px a side is refused; a frame of threaded text that is rasterized leaves its thread with the text it shows → {id (the image, or its clip group), width, height, warning?: \"an image couldn't be decoded and is missing from the picture\" (as raster exports say)}",
             has_selection,
             rasterize
         ),
@@ -670,7 +670,9 @@ fn render_pixels(
     opts: &vectorcraft_render::RenderOptions,
     color: RasterColorModel,
 ) -> Result<(vectorcraft_render::Rendered, Option<&'static str>)> {
-    if region.width() * region.height() * scale * scale > MAX_PIXELS || region.width() * scale > 65535.0 || region.height() * scale > 65535.0 {
+    // No side longer than the renderer draws: it would cut the image off there.
+    let side = f64::from(vectorcraft_render::MAX_RASTER_SIDE);
+    if region.width() * region.height() * scale * scale > MAX_PIXELS || region.width() * scale > side || region.height() * scale > side {
         return Err(EngineError::Other("image would be too large; lower the resolution".into()));
     }
     let mut r = vectorcraft_render::Renderer::new();

crates/engine/src/tests/menucmds.rs

@@ -1461,6 +1461,26 @@ fn rasterize_replaces_selection_with_image() {
     assert_eq!(selected(&s), vec![img]);
 }
 
+/// Rasterize refuses art that would be longer than the renderer draws (32,768 pixels a side), and
+/// the art stays as it was; at a resolution that fits, the image covers all of it.
+#[test]
+fn rasterize_refuses_an_image_longer_than_the_renderer_draws() {
+    let mut s = session();
+    let a = rect(&mut s, 10.0, 20.0, 100.0, 5569.0);
+    sel(&mut s, &[a]);
+    let vb = s.doc().unwrap().doc.bounds_of(&[a], true).unwrap();
+    let before = (undo_len(&s), node(&s, a));
+    let e = s.execute("object.rasterize", &json!({"ppi": 600, "padding": 0})).unwrap_err().to_string();
+    assert!(e.contains("too large"), "{e}");
+    assert_eq!((undo_len(&s), node(&s, a)), before);
+    let img = id_of(&s.execute("object.rasterize", &json!({"ppi": 400, "padding": 0})).unwrap());
+    let n = node(&s, img);
+    let NodeKind::Image(im) = &n.kind else { panic!() };
+    assert!(im.height <= vectorcraft_render::MAX_RASTER_SIDE, "{} pixels", im.height);
+    let b = n.geometric_bounds().unwrap();
+    assert!(close(b.y0, vb.y0) && b.y1 >= vb.y1, "{b:?} covers {vb:?}");
+}
+
 /// Rasterize refuses a resolution given as text or as another value that is not a number, and the
 /// selected art stays as it was.
 #[test]

crates/engine/tests/command_sweep.rs

@@ -9,10 +9,11 @@ use vectorcraft_engine::{Session, command_specs};
 use vectorcraft_testkit::catch_quiet;
 use vectorcraft_testkit::fixtures::Fixture;
 use vectorcraft_testkit::invariants::{check_all, check_session, doc_json};
-use vectorcraft_testkit::strategies::{junk_params, junk_values, param_keys};
+use vectorcraft_testkit::strategies::{junk_params, junk_values, param_keys, safe_value};
 
 /// Commands that replace/close the active document or touch the filesystem are fine to call, but a
-/// few need their paths redirected into a temp dir (handled by `junk_params`).
+/// few need their file and folder params redirected into a temp dir (`safe_value`, which
+/// `junk_params` applies).
 fn safe_path() -> String {
     vectorcraft_testkit::temp_dir("sweep").join("out.bin").to_string_lossy().to_string()
 }
@@ -230,6 +231,54 @@ fn tool_edit_state(s: &Session) -> ToolEditState {
     (docs, s.active_index(), s.paint.clone())
 }
 
+/// The params the tool-edit sweep sends `c`: junk params, then each documented key turned on and
+/// off, with a string under a file or folder key replaced by a location inside `path`'s folder.
+fn tool_edit_cases(c: &vectorcraft_engine::CommandSpec, path: &str) -> Vec<Value> {
+    let mut cases = junk_params(c.params, path);
+    if heavy(c.id) {
+        cases.truncate(12);
+    }
+    for k in param_keys(c.params) {
+        for v in [json!(true), json!(false), json!(1), json!("on")] {
+            cases.push(json!({ k.as_str(): safe_value(&k, &v, path) }));
+        }
+    }
+    // Find Objects selects only with a filter, which one key at a time does not give it.
+    if c.id == "document.find" {
+        cases.extend([json!({"kind": "Path", "select": true}), json!({"name": "no such name", "select": true})]);
+    }
+    cases
+}
+
+/// Every string the sweeps send under a key that names a file or folder to write is inside the
+/// sweep's temp folder, so no run writes into the working directory.
+#[test]
+fn sweep_cases_name_files_only_inside_the_temp_folder() {
+    let path = safe_path();
+    let dir = vectorcraft_testkit::temp_dir("sweep");
+    let keys = ["path", "diffPath", "folder", "destinationFolder"];
+    let mut seen = std::collections::BTreeSet::new();
+    let mut outside = std::collections::BTreeSet::new();
+    for c in command_specs() {
+        let mut cases = junk_params(c.params, &path);
+        cases.extend(tool_edit_cases(c, &path));
+        for p in &cases {
+            let Some(o) = p.as_object() else { continue };
+            for (k, v) in o.iter().filter(|(k, _)| keys.contains(&k.as_str())) {
+                seen.insert(k.clone());
+                if let Some(s) = v.as_str()
+                    && !std::path::Path::new(s).starts_with(&dir)
+                {
+                    outside.insert(format!("{} {{\"{k}\": {v}}}", c.id));
+                }
+            }
+        }
+    }
+    assert_eq!(seen.len(), keys.len(), "the sweeps send only {seen:?}");
+    let outside: Vec<String> = outside.into_iter().collect();
+    assert!(outside.is_empty(), "{} cases name a location outside {}:\n{}", outside.len(), dir.display(), outside.join("\n"));
+}
+
 /// Every command that runs at once while the active tool computes an edit on another thread (a
 /// Shape Builder merge), rather than applying the edit first (`waits_for_tool_edit`), leaves the
 /// documents' revisions and selections, the active document and the current paint as they were,
@@ -241,19 +290,7 @@ fn commands_that_do_not_wait_for_a_tool_edit_change_no_document() {
     let mut failures = vec![];
     for fx in [Fixture::Single, Fixture::Multi] {
         for c in command_specs().iter().filter(|c| !vectorcraft_engine::waits_for_tool_edit(c.id)) {
-            let mut cases = junk_params(c.params, &path);
-            if heavy(c.id) {
-                cases.truncate(12);
-            }
-            for k in param_keys(c.params) {
-                for v in [json!(true), json!(false), json!(1), json!("on")] {
-                    cases.push(json!({ k.as_str(): v }));
-                }
-            }
-            // Find Objects selects only with a filter, which one key at a time does not give it.
-            if c.id == "document.find" {
-                cases.extend([json!({"kind": "Path", "select": true}), json!({"name": "no such name", "select": true})]);
-            }
+            let cases = tool_edit_cases(c, &path);
             for p in &cases {
                 let mut s = fx.session();
                 let before = tool_edit_state(&s);

crates/testkit/src/strategies.rs

@@ -502,10 +502,26 @@ pub fn param_keys(doc: &str) -> Vec<String> {
     out
 }
 
-/// Parameter-name keys that name filesystem paths: fuzzing these with arbitrary strings could
-/// write files anywhere, so callers substitute a temp path.
+/// Parameter-name keys that name a file or folder a command may write: fuzzing these with
+/// arbitrary strings could write files anywhere, so callers substitute a temp path.
 pub fn is_path_key(k: &str) -> bool {
-    k == "path"
+    matches!(k, "path" | "diffPath") || is_folder_key(k)
+}
+
+/// Path keys that name a folder a command creates or writes files into.
+pub fn is_folder_key(k: &str) -> bool {
+    matches!(k, "folder" | "destinationFolder")
+}
+
+/// The location to send under the path key `k`: `safe_path` for a file, a folder next to it for
+/// a folder key.
+pub fn safe_location(k: &str, safe_path: &str) -> String {
+    if is_folder_key(k) { format!("{safe_path}-folder") } else { safe_path.to_string() }
+}
+
+/// `v`, or the temp location for `k` when `k` is a path key and `v` is a string.
+pub fn safe_value(k: &str, v: &Value, safe_path: &str) -> Value {
+    if is_path_key(k) && v.is_string() { json!(safe_location(k, safe_path)) } else { v.clone() }
 }
 
 /// A deterministic list of junk param objects for `doc` (the command's params doc): each
@@ -516,7 +532,7 @@ pub fn junk_params(doc: &str, safe_path: &str) -> Vec<Value> {
     let mut out = vec![Value::Null, json!([]), json!("str"), json!(42), json!({"unknownKey": 1})];
     for k in &keys {
         for (i, v) in junk.iter().enumerate() {
-            let v = if is_path_key(k) && v.is_string() { json!(safe_path) } else { v.clone() };
+            let v = safe_value(k, v, safe_path);
             // Sparse: one key at a time with every junk value, plus a few combos.
             let mut o = serde_json::Map::new();
             o.insert(k.clone(), v.clone());
@@ -524,7 +540,7 @@ pub fn junk_params(doc: &str, safe_path: &str) -> Vec<Value> {
             if i % 5 == 0 {
                 let mut all = serde_json::Map::new();
                 for k2 in &keys {
-                    let v2 = if is_path_key(k2) { json!(safe_path) } else { v.clone() };
+                    let v2 = if is_path_key(k2) { json!(safe_location(k2, safe_path)) } else { v.clone() };
                     all.insert(k2.clone(), v2);
                 }
                 out.push(Value::Object(all));

docs/mcp.md

@@ -5232,7 +5232,8 @@ fills or strokes carry effects is written as one piece per fill and stroke, so o
 When an image in the art can't be decoded, `object.rasterize` and `object.cropImage` (without `trim`) make their image
 without it, and their results carry the `warning` that Flatten Transparency gives. `object.rasterize` takes each frame
 of threaded text that it rasterizes out of its thread with the text it shows, as `type.createOutlines` does (see
-Threading text from the in and out ports), and the other frames keep the text they hold.
+Threading text from the in and out ports), and the other frames keep the text they hold. `object.rasterize`
+refuses an image larger than 64 million pixels or longer than 32,768 pixels a side, and the art stays as it was.
 
 ```json
 {"name":"run_command","arguments":{"command":"document.rasterEffectsSettings","params":{"resolution":"high","background":"white","addAround":36}}}