From fca9055229984c9741d2b413cc0c917acf18536d Mon Sep 17 00:00:00 2001 From: Danny Ward Date: Thu, 15 Oct 2026 15:50:55 -0400 Subject: [PATCH 1/3] fix(protocol): enforce request IDs and params --- .../v1/08-notification-not-supported.json | 3 +- .../v1/68-invalid-request-id-types.json | 11 +++ .../v1/69-required-object-params.json | 12 +++ conformance/mock-provider/src/server.rs | 89 +++++++++++++++++-- docs/spec/transport.md | 5 +- 5 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 conformance/fixtures/v1/68-invalid-request-id-types.json create mode 100644 conformance/fixtures/v1/69-required-object-params.json diff --git a/conformance/fixtures/v1/08-notification-not-supported.json b/conformance/fixtures/v1/08-notification-not-supported.json index ffae975..5a22986 100644 --- a/conformance/fixtures/v1/08-notification-not-supported.json +++ b/conformance/fixtures/v1/08-notification-not-supported.json @@ -1,9 +1,10 @@ { "name": "notification-not-supported", - "description": "LPP v1 defines no notifications: a message without an id is a protocol violation answered with an LPP invalidRequest error (id null in the response).", + "description": "LPP v1 defines no notifications: a missing or null id is a protocol violation answered with an LPP invalidRequest error (id null in the response).", "scope": "protocol", "steps": [ {"request":{"method":"lpp/shutdown"},"expectResponse":{"error":{"code":-32000,"message":{"$provider":"nonEmptyString"},"data":{"lpp":{"kind":"invalidRequest","details":{"reason":"notificationNotSupported"}}}}}}, + {"rawLine":"{\"jsonrpc\":\"2.0\",\"id\":null,\"method\":\"lpp/shutdown\",\"params\":{}}","expectResponse":{"error":{"code":-32000,"message":{"$provider":"nonEmptyString"},"data":{"lpp":{"kind":"invalidRequest","details":{"reason":"notificationNotSupported"}}}}}}, {"request":{"id":1,"method":"lpp/initialize","params":{"protocolVersion":"1.0"}},"expectResponse":{"result":{"protocolVersion":"1.0","serverInfo":{"name":{"$provider":"nonEmptyString"},"version":{"$provider":"nonEmptyString"}},"languages":{"$provider":"languageList"},"capabilities":{"check":{"$provider":"boolean"},"compile":{"$provider":"boolean"},"reconstruct":{"$provider":"boolean"},"symbols":{"$provider":"boolean"},"definition":{"$provider":"boolean"},"references":{"$provider":"boolean"},"rename":{"$provider":"boolean"},"editValidation":{"$provider":"boolean"}}}}}, {"request":{"id":2,"method":"lpp/shutdown","params":{}},"expectResponse":{"result":null}} ] diff --git a/conformance/fixtures/v1/68-invalid-request-id-types.json b/conformance/fixtures/v1/68-invalid-request-id-types.json new file mode 100644 index 0000000..e0fb8e1 --- /dev/null +++ b/conformance/fixtures/v1/68-invalid-request-id-types.json @@ -0,0 +1,11 @@ +{ + "name": "invalid-request-id-types", + "description": "A non-null request id must be an integer or string; other JSON values receive standard -32600 with a null response id.", + "scope": "protocol", + "steps": [ + {"rawLine":"{\"jsonrpc\":\"2.0\",\"id\":1.5,\"method\":\"lpp/shutdown\",\"params\":{}}","expectResponse":{"error":{"code":-32600,"message":{"$provider":"nonEmptyString"}}}}, + {"rawLine":"{\"jsonrpc\":\"2.0\",\"id\":true,\"method\":\"lpp/shutdown\",\"params\":{}}","expectResponse":{"error":{"code":-32600,"message":{"$provider":"nonEmptyString"}}}}, + {"rawLine":"{\"jsonrpc\":\"2.0\",\"id\":[1],\"method\":\"lpp/shutdown\",\"params\":{}}","expectResponse":{"error":{"code":-32600,"message":{"$provider":"nonEmptyString"}}}}, + {"rawLine":"{\"jsonrpc\":\"2.0\",\"id\":{},\"method\":\"lpp/shutdown\",\"params\":{}}","expectResponse":{"error":{"code":-32600,"message":{"$provider":"nonEmptyString"}}}} + ] +} diff --git a/conformance/fixtures/v1/69-required-object-params.json b/conformance/fixtures/v1/69-required-object-params.json new file mode 100644 index 0000000..a01bdb9 --- /dev/null +++ b/conformance/fixtures/v1/69-required-object-params.json @@ -0,0 +1,12 @@ +{ + "name": "required-object-params", + "description": "Every request requires an object-valued params field; missing or non-object params receive -32602.", + "scope": "protocol", + "steps": [ + {"request":{"id":1,"method":"lpp/initialize","params":{"protocolVersion":"1.0","clientInfo":{"name":"wright","version":"0.2.0"}}},"expectResponse":{"result":{"protocolVersion":"1.0","serverInfo":{"name":{"$provider":"nonEmptyString"},"version":{"$provider":"nonEmptyString"}},"languages":{"$provider":"languageList"},"capabilities":{"check":{"$provider":"boolean"},"compile":{"$provider":"boolean"},"reconstruct":{"$provider":"boolean"},"symbols":{"$provider":"boolean"},"definition":{"$provider":"boolean"},"references":{"$provider":"boolean"},"rename":{"$provider":"boolean"},"editValidation":{"$provider":"boolean"}}}}}, + {"request":{"id":2,"method":"lpp/shutdown"},"expectResponse":{"error":{"code":-32602,"message":{"$provider":"nonEmptyString"}}}}, + {"request":{"id":3,"method":"lpp/shutdown","params":[]},"expectResponse":{"error":{"code":-32602,"message":{"$provider":"nonEmptyString"}}}}, + {"request":{"id":4,"method":"lpp/shutdown","params":null},"expectResponse":{"error":{"code":-32602,"message":{"$provider":"nonEmptyString"}}}}, + {"request":{"id":5,"method":"lpp/shutdown","params":{}},"expectResponse":{"result":null}} + ] +} diff --git a/conformance/mock-provider/src/server.rs b/conformance/mock-provider/src/server.rs index f1bbabd..8b7d8c9 100644 --- a/conformance/mock-provider/src/server.rs +++ b/conformance/mock-provider/src/server.rs @@ -125,10 +125,13 @@ impl Server { return invalid_request(); }; let id = match object.get("id") { - Some(id @ (Value::Number(_) | Value::String(_))) => id.clone(), - // No notifications in LPP v1: a message without an id (or with a - // null id) is a protocol violation. - _ => { + Some(id @ Value::String(_)) => id.clone(), + Some(Value::Number(number)) if number.is_i64() || number.is_u64() => { + Value::Number(number.clone()) + } + // No notifications in LPP v1: a missing or null id is a protocol + // violation, distinct from an invalid non-null id. + None | Some(Value::Null) => { return lpp_error( Value::Null, "invalidRequest", @@ -136,8 +139,12 @@ impl Server { "invalid request: LPP v1 defines no notifications", ); } + Some(_) => return invalid_request(), + }; + let params = match object.get("params") { + Some(params) if params.is_object() => params.clone(), + _ => return std_error(id, -32602, "Invalid params"), }; - let params = object.get("params").cloned().unwrap_or_else(|| json!({})); match method { "lpp/initialize" => self.initialize(&id, params), "lpp/shutdown" => self.shutdown(&id), @@ -215,3 +222,75 @@ impl Server { } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn server() -> Server { + Server::new(DEFAULT_PROTOCOL_VERSION.to_string(), HashSet::new()) + } + + #[test] + fn invalid_non_null_request_ids_are_invalid_requests() { + let mut server = server(); + for request in [ + r#"{"jsonrpc":"2.0","id":1.5,"method":"lpp/shutdown","params":{}}"#, + r#"{"jsonrpc":"2.0","id":true,"method":"lpp/shutdown","params":{}}"#, + r#"{"jsonrpc":"2.0","id":[1],"method":"lpp/shutdown","params":{}}"#, + r#"{"jsonrpc":"2.0","id":{},"method":"lpp/shutdown","params":{}}"#, + ] { + let response = server.handle_message(request); + assert_eq!(response["error"]["code"], json!(-32600)); + assert_eq!(response["id"], Value::Null); + } + } + + #[test] + fn missing_and_null_ids_keep_the_notification_error() { + let mut server = server(); + for request in [ + r#"{"jsonrpc":"2.0","method":"lpp/shutdown","params":{}}"#, + r#"{"jsonrpc":"2.0","id":null,"method":"lpp/shutdown","params":{}}"#, + ] { + let response = server.handle_message(request); + assert_eq!( + response["error"]["data"]["lpp"]["details"]["reason"], + json!("notificationNotSupported") + ); + assert_eq!(response["id"], Value::Null); + } + } + + #[test] + fn params_must_be_present_and_an_object() { + let mut server = server(); + let initialized = server.handle_message( + r#"{"jsonrpc":"2.0","id":1,"method":"lpp/initialize","params":{"protocolVersion":"1.0"}}"#, + ); + assert!(initialized.get("result").is_some()); + + for (request, id) in [ + ( + r#"{"jsonrpc":"2.0","id":2,"method":"lpp/shutdown"}"#, + json!(2), + ), + ( + r#"{"jsonrpc":"2.0","id":3,"method":"lpp/shutdown","params":[]}"#, + json!(3), + ), + ( + r#"{"jsonrpc":"2.0","id":4,"method":"lpp/shutdown","params":null}"#, + json!(4), + ), + ] { + let response = server.handle_message(request); + assert_eq!(response["error"]["code"], json!(-32602)); + assert_eq!(response["id"], id); + } + + let shutdown = server + .handle_message(r#"{"jsonrpc":"2.0","id":5,"method":"lpp/shutdown","params":{}}"#); + assert_eq!(shutdown["result"], Value::Null); + } +} diff --git a/docs/spec/transport.md b/docs/spec/transport.md index de894de..d56fe83 100644 --- a/docs/spec/transport.md +++ b/docs/spec/transport.md @@ -48,6 +48,9 @@ rules: `details.reason` = `notificationNotSupported`, with `id` null in the response. (This is an explicit deviation from JSON-RPC notification semantics, chosen so that client mistakes are always observable.) +* **Invalid ids**: a non-null `id` that is neither an integer nor a string is + an invalid request and MUST receive the standard `-32600` error with a null + response `id`. * **No batches**: clients MUST NOT send batch messages. If a provider receives an array message, it MUST respond with the standard JSON-RPC error `-32600` ("Invalid Request") and `id` null. @@ -63,7 +66,7 @@ data attached: | Code | Name | When | | --- | --- | --- | | `-32700` | Parse error | A message line is not valid JSON. Response `id` is null. | -| `-32600` | Invalid Request | The message is not a valid JSON-RPC request (wrong `jsonrpc` value, missing `method`, non-object message, batch). | +| `-32600` | Invalid Request | The message is not a valid JSON-RPC request (wrong `jsonrpc` value, missing `method`, invalid non-null `id`, non-object message, batch). | | `-32601` | Method not found | The method name is not a known LPP v1 method. | | `-32602` | Invalid params | The `params` value does not match the method's schema (wrong types or missing required fields). | | `-32603` | Internal error | The provider failed internally. | From 03c814d6333886315c70b45c90e8d8115090668f Mon Sep 17 00:00:00 2001 From: Danny Ward Date: Thu, 15 Oct 2026 16:36:37 -0400 Subject: [PATCH 2/3] fix(protocol): preserve large integer request IDs --- .../v1/70-large-integer-request-ids.json | 15 ++++++++++++ conformance/mock-provider/Cargo.toml | 2 +- conformance/mock-provider/src/server.rs | 24 ++++++++++++++++++- conformance/runner/Cargo.toml | 2 +- 4 files changed, 40 insertions(+), 3 deletions(-) create mode 100644 conformance/fixtures/v1/70-large-integer-request-ids.json diff --git a/conformance/fixtures/v1/70-large-integer-request-ids.json b/conformance/fixtures/v1/70-large-integer-request-ids.json new file mode 100644 index 0000000..d26621f --- /dev/null +++ b/conformance/fixtures/v1/70-large-integer-request-ids.json @@ -0,0 +1,15 @@ +{ + "name": "large-integer-request-ids", + "description": "Integer request ids outside the signed and unsigned 64-bit ranges are accepted and echoed exactly.", + "scope": "protocol", + "steps": [ + { + "request": {"id": 18446744073709551616, "method": "lpp/shutdown", "params": {}}, + "expectResponse": {"error": {"code": -32000, "message": {"$provider": "nonEmptyString"}, "data": {"lpp": {"kind": "invalidRequest", "details": {"reason": "notInitialized"}}}}} + }, + { + "request": {"id": -9223372036854775809, "method": "lpp/shutdown", "params": {}}, + "expectResponse": {"error": {"code": -32000, "message": {"$provider": "nonEmptyString"}, "data": {"lpp": {"kind": "invalidRequest", "details": {"reason": "notInitialized"}}}}} + } + ] +} diff --git a/conformance/mock-provider/Cargo.toml b/conformance/mock-provider/Cargo.toml index 415b334..02118c8 100644 --- a/conformance/mock-provider/Cargo.toml +++ b/conformance/mock-provider/Cargo.toml @@ -11,7 +11,7 @@ publish = false lpp-conformance-common = { path = "../common" } sha2 = "0.10" serde = { workspace = true, features = ["derive"] } -serde_json.workspace = true +serde_json = { workspace = true, features = ["arbitrary_precision"] } [lints] workspace = true diff --git a/conformance/mock-provider/src/server.rs b/conformance/mock-provider/src/server.rs index 8b7d8c9..7874623 100644 --- a/conformance/mock-provider/src/server.rs +++ b/conformance/mock-provider/src/server.rs @@ -44,6 +44,12 @@ fn invalid_request() -> Value { std_error(Value::Null, -32600, "Invalid Request") } +fn is_integer_number(number: &serde_json::Number) -> bool { + let number = number.to_string(); + let digits = number.strip_prefix('-').unwrap_or(&number); + !digits.is_empty() && digits.bytes().all(|byte| byte.is_ascii_digit()) +} + /// `invalidRequest`/`notInitialized` for any method called pre-`initialize`. fn not_initialized(id: &Value) -> Value { lpp_error( @@ -126,7 +132,7 @@ impl Server { }; let id = match object.get("id") { Some(id @ Value::String(_)) => id.clone(), - Some(Value::Number(number)) if number.is_i64() || number.is_u64() => { + Some(Value::Number(number)) if is_integer_number(number) => { Value::Number(number.clone()) } // No notifications in LPP v1: a missing or null id is a protocol @@ -246,6 +252,22 @@ mod tests { } } + #[test] + fn integer_request_ids_larger_than_u64_are_preserved() { + let mut server = server(); + for id in ["18446744073709551616", "-9223372036854775809"] { + let request = + format!(r#"{{"jsonrpc":"2.0","id":{id},"method":"lpp/shutdown","params":{{}}}}"#); + let response = server.handle_message(&request); + + assert_eq!( + response["error"]["data"]["lpp"]["details"]["reason"], + "notInitialized" + ); + assert_eq!(response["id"].as_number().unwrap().to_string(), id); + } + } + #[test] fn missing_and_null_ids_keep_the_notification_error() { let mut server = server(); diff --git a/conformance/runner/Cargo.toml b/conformance/runner/Cargo.toml index 2c7fc4c..eee0574 100644 --- a/conformance/runner/Cargo.toml +++ b/conformance/runner/Cargo.toml @@ -10,7 +10,7 @@ publish = false [dependencies] lpp-conformance-common = { path = "../common" } serde = { workspace = true, features = ["derive"] } -serde_json.workspace = true +serde_json = { workspace = true, features = ["arbitrary_precision"] } [lints] workspace = true From 67dac9cc44ecc8492ba14e979167cd34c43c0724 Mon Sep 17 00:00:00 2001 From: Danny Ward Date: Thu, 15 Oct 2026 16:47:01 -0400 Subject: [PATCH 3/3] fix(protocol): prioritize lifecycle errors --- .../v1/03-request-before-initialize.json | 8 +++-- .../fixtures/v1/04-initialize-twice.json | 4 ++- conformance/mock-provider/src/server.rs | 30 ++++++++++--------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/conformance/fixtures/v1/03-request-before-initialize.json b/conformance/fixtures/v1/03-request-before-initialize.json index a7ada81..bedf36f 100644 --- a/conformance/fixtures/v1/03-request-before-initialize.json +++ b/conformance/fixtures/v1/03-request-before-initialize.json @@ -1,10 +1,12 @@ { "name": "request-before-initialize", - "description": "No method except lpp/initialize may be processed before the handshake.", + "description": "Lifecycle errors take precedence over params validation before the handshake.", "scope": "protocol", "steps": [ {"request":{"id":1,"method":"lpp/check","params":{"documents":{}}},"expectResponse":{"error":{"code":-32000,"message":{"$provider":"nonEmptyString"},"data":{"lpp":{"kind":"invalidRequest","details":{"reason":"notInitialized"}}}}}}, - {"request":{"id":2,"method":"lpp/initialize","params":{"protocolVersion":"1.0"}},"expectResponse":{"result":{"protocolVersion":"1.0","serverInfo":{"name":{"$provider":"nonEmptyString"},"version":{"$provider":"nonEmptyString"}},"languages":{"$provider":"languageList"},"capabilities":{"check":{"$provider":"boolean"},"compile":{"$provider":"boolean"},"reconstruct":{"$provider":"boolean"},"symbols":{"$provider":"boolean"},"definition":{"$provider":"boolean"},"references":{"$provider":"boolean"},"rename":{"$provider":"boolean"},"editValidation":{"$provider":"boolean"}}}}}, - {"request":{"id":3,"method":"lpp/shutdown","params":{}},"expectResponse":{"result":null}} + {"request":{"id":2,"method":"lpp/check"},"expectResponse":{"error":{"code":-32000,"message":{"$provider":"nonEmptyString"},"data":{"lpp":{"kind":"invalidRequest","details":{"reason":"notInitialized"}}}}}}, + {"request":{"id":3,"method":"lpp/check","params":[]},"expectResponse":{"error":{"code":-32000,"message":{"$provider":"nonEmptyString"},"data":{"lpp":{"kind":"invalidRequest","details":{"reason":"notInitialized"}}}}}}, + {"request":{"id":4,"method":"lpp/initialize","params":{"protocolVersion":"1.0"}},"expectResponse":{"result":{"protocolVersion":"1.0","serverInfo":{"name":{"$provider":"nonEmptyString"},"version":{"$provider":"nonEmptyString"}},"languages":{"$provider":"languageList"},"capabilities":{"check":{"$provider":"boolean"},"compile":{"$provider":"boolean"},"reconstruct":{"$provider":"boolean"},"symbols":{"$provider":"boolean"},"definition":{"$provider":"boolean"},"references":{"$provider":"boolean"},"rename":{"$provider":"boolean"},"editValidation":{"$provider":"boolean"}}}}}, + {"request":{"id":5,"method":"lpp/shutdown","params":{}},"expectResponse":{"result":null}} ] } diff --git a/conformance/fixtures/v1/04-initialize-twice.json b/conformance/fixtures/v1/04-initialize-twice.json index 8ce53c2..1ee364a 100644 --- a/conformance/fixtures/v1/04-initialize-twice.json +++ b/conformance/fixtures/v1/04-initialize-twice.json @@ -5,6 +5,8 @@ "steps": [ {"request":{"id":1,"method":"lpp/initialize","params":{"protocolVersion":"1.0"}},"expectResponse":{"result":{"protocolVersion":"1.0","serverInfo":{"name":{"$provider":"nonEmptyString"},"version":{"$provider":"nonEmptyString"}},"languages":{"$provider":"languageList"},"capabilities":{"check":{"$provider":"boolean"},"compile":{"$provider":"boolean"},"reconstruct":{"$provider":"boolean"},"symbols":{"$provider":"boolean"},"definition":{"$provider":"boolean"},"references":{"$provider":"boolean"},"rename":{"$provider":"boolean"},"editValidation":{"$provider":"boolean"}}}}}, {"request":{"id":2,"method":"lpp/initialize","params":{"protocolVersion":"1.0"}},"expectResponse":{"error":{"code":-32000,"message":{"$provider":"nonEmptyString"},"data":{"lpp":{"kind":"invalidRequest","details":{"reason":"alreadyInitialized"}}}}}}, - {"request":{"id":3,"method":"lpp/shutdown","params":{}},"expectResponse":{"result":null}} + {"request":{"id":3,"method":"lpp/initialize"},"expectResponse":{"error":{"code":-32000,"message":{"$provider":"nonEmptyString"},"data":{"lpp":{"kind":"invalidRequest","details":{"reason":"alreadyInitialized"}}}}}}, + {"request":{"id":4,"method":"lpp/initialize","params":[]},"expectResponse":{"error":{"code":-32000,"message":{"$provider":"nonEmptyString"},"data":{"lpp":{"kind":"invalidRequest","details":{"reason":"alreadyInitialized"}}}}}}, + {"request":{"id":5,"method":"lpp/shutdown","params":{}},"expectResponse":{"result":null}} ] } diff --git a/conformance/mock-provider/src/server.rs b/conformance/mock-provider/src/server.rs index 7874623..8314a5d 100644 --- a/conformance/mock-provider/src/server.rs +++ b/conformance/mock-provider/src/server.rs @@ -60,6 +60,15 @@ fn not_initialized(id: &Value) -> Value { ) } +fn already_initialized(id: &Value) -> Value { + lpp_error( + id.clone(), + "invalidRequest", + json!({ "reason": "alreadyInitialized" }), + "invalid request: already initialized", + ) +} + /// Mark `name` disabled for `--without`. Returns false for unknown names. pub(crate) fn disable_capability(disabled: &mut HashSet<&'static str>, name: &str) -> bool { match CAPABILITIES.iter().find(|(n, _)| *n == name) { @@ -147,6 +156,13 @@ impl Server { } Some(_) => return invalid_request(), }; + if method == "lpp/initialize" { + if self.initialized { + return already_initialized(&id); + } + } else if !self.initialized { + return not_initialized(&id); + } let params = match object.get("params") { Some(params) if params.is_object() => params.clone(), _ => return std_error(id, -32602, "Invalid params"), @@ -159,14 +175,6 @@ impl Server { } fn initialize(&mut self, id: &Value, params: Value) -> Value { - if self.initialized { - return lpp_error( - id.clone(), - "invalidRequest", - json!({ "reason": "alreadyInitialized" }), - "invalid request: already initialized", - ); - } let params: InitParams = match parse_params(params) { Ok(params) => params, Err(_) => return std_error(id.clone(), -32602, "Invalid params"), @@ -200,17 +208,11 @@ impl Server { } fn shutdown(&mut self, id: &Value) -> Value { - if !self.initialized { - return not_initialized(id); - } self.exiting = true; ok(id.clone(), Value::Null) } fn dispatch(&mut self, id: &Value, method: &str, params: Value) -> Value { - if !self.initialized { - return not_initialized(id); - } let Some(entry) = METHODS.iter().find(|m| m.name == method) else { return std_error(id.clone(), -32601, "Method not found"); };