mbr / tinyrpc

A compact, modular transport and protocol agnostic RPC library. Does jsonrpc v2.
https://tinyrpc.readthedocs.org
MIT License
156 stars 53 forks source link

'NoneType' object has no attribute 'serialize' when Id is zero or unspecified #27

Closed mat013 closed 7 years ago

mat013 commented 8 years ago

Hi

I am a newbie on the python so please bare with me if the analysis is not accurately enough.

We have seen that when a json request is sent, where either the id is omitted or alternatively the id is set to the number 0 the following error is experienced:

Traceback (most recent call last): File "src\test\python\plugins\TestPlugin.py", line 80, in rpc_server.serve_forever() File "C:\Python27\lib\site-packages\tinyrpc\serverinit.py", line 56, in serve_forever self._spawn(handle_message, context, message) File "C:\Python27\lib\site-packages\tinyrpc\serverinit.py", line 71, in _spawn func(_args, *_kwargs) File "C:\Python27\lib\site-packages\tinyrpc\serverinit.py", line 54, in handle_message self.transport.send_reply(context, response.serialize()) AttributeError: 'NoneType' object has no attribute 'serialize'

The reason for this is is that the RPCServer in tinyrpc/server/init.py (line 57) has logic that assumes that it has a serialize method:

        # send reply
        self.transport.send_reply(context, response.serialize())

The reason why the serialize is not available is due to the JSONRpcRequest in the file tinyrpc/protocols/jsonrpc.py(lines 110-111 and lines 125-126):

    if not self.unique_id:
        return None

I was wondering wouldn't it be more appropriate either to raise an exception, set an arbitrary id or alternatively return an object, which has serialize on it.

br

Meang

mat013 commented 8 years ago

I would happy to make a fix, if someone can decide what the appropriate behavior should be

corbinbs commented 8 years ago

Hi - I also ran into this issue when attempting to use https://github.com/ConsenSys/ethjsonrpc to make requests to https://github.com/ethereum/pyethapp When id is 0 (the default in ethjsonrpc https://github.com/ConsenSys/ethjsonrpc/blob/master/ethjsonrpc/client.py#L29 ), the request to pyethapp actually hangs due to returning None instead of the response.
https://github.com/mbr/tinyrpc/blob/master/tinyrpc/protocols/jsonrpc.py#L125 I've not been able to find anything in http://www.jsonrpc.org/specification#request_object that indicates an id cannot be 0. I'm not sure about other cases when None might need to be returned but it seems like the code should at least allow for an id of 0.

Thanks,

Brian

lnoor commented 8 years ago

I also ran into this. The integer value 0 should definitely be allowed. As far as I can tell so should the empty string for that matter which exhibits the same behaviour.

The error is triggered by returning None in protocols/jsonrpc.py:respond line 124 which return value is used in server/__init__.py:handle_message line 54 where the response.serialize() is called without checking if response is an object with a serialize() method.

So either in respond() an exception should be raised or in handle_message() the type of response should be checked.

Is this supposed to handle json-rpc notifications? If not I think that the entire test in respond() can be dropped.

The same issue exists for the method error_respond() in jsonrpc.py