Merge branch 'master' of ssh://git.bind10.isc.org/var/bind10/git/bind10

ChangeLog

@@ -1,3 +1,12 @@
+295.	[func]*		jinmei
+	b10-xfrin: the AXFR implementation is unified with IXFR, and
+	handles corner cases more carefully.  Note: As a result of this
+	change, xfrin does not create a new (SQLite3) zone in a fresh DB
+	file upon receiving AXFR any more.  Initial zone content must be
+	prepared by hand (e.g. with b10-loadzone) until a more generic
+	tool for zone management is provided.
+	(Trac #1209, git 5ca7b409bccc815cee58c804236504fda1c1c147)
+
 294.	[func]		jelte, jinmei, vorner
 	b10-xfrin now supports incoming IXFR.  See BIND 10 Guide for
 	how to configure it and operational notes.
@@ -50,7 +59,7 @@
 	configuration.
 	(Trac #1165, git 698176eccd5d55759fe9448b2c249717c932ac31)
 
-288.    [bug]       stephen
+288.    [bug]		stephen
 	Fixed problem whereby the order in which component files appeared in
 	rdataclass.cc was system dependent, leading to problems on some
 	systems where data types were used before the header file in which

doc/guide/bind10-guide.xml

@@ -1272,6 +1272,19 @@ TODO
       and care should be taken to enable IXFR.
     </para>
 
+    <note><simpara>
+     In the current development release of BIND 10, incoming zone
+     transfers are only available for SQLite3-based data sources,
+     that is, they don't work for an in-memory data source.
+     Furthermore, the corresponding SQLite3 database must be
+     configured with a list of zone names by hand.  One possible way
+     to do this is to use the <command>b10-loadzone</command> command
+     to load dummy zone content of the zone for which the secondary
+     service is provided (and then force transfer using AXFR from the primary
+     server).  In future versions we will provide more convenient way
+     to set up the secondary.
+    </simpara></note>
+
     <para>
       To enable IXFR, you need to
       configure <command>b10-xfrin</command> with an explicit zone
@@ -1311,12 +1324,6 @@ TODO
       version, at which point we will enable IXFR by default.
     </para>
 
-    <note><simpara>
-     In the current development release of BIND 10, incoming zone
-     transfers are only available for SQLite3-based data sources,
-     that is, they don't work for an in-memory data source.
-    </simpara></note>
-
 <!-- TODO:
 
 how to tell bind10 you are a secondary?

src/bin/xfrin/b10-xfrin.xml

@@ -59,7 +59,7 @@
       <citerefentry><refentrytitle>bind10</refentrytitle><manvolnum>8</manvolnum></citerefentry>
       boss process.
       When triggered it can request and receive a zone transfer and store
-      the zone in a BIND 10 zone data store.
+      the zone in a BIND 10 zone data source.
     </para>
 
 <!-- TODO:
@@ -68,9 +68,13 @@ The logic for handling transfer triggers or zone management is handled
 in separate zonemgr process.
 -->
 
-    <note><simpara>
-      This prototype release only supports AXFR. IXFR is not implemented.
-    </simpara></note>
+    <para>
+      The <command>b10-xfrin</command> daemon supports both AXFR and
+      IXFR.  Due to some implementation limitations of the current
+      development release, however, it only tries AXFR by default,
+      and care should be taken to enable IXFR.
+      See the BIND 10 Guide for more details.
+    </para>
 
     <para>
       This daemon communicates with BIND 10 over a
@@ -105,7 +109,8 @@ in separate zonemgr process.
       <varname>name</varname> (the zone name),
       <varname>class</varname> (defaults to <quote>IN</quote>),
       <varname>master_addr</varname> (the zone master to transfer from),
-      <varname>master_port</varname> (defaults to 53), and
+      <varname>master_port</varname> (defaults to 53),
+      <varname>ixfr_disabled</varname> (defaults to false), and
       <varname>tsig_key</varname> (optional TSIG key to use).
       The <varname>tsig_key</varname> is specified using a full string
       colon-delimited name:key:algorithm representation (e.g.

src/bin/xfrin/tests/Makefile.am

@@ -9,6 +9,9 @@ EXTRA_DIST = $(PYTESTS)
 LIBRARY_PATH_PLACEHOLDER =
 if SET_ENV_LIBRARY_PATH
 LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/util/io/.libs:$(abs_top_builddir)/src/lib/datasrc/.libs:$$$(ENV_LIBRARY_PATH)
+else
+# sunstudio needs the ds path even if not all paths are necessary
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/datasrc/.libs
 endif
 
 # test using command-line arguments, so use check-local target instead of TESTS

src/bin/xfrin/tests/xfrin_test.py

@@ -93,6 +93,9 @@ def check_diffs(assert_fn, expected, actual):
 class XfrinTestException(Exception):
     pass
 
+class XfrinTestTimeoutException(Exception):
+    pass
+
 class MockCC():
     def get_default_value(self, identifier):
         if identifier == "zones/master_port":
@@ -109,6 +112,7 @@ class MockDataSourceClient():
 
     '''
     def __init__(self):
+        self.force_fail = False # if True, raise an exception on commit
         self.committed_diffs = []
         self.diffs = []
 
@@ -169,6 +173,8 @@ class MockDataSourceClient():
         self.diffs.append(('delete', rrset))
 
     def commit(self):
+        if self.force_fail:
+            raise isc.datasrc.Error('Updater.commit() failed')
         self.committed_diffs.append(self.diffs)
         self.diffs = []
 
@@ -205,10 +211,10 @@ class MockXfrin(Xfrin):
                                  request_type, check_soa)
 
 class MockXfrinConnection(XfrinConnection):
-    def __init__(self, sock_map, zone_name, rrclass, db_file, shutdown_event,
+    def __init__(self, sock_map, zone_name, rrclass, shutdown_event,
                  master_addr):
         super().__init__(sock_map, zone_name, rrclass, MockDataSourceClient(),
-                         db_file, shutdown_event, master_addr)
+                         shutdown_event, master_addr)
         self.query_data = b''
         self.reply_data = b''
         self.force_time_out = False
@@ -229,6 +235,8 @@ class MockXfrinConnection(XfrinConnection):
     def recv(self, size):
         data = self.reply_data[:size]
         self.reply_data = self.reply_data[size:]
+        if len(data) == 0:
+            raise XfrinTestTimeoutException('Emulated timeout')
         if len(data) < size:
             raise XfrinTestException('cannot get reply data (' + str(size) +
                                      ' bytes)')
@@ -287,8 +295,7 @@ class TestXfrinState(unittest.TestCase):
     def setUp(self):
         self.sock_map = {}
         self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME,
-                                        TEST_RRCLASS, TEST_DB_FILE,
-                                        threading.Event(),
+                                        TEST_RRCLASS, threading.Event(),
                                         TEST_MASTER_IPV4_ADDRINFO)
         self.begin_soa = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(),
                                RRTTL(3600))
@@ -505,6 +512,7 @@ class TestXfrinAXFR(TestXfrinState):
     def setUp(self):
         super().setUp()
         self.state = XfrinAXFR()
+        self.conn._end_serial = 1234
 
     def test_handle_rr(self):
         """
@@ -524,6 +532,13 @@ class TestXfrinAXFR(TestXfrinState):
         self.assertEqual([('add', self.a_rrset), ('add', soa_rrset)],
                          self.conn._diff.get_buffer())
 
+    def test_handle_rr_mismatch_soa(self):
+        """ SOA with inconsistent serial - unexpected, but we accept it.
+
+        """
+        self.assertTrue(self.state.handle_rr(self.conn, begin_soa_rrset))
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+
     def test_finish_message(self):
         """
         Check normal end of message.
@@ -565,8 +580,7 @@ class TestXfrinConnection(unittest.TestCase):
             os.remove(TEST_DB_FILE)
         self.sock_map = {}
         self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME,
-                                        TEST_RRCLASS, TEST_DB_FILE,
-                                        threading.Event(),
+                                        TEST_RRCLASS, threading.Event(),
                                         TEST_MASTER_IPV4_ADDRINFO)
         self.soa_response_params = {
             'questions': [example_soa_question],
@@ -577,6 +591,10 @@ class TestXfrinConnection(unittest.TestCase):
             'axfr_after_soa': self._create_normal_response_data
             }
         self.axfr_response_params = {
+            'question_1st': default_questions,
+            'question_2nd': default_questions,
+            'answer_1st': [soa_rrset, self._create_ns()],
+            'answer_2nd': default_answers,
             'tsig_1st': None,
             'tsig_2nd': None
             }
@@ -586,24 +604,23 @@ class TestXfrinConnection(unittest.TestCase):
         if os.path.exists(TEST_DB_FILE):
             os.remove(TEST_DB_FILE)
 
-    def _handle_xfrin_response(self):
-        # This helper methods iterates over all RRs (excluding the ending SOA)
-        # transferred, and simply returns the number of RRs.  The return value
-        # may be used an assertion value for test cases.
-        rrs = 0
-        for rr in self.conn._handle_axfrin_response():
-            rrs += 1
-        return rrs
-
     def _create_normal_response_data(self):
         # This helper method creates a simple sequence of DNS messages that
-        # forms a valid AXFR transaction.  It consists of two messages, each
-        # containing just a single SOA RR.
+        # forms a valid AXFR transaction.  It consists of two messages: the
+        # first one containing SOA, NS, the second containing the trailing SOA.
+        question_1st = self.axfr_response_params['question_1st']
+        question_2nd = self.axfr_response_params['question_2nd']
+        answer_1st = self.axfr_response_params['answer_1st']
+        answer_2nd = self.axfr_response_params['answer_2nd']
         tsig_1st = self.axfr_response_params['tsig_1st']
         tsig_2nd = self.axfr_response_params['tsig_2nd']
-        self.conn.reply_data = self.conn.create_response_data(tsig_ctx=tsig_1st)
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=question_1st, answers=answer_1st,
+            tsig_ctx=tsig_1st)
         self.conn.reply_data += \
-            self.conn.create_response_data(tsig_ctx=tsig_2nd)
+            self.conn.create_response_data(questions=question_2nd,
+                                           answers=answer_2nd,
+                                           tsig_ctx=tsig_2nd)
 
     def _create_soa_response_data(self):
         # This helper method creates a DNS message that is supposed to be
@@ -661,6 +678,7 @@ class TestXfrinConnection(unittest.TestCase):
 class TestAXFR(TestXfrinConnection):
     def setUp(self):
         super().setUp()
+        XfrinInitialSOA().set_xfrstate(self.conn, XfrinInitialSOA())
 
     def __create_mock_tsig(self, key, error):
         # This helper function creates a MockTSIGContext for a given key
@@ -697,14 +715,13 @@ class TestAXFR(TestXfrinConnection):
         # to confirm an AF_INET6 socket has been created.  A naive application
         # tends to assume it's IPv4 only and hardcode AF_INET.  This test
         # uncovers such a bug.
-        c = MockXfrinConnection({}, TEST_ZONE_NAME, TEST_RRCLASS, TEST_DB_FILE,
-                                threading.Event(),
-                                TEST_MASTER_IPV6_ADDRINFO)
+        c = MockXfrinConnection({}, TEST_ZONE_NAME, TEST_RRCLASS,
+                                threading.Event(), TEST_MASTER_IPV6_ADDRINFO)
         c.bind(('::', 0))
         c.close()
 
     def test_init_chclass(self):
-        c = MockXfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(), TEST_DB_FILE,
+        c = MockXfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(),
                                 threading.Event(), TEST_MASTER_IPV4_ADDRINFO)
         axfrmsg = c._create_query(RRType.AXFR())
         self.assertEqual(axfrmsg.get_question()[0].get_class(),
@@ -792,24 +809,28 @@ class TestAXFR(TestXfrinConnection):
 
     def test_response_with_invalid_msg(self):
         self.conn.reply_data = b'aaaxxxx'
-        self.assertRaises(XfrinTestException, self._handle_xfrin_response)
+        self.assertRaises(XfrinTestException,
+                          self.conn._handle_xfrin_responses)
 
     def test_response_with_tsigfail(self):
         self.conn._tsig_key = TSIG_KEY
         # server tsig check fail, return with RCODE 9 (NOTAUTH)
         self.conn._send_query(RRType.SOA())
         self.conn.reply_data = self.conn.create_response_data(rcode=Rcode.NOTAUTH())
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
+        self.assertRaises(XfrinException, self.conn._handle_xfrin_responses)
 
     def test_response_without_end_soa(self):
         self.conn._send_query(RRType.AXFR())
         self.conn.reply_data = self.conn.create_response_data()
-        self.assertRaises(XfrinTestException, self._handle_xfrin_response)
+        # This should result in timeout in the asyncore loop.  We emulate
+        # that situation in recv() by emptying the reply data buffer.
+        self.assertRaises(XfrinTestTimeoutException,
+                          self.conn._handle_xfrin_responses)
 
     def test_response_bad_qid(self):
         self.conn._send_query(RRType.AXFR())
-        self.conn.reply_data = self.conn.create_response_data(bad_qid = True)
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
+        self.conn.reply_data = self.conn.create_response_data(bad_qid=True)
+        self.assertRaises(XfrinException, self.conn._handle_xfrin_responses)
 
     def test_response_error_code_bad_sig(self):
         self.conn._tsig_key = TSIG_KEY
@@ -822,7 +843,7 @@ class TestAXFR(TestXfrinConnection):
         # validate log message for XfrinException
         self.__match_exception(XfrinException,
                                "TSIG verify fail: BADSIG",
-                               self._handle_xfrin_response)
+                               self.conn._handle_xfrin_responses)
 
     def test_response_bad_qid_bad_key(self):
         self.conn._tsig_key = TSIG_KEY
@@ -834,36 +855,29 @@ class TestAXFR(TestXfrinConnection):
         # validate log message for XfrinException
         self.__match_exception(XfrinException,
                                "TSIG verify fail: BADKEY",
-                               self._handle_xfrin_response)
+                               self.conn._handle_xfrin_responses)
 
     def test_response_non_response(self):
         self.conn._send_query(RRType.AXFR())
-        self.conn.reply_data = self.conn.create_response_data(response = False)
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
+        self.conn.reply_data = self.conn.create_response_data(response=False)
+        self.assertRaises(XfrinException, self.conn._handle_xfrin_responses)
 
     def test_response_error_code(self):
         self.conn._send_query(RRType.AXFR())
         self.conn.reply_data = self.conn.create_response_data(
             rcode=Rcode.SERVFAIL())
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
+        self.assertRaises(XfrinException, self.conn._handle_xfrin_responses)
 
     def test_response_multi_question(self):
         self.conn._send_query(RRType.AXFR())
         self.conn.reply_data = self.conn.create_response_data(
             questions=[example_axfr_question, example_axfr_question])
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
-
-    def test_response_empty_answer(self):
-        self.conn._send_query(RRType.AXFR())
-        self.conn.reply_data = self.conn.create_response_data(answers=[])
-        # Should an empty answer trigger an exception?  Even though it's very
-        # unusual it's not necessarily invalid.  Need to revisit.
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
+        self.assertRaises(XfrinException, self.conn._handle_xfrin_responses)
 
     def test_response_non_response(self):
         self.conn._send_query(RRType.AXFR())
         self.conn.reply_data = self.conn.create_response_data(response = False)
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
+        self.assertRaises(XfrinException, self.conn._handle_xfrin_responses)
 
     def test_soacheck(self):
         # we need to defer the creation until we know the QID, which is
@@ -954,30 +968,155 @@ class TestAXFR(TestXfrinConnection):
         self.conn.response_generator = self._create_normal_response_data
         self.conn._shutdown_event.set()
         self.conn._send_query(RRType.AXFR())
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
+        self.assertRaises(XfrinException, self.conn._handle_xfrin_responses)
 
     def test_response_timeout(self):
         self.conn.response_generator = self._create_normal_response_data
         self.conn.force_time_out = True
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
+        self.assertRaises(XfrinException, self.conn._handle_xfrin_responses)
 
     def test_response_remote_close(self):
         self.conn.response_generator = self._create_normal_response_data
         self.conn.force_close = True
-        self.assertRaises(XfrinException, self._handle_xfrin_response)
+        self.assertRaises(XfrinException, self.conn._handle_xfrin_responses)
 
     def test_response_bad_message(self):
         self.conn.response_generator = self._create_broken_response_data
         self.conn._send_query(RRType.AXFR())
-        self.assertRaises(Exception, self._handle_xfrin_response)
+        self.assertRaises(Exception, self.conn._handle_xfrin_responses)
 
     def test_axfr_response(self):
-        # normal case.
+        # A simple normal case: AXFR consists of SOA, NS, then trailing SOA.
+        self.conn.response_generator = self._create_normal_response_data
+        self.conn._send_query(RRType.AXFR())
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_response_empty_answer(self):
+        '''Test with an empty AXFR answer section.
+
+        This is an unusual response, but there is no reason to reject it.
+        The second message is a complete AXFR response, and transfer should
+        succeed just like the normal case.
+
+        '''
+
+        self.axfr_response_params['answer_1st'] = []
+        self.axfr_response_params['answer_2nd'] = [soa_rrset,
+                                                   self._create_ns(),
+                                                   soa_rrset]
+        self.conn.response_generator = self._create_normal_response_data
+        self.conn._send_query(RRType.AXFR())
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_soa_mismatch(self):
+        '''AXFR response whose begin/end SOAs are not same.
+
+        What should we do this is moot, for now we accept it, so does BIND 9.
+
+        '''
+        ns_rr = self._create_ns()
+        a_rr = self._create_a('192.0.2.1')
+        self.conn._send_query(RRType.AXFR())
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                RRType.AXFR())],
+            # begin serial=1230, end serial=1234. end will be used.
+            answers=[begin_soa_rrset, ns_rr, a_rr, soa_rrset])
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', ns_rr), ('add', a_rr), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_extra(self):
+        '''Test with an extra RR after the end of AXFR session.
+
+        The session should be rejected, and nothing should be committed.
+
+        '''
+        ns_rr = self._create_ns()
+        a_rr = self._create_a('192.0.2.1')
+        self.conn._send_query(RRType.AXFR())
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                RRType.AXFR())],
+            answers=[soa_rrset, ns_rr, a_rr, soa_rrset, a_rr])
+        self.assertRaises(XfrinProtocolError,
+                          self.conn._handle_xfrin_responses)
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        self.assertEqual([], self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_qname_mismatch(self):
+        '''AXFR response with a mismatch question name.
+
+        Our implementation accepts that, so does BIND 9.
+
+        '''
+        self.axfr_response_params['question_1st'] = \
+            [Question(Name('mismatch.example'), TEST_RRCLASS, RRType.AXFR())]
+        self.conn.response_generator = self._create_normal_response_data
+        self.conn._send_query(RRType.AXFR())
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_qclass_mismatch(self):
+        '''AXFR response with a mismatch RR class.
+
+        Our implementation accepts that, so does BIND 9.
+
+        '''
+        self.axfr_response_params['question_1st'] = \
+            [Question(TEST_ZONE_NAME, RRClass.CH(), RRType.AXFR())]
         self.conn.response_generator = self._create_normal_response_data
         self.conn._send_query(RRType.AXFR())
-        # two SOAs, and only these have been transfered.  the 2nd SOA is just
-        # a marker, so only 1 RR has been provided in the iteration.
-        self.assertEqual(self._handle_xfrin_response(), 1)
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_qtype_mismatch(self):
+        '''AXFR response with a mismatch RR type.
+
+        Our implementation accepts that, so does BIND 9.
+
+        '''
+        # returning IXFR in question to AXFR query
+        self.axfr_response_params['question_1st'] = \
+            [Question(TEST_ZONE_NAME, RRClass.CH(), RRType.IXFR())]
+        self.conn.response_generator = self._create_normal_response_data
+        self.conn._send_query(RRType.AXFR())
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_empty_question(self):
+        '''AXFR response with an empty question.
+
+        Our implementation accepts that, so does BIND 9.
+
+        '''
+        self.axfr_response_params['question_1st'] = []
+        self.conn.response_generator = self._create_normal_response_data
+        self.conn._send_query(RRType.AXFR())
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
 
     def test_do_xfrin(self):
         self.conn.response_generator = self._create_normal_response_data
@@ -991,9 +1130,10 @@ class TestAXFR(TestXfrinConnection):
             lambda key: self.__create_mock_tsig(key, TSIGError.NOERROR)
         self.conn.response_generator = self._create_normal_response_data
         self.assertEqual(self.conn.do_xfrin(False), XFRIN_OK)
-        # We use two messages in the tests.  The same context should have been
-        # usef for both.
-        self.assertEqual(2, self.conn._tsig_ctx.verify_called)
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
 
     def test_do_xfrin_with_tsig_fail(self):
         # TSIG verify will fail for the first message.  xfrin should fail
@@ -1073,10 +1213,10 @@ class TestAXFR(TestXfrinConnection):
         self.conn.response_generator = self._create_broken_response_data
         self.assertEqual(self.conn.do_xfrin(False), XFRIN_FAIL)
 
-    def test_do_xfrin_dberror(self):
-        # DB file is under a non existent directory, so its creation will fail,
-        # which will make the transfer fail.
-        self.conn._db_file = "not_existent/" + TEST_DB_FILE
+    def test_do_xfrin_datasrc_error(self):
+        # Emulate failure in the data source client on commit.
+        self.conn._datasrc_client.force_fail = True
+        self.conn.response_generator = self._create_normal_response_data
         self.assertEqual(self.conn.do_xfrin(False), XFRIN_FAIL)
 
     def test_do_soacheck_and_xfrin(self):
@@ -1331,8 +1471,8 @@ class TestIXFRSession(TestXfrinConnection):
         self._create_broken_response_data()
         self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR()))
 
-class TestIXFRSessionWithSQLite3(TestXfrinConnection):
-    '''Tests for IXFR sessions using an SQLite3 DB.
+class TestXFRSessionWithSQLite3(TestXfrinConnection):
+    '''Tests for XFR sessions using an SQLite3 DB.
 
     These are provided mainly to confirm the implementation actually works
     in an environment closer to actual operational environments.  So we
@@ -1343,11 +1483,14 @@ class TestIXFRSessionWithSQLite3(TestXfrinConnection):
     def setUp(self):
         self.sqlite3db_src = TESTDATA_SRCDIR + '/example.com.sqlite3'
         self.sqlite3db_obj = TESTDATA_OBJDIR + '/example.com.sqlite3.copy'
+        self.sqlite3db_cfg = "{ \"database_file\": \"" +\
+                             self.sqlite3db_obj + "\"}"
         super().setUp()
         if os.path.exists(self.sqlite3db_obj):
             os.unlink(self.sqlite3db_obj)
         shutil.copyfile(self.sqlite3db_src, self.sqlite3db_obj)
-        self.conn._datasrc_client = DataSourceClient(self.sqlite3db_obj)
+        self.conn._datasrc_client = DataSourceClient("sqlite3",
+                                                     self.sqlite3db_cfg)
 
     def tearDown(self):
         if os.path.exists(self.sqlite3db_obj):
@@ -1368,7 +1511,7 @@ class TestIXFRSessionWithSQLite3(TestXfrinConnection):
         result, soa = finder.find(name, type, None, ZoneFinder.FIND_DEFAULT)
         return result == ZoneFinder.SUCCESS
 
-    def test_do_xfrin_sqlite3(self):
+    def test_do_ixfrin_sqlite3(self):
         def create_ixfr_response():
             self.conn.reply_data = self.conn.create_response_data(
                 questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
@@ -1381,7 +1524,7 @@ class TestIXFRSessionWithSQLite3(TestXfrinConnection):
         self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, RRType.IXFR()))
         self.assertEqual(1234, self.get_zone_serial())
 
-    def test_do_xfrin_sqlite3_fail(self):
+    def test_do_ixfrin_sqlite3_fail(self):
         '''Similar to the previous test, but xfrin fails due to error.
 
         Check the DB is not changed.
@@ -1399,47 +1542,91 @@ class TestIXFRSessionWithSQLite3(TestXfrinConnection):
         self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR()))
         self.assertEqual(1230, self.get_zone_serial())
 
-    def test_do_xfrin_axfr_sqlite3(self):
-        '''AXFR-style IXFR.
+    def test_do_ixfrin_nozone_sqlite3(self):
+        self.conn._zone_name = Name('nosuchzone.example')
+        self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR()))
+        # This should fail even before starting state transition
+        self.assertEqual(None, self.conn.get_xfrstate())
+
+    def axfr_check(self, type):
+        '''Common checks for AXFR and AXFR-style IXFR
 
         '''
-        def create_ixfr_response():
+        def create_response():
             self.conn.reply_data = self.conn.create_response_data(
-                questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
-                                    RRType.IXFR())],
+                questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, type)],
                 answers=[soa_rrset, self._create_ns(), soa_rrset])
-        self.conn.response_generator = create_ixfr_response
+        self.conn.response_generator = create_response
 
         # Confirm xfrin succeeds and SOA is updated, A RR is deleted.
         self.assertEqual(1230, self.get_zone_serial())
         self.assertTrue(self.record_exist(Name('dns01.example.com'),
                                           RRType.A()))
-        self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, RRType.IXFR()))
+        self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, type))
         self.assertEqual(1234, self.get_zone_serial())
         self.assertFalse(self.record_exist(Name('dns01.example.com'),
                                            RRType.A()))
 
-    def test_do_xfrin_axfr_sqlite3_fail(self):
-        '''Similar to the previous test, but xfrin fails due to error.
+    def test_do_ixfrin_axfr_sqlite3(self):
+        '''AXFR-style IXFR.
+
+        '''
+        self.axfr_check(RRType.IXFR())
+
+    def test_do_axfrin_sqlite3(self):
+        '''AXFR.
+
+        '''
+        self.axfr_check(RRType.AXFR())
+
+    def axfr_failure_check(self, type):
+        '''Similar to the previous two tests, but xfrin fails due to error.
 
         Check the DB is not changed.
 
         '''
-        def create_ixfr_response():
+        def create_response():
             self.conn.reply_data = self.conn.create_response_data(
-                questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
-                                    RRType.IXFR())],
+                questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, type)],
                 answers=[soa_rrset, self._create_ns(), soa_rrset, soa_rrset])
