[1457] fix a few final todo's

most important, a catchall during processing of the update section

src/lib/python/isc/ddns/libddns_messages.mes

@@ -172,9 +172,17 @@ the zone specified in the Zone section of the UPDATE message.
 The specific update record is shown. A NOTZONE error response is
 sent to the client.
 
-% LIBDDNS_UPDATE_PREREQUISITE_FAILED prerequisite failed in update update client %1 for zone %2: result code %3
+% LIBDDNS_UPDATE_PREREQUISITE_FAILED prerequisite failed in update client %1 for zone %2: result code %3
 The handling of the prerequisite section (RFC2136 Section 3.2) found
 that one of the prerequisites was not satisfied. The result code
 should give more information on what prerequisite type failed.
 If the result code is FORMERR, the prerequisite section was not well-formed.
 An error response with the given result code is sent back to the client.
+
+% LIBDDNS_UPDATE_UNCAUGHT_EXCEPTION update client %1 for zone %2: uncaught exception while processing update section: %1
+An uncaught exception was encountered while processing the Update
+section of a DDNS message. The specific exception is shown in the log message.
+To make sure DDNS service is not interrupted, this problem is caught instead
+of reraised; The update is aborted, and a SERVFAIL is sent back to the client.
+This is most probably a bug in the DDNS code, but *could* be caused by
+the data source.

src/lib/python/isc/ddns/session.py

