From 32a7c1c0a458f589e2710525e03630f00b538cda Mon Sep 17 00:00:00 2001 From: Gevorg Davoian Date: Mon, 25 Apr 2016 15:42:12 +0300 Subject: [PATCH] Fix bug with version_cap and target.version in RPCClient This patch fixes the bug in the RPCClient class when a client's version_cap is set, but target.version is unset. The code does not check this case, which results in unhandled exceptions. Change-Id: I623c14b74b9101bb4ab199dff6609fab44388c4a Closes-Bug: 1574615 --- oslo_messaging/_utils.py | 6 ++++++ oslo_messaging/rpc/client.py | 11 ++++------- oslo_messaging/tests/rpc/test_client.py | 25 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/oslo_messaging/_utils.py b/oslo_messaging/_utils.py index e0025e688..da0012ade 100644 --- a/oslo_messaging/_utils.py +++ b/oslo_messaging/_utils.py @@ -22,6 +22,12 @@ def version_is_compatible(imp_version, version): :param imp_version: The version implemented :param version: The version requested by an incoming message. """ + if imp_version is None: + return True + + if version is None: + return False + version_parts = version.split('.') imp_version_parts = imp_version.split('.') try: diff --git a/oslo_messaging/rpc/client.py b/oslo_messaging/rpc/client.py index 2a353fab7..63f388cf4 100644 --- a/oslo_messaging/rpc/client.py +++ b/oslo_messaging/rpc/client.py @@ -123,12 +123,11 @@ class _BaseCallContext(object): def can_send_version(self, version=_marker): """Check to see if a version is compatible with the version cap.""" version = self.target.version if version is self._marker else version - return (not self.version_cap or - utils.version_is_compatible(self.version_cap, version)) + return utils.version_is_compatible(self.version_cap, version) @classmethod def _check_version(cls, version): - if version is not None and version is not cls._marker: + if version is not cls._marker: # quick sanity check to make sure parsable version numbers are used try: utils.version_is_compatible(version, version) @@ -142,8 +141,7 @@ class _BaseCallContext(object): msg = self._make_message(ctxt, method, kwargs) msg_ctxt = self.serializer.serialize_context(ctxt) - if self.version_cap: - self._check_version_cap(msg.get('version')) + self._check_version_cap(msg.get('version')) try: self.transport._send(self.target, msg_ctxt, msg, retry=self.retry) @@ -163,8 +161,7 @@ class _BaseCallContext(object): if self.timeout is None: timeout = self.conf.rpc_response_timeout - if self.version_cap: - self._check_version_cap(msg.get('version')) + self._check_version_cap(msg.get('version')) try: result = self.transport._send(self.target, msg_ctxt, msg, diff --git a/oslo_messaging/tests/rpc/test_client.py b/oslo_messaging/tests/rpc/test_client.py index 197a296c6..4a527bcd5 100644 --- a/oslo_messaging/tests/rpc/test_client.py +++ b/oslo_messaging/tests/rpc/test_client.py @@ -392,6 +392,14 @@ class TestVersionCap(test_utils.BaseTestCase): dict(cap='2.0', prepare_cap=_notset, version=None, prepare_version='1.0', success=False)), + ('ctor_cap_none_version_ok', + dict(cap=None, prepare_cap=_notset, + version='1.0', prepare_version=_notset, + success=True)), + ('ctor_cap_version_none_fail', + dict(cap='1.0', prepare_cap=_notset, + version=None, prepare_version=_notset, + success=False)), ] @classmethod @@ -492,6 +500,21 @@ class TestCanSendVersion(test_utils.BaseTestCase): version=None, prepare_version='1.0', can_send_version=_notset, can_send=False)), + ('ctor_cap_none_version_ok', + dict(cap=None, prepare_cap=_notset, + version='1.0', prepare_version=_notset, + can_send_version=_notset, + can_send=True)), + ('ctor_cap_version_none_fail', + dict(cap='1.0', prepare_cap=_notset, + version=None, prepare_version=_notset, + can_send_version=_notset, + can_send=False)), + ('ctor_cap_version_can_send_none_fail', + dict(cap='1.0', prepare_cap=_notset, + version='1.0', prepare_version=_notset, + can_send_version=None, + can_send=False)), ] def test_version_cap(self): @@ -529,3 +552,5 @@ class TestCanSendVersion(test_utils.BaseTestCase): client.prepare, version='5') self.assertRaises(exceptions.MessagingException, client.prepare, version='5.a') + self.assertRaises(exceptions.MessagingException, + client.prepare, version='5.5.a')