Merge pull request #6666 from dotCooCoo/fix/malformed-id-not-missing

MCP: a malformed object id is refused as such, not reported as missing
This commit is contained in:
BFlat
2026-11-12 19:14:06 +07:00
committed by GitHub
20 changed files with 174 additions and 76 deletions
+5 -5
View File
@@ -32,8 +32,8 @@ use super::opacitymask::percent;
use super::paint::paint_from;
use super::{AppearanceAttrs, EyedropperAttrs, FillAttrs, StrokeAttrs};
use super::{
CommandSpec, NodeId, Result, Session, bad, check_numbers, check_switches, cmd, has_doc, has_selection, id_param, ids_param, index, leaf_targets,
ok, paint_targets, str_param, switch_or, targets,
CommandSpec, NodeId, Result, Session, bad, check_numbers, check_switches, cmd, has_doc, has_selection, ids_param, index, leaf_targets,
object_param, ok, paint_targets, str_param, switch_or, targets,
};
use crate::EngineError;
@@ -726,8 +726,8 @@ fn clear(n: &mut Node) {
fn transfer(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "appearance.transfer";
let source = id_param(p, "source").ok_or_else(|| bad(C, "missing `source` id"))?;
let target = id_param(p, "target").ok_or_else(|| bad(C, "missing `target` id"))?;
let source = object_param(p, "source", C)?.ok_or_else(|| bad(C, "missing `source` id"))?;
let target = object_param(p, "target", C)?.ok_or_else(|| bad(C, "missing `target` id"))?;
if source == target {
return Err(bad(C, "`source` and `target` are the same object"));
}
@@ -1201,7 +1201,7 @@ fn target_contents(s: &mut Session, p: &Value) -> Result<Value> {
/// Sample enabled attributes with transform-aware character dimensions and atomic validation.
fn copy_from(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "appearance.copyFrom";
let clicked = id_param(p, "source").ok_or_else(|| bad(C, "missing `source` id"))?;
let clicked = object_param(p, "source", C)?.ok_or_else(|| bad(C, "missing `source` id"))?;
let (reverse, append) = (switch_or(p, "reverse", false, C)?, switch_or(p, "append", false, C)?);
let defaults_only = switch_or(p, "defaultsOnly", false, C)?;
if defaults_only && (reverse || append || p.get("ids").is_some()) {
+2 -2
View File
@@ -29,8 +29,8 @@ use super::edit::{outermost, selected_roots};
use super::paint::paint_from;
use super::pathops::{Beside, shape_node};
use super::{
CommandSpec, EngineError, Result, Session, always, bad, check_numbers, cmd, color_value, f64_or, finite_numbers, has_doc, id_param, named_ids,
number_or, point_param, str_param, switch_or,
CommandSpec, EngineError, Result, Session, always, bad, check_numbers, cmd, color_value, f64_or, finite_numbers, has_doc, named_ids, number_or,
object_param, point_param, str_param, switch_or,
};
mod live_paint;
@@ -1,7 +1,7 @@
use super::{
Appearance, Arc, Beside, Document, EDGE_NAME, EngineError, FACE_NAME, GapOptions, GapSize, LIVE_PAINT_NAME, Node, NodeId, Paint, PathData, Point,
Rect, Result, SOURCES_NAME, Selection, Session, SubPath, Value, b, bad, build_children, edge_near, face_at, finite, id_param, ids_json,
is_live_paint, json, number_or, paint_from, po, point_param, repaint_from, replace_roots, root_ids, source_nodes, source_shapes, switch_or,
Rect, Result, SOURCES_NAME, Selection, Session, SubPath, Value, b, bad, build_children, edge_near, face_at, finite, ids_json, is_live_paint,
json, number_or, object_param, paint_from, po, point_param, repaint_from, replace_roots, root_ids, source_nodes, source_shapes, switch_or,
};
/// `options` as stored on a Live Paint group: none for the defaults.
@@ -303,7 +303,7 @@ pub(super) fn lp_fill(s: &mut Session, p: &Value) -> Result<Value> {
};
crate::cmd::paint::check_keep_model(p, C)?;
let paint = paint_from(s, p)?.unwrap_or_else(|| s.paint.fill.clone());
let group = id_param(p, "group");
let group = object_param(p, "group", C)?;
let roots = if group.is_none() { root_ids(s, p)? } else { vec![] };
let (g, face) = s.edit("Live Paint Bucket", |d, sel| {
let gid = match group {
@@ -367,7 +367,7 @@ pub(super) fn lp_fill(s: &mut Session, p: &Value) -> Result<Value> {
pub(super) fn lp_stroke_edge(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "livePaint.strokeEdge";
let pt = point_param(p, "point").filter(|q| finite(*q)).ok_or_else(|| bad(C, "missing point [x,y]"))?;
let g = id_param(p, "group").ok_or_else(|| bad(C, "missing group"))?;
let g = object_param(p, "group", C)?.ok_or_else(|| bad(C, "missing group"))?;
crate::cmd::paint::check_keep_model(p, C)?;
let paint = paint_from(s, p)?.unwrap_or_else(|| s.paint.stroke.clone());
let width = number_or(p, "width", s.paint.stroke_width.max(0.0), C)?;
@@ -390,7 +390,10 @@ pub(super) fn lp_stroke_edge(s: &mut Session, p: &Value) -> Result<Value> {
}
pub(super) fn lp_info(s: &mut Session, p: &Value) -> Result<Value> {
let g = id_param(p, "group").or_else(|| id_param(p, "id")).ok_or_else(|| bad("livePaint.info", "missing group"))?;
let g = match object_param(p, "group", "livePaint.info")? {
Some(g) => g,
None => object_param(p, "id", "livePaint.info")?.ok_or_else(|| bad("livePaint.info", "missing group"))?,
};
let d = &s.doc()?.doc;
let n = d.node(g).filter(|n| is_live_paint(n)).ok_or_else(|| bad("livePaint.info", format!("{g} is not a Live Paint group")))?;
let faces: Vec<Value> = b::faces(n)
+5 -5
View File
@@ -15,8 +15,8 @@ use vectorcraft_tools::distort::{PinSet, arap, collect_points, mesh_for, project
use super::edit::{duplicate_in, outermost, refuse_symbol_part, selected_roots};
use super::{
CommandSpec, Result, Session, bad, check_numbers, check_switches, cmd, f64_or, f64_req, finite_numbers, has_doc, has_selection, id_param,
matrix_param, named_id, named_ids, number_or, ok, point_indices, point_param, str_param, switch_or, targets,
CommandSpec, Result, Session, bad, check_numbers, check_switches, cmd, f64_or, f64_req, finite_numbers, has_doc, has_selection, matrix_param,
named_id, named_ids, number_or, object_param, ok, point_indices, point_param, str_param, switch_or, targets,
};
use crate::EngineError;
@@ -305,7 +305,7 @@ fn same_points(a: Option<&WidthProfile>, b: Option<&WidthProfile>) -> bool {
fn width_point_set(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "stroke.widthPoint.set";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let t = f64_req(p, "t", C)?.clamp(0.0, 1.0);
let (left, right) = (f64_req(p, "left", C)?, f64_req(p, "right", C)?);
if !(left.is_finite() && right.is_finite()) || left < 0.0 || right < 0.0 || left.max(right) > 1.0e5 {
@@ -362,7 +362,7 @@ fn width_point_set(s: &mut Session, p: &Value) -> Result<Value> {
fn width_point_copy(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "stroke.widthPoint.copy";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let t = f64_req(p, "t", C)?.clamp(0.0, 1.0);
// When the profile comes out as it was (the copy replaces a side of a discontinuous point that has
// its widths already), the document and its undo history stay as they are.
@@ -382,7 +382,7 @@ fn width_point_copy(s: &mut Session, p: &Value) -> Result<Value> {
fn width_point_remove(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "stroke.widthPoint.remove";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let indices = match (p.get("indices"), p.get("index")) {
(Some(a), _) => point_indices(a),
(None, Some(i)) => point_indices(&json!([i])),
+2 -2
View File
@@ -18,8 +18,8 @@ use super::create::{add_look, insert_look};
use super::newart::NewArt;
use super::pathops::{Beside, remove_each};
use super::{
CommandSpec, Result, Session, bad, check_numbers, cmd, f64_or, has_anchors, has_doc, has_selection, id_param, index, number_or, ok, str_param,
switch_or,
CommandSpec, Result, Session, bad, check_numbers, cmd, f64_or, has_anchors, has_doc, has_selection, index, number_or, object_param, ok,
str_param, switch_or,
};
use crate::EngineError;
+5 -5
View File
@@ -1,7 +1,7 @@
use super::common::{MAX_INPUT, path_kind, path_mut, point_entry, usize_opt, usize_req, within_input_bound};
use super::{
Anchor, BTreeSet, Beside, Color, Document, EngineError, Node, NodeId, NodeKind, Paint, PathData, Point, Result, Selection, Session, Value, Vec2,
add_look, bad, check_numbers, f64_or, id_param, json, number_or, ok, po, str_param, switch_or,
add_look, bad, check_numbers, f64_or, json, number_or, object_param, ok, po, str_param, switch_or,
};
// ---------- Curvature ----------
@@ -41,7 +41,7 @@ enum CurvatureOp {
pub(super) fn curvature_edit(s: &mut Session, p: &Value) -> Result<Value> {
use vectorcraft_tools::draw2::{EndCurve, curvature_close, curvature_extend, curvature_insert, curvature_move};
const C: &str = "path.curvatureEdit";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let si = usize_opt(p, "subpath", C)?.unwrap_or(0);
check_numbers(p, &["x", "y", "t"], C)?;
let at = match (p.get("x").and_then(Value::as_f64), p.get("y").and_then(Value::as_f64)) {
@@ -106,7 +106,7 @@ pub(super) fn curvature_edit(s: &mut Session, p: &Value) -> Result<Value> {
pub(super) fn remove_anchor(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "path.removeAnchor";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let si = usize_opt(p, "subpath", C)?.unwrap_or(0);
let ai = usize_req(p, "anchor", C)?;
let curvature = switch_or(p, "curvature", false, C)?;
@@ -220,7 +220,7 @@ fn remove_anchors_of(
pub(super) fn convert_anchor(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "path.convertAnchor";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let si = usize_opt(p, "subpath", C)?.unwrap_or(0);
let ai = usize_req(p, "anchor", C)?;
let smooth = str_param(p, "to") == Some("smooth");
@@ -257,7 +257,7 @@ pub(super) fn convert_anchor(s: &mut Session, p: &Value) -> Result<Value> {
pub(super) fn reshape_segment(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "path.reshapeSegment";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let si = usize_opt(p, "subpath", C)?.unwrap_or(0);
let seg = usize_req(p, "segment", C)?;
let t = number_or(p, "t", 0.5, C)?.clamp(0.05, 0.95);
+2 -2
View File
@@ -4,7 +4,7 @@ use super::common::{
};
use super::{
Anchor, AnchorKind, Beside, BezPath, BoolOp, Cap, EngineError, FillRule, Join, NodeId, ParamCurve, PathData, Point, Result, Session, SubPath,
Value, bad, eraser_area, eraser_tip, f64_or, fill_contains, id_param, json, number_or, po,
Value, bad, eraser_area, eraser_tip, f64_or, fill_contains, json, number_or, object_param, po,
};
/// Split a filled path into connected pieces (each outer contour with its holes).
@@ -170,7 +170,7 @@ pub(in super::super) fn erase_outline(pd: &PathData, step: f64, remove: &dyn Fn(
pub(super) fn split(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "path.split";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let si = usize_opt(p, "subpath", C)?.unwrap_or(0);
let anchor = usize_opt(p, "anchor", C)?;
let seg = usize_opt(p, "segment", C)?;
+5 -4
View File
@@ -4,7 +4,8 @@ use super::common::{
};
use super::{
Anchor, Appearance, BTreeSet, BoolOp, Cap, Color, Document, EngineError, FillRule, Join, NewArt, Node, NodeId, NodeKind, Paint, PathData, Point,
Result, Selection, Session, SimplifyOptions, SubPath, Value, bad, id_param, insert_look, json, number_or, po, remove_each, str_param, switch_or,
Result, Selection, Session, SimplifyOptions, SubPath, Value, bad, insert_look, json, number_or, object_param, po, remove_each, str_param,
switch_or,
};
// ---------- Pencil / Paintbrush ----------
@@ -158,7 +159,7 @@ pub(super) fn freehand(s: &mut Session, p: &Value) -> Result<Value> {
let keep_selected = switch_or(p, "keepSelected", true, C)?;
let filled = switch_or(p, "fill", false, C)?;
if let Some(ext) = p.get("extend").filter(|v| !v.is_null()) {
let id = id_param(ext, "id").ok_or_else(|| bad(C, "extend needs id"))?;
let id = object_param(ext, "id", C)?.ok_or_else(|| bad(C, "extend needs id"))?;
let at_start = str_param(ext, "end") == Some("start");
let (pd, _) = path_of(&s.doc()?.doc, id)?;
let si = pd.subpaths.iter().rposition(|s| !s.closed && !s.anchors.is_empty()).ok_or_else(|| bad(C, "path has no open end"))?;
@@ -179,7 +180,7 @@ pub(super) fn freehand(s: &mut Session, p: &Value) -> Result<Value> {
// open subpath, that subpath starting at the end the stroke ends on, it ends at its end).
let connect = match p.get("connect").filter(|v| !v.is_null()) {
Some(cn) => {
let other = id_param(cn, "id").ok_or_else(|| bad(C, "connect needs id"))?;
let other = object_param(cn, "id", C)?.ok_or_else(|| bad(C, "connect needs id"))?;
if other == id {
return Err(bad(C, "connect needs a path other than the one extended"));
}
@@ -299,7 +300,7 @@ pub(super) fn freehand(s: &mut Session, p: &Value) -> Result<Value> {
return Ok(json!({ "id": id.0 }));
}
if let Some(rd) = p.get("redraw").filter(|v| !v.is_null()) {
let id = id_param(rd, "id").ok_or_else(|| bad(C, "redraw needs id"))?;
let id = object_param(rd, "id", C)?.ok_or_else(|| bad(C, "redraw needs id"))?;
let within = number_or(rd, "within", 12.0, C)?;
if !within.is_finite() || within < 0.0 {
return Err(bad(C, "redraw within must be a finite distance of 0 or more"));
+3 -3
View File
@@ -10,8 +10,8 @@ use vectorcraft_geom::{Affine, Point, Vec2};
use super::clipboard::SwatchChoices;
use super::{
CommandSpec, Result, Session, always, bad, can_clear_history, can_redo, can_undo, check_numbers, checked_ids_param, clock_date, cmd, date_param,
f64_req, finite_numbers, has_clipboard, has_doc, has_selection, has_selection_guides_or_artboard_tool, has_selection_or_guides, id_param, index,
number_or, ok, point_param, str_param, switch_or,
f64_req, finite_numbers, has_clipboard, has_doc, has_selection, has_selection_guides_or_artboard_tool, has_selection_or_guides, index, number_or,
object_param, ok, point_param, str_param, switch_or,
};
use crate::{Clipboard, EngineError, inspect};
@@ -267,7 +267,7 @@ fn doc_move(s: &mut Session, p: &Value) -> Result<Value> {
}
fn doc_node(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("document.node", "missing id"))?;
let id = object_param(p, "id", "document.node")?.ok_or_else(|| bad("document.node", "missing id"))?;
let mut opts = slice_opts(p, "document.node")?;
let summary = switch_or(p, "summary", false, "document.node")?;
if !summary && (opts.depth.is_some() || opts.child_limit.is_some()) {
+11 -8
View File
@@ -15,7 +15,7 @@ use vectorcraft_geom::{Affine, Rect};
use super::{
CommandSpec, Result, Session, bad, bool_or, check_numbers, check_switches, checked_id, checked_ids_param, cmd, color_value, f64_or, has_doc,
has_selection, id_param, ids_param, index, ok, str_param, switch_or,
has_selection, ids_param, index, object_param, ok, str_param, switch_or,
};
use crate::EngineError;
@@ -426,7 +426,7 @@ fn paste_remembers_layers(s: &mut Session, p: &Value) -> Result<Value> {
/// The Layers panel's target circle.
fn target(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "layer.target";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let st = s.doc()?;
let n = st.doc.node(id).ok_or(EngineError::NoNode(id))?;
if !st.doc.is_editable(id) {
@@ -594,7 +594,7 @@ fn duplicate_rows(s: &mut Session, p: &Value) -> Result<Value> {
fn set_current(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "layer.setCurrent";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let st = s.doc_mut()?;
let layer = st.doc.layer_containing(id).ok_or(EngineError::NoNode(id))?;
st.active_layer = Some(layer);
@@ -700,7 +700,7 @@ fn in_isolation(doc: &Document, isolation: Option<NodeId>, id: NodeId) -> bool {
/// The selection column: select a row's art.
fn select_art(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "layer.selectAll";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let add = switch_or(p, "add", false, C)?;
let st = s.doc()?;
let n = st.doc.node(id).ok_or(EngineError::NoNode(id))?;
@@ -747,9 +747,12 @@ enum Place {
/// Drag rows in the Layers panel.
fn move_rows(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "layer.move";
let mut ids = ids_param(p, "ids", C)?.or_else(|| id_param(p, "id").map(|i| vec![i])).ok_or_else(|| bad(C, "missing ids"))?;
let mut ids = match ids_param(p, "ids", C)? {
Some(ids) => ids,
None => vec![object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing ids"))?],
};
ids.truncate(MAX_ROWS);
let target = id_param(p, "target").ok_or_else(|| bad(C, "missing target"))?;
let target = object_param(p, "target", C)?.ok_or_else(|| bad(C, "missing target"))?;
let place = match str_param(p, "place").unwrap_or("inside") {
"above" => Place::Above,
"below" => Place::Below,
@@ -831,8 +834,8 @@ fn move_rows(s: &mut Session, p: &Value) -> Result<Value> {
fn move_node(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "node.move";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let parent = id_param(p, "parent");
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let parent = object_param(p, "parent", C)?;
let index = p.get("index").and_then(Value::as_u64).map_or(usize::MAX, index);
let d = &s.doc()?.doc;
let n = d.node(id).ok_or(EngineError::NoNode(id))?;
+1 -1
View File
@@ -21,7 +21,7 @@ use super::expand::gradient_sampler;
use super::pathops::Beside;
use super::{
CommandSpec, EngineError, Result, Session, always, bad, check_numbers, cmd, color_param, f64_or, f64_req, finite_numbers, has_doc, has_multi,
has_selection, id_param, named_ids, number_or, ok, str_param, switch_or, text_param,
has_selection, named_ids, number_or, object_param, ok, str_param, switch_or, text_param,
};
mod blend;
+2 -2
View File
@@ -3,7 +3,7 @@ use super::common::{fix_ids, ids_json, is_envelope, roots_param, selected_of, wr
use super::mesh::{handle_param, index_param, whole};
use super::{
Affine, Appearance, Arc, Beside, Color, Document, EngineError, EnvelopeKind, EnvelopeMap, EnvelopeOptions, Node, NodeId, NodeKind, Paint,
PathData, Point, PreserveShape, Result, Session, Value, bad, check_numbers, f64_or, f64_req, id_param, json, live, ok, switch_or, text_param,
PathData, Point, PreserveShape, Result, Session, Value, bad, check_numbers, f64_or, f64_req, json, live, object_param, ok, switch_or, text_param,
};
// ---------- Envelope ----------
@@ -419,7 +419,7 @@ pub(super) fn env_edit_contents(s: &mut Session, p: &Value) -> Result<Value> {
pub(super) fn env_set_mesh_point(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "object.envelope.setMeshPoint";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?;
let index = index_param(p, "index", C)?;
let q = Point::new(f64_req(p, "x", C)?, f64_req(p, "y", C)?);
let handle = handle_param(p, C)?;
+3 -3
View File
@@ -4,7 +4,7 @@ use super::common::{fix_ids, ids_json, is_mesh, replace_with, roots_param, selec
use super::envelope::{outline_of, rows_cols};
use super::{
Color, Document, EngineError, EnvelopeKind, GradientMesh, MeshAppearance, Node, NodeId, NodeKind, Paint, Point, Result, Session, Value, bad,
check_numbers, color_param, f64_or, f64_req, finite_numbers, gradient_sampler, id_param, json, live, ok, str_param,
check_numbers, color_param, f64_or, f64_req, finite_numbers, gradient_sampler, json, live, object_param, ok, str_param,
};
// ---------- Gradient mesh ----------
@@ -115,7 +115,7 @@ fn image_mesh(d: &Document, im: &vectorcraft_doc::ImageObject, rows: u32, cols:
/// Run `f` on the gradient mesh `p.id` (one undo step `label`); with `envelopes`, a mesh
/// envelope's grid too (see [`edit_grid`]).
fn with_mesh<T>(s: &mut Session, p: &Value, cmd: &str, label: &str, envelopes: bool, f: impl FnOnce(&mut GradientMesh) -> Result<T>) -> Result<T> {
let id = id_param(p, "id").ok_or_else(|| bad(cmd, "missing `id`"))?;
let id = object_param(p, "id", cmd)?.ok_or_else(|| bad(cmd, "missing `id`"))?;
s.edit(label, move |d, _| edit_grid(d, id, cmd, envelopes, f))
}
@@ -147,7 +147,7 @@ const MAX_POINTS: usize = 1 << 20;
/// points}…]}`. Each mesh comes once, with its points in order and without repeats.
fn point_items(p: &Value, cmd: &str) -> Result<Vec<(NodeId, BTreeSet<usize>)>> {
let one = |q: &Value| -> Result<(NodeId, BTreeSet<usize>)> {
let id = id_param(q, "id").ok_or_else(|| bad(cmd, "missing `id`"))?;
let id = object_param(q, "id", cmd)?.ok_or_else(|| bad(cmd, "missing `id`"))?;
let points = match q.get("points") {
None | Some(Value::Null) => BTreeSet::from([index_param(q, "index", cmd)?]),
Some(Value::Array(v)) if v.len() <= MAX_POINTS => v
+8 -8
View File
@@ -12,7 +12,7 @@ use super::draw2::usize_opt;
use super::pathops::Beside;
use super::{
CommandSpec, Result, Session, bad, check_numbers, check_switches, cmd, f64_or, f64_req, finite_numbers, has_anchors, has_doc, has_selected_path,
has_selection, id_param, ids_param, index, number_or, ok, str_param, switch_or, targets,
has_selection, ids_param, index, number_or, object_param, ok, str_param, switch_or, targets,
};
use crate::EngineError;
@@ -168,7 +168,7 @@ pub(super) fn path_mut(d: &mut vectorcraft_doc::Document, id: NodeId) -> Result<
}
fn append_anchor(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("path.appendAnchor", "missing id"))?;
let id = object_param(p, "id", "path.appendAnchor")?.ok_or_else(|| bad("path.appendAnchor", "missing id"))?;
let a = anchor_from_json(p, "path.appendAnchor")?;
let subpath = p.get("subpath").and_then(Value::as_u64).map(|u| u as usize);
s.edit("Pen", |d, sel| {
@@ -206,7 +206,7 @@ fn append_anchor(s: &mut Session, p: &Value) -> Result<Value> {
/// incoming handle moves as Move Direction Handle moves it: the outgoing handle turns with it only
/// on a smooth anchor, so the first segment of a path that starts at a corner keeps its shape.
fn close(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("path.close", "missing id"))?;
let id = object_param(p, "id", "path.close")?.ok_or_else(|| bad("path.close", "missing id"))?;
let subpath = p.get("subpath").and_then(Value::as_u64).map(|u| u as usize);
let handle = |key: &str| match p.get(key).filter(|h| !h.is_null()) {
None => Ok(None),
@@ -303,7 +303,7 @@ pub(crate) fn transform_anchors(s: &mut Session, label: &str, xf: vectorcraft_ge
}
fn set_handle(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("path.setHandle", "missing id"))?;
let id = object_param(p, "id", "path.setHandle")?.ok_or_else(|| bad("path.setHandle", "missing id"))?;
let si = usize_opt(p, "subpath", "path.setHandle")?.unwrap_or(0);
let ai = p.get("anchor").and_then(Value::as_u64).map(index).ok_or_else(|| bad("path.setHandle", "missing anchor"))?;
let out = str_param(p, "which") != Some("in");
@@ -321,7 +321,7 @@ fn set_handle(s: &mut Session, p: &Value) -> Result<Value> {
/// `path.moveAnchor`: one anchor to (x, y), its handles keeping their offsets (the Anchor Point
/// tool's Shift-drag).
fn move_anchor(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("path.moveAnchor", "missing id"))?;
let id = object_param(p, "id", "path.moveAnchor")?.ok_or_else(|| bad("path.moveAnchor", "missing id"))?;
let si = usize_opt(p, "subpath", "path.moveAnchor")?.unwrap_or(0);
let ai = p.get("anchor").and_then(Value::as_u64).map(index).ok_or_else(|| bad("path.moveAnchor", "missing anchor"))?;
let pos = Point::new(f64_req(p, "x", "path.moveAnchor")?, f64_req(p, "y", "path.moveAnchor")?);
@@ -341,7 +341,7 @@ fn move_anchor(s: &mut Session, p: &Value) -> Result<Value> {
}
fn set_anchors(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("path.setAnchors", "missing id"))?;
let id = object_param(p, "id", "path.setAnchors")?.ok_or_else(|| bad("path.setAnchors", "missing id"))?;
let subs = p.get("subpaths").and_then(Value::as_array).ok_or_else(|| bad("path.setAnchors", "missing subpaths"))?;
let data = PathData::new(
subs.iter()
@@ -1562,7 +1562,7 @@ fn renumber_after_insert(sel: &mut Selection, id: NodeId, si: usize, added: usiz
}
fn reshape(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("path.reshape", "missing id"))?;
let id = object_param(p, "id", "path.reshape")?.ok_or_else(|| bad("path.reshape", "missing id"))?;
let at = Point::new(f64_req(p, "x", "path.reshape")?, f64_req(p, "y", "path.reshape")?);
check_numbers(p, &["dx", "dy", "tol", "radius"], "path.reshape")?;
let delta = Vec2::new(f64_or(p, "dx", 0.0), f64_or(p, "dy", 0.0));
@@ -1684,7 +1684,7 @@ fn reshape(s: &mut Session, p: &Value) -> Result<Value> {
}
fn insert_anchor(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("path.insertAnchor", "missing id"))?;
let id = object_param(p, "id", "path.insertAnchor")?.ok_or_else(|| bad("path.insertAnchor", "missing id"))?;
let si = usize_opt(p, "subpath", "path.insertAnchor")?.unwrap_or(0);
let seg = p.get("segment").and_then(Value::as_u64).map(index).ok_or_else(|| bad("path.insertAnchor", "missing segment"))?;
let t = number_or(p, "t", 0.5, "path.insertAnchor")?.clamp(0.0, 1.0);
+3 -3
View File
@@ -13,8 +13,8 @@ use vectorcraft_doc::{Appearance, CellAlign, CellVAlign, Document, Node, NodeKin
use vectorcraft_geom::{Rect, shapes};
use super::{
CommandSpec, EngineError, NodeId, Result, Session, bad, cmd, f64_req, has_doc, has_selection, id_param, ids_param, index, number_or, switch_or,
targets,
CommandSpec, EngineError, NodeId, Result, Session, bad, cmd, f64_req, has_doc, has_selection, ids_param, index, number_or, object_param,
switch_or, targets,
};
/// The most slices one Divide Slices makes.
@@ -680,7 +680,7 @@ fn create(s: &mut Session, p: &Value) -> Result<Value> {
fn set_rect(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "object.slice.setRect";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?;
let r = rect_param(p, C)?;
let d = &s.doc()?.doc;
if d.slice(id).is_none() {
+10 -10
View File
@@ -11,8 +11,8 @@ use super::menucmds::whole_or;
use super::stroke::StrokeChange;
use super::typecmd::refresh_bounds;
use super::{
CommandSpec, EngineError, Result, Session, bad, bool_or, check_numbers, check_switches, cmd, color_value, has_doc, has_selection, id_param,
index, number_or, point_param, str_param, switch_or, targets, text_param,
CommandSpec, EngineError, Result, Session, bad, bool_or, check_numbers, check_switches, cmd, color_value, has_doc, has_selection, index,
number_or, object_param, point_param, str_param, switch_or, targets, text_param,
};
pub fn specs() -> Vec<CommandSpec> {
@@ -272,7 +272,7 @@ fn range_of(p: &Value, len: usize, cmd: &str) -> Result<(usize, usize)> {
fn edit_range(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "text.editRange";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?;
let t = text_ref(s, id)?;
let len = edit::runs_len(&t.runs);
let (a, b) = range_of(p, len, C)?;
@@ -337,7 +337,7 @@ fn clear_param(s: &Session, p: &Value, id: NodeId, c: &str) -> Result<Vec<NodeId
fn start_list(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "text.startList";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?;
let t = text_ref(s, id)?;
let len = edit::runs_len(&t.runs);
let at = offset_or(p, "at", len, C)?.min(len);
@@ -365,7 +365,7 @@ fn start_list(s: &mut Session, p: &Value) -> Result<Value> {
}
fn get_range(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("text.getRange", "missing `id`"))?;
let id = object_param(p, "id", "text.getRange")?.ok_or_else(|| bad("text.getRange", "missing `id`"))?;
if switch_or(p, "story", false, "text.getRange")? {
return get_story(s, p, id);
}
@@ -1261,7 +1261,7 @@ fn drawn_glyph(
fn set_glyph_alternate(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "text.setGlyphAlternate";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?;
let gid = match p.get("gid") {
None => return Err(bad(C, "missing `gid` (a glyph id, or null for the default glyph)")),
Some(Value::Null) => None,
@@ -1351,7 +1351,7 @@ fn set_glyph_alternate(s: &mut Session, p: &Value) -> Result<Value> {
fn glyph_alternates(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "text.glyphAlternates";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?;
let t = text_ref(s, id)?;
let (ch, bytes, face) = glyph_target(&t, p, C)?;
let default = face.glyph_for(ch);
@@ -1366,7 +1366,7 @@ fn glyph_alternates(s: &mut Session, p: &Value) -> Result<Value> {
fn touch_type(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "text.touchType";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?;
check_numbers(p, &["start"], C)?;
let start = byte_param(p, "start").ok_or_else(|| bad(C, "missing `start` (the byte offset of a character)"))?;
if !["dx", "dy", "hScale", "vScale", "rotation"].iter().any(|k| p.get(*k).is_some_and(|v| !v.is_null())) {
@@ -1592,7 +1592,7 @@ fn touch_type(s: &mut Session, p: &Value) -> Result<Value> {
fn create_in_path(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "text.createInPath";
let pid = id_param(p, "path").ok_or_else(|| bad(C, "missing `path` (a path object id)"))?;
let pid = object_param(p, "path", C)?.ok_or_else(|| bad(C, "missing `path` (a path object id)"))?;
let on_path = match str_param(p, "mode").unwrap_or("area") {
"area" => false,
"onPath" | "path" => true,
@@ -1996,7 +1996,7 @@ fn fit_headline(s: &mut Session, p: &Value) -> Result<Value> {
fn discard_empty(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "text.discardEmpty";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing `id`"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing `id`"))?;
if edit::runs_len(&text_ref(s, id)?.runs) > 0 {
return Ok(json!({"removed": false}));
}
+5 -5
View File
@@ -10,8 +10,8 @@ use vectorcraft_geom::{Affine, PathData, Vec2};
use super::edit::selected_roots;
use super::pathops::{length_given, number_given, shape_node};
use super::{
CommandSpec, EngineError, Result, Session, bad, check_numbers, cmd, color_value, f64_req, has_doc, has_selection, id_param, index, number_or, ok,
str_param, switch_or, targets, text_param,
CommandSpec, EngineError, Result, Session, bad, check_numbers, cmd, color_value, f64_req, has_doc, has_selection, index, number_or, object_param,
ok, str_param, switch_or, targets, text_param,
};
pub fn specs() -> Vec<CommandSpec> {
@@ -1273,7 +1273,7 @@ fn reshape_area(s: &mut Session, p: &Value) -> Result<Value> {
};
let mut items = Vec::with_capacity(raw.len());
for it in raw {
let id = id_param(it, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(it, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let mut refs = super::select::checked_refs(it.get("anchors"), C)?;
refs.sort_unstable();
refs.dedup();
@@ -1304,7 +1304,7 @@ fn reshape_area(s: &mut Session, p: &Value) -> Result<Value> {
fn reshape_path(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "text.reshapePath";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let mut refs = super::select::checked_refs(p.get("anchors"), C)?;
refs.sort_unstable();
refs.dedup();
@@ -1335,7 +1335,7 @@ fn reshape_path(s: &mut Session, p: &Value) -> Result<Value> {
fn set_path_handle(s: &mut Session, p: &Value) -> Result<Value> {
const C: &str = "text.setPathHandle";
let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?;
let id = object_param(p, "id", C)?.ok_or_else(|| bad(C, "missing id"))?;
let si = super::draw2::usize_opt(p, "subpath", C)?.unwrap_or(0);
let ai = p.get("anchor").and_then(Value::as_u64).map(index).ok_or_else(|| bad(C, "missing anchor"))?;
let out = super::object::choice(p, C, "which", &["in", "out"])? != Some("in");
+2 -2
View File
@@ -8,7 +8,7 @@ use vectorcraft_color::Paint;
use vectorcraft_doc::{Document, Node, NodeId, NodeKind};
use super::select::selectable_ids;
use super::{CommandSpec, EngineError, Result, Session, always, bad, bool_or, cmd, has_doc, id_param, str_param};
use super::{CommandSpec, EngineError, Result, Session, always, bad, bool_or, cmd, has_doc, object_param, str_param};
/// Magic Wand panel settings. Tolerances: colours as 0–255 RGB distance, stroke weight in
/// points, opacity in percent.
@@ -152,7 +152,7 @@ fn paintable(doc: &Document) -> Vec<NodeId> {
}
fn magic_wand(s: &mut Session, p: &Value) -> Result<Value> {
let id = id_param(p, "id").ok_or_else(|| bad("select.magicWand", "missing id"))?;
let id = object_param(p, "id", "select.magicWand")?.ok_or_else(|| bad("select.magicWand", "missing id"))?;
let st = s.doc()?;
let reference = st.doc.node(id).ok_or(EngineError::NoNode(id))?.clone();
let w = s.menu.wand.clone();
+84
View File
@@ -400,6 +400,90 @@ fn stroke_set_refuses_an_unknown_key() {
}
}
/// An object id given as something other than a whole number (`"5"`, `-1`) is refused as such,
/// naming its key, not as a missing id; an id left out is still a missing one.
#[test]
fn a_malformed_object_id_is_not_reported_as_missing() {
let mut s = Session::new();
s.execute("file.new", &json!({})).unwrap();
s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 40})).unwrap();
let undo = |s: &Session| s.doc().unwrap().history.undo.len();
let before = undo(&s);
let points = json!([[0, 0], [10, 10], [20, 0]]);
for bad in [json!("5"), json!(-1)] {
let id = || json!({ "id": bad });
let with = |extra: serde_json::Value| {
let mut p = id();
p.as_object_mut().unwrap().extend(extra.as_object().unwrap().clone());
p
};
for (cmd, p, key) in [
("stroke.widthPoint.set", id(), "id"),
("stroke.widthPoint.copy", id(), "id"),
("stroke.widthPoint.remove", id(), "id"),
("path.curvatureEdit", id(), "id"),
("path.removeAnchor", id(), "id"),
("path.convertAnchor", id(), "id"),
("path.reshapeSegment", id(), "id"),
("path.split", id(), "id"),
("path.appendAnchor", id(), "id"),
("path.close", id(), "id"),
("path.setHandle", id(), "id"),
("path.moveAnchor", id(), "id"),
("path.setAnchors", id(), "id"),
("path.reshape", id(), "id"),
("path.insertAnchor", id(), "id"),
("path.freehand", json!({"points": points, "extend": {"id": bad}}), "id"),
("path.freehand", json!({"points": points, "redraw": {"id": bad}}), "id"),
("document.node", id(), "id"),
("layer.target", id(), "id"),
("layer.setCurrent", id(), "id"),
("layer.selectAll", id(), "id"),
("layer.move", with(json!({"target": 1})), "id"),
("layer.move", json!({"ids": [1], "target": bad}), "target"),
("node.move", id(), "id"),
("node.move", json!({"id": 1, "parent": bad}), "parent"),
("text.reshapeArea", with(json!({"anchors": []})), "id"),
("text.reshapeArea", json!({"items": [{"id": bad, "anchors": []}]}), "id"),
("text.reshapePath", id(), "id"),
("text.setPathHandle", id(), "id"),
("text.editRange", id(), "id"),
("text.startList", id(), "id"),
("text.getRange", id(), "id"),
("text.setGlyphAlternate", id(), "id"),
("text.glyphAlternates", id(), "id"),
("text.touchType", id(), "id"),
("text.discardEmpty", id(), "id"),
("text.createInPath", json!({"path": bad}), "path"),
("object.envelope.setMeshPoint", id(), "id"),
("object.mesh.setPointColor", with(json!({"index": 0})), "id"),
("object.mesh.movePoint", with(json!({"index": 0, "x": 0, "y": 0})), "id"),
("object.mesh.movePoints", json!({"items": [{"id": bad, "index": 0}]}), "id"),
("object.mesh.addLine", with(json!({"x": 0, "y": 0})), "id"),
("object.mesh.deletePoint", with(json!({"index": 0})), "id"),
("object.slice.setRect", id(), "id"),
("appearance.transfer", json!({"source": bad, "target": 1}), "source"),
("appearance.transfer", json!({"source": 1, "target": bad}), "target"),
("appearance.copyFrom", json!({"source": bad}), "source"),
("livePaint.strokeEdge", json!({"point": [0, 0], "group": bad}), "group"),
("livePaint.fill", json!({"point": [0, 0], "group": bad}), "group"),
("livePaint.info", json!({"group": bad}), "group"),
("livePaint.info", id(), "id"),
("select.magicWand", id(), "id"),
] {
let e = s.execute(cmd, &p).unwrap_err().to_string();
// `layer.move` checks an `id` that stands in for `ids` before it runs.
let named = e.contains(&format!("`{key}` must be a whole number")) || (cmd == "layer.move" && e.contains("invalid object id"));
assert!(named, "{cmd} {p}: {e}");
}
}
for cmd in ["path.close", "layer.target", "text.reshapePath", "object.mesh.movePoint", "select.magicWand"] {
let e = s.execute(cmd, &json!({})).unwrap_err().to_string();
assert!(e.contains("missing"), "{cmd}: {e}");
}
assert_eq!(undo(&s), before);
}
/// A command that takes the keys of one that isn't checked yet isn't checked either.
#[test]
fn a_reference_to_an_unchecked_command_is_unchecked() {
+8 -1
View File
@@ -463,7 +463,14 @@ or a new object (`css.export`, `object.isolate`, `layer.locate`, `layer.releaseT
counterparts, `image.info`, `links.info`, `links.goTo`, `links.unembed`, `shaper.release`, `shaper.expand`,
`transparency.editOpacityMask`, `transparency.viewOpacityMask`, `symbol.edit`, the `object.blend.spine.*` commands
and `path.curvature`) refuse an `id` that is not a non-negative integer (`"abc"`, `"5"`, `true`) with an error and
change nothing; an omitted or null `id` keeps the default. A command that leaves out an object selected inside a
change nothing; an omitted or null `id` keeps the default. Commands that need an object id (`document.node`, the
`path.*` anchor and handle commands, `text.reshapePath`, `text.editRange`, the `object.mesh.*` point commands,
`layer.target`, `node.move`, `select.magicWand` and the others like them, and the `source`, `target`, `group` and
`path` keys of `appearance.transfer`, `appearance.copyFrom`, `layer.move`, the `livePaint.*` commands and
`text.createInPath`) refuse a value that is not a non-negative integer with "`id` must be a whole number", naming the
key, and report a key left out or null as missing. The optional `group` of `livePaint.fill` and `parent` of
`node.move` are refused the same way when given; left out, they keep their defaults. `livePaint.info` reads `id` when
`group` is left out or null, and reports a missing group only when both are left out. A command that leaves out an object selected inside a
selected group also leaves it out when `ids` list it together with a group that holds it. Every command that takes
`ids` (or `slices`, for the slice commands) refuses a list with an entry that is not a non-negative integer, or a value
that is not a list, and changes nothing; it does not act on the other entries. The error for an entry names the key,