@@ -82,8 +82,7 @@ def foreach_rr_in_rrset(rrset, method, *kwargs):
        the second as the 'real' argument to my_print (which is replaced
        by this function.
     '''
-    #result = None
-    # todo: check for empty?
+    result = None
     for rdata in rrset.get_rdata():
         tmp_rrset = isc.dns.RRset(rrset.get_name(),
                                   rrset.get_class(),
@@ -702,40 +701,43 @@ class UpdateSession:
             return prescan_result
 
         # update
-        # TODO: catchall? any error should result in abort and servfail...
-        # Don't like catchalls much, though
-
-        # create an ixfr-out-friendly diff structure to work on
-        diff = isc.xfrin.diff.Diff(self.__datasrc_client, self.__zname,
-                                   journaling=True, single_update_mode=True)
-
-        # Do special handling for SOA first
-        self.__update_soa(diff)
-
-        # Algorithm from RFC2136 Section 3.4
-        # Note that this works on full rrsets, not individual RRs.
-        # Some checks might be easier with individual RRs, but only if we
-        # would use the ZoneUpdater directly (so we can query the
-        # 'zone-as-it-would-be-so-far'. However, due to the current use
-        # of the Diff class, this is not the case, and therefore it
-        # is easier to work with full rrsets for the most parts
-        # (less lookups needed; conversion to individual rrs is
-        # the same offort whether it is done here or in the several
-        # do_update statements)
-        for rrset in self.__message.get_section(SECTION_UPDATE):
-            if rrset.get_class() == self.__zclass:
-                self.__do_update_add_rrs_to_rrset(diff, rrset)
-            elif rrset.get_class() == RRClass.ANY():
-                if rrset.get_type() == RRType.ANY():
-                    self.__do_update_delete_name(diff, rrset)
-                else:
-                    self.__do_update_delete_rrset(diff, rrset)
-            elif rrset.get_class() == RRClass.NONE():
-                self.__do_update_delete_rrs_from_rrset(diff, rrset)
-
-        #try:
-        diff.commit()
-        return Rcode.NOERROR()
-        #except isc.datasrc.Error as dse:
-        #    logger.info(LIBDDNS_UPDATE_DATASRC_ERROR, dse)
-        #    return Rcode.SERVFAIL()
+        try:
+            # create an ixfr-out-friendly diff structure to work on
+            diff = isc.xfrin.diff.Diff(self.__datasrc_client, self.__zname,
+                                       journaling=True, single_update_mode=True)
+
+            # Do special handling for SOA first
+            self.__update_soa(diff)
+
+            # Algorithm from RFC2136 Section 3.4
+            # Note that this works on full rrsets, not individual RRs.
+            # Some checks might be easier with individual RRs, but only if we
+            # would use the ZoneUpdater directly (so we can query the
+            # 'zone-as-it-would-be-so-far'. However, due to the current use
+            # of the Diff class, this is not the case, and therefore it
+            # is easier to work with full rrsets for the most parts
+            # (less lookups needed; conversion to individual rrs is
+            # the same offort whether it is done here or in the several
+            # do_update statements)
+            for rrset in self.__message.get_section(SECTION_UPDATE):
+                if rrset.get_class() == self.__zclass:
+                    self.__do_update_add_rrs_to_rrset(diff, rrset)
+                elif rrset.get_class() == RRClass.ANY():
+                    if rrset.get_type() == RRType.ANY():
+                        self.__do_update_delete_name(diff, rrset)
+                    else:
+                        self.__do_update_delete_rrset(diff, rrset)
+                elif rrset.get_class() == RRClass.NONE():
+                    self.__do_update_delete_rrs_from_rrset(diff, rrset)
+
+            diff.commit()
+            return Rcode.NOERROR()
+        except isc.datasrc.Error as dse:
+            logger.info(LIBDDNS_UPDATE_DATASRC_ERROR, dse)
+            return Rcode.SERVFAIL()
+        except Exception as uce:
+            logger.error(LIBDDNS_UPDATE_UNCAUGHT_EXCEPTION,
+                         ClientFormatter(self.__client_addr),
+                         ZoneFormatter(self.__zname, self.__zclass),
+                         uce)
+            return Rcode.SERVFAIL()

src/lib/python/isc/ddns/tests/session_tests.py

@@ -426,7 +426,6 @@ class SessionTest(unittest.TestCase):
         else:
             self.assertEqual(UPDATE_ERROR, result)
 
-    # TODO: remove dupe with above one
     def check_prescan_result(self, expected, updates, expected_soa = None):
         '''Helper method for checking the result of a prerequisite check;
            creates an update session, and fills it with the list of rrsets
@@ -446,18 +445,6 @@ class SessionTest(unittest.TestCase):
         self.assertEqual(str(expected_soa),
                          str(session._UpdateSession__added_soa))
 
-        # REMOVED, don't mess with actual data during prescan tests
-        # Now see if handle finds the same result
-        #(result, _, _) = session.handle()
-        #self.assertEqual(expected,
-        #                 session._UpdateSession__message.get_rcode())
-        ## And that the result looks right
-        #if expected == Rcode.NOERROR():
-        #    self.assertEqual(UPDATE_SUCCESS, result)
-        #else:
-        #    self.assertEqual(UPDATE_ERROR, result)
-
-    # TODO XXX: remove dupe with above
     def check_full_handle_result(self, expected, updates):
         '''Helper method for checking the result of a full handle;
            creates an update session, and fills it with the list of rrsets
@@ -742,7 +729,6 @@ class SessionTest(unittest.TestCase):
         # Two soas. Should we reject or simply use the last?
         # (RFC is not really explicit on this, but between the lines I read
         # use the last)
-        # TODO this fails ;)
         self.check_prescan_result(Rcode.NOERROR(),
                                   [ self.rrset_update_soa,
                                     self.rrset_update_soa2 ],
@@ -1099,6 +1085,13 @@ class SessionTest(unittest.TestCase):
                              [ "foo" ])
         self.check_full_handle_result(Rcode.FORMERR(), [ rrset ])
 
+    def test_uncaught_exception(self):
+        def my_exc():
+            raise Exception("foo")
+        self.__session._UpdateSession__update_soa = my_exc
+        self.assertEqual(Rcode.SERVFAIL().to_text(),
+                         self.__session._UpdateSession__do_update().to_text())
+
 if __name__ == "__main__":
     isc.log.init("bind10")
     isc.log.resetUnitTestRootLogger()