[master] Merge branch 'trac964'

src/lib/python/isc/notify/notify_out.py

@@ -21,6 +21,7 @@ import threading
 import time
 import errno
 from isc.datasrc import sqlite3_ds
+from isc.net import addr
 import isc
 try: 
     from pydnspp import * 
@@ -66,7 +67,7 @@ class ZoneNotifyInfo:
         self.zone_name = zone_name_
         self.zone_class = class_
         self.notify_msg_id = 0
-        self.notify_timeout = 0
+        self.notify_timeout = None
         self.notify_try_num = 0  #Notify times sending to one target.
        
     def set_next_notify_target(self):
@@ -77,8 +78,7 @@ class ZoneNotifyInfo:
             self._notify_current = None
 
     def prepare_notify_out(self):
-        '''Create the socket and set notify timeout time to now'''
-        self._sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) #TODO support IPv6?
+        '''Set notify timeout time to now'''
         self.notify_timeout = time.time()
         self.notify_try_num = 0
         self._slave_index = 0
@@ -89,6 +89,12 @@ class ZoneNotifyInfo:
         if self._sock:
             self._sock.close()
             self._sock = None
+        self.notify_timeout = None
+
+    def create_socket(self, dest_addr):
+        self._sock = socket.socket(addr.IPAddr(dest_addr).family,
+                                   socket.SOCK_DGRAM)
+        return self._sock
 
     def get_socket(self):
         return self._sock
@@ -270,8 +276,15 @@ class NotifyOut:
             sock = self._notify_infos[info].get_socket()
             if sock:
                 valid_socks.append(sock)
+
+            # If a non null timeout is specified notify has been scheduled
+            # (in which case socket is still None) or sent (with a valid
+            # socket).  In either case we need add the zone to notifying_zones
+            # so that we can invoke the appropriate event for the zone after
+            # select.
+            tmp_timeout = self._notify_infos[info].notify_timeout
+            if tmp_timeout is not None:
                 notifying_zones[info] = self._notify_infos[info]
-                tmp_timeout = self._notify_infos[info].notify_timeout
                 if min_timeout is not None:
                     if tmp_timeout < min_timeout:
                         min_timeout = tmp_timeout
@@ -380,12 +393,13 @@ class NotifyOut:
         render.set_length_limit(512) 
         msg.to_wire(render)
         zone_notify_info.notify_msg_id = qid
-        sock = zone_notify_info.get_socket()
         try:
+            sock = zone_notify_info.create_socket(addrinfo[0])
             sock.sendto(render.get_data(), 0, addrinfo)
             self._log_msg('info', 'sending notify to %s' % addr_to_str(addrinfo))
-        except socket.error as err:
-            self._log_msg('error', 'send notify to %s failed: %s' % (addr_to_str(addrinfo), str(err)))
+        except (socket.error, addr.InvalidAddress) as err:
+            self._log_msg('error', 'send notify to %s failed: %s' %
+                          (addr_to_str(addrinfo), str(err)))
             return False
 
         return True

src/lib/python/isc/notify/tests/notify_out_test.py

@@ -24,9 +24,7 @@ from isc.notify import notify_out, SOCK_DATA
 
 # our fake socket, where we can read and insert messages
 class MockSocket():
-    def __init__(self, family, type):
-        self.family = family
-        self.type = type
+    def __init__(self):
         self._local_sock, self._remote_sock = socket.socketpair()
 
     def connect(self, to):
@@ -51,12 +49,16 @@ class MockSocket():
         return self._remote_sock
 
 # We subclass the ZoneNotifyInfo class we're testing here, only
-# to override the prepare_notify_out() method.
+# to override the create_socket() method.
 class MockZoneNotifyInfo(notify_out.ZoneNotifyInfo):
-    def prepare_notify_out(self):
-        super().prepare_notify_out();
+    def create_socket(self, addrinfo):
+        super().create_socket(addrinfo)
+        # before replacing the underlying socket, remember the address family
+        # of the original socket so that tests can check that.
+        self.sock_family = self._sock.family
         self._sock.close()
-        self._sock = MockSocket(socket.AF_INET, socket.SOCK_DGRAM)
+        self._sock = MockSocket()
+        return self._sock
 
 class TestZoneNotifyInfo(unittest.TestCase):
     def setUp(self):
@@ -64,11 +66,12 @@ class TestZoneNotifyInfo(unittest.TestCase):
 
     def test_prepare_finish_notify_out(self):
         self.info.prepare_notify_out()
-        self.assertNotEqual(self.info._sock, None)
+        self.assertNotEqual(self.info.notify_timeout, None)
         self.assertIsNone(self.info._notify_current)
 
         self.info.finish_notify_out()
         self.assertEqual(self.info._sock, None)
+        self.assertEqual(self.info.notify_timeout, None)
 
     def test_set_next_notify_target(self):
         self.info.notify_slaves.append(('127.0.0.1', 53))
@@ -155,6 +158,11 @@ class TestNotifyOut(unittest.TestCase):
         self.assertEqual(len(replied_zones), 0)
         self.assertEqual(len(timeout_zones), 2)
 
+        # Trigger timeout events to "send" notifies via a mock socket
+        for zone in timeout_zones:
+            self._notify._zone_notify_handler(timeout_zones[zone],
+                                              notify_out._EVENT_TIMEOUT)
+
         # Now make one socket be readable
         self._notify._notify_infos[('example.net.', 'IN')].notify_timeout = time.time() + 10
         self._notify._notify_infos[('example.com.', 'IN')].notify_timeout = time.time() + 10
@@ -234,11 +242,31 @@ class TestNotifyOut(unittest.TestCase):
         data = b'\x2f\x18\x10\x10\x00\x01\x00\x00\x00\x00\x00\x00\x07example\03com\x00\x00\x06\x00\x01'
         self.assertEqual(notify_out._BAD_QR, self._notify._handle_notify_reply(example_com_info, data))
 
-    def test_send_notify_message_udp(self):
+    def test_send_notify_message_udp_ipv4(self):
         example_com_info = self._notify._notify_infos[('example.net.', 'IN')]
         example_com_info.prepare_notify_out()
-        ret = self._notify._send_notify_message_udp(example_com_info, ('1.1.1.1', 53))
+        ret = self._notify._send_notify_message_udp(example_com_info,
+                                                    ('192.0.2.1', 53))
+        self.assertTrue(ret)
+        self.assertEqual(socket.AF_INET, example_com_info.sock_family)
+
+    def test_send_notify_message_udp_ipv6(self):
+        example_com_info = self._notify._notify_infos[('example.net.', 'IN')]
+        ret = self._notify._send_notify_message_udp(example_com_info,
+                                                    ('2001:db8::53', 53))
         self.assertTrue(ret)
+        self.assertEqual(socket.AF_INET6, example_com_info.sock_family)
+
+    def test_send_notify_message_with_bogus_address(self):
+        example_com_info = self._notify._notify_infos[('example.net.', 'IN')]
+
+        # As long as the underlying data source validates RDATA this shouldn't
+        # happen, but right now it's not actually the case.  Even if the
+        # data source does its job, it's prudent to confirm the behavior for
+        # an unexpected case.
+        ret = self._notify._send_notify_message_udp(example_com_info,
+                                                    ('invalid', 53))
+        self.assertFalse(ret)
 
     def test_zone_notify_handler(self):
         old_send_msg = self._notify._send_notify_message_udp