Merge pull request #639 from MarekSpodBiedry:issue-516

crates/engine/src/cmd/extra.rs

@@ -1073,8 +1073,13 @@ fn toggle_checkbox(s: &mut Session, p: &Json) -> Result<Json> {
     })
 }
 
+/// Copy of the params that is always a JSON object, so keys can be assigned; any other JSON value counts as no params (like null).
+fn object_params(p: &Json) -> Json {
+    Json::Object(p.as_object().cloned().unwrap_or_default())
+}
+
 fn theme_colors(s: &mut Session, p: &Json) -> Result<Json> {
-    let mut params = p.clone();
+    let mut params = object_params(p);
     if params.get("name").is_none() {
         params["name"] = json!(s.doc()?.wb.theme.name.clone());
     }
@@ -1095,7 +1100,7 @@ fn background(s: &mut Session, p: &Json) -> Result<Json> {
             Ok(Json::Null)
         });
     }
-    let mut params = p.clone();
+    let mut params = object_params(p);
     params["alt"] = json!("Sheet background");
     params["at"] = json!("A1");
     s.execute("insert.picture", params)
@@ -1111,7 +1116,7 @@ fn draw_border(s: &mut Session, p: &Json) -> Result<Json> {
         "outline" => "outside",
         _ => "all",
     };
-    let mut params = p.clone();
+    let mut params = object_params(p);
     params["preset"] = json!(preset);
     s.execute("home.borders", params)
 }

crates/engine/src/tests.rs

@@ -1738,6 +1738,23 @@ fn every_command_survives_empty_params() {
     }
 }
 
+#[test]
+fn non_object_params_do_not_panic_in_wrapper_commands() {
+    // Issue #516.
+    for cmd in ["pageLayout.themeColors", "pageLayout.background", "home.drawBorder"] {
+        for params in [json!([]), json!(5), json!("x"), json!(true)] {
+            let mut s = s();
+            let r = s.execute(cmd, params.clone());
+            assert!(!matches!(r, Err(crate::EngineError::Internal(..))), "{cmd} {params}: {r:?}");
+        }
+    }
+    // A saved script step forwards its params unchanged into the same call.
+    let mut s = s();
+    s.execute("automate.newScript", json!({"name": "bad", "commands": [{"command": "home.drawBorder", "params": []}]})).unwrap();
+    let r = s.execute("automate.runScript", json!({"name": "bad"}));
+    assert!(!matches!(r, Err(crate::EngineError::Internal(..))), "script: {r:?}");
+}
+
 fn fmt_of(s: &Session, a: &str) -> String {
     let wb = &s.doc().unwrap().wb;
     let sh = wb.active().unwrap();