[1298] address review comments

src/bin/xfrin/tests/xfrin_test.py

@@ -1922,21 +1922,6 @@ class TestXfrin(unittest.TestCase):
         self.assertEqual(self.xfr.command_handler("notify",
                                                   self.args)['result'][0], 0)
 
-        # Note: The rest of the tests won't pass due to the change in #1298
-        # We should probably simply remove the test cases, but for now we
-        # just comment them out.  (Note also that the comment about 'not
-        # from the config' is now wrong, because we used the matching address.)
-        #
-        # and see if we used the address from the command, and not from
-        # the config
-        # This is actually NOT the address given in the command, which
-        # would at this point not make sense, see the TODO in
-        # xfrin.py.in Xfrin.command_handler())
-#         self.assertEqual(TEST_MASTER_IPV4_ADDRESS,
-#                          self.xfr.xfrin_started_master_addr)
-#         self.assertEqual(int(TEST_MASTER_PORT),
-#                          self.xfr.xfrin_started_master_port)
-
     def test_command_handler_unknown(self):
         self.assertEqual(self.xfr.command_handler("xxx", None)['result'][0], 1)
 

src/bin/xfrin/xfrin.py.in

@@ -122,6 +122,32 @@ def _check_zone_class(zone_class_str):
     except InvalidRRClass as irce:
         raise XfrinZoneInfoException("bad zone class: " + zone_class_str + " (" + str(irce) + ")")
 
+def format_zone_str(zone_name, zone_class):
+    """Helper function to format a zone name and class as a string of
+       the form '<name>/<class>'.
+       Parameters:
+       zone_name (isc.dns.Name) name to format
+       zone_class (isc.dns.RRClass) class to format
+    """
+    return zone_name.to_text() + '/' + str(zone_class)
+
+def format_addrinfo(addrinfo):
+    """Helper function to format the addrinfo as a string of the form
+       <addr>:<port> (for IPv4) or [<addr>]:port (for IPv6). For unix domain
+       sockets, and unknown address families, it returns a basic string
+       conversion of the third element of the passed tuple.
+       Parameters:
+       addrinfo: an 3-tuple consisting of address family, socket type, and,
+                 depending on the family, either a 2-tuple with the address
+                 and port, or a filename
+    """
+    if addrinfo[0] == socket.AF_INET:
+        return str(addrinfo[2][0]) + ":" + str(addrinfo[2][1])
+    elif addrinfo[0] == socket.AF_INET6:
+        return "[" + str(addrinfo[2][0]) + "]:" + str(addrinfo[2][1])
+    else:
+        return str(addrinfo[2])
+
 def get_soa_serial(soa_rdata):
     '''Extract the serial field of an SOA RDATA and returns it as an intger.
 
@@ -491,8 +517,8 @@ class XfrinConnection(asyncore.dispatcher):
         return self.__state
 
     def zone_str(self):
-        '''A convenient function for logging to include zone name and class'''
-        return self._zone_name.to_text() + '/' + str(self._rrclass)
+        '''A convenience function for logging to include zone name and class'''
+        return format_zone_str(self._zone_name, self._rrclass)
 
     def connect_to_master(self):
         '''Connect to master in TCP.'''
@@ -1060,13 +1086,14 @@ class Xfrin:
                 # a security hole. Once we add the ability to have multiple master addresses,
                 # we should check if it matches one of them, and then use it.)
                 (zone_name, rrclass) = self._parse_zone_name_and_class(args)
+                zone_str = format_zone_str(zone_name, rrclass)
                 zone_info = self._get_zone_info(zone_name, rrclass)
                 notify_addr = self._parse_master_and_port(args, zone_name,
                                                           rrclass)
                 if zone_info is None:
                     # TODO what to do? no info known about zone. defaults?
-                    errmsg = "Got notification to retransfer unknown zone " + zone_name.to_text()
-                    logger.error(XFRIN_RETRANSFER_UNKNOWN_ZONE, zone_name.to_text())
+                    errmsg = "Got notification to retransfer unknown zone " + zone_str
+                    logger.error(XFRIN_RETRANSFER_UNKNOWN_ZONE, zone_str)
                     answer = create_answer(1, errmsg)
                 else:
                     master_addr = zone_info.get_master_addr_info()
@@ -1080,11 +1107,12 @@ class Xfrin:
                                                True)
                         answer = create_answer(ret[0], ret[1])
                     else:
-                        errmsg = "Got notification for " + zone_name.to_text()\
-                               + "from unknown address: " + notify_addr[2][0];
-                        logger.error(XFRIN_NOTIFY_UNKNOWN_MASTER,
-                                     zone_name.to_text(), notify_addr[2][0],
-                                     master_addr[2][0])
+                        notify_addr_str = format_addrinfo(notify_addr)
+                        master_addr_str = format_addrinfo(master_addr)
+                        errmsg = "Got notification for " + zone_str\
+                               + "from unknown address: " + notify_addr_str;
+                        logger.INFO(XFRIN_NOTIFY_UNKNOWN_MASTER, zone_str,
+                                    notify_addr_str, master_addr_str)
                         answer = create_answer(1, errmsg)
 
             elif command == 'retransfer' or command == 'refresh':

src/bin/xfrin/xfrin_messages.mes

@@ -70,7 +70,7 @@ was killed.
 There was a problem sending a message to the zone manager. This most
 likely means that the msgq daemon has quit or was killed.
 
-% XFRIN_NOTIFY_UNKNOWN_MASTER got notification to retransfer zone %1 from %2, expected %3
+% XFRIN_NOTIFY_UNKNOWN_MASTER got notification to retransfer zone %1 from %2/%3, expected %4/%5
 The system received a notify for the given zone, but the address it came
 from does not match the master address in the Xfrin configuration. The notify
 is ignored. This may indicate that the configuration for the master is wrong,