Merge branch 'master' into trac2656_2

ChangeLog

@@ -1,3 +1,14 @@
+563.	[build]		jinmei
+	Added --disable-rpath configure option to avoid embedding library
+	paths to binaries.  Patch from Adam Tkac.
+	(Trac #2667, git 1c50c5a6ee7e9675e3ab154f2c7f975ef519fca2)
+
+562.	[func]*		vorner
+	The b10-xfrin now performs basic sanity check on just received
+	zone. It'll reject severely broken zones (such as missng NS
+	records).
+	(Trac #2439, git 44699b4b18162581cd1dd39be5fb76ca536012e6)
+
 561.	[bug]		kambe, jelte
 	b10-stats-httpd no longer dumps request information to the console,
 	but uses the bind10 logging system. Additionally, the logging

doc/Makefile.am

@@ -1,6 +1,6 @@
 SUBDIRS = guide
 
-EXTRA_DIST = version.ent.in
+EXTRA_DIST = version.ent.in differences.txt
 
 devel:
 	mkdir -p html

doc/differences.txt

@@ -0,0 +1,12 @@
+Differences of Bind 10 to other software
+========================================
+
+Bind 9
+------
+
+TODO: There are definitely more differences than just this.
+
+* When an incoming zone transfer fails, for example because the
+  received zone doesn't contain a NS record, bind 9 stops serving the
+  zone and returns SERVFAIL to queries for that zone. Bind 10 still
+  uses the previous version of zone.

examples/configure.ac

@@ -14,9 +14,10 @@ AC_LANG([C++])
 # Checks for BIND 10 headers and libraries
 AX_ISC_BIND10
 
-# We use -R, -rpath etc so the resulting program will be more likekly to
+# We use -R option etc so the resulting program will be more likekly to
 # "just work" by default.  Embedding a specific library path is a controversial
-# practice, though; if you don't like it you can remove the following setting.
+# practice, though; if you don't like it you can remove the following setting,
+# or use the --disable-rpath option.
 if test "x$BIND10_RPATH" != "x"; then
    LDFLAGS="$LDFLAGS $BIND10_RPATH"
 fi

examples/m4/ax_isc_rpath.m4

@@ -3,44 +3,54 @@ dnl
 dnl @summary figure out whether and which "rpath" linker option is available
 dnl
 dnl This macro checks if the linker supports an option to embed a path
-dnl to a runtime library (often installed in an uncommon place), such as
-dnl gcc's -rpath option.  If found, it sets the ISC_RPATH_FLAG variable to
+dnl to a runtime library (often installed in an uncommon place), such as the
+dnl commonly used -R option.  If found, it sets the ISC_RPATH_FLAG variable to
 dnl the found option flag.  The main configure.ac can use it as follows:
 dnl if test "x$ISC_RPATH_FLAG" != "x"; then
 dnl     LDFLAGS="$LDFLAGS ${ISC_RPATH_FLAG}/usr/local/lib/some_library"
 dnl fi
+dnl
+dnl If you pass --disable-rpath to configure, ISC_RPATH_FLAG is not set
 
 AC_DEFUN([AX_ISC_RPATH], [
 
-# We'll tweak both CXXFLAGS and CCFLAGS so this function will work whichever
-# language is used in the main script.  Note also that it's not LDFLAGS;
-# technically this is a linker flag, but we've noticed $LDFLAGS can be placed
-# where the compiler could interpret it as a compiler option, leading to
-# subtle failure mode.  So, in the check below using the compiler flag is
-# safer (in the actual Makefiles the flag should be set in LDFLAGS).
-CXXFLAGS_SAVED="$CXXFLAGS"
-CXXFLAGS="$CXXFLAGS -Wl,-R/usr/lib"
-CCFLAGS_SAVED="$CCFLAGS"
-CCFLAGS="$CCFLAGS -Wl,-R/usr/lib"
+AC_ARG_ENABLE(rpath,
+    [AC_HELP_STRING([--disable-rpath], [don't hardcode library path into binaries])],
+    rpath=$enableval, rpath=yes)
+
+if test x$rpath != xno; then
+    # We'll tweak both CXXFLAGS and CCFLAGS so this function will work
+    # whichever language is used in the main script.  Note also that it's not
+    #LDFLAGS; technically this is a linker flag, but we've noticed $LDFLAGS
+    # can be placed where the compiler could interpret it as a compiler
+    # option, leading to subtle failure mode.  So, in the check below using
+    # the compiler flag is safer (in the actual Makefiles the flag should be
+    # set in LDFLAGS).
+    CXXFLAGS_SAVED="$CXXFLAGS"
+    CXXFLAGS="$CXXFLAGS -Wl,-R/usr/lib"
+    CCFLAGS_SAVED="$CCFLAGS"
+    CCFLAGS="$CCFLAGS -Wl,-R/usr/lib"
 
-# check -Wl,-R and -R rather than gcc specific -rpath to be as portable
-# as possible.  -Wl,-R seems to be safer, so we try it first.  In some cases
-# -R is not actually recognized but AC_TRY_LINK doesn't fail due to that.
-AC_MSG_CHECKING([whether -Wl,-R flag is available in linker])
-AC_TRY_LINK([],[],
-    [ AC_MSG_RESULT(yes)
-        ISC_RPATH_FLAG=-Wl,-R
-    ],[ AC_MSG_RESULT(no)
-        AC_MSG_CHECKING([whether -R flag is available in linker])
-	CXXFLAGS="$CXXFLAGS_SAVED -R"
-	CCFLAGS="$CCFLAGS_SAVED -R"
+    # check -Wl,-R and -R rather than gcc specific -rpath to be as portable
+    # as possible.  -Wl,-R seems to be safer, so we try it first.  In some
+    # cases -R is not actually recognized but AC_TRY_LINK doesn't fail due to
+    # that.
+    AC_MSG_CHECKING([whether -Wl,-R flag is available in linker])
+    AC_TRY_LINK([],[],
+        [ AC_MSG_RESULT(yes)
+            ISC_RPATH_FLAG=-Wl,-R
+        ],[ AC_MSG_RESULT(no)
+            AC_MSG_CHECKING([whether -R flag is available in linker])
+            CXXFLAGS="$CXXFLAGS_SAVED -R"
+            CCFLAGS="$CCFLAGS_SAVED -R"
         AC_TRY_LINK([], [],
             [ AC_MSG_RESULT([yes; note that -R is more sensitive about the position in option arguments])
                 ISC_RPATH_FLAG=-R
             ],[ AC_MSG_RESULT(no) ])
-    ])
+        ])
 
-CXXFLAGS=$CXXFLAGS_SAVED
-CCFLAGS=$CCFLAGS_SAVED
+    CXXFLAGS=$CXXFLAGS_SAVED
+    CCFLAGS=$CCFLAGS_SAVED
+fi
 
 ])dnl AX_ISC_RPATH

src/bin/dbutil/b10-dbutil.xml

@@ -93,7 +93,7 @@
       file.  It has the same name, with ".backup" appended to it.  If a
       file of that name already exists, the file will have the suffix
       ".backup-1".  If that exists, the file will be suffixed ".backup-2",
-      and so on). Exit status is 0 if the upgrade is either succesful or
+      and so on). Exit status is 0 if the upgrade is either successful or
       aborted by the user, and non-zero if there is an error.
     </para>
 

src/bin/dhcp4/config_parser.cc

@@ -687,7 +687,7 @@ public:
     virtual void commit() {
         if (options_ == NULL) {
             isc_throw(isc::InvalidOperation, "parser logic error: storage must be set before "
-                      "commiting option data.");
+                      "committing option data.");
         } else  if (!option_descriptor_.option) {
             // Before we can commit the new option should be configured. If it is not
             // than somebody must have called commit() before build().
@@ -1855,7 +1855,7 @@ configureDhcp4Server(Dhcpv4Srv&, ConstElementPtr config_set) {
     LOG_INFO(dhcp4_logger, DHCP4_CONFIG_COMPLETE).arg(config_details);
 
     // Everything was fine. Configuration is successful.
-    answer = isc::config::createAnswer(0, "Configuration commited.");
+    answer = isc::config::createAnswer(0, "Configuration committed.");
     return (answer);
 }
 

src/bin/dhcp4/dhcp4_srv.cc

@@ -61,6 +61,7 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig) {
         string srvid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_ID_FILE);
         if (loadServerID(srvid_file)) {
             LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_SERVERID_LOADED)
+                .arg(srvidToString(getServerID()))
                 .arg(srvid_file);
         } else {
             generateServerID();

src/bin/dhcp6/config_parser.cc

@@ -716,7 +716,7 @@ public:
     virtual void commit() {
         if (options_ == NULL) {
             isc_throw(isc::InvalidOperation, "parser logic error: storage must be set before "
-                      "commiting option data.");
+                      "committing option data.");
         } else  if (!option_descriptor_.option) {
             // Before we can commit the new option should be configured. If it is not
             // than somebody must have called commit() before build().
@@ -1908,7 +1908,7 @@ configureDhcp6Server(Dhcpv6Srv&, ConstElementPtr config_set) {
     LOG_INFO(dhcp6_logger, DHCP6_CONFIG_COMPLETE).arg(config_details);
 
     // Everything was fine. Configuration is successful.
-    answer = isc::config::createAnswer(0, "Configuration commited.");
+    answer = isc::config::createAnswer(0, "Configuration committed.");
     return (answer);
 }
 

src/bin/dhcp6/dhcp6_srv.cc

@@ -78,6 +78,7 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
         string duid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_DUID_FILE);
         if (loadServerID(duid_file)) {
             LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_SERVERID_LOADED)
+                .arg(duidToString(getServerID()))
                 .arg(duid_file);
         } else {
             generateServerID();

src/bin/xfrin/tests/xfrin_test.py

@@ -153,13 +153,19 @@ class MockCC(MockModuleCCSession):
     def remove_remote_config(self, module_name):
         pass
 
+class MockRRsetCollection:
+    '''
+    A mock RRset collection. We don't use it really (we mock the method that
+    it is passed to too), so it's empty.
+    '''
+    pass
+
 class MockDataSourceClient():
     '''A simple mock data source client.
 
     This class provides a minimal set of wrappers related the data source
     API that would be used by Diff objects.  For our testing purposes they
-    only keep truck of the history of the changes.
-
+    only keep track of the history of the changes.
     '''
     def __init__(self):
         self.force_fail = False # if True, raise an exception on commit
@@ -217,6 +223,12 @@ class MockDataSourceClient():
         self._journaling_enabled = journaling
         return self
 
+    def get_rrset_collection(self):
+        '''
+        Pretend to be a zone updater and provide a (dummy) rrset collection.
+        '''
+        return MockRRsetCollection()
+
     def add_rrset(self, rrset):
         self.diffs.append(('add', rrset))
 
@@ -726,11 +738,27 @@ class TestXfrinConnection(unittest.TestCase):
             'tsig_1st': None,
             'tsig_2nd': None
             }
+        self.__orig_check_zone = xfrin.check_zone
+        xfrin.check_zone = self.__check_zone
+        self._check_zone_result = True
+        self._check_zone_params = None
 
     def tearDown(self):
         self.conn.close()
         if os.path.exists(TEST_DB_FILE):
             os.remove(TEST_DB_FILE)
+        xfrin.check_zone = self.__orig_check_zone
+
+    def __check_zone(self, name, rrclass, rrsets, callbacks):
+        '''
+        A mock function used instead of dns.check_zone.
+        '''
+        self._check_zone_params = (name, rrclass, rrsets, callbacks)
+        # Call both callbacks to see they do nothing. This checks
+        # the transfer depends on the result only.
+        callbacks[0]("Test error")
+        callbacks[1]("Test warning")
+        return self._check_zone_result
 
     def _create_normal_response_data(self):
         # This helper method creates a simple sequence of DNS messages that
@@ -825,6 +853,7 @@ class TestAXFR(TestXfrinConnection):
 
     def tearDown(self):
         time.time = self.orig_time_time
+        super().tearDown()
 
     def __create_mock_tsig(self, key, error, has_last_signature=True):
         # This helper function creates a MockTSIGContext for a given key
@@ -1297,6 +1326,33 @@ class TestAXFR(TestXfrinConnection):
                     [[('add', ns_rr), ('add', a_rr), ('add', soa_rrset)]],
                     self.conn._datasrc_client.committed_diffs)
 