-        self.conn.response_generator = create_ixfr_response
+        self.conn.response_generator = create_response
 
         self.assertEqual(1230, self.get_zone_serial())
         self.assertTrue(self.record_exist(Name('dns01.example.com'),
                                           RRType.A()))
-        self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR()))
+        self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, type))
         self.assertEqual(1230, self.get_zone_serial())
         self.assertTrue(self.record_exist(Name('dns01.example.com'),
                                           RRType.A()))
 
+    def test_do_xfrin_axfr_sqlite3_fail(self):
+        '''Failure case for AXFR-style IXFR.
+
+        '''
+        self.axfr_failure_check(RRType.IXFR())
+
+    def test_do_axfrin_sqlite3_fail(self):
+        '''Failure case for AXFR.
+
+        '''
+        self.axfr_failure_check(RRType.AXFR())
+
+    def test_do_axfrin_nozone_sqlite3(self):
+        def create_response():
+            # Within this test, owner names of the question/RRs don't matter,
+            # so we use pre-defined names (which are "out of zone") for
+            # simplicity.
+            self.conn.reply_data = self.conn.create_response_data(
+                questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                    RRType.AXFR())],
+                answers=[soa_rrset, self._create_ns(), soa_rrset, soa_rrset])
+        self.conn.response_generator = create_response
+        self.conn._zone_name = Name('nosuchzone.example')
+        self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.AXFR()))
+        # This should fail in the FirstData state
+        self.assertEqual(type(XfrinFirstData()),
+                         type(self.conn.get_xfrstate()))
+
 class TestXfrinRecorder(unittest.TestCase):
     def setUp(self):
         self.recorder = XfrinRecorder()

