Currently, RPC requests using rpc_cli.py will crash on Python 3, because the decoded string through msgpack-rpc is not str type when the default encoding is not specified into msgpack.Unpacker. On Python 2, bytes type is the same as str type, and this problem does not occur.
The old spec of msgpack had no notation of the encoding, but now, msgpack defines "UTF-8" as the default encoding and has the explicit type definitions for String and Binary. https://github.com/msgpack/msgpack/blob/master/spec.md This patch fixes to specify the encoding to msgpack.Packer/Unpacker and enable to use Binary type when packing for the Python 3 compatibility. Signed-off-by: IWASE Yusuke <[email protected]> --- ryu/lib/rpc.py | 20 ++--- ryu/services/protocols/bgp/net_ctrl.py | 8 +- ryu/tests/unit/lib/test_rpc.py | 148 ++++++++++++++------------------- 3 files changed, 77 insertions(+), 99 deletions(-) diff --git a/ryu/lib/rpc.py b/ryu/lib/rpc.py index ed38f97..bb4742a 100644 --- a/ryu/lib/rpc.py +++ b/ryu/lib/rpc.py @@ -14,8 +14,11 @@ # See the License for the specific language governing permissions and # limitations under the License. -# msgpack-rpc -# http://wiki.msgpack.org/display/MSGPACK/RPC+specification +# Specification: +# - msgpack +# https://github.com/msgpack/msgpack/blob/master/spec.md +# - msgpack-rpc +# https://github.com/msgpack-rpc/msgpack-rpc/blob/master/spec.md from collections import deque import select @@ -36,13 +39,8 @@ class MessageEncoder(object): """ def __init__(self): super(MessageEncoder, self).__init__() - # note: on-wire msgpack has no notion of encoding. - # the msgpack-python library implicitly converts unicode to - # utf-8 encoded bytes by default. we don't want to rely on - # the behaviour though because it seems to be going to change. - # cf. https://gist.github.com/methane/5022403 - self._packer = msgpack.Packer(encoding=None) - self._unpacker = msgpack.Unpacker(encoding=None) + self._packer = msgpack.Packer(encoding='utf-8', use_bin_type=True) + self._unpacker = msgpack.Unpacker(encoding='utf-8') self._next_msgid = 0 def _create_msgid(self): @@ -51,7 +49,7 @@ class MessageEncoder(object): return this_id def create_request(self, method, params): - assert isinstance(method, six.binary_type) + assert isinstance(method, (str, six.binary_type)) assert isinstance(params, list) msgid = self._create_msgid() return (self._packer.pack( @@ -64,7 +62,7 @@ class MessageEncoder(object): return self._packer.pack([MessageType.RESPONSE, msgid, error, result]) def create_notification(self, method, params): - assert isinstance(method, six.binary_type) + assert isinstance(method, (str, six.binary_type)) assert isinstance(params, list) return self._packer.pack([MessageType.NOTIFY, method, params]) diff --git a/ryu/services/protocols/bgp/net_ctrl.py b/ryu/services/protocols/bgp/net_ctrl.py index 7cb1046..dcd9870 100644 --- a/ryu/services/protocols/bgp/net_ctrl.py +++ b/ryu/services/protocols/bgp/net_ctrl.py @@ -23,6 +23,8 @@ import logging import socket import traceback +import msgpack + from ryu.services.protocols.bgp import api from ryu.services.protocols.bgp.api.base import ApiException from ryu.services.protocols.bgp.api.base import NEXT_HOP @@ -95,10 +97,8 @@ class RpcSession(Activity): def __init__(self, socket, outgoing_msg_sink_iter): super(RpcSession, self).__init__("RpcSession(%s)" % socket) - import msgpack - - self._packer = msgpack.Packer() - self._unpacker = msgpack.Unpacker() + self._packer = msgpack.Packer(encoding='utf-8') + self._unpacker = msgpack.Unpacker(encoding='utf-8') self._next_msgid = 0 self._socket = socket self._outgoing_msg_sink_iter = outgoing_msg_sink_iter diff --git a/ryu/tests/unit/lib/test_rpc.py b/ryu/tests/unit/lib/test_rpc.py index b0adf20..2df123e 100644 --- a/ryu/tests/unit/lib/test_rpc.py +++ b/ryu/tests/unit/lib/test_rpc.py @@ -15,17 +15,14 @@ # limitations under the License. import numbers -import time -import sys -if sys.version_info < (2, 7): - import unittest2 as unittest -else: - import unittest +import socket +import struct +import unittest + from nose.tools import raises import six from ryu.lib import hub -hub.patch() from ryu.lib import rpc @@ -40,22 +37,19 @@ class Test_rpc(unittest.TestCase): def _handle_request(self, m): e = self._server_endpoint msgid, method, params = m - if method == b'resp': + if method == 'resp': e.send_response(msgid, result=params[0]) - elif method == b'err': + elif method == 'err': e.send_response(msgid, error=params[0]) - elif method == b'callback': + elif method == 'callback': n, cb, v = params assert n > 0 self._requests.add(e.send_request(cb, [msgid, n, cb, v])) - elif method == b'notify1': + elif method == 'notify1': e.send_notification(params[1], params[2]) e.send_response(msgid, result=params[0]) - elif method == b'shutdown': - import socket - # Though six.text_type is not needed in python2, it is - # unconditionally applied for code simplicityp - how = getattr(socket, six.text_type(params[0], 'utf-8')) + elif method == 'shutdown': + how = getattr(socket, params[0]) self._server_sock.shutdown(how) e.send_response(msgid, result=method) else: @@ -64,7 +58,7 @@ class Test_rpc(unittest.TestCase): def _handle_notification(self, m): e = self._server_endpoint method, params = m - if method == b'notify2': + if method == 'notify2': e.send_notification(params[0], params[1]) def _handle_response(self, m): @@ -80,8 +74,6 @@ class Test_rpc(unittest.TestCase): self._requests.add(e.send_request(cb, [omsgid, n, cb, v])) def setUp(self): - import socket - self._server_sock, self._client_sock = socket.socketpair() table = { rpc.MessageType.REQUEST: self._handle_request, @@ -100,24 +92,24 @@ class Test_rpc(unittest.TestCase): def test_0_call_str(self): c = rpc.Client(self._client_sock) - obj = b'hoge' - result = c.call(b'resp', [obj]) + obj = 'hoge' + result = c.call('resp', [obj]) assert result == obj - assert isinstance(result, bytes) + assert isinstance(result, str) def test_0_call_int(self): c = rpc.Client(self._client_sock) obj = 12345 assert isinstance(obj, int) - result = c.call(b'resp', [obj]) + result = c.call('resp', [obj]) assert result == obj - assert isinstance(result, type(obj)) + assert isinstance(result, numbers.Integral) def test_0_call_int2(self): c = rpc.Client(self._client_sock) obj = six.MAXSIZE assert isinstance(obj, int) - result = c.call(b'resp', [obj]) + result = c.call('resp', [obj]) assert result == obj assert isinstance(result, numbers.Integral) @@ -125,7 +117,7 @@ class Test_rpc(unittest.TestCase): c = rpc.Client(self._client_sock) obj = - six.MAXSIZE - 1 assert isinstance(obj, int) - result = c.call(b'resp', [obj]) + result = c.call('resp', [obj]) assert result == obj assert isinstance(result, numbers.Integral) @@ -133,120 +125,108 @@ class Test_rpc(unittest.TestCase): c = rpc.Client(self._client_sock) obj = 0xffffffffffffffff # max value for msgpack assert isinstance(obj, numbers.Integral) - result = c.call(b'resp', [obj]) + result = c.call('resp', [obj]) assert result == obj assert isinstance(result, numbers.Integral) def test_0_call_long2(self): c = rpc.Client(self._client_sock) - # NOTE: the python type of this value is int for 64-bit arch + # Note: the python type of this value is int for 64-bit arch obj = -0x8000000000000000 # min value for msgpack assert isinstance(obj, numbers.Integral) - result = c.call(b'resp', [obj]) + result = c.call('resp', [obj]) assert result == obj - assert isinstance(result, type(obj)) + assert isinstance(result, numbers.Integral) @raises(TypeError) def test_0_call_bytearray(self): c = rpc.Client(self._client_sock) obj = bytearray(b'foo') - result = c.call(b'resp', [obj]) + result = c.call('resp', [obj]) assert result == obj - assert isinstance(result, bytes) + assert isinstance(result, str) def test_1_shutdown_wr(self): # test if the server shutdown on disconnect - import socket self._client_sock.shutdown(socket.SHUT_WR) hub.joinall([self._server_thread]) @raises(EOFError) def test_1_client_shutdown_wr(self): c = rpc.Client(self._client_sock) - c.call(b'shutdown', [b'SHUT_WR']) + c.call('shutdown', ['SHUT_WR']) def test_1_call_True(self): c = rpc.Client(self._client_sock) obj = True - assert c.call(b'resp', [obj]) == obj + assert c.call('resp', [obj]) == obj def test_2_call_None(self): c = rpc.Client(self._client_sock) obj = None - assert c.call(b'resp', [obj]) is None + assert c.call('resp', [obj]) is None def test_2_call_False(self): c = rpc.Client(self._client_sock) obj = False - assert c.call(b'resp', [obj]) == obj + assert c.call('resp', [obj]) == obj def test_2_call_dict(self): c = rpc.Client(self._client_sock) - obj = {b'hoge': 1, b'fuga': 2} - assert c.call(b'resp', [obj]) == obj + obj = {'hoge': 1, 'fuga': 2} + assert c.call('resp', [obj]) == obj def test_2_call_empty_dict(self): c = rpc.Client(self._client_sock) obj = {} - assert c.call(b'resp', [obj]) == obj + assert c.call('resp', [obj]) == obj def test_2_call_array(self): c = rpc.Client(self._client_sock) obj = [1, 2, 3, 4] - assert c.call(b'resp', [obj]) == obj + assert c.call('resp', [obj]) == obj def test_2_call_empty_array(self): c = rpc.Client(self._client_sock) obj = [] - assert c.call(b'resp', [obj]) == obj + assert c.call('resp', [obj]) == obj def test_2_call_tuple(self): c = rpc.Client(self._client_sock) - # note: msgpack library implicitly convert a tuple into a list + # Note: msgpack library implicitly convert a tuple into a list obj = (1, 2, 3) - assert c.call(b'resp', [obj]) == list(obj) + assert c.call('resp', [obj]) == list(obj) - @raises(TypeError) def test_2_call_unicode(self): c = rpc.Client(self._client_sock) - # note: on-wire msgpack has no notion of encoding. - # the msgpack library implicitly converts unicode to - # utf-8 encoded bytes by default. - # we don't want to rely on the behaviour though because - # it seems to be going to change. - # https://gist.github.com/methane/5022403 + # Note: We use encoding='utf-8' option in msgpack.Packer/Unpacker + # in order to support Python 3. + # With this option, utf-8 encoded bytes will be decoded into unicode + # type in Python 2 and str type in Python 3. obj = u"hoge" - result = c.call(b'resp', [obj]) + result = c.call('resp', [obj]) assert result == obj - assert isinstance(result, bytes) + assert isinstance(result, six.text_type) def test_2_call_small_binary(self): - import struct c = rpc.Client(self._client_sock) obj = struct.pack("100x") - result = c.call(b'resp', [obj]) + result = c.call('resp', [obj]) assert result == obj - assert isinstance(result, bytes) + assert isinstance(result, six.binary_type) def test_3_call_complex(self): c = rpc.Client(self._client_sock) - obj = [1, b'hoge', {b'foo': 1, 3: b'bar'}] - assert c.call(b'resp', [obj]) == list(obj) + obj = [1, 'hoge', {'foo': 1, 3: 'bar'}] + assert c.call('resp', [obj]) == obj @unittest.skip("doesn't work with eventlet 0.18 and later") def test_4_call_large_binary(self): - import struct - import sys - # note: on PyPy, this test case may hang up. - sv = getattr(sys, 'subversion', None) - if sv is not None and sv[0] == 'PyPy': - return - c = rpc.Client(self._client_sock) obj = struct.pack("10000000x") - result = c.call(b'resp', [obj]) + result = c.call('resp', [obj]) assert result == obj - assert isinstance(result, bytes) + assert isinstance(result, six.binary_type) def test_0_notification1(self): l = [] @@ -254,15 +234,15 @@ class Test_rpc(unittest.TestCase): def callback(n): l.append(n) c = rpc.Client(self._client_sock, notification_callback=callback) - obj = b'hogehoge' - robj = b'fugafuga' - assert c.call(b'notify1', [robj, b'notify_hoge', [obj]]) == robj + obj = 'hogehoge' + robj = 'fugafuga' + assert c.call('notify1', [robj, 'notify_hoge', [obj]]) == robj c.receive_notification() assert len(l) == 1 n = l.pop(0) assert n is not None method, params = n - assert method == b'notify_hoge' + assert method == 'notify_hoge' assert params[0] == obj def test_0_notification2(self): @@ -271,21 +251,21 @@ class Test_rpc(unittest.TestCase): def callback(n): l.append(n) c = rpc.Client(self._client_sock, notification_callback=callback) - obj = b'hogehogehoge' - c.send_notification(b'notify2', [b'notify_hoge', [obj]]) + obj = 'hogehogehoge' + c.send_notification('notify2', ['notify_hoge', [obj]]) c.receive_notification() assert len(l) == 1 n = l.pop(0) assert n is not None method, params = n - assert method == b'notify_hoge' + assert method == 'notify_hoge' assert params[0] == obj def test_0_call_error(self): c = rpc.Client(self._client_sock) - obj = b'hoge' + obj = 'hoge' try: - c.call(b'err', [obj]) + c.call('err', [obj]) raise Exception("unexpected") except rpc.RPCError as e: assert e.get_value() == obj @@ -296,18 +276,18 @@ class Test_rpc(unittest.TestCase): def callback(n): l.append(n) c = rpc.Client(self._client_sock, notification_callback=callback) - c.send_notification(b'notify2', [b'notify_foo', []]) + c.send_notification('notify2', ['notify_foo', []]) hub.sleep(0.5) # give the peer a chance to run - obj = b'hoge' + obj = 'hoge' try: - c.call(b'err', [obj]) + c.call('err', [obj]) raise Exception("unexpected") except rpc.RPCError as e: assert e.get_value() == obj assert len(l) == 1 n = l.pop(0) method, params = n - assert method == b'notify_foo' + assert method == 'notify_foo' assert params == [] def test_4_async_call(self): @@ -319,7 +299,7 @@ class Test_rpc(unittest.TestCase): e = rpc.EndPoint(self._client_sock) s = set() for i in range(1, num_calls + 1): - s.add(e.send_request(b'resp', [i])) + s.add(e.send_request('resp', [i])) sum = 0 while s: e.block() @@ -349,7 +329,7 @@ class Test_rpc(unittest.TestCase): e = rpc.EndPoint(self._client_sock) s = set() for i in range(1, num_calls + 1): - s.add(e.send_request(b'callback', [i, b'ourcallback', 0])) + s.add(e.send_request('callback', [i, 'ourcallback', 0])) sum = 0 while s: e.block() @@ -368,10 +348,10 @@ class Test_rpc(unittest.TestCase): r = e.get_request() if r is not None: msgid, method, params = r - assert method == b'ourcallback' + assert method == 'ourcallback' omsgid, n, cb, v = params assert omsgid in s - assert cb == b'ourcallback' + assert cb == 'ourcallback' assert n > 0 e.send_response(msgid, result=[omsgid, n - 1, cb, v + 1]) assert sum == (1 + num_calls) * num_calls / 2 -- 2.7.4 ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi _______________________________________________ Ryu-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ryu-devel
