seznam / fastrpc

FastRPC library
http://seznam.github.io/frpc
GNU Lesser General Public License v2.1
46 stars 46 forks source link

ServerProxy seems to revert to protocol version 1.0 after xmlrpc response #74

Open Lameorc opened 5 years ago

Lameorc commented 5 years ago

Hi. I've recently encountered an issue where a second call with number that's serialized into <i8> on ServerProxy object fails with error:

 fastrpc.ResponseError: <fastrpc.ResponseError: Number is too big for protocol version 1.0 [method test_func() @ http://localhost:8000/]>

Provided are server and client scripts used to simulate this behavior, run using python2:

import xmlrpclib
from SimpleXMLRPCServer import SimpleXMLRPCServer

def test_func(param):
    return "param: {}".format(param)

def main():
    server = SimpleXMLRPCServer(("localhost", 8000))
    print "Listening on port 8000..."
    server.register_function(test_func, "test_func")
    server.serve_forever()

if __name__ == '__main__':
    main()
fastrpc import ServerProxy

p = ServerProxy("http://localhost:8000")

p.test_func(12)
p.test_func(223124215125125)

Seems to me like fastrpc does not only fallback from using binary when communicating with xmlrpc server but also to a lower protocol version. This is backed up by the fact that making only the call with a large number does not fail, eg. this script runs fine:

from fastrpc import ServerProxy

p = ServerProxy("http://localhost:8000")

#  p.test_func(12)
p.test_func(223124215125125

Versions as reported by dpkg:

ii  libfastrpc8                                 8.0.10                                      amd64        Fastrpc - RPC library, supports binary and XML encoding
ii  python-fastrpc                              8.0.5                                       amd64        Fastrpc -- RPC using XML and Binary protocol
volca02 commented 5 years ago

Hi,

looking at the problem, there is a default protocol version 1.0 set after each call if there is no protocolVersion in the response from server (see src/frpchttpclient.cc:235, the default version is set in src/frpcxmlunarshaller.cc:290).

This limits the usage serverproxy for second and consecutive calls because the default version (FRPC_MAJOR_VERSION_DEFAULT, FRPC_MINOR_VERSION_DEFAULT set in ProtocolVersion_t ctor) is overwritten with 1.0 default.

I'd propose a fix that would set protocolVersion to the pair FRPC_MAJOR_VERSION_DEFAULT, FRPC_MINOR_VERSION_DEFAULT if there is no protocolVersion field in the XML response.

Lameorc commented 5 years ago

If I'm understanding your proposed solution correctly, it would cause the protocol version set in ctor to be kept for consecutive requests. This seems like a sensible solution to the problem so I am all for it.

volca02 commented 5 years ago

I mean if we can't get any protocol version from the server response, it's the only reasonable course of action, since we already did a call to the server before validating the protocol version anyway.

Commenting out the else branch in frpcxmlunarshaller.cc should also work.