From fae9bc84ce244a3dc50e169163507f4915756eab Mon Sep 17 00:00:00 2001 From: KnottyDyes <47539659+KnottyDyes@users.noreply.github.com> Date: Sun, 11 Oct 2026 22:30:21 -0500 Subject: [PATCH] fix(text): reject unknown character fill swatches Reject unknown character fill swatches before modifying text or undo history. Registered swatch names remain valid, including names that resemble hex colours. Validation: invalid-name atomicity and save/reopen regressions, plus all six cargo xtask ci gates on main after #514 and #523 (1,186 native tests passed, 3 ignored; 15 crates passed WASM compilation checks). Task: #403 --- crates/engine/src/cmd/find.rs | 1 + crates/engine/src/cmd/style.rs | 152 +++++++++++++++++++++++++++++++++ crates/engine/src/cmd/text.rs | 13 +++ 3 files changed, 166 insertions(+) diff --git a/crates/engine/src/cmd/find.rs b/crates/engine/src/cmd/find.rs index 8781604..5dc0bc8 100644 --- a/crates/engine/src/cmd/find.rs +++ b/crates/engine/src/cmd/find.rs @@ -181,6 +181,7 @@ fn change(s: &mut Session, p: &Value) -> Result { over.set_json(k, v).map_err(|e| bad("find.change", e))?; } } + super::text::validate_char_fill(&s.doc()?.doc, &over, "find.change")?; let targets = scope(s, p)?; s.edit(|d, sel| { let mut count = 0usize; diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index 296a8d0..8a520b6 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -444,6 +444,7 @@ fn create_para(s: &mut Session, p: &Value) -> Result { chars = full; } } + super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.paragraph.create")?; let based_on = str_param(p, "basedOn").map(str::to_string); let next = str_param(p, "nextStyle").map(str::to_string); s.edit(|d, _| { @@ -470,6 +471,7 @@ fn CharProps_to_attrs(p: &designcraft_doc::CharProps) -> CharAttrs { fn edit_para(s: &mut Session, p: &Value) -> Result { let name = str_param(p, "name").ok_or_else(|| bad("style.paragraph.edit", "missing name"))?.to_string(); let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.paragraph.edit")?; let rename = str_param(p, "rename").map(str::to_string); let based = p.get("basedOn").cloned(); s.edit(|d, _| { @@ -621,6 +623,7 @@ fn delete_para(s: &mut Session, p: &Value) -> Result { fn create_char(s: &mut Session, p: &Value) -> Result { let base = str_param(p, "name").unwrap_or("Character Style 1").to_string(); let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.character.create")?; // Based on [None] is based on nothing. let based_on = str_param(p, "basedOn").filter(|b| *b != designcraft_doc::NO_CHAR_STYLE).map(str::to_string); s.edit(|d, _| { @@ -641,6 +644,7 @@ fn edit_char(s: &mut Session, p: &Value) -> Result { return Err(bad("style.character.edit", "[None] can't be edited")); } let chars: CharAttrs = attrs(p.get("chars"), |a: &mut CharAttrs, k, v| a.set_json(k, v))?; + super::text::validate_char_fill(&s.doc()?.doc, &chars, "style.character.edit")?; // `null` unsets an attribute (the style no longer sets it). let unset: Vec = p .get("chars") @@ -1045,6 +1049,154 @@ fn rename_in(it: &mut designcraft_doc::Item, from: &str, to: &str) { mod color_tests { use super::*; + fn fill_session() -> Session { + let mut session = Session::new(); + session.execute("file.new", &json!({})).unwrap(); + let frame = session.execute("frame.create", &json!({"rect": [0, 0, 200, 100], "content": "text", "text": "Hi"})).unwrap(); + session.execute("text.select", &json!({"story": frame["story"], "anchor": 0, "focus": 2})).unwrap(); + session.execute("style.paragraph.create", &json!({"name": "Existing"})).unwrap(); + session.execute("style.character.create", &json!({"name": "Existing"})).unwrap(); + session.execute("type.char", &json!({"size": 18})).unwrap(); + session.execute("edit.undo", &json!({})).unwrap(); + session + } + + fn assert_fill_rejected(session: &mut Session, command: &str, params: Value) { + let before = session.doc().unwrap().clone(); + let journal = session.journal.clone(); + let fill = params + .pointer("/chars/fill") + .or_else(|| params.pointer("/attrs/fill")) + .or_else(|| params.get("fill")) + .and_then(Value::as_str) + .unwrap_or("Missing Ink"); + + let result = session.execute(command, ¶ms); + + match result { + Err(crate::EngineError::BadParams { cmd, msg }) => { + assert_eq!(cmd, command); + assert!(msg.contains(fill), "{msg}"); + assert!(msg.contains("swatch.create") && msg.contains("returned name"), "{msg}"); + } + other => panic!("expected BadParams for {command}, got {other:?}"), + } + let after = session.doc().unwrap(); + assert!(std::sync::Arc::ptr_eq(&before.doc, &after.doc)); + assert_eq!(before.selection, after.selection); + assert_eq!(before.revision, after.revision); + assert_eq!(format!("{:?}", before.history), format!("{:?}", after.history)); + assert_eq!(journal, session.journal); + } + + #[test] + fn char_fill_rejects_unknown_flat_and_nested_names_when_formatting() { + for fill in ["Missing Ink", "#ff0000", "[black]", " [Black]"] { + for params in [json!({"fill": fill, "size": 24}), json!({"attrs": {"fill": fill, "size": 24}})] { + let mut session = fill_session(); + assert_fill_rejected(&mut session, "type.char", params); + } + } + } + + #[test] + fn char_fill_rejects_unknown_names_when_creating_or_editing_styles() { + for command in ["style.paragraph.create", "style.paragraph.edit", "style.character.create", "style.character.edit"] { + for fill in ["Missing Ink", "#ff0000"] { + let mut session = fill_session(); + session.execute("style.paragraph.apply", &json!({"name": "Existing"})).unwrap(); + session.execute("style.character.apply", &json!({"name": "Existing"})).unwrap(); + assert_fill_rejected(&mut session, command, json!({"name": "Existing", "rename": "Renamed", "chars": {"fill": fill, "size": 24}})); + } + } + } + + #[test] + fn char_fill_rejects_inherited_unknown_name_when_creating_from_selection() { + let mut session = fill_session(); + session + .edit(|document, selection| { + let selected = selection.text.unwrap(); + document.story_mut(selected.story).unwrap().format_chars(selected.range(), |format| format.over.fill = Some("Missing Ink".into())); + Ok(()) + }) + .unwrap(); + + assert_fill_rejected(&mut session, "style.paragraph.create", json!({"name": "Inherited", "fromSelection": true})); + } + + #[test] + fn char_fill_accepts_valid_override_when_selection_has_unknown_name() { + let mut session = fill_session(); + session + .edit(|document, selection| { + let selected = selection.text.unwrap(); + document.story_mut(selected.story).unwrap().format_chars(selected.range(), |format| format.over.fill = Some("Missing Ink".into())); + Ok(()) + }) + .unwrap(); + + session.execute("style.paragraph.create", &json!({"name": "Override", "fromSelection": true, "chars": {"fill": "[Paper]"}})).unwrap(); + + assert_eq!(session.doc().unwrap().doc.styles.para("Override").unwrap().chars.fill.as_deref(), Some("[Paper]")); + } + + #[test] + fn char_fill_rejects_unknown_name_when_changing_text_even_without_output() { + for (find, change) in [("Hi", "Bye"), ("Hi", ""), ("Absent", "Bye")] { + for fill in ["Missing Ink", "#ff0000"] { + let mut session = fill_session(); + assert_fill_rejected(&mut session, "find.change", json!({"find": find, "change": change, "attrs": {"fill": fill}})); + } + } + } + + #[test] + fn char_fill_preserves_exact_registered_names_when_used_at_all_boundaries() { + for fill in ["Brand Ink", "#ff0000", "[None]", "[Black]"] { + let mut session = fill_session(); + if !fill.starts_with('[') { + let created = session.execute("swatch.create", &json!({"name": fill, "color": "#108080"})).unwrap(); + assert_eq!(created["name"], fill); + } + for params in [json!({"fill": fill}), json!({"attrs": {"fill": fill}})] { + session.execute("type.char", ¶ms).unwrap(); + assert_eq!(session.execute("type.selectionAttrs", &json!({})).unwrap()["chars"]["fill"], fill); + } + for kind in ["paragraph", "character"] { + session.execute(&format!("style.{kind}.create"), &json!({"name": "Colored", "chars": {"fill": fill}})).unwrap(); + session.execute(&format!("style.{kind}.edit"), &json!({"name": "Existing", "chars": {"fill": fill}})).unwrap(); + let document = &session.doc().unwrap().doc; + for name in ["Colored", "Existing"] { + let attrs = if kind == "paragraph" { + &document.styles.para(name).unwrap().chars + } else { + &document.styles.char_style(name).unwrap().chars + }; + assert_eq!(attrs.fill.as_deref(), Some(fill)); + } + } + assert_eq!(session.execute("find.change", &json!({"find": "Hi", "change": "Bye", "attrs": {"fill": fill}})).unwrap()["count"], 1); + let selected = session.doc().unwrap().selection.text.unwrap(); + let story = session.doc().unwrap().doc.story(selected.story).unwrap(); + assert_eq!(story.text, "Bye"); + assert_eq!(story.format_after(0).over.fill.as_deref(), Some(fill)); + } + } + + #[test] + fn char_fill_accepts_omitted_and_null_attributes_when_used_at_all_boundaries() { + for chars in [json!({"size": 18}), json!({"fill": null})] { + let mut session = fill_session(); + session.execute("type.char", &json!({"attrs": chars})).unwrap(); + for kind in ["paragraph", "character"] { + session.execute(&format!("style.{kind}.create"), &json!({"name": "Nullable", "chars": chars})).unwrap(); + session.execute(&format!("style.{kind}.edit"), &json!({"name": "Existing", "chars": chars})).unwrap(); + } + assert_eq!(session.execute("find.change", &json!({"find": "Hi", "change": "Bye", "attrs": chars})).unwrap()["count"], 1); + } + } + #[test] fn percent_components_at_most_one_stay_percent() { use designcraft_color::Color; diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index 4950d82..71d390d 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -819,6 +819,18 @@ fn warichu_cmd(s: &mut Session, p: &Value) -> Result { format_chars(s, &Value::Object(body)) } +pub(crate) fn validate_char_fill(document: &designcraft_doc::Document, attrs: &CharAttrs, command_id: &str) -> Result<()> { + if let Some(fill) = &attrs.fill + && document.swatch(fill).is_none() + { + return Err(bad( + command_id, + format!("unknown character fill `{fill}`; create a swatch with `swatch.create` and use its returned name as `fill`"), + )); + } + Ok(()) +} + pub(crate) fn format_chars(s: &mut Session, attrs: &Value) -> Result { let mut a = CharAttrs::default(); if let Some(o) = attrs.as_object() { @@ -826,6 +838,7 @@ pub(crate) fn format_chars(s: &mut Session, attrs: &Value) -> Result { a.set_json(k, v).map_err(|e| bad("type.char", e))?; } } + validate_char_fill(&s.doc()?.doc, &a, "type.char")?; let cleared: Vec = attrs.as_object().map(|o| o.iter().filter(|(_, v)| v.is_null()).map(|(k, _)| k.clone()).collect()).unwrap_or_default(); let targets = format_targets(s); s.edit(|d, _| {