foglcz / JSONRpc2

Generic JSON-RPC v2 implementation
Other
19 stars 10 forks source link

Invalid Request when handling raw JSON string in Server #18

Closed RisoP closed 4 years ago

RisoP commented 5 years ago

Server throws 'Invalid Request' exception, when calling 'handle' function with parameter as raw JSON string. It seems like request_version variable is not initialized in this case.

scottchiefbaker commented 5 years ago

Can you provide a test case for this.

RisoP commented 5 years ago

$server = new \Lightbulb\Json\Rpc2\Server; $server->substract = function($subtrahend, $minuend) { return $subtrahend - $minuend; }; $response = $server->handle('{"jsonrpc": "2.0", "method": "substract", "params": [42, 23], "id": 1}');

Result in version 1.1: {"error":{"code":-32600,"message":"Invalid Request."},"id":1}

Result in current master (returns different error): {"error":{"code":0,"message":"Creating default object from empty value in file Server.php on line 237"},"id":1}

scottchiefbaker commented 5 years ago

You don't call the API with handle(). handle() is used on the server side to process the incoming API request.

require('lib/Server.php');
$server = new Lightbulb\Json\Rpc2\Server;

$server->echo_data = 'echo_data';

// Process the request
$response = $server->handle();

You want to look at the client library to make an actual request.

RisoP commented 5 years ago

Thank you for your quick reply.

Here is the whole scenario: I have C# client calling server API by HTTP POST. The request is compressed JSON. On the server side, I read raw POST data, unzip it and then pass JSON string to $server->handle function.

I guess it's not common way of calling handle, but it used to work in the past.

To fix it, I had to comment out a few lines in local copy of Server.php and remove installing library by composer.

scottchiefbaker commented 5 years ago

I think your work flow is OK, but you need to use client not server to send the raw JSON.

scottchiefbaker commented 5 years ago

Actually re-reading the code it is valid to use JSON with handle(). I went back and tried your test case and I did get an error about object initialization. I think I've fixed it though in master. Can you try the latest version and see if you still get the error?

scottchiefbaker commented 5 years ago

I dunno why it auto closed this issue... re-opening.

RisoP commented 5 years ago

I tried latest version and it's OK. Thanks. I would appreciate if you make a release with this fix.

The issue was auto closed due to keyword "fix" + issue number in commit message (https://help.github.com/en/articles/closing-issues-using-keywords).

RadoZu commented 4 years ago

@scottchiefbaker what about some release from master? It would help me. Thanks.

scottchiefbaker commented 4 years ago

@RadoZu I pushed master to v1.2.1 and did a release. Can you test this and make sure it addresses your issue and then close this?

RadoZu commented 4 years ago

@scottchiefbaker I've just test it. New release 1.2.1 works great. Could you also publish new composer package 1.2.1 ? Thanks. @RisoP you can close it.

scottchiefbaker commented 4 years ago

@RadoZu I've never used composer... someone else must have posted it? Any way to track that down?

RadoZu commented 4 years ago

@scottchiefbaker maybe it's task for @foglcz to publish new composer package because he is maintainer of it. BTW here is how to do it.