src/bin/xfrin/xfrin.py.in

@@ -148,8 +148,8 @@ class XfrinState:
     IXFR/AXFR response begins with an SOA).  When it reaches IXFREnd
     or AXFREnd, the process successfully completes.
 
-
-            (recv SOA)       (AXFR-style IXFR)   (SOA, add)
+                             (AXFR or
+            (recv SOA)        AXFR-style IXFR)  (SOA, add)
     InitialSOA------->FirstData------------->AXFR--------->AXFREnd
                           |                  |  ^         (post xfr
                           |                  |  |        checks, then
@@ -321,7 +321,7 @@ class XfrinFirstData(XfrinState):
         else:
             logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_NONINCREMENTAL_RESP,
                  conn.zone_str())
-            # We are now goint to add RRs to the new zone.  We need create
+            # We are now going to add RRs to the new zone.  We need create
             # a Diff object.  It will be used throughtout the XFR session.
             conn._diff = Diff(conn._datasrc_client, conn._zone_name, True)
             self.set_xfrstate(conn, XfrinAXFR())
@@ -405,6 +405,12 @@ class XfrinAXFR(XfrinState):
         if rr.get_type() == RRType.SOA():
             # SOA means end.  Don't commit it yet - we need to perform
             # post-transfer checks
+
+            soa_serial = get_soa_serial(rr.get_rdata()[0])
+            if conn._end_serial != soa_serial:
+                logger.warn(XFRIN_AXFR_INCONSISTENT_SOA, conn.zone_str(),
+                            conn._end_serial, soa_serial)
+
             self.set_xfrstate(conn, XfrinAXFREnd())
         # Yes, we've eaten this RR.
         return True
@@ -432,15 +438,14 @@ class XfrinConnection(asyncore.dispatcher):
     '''Do xfrin in this class. '''
 
     def __init__(self,
-                 sock_map, zone_name, rrclass, datasrc_client, db_file,
-                 shutdown_event, master_addrinfo, tsig_key = None,
-                 verbose=False, idle_timeout=60):
-        '''Constructor of the XfrinConnection class.
+                 sock_map, zone_name, rrclass, datasrc_client,
+                 shutdown_event, master_addrinfo, tsig_key=None,
+                 idle_timeout=60):
+        '''Constructor of the XfirnConnection class.
 
         idle_timeout: max idle time for read data from socket.
         datasrc_client: the data source client object used for the XFR session.
                         This will eventually replace db_file completely.
