Method parameter conversion error handling logic should preserve JSON-RPC id from the request

[cyb3r4nt]

During upgrade to newer 1.6.0 version an error related to invalid handling of JSON-RPC id was discovered.

Suppose you have a server with following interface:

public interface ServiceInterfaceWithParamNameAnnotation {
	String methodWithDifferentTypes(
		@JsonRpcParam("param1") Boolean booleanParam1,
		@JsonRpcParam("param2") Double doubleParam2,
		@JsonRpcParam("param3") UUID doubleParam3
	);
}

When a valid json, but with invalid parameters, is sent to server:

{"method":"methodWithDifferentTypes","id":4,"jsonrpc":"2.0","params":[true,3.14,"iWantToBeAnUUID"]} 

Then server responds with

 {"jsonrpc":"2.0","id":"null","error":{"code":-32700,"message":"JSON parse error"}} 

, where initial id value is lost.

This is also true for the batch requests, where multiple requests are sent at once.
It should be possible to find a failed request by id from the response.

The "JSON parse error" -32700 is not really correct here because:

• input is a valid JSON-RPC object
• method exists
• parameter count is correct

Valid error probably should be the "Invalid params" -32602 , because wrong parameter was sent.

More detailed log (code line numbers may be wrong):

2022-06-11 01:33:06,940 DEBUG c.g.j.JsonRpcBasicServer:109 [Test worker] created server for interface interface com.googlecode.jsonrpc4j.server.JsonRpcServerAnnotatedParamTest$ServiceInterfaceWithParamNameAnnotation with handler class com.sun.proxy.$Proxy13 
2022-06-11 01:33:07,025 DEBUG c.g.j.JsonRpcBasicServer:420 [Test worker] Request: {"method":"methodWithDifferentTypes","id":4,"jsonrpc":"2.0","params":[true,3.14,"iWantToBeAnUUID"]} 
2022-06-11 01:33:07,075 DEBUG c.g.j.JsonRpcBasicServer:585 [Test worker] Invoking method: methodWithDifferentTypes with args [true, 3.14, "iWantToBeAnUUID"] 
2022-06-11 01:33:07,097 DEBUG c.g.j.JsonRpcBasicServer:1011 [Test worker] Response: {"jsonrpc":"2.0","id":"null","error":{"code":-32700,"message":"JSON parse error"}} 

Variant with batching:

2022-06-11 02:03:41,279 DEBUG c.g.j.JsonRpcBasicServer:109 [Test worker] created server for interface interface com.googlecode.jsonrpc4j.server.JsonRpcServerTest$ServiceInterface with handler class com.sun.proxy.$Proxy13 
2022-06-11 02:03:41,294 DEBUG c.g.j.JsonRpcBasicServer:109 [Test worker] created server for interface interface com.googlecode.jsonrpc4j.server.JsonRpcServerAnnotatedParamTest$ServiceInterfaceWithParamNameAnnotation with handler class com.sun.proxy.$Proxy28 
2022-06-11 02:03:41,456 DEBUG c.g.j.JsonRpcServer:115 [Test worker] Handling HttpServletRequest org.springframework.mock.web.MockHttpServletRequest@1578104e 
2022-06-11 02:03:41,500 DEBUG c.g.j.JsonRpcBasicServer:303 [Test worker] Handling 3 requests 
2022-06-11 02:03:41,501 DEBUG c.g.j.JsonRpcBasicServer:420 [Test worker] Request: {"method":"methodWithDifferentTypes","id":1,"jsonrpc":"2.0","params":[true,3.14,"cbddeb26-ea0c-48f9-adeb-5b28158fc71c"]} 
2022-06-11 02:03:41,523 DEBUG c.g.j.JsonRpcBasicServer:585 [Test worker] Invoking method: methodWithDifferentTypes with args [true, 3.14, "cbddeb26-ea0c-48f9-adeb-5b28158fc71c"] 
2022-06-11 02:03:41,550 DEBUG c.g.j.JsonRpcBasicServer:602 [Test worker] Invoked method: methodWithDifferentTypes, result passResult1 
2022-06-11 02:03:41,557 DEBUG c.g.j.JsonRpcBasicServer:420 [Test worker] Request: {"method":"methodWithDifferentTypes","id":2,"jsonrpc":"2.0","params":["truth",3.14,"e43f3ef3-88c4-4ded-9be0-8efaecd3c1dc"]} 
2022-06-11 02:03:41,557 DEBUG c.g.j.JsonRpcBasicServer:585 [Test worker] Invoking method: methodWithDifferentTypes with args ["truth", 3.14, "e43f3ef3-88c4-4ded-9be0-8efaecd3c1dc"] 
2022-06-11 02:03:41,559 DEBUG c.g.j.JsonRpcBasicServer:420 [Test worker] Request: {"method":"methodWithDifferentTypes","id":3,"jsonrpc":"2.0","params":[true,3.14,"cbddeb26-ea0c-48f9-adeb-5b28158fc71c"]} 
2022-06-11 02:03:41,559 DEBUG c.g.j.JsonRpcBasicServer:585 [Test worker] Invoking method: methodWithDifferentTypes with args [true, 3.14, "cbddeb26-ea0c-48f9-adeb-5b28158fc71c"] 
2022-06-11 02:03:41,560 DEBUG c.g.j.JsonRpcBasicServer:602 [Test worker] Invoked method: methodWithDifferentTypes, result passResult3 
2022-06-11 02:03:41,561 DEBUG c.g.j.JsonRpcBasicServer:339 [Test worker] served 3 requests, error 1, result JsonError{code=-32002, message='bulk error', data=null} 
2022-06-11 02:03:41,561 DEBUG c.g.j.JsonRpcBasicServer:1011 [Test worker] Response: [{"jsonrpc":"2.0","id":1,"result":"passResult1"},{"jsonrpc":"2.0","id":"null","error":{"code":-32700,"message":"JSON parse error"}},{"jsonrpc":"2.0","id":3,"result":"passResult3"}] 

The behavior was correct in previous 1.5.3 version, but in 1.6.0 server started to output null values for id in responses.
This is caused by the recent change in the error handling logic:
https://github.com/briandilley/jsonrpc4j/blob/1.6.0/src/main/java/com/googlecode/jsonrpc4j/JsonRpcBasicServer.java#L466

[shubhamChouksey1]

What is the best way to add validations for the API input parameters ?
Also how to handle the case when the API input parameter is an interface, and several different implementation class are valid input parameters for the API ?

[cyb3r4nt]

What is the best way to add validations for the API input parameters ?

In my opinion, this framework should not deal with validation of users' business objects.
It may only provide provide deserialization for required types, but validation should be done somewhere else in the users' code.
Also, in my opinion, deserialization and validation should be two different steps: successful receival of the object and validation with proper logging inside the application.

The best option is to manually check the object just after framework has returned control into users' code.
The second option is to use request interceptors, like this one: https://github.com/briandilley/jsonrpc4j/blob/master/src/main/java/com/googlecode/jsonrpc4j/RequestInterceptor.java
There is also possibility to use javax.validation, but this is not quite supported yet.
There is also one open PR which tries to do this, #285.
It is also not quite clear who and where needs to call the javax.validation: jsonrpc4j or Spring framework or users' code.

Current issue is not quite related to the object validation, an it descibes a bug about the JSON-RPC id field and error handling.

Also how to handle the case when the API input parameter is an interface, and several different implementation class are valid input parameters for the API ?

How the framework will know which concrete implementation to inject into users' method?
Is there any good possibility how to express this using JSON objects?
What are you trying to achieve when interface is declared in the method parameters?

When remote methods are called via RPC, then data is exchanged between remote processes.
And framework gives the ability to map that data into some objects.
This data is only some messages between two processes.
It is better if remote logic does not know anything about your interfaces and various implementations.
API contract usually requires some well known parameters, like primitive types and collections, and their composition.

If you really need this, then there are some interceptors provided by this framework, which may transform objects.
Or even better approach to receive the message, as data object, and transform it to required interface and implementation directly in the users' code.