JSON-RPC 1.0 compliance bugs

[Daveycoder]

I've been getting several JSON errors on my server and have tracked them down to the client sending an empty message back; i.e.

{
  "jsonrpc": "2.0",
  "id": null,
  "result": null
}

This seems to be down to line 73 in the RequestId class which will always return a false; the this.IsNull in the RequestId(string? id) ctor will always return true if it is null (which is correct), but the IsEmpty method (line 73) breaks this. Changing it from

public bool IsEmpty => this.Number is null && this.String is null && !this.IsNull;

to

public bool IsEmpty => (this.Number is null && this.String is null) || this.IsNull;

seems to fix the issue (I'm not sure of the relevance of the other checks - IMHO the RequestId either IsNull or not, but I'm very likely to be missing something as it's late at night and I've spent all day trying to work out why I was getting these messages!)

[AArnott]

the client sending an empty message back

The client is sending an empty message back? Do you mean the server is? The sample message you shared is a response message, so the RPC server would presumably be sending this back to the client that issued the request.

I'll assume you just got the roles mixed up and that your observation is that a StreamJsonRpc server is sending the response back with id: null. Did the client send the request with id: null as well? It sounds like if it did, you expect the server to not send a response. Is that right?

Per the JSON-RPC spec, requests (that merit responses) have an id property and notifications that do not get responses have no id property:

An identifier established by the Client that MUST contain a String, Number, or NULL value if included. If it is not included it is assumed to be a notification.

The spec has more to say about this, including discouraging use of null as a value for a request id, but it nevertheless makes it clear that it's permissible, and that that too is a request. If you don't expect a response, the request id must be absent:

A Notification is a Request object without an "id" member.

I believe then that the IsEmpty property is defined correctly. If your client doesn't want a response, it should not set id to null but should instead leave it off.

[Daveycoder]

Yeah, just re-read my initial issue and realised what I was saying wasn't all that clear, I'd completely neglected to mention that I'm talking to a JSON-RPC 1.0 service.

The issue I currently have is that my client is talking to a JSON-RPC 1.0 service; simply listening for notifications. However, as per JSON-RPC 1.0, the ID must be present, but null. This is slightly different to notifications in JSON-RPC 2.0 which state that the ID must not be present.

What I've noticed is that the Version seems to be ignored - setting it to (1. 0) to use JSON-RPC seems to cause issues/crashes when communication is between both Client and Server running StreamJsonRPC.

The issues I'm getting seem to be:

1. When sending a message (either a notifcation from the server, or a request to the server), the message can be sent but contains a json-rpc string of "2.0".
2. When the server receives a request, this causes an exception and terminates the stream.
3. When the client recieves a notification, it sends a response (this is the string issue I mentioned in the initial issue comment).

I've put a simple example my issue on GitHub here. Running both the client/server and shows the notifications being sent okay, just changing both versions to 1.0 and it'll fall over.

[AArnott]

@Daveycoder: 1.0 support certainly isn't top of our radar, so it's conceivable we have bugs in this area. Are you putting StreamJsonRpc into 1.0 mode by setting JsonMessageFormatter.ProtocolVersion? I believe that's the supported way to do it. But setting this doesn't appear to override the value in JsonRpcMessage.Version which may explain why "2.0" is still transmitted in outbound messages.

So for at least 2 of the 3 issues you list, we probably have legit bugs. I'm going to rename your issue to reflect that this is around JSON-RPC 1.0 support.

[Daveycoder]

@AArnott Thanks for the reply; yes, I'm setting the ProtocolVersion, and the title is now certainly more appropriate!

Totally appreciate that 1.0 support isn't at the top of anyone's radar at the moment - unfortunately, some of us still have to deal with legacy systems :-(

Thanks for your time. Cheers, Dave.

[dimabarbul]

We found a workaround by using custom message handler.

public class MyWebSocketMessageHandler : WebSocketMessageHandler
{
    public MyWebSocketMessageHandler(WebSocket webSocket) : base(webSocket)
    {
    }

    public MyWebSocketMessageHandler(WebSocket webSocket, IJsonRpcMessageFormatter formatter, int sizeHint = 4096) : base(webSocket, formatter, sizeHint)
    {
    }

    protected override async ValueTask<JsonRpcMessage?> ReadCoreAsync(CancellationToken cancellationToken)
    {
        JsonRpcMessage? rpcMessage = await base.ReadCoreAsync(cancellationToken);

        if (rpcMessage is JsonRpcRequest request)
        {
            if ((request.RequestId.Number is null && request.RequestId.String is null) || request.RequestId.IsNull)
            {
                request.RequestId = RequestId.NotSpecified;
            }
        }

        return rpcMessage;
    }
}