-        db_file: specify the data source file (should soon be deprecated).
 
         '''
 
@@ -460,8 +465,7 @@ class XfrinConnection(asyncore.dispatcher):
         self._zone_name = zone_name
         self._rrclass = rrclass
 
-        # Data source handlers
-        self._db_file = db_file # temporary for sqlite3 specific code
+        # Data source handler
         self._datasrc_client = datasrc_client
 
         self.create_socket(master_addrinfo[0], master_addrinfo[1])
@@ -470,7 +474,6 @@ class XfrinConnection(asyncore.dispatcher):
         self._idle_timeout = idle_timeout
         self.setblocking(1)
         self._shutdown_event = shutdown_event
-        self._verbose = verbose
         self._master_address = master_addrinfo[2]
         self._tsig_key = tsig_key
         self._tsig_ctx = None
@@ -648,16 +651,9 @@ class XfrinConnection(asyncore.dispatcher):
             if ret == XFRIN_OK:
                 logger.info(XFRIN_XFR_TRANSFER_STARTED, request_str,
                             self.zone_str())
-                if self._request_type == RRType.IXFR():
-                    self._request_type = RRType.IXFR()
-                    self._send_query(self._request_type)
-                    self.__state = XfrinInitialSOA()
-                    self._handle_xfrin_responses()
-                else:
-                    self._send_query(self._request_type)
-                    isc.datasrc.sqlite3_ds.load(self._db_file,
-                                                self._zone_name.to_text(),
-                                                self._handle_axfrin_response)
+                self._send_query(self._request_type)
+                self.__state = XfrinInitialSOA()
+                self._handle_xfrin_responses()
                 logger.info(XFRIN_XFR_TRANSFER_SUCCESS, request_str,
                             self.zone_str())
 
@@ -665,11 +661,6 @@ class XfrinConnection(asyncore.dispatcher):
             logger.error(XFRIN_XFR_TRANSFER_FAILURE, request_str,
                          self.zone_str(), str(e))
             ret = XFRIN_FAIL
-        except isc.datasrc.sqlite3_ds.Sqlite3DSError as e:
-            # Note: this is old code and used only for AXFR.  This will be
-            # soon removed anyway, so we'll leave it.
-            logger.error(XFRIN_AXFR_DATABASE_FAILURE, self.zone_str(), str(e))
-            ret = XFRIN_FAIL
         except Exception as e:
             # Catching all possible exceptions like this is generally not a
             # good practice, but handling an xfr session could result in
@@ -717,9 +708,6 @@ class XfrinConnection(asyncore.dispatcher):
 
         self._check_response_header(msg)
 
-        if msg.get_rr_count(Message.SECTION_ANSWER) == 0:
-            raise XfrinException('answer section is empty')
-
         if msg.get_rr_count(Message.SECTION_QUESTION) > 1:
             raise XfrinException('query section count greater than 1')
 
@@ -775,31 +763,6 @@ class XfrinConnection(asyncore.dispatcher):
             if self._shutdown_event.is_set():
                 raise XfrinException('xfrin is forced to stop')
 
-    def _handle_axfrin_response(self):
-        '''Return a generator for the response to a zone transfer. '''
-        while True:
-            data_len = self._get_request_response(2)
-            msg_len = socket.htons(struct.unpack('H', data_len)[0])
-            recvdata = self._get_request_response(msg_len)
-            msg = Message(Message.PARSE)
-            msg.from_wire(recvdata)
-
-            # TSIG related checks, including an unexpected signed response
-            self._check_response_tsig(msg, recvdata)
-
-            # Perform response status validation
-            self._check_response_status(msg)
-
-            answer_section = msg.get_section(Message.SECTION_ANSWER)
-            for rr in self._handle_answer_section(answer_section):
-                yield rr
-
-            if self._soa_rr_count == 2:
-                break
-
-            if self._shutdown_event.is_set():
-                raise XfrinException('xfrin is forced to stop')
-
     def handle_read(self):
         '''Read query's response from socket. '''
 
@@ -817,8 +780,8 @@ class XfrinConnection(asyncore.dispatcher):
         pass
 
 def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file,
-                  shutdown_event, master_addrinfo, check_soa, verbose,
-                  tsig_key, request_type):
+                  shutdown_event, master_addrinfo, check_soa, tsig_key,
+                  request_type):
     xfrin_recorder.increment(zone_name)
 
     # Create a data source client used in this XFR session.  Right now we
@@ -829,13 +792,17 @@ def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file,
     # this code will be much cleaner.
     datasrc_client = None
     if db_file is not None:
-        datasrc_client = DataSourceClient(db_file)
+        # temporary hardcoded sqlite initialization. Once we decide on
+        # the config specification, we need to update this (TODO)
+        # this may depend on #1207, or any followup ticket created for #1207
+        datasrc_type = "sqlite3"
+        datasrc_config = "{ \"database_file\": \"" + db_file + "\"}"
+        datasrc_client = DataSourceClient(datasrc_type, datasrc_config)
 
     # Create a TCP connection for the XFR session and perform the operation.
     sock_map = {}
     conn = XfrinConnection(sock_map, zone_name, rrclass, datasrc_client,
-                           db_file, shutdown_event, master_addrinfo,
-                           tsig_key, verbose)
+                           shutdown_event, master_addrinfo, tsig_key)
     ret = XFRIN_FAIL
     if conn.connect_to_master():
         ret = conn.do_xfrin(check_soa, request_type)
@@ -977,13 +944,12 @@ class ZoneInfo:
                 (str(self.master_addr), self.master_port))
 
 class Xfrin:
-    def __init__(self, verbose = False):
+    def __init__(self):
         self._max_transfers_in = 10
         self._zones = {}
         self._cc_setup()
         self.recorder = XfrinRecorder()
         self._shutdown_event = threading.Event()
