[trac714] handle bad data on the cc session in cfgmgr

diff --git a/src/lib/python/isc/cc/session.py b/src/lib/python/isc/cc/session.py
index fb7dd06ff077a0d12407116ba278d63a64a2cdcb..f6b62653beb906ed75c8c2113404ec52f0bfd7cd 100644 (file)
--- a/src/lib/python/isc/cc/session.py
+++ b/src/lib/python/isc/cc/session.py
@@ -93,6 +93,19 @@ class Session:
                 self._socket.send(msg)
 
     def recvmsg(self, nonblock = True, seq = None):
+        """Reads a message. If nonblock is true, and there is no
+           message to read, it returns (None, None).
+           If seq is not None, it should be a value as returned by
+           group_sendmsg(), in which case only the response to
+           that message is returned, and others will be queued until
+           the next call to this method.
+           If seq is None, only messages that are *not* responses
+           will be returned, and responses will be queued.
+           The queue is checked for relevant messages before data
+           is read from the socket.
+           Raises a SessionError if there is a JSON decode problem in
+           the message that is read, or if the session has been closed
+           prior to the call of recvmsg()"""
         with self._lock:
             if len(self._queue) > 0:
                 i = 0;
@@ -109,16 +122,22 @@ class Session:
             if data and len(data) > 2:
                 header_length = struct.unpack('>H', data[0:2])[0]
                 data_length = len(data) - 2 - header_length
-                if data_length > 0:
-                    env = isc.cc.message.from_wire(data[2:header_length+2])
-                    msg = isc.cc.message.from_wire(data[header_length + 2:])
-                    if (seq == None and "reply" not in env) or (seq != None and "reply" in env and seq == env["reply"]):
-                        return env, msg
+                try:
+                    if data_length > 0:
+                        env = isc.cc.message.from_wire(data[2:header_length+2])
+                        msg = isc.cc.message.from_wire(data[header_length + 2:])
+                        if (seq == None and "reply" not in env) or (seq != None and "reply" in env and seq == env["reply"]):
+                            return env, msg
+                        else:
+                            self._queue.append((env,msg))
+                            return self.recvmsg(nonblock, seq)
                     else:
-                        self._queue.append((env,msg))
-                        return self.recvmsg(nonblock, seq)
-                else:
-                    return isc.cc.message.from_wire(data[2:header_length+2]), None
+                        return isc.cc.message.from_wire(data[2:header_length+2]), None
+                except ValueError as ve:
+                    # TODO: when we have logging here, add a debug
+                    # message printing the data that we were unable
+                    # to parse as JSON
+                    raise SessionError(ve)
             return None, None
 
     def _receive_bytes(self, size):

diff --git a/src/lib/python/isc/cc/tests/session_test.py b/src/lib/python/isc/cc/tests/session_test.py
index fe35a6cbfafba7aa2e2892efd731b809e3c590b0..772ed0c961e81de172c9898dea528f9c2b3edd94 100644 (file)
--- a/src/lib/python/isc/cc/tests/session_test.py
+++ b/src/lib/python/isc/cc/tests/session_test.py
@@ -274,6 +274,16 @@ class testSession(unittest.TestCase):
         self.assertEqual({"hello": "b"}, msg)
         self.assertFalse(sess.has_queued_msgs())
 
+    def test_recv_bad_msg(self):
+        sess = MySession()
+        self.assertFalse(sess.has_queued_msgs())
+        sess._socket.addrecv({'to': 'someone' }, {'hello': 'b'})
+        sess._socket.addrecv({'to': 'someone', 'reply': 1}, {'hello': 'a'})
+        # mangle the bytes a bit
+        sess._socket.recvqueue[5] = sess._socket.recvqueue[5] - 2
+        sess._socket.recvqueue = sess._socket.recvqueue[:-2]
+        self.assertRaises(SessionError, sess.recvmsg, True, 1)
+
     def test_next_sequence(self):
         sess = MySession()
         self.assertEqual(sess._sequence, 1)

diff --git a/src/lib/python/isc/config/cfgmgr.py b/src/lib/python/isc/config/cfgmgr.py
index 83db159794db25feba27c2888eae886ac8c20fa5..18e001c306c9083461dfe4cfffce3f861fb71a4f 100644 (file)
--- a/src/lib/python/isc/config/cfgmgr.py
+++ b/src/lib/python/isc/config/cfgmgr.py
@@ -380,6 +380,9 @@ class ConfigManager:
                 answer, env = self.cc.group_recvmsg(False, seq)
             except isc.cc.SessionTimeout:
                 answer = ccsession.create_answer(1, "Timeout waiting for answer from " + module_name)
+            except isc.cc.SessionError as se:
+                logger.error(CFGMGR_BAD_UPDATE_RESPONSE_FROM_MODULE, module_name, se)
+                answer = ccsession.create_answer(1, "Unable to parse response from " + module_name + ": " + str(se))
         if answer:
             rcode, val = ccsession.parse_answer(answer)
             if rcode == 0:

diff --git a/src/lib/python/isc/config/cfgmgr_messages.mes b/src/lib/python/isc/config/cfgmgr_messages.mes
index 9355e4d9769e1708318e33b10788a2640888e0fd..eb09ab4692aa9c020d0a45714a2719355537ce0c 100644 (file)
--- a/src/lib/python/isc/config/cfgmgr_messages.mes
+++ b/src/lib/python/isc/config/cfgmgr_messages.mes
@@ -20,6 +20,12 @@ An older version of the configuration database has been found, from which
 there was an automatic upgrade path to the current version. These changes
 are now applied, and no action from the administrator is necessary.
 
+% CFGMGR_BAD_UPDATE_RESPONSE_FROM_MODULE Unable to parse response from module %1: %2
+The configuration manager sent a configuration update to a module, but
+the module responded with an answer that could not be parsed. The answer
+message appears to be invalid JSON data, or not decodable to a string.
+This is likely to be a problem in the module in question.
+
 % CFGMGR_CC_SESSION_ERROR Error connecting to command channel: %1
 The configuration manager daemon was unable to connect to the messaging
 system. The most likely cause is that msgq is not running.