+    def test_axfr_response_fail_validation(self):
+        """
+        Test we reject a zone transfer if it fails the check_zone validation.
+        """
+        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, a_rr, soa_rrset])
+        # Make it fail the validation
+        self._check_zone_result = False
+        self.assertRaises(XfrinZoneError, self.conn._handle_xfrin_responses)
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        self.assertEqual([], self.conn._datasrc_client.committed_diffs)
+        # Check the validation is called with the correct parameters
+        self.assertEqual(TEST_ZONE_NAME, self._check_zone_params[0])
+        self.assertEqual(TEST_RRCLASS, self._check_zone_params[1])
+        self.assertTrue(isinstance(self._check_zone_params[2],
+                                   MockRRsetCollection))
+        # Check we can safely call the callbacks. They have no sideeffects
+        # we can check (checking logging is hard), but we at least check
+        # they don't crash.
+        self._check_zone_params[3][0]("Test error")
+        self._check_zone_params[3][1]("Test warning")
+
     def test_axfr_response_extra(self):
         '''Test with an extra RR after the end of AXFR session.
 
@@ -1499,6 +1555,15 @@ class TestAXFR(TestXfrinConnection):
         self.conn.response_generator = self._create_normal_response_data
         self.assertEqual(self.conn.do_xfrin(False), XFRIN_FAIL)
 
+    def test_do_xfrin_invalid_zone(self):
+        """
+        Test receiving an invalid zone. We mock the check and see the whole
+        transfer is rejected.
+        """
+        self._check_zone_result = False
+        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):
         self.conn.response_generator = self._create_soa_response_data
         self.assertEqual(self.conn.do_xfrin(True), XFRIN_OK)
@@ -1576,6 +1641,26 @@ class TestIXFRResponse(TestXfrinConnection):
                     [[('delete', begin_soa_rrset), ('add', soa_rrset)]],
                     self.conn._datasrc_client.committed_diffs)
 
+    def test_ixfr_response_fail_validation(self):
+        '''
+        An IXFR that fails validation later on. Check it is rejected.
+        '''
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())],
+            answers=[soa_rrset, begin_soa_rrset, soa_rrset, soa_rrset])
+        self._check_zone_result = False
+        self.assertRaises(XfrinZoneError, self.conn._handle_xfrin_responses)
+        self.assertEqual([], self.conn._datasrc_client.committed_diffs)
+        self.assertEqual(TEST_ZONE_NAME, self._check_zone_params[0])
+        self.assertEqual(TEST_RRCLASS, self._check_zone_params[1])
+        self.assertTrue(isinstance(self._check_zone_params[2],
+                                   MockRRsetCollection))
+        # Check we can safely call the callbacks. They have no sideeffects
+        # we can check (checking logging is hard), but we at least check
+        # they don't crash.
+        self._check_zone_params[3][0]("Test error")
+        self._check_zone_params[3][1]("Test warning")
+
     def test_ixfr_response_multi_sequences(self):
         '''Similar to the previous case, but with multiple diff seqs.
 

src/bin/xfrin/xfrin.py.in

@@ -36,6 +36,7 @@ from isc.xfrin.diff import Diff
 from isc.server_common.auth_command import auth_loadzone_command
 from isc.server_common.tsig_keyring import init_keyring, get_keyring
 from isc.log_messages.xfrin_messages import *
+from isc.dns import *
 
 isc.log.init("b10-xfrin", buffer=True)
 logger = isc.log.Logger("xfrin")
@@ -45,13 +46,6 @@ logger = isc.log.Logger("xfrin")
 DBG_PROCESS = logger.DBGLVL_TRACE_BASIC
 DBG_COMMANDS = logger.DBGLVL_TRACE_DETAIL
 
-try:
-    from pydnspp import *
-except ImportError as e:
-    # C++ loadable module may not be installed; even so the xfrin process
-    # must keep running, so we warn about it and move forward.
-    logger.error(XFRIN_IMPORT_DNS, str(e))
-
 isc.util.process.rename()
 
 # If B10_FROM_BUILD is set in the environment, we use data files
@@ -100,8 +94,17 @@ class XfrinProtocolError(Exception):
     '''
     pass
 
+class XfrinZoneError(Exception):
+    '''
+    An exception raised when the received zone is broken enough to be unusable.
+    '''
+    pass
+
 class XfrinZoneUptodate(Exception):
-    '''TBD
+    '''
+    Thrown when the zone is already up to date, so there's no need to download
+    the zone. This is not really an error case (but it's still an exceptional
+    condition and the control flow is different than usual).
     '''
     pass
 
@@ -427,10 +430,10 @@ class XfrinIXFRAdd(XfrinState):
             conn.get_transfer_stats().ixfr_changeset_count += 1
             soa_serial = get_soa_serial(rr.get_rdata()[0])
             if soa_serial == conn._end_serial:
-                # The final part is there. Check all was signed
-                # and commit it to the database.
-                conn._check_response_tsig_last()
-                conn._diff.commit()
+                # The final part is there. Finish the transfer by
+                # checking the last TSIG (if required), the zone data and
+                # commiting.
+                conn.finish_transfer()
                 self.set_xfrstate(conn, XfrinIXFREnd())
                 return True
             elif soa_serial != conn._current_serial:
@@ -500,15 +503,11 @@ class XfrinAXFREnd(XfrinState):
         """
         Final processing after processing an entire AXFR session.
 
-        In this process all the AXFR changes are committed to the
-        data source.
-
-        There might be more actions here, but for now we simply return False,
-        indicating there will be no more message to receive.
-
+        This simply calls the finish_transfer method of the connection
+        that ensures it is signed by TSIG (if required), the zone data
+        is valid and commits it.
         """
-        conn._check_response_tsig_last()
-        conn._diff.commit()
+        conn.finish_transfer()
         return False
 
 class XfrinTransferStats:
@@ -805,6 +804,31 @@ class XfrinConnection(asyncore.dispatcher):
                 raise XfrinProtocolError('TSIG verify fail: no TSIG on last '+
                                          'message')
 
