[bilboquet]

Fixes #1653. Depends on #1661.

Reject a present ID in the server notification decoder, so unsupported IDs receive Invalid Request in single requests and mixed HTTP/WebSocket batches. Preserve supported-ID calls, notification parameters and extensions.

Validation: offline release core/server rebuild; 44 passing Rust tests and a named failing baseline regression; rustfmt and targeted Clippy.

[lexnv]

nit: We could parse the same without the need of the hardcoded {, since we know it to be a valid JSON from the above check.

Suggested change

	let id = if value.get().starts_with('{') {
	serde_json::from_slice::<InvalidRequest>(data).map(|request| request.id).unwrap_or(Id::Null)
	} else {
	Id::Null
	};
	let id = serde_json::from_slice::<InvalidRequest>(data)
	.map(|request| request.id)
	.unwrap_or(Id::Null);

[bilboquet]

Thanks for the feedback!

There’s a subtle Serde behavior here: derived struct deserialization also accepts positional arrays. Without the object check, [1] deserializes into InvalidRequest { id: Id::Number(1) }, although there is no id member.

The existing error_classification_unrecoverable_ids_and_nonobjects test covers this. That’s why the object check is needed here. Your second comment suggests another approach, which I’ll address separately.

[lexnv]

We could improve a bit the parsing error function, we'd probably have 2 options:

• the suggested comment to remove the need of hardcoding open brackets, but still has the downside of double parsing
• an optimized way which borrows the ID and maps on the json-rpc spec:

pub fn prepare_error(data: &[u8]) -> (Id<'_>, ErrorCode) {
  enum JsonRpcProbe<'a> {
      HasId { #[serde(borrow) id: Id<'a> },
      ValidJson(&'a Rawvalue),
 }
 
  match serde_json::from_slice::<JsonRpcProbe>(data) {

}

Would probably lean towards the second option, wdyt? 🙏

[bilboquet]

We could improve a bit the parsing error function, we'd probably have 2 options:

* the suggested comment to remove the need of hardcoding open brackets, but still has the downside of double parsing

* an optimized way which borrows the ID and maps on the json-rpc spec:

pub fn prepare_error(data: &[u8]) -> (Id<'_>, ErrorCode) {
  enum JsonRpcProbe<'a> {
      HasId { #[serde(borrow) id: Id<'a> },
      ValidJson(&'a Rawvalue),
 }
 
  match serde_json::from_slice::<JsonRpcProbe>(data) {

}

Would probably lean towards the second option, wdyt? 🙏

Thanks for suggesting this alternative! If I understand correctly, there are two possible ways to implement it:

• With an untagged derive, the struct variant would indeed reject arrays, addressing the object-check concern. However, Serde buffers the input into intermediate content before trying the variants, so the borrowed &RawValue fallback would fail for otherwise valid JSON such as {}.

• A custom deserializer could avoid that buffering, but preserving the current behavior would require more code than the original implementation, particularly to consume invalid IDs without losing the distinction between malformed JSON and invalid requests.

Were you thinking of a custom visitor, or is there a simpler implementation I’m missing?

[lexnv]

Had another review round, LGTM!