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

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):

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)

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:

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.