+    def __validate_error(self, reason):
+        '''
+        Used as error callback below.
+        '''
+        logger.error(XFRIN_ZONE_INVALID, self._zone_name, self._rrclass,
+                     reason)
+
+    def __validate_warning(self, reason):
+        '''
+        Used as warning callback below.
+        '''
+        logger.warn(XFRIN_ZONE_WARN, self._zone_name, self._rrclass, reason)
+
+    def finish_transfer(self):
+        """
+        Perform any necessary checks after a transfer. Then complete the
+        transfer by commiting the transaction into the data source.
+        """
+        self._check_response_tsig_last()
+        if not check_zone(self._zone_name, self._rrclass,
+                          self._diff.get_rrset_collection(),
+                          (self.__validate_error, self.__validate_warning)):
+            raise XfrinZoneError('Validation of the new zone failed')
+        self._diff.commit()
+
     def __parse_soa_response(self, msg, response_data):
         '''Parse a response to SOA query and extract the SOA from answer.
 
@@ -950,8 +974,16 @@ class XfrinConnection(asyncore.dispatcher):
             # of trying another primary server, etc, but for now we treat it
             # as "success".
             pass
+        except XfrinZoneError:
+            # The log message doesn't contain the exception text, since there's
+            # only one place where the exception is thrown now and it'd be the
+            # same generic message every time.
+            logger.error(XFRIN_INVALID_ZONE_DATA, self.zone_str(),
+                         format_addrinfo(self._master_addrinfo))
+            ret = XFRIN_FAIL
         except XfrinProtocolError as e:
-            logger.info(XFRIN_XFR_TRANSFER_PROTOCOL_ERROR, req_str,
+            # FIXME: Why is this .info? Even the messageID contains "ERROR".
+            logger.info(XFRIN_XFR_TRANSFER_PROTOCOL_VIOLATION, req_str,
                         self.zone_str(),
                         format_addrinfo(self._master_addrinfo), str(e))
             ret = XFRIN_FAIL

src/bin/xfrin/xfrin_messages.mes

@@ -77,6 +77,11 @@ is not equal to the requested SOA serial.
 There was an error importing the python DNS module pydnspp. The most
 likely cause is a PYTHONPATH problem.
 
+% XFRIN_INVALID_ZONE_DATA zone %1 received from %2 is broken and unusable
+The zone was received, but it failed sanity validation. The previous version
+of zone (if any is available) will be used. Look for previous
+XFRIN_ZONE_INVALID messages to see the exact problem(s).
+
 % XFRIN_IXFR_TRANSFER_SUCCESS incremental IXFR transfer of zone %1 succeeded (messages: %2, changesets: %3, deletions: %4, additions: %5, bytes: %6, run time: %7 seconds, %8 bytes/second)
 The IXFR transfer for the given zone was successful.
 The provided information contains the following values:
@@ -205,7 +210,7 @@ such that the remote server doesn't support IXFR, we don't have the SOA record
 (or the zone at all), we are out of sync, etc. In many of these situations,
 AXFR could still work. Therefore we try that one in case it helps.
 
-% XFRIN_XFR_TRANSFER_PROTOCOL_ERROR %1 transfer of zone %2 with %3 failed: %4
+% XFRIN_XFR_TRANSFER_PROTOCOL_VIOLATION %1 transfer of zone %2 with %3 failed: %4
 The XFR transfer for the given zone has failed due to a protocol
 error, such as an unexpected response from the primary server.  The
 error is shown in the log message.  It may be because the primary
@@ -232,6 +237,12 @@ zones at a higher level.  In future it is more likely that a separate
 zone management framework is provided, and the situation where the
 given zone isn't found in xfrout will be treated as an error.
 
+% XFRIN_ZONE_INVALID Newly received zone %1/%2 fails validation: %3
+The zone was received successfully, but it failed validation. The problem
+is severe enough that the new version of zone is discarded and the old version,
+if any, will stay in use. New transfer will be attempted after some time.
+The problem needs to be fixed in the zone data on the remote server.
+
 % XFRIN_ZONE_MULTIPLE_SOA Zone %1 has %2 SOA RRs
 On starting an xfrin session, it is identified that the zone to be
 transferred has multiple SOA RRs.  Such a zone is broken, but could be
@@ -258,3 +269,9 @@ the latest version of the zone.  But if the primary server is known to
 be the real source of the zone, some unexpected inconsistency may have
 happened, and you may want to take a closer look.  In this case xfrin
 doesn't perform subsequent zone transfer.
+
+% XFRIN_ZONE_WARN Newly received zone %1/%2 has a problem: %3
+The zone was received successfully, but when checking it, it was discovered
+there's some issue with it. It might be correct, but it should be checked
+and possibly fixed on the remote server. The problem is described in the
+message. The problem does not stop the zone from being used.

src/lib/datasrc/datasrc_messages.mes

@@ -197,6 +197,16 @@ modify the database). This is what the client would do when such RRs
 were given in a DNS response according to RFC2181. The data in
 database should be checked and fixed.
 
+% DATASRC_DATABASE_JOURNALREADER_BADDATA failed to convert a diff to RRset in %1/%2 on %3 between %4 and %5: %6
+This is an error message indicating that a zone's diff is broken and
+the data source library failed to convert it to a valid RRset.  The
+most likely cause of this is that someone has manually modified the
+zone's diff in the database and inserted invalid data as a result.
+The zone's name and class, database name, and the start and end
+serials, and an additional detail of the error are shown in the
+message.  The administrator should examine the diff in the database
+to find any invalid data and fix it.
+
 % DATASRC_DATABASE_JOURNALREADER_END %1/%2 on %3 from %4 to %5
 This is a debug message indicating that the program (successfully)
 reaches the end of sequences of a zone's differences.  The zone's name
@@ -215,16 +225,6 @@ a zone's difference sequences from a database-based data source.  The
 zone's name and class, database name, and the start and end serials
 are shown in the message.
 
-% DATASRC_DATABASE_JOURNALREADER_BADDATA failed to convert a diff to RRset in %1/%2 on %3 between %4 and %5: %6
-This is an error message indicating that a zone's diff is broken and
-the data source library failed to convert it to a valid RRset.  The
-most likely cause of this is that someone has manually modified the
-zone's diff in the database and inserted invalid data as a result.
-The zone's name and class, database name, and the start and end
-serials, and an additional detail of the error are shown in the
-message.  The administrator should examine the diff in the database
-to find any invalid data and fix it.
-
 % DATASRC_DATABASE_NO_MATCH not match for %2/%3/%4 in %1
 No match (not even a wildcard) was found in the named data source for the given
 name/type/class in the data source.
@@ -442,6 +442,10 @@ shown name, the search tries the superdomain name that share the shown
 www.example.com. with shown label count of 3, example.com. is being
 tried).
 
+% DATASRC_MEM_FIND_TYPE_AT_ORIGIN origin query for type %1 in in-memory zone %2/%3 successful
+Debug information.  A specific type RRset is requested at a zone origin
+of an in-memory zone and it is found.
+
 % DATASRC_MEM_FIND_ZONE looking for zone '%1'
 Debug information. A zone object for this zone is being searched for in the
 in-memory data source.

src/lib/datasrc/memory/treenode_rrset.cc

@@ -59,8 +59,7 @@ TreeNodeRRset::getName() const {
 const RRTTL&
 TreeNodeRRset::getTTL() const {
     if (ttl_ == NULL) {
-        util::InputBuffer ttl_buffer(rdataset_->getTTLData(),
-                                     sizeof(uint32_t));
+        util::InputBuffer ttl_buffer(ttl_data_, sizeof(uint32_t));
         ttl_ = new RRTTL(ttl_buffer);
     }
 
@@ -169,7 +168,7 @@ TreeNodeRRset::toWire(AbstractMessageRenderer& renderer) const {
     // Render the main (non RRSIG) RRs
     const size_t rendered_rdata_count =
         writeRRs(renderer, rdataset_->getRdataCount(), name_labels,
-                 rdataset_->type, rrclass_, rdataset_->getTTLData(), reader,
+                 rdataset_->type, rrclass_, ttl_data_, reader,
                  &RdataReader::iterateRdata);
     if (renderer.isTruncated()) {
         return (rendered_rdata_count);
@@ -180,7 +179,7 @@ TreeNodeRRset::toWire(AbstractMessageRenderer& renderer) const {
     // Render any RRSIGs, if we supposed to do so
     const size_t rendered_rrsig_count = dnssec_ok_ ?
         writeRRs(renderer, rrsig_count_, name_labels, RRType::RRSIG(),
-                 rrclass_, rdataset_->getTTLData(), reader,
+                 rrclass_, ttl_data_, reader,
                  &RdataReader::iterateSingleSig) : 0;
 
     return (rendered_rdata_count + rendered_rrsig_count);

src/lib/datasrc/memory/treenode_rrset.h

@@ -112,12 +112,34 @@ public:
                   const RdataSet* rdataset, bool dnssec_ok) :
         node_(node), rdataset_(rdataset),
         rrsig_count_(rdataset_->getSigRdataCount()), rrclass_(rrclass),
-        dnssec_ok_(dnssec_ok), name_(NULL), realname_(NULL), ttl_(NULL)
+        dnssec_ok_(dnssec_ok), name_(NULL), realname_(NULL),
+        ttl_data_(rdataset->getTTLData()), ttl_(NULL)
+    {}
+
+    /// \brief Constructor with a specific TTL.
+    ///
+    /// This constructor is mostly the same as the normal version, but takes
+    /// an extra parameter, \c ttl_data.  It's expected to point to a memory
+    /// region at least for 32 bits, and the corresponding 32-bit data will
+    /// be used as wire-format TTL value of the RRset, instead of the TTL
+    /// associated with \c rdataset.
+    ///
+    /// It's the caller's responsibility to guarantee the memory region is
+    /// valid and intact throughout the lifetime of the RRset.
+    ///
+    /// \throw None
+    TreeNodeRRset(const dns::RRClass& rrclass, const ZoneNode* node,
+                  const RdataSet* rdataset, bool dnssec_ok,
+                  const void* ttl_data) :
+        node_(node), rdataset_(rdataset),
+        rrsig_count_(rdataset_->getSigRdataCount()), rrclass_(rrclass),
+        dnssec_ok_(dnssec_ok), name_(NULL), realname_(NULL),
+        ttl_data_(ttl_data), ttl_(NULL)
     {}
 
     /// \brief Constructor for wildcard-expanded owner name.
     ///
-    /// This constructor is mostly the same as the other version, but takes
+    /// This constructor is mostly the same as the normal version, but takes
     /// an extra parameter, \c realname.  It effectively overrides the owner
     /// name of the RRset; wherever the owner name is used (e.g., in the
     /// \c toWire() method), the specified name will be used instead of
@@ -133,7 +155,7 @@ public:
         node_(node), rdataset_(rdataset),
         rrsig_count_(rdataset_->getSigRdataCount()), rrclass_(rrclass),
         dnssec_ok_(dnssec_ok), name_(NULL), realname_(new dns::Name(realname)),
-	ttl_(NULL)
+	ttl_data_(rdataset->getTTLData()), ttl_(NULL)
     {}
 
     virtual ~TreeNodeRRset() {
@@ -255,6 +277,7 @@ private:
     const bool dnssec_ok_;
     mutable dns::Name* name_;
     const dns::Name* const realname_;
+    const void* const ttl_data_;
     mutable dns::RRTTL* ttl_;
 };
 

src/lib/datasrc/memory/zone_data.cc

@@ -134,6 +134,31 @@ NSEC3Data::insertName(util::MemorySegment& mem_sgmt, const Name& name,
             result == ZoneTree::ALREADYEXISTS) && node != NULL);
 }
 
+namespace {
+// A helper to convert a TTL value in network byte order and set it in
+// ZoneData::min_ttl_.  We can use util::OutputBuffer, but copy the logic
+// here to guarantee it is exception free.
+// Note: essentially this function is a local (re)implementation of the
+// standard htonl() library function, but we avoid relying on it in case it's
+// not available (it's not in the C++ standard library).
+void
+setTTLInNetOrder(uint32_t val, uint32_t* result) {
+    uint8_t buf[4];
+    buf[0] = static_cast<uint8_t>((val & 0xff000000) >> 24);
+    buf[1] = static_cast<uint8_t>((val & 0x00ff0000) >> 16);
+    buf[2] = static_cast<uint8_t>((val & 0x0000ff00) >> 8);
+    buf[3] = static_cast<uint8_t>(val & 0x000000ff);
+    std::memcpy(result, buf, sizeof(*result));
+}
+}
+
+ZoneData::ZoneData(ZoneTree* zone_tree, ZoneNode* origin_node) :
+    zone_tree_(zone_tree), origin_node_(origin_node),
+    min_ttl_(0)          // tentatively set to silence static checkers
+{
+    setTTLInNetOrder(RRTTL::MAX_TTL().getValue(), &min_ttl_);
+}
+
 ZoneData*
 ZoneData::create(util::MemorySegment& mem_sgmt, const Name& zone_origin) {
     // ZoneTree::insert() and ZoneData allocation can throw.  See also
@@ -178,6 +203,11 @@ ZoneData::insertName(util::MemorySegment& mem_sgmt, const Name& name,
             result == ZoneTree::ALREADYEXISTS) && node != NULL);
 }
 
+void
+ZoneData::setMinTTL(uint32_t min_ttl_val) {
+    setTTLInNetOrder(min_ttl_val, &min_ttl_);
+}
+
 } // namespace memory
 } // namespace datasrc
 } // datasrc isc

src/lib/datasrc/memory/zone_data.h

@@ -287,7 +287,7 @@ private:
 /// from NSEC to NSEC3 or vice versa, support incremental signing, or support
 /// multiple sets of NSEC3 parameters.
 ///
-/// One last type of meta data is the status of the zone in terms of DNSSEC
+/// One other type of meta data is the status of the zone in terms of DNSSEC
 /// signing.  This class supports the following concepts:
 /// - Whether the zone is signed or not, either with NSEC records or NSEC3
 ///   records.
@@ -315,6 +315,15 @@ private:
 /// because we won't have to change the application code when we implement
 /// the future separation.
 ///
+/// One last type of meta data is the zone's "minimum" TTL.  It's expected
+/// to be a shortcut copy of the minimum field of the zone's SOA RDATA,
+/// and is expected to be used to create an SOA RR for a negative response,
+/// whose RR TTL may have to be set to this value according to RFC2308.
+/// This class is not aware of such usage, however, and only provides a
+/// simple getter and setter method for this value: \c getMinTTLData() and
+/// \c setMinTTL().  The user of this class is responsible for setting the
+/// value with \c setMinTTL() when it loads or updates the SOA RR.
+///
 /// The intended usage of these two status concepts is to implement the
 /// \c ZoneFinder::Context::isNSECSigned() and
 /// \c ZoneFinder::Context::isNSEC3Signed() methods.  A possible implementation
@@ -349,9 +358,7 @@ private:
     /// allocator (\c create()), so the constructor is hidden as private.
     ///
     /// It never throws an exception.
-    ZoneData(ZoneTree* zone_tree, ZoneNode* origin_node) :
-        zone_tree_(zone_tree), origin_node_(origin_node)
-    {}
+    ZoneData(ZoneTree* zone_tree, ZoneNode* origin_node);
 
     // Zone node flags.
 private:
@@ -413,7 +420,7 @@ public:
     ///
     /// The class encapsulation ensures that the origin node always exists at
     /// the same address, so this method always returns a non-NULL valid
-    /// valid pointer.
+    /// pointer.
     ///
     /// \throw none
     const ZoneNode* getOriginNode() const {
@@ -456,6 +463,26 @@ public:
     ///
     /// \throw none
     const NSEC3Data* getNSEC3Data() const { return (nsec3_data_.get()); }
+
+    /// \brief Return a pointer to the zone's minimum TTL data.
+    ///
+    /// The returned pointer points to a memory region that is valid at least
+    /// for 32 bits, storing the zone's minimum TTL in the network byte
+    /// order.  The corresponding 32-bit value as an integer is initially
+    /// set to the value of \c dns::RRTTL::MAX_TTL(), and, once
+    /// \c setMinTTL() is called, set to the value specified at the latest
+    /// call to \c setMinTTL().
+    ///
+    /// It returns opaque data to make it clear that unless the wire
+    /// format data is necessary (e.g., when rendering it in a DNS message),
+    /// it should be converted to, e.g., an \c RRTTL object explicitly.
+    ///
+    /// The returned pointer is valid as long as the \c ZoneData is valid,
+    /// and the corresponding 32-bit data are the same until \c setMinTTL()
+    /// is called.
+    ///
+    /// \throw none
+    const void* getMinTTLData() const { return (&min_ttl_); }
     //@}
 
     ///
@@ -552,12 +579,32 @@ public:
         nsec3_data_ = nsec3_data;
         return (old);
     }
+
+    /// \brief Set the zone's "minimum" TTL.
+    ///
+    /// This method updates the recorded minimum TTL of the zone data.
+    /// It's expected to be identical to the value of the Minimum field
+    /// of the SOA RR at the zone apex, but this method does not check the
+    /// consistency; it's the caller's responsibility.
+    ///
+    /// While RFC2181 specifies the max TTL value to be 2^31-1, this method
+    /// does not check the range; it accepts any unsigned 32-bit integer
+    /// value.  In practice, this shouldn't cause a problem, however, because
+    /// the only expected usage of this value is to use the minimum of this
+    /// value and SOA RR's TTL, and the latter is expected to be in the
+    /// valid range.
+    ///
+    /// \throw None
+    /// \param min_ttl_val The minimum TTL value as unsigned 32-bit integer
+    /// in the host byte order.
+    void setMinTTL(uint32_t min_ttl_val);
     //@}
 
 private:
     const boost::interprocess::offset_ptr<ZoneTree> zone_tree_;
     const boost::interprocess::offset_ptr<ZoneNode> origin_node_;
     boost::interprocess::offset_ptr<NSEC3Data> nsec3_data_;
+    uint32_t min_ttl_;
 };
 
 } // namespace memory

src/lib/datasrc/memory/zone_data_updater.cc

@@ -336,6 +336,17 @@ ZoneDataUpdater::addRdataSet(const Name& name, const RRType& rrtype,
             // "NSEC signed")
             zone_data_.setSigned(true);
         }
+
+        // If we are adding a new SOA at the origin, update zone's min TTL.
+        // Note: if the input is broken and contains multiple SOAs, the load
+        // or update will be rejected higher level.  We just always (though
+        // this should be only once in normal cases) update the TTL.
+        if (rrset && rrtype == RRType::SOA() && is_origin) {
+            // Our own validation ensures the RRset is not empty.
+            zone_data_.setMinTTL(
+                dynamic_cast<const generic::SOA&>(
+                    rrset->getRdataIterator()->getCurrent()).getMinimum());
+        }
     }
 }
 

src/lib/datasrc/memory/zone_finder.cc

@@ -23,10 +23,13 @@
 #include <dns/name.h>
 #include <dns/rrset.h>
 #include <dns/rrtype.h>
+#include <dns/rrttl.h>
 #include <dns/nsec3hash.h>
 
 #include <datasrc/logger.h>
 
+#include <util/buffer.h>
+
 #include <boost/scoped_ptr.hpp>
 #include <boost/bind.hpp>
 
@@ -104,14 +107,19 @@ createTreeNodeRRset(const ZoneNode* node,
                     const RdataSet* rdataset,
                     const RRClass& rrclass,
                     ZoneFinder::FindOptions options,
-                    const Name* realname = NULL)
+                    const Name* realname = NULL,
+                    const void* ttl_data = NULL)
 {
     const bool dnssec = ((options & ZoneFinder::FIND_DNSSEC) != 0);
-    if (node != NULL && rdataset != NULL) {
-        if (realname != NULL) {
+    if (node && rdataset) {
+        if (realname) {
             return (TreeNodeRRsetPtr(new TreeNodeRRset(*realname, rrclass,
                                                        node, rdataset,
                                                        dnssec)));
+        } else if (ttl_data) {
+            assert(!realname);  // these two cases should be mixed in our use
+            return (TreeNodeRRsetPtr(new TreeNodeRRset(rrclass, node, rdataset,
+                                                       dnssec, ttl_data)));
         } else {
             return (TreeNodeRRsetPtr(new TreeNodeRRset(rrclass, node, rdataset,
                                                        dnssec)));
@@ -229,6 +237,12 @@ createNSEC3RRset(const ZoneNode* node, const RRClass& rrclass) {
                                 ZoneFinder::FIND_DNSSEC));
 }
 
+inline RRTTL
+createTTLFromData(const void* ttl_data) {
+    util::InputBuffer b(ttl_data, sizeof(uint32_t));
+    return (RRTTL(b));
+}
+
 // convenience function to fill in the final details
 //
 // Set up ZoneFinderResultContext object as a return value of find(),
@@ -250,7 +264,8 @@ createFindResult(const RRClass& rrclass,
                  const RdataSet* rdataset,
                  ZoneFinder::FindOptions options,
                  bool wild = false,
-                 const Name* qname = NULL)
+                 const Name* qname = NULL,
+                 bool use_minttl = false)
 {
     ZoneFinder::FindResultFlags flags = ZoneFinder::RESULT_DEFAULT;
     const Name* rename = NULL;
@@ -268,6 +283,15 @@ createFindResult(const RRClass& rrclass,
         }
     }
 
+    if (use_minttl && rdataset &&
+        createTTLFromData(zone_data.getMinTTLData()) <
+        createTTLFromData(rdataset->getTTLData())) {
+        return (ZoneFinderResultContext(
+                    code,
+                    createTreeNodeRRset(node, rdataset, rrclass, options,
+                                        rename, zone_data.getMinTTLData()),
+                    flags, zone_data, node, rdataset));
+    }
     return (ZoneFinderResultContext(code, createTreeNodeRRset(node, rdataset,
                                                               rrclass, options,
                                                               rename),
@@ -721,8 +745,8 @@ InMemoryZoneFinder::Context::findAdditional(
 
 boost::shared_ptr<ZoneFinder::Context>
 InMemoryZoneFinder::find(const isc::dns::Name& name,
-                const isc::dns::RRType& type,
-                const FindOptions options)
+                         const isc::dns::RRType& type,
+                         const FindOptions options)
 {
     return (ZoneFinderContextPtr(new Context(*this, options, rrclass_,
                                              findInternal(name, type,
@@ -731,8 +755,8 @@ InMemoryZoneFinder::find(const isc::dns::Name& name,
 
 boost::shared_ptr<ZoneFinder::Context>
 InMemoryZoneFinder::findAll(const isc::dns::Name& name,
-        std::vector<isc::dns::ConstRRsetPtr>& target,
-        const FindOptions options)
+                            std::vector<isc::dns::ConstRRsetPtr>& target,
+                            const FindOptions options)
 {
     return (ZoneFinderContextPtr(new Context(*this, options, rrclass_,
                                              findInternal(name,
@@ -741,6 +765,44 @@ InMemoryZoneFinder::findAll(const isc::dns::Name& name,
                                                           options))));
 }
 
+// The implementation is a special case of the generic findInternal: we know
+// the qname should have an "exact match" and its node is accessible via
+// getOriginNode(); and, since there should be at least SOA RR at the origin
+// the case of CNAME can be eliminated (these should be guaranteed at the load
+// or update time, but even if they miss a corner case and allows a CNAME to
+// be added at origin, the zone is broken anyway, so we'd just let this
+// method return garbage, too).  As a result, there can be only too cases
+// for the result codes: SUCCESS if the requested type of RR exists; NXRRSET
+// otherwise.  Due to its simplicity we implement it separately, rather than
+// sharing the code with findInternal.
+boost::shared_ptr<ZoneFinder::Context>
+InMemoryZoneFinder::findAtOrigin(const isc::dns::RRType& type,
+                                 bool use_minttl,
+                                 const FindOptions options)
+{
+    const ZoneNode* const node = zone_data_.getOriginNode();
+    const RdataSet* const found = RdataSet::find(node->getData(), type);
+
+    if (found != NULL) {
+        LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_FIND_TYPE_AT_ORIGIN).
+            arg(type).arg(getOrigin()).arg(rrclass_);
+        return (ZoneFinderContextPtr(
+                    new Context(*this, options, rrclass_,
+                                createFindResult(rrclass_, zone_data_, SUCCESS,
+                                                 node, found, options, false,
+                                                 NULL, use_minttl))));
+    }
+    return (ZoneFinderContextPtr(
+                    new Context(*this, options, rrclass_,
+                                createFindResult(rrclass_, zone_data_, NXRRSET,
+                                                 node,
+                                                 getNSECForNXRRSET(zone_data_,
+                                                                   options,
+                                                                   node),
+                                                 options, false, NULL,
+                                                 use_minttl))));
+}
+
 ZoneFinderResultContext
 InMemoryZoneFinder::findInternal(const isc::dns::Name& name,
                                  const isc::dns::RRType& type,

src/lib/datasrc/memory/zone_finder.h

@@ -60,6 +60,16 @@ public:
         const isc::dns::RRType& type,
         const FindOptions options = FIND_DEFAULT);
 
+    /// \brief Search for an RRset of given RR type at the zone origin
+    /// specialized for in-memory data source.
+    ///
+    /// This specialized version exploits internal data structure to find
+    /// RRsets at the zone origin and (if \c use_minttl is true) extract
+    /// the SOA Minimum TTL much more efficiently.
+    virtual boost::shared_ptr<ZoneFinder::Context> findAtOrigin(
+        const isc::dns::RRType& type, bool use_minttl,
+        FindOptions options);
+
     /// \brief Version of find that returns all types at once
     ///
     /// It acts the same as find, just that when the correct node is found,
@@ -108,3 +118,7 @@ private:
 } // namespace isc
 
 #endif // DATASRC_MEMORY_ZONE_FINDER_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/tests/memory/treenode_rrset_unittest.cc

@@ -194,8 +194,14 @@ checkBasicFields(const AbstractRRset& actual_rrset, const RdataSet* rdataset,
 // a temporary non-copyable object.
 boost::shared_ptr<TreeNodeRRset>
 createRRset(const RRClass& rrclass, const ZoneNode* node,
-            const RdataSet* rdataset, bool dnssec_ok)
+            const RdataSet* rdataset, bool dnssec_ok,
+            const void* ttl_data = NULL)
 {
+    if (ttl_data) {
+        return (boost::shared_ptr<TreeNodeRRset>(
+                    new TreeNodeRRset(rrclass, node, rdataset, dnssec_ok,
+                                      ttl_data)));
+    }
     return (boost::shared_ptr<TreeNodeRRset>(
                 new TreeNodeRRset(rrclass, node, rdataset, dnssec_ok)));
 }
@@ -243,6 +249,13 @@ TEST_F(TreeNodeRRsetTest, create) {
                                   true),
                      wildcard_rdataset_, match_name_, rrclass_, RRType::A(),
                      3600, 2, 1);
+
+    // Constructed with explicit TTL
+    const uint32_t ttl = 0;     // use 0 to avoid byte-order conversion
+    checkBasicFields(*createRRset(rrclass_, www_node_, a_rdataset_, true,
+                                  &ttl),
+                     a_rdataset_, www_name_, rrclass_, RRType::A(), 0, 2,
+                     1);
 }
 
 // The following two templated functions are helper to encapsulate the
@@ -337,6 +350,21 @@ TEST_F(TreeNodeRRsetTest, toWire) {
     }
 
     {
+        SCOPED_TRACE("with RRSIG, DNSSEC OK, explicit TTL");
+        const uint32_t ttl = 0;
+        const TreeNodeRRset rrset(rrclass_, www_node_, a_rdataset_, true,
+                                  &ttl);
+        checkToWireResult(expected_renderer, actual_renderer, rrset,
+                          www_name_,
+                          textToRRset("www.example.com. 0 IN A 192.0.2.1\n"
+                                      "www.example.com. 0 IN A 192.0.2.2"),
+                          textToRRset("www.example.com. 0 IN RRSIG "
+                                      "A 5 2 3600 20120814220826 "
+                                      "20120715220826 1234 example.com. FAKE"),
+                          true);
+    }
+
+    {
         SCOPED_TRACE("with RRSIG, DNSSEC not OK");
         const TreeNodeRRset rrset(rrclass_, www_node_, a_rdataset_, false);
         checkToWireResult(expected_renderer, actual_renderer, rrset,
@@ -396,7 +424,7 @@ TEST_F(TreeNodeRRsetTest, toWire) {
         const TreeNodeRRset rrset(rrclass_, www_node_, rrsig_only_rdataset_,
                                   true);
         checkToWireResult(expected_renderer, actual_renderer, rrset,
-                          www_name_, ConstRRsetPtr(), txt_rrsig_rrset_,true);
+                          www_name_, ConstRRsetPtr(), txt_rrsig_rrset_, true);
     }
 
     {
@@ -407,7 +435,7 @@ TEST_F(TreeNodeRRsetTest, toWire) {
         const TreeNodeRRset rrset(rrclass_, www_node_, rrsig_only_rdataset_,
                                   false);
         checkToWireResult(expected_renderer, actual_renderer, rrset,
-                          www_name_, ConstRRsetPtr(), txt_rrsig_rrset_,false);
+                          www_name_, ConstRRsetPtr(), txt_rrsig_rrset_, false);
     }
 }
 
@@ -522,6 +550,14 @@ TEST_F(TreeNodeRRsetTest, toText) {
     // Constructed with RRSIG, and it should be visible.
     checkToText(*createRRset(rrclass_, www_node_, a_rdataset_, true),
                 a_rrset_, a_rrsig_rrset_);
+    // Same as the previous, but with explicit TTL.
+    const uint32_t ttl = 0;
+    checkToText(*createRRset(rrclass_, www_node_, a_rdataset_, true, &ttl),
+                textToRRset("www.example.com. 0 IN A 192.0.2.1\n"
+                            "www.example.com. 0 IN A 192.0.2.2"),
+                textToRRset("www.example.com. 0 IN RRSIG A 5 2 3600 "
+                            "20120814220826 20120715220826 1234 example.com. "
+                            "FAKE"));
     // Constructed with RRSIG, and it should be invisible.
     checkToText(*createRRset(rrclass_, www_node_, a_rdataset_, false),
                 a_rrset_, ConstRRsetPtr());
@@ -556,6 +592,11 @@ TEST_F(TreeNodeRRsetTest, isSameKind) {
     EXPECT_TRUE(rrset.isSameKind(*createRRset(rrclass_, www_node_,
                                               a_rdataset_, true)));
 
+    // Similar to the previous, but with explicit (different TTL) => still same
+    const uint32_t ttl = 0;
+    EXPECT_TRUE(rrset.isSameKind(*createRRset(rrclass_, www_node_,
+                                              a_rdataset_, true, &ttl)));
+
     // Same name (node), different type (rdataset) => not same kind
     EXPECT_FALSE(rrset.isSameKind(*createRRset(rrclass_, www_node_,
                                                aaaa_rdataset_, true)));

src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc

@@ -12,13 +12,15 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <dns/name.h>
-#include <dns/rrclass.h>
-
+#include <datasrc/memory/zone_data_loader.h>
 #include <datasrc/memory/rdataset.h>
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_data_updater.h>
-#include <datasrc/memory/zone_data_loader.h>
+
+#include <util/buffer.h>
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
 
 #include "memory_segment_test.h"
 
@@ -62,4 +64,13 @@ TEST_F(ZoneDataLoaderTest, loadRRSIGFollowsNothing) {
     // Teardown checks for memory segment leaks
 }
 
+TEST_F(ZoneDataLoaderTest, zoneMinTTL) {
+    // This should hold outside of the loader class, but we do double check.
+    zone_data_ = loadZoneData(mem_sgmt_, zclass_, Name("example.org"),
+                              TEST_DATA_DIR
+                              "/example.org-nsec3-signed.zone");
+    isc::util::InputBuffer b(zone_data_->getMinTTLData(), sizeof(uint32_t));
+    EXPECT_EQ(RRTTL(1200), RRTTL(b));
+}
+
 }

src/lib/datasrc/tests/memory/zone_data_unittest.cc

@@ -12,19 +12,22 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <datasrc/memory/zone_data.h>
+#include <datasrc/memory/rdata_serialization.h>
+#include <datasrc/memory/rdataset.h>
+
 #include "memory_segment_test.h"
 
 #include <dns/rdataclass.h>
 
 #include <exceptions/exceptions.h>
 
+#include <util/buffer.h>
+
 #include <dns/name.h>
 #include <dns/labelsequence.h>
 #include <dns/rrclass.h>
-
-#include <datasrc/memory/rdata_serialization.h>
-#include <datasrc/memory/rdataset.h>
-#include <datasrc/memory/zone_data.h>
+#include <dns/rrttl.h>
 
 #include <testutils/dnsmessage_test.h>
 
@@ -258,4 +261,21 @@ TEST_F(ZoneDataTest, isSigned) {
     zone_data_->setSigned(false);
     EXPECT_FALSE(zone_data_->isSigned());
 }
+
+// A simple wrapper to reconstruct an RRTTL object from wire-format TTL
+// data (32 bits)
+RRTTL
+createRRTTL(const void* ttl_data) {
+    isc::util::InputBuffer b(ttl_data, sizeof(uint32_t));
+    return (RRTTL(b));
+}
+
+TEST_F(ZoneDataTest, minTTL) {
+    // By default it's tentatively set to "max TTL"
+    EXPECT_EQ(RRTTL::MAX_TTL(), createRRTTL(zone_data_->getMinTTLData()));
+
+    // Explicitly set, then retrieve it.
+    zone_data_->setMinTTL(1200);
+    EXPECT_EQ(RRTTL(1200), createRRTTL(zone_data_->getMinTTLData()));
+}
 }

src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc

@@ -12,6 +12,10 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <datasrc/memory/zone_data_updater.h>
+#include <datasrc/memory/rdataset.h>
+#include <datasrc/memory/zone_data.h>
+
 #include <testutils/dnsmessage_test.h>
 
 #include <exceptions/exceptions.h>
@@ -19,10 +23,7 @@
 #include <dns/name.h>
 #include <dns/rrclass.h>
 #include <dns/rrset.h>
-
-#include <datasrc/memory/rdataset.h>
-#include <datasrc/memory/zone_data.h>
-#include <datasrc/memory/zone_data_updater.h>
+#include <dns/rrttl.h>
 
 #include "memory_segment_test.h"
 
@@ -86,6 +87,16 @@ getNode(isc::util::MemorySegment& mem_sgmt, const Name& name,
     return (node);
 }
 
+TEST_F(ZoneDataUpdaterTest, zoneMinTTL) {
+    // If we add SOA, zone's min TTL will be updated.
+    updater_->add(textToRRset(
+                      "example.org. 3600 IN SOA . . 0 0 0 0 1200",
+                      zclass_, zname_),
+                  ConstRRsetPtr());
+    isc::util::InputBuffer b(zone_data_->getMinTTLData(), sizeof(uint32_t));
+    EXPECT_EQ(RRTTL(1200), RRTTL(b));
+}
+
 TEST_F(ZoneDataUpdaterTest, rrsigOnly) {
     // RRSIG that doesn't have covered RRset can be added.  The resulting
     // rdataset won't have "normal" RDATA but sig RDATA.

src/lib/datasrc/tests/memory/zone_finder_unittest.cc

@@ -53,19 +53,6 @@ namespace {
 using result::SUCCESS;
 using result::EXIST;
 
-/// \brief expensive rrset converter
-///
-/// converts any specialized rrset (which may not have implemented some
-/// methods for efficiency) into a 'full' RRsetPtr, for easy use in test
-/// checks.
-///
-/// Done very inefficiently through text representation, speed should not
-/// be a concern here.
-ConstRRsetPtr
-convertRRset(ConstRRsetPtr src) {
-    return (textToRRset(src->toText()));
-}
-
 /// \brief Test fixture for the InMemoryZoneFinder class
 class InMemoryZoneFinderTest : public ::testing::Test {
     // A straightforward pair of textual RR(set) and a RRsetPtr variable
@@ -105,7 +92,6 @@ protected:
                           ZoneFinder::FindResultFlags expected_flags =
                           ZoneFinder::RESULT_DEFAULT);
 
-public:
     InMemoryZoneFinderTest() :
         class_(RRClass::IN()),
         origin_("example.org"),
@@ -119,6 +105,7 @@ public:
         // Note that this contains an out-of-zone RR, and due to the
         // validation check of masterLoad() used below, we cannot add SOA.
         const RRsetData zone_data[] = {
+            {"example.org. 300 IN SOA . . 0 0 0 0 100", &rr_soa_},
             {"example.org. 300 IN NS ns.example.org.", &rr_ns_},
             {"example.org. 300 IN A 192.0.2.1", &rr_a_},
             {"ns.example.org. 300 IN A 192.0.2.2", &rr_ns_a_},
@@ -185,7 +172,18 @@ public:
         };
 
         for (unsigned int i = 0; zone_data[i].text != NULL; ++i) {
-            *zone_data[i].rrset = textToRRset(zone_data[i].text);
+            if (zone_data[i].rrset == &rr_soa_) {
+                // This is zone's SOA.  We need to specify the origin for
+                // textToRRset; otherwise it would throw.
+                *zone_data[i].rrset = textToRRset(zone_data[i].text, class_,
+                                                  origin_);
+            } else {
+                // For other data, we should rather omit the origin (the root
+                // name will be used by default); there's some out-of-zone
+                // name, which would trigger an exception if we specified
+                // origin_.
+                *zone_data[i].rrset = textToRRset(zone_data[i].text);
+            }
         }
 
     }
@@ -200,6 +198,24 @@ public:
         updater_.add(rrset, rrset->getRRsig());
     }
 
+    /// \brief expensive rrset converter
+    ///
+    /// converts any specialized rrset (which may not have implemented some
+    /// methods for efficiency) into a 'full' RRsetPtr, for easy use in test
+    /// checks.
+    ///
+    /// Done very inefficiently through text representation, speed should not
+    /// be a concern here.
+    ConstRRsetPtr
+    convertRRset(ConstRRsetPtr src) {
+        // If the type is SOA, textToRRset performs a stricter check, so we
+        // should specify the origin.  For now we don't use out-of-zone
+        // owner names (e.g. for pathological cases) with this method, so it
+        // works for all test data.  If future changes break this assumption
+        // we should adjust it.
+        return (textToRRset(src->toText(), class_, origin_));
+    }
+
     // Some data to test with
     const RRClass class_;
     const Name origin_;
@@ -218,6 +234,8 @@ public:
     RRsetPtr
         // Out of zone RRset
         rr_out_,
+        // SOA of example.org
+        rr_soa_,
         // NS of example.org
         rr_ns_,
         // A of ns.example.org
@@ -293,75 +311,110 @@ public:
         if (zone_finder == NULL) {
             zone_finder = &zone_finder_;
         }
-        const ConstRRsetPtr answer_sig = answer ? answer->getRRsig() :
-            RRsetPtr(); // note we use the same type as of retval of getRRsig()
         // The whole block is inside, because we need to check the result and
         // we can't assign to FindResult
         EXPECT_NO_THROW({
                 ZoneFinderContextPtr find_result(zone_finder->find(
                                                      name, rrtype, options));
-                // Check it returns correct answers
-                EXPECT_EQ(result, find_result->code);
-                EXPECT_EQ((expected_flags & ZoneFinder::RESULT_WILDCARD) != 0,
-                          find_result->isWildcard());
-                EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED)
-                          != 0, find_result->isNSECSigned());
-                EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED)
-                          != 0, find_result->isNSEC3Signed());
-                if (check_answer) {
-                    if (!answer) {
-                        ASSERT_FALSE(find_result->rrset);
-                    } else {
-                        ASSERT_TRUE(find_result->rrset);
-                        ConstRRsetPtr result_rrset(
-                            convertRRset(find_result->rrset));
-                        rrsetCheck(answer, result_rrset);
-                        if (answer_sig &&
-                            (options & ZoneFinder::FIND_DNSSEC) != 0) {
-                            ASSERT_TRUE(result_rrset->getRRsig());
-                            rrsetCheck(answer_sig, result_rrset->getRRsig());
-                        } else {
-                            EXPECT_FALSE(result_rrset->getRRsig());
-                        }
-                    }
-                } else if (check_wild_answer) {
-                    ASSERT_NE(ConstRRsetPtr(), answer) <<
-                        "Wrong test, don't check for wild names if you expect "
-                        "empty answer";
-                    ASSERT_NE(ConstRRsetPtr(), find_result->rrset) <<
-                        "No answer found";
-                    // Build the expected answer using the given name and
-                    // other parameter of the base wildcard RRset.
-                    RRsetPtr wildanswer(new RRset(name, answer->getClass(),
-                                                  answer->getType(),
-                                                  answer->getTTL()));
-                    RdataIteratorPtr expectedIt(answer->getRdataIterator());
-                    for (; !expectedIt->isLast(); expectedIt->next()) {
-                        wildanswer->addRdata(expectedIt->getCurrent());
-                    }
-
-                    ConstRRsetPtr result_rrset(
-                        convertRRset(find_result->rrset));
-                    rrsetCheck(wildanswer, result_rrset);
-
-                    // Same for the RRSIG, if any.
-                    if (answer_sig) {
-                        ASSERT_TRUE(result_rrset->getRRsig());
-
-                        RRsetPtr wildsig(new RRset(name,
-                                                   answer_sig->getClass(),
-                                                   RRType::RRSIG(),
-                                                   answer_sig->getTTL()));
-                        RdataIteratorPtr expectedIt(
-                            answer_sig->getRdataIterator());
-                        for (; !expectedIt->isLast(); expectedIt->next()) {
-                            wildsig->addRdata(expectedIt->getCurrent());
-                        }
-                        rrsetCheck(wildsig, result_rrset->getRRsig());
-                    }
-                }
+                findTestCommon(name, result, find_result, check_answer,
+                               answer, expected_flags, options,
+                               check_wild_answer);
             });
     }
+
+    void findAtOriginTest(const RRType& rrtype,
+                          ZoneFinder::Result result,
+                          bool check_answer = true,
+                          const ConstRRsetPtr& answer = ConstRRsetPtr(),
+                          ZoneFinder::FindResultFlags expected_flags =
+                          ZoneFinder::RESULT_DEFAULT,
+                          memory::InMemoryZoneFinder* zone_finder = NULL,
+                          ZoneFinder::FindOptions options =
+                          ZoneFinder::FIND_DEFAULT,
+                          bool use_minttl = false)
+    {
+        SCOPED_TRACE("findAtOriginTest for " + rrtype.toText());
+
+        if (zone_finder == NULL) {
+            zone_finder = &zone_finder_;
+        }
+        ZoneFinderContextPtr find_result(zone_finder->findAtOrigin(
+                                             rrtype, use_minttl, options));
+        findTestCommon(origin_, result, find_result, check_answer, answer,
+                       expected_flags, options, false);
+    }
+
+private:
+    void findTestCommon(const Name& name, ZoneFinder::Result result,
+                        ZoneFinderContextPtr find_result,
+                        bool check_answer,
+                        const ConstRRsetPtr& answer,
+                        ZoneFinder::FindResultFlags expected_flags,
+                        ZoneFinder::FindOptions options,
+                        bool check_wild_answer)
+    {
+        const ConstRRsetPtr answer_sig = answer ? answer->getRRsig() :
+            RRsetPtr(); // note we use the same type as of retval of getRRsig()
+
+        // Check it returns correct answers
+        EXPECT_EQ(result, find_result->code);
+        EXPECT_EQ((expected_flags & ZoneFinder::RESULT_WILDCARD) != 0,
+                  find_result->isWildcard());
+        EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0,
+                  find_result->isNSECSigned());
+        EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) != 0,
+                  find_result->isNSEC3Signed());
+        if (check_answer) {
+            if (!answer) {
+                ASSERT_FALSE(find_result->rrset);
+            } else {
+                ASSERT_TRUE(find_result->rrset);
+                ConstRRsetPtr result_rrset(convertRRset(find_result->rrset));
+                rrsetCheck(answer, result_rrset);
+                if (answer_sig && (options & ZoneFinder::FIND_DNSSEC) != 0) {
+                    ASSERT_TRUE(result_rrset->getRRsig());
+                    rrsetCheck(answer_sig, result_rrset->getRRsig());
+                } else {
+                    EXPECT_FALSE(result_rrset->getRRsig());
+                }
+            }
+        } else if (check_wild_answer) {
+            ASSERT_NE(ConstRRsetPtr(), answer) <<
+                "Wrong test, don't check for wild names if you expect "
+                "empty answer";
+            ASSERT_NE(ConstRRsetPtr(), find_result->rrset) <<
+                "No answer found";
+            // Build the expected answer using the given name and
+            // other parameter of the base wildcard RRset.
+            RRsetPtr wildanswer(new RRset(name, answer->getClass(),
+                                          answer->getType(),
+                                          answer->getTTL()));
+            RdataIteratorPtr expectedIt(answer->getRdataIterator());
+            for (; !expectedIt->isLast(); expectedIt->next()) {
+                wildanswer->addRdata(expectedIt->getCurrent());
+            }
+
+            ConstRRsetPtr result_rrset(convertRRset(find_result->rrset));
+            rrsetCheck(wildanswer, result_rrset);
+
+            // Same for the RRSIG, if any.
+            if (answer_sig) {
+                ASSERT_TRUE(result_rrset->getRRsig());
+
+                RRsetPtr wildsig(new RRset(name, answer_sig->getClass(),
+                                           RRType::RRSIG(),
+                                           answer_sig->getTTL()));
+                RdataIteratorPtr expectedIt(
+                    answer_sig->getRdataIterator());
+                for (; !expectedIt->isLast(); expectedIt->next()) {
+                    wildsig->addRdata(expectedIt->getCurrent());
+                }
+                rrsetCheck(wildsig, result_rrset->getRRsig());
+            }
+        }
+    }
+
+protected:
     /**
      * \brief Calls the findAll on the finder and checks the result.
      */
@@ -583,7 +636,6 @@ TEST_F(InMemoryZoneFinderTest, glue) {
     findTest(rr_child_glue_->getName(), RRType::A(), ZoneFinder::DELEGATION,
              true, rr_child_ns_);
 
-
     // If we do it in the "glue OK" mode, we should find the exact match.
     findTest(rr_child_glue_->getName(), RRType::A(), ZoneFinder::SUCCESS, true,
              rr_child_glue_, ZoneFinder::RESULT_DEFAULT, NULL,
@@ -619,6 +671,101 @@ TEST_F(InMemoryZoneFinderTest, glue) {
              NULL, ZoneFinder::FIND_GLUE_OK);
 }
 
+TEST_F(InMemoryZoneFinderTest, findAtOrigin) {
+    // Add origin NS.
+    rr_ns_->addRRsig(createRdata(RRType::RRSIG(), RRClass::IN(),
+                                "NS 5 3 3600 20120814220826 20120715220826 "
+                                "1234 example.org. FAKE"));
+    addToZoneData(rr_ns_);
+
+    // Specified type of RR exists, no DNSSEC
+    findAtOriginTest(RRType::NS(), ZoneFinder::SUCCESS, true, rr_ns_);
+
+    // Specified type of RR exists, with DNSSEC
+    findAtOriginTest(RRType::NS(), ZoneFinder::SUCCESS, true, rr_ns_,
+                     ZoneFinder::RESULT_DEFAULT, NULL,
+                     ZoneFinder::FIND_DNSSEC);
+
+    // Specified type of RR doesn't exist, no DNSSEC
+    findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET);
+
+    // Specified type of RR doesn't exist, with DNSSEC.  First, make the
+    // zone "NSEC-signed", then check.
+    rr_nsec_->addRRsig(createRdata(RRType::RRSIG(), RRClass::IN(),
+                                   "NSEC 5 3 3600 20120814220826 "
+                                   "20120715220826 1234 example.org. FAKE"));
+    addToZoneData(rr_nsec_);
+    findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET, true, rr_nsec_,
+                     ZoneFinder::RESULT_NSEC_SIGNED, NULL,
+                     ZoneFinder::FIND_DNSSEC);
+
+    // Specified type of RR doesn't exist, with DNSSEC, enabling NSEC3.  First,
+    // make the zone "NSEC3-signed" (by just installing NSEC3PARAM; we don't
+    // need to add NSEC3s for the purpose of this test), then check.
+    addToZoneData(textToRRset("example.org. 300 IN NSEC3PARAM "
+                            "1 0 12 aabbccdd"));
+    findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET, true, ConstRRsetPtr(),
+                     ZoneFinder::RESULT_NSEC3_SIGNED, NULL,
+                     ZoneFinder::FIND_DNSSEC);
+}
+
+TEST_F(InMemoryZoneFinderTest, findAtOriginWithMinTTL) {
+    // Install zone's SOA.  This also sets internal zone data min TTL field.
+    addToZoneData(rr_soa_);
+
+    // Specify the use of min TTL, then the resulting TTL should be derived
+    // from the SOA MINTTL (which is smaller).
+    findAtOriginTest(RRType::SOA(), ZoneFinder::SUCCESS, true,
+                     textToRRset("example.org. 100 IN SOA . . 0 0 0 0 100",
+                                 class_, origin_),
+                     ZoneFinder::RESULT_DEFAULT, NULL,
+                     ZoneFinder::FIND_DEFAULT, true);
+
+    // Add signed NS for the following test.
+    RRsetPtr ns_rrset(textToRRset("example.org. 300 IN NS ns.example.org."));
+    ns_rrset->addRRsig(createRdata(RRType::RRSIG(), RRClass::IN(),
+                                   "NS 5 3 3600 20120814220826 20120715220826 "
+                                   "1234 example.org. FAKE"));
+    addToZoneData(ns_rrset);
+
+    // If DNSSEC is requested, TTL of the RRSIG should also be the min.
+    ns_rrset->setTTL(RRTTL(100));      // reset TTL to the expected one
+    findAtOriginTest(RRType::NS(), ZoneFinder::SUCCESS, true, ns_rrset,
+                     ZoneFinder::RESULT_DEFAULT, NULL,
+                     ZoneFinder::FIND_DEFAULT, true);
+
+    // If we don't request the use of min TTL, the original TTL will be used.
+    findAtOriginTest(RRType::SOA(), ZoneFinder::SUCCESS, true, rr_soa_,
+                     ZoneFinder::RESULT_DEFAULT, NULL,
+                     ZoneFinder::FIND_DEFAULT, false);
+
+    // If the found RRset has a smaller TTL than SOA, the original TTL should
+    // win.
+    rr_a_->setTTL(RRTTL(10));
+    addToZoneData(rr_a_);
+    findAtOriginTest(RRType::A(), ZoneFinder::SUCCESS, true, rr_a_,
+                     ZoneFinder::RESULT_DEFAULT, NULL,
+                     ZoneFinder::FIND_DEFAULT, true);
+
+    // If no RRset is returned, use_minttl doesn't matter (it shouldn't cause
+    // disruption)
+    findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET, true, ConstRRsetPtr(),
+                     ZoneFinder::RESULT_DEFAULT, NULL,
+                     ZoneFinder::FIND_DEFAULT, true);
+
+    // If it results in NXRRSET with NSEC, and if we specify the use of min
+    // TTL, the NSEC and RRSIG should have the min TTL (again, though, this
+    // use case is not really the intended one)
+    rr_nsec_->addRRsig(createRdata(RRType::RRSIG(), RRClass::IN(),
+                                   "NSEC 5 3 3600 20120814220826 "
+                                   "20120715220826 1234 example.org. FAKE"));
+    addToZoneData(rr_nsec_);
+    rr_nsec_->setTTL(RRTTL(100)); // reset it to the expected one
+    findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET, true, rr_nsec_,
+                     ZoneFinder::RESULT_NSEC_SIGNED, NULL,
+                     ZoneFinder::FIND_DNSSEC, true);
+}
+
 /**
  * \brief Test searching.
  *

src/lib/dns/python/tests/rrset_python_test.py

@@ -23,7 +23,7 @@ import os
 from pydnspp import *
 
 class TestModuleSpec(unittest.TestCase):
-    
+
     def setUp(self):
         self.test_name = Name("test.example.com")
         self.test_domain = Name("example.com")
@@ -78,8 +78,8 @@ class TestModuleSpec(unittest.TestCase):
     def test_add_rdata(self):
         # no iterator to read out yet (TODO: add addition test once implemented)
 
-        self.assertRaises(TypeError, self.rrset_a.add_rdata, Rdata(RRType("NS"), RRClass("IN"), "test.name."))
-        pass
+        self.assertRaises(TypeError, self.rrset_a.add_rdata,
+                          Rdata(RRType("NS"), RRClass("IN"), "test.name."))
 
     def test_to_text(self):
         self.assertEqual("test.example.com. 3600 IN A 192.0.2.1\n"
@@ -126,6 +126,6 @@ class TestModuleSpec(unittest.TestCase):
         # they would leak.
         self.assertEqual(1, sys.getrefcount(self.rrset_a.get_rdata()))
         self.assertEqual(1, sys.getrefcount(self.rrset_a.get_rdata()[0]))
-        
+
 if __name__ == '__main__':
     unittest.main()

src/lib/dns/rdata/generic/mx_15.cc

@@ -71,7 +71,8 @@ MX::MX(const std::string& mx_str) :
         const uint32_t num =
             lexer.getNextToken(MasterToken::NUMBER).getNumber();
         if (num > 65535) {
-            isc_throw(InvalidRdataText, "Invalid MX preference");
+            isc_throw(InvalidRdataText, "Invalid MX preference in: "
+                      << mx_str);
         }
         preference_ = static_cast<uint16_t>(num);
 
@@ -111,7 +112,7 @@ MX::MX(MasterLexer& lexer, const Name* origin,
 {
     const uint32_t num = lexer.getNextToken(MasterToken::NUMBER).getNumber();
     if (num > 65535) {
-        isc_throw(InvalidRdataText, "Invalid MX preference");
+        isc_throw(InvalidRdataText, "Invalid MX preference: " << num);
     }
     preference_ = static_cast<uint16_t>(num);
 

src/lib/dns/tests/rdata_mx_unittest.cc

@@ -51,6 +51,9 @@ TEST_F(Rdata_MX_Test, badText) {
     // No origin and relative
     EXPECT_THROW(const generic::MX rdata_mx("10 mx.example.com"),
                  MissingNameOrigin);
+    // Extra text at end of line
+    EXPECT_THROW(const generic::MX rdata_mx("10 mx.example.com. extra."),
+                 InvalidRdataText);
 }
 
 TEST_F(Rdata_MX_Test, copy) {
@@ -70,6 +73,12 @@ TEST_F(Rdata_MX_Test, createFromLexer) {
         *test::createRdataUsingLexer(RRType::MX(), RRClass::IN(),
                                      "10 mx.example.com.")));
 
+    // test::createRdataUsingLexer() constructs relative to
+    // "example.org." origin.
+    EXPECT_EQ(0, generic::MX("10 mx2.example.org.").compare(
+        *test::createRdataUsingLexer(RRType::MX(), RRClass::IN(),
+                                     "10 mx2")));
+
     // Exceptions cause NULL to be returned.
     EXPECT_FALSE(test::createRdataUsingLexer(RRType::MX(), RRClass::IN(),
                                              "10 mx. example.com."));
@@ -77,6 +86,10 @@ TEST_F(Rdata_MX_Test, createFromLexer) {
     // 65536 is larger than maximum possible preference
     EXPECT_FALSE(test::createRdataUsingLexer(RRType::MX(), RRClass::IN(),
                                              "65536 mx.example.com."));
+
+    // Extra text at end of line
+    EXPECT_FALSE(test::createRdataUsingLexer(RRType::MX(), RRClass::IN(),
+                                             "10 mx.example.com. extra."));
 }
 
 TEST_F(Rdata_MX_Test, toWireRenderer) {

src/lib/dns/tests/rdata_ns_unittest.cc

@@ -61,6 +61,11 @@ TEST_F(Rdata_NS_Test, createFromText) {
                                                "ns.example.com.")));
 }
 
+TEST_F(Rdata_NS_Test, badText) {
+    // Extra input at end of line
+    EXPECT_THROW(generic::NS("ns.example.com. extra."), InvalidRdataText);
+}
+
 TEST_F(Rdata_NS_Test, createFromWire) {
     EXPECT_EQ(0, rdata_ns.compare(
                   *rdataFactoryFromFile(RRType("NS"), RRClass("IN"),
@@ -91,9 +96,19 @@ TEST_F(Rdata_NS_Test, createFromLexer) {
         *test::createRdataUsingLexer(RRType::NS(), RRClass::IN(),
                                      "ns.example.com.")));
 
+    // test::createRdataUsingLexer() constructs relative to
+    // "example.org." origin.
+    EXPECT_EQ(0, generic::NS("ns8.example.org.").compare(
+        *test::createRdataUsingLexer(RRType::NS(), RRClass::IN(),
+                                     "ns8")));
+
     // Exceptions cause NULL to be returned.
     EXPECT_FALSE(test::createRdataUsingLexer(RRType::NS(), RRClass::IN(),
                                              ""));
+
+    // Extra input at end of line
+    EXPECT_FALSE(test::createRdataUsingLexer(RRType::NS(), RRClass::IN(),
+                                             "ns.example.com. extra."));
 }
 
 TEST_F(Rdata_NS_Test, toWireBuffer) {

src/lib/dns/tests/rdata_ptr_unittest.cc

@@ -65,6 +65,11 @@ TEST_F(Rdata_PTR_Test, createFromText) {
                                                "ns.example.com.")));
 }
 
+TEST_F(Rdata_PTR_Test, badText) {
+    // Extra text at end of line
+    EXPECT_THROW(generic::PTR("foo.example.com. extra."), InvalidRdataText);
+}
+
 TEST_F(Rdata_PTR_Test, createFromWire) {
     EXPECT_EQ(0, rdata_ptr.compare(
                   *rdataFactoryFromFile(RRType("PTR"), RRClass("IN"),
@@ -94,6 +99,16 @@ TEST_F(Rdata_PTR_Test, createFromLexer) {
     EXPECT_EQ(0, rdata_ptr.compare(
         *test::createRdataUsingLexer(RRType::PTR(), RRClass::IN(),
                                      "ns.example.com.")));
+
+    // test::createRdataUsingLexer() constructs relative to
+    // "example.org." origin.
+    EXPECT_EQ(0, generic::PTR("foo0.example.org.").compare(
+        *test::createRdataUsingLexer(RRType::PTR(), RRClass::IN(),
+                                     "foo0")));
+
+    // Extra text at end of line
+    EXPECT_FALSE(test::createRdataUsingLexer(RRType::PTR(), RRClass::IN(),
+                                             "foo.example.com. extra."));
 }
 
 TEST_F(Rdata_PTR_Test, toWireBuffer) {

src/lib/python/isc/xfrin/diff.py

@@ -584,3 +584,16 @@ class Diff:
             if rr.get_name() == name:
                 new_rrsets.append(rr)
         return result, new_rrsets, flags
+
+    def get_rrset_collection(self):
+        '''
+        This first applies all changes to the data source. Then it creates
+        and returns an RRsetCollection on top of the corresponding zone
+        updater. Notice it might be impossible to apply more changes after
+        that.
+
+        This must not be called after a commit, or it'd throw ValueError.
+        '''
+        # Apply itself will check it is not yet commited.
+        self.apply()
+        return self.__updater.get_rrset_collection()

src/lib/python/isc/xfrin/tests/diff_tests.py

@@ -16,7 +16,8 @@
 import isc.log
 import unittest
 from isc.datasrc import ZoneFinder
-from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata
+from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata, \
+    RRsetCollectionBase
 from isc.xfrin.diff import Diff, NoSuchZone
 
 class TestError(Exception):
@@ -1087,6 +1088,52 @@ class DiffTest(unittest.TestCase):
             self.__check_find_all_call(diff.find_all, self.__rrset3,
                                        rcode)
 
+    class Collection(isc.dns.RRsetCollectionBase):
+        '''
+        Our own mock RRsetCollection. We only pass it through, but we
+        still define an (mostly empty) find method to satisfy the
+        expectations.
+        '''
+        def __init__(self):
+            '''
+            Empty init. The base class's __init__ can't be called,
+            so we need to provide our own to shadow it -- and make sure
+            not to call the super().__init__().
+            '''
+            pass
+
+        def find(self, name, rrclass, rrtype):
+            '''
+            Empty find method. Returns None to each query (pretends
+            the collection is empty. Present mostly for completeness.
+            '''
+            return None
+
+    def get_rrset_collection(self):
+        '''
+        Part of pretending to be the zone updater. This returns the rrset
+        collection (a mock one, unuseable) for the updater.
+        '''
+        return self.Collection()
+
+    def test_get_rrset_collection(self):
+        '''
+        Test the diff can return corresponding rrset collection. Test
+        it applies the data first.
+        '''
+        diff = Diff(self, Name('example.org'), single_update_mode=True)
+        diff.add_data(self.__rrset_soa)
+        collection = diff.get_rrset_collection()
+        # Check it is applied
+        self.assertEqual(1, len(self.__data_operations))
+        self.assertEqual('add', self.__data_operations[0][0])
+        # Check the returned one is actually RRsetCollection
+        self.assertTrue(isinstance(collection, self.Collection))
+        # The collection is just the mock from above, so this doesn't do much
+        # testing, but we check that the mock got through and didn't get hurt.
+        self.assertIsNone(collection.find(Name('example.org'), RRClass.IN(),
+                                          RRType.SOA()))
+
 if __name__ == "__main__":
     isc.log.init("bind10")
     isc.log.resetUnitTestRootLogger()

tests/lettuce/configurations/xfrin/retransfer_master_nons.conf.orig

@@ -0,0 +1,48 @@
+{
+    "version": 2,
+    "Logging": {
+        "loggers": [ {
+            "debuglevel": 99,
+            "severity": "DEBUG",
+            "name": "*"
+        } ]
+    },
+    "Auth": {
+        "database_file": "data/example.org-nons.sqlite3",
+        "listen_on": [ {
+            "address": "::1",
+            "port": 47807
+        } ]
+    },
+    "data_sources": {
+        "classes": {
+            "IN": [{
+                "type": "sqlite3",
+                "params": {
+                    "database_file": "data/example.org-nons.sqlite3"
+                }
+            }]
+        }
+    },
+    "Xfrout": {
+        "zone_config": [ {
+            "origin": "example.org"
+        } ],
+        "also_notify": [ {
+            "address": "::1",
+            "port": 47806
+        } ]
+    },
+    "Stats": {
+        "poll-interval": 1
+    },
+    "Boss": {
+        "components": {
+            "b10-auth": { "kind": "needed", "special": "auth" },
+            "b10-xfrout": { "address": "Xfrout", "kind": "dispensable" },
+            "b10-zonemgr": { "address": "Zonemgr", "kind": "dispensable" },
+            "b10-stats": { "address": "Stats", "kind": "dispensable" },
+            "b10-cmdctl": { "special": "cmdctl", "kind": "needed" }
+        }
+    }
+}

tests/lettuce/features/terrain/steps.py

@@ -30,7 +30,7 @@ def stop_a_named_process(step, process_name):
     """
     world.processes.stop_process(process_name)
 
-@step('wait (?:(\d+) times )?for (new )?(\w+) stderr message (.+)(?: not (.+))?')
+@step('wait (?:(\d+) times )?for (new )?(\w+) stderr message (\S+)(?: not (\S+))?')
 def wait_for_stderr_message(step, times, new, process_name, message, not_message):
     """
     Block until the given message is printed to the given process's stderr

tests/lettuce/features/terrain/terrain.py

@@ -64,6 +64,8 @@ copylist = [
      "configurations/ddns/noddns.config"],
     ["configurations/xfrin/retransfer_master.conf.orig",
      "configurations/xfrin/retransfer_master.conf"],
+    ["configurations/xfrin/retransfer_master_nons.conf.orig",
+     "configurations/xfrin/retransfer_master_nons.conf"],
     ["configurations/xfrin/retransfer_slave.conf.orig",
      "configurations/xfrin/retransfer_slave.conf"],
     ["data/inmem-xfrin.sqlite3.orig",
@@ -84,7 +86,7 @@ removelist = [
 # If we have waited OUTPUT_WAIT_MAX_INTERVALS times, we will abort with an
 # error (so as not to hang indefinitely)
 OUTPUT_WAIT_INTERVAL = 0.5
-OUTPUT_WAIT_MAX_INTERVALS = 20
+OUTPUT_WAIT_MAX_INTERVALS = 120
 
 # class that keeps track of one running process and the files
 # we created for it.

tests/lettuce/features/xfrin_bind10.feature

@@ -25,6 +25,13 @@ Feature: Xfrin
 
     A query for www.example.org to [::1]:47806 should have rcode REFUSED
     When I send bind10 the command Xfrin retransfer example.org IN ::1 47807
+    # The data we receive contain a NS RRset that refers to three names in the
+    # example.org. zone. All these three are nonexistent in the data, producing
+    # 3 separate warning messages in the log.
+    And wait for new bind10 stderr message XFRIN_ZONE_WARN
+    And wait for new bind10 stderr message XFRIN_ZONE_WARN
+    And wait for new bind10 stderr message XFRIN_ZONE_WARN
+    # But after complaining, the zone data should be accepted.
     Then wait for new bind10 stderr message XFRIN_TRANSFER_SUCCESS not XFRIN_XFR_PROCESS_FAILURE
     Then wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_SUCCESS
     A query for www.example.org to [::1]:47806 should have rcode NOERROR
@@ -38,7 +45,20 @@ Feature: Xfrin
     When I do an AXFR transfer of example.org
     Then transfer result should have 13 rrs
 
-
+    # Now try to offer another update. However, the validation of
+    # data should fail. The old version shoud still be available.
+    When I send bind10 the following commands with cmdctl port 47804:
+    """
+    config set data_sources/classes/IN[0]/params/database_file data/example.org-nons.sqlite3
+    config set Auth/database_file data/example.org-nons.sqlite3
+    config commit
+    """
+    Then I send bind10 the command Xfrin retransfer example.org IN ::1 47807
+    And wait for new bind10 stderr message XFRIN_ZONE_INVALID
+    And wait for new bind10 stderr message XFRIN_INVALID_ZONE_DATA
+    Then wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_FAILED
+    A query for example.org type NS to [::1]:47806 should have rcode NOERROR
+    And transfer result should have 13 rrs
 
     Scenario: Transfer with TSIG
     # Similar setup to the test above, but this time, we add TSIG configuration
@@ -74,7 +94,7 @@ Feature: Xfrin
 
     # Transfer should fail
     When I send bind10 the command Xfrin retransfer example.org
-    Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_PROTOCOL_ERROR not XFRIN_TRANSFER_SUCCESS
+    Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_PROTOCOL_VIOLATION not XFRIN_TRANSFER_SUCCESS
     # Set client to use TSIG as well
     When I send bind10 the following commands:
     """
@@ -86,3 +106,41 @@ Feature: Xfrin
     # Transwer should succeed now
     When I send bind10 the command Xfrin retransfer example.org
     Then wait for new bind10 stderr message XFRIN_TRANSFER_SUCCESS not XFRIN_XFR_PROCESS_FAILURE
+
+    Scenario: Validation fails
+    # In this test, the source data of the XFR is invalid (missing NS record
+    # at the origin). We check it is rejected after the transfer.
+    #
+    # We use abuse the fact that we do not check data when we read it from
+    # the sqlite3 database (unless we load into in-memory, which we don't
+    # do here).
+    The file data/test_nonexistent_db.sqlite3 should not exist
+
+    Given I have bind10 running with configuration xfrin/retransfer_master_nons.conf with cmdctl port 47804 as master
+    And wait for master stderr message BIND10_STARTED_CC
+    And wait for master stderr message CMDCTL_STARTED
+    And wait for master stderr message AUTH_SERVER_STARTED
+    And wait for master stderr message XFROUT_STARTED
+    And wait for master stderr message ZONEMGR_STARTED
+
+    And I have bind10 running with configuration xfrin/retransfer_slave.conf
+    And wait for bind10 stderr message BIND10_STARTED_CC
+    And wait for bind10 stderr message CMDCTL_STARTED
+    And wait for bind10 stderr message AUTH_SERVER_STARTED
+    And wait for bind10 stderr message XFRIN_STARTED
+    And wait for bind10 stderr message ZONEMGR_STARTED
+
+    # Now we use the first step again to see if the file has been created
+    The file data/test_nonexistent_db.sqlite3 should exist
+
+    A query for www.example.org to [::1]:47806 should have rcode REFUSED
+    When I send bind10 the command Xfrin retransfer example.org IN ::1 47807
+    # It should complain once about invalid data, then again that the whole
+    # zone is invalid and then reject it.
+    And wait for new bind10 stderr message XFRIN_ZONE_INVALID
+    And wait for new bind10 stderr message XFRIN_INVALID_ZONE_DATA
+    Then wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_FAILED
+    # The zone still doesn't exist as it is rejected.
+    # FIXME: This step fails. Probably an empty zone is created in the data
+    # source :-|. This should be REFUSED, not SERVFAIL.
+    A query for www.example.org to [::1]:47806 should have rcode SERVFAIL

tests/lettuce/features/xfrin_notify_handling.feature

@@ -123,7 +123,7 @@ Feature: Xfrin incoming notify handling
     Then wait for new bind10 stderr message AUTH_RECEIVED_NOTIFY
     Then wait for new bind10 stderr message ZONEMGR_RECEIVE_NOTIFY
     Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_STARTED
-    Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_PROTOCOL_ERROR not XFRIN_XFR_TRANSFER_STARTED
+    Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_PROTOCOL_VIOLATION not XFRIN_XFR_TRANSFER_STARTED
     Then wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_FAILED not ZONEMGR_RECEIVE_XFRIN_SUCCESS
     Then wait 5 times for new master stderr message NOTIFY_OUT_SENDING_NOTIFY
     Then wait for new master stderr message NOTIFY_OUT_RETRY_EXCEEDED