-        self._verbose = verbose
 
     def _cc_setup(self):
         '''This method is used only as part of initialization, but is
@@ -1243,7 +1209,6 @@ class Xfrin:
                                                 db_file,
                                                 self._shutdown_event,
                                                 master_addrinfo, check_soa,
-                                                self._verbose,
                                                 tsig_key, request_type))
 
         xfrin_thread.start()
@@ -1263,9 +1228,9 @@ def set_signal_handler():
 
 def set_cmd_options(parser):
     parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
-            help="display more about what is going on")
+            help="This option is obsolete and has no effect.")
 
-def main(xfrin_class, use_signal = True):
+def main(xfrin_class, use_signal=True):
     """The main loop of the Xfrin daemon.
 
     @param xfrin_class: A class of the Xfrin object.  This is normally Xfrin,
@@ -1282,7 +1247,7 @@ def main(xfrin_class, use_signal = True):
 
         if use_signal:
             set_signal_handler()
-        xfrind = xfrin_class(verbose = options.verbose)
+        xfrind = xfrin_class()
         xfrind.startup()
     except KeyboardInterrupt:
         logger.info(XFRIN_STOPPED_BY_KEYBOARD)

src/bin/xfrin/xfrin_messages.mes

@@ -98,4 +98,24 @@ differences) was found.  This means a connection for xfrin tried IXFR
 and really aot a response for incremental updates.
 
 % XFRIN_GOT_NONINCREMENTAL_RESP got nonincremental response for %1
-TBD
+Non incremental transfer was detected at the "first data" of a transfer,
+which is the RR following the initial SOA.  Non incremental transfer is
+either AXFR or AXFR-style IXFR.  In the latter case, it means that
+in a response to IXFR query the first data is not SOA or its SOA serial
+is not equal to the requested SOA serial.
+
+% XFRIN_AXFR_INCONSISTENT_SOA AXFR SOAs are inconsistent for %1: %2 expected, %3 received
+The serial fields of the first and last SOAs of AXFR (including AXFR-style
+IXFR) are not the same.  According to RFC 5936 these two SOAs must be the
+"same" (not only for the serial), but it is still not clear what the
+receiver should do if this condition does not hold.  There was a discussion
+about this at the IETF dnsext wg:
+http://www.ietf.org/mail-archive/web/dnsext/current/msg07908.html
+and the general feeling seems that it would be better to reject the
+transfer if a mismatch is detected.  On the other hand, also as noted
+in that email thread, neither BIND 9 nor NSD performs any comparison
+on the SOAs.  For now, we only check the serials (ignoring other fields)
+and only leave a warning log message when a mismatch is found.  If it
+turns out to happen with a real world primary server implementation
+and that server actually feeds broken data (e.g. mixed versions of
+zone), we can consider a stricter action.

src/lib/datasrc/Makefile.am

@@ -22,15 +22,19 @@ libdatasrc_la_SOURCES += result.h
 libdatasrc_la_SOURCES += logger.h logger.cc
 libdatasrc_la_SOURCES += client.h iterator.h
 libdatasrc_la_SOURCES += database.h database.cc
-#libdatasrc_la_SOURCES += sqlite3_accessor.h sqlite3_accessor.cc
 libdatasrc_la_SOURCES += factory.h factory.cc
 nodist_libdatasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 
 sqlite3_ds_la_SOURCES = sqlite3_accessor.h sqlite3_accessor.cc
 sqlite3_ds_la_LDFLAGS = -module
+sqlite3_ds_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
+sqlite3_ds_la_LIBADD += libdatasrc.la
+sqlite3_ds_la_LIBADD += $(SQLITE_LIBS)
 
 memory_ds_la_SOURCES = memory_datasrc.h memory_datasrc.cc
 memory_ds_la_LDFLAGS = -module
+memory_ds_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
+memory_ds_la_LIBADD += libdatasrc.la
 
 libdatasrc_la_LIBADD = $(top_builddir)/src/lib/exceptions/libexceptions.la
 libdatasrc_la_LIBADD += $(top_builddir)/src/lib/dns/libdns++.la

src/lib/datasrc/factory.cc

@@ -30,7 +30,9 @@ namespace isc {
 namespace datasrc {
 
 LibraryContainer::LibraryContainer(const std::string& name) {
-    ds_lib_ = dlopen(name.c_str(), RTLD_NOW | RTLD_LOCAL);
+    // use RTLD_GLOBAL so that shared symbols (e.g. exceptions)
+    // are recognized as such
+    ds_lib_ = dlopen(name.c_str(), RTLD_NOW | RTLD_GLOBAL);
     if (ds_lib_ == NULL) {
         isc_throw(DataSourceLibraryError, dlerror());
     }
@@ -70,7 +72,18 @@ DataSourceClientContainer::DataSourceClientContainer(const std::string& type,
     ds_creator* ds_create = (ds_creator*)ds_lib_.getSym("createInstance");
     destructor_ = (ds_destructor*)ds_lib_.getSym("destroyInstance");
 
-    instance_ = ds_create(config);
+    std::string error;
+    try {
+        instance_ = ds_create(config, error);
+        if (instance_ == NULL) {
+            isc_throw(DataSourceError, error);
+        }
+    } catch (const std::exception& exc) {
+        isc_throw(DataSourceError, "Unknown uncaught exception from " + type +
+                                   " createInstance: " + exc.what());
+    } catch (...) {
+        isc_throw(DataSourceError, "Unknown uncaught exception from " + type);
+    }
 }
 
 DataSourceClientContainer::~DataSourceClientContainer() {

src/lib/datasrc/factory.h

@@ -44,21 +44,8 @@ public:
         DataSourceError(file, line, what) {}
 };
 
-/// \brief Raised if the given config contains bad data
-///
-/// Depending on the datasource type, the configuration may differ (for
-/// instance, the sqlite3 datasource needs a database file).
-class DataSourceConfigError : public DataSourceError {
-public:
-    DataSourceConfigError(const char* file, size_t line, const char* what) :
-        DataSourceError(file, line, what) {}
-    // This exception is created in the dynamic modules. Apparently
-    // sunstudio can't handle it if we then automatically derive the
-    // destructor, so we provide it explicitely
-    ~DataSourceConfigError() throw() {}
-};
-
-typedef DataSourceClient* ds_creator(isc::data::ConstElementPtr config);
+typedef DataSourceClient* ds_creator(isc::data::ConstElementPtr config,
+                                     std::string& error);
 typedef void ds_destructor(DataSourceClient* instance);
 
 /// \brief Container class for dynamically loaded libraries
@@ -146,8 +133,9 @@ public:
     ///            backend library
     /// \exception DataSourceLibrarySymbolError if the library does not have
     ///            the needed symbols, or if there is an error reading them
-    /// \exception DataSourceConfigError if the given config is not correct
-    ///            for the given type
+    /// \exception DataError if the given config is not correct
+    ///            for the given type, or if there was a problem during
+    ///            initialization
     ///
     /// \param type The type of the datasource client. Based on the value of
     ///             type, a specific backend library is used, by appending the

src/lib/datasrc/memory_datasrc.cc

@@ -931,12 +931,22 @@ checkConfig(ConstElementPtr config, ElementPtr errors) {
 } // end anonymous namespace
 
 DataSourceClient *
-createInstance(isc::data::ConstElementPtr config) {
+createInstance(isc::data::ConstElementPtr config, std::string& error) {
     ElementPtr errors(Element::createList());
     if (!checkConfig(config, errors)) {
-        isc_throw(DataSourceConfigError, errors->str());
+        error = "Configuration error: " + errors->str();
+        return (NULL);
+    }
+    try {
+        return (new InMemoryClient());
+    } catch (const std::exception& exc) {
+        error = std::string("Error creating memory datasource: ") + exc.what();
+        return (NULL);
+    } catch (...) {
+        error = std::string("Error creating memory datasource, "
+                            "unknown exception");
+        return (NULL);
     }
-    return (new InMemoryClient());
 }
 
 void destroyInstance(DataSourceClient* instance) {

src/lib/datasrc/memory_datasrc.h

@@ -310,7 +310,14 @@ private:
 ///
 /// This configuration setup is currently under discussion and will change in
 /// the near future.
-extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config);
+///
+/// \param config The configuration for the datasource instance
+/// \param error This string will be set to an error message if an error occurs
+///              during initialization
+/// \return An instance of the memory datasource client, or NULL if there was
+///         an error
+extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config,
+                                            std::string& error);
 
 /// \brief Destroy the instance created by createInstance()
 extern "C" void destroyInstance(DataSourceClient* instance);

src/lib/datasrc/sqlite3_accessor.cc

@@ -751,15 +751,25 @@ checkConfig(ConstElementPtr config, ElementPtr errors) {
 } // end anonymous namespace
 
 DataSourceClient *
-createInstance(isc::data::ConstElementPtr config) {
+createInstance(isc::data::ConstElementPtr config, std::string& error) {
     ElementPtr errors(Element::createList());
     if (!checkConfig(config, errors)) {
-        isc_throw(DataSourceConfigError, errors->str());
+        error = "Configuration error: " + errors->str();
+        return (NULL);
     }
     std::string dbfile = config->get(CONFIG_ITEM_DATABASE_FILE)->stringValue();
-    boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
-        new SQLite3Accessor(dbfile, "IN")); // XXX: avoid hardcode RR class
-    return (new DatabaseClient(isc::dns::RRClass::IN(), sqlite3_accessor));
+    try {
+        boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
+            new SQLite3Accessor(dbfile, "IN")); // XXX: avoid hardcode RR class
+        return (new DatabaseClient(isc::dns::RRClass::IN(), sqlite3_accessor));
+    } catch (const std::exception& exc) {
+        error = std::string("Error creating sqlite3 datasource: ") + exc.what();
+        return (NULL);
+    } catch (...) {
+        error = std::string("Error creating sqlite3 datasource, "
+                            "unknown exception");
+        return (NULL);
+    }
 }
 
 void destroyInstance(DataSourceClient* instance) {

src/lib/datasrc/sqlite3_accessor.h

@@ -190,7 +190,14 @@ private:
 ///
 /// This configuration setup is currently under discussion and will change in
 /// the near future.
-extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config);
+///
+/// \param config The configuration for the datasource instance
+/// \param error This string will be set to an error message if an error occurs
+///              during initialization
+/// \return An instance of the sqlite3 datasource client, or NULL if there was
+///         an error
+extern "C" DataSourceClient* createInstance(isc::data::ConstElementPtr config,
+                                            std::string& error);
 
 /// \brief Destroy the instance created by createInstance()
 extern "C" void destroyInstance(DataSourceClient* instance);

src/lib/datasrc/tests/factory_unittest.cc

@@ -37,43 +37,43 @@ TEST(FactoryTest, sqlite3ClientBadConfig) {
     // tests are left to the implementation-specific backends)
     ElementPtr config;
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config = Element::create("asdf");
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config = Element::createMap();
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create("FOO"));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create("IN"));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("database_file", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("database_file", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("database_file", Element::create("/foo/bar/doesnotexist"));
     ASSERT_THROW(DataSourceClientContainer("sqlite3", config),
-                 SQLite3Error);
+                 DataSourceError);
 
     config->set("database_file", Element::create(SQLITE_DBFILE_EXAMPLE_ORG));
     DataSourceClientContainer dsc("sqlite3", config);
@@ -100,55 +100,55 @@ TEST(FactoryTest, memoryClient) {
     // tests are left to the implementation-specific backends)
     ElementPtr config;
     ASSERT_THROW(DataSourceClientContainer client("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config = Element::create("asdf");
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config = Element::createMap();
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("type", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("type", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("type", Element::create("FOO"));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("type", Element::create("memory"));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create("FOO"));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("class", Element::create("IN"));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("zones", ElementPtr());
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("zones", Element::create(1));
     ASSERT_THROW(DataSourceClientContainer("memory", config),
-                 DataSourceConfigError);
+                 DataSourceError);
 
     config->set("zones", Element::createList());
     DataSourceClientContainer dsc("memory", config);

src/lib/python/isc/datasrc/Makefile.am

@@ -16,12 +16,6 @@ datasrc_la_SOURCES += client_python.cc client_python.h
 datasrc_la_SOURCES += iterator_python.cc iterator_python.h
 datasrc_la_SOURCES += finder_python.cc finder_python.h
 datasrc_la_SOURCES += updater_python.cc updater_python.h
-# This is a temporary workaround for #1206, where the InMemoryClient has been
-# moved to an ldopened library. We could add that library to LDADD, but that
-# is nonportable. When #1207 is done this becomes moot anyway, and the
-# specific workaround is not needed anymore, so we can then remove this
-# line again.
-datasrc_la_SOURCES += ${top_srcdir}/src/lib/datasrc/sqlite3_accessor.cc
 
 datasrc_la_CPPFLAGS = $(AM_CPPFLAGS) $(PYTHON_INCLUDES)
 datasrc_la_CXXFLAGS = $(AM_CXXFLAGS) $(PYTHON_CXXFLAGS)
@@ -30,7 +24,6 @@ datasrc_la_LDFLAGS += -module
 datasrc_la_LIBADD = $(top_builddir)/src/lib/datasrc/libdatasrc.la
 datasrc_la_LIBADD += $(top_builddir)/src/lib/dns/python/libpydnspp.la
 datasrc_la_LIBADD += $(PYTHON_LIB)
-#datasrc_la_LIBADD += $(SQLITE_LIBS)
 
 EXTRA_DIST = client_inc.cc
 EXTRA_DIST += finder_inc.cc

src/lib/python/isc/datasrc/client_inc.cc

@@ -7,7 +7,20 @@ This is the python wrapper for the abstract base class that defines\n\
 the common interface for various types of data source clients. A data\n\
 source client is a top level access point to a data source, allowing \n\
 various operations on the data source such as lookups, traversing or \n\
-updates. The client class itself has limited focus and delegates \n\
+updates.\n\
+This class serves as both the factory and the main interface to those \n\
+classes.\n\
+\n\
+The constructor takes two arguments; a type (string), and\n\
+configuration data for a datasource client of that type. The configuration\n\
+data is currently passed as a JSON in string form, and its contents depend\n\
+on the type of datasource from the first argument. For instance, a\n\
+datasource of type \"sqlite3\" takes the config \n\
+{ \"database_file\": \"/var/example.org\" }\n\
+We may in the future add support for passing configuration data,\n\
+but right now we limit it to a JSON-formatted string\n\
+\n\
+The client class itself has limited focus and delegates \n\
 the responsibility for these specific operations to other (c++) classes;\n\
 in general methods of this class act as factories of these other classes.\n\
 \n\
@@ -110,7 +123,7 @@ Return an updater to make updates to a specific zone.\n\
 The RR class of the zone is the one that the client is expected to\n\
 handle (see the detailed description of this class).\n\
 \n\
-If the specified zone is not found via the client, a NULL pointer will\n\
+If the specified zone is not found via the client, a None object will\n\
 be returned; in other words a completely new zone cannot be created\n\
 using an updater. It must be created beforehand (even if it's an empty\n\
 placeholder) in a way specific to the underlying data source.\n\

src/lib/python/isc/datasrc/client_python.cc

@@ -23,6 +23,7 @@
 #include <util/python/pycppwrapper_util.h>
 
 #include <datasrc/client.h>
+#include <datasrc/factory.h>
 #include <datasrc/database.h>
 #include <datasrc/data_source.h>
 #include <datasrc/sqlite3_accessor.h>
@@ -50,13 +51,9 @@ namespace {
 class s_DataSourceClient : public PyObject {
 public:
     s_DataSourceClient() : cppobj(NULL) {};
-    DataSourceClient* cppobj;
+    DataSourceClientContainer* cppobj;
 };
 
-// Shortcut type which would be convenient for adding class variables safely.
-typedef CPPPyObjectContainer<s_DataSourceClient, DataSourceClient>
-    DataSourceClientContainer;
-
 PyObject*
 DataSourceClient_findZone(PyObject* po_self, PyObject* args) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
@@ -64,12 +61,12 @@ DataSourceClient_findZone(PyObject* po_self, PyObject* args) {
     if (PyArg_ParseTuple(args, "O!", &name_type, &name)) {
         try {
             DataSourceClient::FindResult find_result(
-                self->cppobj->findZone(PyName_ToName(name)));
+                self->cppobj->getInstance().findZone(PyName_ToName(name)));
 
             result::Result r = find_result.code;
             ZoneFinderPtr zfp = find_result.zone_finder;
             // Use N instead of O so refcount isn't increased twice
-            return (Py_BuildValue("IN", r, createZoneFinderObject(zfp)));
+            return (Py_BuildValue("IN", r, createZoneFinderObject(zfp, po_self)));
         } catch (const std::exception& exc) {
             PyErr_SetString(getDataSourceException("Error"), exc.what());
             return (NULL);
@@ -90,7 +87,8 @@ DataSourceClient_getIterator(PyObject* po_self, PyObject* args) {
     if (PyArg_ParseTuple(args, "O!", &name_type, &name_obj)) {
         try {
             return (createZoneIteratorObject(
-                        self->cppobj->getIterator(PyName_ToName(name_obj))));
+                self->cppobj->getInstance().getIterator(PyName_ToName(name_obj)),
+                po_self));
         } catch (const isc::NotImplemented& ne) {
             PyErr_SetString(getDataSourceException("NotImplemented"),
                             ne.what());
@@ -120,9 +118,13 @@ DataSourceClient_getUpdater(PyObject* po_self, PyObject* args) {
         PyBool_Check(replace_obj)) {
         bool replace = (replace_obj != Py_False);
         try {
-            return (createZoneUpdaterObject(
-                        self->cppobj->getUpdater(PyName_ToName(name_obj),
-                                                 replace)));
+            ZoneUpdaterPtr updater =
+                self->cppobj->getInstance().getUpdater(PyName_ToName(name_obj),
+                                                       replace);
+            if (!updater) {
+                return (Py_None);
+            }
+            return (createZoneUpdaterObject(updater, po_self));
         } catch (const isc::NotImplemented& ne) {
             PyErr_SetString(getDataSourceException("NotImplemented"),
                             ne.what());
@@ -162,23 +164,33 @@ PyMethodDef DataSourceClient_methods[] = {
 
 int
 DataSourceClient_init(s_DataSourceClient* self, PyObject* args) {
-    // TODO: we should use the factory function which hasn't been written
-    // yet. For now we hardcode the sqlite3 initialization, and pass it one
-    // string for the database file. (similar to how the 'old direct'
-    // sqlite3_ds code works).  Of course, we shouldn't hardcode the RR class
-    // at that point.
+    char* ds_type_str;
+    char* ds_config_str;
     try {
-        char* db_file_name;
-        if (PyArg_ParseTuple(args, "s", &db_file_name)) {
-            boost::shared_ptr<DatabaseAccessor> sqlite3_accessor(
-                new SQLite3Accessor(db_file_name, "IN"));
-            self->cppobj = new DatabaseClient(isc::dns::RRClass::IN(),
-                                              sqlite3_accessor);
+        // Turn the given argument into config Element; then simply call
+        // factory class to do its magic
+
+        // for now, ds_config must be JSON string
+        if (PyArg_ParseTuple(args, "ss", &ds_type_str, &ds_config_str)) {
+            isc::data::ConstElementPtr ds_config =
+                isc::data::Element::fromJSON(ds_config_str);
+            self->cppobj = new DataSourceClientContainer(ds_type_str,
+                                                         ds_config);
             return (0);
         } else {
             return (-1);
         }
-
+    } catch (const isc::data::JSONError& je) {
+        const string ex_what = "JSON parse error in data source configuration "
+                               "data for type " +
+                               string(ds_type_str) + ":" + je.what();
+        PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
+        return (-1);
+    } catch (const DataSourceError& dse) {
+        const string ex_what = "Failed to create DataSourceClient of type " +
+                               string(ds_type_str) + ":" + dse.what();
+        PyErr_SetString(getDataSourceException("Error"), ex_what.c_str());
+        return (-1);
     } catch (const exception& ex) {
         const string ex_what = "Failed to construct DataSourceClient object: " +
             string(ex.what());

src/lib/python/isc/datasrc/datasrc.cc

@@ -77,14 +77,26 @@ initModulePart_DataSourceClient(PyObject* mod) {
     }
     Py_INCREF(&datasourceclient_type);
 
-    addClassVariable(datasourceclient_type, "SUCCESS",
-                     Py_BuildValue("I", result::SUCCESS));
-    addClassVariable(datasourceclient_type, "EXIST",
-                     Py_BuildValue("I", result::EXIST));
-    addClassVariable(datasourceclient_type, "NOTFOUND",
-                     Py_BuildValue("I", result::NOTFOUND));
-    addClassVariable(datasourceclient_type, "PARTIALMATCH",
-                     Py_BuildValue("I", result::PARTIALMATCH));
+    try {
+        installClassVariable(datasourceclient_type, "SUCCESS",
+                             Py_BuildValue("I", result::SUCCESS));
+        installClassVariable(datasourceclient_type, "EXIST",
+                             Py_BuildValue("I", result::EXIST));
+        installClassVariable(datasourceclient_type, "NOTFOUND",
+                             Py_BuildValue("I", result::NOTFOUND));
+        installClassVariable(datasourceclient_type, "PARTIALMATCH",
+                             Py_BuildValue("I", result::PARTIALMATCH));
+    } catch (const std::exception& ex) {
+        const std::string ex_what =
+            "Unexpected failure in DataSourceClient initialization: " +
+            std::string(ex.what());
+        PyErr_SetString(po_IscException, ex_what.c_str());
+        return (false);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError,
+            "Unexpected failure in DataSourceClient initialization");
+        return (false);
+    }
 
     return (true);
 }
@@ -103,26 +115,41 @@ initModulePart_ZoneFinder(PyObject* mod) {
     }
     Py_INCREF(&zonefinder_type);
 
-    addClassVariable(zonefinder_type, "SUCCESS",
-                     Py_BuildValue("I", ZoneFinder::SUCCESS));
-    addClassVariable(zonefinder_type, "DELEGATION",
-                     Py_BuildValue("I", ZoneFinder::DELEGATION));
-    addClassVariable(zonefinder_type, "NXDOMAIN",
-                     Py_BuildValue("I", ZoneFinder::NXDOMAIN));
-    addClassVariable(zonefinder_type, "NXRRSET",
-                     Py_BuildValue("I", ZoneFinder::NXRRSET));
-    addClassVariable(zonefinder_type, "CNAME",
-                     Py_BuildValue("I", ZoneFinder::CNAME));
-    addClassVariable(zonefinder_type, "DNAME",
-                     Py_BuildValue("I", ZoneFinder::DNAME));
-
-    addClassVariable(zonefinder_type, "FIND_DEFAULT",
-                     Py_BuildValue("I", ZoneFinder::FIND_DEFAULT));
-    addClassVariable(zonefinder_type, "FIND_GLUE_OK",
-                     Py_BuildValue("I", ZoneFinder::FIND_GLUE_OK));
-    addClassVariable(zonefinder_type, "FIND_DNSSEC",
-                     Py_BuildValue("I", ZoneFinder::FIND_DNSSEC));
-
+    try {
+        installClassVariable(zonefinder_type, "SUCCESS",
+                             Py_BuildValue("I", ZoneFinder::SUCCESS));
+        installClassVariable(zonefinder_type, "DELEGATION",
+                             Py_BuildValue("I", ZoneFinder::DELEGATION));
+        installClassVariable(zonefinder_type, "NXDOMAIN",
+                             Py_BuildValue("I", ZoneFinder::NXDOMAIN));
+        installClassVariable(zonefinder_type, "NXRRSET",
+                             Py_BuildValue("I", ZoneFinder::NXRRSET));
+        installClassVariable(zonefinder_type, "CNAME",
+                             Py_BuildValue("I", ZoneFinder::CNAME));
+        installClassVariable(zonefinder_type, "DNAME",
+                             Py_BuildValue("I", ZoneFinder::DNAME));
+        installClassVariable(zonefinder_type, "WILDCARD",
+                             Py_BuildValue("I", ZoneFinder::WILDCARD));
+        installClassVariable(zonefinder_type, "WILDCARD_NXRRSET",
+                             Py_BuildValue("I", ZoneFinder::WILDCARD_NXRRSET));
+
+        installClassVariable(zonefinder_type, "FIND_DEFAULT",
+                             Py_BuildValue("I", ZoneFinder::FIND_DEFAULT));
+        installClassVariable(zonefinder_type, "FIND_GLUE_OK",
+                             Py_BuildValue("I", ZoneFinder::FIND_GLUE_OK));
+        installClassVariable(zonefinder_type, "FIND_DNSSEC",
+                             Py_BuildValue("I", ZoneFinder::FIND_DNSSEC));
+    } catch (const std::exception& ex) {
+        const std::string ex_what =
+            "Unexpected failure in ZoneFinder initialization: " +
+            std::string(ex.what());
+        PyErr_SetString(po_IscException, ex_what.c_str());
+        return (false);
+    } catch (...) {
+        PyErr_SetString(PyExc_SystemError,
+                        "Unexpected failure in ZoneFinder initialization");
+        return (false);
+    }
 
     return (true);
 }

src/lib/python/isc/datasrc/finder_inc.cc

@@ -62,6 +62,10 @@ Search the zone for a given pair of domain name and RR type.\n\
   and the code of SUCCESS will be returned.\n\
 - If the search name matches a delegation point of DNAME, it returns\n\
   the code of DNAME and that DNAME RR.\n\
+- If the result was synthesized by a wildcard match, it returns the\n\
+  code WILDCARD and the synthesized RRset\n\
+- If the query matched a wildcard name, but not its type, it returns the\n\
+  code WILDCARD_NXRRSET, and None\n\
 - If the target is a list, all RRsets under the domain are inserted\n\
   there and SUCCESS (or NXDOMAIN, in case of empty domain) is returned\n\
   instead of normall processing. This is intended to handle ANY query.\n\
@@ -93,4 +97,22 @@ Parameters:\n\
 Return Value(s): A tuple of a result code an a FindResult object enclosing\n\
 the search result (see above).\n\
 ";
+
+const char* const ZoneFinder_find_previous_name_doc = "\
+find_previous_name(isc.dns.Name) -> isc.dns.Name\n\
+\n\
+Gets the previous name in the DNSSEC order. This can be used\n\
+to find the correct NSEC records for proving nonexistence\n\
+of domains.\n\
+\n\
+This method does not include under-zone-cut data (glue data).\n\
+\n\
+Raises isc.datasrc.NotImplemented in case the data source backend\n\
+doesn't support DNSSEC or there is no previous in the zone (NSEC\n\
+records might be missing in the DB, the queried name is less or\n\
+equal to the apex).\n\
+\n\
+Raises isc.datasrc.Error for low-level or internal datasource errors\n\
+(like broken connection to database, wrong data living there).\n\
+";
 } // unnamed namespace

src/lib/python/isc/datasrc/finder_python.cc

@@ -103,8 +103,14 @@ namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_ZoneFinder : public PyObject {
 public:
-    s_ZoneFinder() : cppobj(ZoneFinderPtr()) {};
+    s_ZoneFinder() : cppobj(ZoneFinderPtr()), base_obj(NULL) {};
     ZoneFinderPtr cppobj;
+    // This is a reference to a base object; if the object of this class
+    // depends on another object to be in scope during its lifetime,
+    // we use INCREF the base object upon creation, and DECREF it at
+    // the end of the destructor
+    // This is an optional argument to createXXX(). If NULL, it is ignored.
+    PyObject* base_obj;
 };
 
 // Shortcut type which would be convenient for adding class variables safely.
@@ -125,6 +131,9 @@ ZoneFinder_destroy(s_ZoneFinder* const self) {
     // cppobj is a shared ptr, but to make sure things are not destroyed in
     // the wrong order, we reset it here.
     self->cppobj.reset();
+    if (self->base_obj != NULL) {
+        Py_DECREF(self->base_obj);
+    }
     Py_TYPE(self)->tp_free(self);
 }
 
@@ -160,6 +169,31 @@ ZoneFinder_find(PyObject* po_self, PyObject* args) {
     return (isc_datasrc_internal::ZoneFinder_helper(self->cppobj.get(), args));
 }
 
+PyObject*
+ZoneFinder_findPreviousName(PyObject* po_self, PyObject* args) {
+    s_ZoneFinder* const self = static_cast<s_ZoneFinder*>(po_self);
+    PyObject* name_obj;
+    if (PyArg_ParseTuple(args, "O!", &name_type, &name_obj)) {
+        try {
+            return (createNameObject(
+                self->cppobj->findPreviousName(PyName_ToName(name_obj))));
+        } catch (const isc::NotImplemented& nie) {
+            PyErr_SetString(getDataSourceException("NotImplemented"),
+                            nie.what());
+            return (NULL);
+        } catch (const std::exception& exc) {
+            PyErr_SetString(getDataSourceException("Error"), exc.what());
+            return (NULL);
+        } catch (...) {
+            PyErr_SetString(getDataSourceException("Error"),
+                            "Unexpected exception");
+            return (NULL);
+        }
+    } else {
+        return (NULL);
+    }
+}
+
 // This list contains the actual set of functions we have in
 // python. Each entry has
 // 1. Python method name
@@ -167,12 +201,12 @@ ZoneFinder_find(PyObject* po_self, PyObject* args) {
 // 3. Argument type
 // 4. Documentation
 PyMethodDef ZoneFinder_methods[] = {
-    { "get_origin", reinterpret_cast<PyCFunction>(ZoneFinder_getOrigin),
-      METH_NOARGS, ZoneFinder_getOrigin_doc },
-    { "get_class", reinterpret_cast<PyCFunction>(ZoneFinder_getClass),
-      METH_NOARGS, ZoneFinder_getClass_doc },
-    { "find", reinterpret_cast<PyCFunction>(ZoneFinder_find), METH_VARARGS,
-      ZoneFinder_find_doc },
+    { "get_origin", ZoneFinder_getOrigin, METH_NOARGS,
+       ZoneFinder_getOrigin_doc },
+    { "get_class", ZoneFinder_getClass, METH_NOARGS, ZoneFinder_getClass_doc },
+    { "find", ZoneFinder_find, METH_VARARGS, ZoneFinder_find_doc },
+    { "find_previous_name", ZoneFinder_findPreviousName, METH_VARARGS,
+      ZoneFinder_find_previous_name_doc },
     { NULL, NULL, 0, NULL }
 };
 
@@ -233,11 +267,15 @@ PyTypeObject zonefinder_type = {
 };
 
 PyObject*
-createZoneFinderObject(isc::datasrc::ZoneFinderPtr source) {
+createZoneFinderObject(isc::datasrc::ZoneFinderPtr source, PyObject* base_obj) {
     s_ZoneFinder* py_zi = static_cast<s_ZoneFinder*>(
         zonefinder_type.tp_alloc(&zonefinder_type, 0));
     if (py_zi != NULL) {
         py_zi->cppobj = source;
+        py_zi->base_obj = base_obj;
+    }
+    if (base_obj != NULL) {
+        Py_INCREF(base_obj);
     }
     return (py_zi);
 }

src/lib/python/isc/datasrc/finder_python.h

@@ -24,7 +24,15 @@ namespace python {
 
 extern PyTypeObject zonefinder_type;
 
-PyObject* createZoneFinderObject(isc::datasrc::ZoneFinderPtr source);
+/// \brief Create a ZoneFinder python object
+///
+/// \param source The zone iterator pointer to wrap
+/// \param base_obj An optional PyObject that this ZoneFinder depends on
+///                 Its refcount is increased, and will be decreased when
+///                 this zone iterator is destroyed, making sure that the
+///                 base object is never destroyed before this zonefinder.
+PyObject* createZoneFinderObject(isc::datasrc::ZoneFinderPtr source,
+                                 PyObject* base_obj = NULL);
 
 } // namespace python
 } // namespace datasrc

src/lib/python/isc/datasrc/iterator_python.cc

@@ -45,8 +45,14 @@ namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_ZoneIterator : public PyObject {
 public:
-    s_ZoneIterator() : cppobj(ZoneIteratorPtr()) {};
+    s_ZoneIterator() : cppobj(ZoneIteratorPtr()), base_obj(NULL) {};
     ZoneIteratorPtr cppobj;
+    // This is a reference to a base object; if the object of this class
+    // depends on another object to be in scope during its lifetime,
+    // we use INCREF the base object upon creation, and DECREF it at
+    // the end of the destructor
+    // This is an optional argument to createXXX(). If NULL, it is ignored.
+    PyObject* base_obj;
 };
 
 // Shortcut type which would be convenient for adding class variables safely.
@@ -68,6 +74,9 @@ ZoneIterator_destroy(s_ZoneIterator* const self) {
     // cppobj is a shared ptr, but to make sure things are not destroyed in
     // the wrong order, we reset it here.
     self->cppobj.reset();
+    if (self->base_obj != NULL) {
+        Py_DECREF(self->base_obj);
+    }
     Py_TYPE(self)->tp_free(self);
 }
 
@@ -187,11 +196,17 @@ PyTypeObject zoneiterator_type = {
 };
 
 PyObject*
-createZoneIteratorObject(isc::datasrc::ZoneIteratorPtr source) {
+createZoneIteratorObject(isc::datasrc::ZoneIteratorPtr source,
+                         PyObject* base_obj)
+{
     s_ZoneIterator* py_zi = static_cast<s_ZoneIterator*>(
         zoneiterator_type.tp_alloc(&zoneiterator_type, 0));
     if (py_zi != NULL) {
         py_zi->cppobj = source;
+        py_zi->base_obj = base_obj;
+    }
+    if (base_obj != NULL) {
+        Py_INCREF(base_obj);
     }
     return (py_zi);
 }

src/lib/python/isc/datasrc/iterator_python.h

@@ -25,7 +25,15 @@ namespace python {
 
 extern PyTypeObject zoneiterator_type;
 
-PyObject* createZoneIteratorObject(isc::datasrc::ZoneIteratorPtr source);
+/// \brief Create a ZoneIterator python object
+///
+/// \param source The zone iterator pointer to wrap
+/// \param base_obj An optional PyObject that this ZoneIterator depends on
+///                 Its refcount is increased, and will be decreased when
+///                 this zone iterator is destroyed, making sure that the
+///                 base object is never destroyed before this zone iterator.
+PyObject* createZoneIteratorObject(isc::datasrc::ZoneIteratorPtr source,
+                                   PyObject* base_obj = NULL);
 
 
 } // namespace python

src/lib/python/isc/datasrc/tests/Makefile.am

@@ -10,9 +10,14 @@ CLEANFILES = $(abs_builddir)/rwtest.sqlite3.copied
 
 # If necessary (rare cases), explicitly specify paths to dynamic libraries
 # required by loadable python modules.
-LIBRARY_PATH_PLACEHOLDER =
+# We always add one, the location of the data source modules
+# We may want to add an API method for this to the ds factory, but that is out
+# of scope for this ticket
+LIBRARY_PATH_PLACEHOLDER = $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/datasrc/.libs:
 if SET_ENV_LIBRARY_PATH
 LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/cryptolink/.libs:$(abs_top_builddir)/src/lib/dns/.libs:$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/lib/cc/.libs:$(abs_top_builddir)/src/lib/config/.libs:$(abs_top_builddir)/src/lib/log/.libs:$(abs_top_builddir)/src/lib/util/.libs:$(abs_top_builddir)/src/lib/exceptions/.libs:$(abs_top_builddir)/src/lib/datasrc/.libs:$$$(ENV_LIBRARY_PATH)
+else
+LIBRARY_PATH_PLACEHOLDER += $(ENV_LIBRARY_PATH)=$(abs_top_builddir)/src/lib/datasrc/.libs:$$$(ENV_LIBRARY_PATH)
 endif
 
 # test using command-line arguments, so use check-local target instead of TESTS

src/lib/python/isc/datasrc/tests/datasrc_test.py

@@ -24,9 +24,10 @@ TESTDATA_PATH = os.environ['TESTDATA_PATH'] + os.sep
 TESTDATA_WRITE_PATH = os.environ['TESTDATA_WRITE_PATH'] + os.sep
 
 READ_ZONE_DB_FILE = TESTDATA_PATH + "example.com.sqlite3"
-BROKEN_DB_FILE = TESTDATA_PATH + "brokendb.sqlite3"
 WRITE_ZONE_DB_FILE = TESTDATA_WRITE_PATH + "rwtest.sqlite3.copied"
-NEW_DB_FILE = TESTDATA_WRITE_PATH + "new_db.sqlite3"
+
+READ_ZONE_DB_CONFIG = "{ \"database_file\": \"" + READ_ZONE_DB_FILE + "\" }"
+WRITE_ZONE_DB_CONFIG = "{ \"database_file\": \"" + WRITE_ZONE_DB_FILE + "\"}"
 
 def add_rrset(rrset_list, name, rrclass, rrtype, ttl, rdatas):
     rrset_to_add = isc.dns.RRset(name, rrclass, rrtype, ttl)
@@ -59,13 +60,27 @@ def check_for_rrset(expected_rrsets, rrset):
 
 class DataSrcClient(unittest.TestCase):
 
-    def test_construct(self):
+    def test_constructors(self):
         # can't construct directly
         self.assertRaises(TypeError, isc.datasrc.ZoneIterator)
 
+        self.assertRaises(TypeError, isc.datasrc.DataSourceClient, 1, "{}")
+        self.assertRaises(TypeError, isc.datasrc.DataSourceClient, "sqlite3", 1)
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "foo", "{}")
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "sqlite3", "")
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "sqlite3", "{}")
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "sqlite3",
+                          "{ \"foo\": 1 }")
+        self.assertRaises(isc.datasrc.Error,
+                          isc.datasrc.DataSourceClient, "memory",
+                          "{ \"foo\": 1 }")
 
     def test_iterate(self):
-        dsc = isc.datasrc.DataSourceClient(READ_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
 
         # for RRSIGS, the TTL's are currently modified. This test should
         # start failing when we fix that.
@@ -176,7 +191,7 @@ class DataSrcClient(unittest.TestCase):
         self.assertRaises(TypeError, isc.datasrc.ZoneFinder)
 
     def test_find(self):
-        dsc = isc.datasrc.DataSourceClient(READ_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
 
         result, finder = dsc.find_zone(isc.dns.Name("example.com"))
         self.assertEqual(finder.SUCCESS, result)
@@ -231,6 +246,21 @@ class DataSrcClient(unittest.TestCase):
             "cname-ext.example.com. 3600 IN CNAME www.sql1.example.com.\n",
             rrset.to_text())
 
+        result, rrset = finder.find(isc.dns.Name("foo.wild.example.com"),
+                                    isc.dns.RRType.A(),
+                                    None,
+                                    finder.FIND_DEFAULT)
+        self.assertEqual(finder.WILDCARD, result)
+        self.assertEqual("foo.wild.example.com. 3600 IN A 192.0.2.255\n",
+                         rrset.to_text())
+
+        result, rrset = finder.find(isc.dns.Name("foo.wild.example.com"),
+                                    isc.dns.RRType.TXT(),
+                                    None,
+                                    finder.FIND_DEFAULT)
+        self.assertEqual(finder.WILDCARD_NXRRSET, result)
+        self.assertEqual(None, rrset)
+
         self.assertRaises(TypeError, finder.find,
                           "foo",
                           isc.dns.RRType.A(),
@@ -247,6 +277,24 @@ class DataSrcClient(unittest.TestCase):
                           None,
                           "foo")
 
+    def test_find_previous(self):
+        dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
+
+        result, finder = dsc.find_zone(isc.dns.Name("example.com"))
+        self.assertEqual(finder.SUCCESS, result)
+
+        prev = finder.find_previous_name(isc.dns.Name("bbb.example.com"))
+        self.assertEqual("example.com.", prev.to_text())
+
+        prev = finder.find_previous_name(isc.dns.Name("zzz.example.com"))
+        self.assertEqual("www.example.com.", prev.to_text())
+
+        prev = finder.find_previous_name(prev)
+        self.assertEqual("*.wild.example.com.", prev.to_text())
+
+        self.assertRaises(isc.datasrc.NotImplemented,
+                          finder.find_previous_name,
+                          isc.dns.Name("com"))
 
 class DataSrcUpdater(unittest.TestCase):
 
@@ -260,7 +308,7 @@ class DataSrcUpdater(unittest.TestCase):
 
     def test_update_delete_commit(self):
 
-        dsc = isc.datasrc.DataSourceClient(WRITE_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
 
         # first make sure, through a separate finder, that some record exists
         result, finder = dsc.find_zone(isc.dns.Name("example.com"))
@@ -334,7 +382,7 @@ class DataSrcUpdater(unittest.TestCase):
                          rrset.to_text())
 
     def test_update_delete_abort(self):
-        dsc = isc.datasrc.DataSourceClient(WRITE_ZONE_DB_FILE)
+        dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
 
         # first make sure, through a separate finder, that some record exists
         result, finder = dsc.find_zone(isc.dns.Name("example.com"))
@@ -383,6 +431,11 @@ class DataSrcUpdater(unittest.TestCase):
         self.assertEqual("www.example.com. 3600 IN A 192.0.2.1\n",
                          rrset.to_text())
 
+    def test_update_for_no_zone(self):
+        dsc = isc.datasrc.DataSourceClient("sqlite3", WRITE_ZONE_DB_CONFIG)
+        self.assertEqual(None,
+                         dsc.get_updater(isc.dns.Name("notexistent.example"),
+                                         True))
 
 if __name__ == "__main__":
     isc.log.init("bind10")

src/lib/python/isc/datasrc/updater_python.cc

@@ -54,8 +54,14 @@ namespace {
 // The s_* Class simply covers one instantiation of the object
 class s_ZoneUpdater : public PyObject {
 public:
-    s_ZoneUpdater() : cppobj(ZoneUpdaterPtr()) {};
+    s_ZoneUpdater() : cppobj(ZoneUpdaterPtr()), base_obj(NULL) {};
     ZoneUpdaterPtr cppobj;
+    // This is a reference to a base object; if the object of this class
+    // depends on another object to be in scope during its lifetime,
+    // we use INCREF the base object upon creation, and DECREF it at
+    // the end of the destructor
+    // This is an optional argument to createXXX(). If NULL, it is ignored.
+    PyObject* base_obj;
 };
 
 // Shortcut type which would be convenient for adding class variables safely.
@@ -81,6 +87,9 @@ ZoneUpdater_destroy(s_ZoneUpdater* const self) {
     // cppobj is a shared ptr, but to make sure things are not destroyed in
     // the wrong order, we reset it here.
     self->cppobj.reset();
+    if (self->base_obj != NULL) {
+        Py_DECREF(self->base_obj);
+    }
     Py_TYPE(self)->tp_free(self);
 }
 
@@ -176,51 +185,6 @@ ZoneUpdater_find(PyObject* po_self, PyObject* args) {
                                                     args));
 }
 
-PyObject*
-AZoneUpdater_find(PyObject* po_self, PyObject* args) {
-    s_ZoneUpdater* const self = static_cast<s_ZoneUpdater*>(po_self);
-    PyObject *name;
-    PyObject *rrtype;
-    PyObject *target;
-    int options_int;
-    if (PyArg_ParseTuple(args, "O!O!OI", &name_type, &name,
-                                         &rrtype_type, &rrtype,
-                                         &target, &options_int)) {
-        try {
-            ZoneFinder::FindOptions options =
-                static_cast<ZoneFinder::FindOptions>(options_int);
-            ZoneFinder::FindResult find_result(
-                self->cppobj->getFinder().find(PyName_ToName(name),
-                                   PyRRType_ToRRType(rrtype),
-                                   NULL,
-                                   options
-                                   ));
-            ZoneFinder::Result r = find_result.code;
-            isc::dns::ConstRRsetPtr rrsp = find_result.rrset;
-            if (rrsp) {
-                // Use N instead of O so the refcount isn't increased twice
-                return Py_BuildValue("IN", r, createRRsetObject(*rrsp));
-            } else {
-                return Py_BuildValue("IO", r, Py_None);
-            }
-        } catch (const DataSourceError& dse) {
-            PyErr_SetString(getDataSourceException("Error"), dse.what());
-            return (NULL);
-        } catch (const std::exception& exc) {
-            PyErr_SetString(getDataSourceException("Error"), exc.what());
-            return (NULL);
-        } catch (...) {
-            PyErr_SetString(getDataSourceException("Error"),
-                            "Unexpected exception");
-            return (NULL);
-        }
-    } else {
-        return (NULL);
-    }
-    return Py_BuildValue("I", 1);
-}
-
-
 // This list contains the actual set of functions we have in
 // python. Each entry has
 // 1. Python method name
@@ -303,12 +267,17 @@ PyTypeObject zoneupdater_type = {
 };
 
 PyObject*
-createZoneUpdaterObject(isc::datasrc::ZoneUpdaterPtr source) {
+createZoneUpdaterObject(isc::datasrc::ZoneUpdaterPtr source,
+                        PyObject* base_obj)
+{
     s_ZoneUpdater* py_zi = static_cast<s_ZoneUpdater*>(
         zoneupdater_type.tp_alloc(&zoneupdater_type, 0));
     if (py_zi != NULL) {
         py_zi->cppobj = source;
     }
+    if (base_obj != NULL) {
+        Py_INCREF(base_obj);
+    }
     return (py_zi);
 }
 

src/lib/python/isc/datasrc/updater_python.h

@@ -26,7 +26,15 @@ namespace python {
 
 extern PyTypeObject zoneupdater_type;
 
-PyObject* createZoneUpdaterObject(isc::datasrc::ZoneUpdaterPtr source);
+/// \brief Create a ZoneUpdater python object
+///
+/// \param source The zone iterator pointer to wrap
+/// \param base_obj An optional PyObject that this ZoneUpdater depends on
+///                 It's refcount is increased, and will be decreased when
+///                 this zone iterator is destroyed, making sure that the
+///                 base object is never destroyed before this zone updater.
+PyObject* createZoneUpdaterObject(isc::datasrc::ZoneUpdaterPtr source,
+                                  PyObject* base_obj = NULL);
 
 
 } // namespace python