diff --git a/cli/src/routing.rs b/cli/src/routing.rs index 55f9e25..4650c85 100644 --- a/cli/src/routing.rs +++ b/cli/src/routing.rs @@ -7,6 +7,23 @@ use serde_json::{Value, json}; use tokio::sync::{mpsc, oneshot, watch}; use uuid::Uuid; +fn params_not_object(params: &Value) -> Value { + let kind = match params { + Value::String(_) => "a string", + Value::Array(_) => "an array", + Value::Number(_) => "a number", + Value::Null => "null", + Value::Bool(_) => "a boolean", + Value::Object(_) => "an object", + }; + error( + -32602, + &format!( + "params must be a JSON object, got {kind}. Send the object itself, not a JSON-encoded string." + ), + ) +} + impl Host { /// Queue an incoming response or notification for its destination. pub(crate) fn send(&self, pid: &str, value: Value) { @@ -69,7 +86,7 @@ impl Host { let host = self.clone(); Box::pin(async move { if !params.is_object() { - return Err(error(-32602, "params must be an object")); + return Err(params_not_object(¶ms)); } if caller != "console" && caller != "core" { let state = host.0.state.lock().unwrap(); @@ -238,7 +255,7 @@ impl Host { // Deliver progress before a following response removes its destination. let params = v.get("params").cloned().unwrap_or(json!({})); let result = if !params.is_object() { - Err(error(-32602, "params must be an object")) + Err(params_not_object(¶ms)) } else if method == "core.unlock" { if let Some(path) = params["path"].as_str() { self.unlock(pid, path); diff --git a/cli/tests/host/core.rs b/cli/tests/host/core.rs index 2a062d2..95707ad 100644 --- a/cli/tests/host/core.rs +++ b/cli/tests/host/core.rs @@ -500,14 +500,119 @@ async fn unimplemented_core_methods_return_method_not_found() { async fn params_must_be_objects() { let w = TestWorld::new(); let h = w.start().await; + for (params, kind) in [ + (json!("{\"text\":\"hello\"}"), "a string"), + (json!([]), "an array"), + (json!(42), "a number"), + (json!(null), "null"), + (json!(true), "a boolean"), + ] { + let expected = json!({ + "code": -32602, + "message": format!("params must be a JSON object, got {kind}. Send the object itself, not a JSON-encoded string.") + }); + for method in ["probe.echo", "core.ps"] { + assert_eq!(h.call(method, params.clone()).await.unwrap_err(), expected); + } + assert_eq!( + h.call("core.call", json!({"method":"probe.echo", "params":params})) + .await + .unwrap_err(), + expected + ); + } + let object = json!({"text":"hello"}); + assert_eq!(call(&h, "probe.echo", object.clone()).await, object); assert_eq!( - h.call("probe.echo", json!([])).await.unwrap_err()["code"], - -32602 + call( + &h, + "core.call", + json!({"method":"probe.echo", "params":object}) + ) + .await, + object + ); + assert_eq!( + call(&h, "core.call", json!({"method":"probe.echo"})).await, + json!({}) ); h.stop().await; } +/// PROTOCOL 3.1: stdout fast paths reject non-objects without changing defaults. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn stdout_params_must_be_objects() { + let w = TestWorld::new(); + let h = w.start().await; + let mut notifications = h.subscribe(); + for method in ["core.notify", "core.unlock", "probe.echo"] { + for (params, kind) in [ + (json!("{}"), "a string"), + (json!([]), "an array"), + (json!(1.5), "a number"), + (json!(null), "null"), + (json!(false), "a boolean"), + ] { + let response = call( + &h, + "probe.raw_call", + json!({ + "jsonrpc":"2.0", "id":1, "method":method, "params":params + }), + ) + .await; + assert_eq!(response["id"], 1); + assert_eq!( + response["error"], + json!({ + "code": -32602, + "message": format!("params must be a JSON object, got {kind}. Send the object itself, not a JSON-encoded string.") + }) + ); + } + let params = match method { + "core.notify" => json!({"name":"valid", "params":{"n":1}}), + "core.unlock" => json!({"path":"/state/not-held"}), + _ => json!({"n":1}), + }; + let response = call( + &h, + "probe.raw_call", + json!({ + "jsonrpc":"2.0", "id":1, "method":method, "params":params + }), + ) + .await; + assert_eq!( + response["result"], + if method == "probe.echo" { + params + } else { + Value::Null + } + ); + assert!(response.get("error").is_none()); + let omitted = call( + &h, + "probe.raw_call", + json!({ + "jsonrpc":"2.0", "id":1, "method":method + }), + ) + .await; + match method { + "core.notify" => { + assert_eq!(omitted["error"]["message"], "name must be a string"); + assert_eq!(notifications.try_recv().unwrap()["params"]["name"], "valid"); + } + "core.unlock" => assert_eq!(omitted["error"]["message"], "path must be a string"), + _ => assert_eq!(omitted["result"], json!({})), + } + } + h.stop().await; +} + /// PROTOCOL 3.9: exit without answer reports status and last logs. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn exit_without_answer_reports_status_and_last_logs() { diff --git a/cli/tests/plugins/probe/src/main.rs b/cli/tests/plugins/probe/src/main.rs index 9ae4d44..c20bd7b 100644 --- a/cli/tests/plugins/probe/src/main.rs +++ b/cli/tests/plugins/probe/src/main.rs @@ -470,6 +470,10 @@ fn dispatch(request: &Value, lines: &mut Lines) -> Option { "network" => network(params), "read" => read_file(params), "write" => write_file(params), + "raw_call" => { + send(params.clone()); + receive(lines) + } "call" => outgoing_call(params, lines), "lock" => lock(params, lines), _ => { diff --git a/sdk/macros/src/lib.rs b/sdk/macros/src/lib.rs index 133408b..a74a0f6 100644 --- a/sdk/macros/src/lib.rs +++ b/sdk/macros/src/lib.rs @@ -270,7 +270,7 @@ pub fn rpc(args: TokenStream, input: TokenStream) -> TokenStream { fn methods_document()->::sausage_plugin::serde_json::Value {let mut doc=::sausage_plugin::document(env!("CARGO_PKG_NAME"),env!("CARGO_PKG_VERSION")); Self::add_methods(&mut doc); #grace ::sausage_plugin::finish_document(doc)} fn describe()->::sausage_plugin::serde_json::Value {let mut doc=::sausage_plugin::document(env!("CARGO_PKG_NAME"),env!("CARGO_PKG_VERSION")); Self::add_methods(&mut doc); #grace #(doc.endpoint::<#types>(#names);)* ::sausage_plugin::finish_document(doc)} fn discover(&self)->::sausage_plugin::serde_json::Value {::describe()} - fn dispatch<'a>(&'a self, method:&'a str, params: ::sausage_plugin::serde_json::Value,ctx:&'a ::sausage_plugin::Ctx)->::std::pin::Pin>+'a>> {Box::pin(async move {if !params.is_object() {return Err(::sausage_plugin::Error::new(-32602,"params must be an object"));} match method {#(#arms,)* _=>Err(::sausage_plugin::Error::new(-32601,"method not found"))}})} + fn dispatch<'a>(&'a self, method:&'a str, params: ::sausage_plugin::serde_json::Value,ctx:&'a ::sausage_plugin::Ctx)->::std::pin::Pin>+'a>> {Box::pin(async move {if !params.is_object() {return Err(::sausage_plugin::Error::params_not_object(¶ms));} match method {#(#arms,)* _=>Err(::sausage_plugin::Error::new(-32601,"method not found"))}})} #stop }}) })(); diff --git a/sdk/src/calls.rs b/sdk/src/calls.rs index 91a24c8..c8be755 100644 --- a/sdk/src/calls.rs +++ b/sdk/src/calls.rs @@ -37,7 +37,7 @@ impl IntoFuture for Call { fn into_future(self) -> Self::IntoFuture { Box::pin(async move { if !self.params.is_object() { - return Err(Error::new(-32602, "params must be an object")); + return Err(Error::params_not_object(&self.params)); } let (tx, rx) = oneshot::channel(); let id = STATE.with(|s| { diff --git a/sdk/src/error.rs b/sdk/src/error.rs index 7f3fa31..132a8a9 100644 --- a/sdk/src/error.rs +++ b/sdk/src/error.rs @@ -15,6 +15,25 @@ pub struct Error { pub data: Option, } impl Error { + /// Build the object-params diagnostic used by calls and generated dispatch. + #[doc(hidden)] + pub fn params_not_object(params: &Value) -> Self { + let kind = match params { + Value::String(_) => "a string", + Value::Array(_) => "an array", + Value::Number(_) => "a number", + Value::Null => "null", + Value::Bool(_) => "a boolean", + Value::Object(_) => "an object", + }; + Self::new( + -32602, + format!( + "params must be a JSON object, got {kind}. Send the object itself, not a JSON-encoded string." + ), + ) + } + /// Construct an error without structured details. pub fn new(code: i64, message: impl Into) -> Self { Self { diff --git a/sdk/tests/sdk.rs b/sdk/tests/sdk.rs index afd8aa2..ce12cba 100644 --- a/sdk/tests/sdk.rs +++ b/sdk/tests/sdk.rs @@ -116,6 +116,53 @@ fn discovery_and_dispatch() { .into(); assert_eq!(internal.code, -32603); } +#[sausage_plugin::test] +async fn params_must_be_objects() { + for (params, kind) in [ + (json!("{\"a\":2,\"b\":3}"), "a string"), + (json!([]), "an array"), + (json!(42), "a number"), + (json!(null), "null"), + (json!(true), "a boolean"), + ] { + let message = format!( + "params must be a JSON object, got {kind}. Send the object itself, not a JSON-encoded string." + ); + let outgoing = sausage_plugin::call("plugin.add", params.clone()) + .await + .unwrap_err(); + assert_eq!(outgoing.code, -32602); + assert_eq!(outgoing.message, message); + for method in ["add", "choice", "unknown"] { + let incoming = Plugin + .dispatch(method, params.clone(), &Ctx::default()) + .await + .unwrap_err(); + assert_eq!(incoming.code, -32602); + assert_eq!(incoming.message, message); + } + } + assert_eq!( + Plugin + .dispatch("add", json!({"a":2,"b":3}), &Ctx::default()) + .await + .unwrap(), + 5 + ); + assert_eq!( + Plugin + .dispatch("defaults", json!({}), &Ctx::default()) + .await + .unwrap(), + json!({"value":""}) + ); + // An object reaches transport validation rather than the params rejection. + let outgoing = sausage_plugin::call("plugin.add", json!({"a":2,"b":3})) + .await + .unwrap_err(); + assert!(outgoing.message.contains("no core transport")); +} + #[sausage_plugin::test] async fn missing_core_fails_without_wait() { assert!(