shifathosn
/
vectorcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #1255 from FredApps/fix/layer-target-validation-785-20261011 

Validate explicit targets in layer row commands (part of #785).
This commit is contained in:
BFlat 2026-10-12 08:17:58 +07:00 committed by GitHub
parent 490989a9f7 714f127100
commit 25c60fd0f5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 189 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
crates/engine/src/cmd/layer.rs
View File

@ -189,15 +189,16 @@ pub fn specs() -> Vec<CommandSpec> {
/// The rows a panel command acts on: `ids` or `id`, else the highlighted rows, else the current
/// layer (capped at [`MAX_ROWS`]).
pub(crate) fn rows_param(s: &Session, p: &Value) -> Result<Vec<NodeId>> {
let mut ids = match (ids_param(p, "ids"), id_param(p, "id")) {
(Some(ids), _) => ids,
(None, Some(id)) => vec![id],
(None, None) => {
let st = s.doc()?;
let rows = st.highlighted_rows();
if rows.is_empty() { st.current_layer().into_iter().collect() } else { rows }
}
pub(crate) fn rows_param(s: &Session, p: &Value, command: &str) -> Result<Vec<NodeId>> {
// Presence is explicit intent: validate every target before fallback, truncation or edits.
let mut ids = if p.get("ids").is_some() {
checked_ids_param(s, p, "ids", command)?
} else if let Some(id) = p.get("id") {
vec![checked_id(s, id, command)?]
} else {
let st = s.doc()?;
let rows = st.highlighted_rows();
if rows.is_empty() { st.current_layer().into_iter().collect() } else { rows }
};
ids.truncate(MAX_ROWS);
Ok(ids)
@ -457,7 +458,7 @@ fn new_sublayer(s: &mut Session, p: &Value) -> Result<Value> {
}
fn delete_rows(s: &mut Session, p: &Value) -> Result<Value> {
let ids = rows_param(s, p)?;
let ids = rows_param(s, p, "layer.delete")?;
let st = s.doc()?;
let ids = row_roots(&st.doc, &ids);
if ids.is_empty() {
@ -484,7 +485,7 @@ fn delete_rows(s: &mut Session, p: &Value) -> Result<Value> {
}
fn duplicate_rows(s: &mut Session, p: &Value) -> Result<Value> {
let ids = rows_param(s, p)?;
let ids = rows_param(s, p, "layer.duplicate")?;
let ids = row_roots(&s.doc()?.doc, &ids);
if ids.is_empty() {
return Err(bad("layer.duplicate", "no such rows"));
@ -551,7 +552,7 @@ fn highlight(s: &mut Session, p: &Value) -> Result<Value> {
fn set_props(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "layer.setProps";
check_options(C, p)?;
let ids = rows_param(s, p)?;
let ids = rows_param(s, p, C)?;
if ids.is_empty() {
return Err(bad(C, "no rows"));
}
@ -567,12 +568,11 @@ fn set_props(s: &mut Session, p: &Value) -> Result<Value> {
fn collect(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "layer.collectInNew";
let ids = match ids_param(p, "ids") {
Some(ids) => ids,
None => {
let rows = s.doc()?.highlighted_rows();
if rows.is_empty() { super::edit::selected_roots(s)? } else { rows }
}
let ids = if p.get("ids").is_some() {
checked_ids_param(s, p, "ids", C)?
} else {
let rows = s.doc()?.highlighted_rows();
if rows.is_empty() { super::edit::selected_roots(s)? } else { rows }
};
let ids = row_roots(&s.doc()?.doc, &ids);
let Some(top) = ids.last().copied() else { return Err(bad(C, "select rows or objects to collect")) };

25
crates/engine/src/cmd/layerpanel.rs
View File

@ -84,7 +84,7 @@ pub fn specs() -> Vec<CommandSpec> {
None,
"{ids?} hide every top-level layer but those holding the highlighted rows (else the current layer), which show. One undo step → {count}",
has_doc,
|s, p| others(s, p, Flag::Visible, false)
|s, p| others(s, p, Flag::Visible, false, "layer.hideOthers")
),
cmd!("layer.showAll", "Show All Layers", ["Window", "Layers"], None, "{} show every layer and sublayer → {count}", has_doc, |s, _| all(
s,
@ -98,7 +98,7 @@ pub fn specs() -> Vec<CommandSpec> {
None,
"{ids?} turn Preview off (outline view) for every top-level layer but those holding the highlighted rows (else the current layer), which preview → {count}",
has_doc,
|s, p| others(s, p, Flag::Preview, false)
|s, p| others(s, p, Flag::Preview, false, "layer.outlineOthers")
),
cmd!(
"layer.previewAll",
@ -116,7 +116,7 @@ pub fn specs() -> Vec<CommandSpec> {
None,
"{ids?} lock every top-level layer but those holding the highlighted rows (else the current layer), which unlock → {count}",
has_doc,
|s, p| others(s, p, Flag::Locked, true)
|s, p| others(s, p, Flag::Locked, true, "layer.lockOthers")
),
cmd!(
"layer.unlockAll",
@ -196,16 +196,16 @@ fn set_flag(s: &mut Session, changes: Vec<(NodeId, bool)>, flag: Flag, label: &s
}
/// The top-level layers holding the rows a command acts on.
fn kept_layers(s: &Session, p: &Value) -> Result<HashSet<NodeId>> {
let ids = rows_param(s, p)?;
fn kept_layers(s: &Session, p: &Value, command: &str) -> Result<HashSet<NodeId>> {
let ids = rows_param(s, p, command)?;
let d = &s.doc()?.doc;
Ok(ids.iter().filter_map(|id| d.layer_of(*id)).collect())
}
/// Every top-level layer but the kept ones set to `v`, and the kept ones to the opposite: Alt-click
/// on a hidden layer's eye shows just that layer (#1158).
fn others(s: &mut Session, p: &Value, flag: Flag, v: bool) -> Result<Value> {
let keep = kept_layers(s, p)?;
fn others(s: &mut Session, p: &Value, flag: Flag, v: bool, command: &str) -> Result<Value> {
let keep = kept_layers(s, p, command)?;
let changes = s.doc()?.doc.layers.iter().map(|l| (l.id, !keep.contains(&l.id) == v)).collect();
set_flag(s, changes, flag, flag.label(v))
}
@ -221,7 +221,7 @@ fn all(s: &mut Session, flag: Flag, v: bool) -> Result<Value> {
}
fn template(s: &mut Session, p: &Value) -> Result<Value> {
let ids = rows_param(s, p)?;
let ids = rows_param(s, p, "layer.template")?;
let d = &s.doc()?.doc;
let ids: Vec<NodeId> = ids.into_iter().filter(|id| d.node(*id).is_some_and(Node::is_layer)).collect();
let Some(first) = ids.first().and_then(|id| d.node(*id)) else { return Err(bad("layer.template", "select a layer")) };
@ -268,7 +268,7 @@ fn merge_into(d: &mut Document, into: NodeId, others: &[NodeId]) -> Result<()> {
fn merge(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "layer.merge";
let ids = rows_param(s, p)?;
let ids = rows_param(s, p, C)?;
let st = s.doc()?;
let d = &st.doc;
let mut seen = HashSet::new();
@ -300,7 +300,10 @@ fn flatten(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "layer.flatten";
let st = s.doc()?;
let d = &st.doc;
let from = id_param(p, "id").or_else(|| st.highlighted_rows().last().copied()).or(st.current_layer());
let from = match p.get("id") {
Some(id) => Some(checked_id(s, id, C)?),
None => st.highlighted_rows().last().copied().or(st.current_layer()),
};
let into = from.and_then(|id| d.layer_of(id)).ok_or_else(|| bad(C, "no layer"))?;
if !d.node(into).is_some_and(|n| n.visible) {
return Err(EngineError::Other("the layer to flatten into is hidden".into()));
@ -389,7 +392,7 @@ fn release(s: &mut Session, p: &Value) -> Result<Value> {
fn reverse(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "layer.reverse";
let ids = rows_param(s, p)?;
let ids = rows_param(s, p, C)?;
let d = &s.doc()?.doc;
let ids = row_roots(d, &ids);
// Rows grouped by the layer or group holding them (None: top-level layers).

157
crates/engine/src/tests_layers.rs
View File

@ -614,3 +614,160 @@ fn the_clipping_mask_button_acts_on_the_highlighted_group() {
assert_eq!(run(&mut s, "layer.clippingMask.toggle", json!({}))["clip"], true);
assert!(node(&s, g).clips());
}
/// #785: explicit malformed layer targets must never become the current-layer default.
#[test]
fn layer_delete_rejects_invalid_explicit_targets() {
let mut unexpected = vec![];
for params in [json!({"id": "1"}), json!({"ids": [1, "2"]}), json!({"id": "$1.id"})] {
let mut s = session();
run(&mut s, "layer.new", json!({}));
run(&mut s, "layer.new", json!({}));
let before = layer_target_snapshot(&s);
let result = s.execute("layer.delete", &params);
if result.is_ok() {
let names: Vec<_> = s.doc().unwrap().doc.layers.iter().map(|l| l.display_name()).collect();
unexpected.push(format!("{params}: {result:?}; remaining layers {names:?}"));
} else {
assert_eq!(layer_target_snapshot(&s), before, "{params}");
}
}
assert!(unexpected.is_empty(), "malformed requests succeeded: {}", unexpected.join("; "));
}
fn layer_target_snapshot(s: &Session) -> Value {
let st = s.doc().unwrap();
let history =
|entries: &[HistoryEntry]| entries.iter().map(|e| json!({"label": e.label, "doc": e.doc, "selection": e.selection})).collect::<Vec<_>>();
json!({"doc": st.doc, "selection": st.selection, "revision": st.revision, "activeLayer": st.active_layer,
"rows": st.layer_rows, "undo": history(&st.history.undo), "redo": history(&st.history.redo)})
}
fn layer_target_fixture() -> (Session, Vec<NodeId>) {
let mut s = session();
let first = s.doc().unwrap().doc.layers[0].id;
rect(&mut s, 0.0);
let second = id(&run(&mut s, "layer.new", json!({})));
rect(&mut s, 40.0);
let third = id(&run(&mut s, "layer.new", json!({})));
rect(&mut s, 80.0);
run(&mut s, "layer.highlight", json!({"ids": [first.0, second.0]}));
(s, vec![first, second, third])
}
fn malformed_layer_targets(first: NodeId, second: NodeId) -> Vec<Value> {
let mut params = vec![];
for bad in [json!("1"), json!("$1.id"), json!(1.0), json!(1.5), json!(-1), Value::Null, json!(true), json!({}), json!([])] {
params.push(json!({"id": bad}));
params.push(json!({"ids": [first.0, bad]}));
}
for bad in [Value::Null, json!("1"), json!(1), json!(false), json!({})] {
params.push(json!({"ids": bad, "id": first.0}));
}
for missing in [0, u64::MAX] {
params.push(json!({"id": missing}));
params.push(json!({"ids": [first.0, second.0, missing]}));
}
params
}
fn assert_layer_target_error(s: &mut Session, command: &str, params: Value) {
let before = layer_target_snapshot(s);
let error = s.execute(command, &params).unwrap_err();
assert!(error.to_string().contains(command), "{command} {params}: {error}");
assert_eq!(layer_target_snapshot(s), before, "{command} {params}: rejected targets must leave all state unchanged");
}
/// #785: shared row commands reject malformed or unknown targets before any partial edits.
#[test]
fn layer_target_validation_keeps_document_selection_revision_and_history_unchanged() {
let (_, rows) = layer_target_fixture();
for command in [
"layer.delete",
"layer.duplicate",
"layer.setProps",
"layer.template",
"layer.merge",
"layer.reverse",
"layer.hideOthers",
"layer.outlineOthers",
"layer.lockOthers",
] {
for mut params in malformed_layer_targets(rows[0], rows[1]) {
let (mut s, _) = layer_target_fixture();
if command == "layer.setProps" {
params["name"] = json!("Changed");
}
assert_layer_target_error(&mut s, command, params);
}
}
}
/// These commands have their own selectors, not the shared row-default parser.
#[test]
fn layer_collect_and_flatten_validate_their_own_explicit_targets() {
let (_, rows) = layer_target_fixture();
for params in malformed_layer_targets(rows[0], rows[1]) {
let command = if params.get("ids").is_some() { "layer.collectInNew" } else { "layer.flatten" };
let (mut s, _) = layer_target_fixture();
assert_layer_target_error(&mut s, command, params);
}
}
/// Truncation limits work, not validation: a malformed or unknown tail cannot be ignored.
#[test]
fn layer_target_arrays_are_fully_validated_before_the_row_limit() {
// The existing panel row limit is 100,000; exercise the public command beyond it.
const ROW_LIMIT: usize = 100_000;
for tail in [json!("$1.id"), json!(u64::MAX)] {
let (mut s, rows) = layer_target_fixture();
let mut values = vec![json!(rows[0].0); ROW_LIMIT + 1];
*values.last_mut().unwrap() = tail;
assert_layer_target_error(&mut s, "layer.delete", json!({"ids": values}));
}
let (mut s, rows) = layer_target_fixture();
let mut values = vec![json!(rows[0].0); ROW_LIMIT + 1];
*values.last_mut().unwrap() = json!(rows[1].0);
run(&mut s, "layer.setProps", json!({"ids": values, "name": "Within limit"}));
assert_eq!(node(&s, rows[0]).name.as_deref(), Some("Within limit"));
assert_ne!(node(&s, rows[1]).name.as_deref(), Some("Within limit"), "the valid tail remains beyond the work cap");
}
#[test]
fn layer_target_defaults_and_valid_explicit_precedence_are_preserved() {
let (mut s, rows) = layer_target_fixture();
run(&mut s, "layer.setProps", json!({"name": "Highlighted"}));
assert_eq!(node(&s, rows[0]).name.as_deref(), Some("Highlighted"));
assert_eq!(node(&s, rows[1]).name.as_deref(), Some("Highlighted"));
run(&mut s, "layer.setProps", json!({"id": rows[2].0, "name": "Single"}));
assert_eq!(node(&s, rows[2]).name.as_deref(), Some("Single"));
let before = undos(&s);
run(&mut s, "layer.setProps", json!({"ids": [rows[1].0, rows[0].0, rows[1].0], "id": "$1.id", "name": "Explicit"}));
assert_eq!(node(&s, rows[0]).name.as_deref(), Some("Explicit"));
assert_eq!(node(&s, rows[1]).name.as_deref(), Some("Explicit"));
assert_eq!(node(&s, rows[2]).name.as_deref(), Some("Single"));
assert_eq!(undos(&s), before + 1, "duplicates remain one edit");
assert_layer_target_error(&mut s, "layer.setProps", json!({"ids": [], "id": rows[2].0, "name": "Must not fall back"}));
run(&mut s, "layer.setCurrent", json!({"id": rows[2].0}));
run(&mut s, "layer.highlight", json!({"ids": []}));
run(&mut s, "layer.setProps", json!({"name": "Current"}));
assert_eq!(node(&s, rows[2]).name.as_deref(), Some("Current"));
let before = undos(&s);
run(&mut s, "layer.delete", json!({}));
assert!(s.doc().unwrap().doc.node(rows[2]).is_none());
assert_eq!(undos(&s), before + 1);
}
#[test]
fn layer_collect_and_flatten_keep_valid_and_omitted_target_behavior() {
let (mut s, rows) = layer_target_fixture();
let r = run(&mut s, "layer.flatten", json!({"id": rows[2].0}));
assert_eq!(r["id"], json!(rows[2].0));
assert_eq!(s.doc().unwrap().doc.layers.len(), 1);
let (mut s, _) = layer_target_fixture();
let selected = s.doc().unwrap().selection.objects.clone();
s.doc_mut().unwrap().layer_rows.clear();
let r = run(&mut s, "layer.collectInNew", json!({}));
let collected = id(&r);
assert!(selected.iter().all(|id| parent(&s, *id) == Some(collected)));
}