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

ChangeLog

@@ -1,4 +1,47 @@
-407.    [build]     haikuo
+413.	[func]		stephen, jelte
+	Created a new tool b10-dbutil, that can check and upgrade database
+	schemas, to be used when incompatible changes are introduced in the
+	backend database schema. Currently it only supports sqlite3 databases.
+	(Trac #963, git 49ba2cf8ac63246f389ab5e8ea3b3d081dba9adf)
+
+412.	[func]		jelte
+	Added a command-line option '--clear-config' to bind10, which causes
+	the system to create a backup of the existing configuration database
+	file, and start out with a clean default configuration. This can be
+	used if the configuration file is corrupted to the point where it
+	cannot be read anymore, and BIND 10 refuses to start. The name of
+	the backup file can be found in the logs (CFGMGR_RENAMED_CONFIG_FILE).
+	(Trac #1443, git 52b36c921ee59ec69deefb6123cbdb1b91dc3bc7)
+
+411.	[func]		muks
+	Add a -i/--no-kill command-line argument to bind10, which stops
+	it from sending SIGTERM and SIGKILL to other b10 processes when
+	they're shutting down.
+	(Trac #1819, git 774554f46b20ca5ec2ef6c6d5e608114f14e2102)
+
+410.	[bug]		jinmei
+	Python CC library now ensures write operations transmit all given
+	data (unless an error happens).  Previously it didn't check the
+	size of transmitted data, which could result in partial write on
+	some systems (notably on OpenBSD) and subsequently cause system
+	hang up or other broken state.  This fix specifically solves start
+	up failure on OpenBSD.
+	(Trac #1829, git 5e5a33213b60d89e146cd5e47d65f3f9833a9297)
+
+409.	[bug]		jelte
+	Fixed a parser bug in bindctl that could make bindctl crash. Also
+	improved 'command help' output; argument order is now shown
+	correctly, and parameter descriptions are shown as well.
+	(Trac #1172, git bec26c6137c9b0a59a3a8ca0f55a17cfcb8a23de)
+
+408.	[bug]		stephen, jinmei
+	b10-auth now filters out duplicate RRsets when building a
+	response message using the new query handling logic.  It's
+	currently only used with the in-memory data source, but will
+	also be used for others soon.
+	(Trac #1688, git b77baca56ffb1b9016698c00ae0a1496d603d197)
+
+407.    [build]		haikuo
 	Remove "--enable-boost-threads" switch in configure command. This
 	thread lock mechanism is useless for bind10 and causes performance 
 	hits. 

configure.ac

@@ -1015,6 +1015,9 @@ AC_CONFIG_FILES([Makefile
                  src/bin/cfgmgr/plugins/Makefile
                  src/bin/cfgmgr/plugins/tests/Makefile
                  src/bin/cfgmgr/tests/Makefile
+                 src/bin/dbutil/Makefile
+                 src/bin/dbutil/tests/Makefile
+                 src/bin/dbutil/tests/testdata/Makefile
                  src/bin/host/Makefile
                  src/bin/loadzone/Makefile
                  src/bin/loadzone/tests/correct/Makefile
@@ -1028,8 +1031,8 @@ AC_CONFIG_FILES([Makefile
                  src/bin/ddns/tests/Makefile
                  src/bin/dhcp6/Makefile
                  src/bin/dhcp6/tests/Makefile
-		 src/bin/dhcp4/Makefile
-		 src/bin/dhcp4/tests/Makefile
+                 src/bin/dhcp4/Makefile
+                 src/bin/dhcp4/tests/Makefile
                  src/bin/resolver/Makefile
                  src/bin/resolver/tests/Makefile
                  src/bin/sockcreator/Makefile
@@ -1143,6 +1146,9 @@ AC_OUTPUT([doc/version.ent
            src/bin/cmdctl/run_b10-cmdctl.sh
            src/bin/cmdctl/tests/cmdctl_test
            src/bin/cmdctl/cmdctl.spec.pre
+           src/bin/dbutil/dbutil.py
+           src/bin/dbutil/run_dbutil.sh
+           src/bin/dbutil/tests/dbutil_test.sh
            src/bin/ddns/ddns.py
            src/bin/xfrin/tests/xfrin_test
            src/bin/xfrin/xfrin.py
@@ -1226,6 +1232,8 @@ AC_OUTPUT([doc/version.ent
            chmod +x src/bin/zonemgr/run_b10-zonemgr.sh
            chmod +x src/bin/bind10/run_bind10.sh
            chmod +x src/bin/cmdctl/tests/cmdctl_test
+           chmod +x src/bin/dbutil/run_dbutil.sh
+           chmod +x src/bin/dbutil/tests/dbutil_test.sh
            chmod +x src/bin/xfrin/tests/xfrin_test
            chmod +x src/bin/xfrout/tests/xfrout_test
            chmod +x src/bin/zonemgr/tests/zonemgr_test

src/bin/Makefile.am

@@ -1,4 +1,4 @@
 SUBDIRS = bind10 bindctl cfgmgr ddns loadzone msgq host cmdctl auth xfrin \
-	xfrout usermgr zonemgr stats tests resolver sockcreator dhcp4 dhcp6
+	xfrout usermgr zonemgr stats tests resolver sockcreator dhcp4 dhcp6 dbutil
 
 check-recursive: all-recursive

src/bin/auth/query.cc

@@ -15,6 +15,7 @@
 #include <dns/message.h>
 #include <dns/rcode.h>
 #include <dns/rrtype.h>
+#include <dns/rrset.h>
 #include <dns/rdataclass.h>
 
 #include <datasrc/client.h>
@@ -25,7 +26,9 @@
 #include <boost/bind.hpp>
 #include <boost/function.hpp>
 
+#include <cassert>
 #include <algorithm>            // for std::max
+#include <functional>
 #include <vector>
 
 using namespace std;
@@ -33,33 +36,9 @@ using namespace isc::dns;
 using namespace isc::datasrc;
 using namespace isc::dns::rdata;
 
-// Commonly used helper callback object for vector<ConstRRsetPtr> to
-// insert them to (the specified section of) a message.
-namespace {
-class RRsetInserter {
-public:
-    RRsetInserter(Message& msg, Message::Section section, bool dnssec) :
-        msg_(msg), section_(section), dnssec_(dnssec)
-    {}
-    void operator()(const ConstRRsetPtr& rrset) {
-        /*
-         * FIXME:
-         * The const-cast is wrong, but the Message interface seems
-         * to insist.
-         */
-        msg_.addRRset(section_,
-                      boost::const_pointer_cast<AbstractRRset>(rrset),
-                      dnssec_);
-    }
-
-private:
-    Message& msg_;
-    const Message::Section section_;
-    const bool dnssec_;
-};
-
 // This is a "constant" vector storing desired RR types for the additional
 // section.  The vector is filled first time it's used.
+namespace {
 const vector<RRType>&
 A_AND_AAAA() {
     static vector<RRType> needed_types;
@@ -69,33 +48,58 @@ A_AND_AAAA() {
     }
     return (needed_types);
 }
+}
+
+namespace isc {
+namespace auth {
 
-// A wrapper for ZoneFinder::Context::getAdditional() so we don't include
-// duplicate RRs.  This is not efficient, and we should actually unify
-// this at the end of the process() method.  See also #1688.
 void
-getAdditional(const Name& qname, RRType qtype,
-              ZoneFinder::Context& ctx, vector<ConstRRsetPtr>& results)
+Query::ResponseCreator::addRRset(isc::dns::Message& message,
+                                 const isc::dns::Message::Section section,
+                                 const ConstRRsetPtr& rrset, const bool dnssec)
 {
-    vector<ConstRRsetPtr> additionals;
-    ctx.getAdditional(A_AND_AAAA(), additionals);
-
-    vector<ConstRRsetPtr>::const_iterator it = additionals.begin();
-    vector<ConstRRsetPtr>::const_iterator it_end = additionals.end();
-    for (; it != it_end; ++it) {
-        if ((qtype == (*it)->getType() || qtype == RRType::ANY()) &&
-            qname == (*it)->getName()) {
-            continue;
-        }
-        results.push_back(*it);
+    /// Is this RRset already in the list of RRsets added to the message?
+    const std::vector<const AbstractRRset*>::const_iterator i =
+        std::find_if(added_.begin(), added_.end(),
+                     std::bind1st(Query::ResponseCreator::IsSameKind(),
+                                  rrset.get()));
+    if (i == added_.end()) {
+        // No - add it to both the message and the list of RRsets processed.
+        // The const-cast is wrong, but the message interface seems to insist.
+        message.addRRset(section,
+                         boost::const_pointer_cast<AbstractRRset>(rrset),
+                         dnssec);
+        added_.push_back(rrset.get());
     }
 }
 
+void
+Query::ResponseCreator::create(Message& response,
+                               const vector<ConstRRsetPtr>& answers,
+                               const vector<ConstRRsetPtr>& authorities,
+                               const vector<ConstRRsetPtr>& additionals,
+                               const bool dnssec)
+{
+    // Inserter should be reset each time the query is reset, so should be
+    // empty at this point.
+    assert(added_.empty());
+
+    // Add the RRsets to the message.  The order of sections is important,
+    // as the ResponseCreator remembers RRsets added and will not add
+    // duplicates.  Adding in the order answer, authory, additional will
+    // guarantee that if there are duplicates, the single RRset added will
+    // appear in the most important section.
+    BOOST_FOREACH(const ConstRRsetPtr& rrset, answers) {
+        addRRset(response, Message::SECTION_ANSWER, rrset, dnssec);
+    }
+    BOOST_FOREACH(const ConstRRsetPtr& rrset, authorities) {
+        addRRset(response, Message::SECTION_AUTHORITY, rrset, dnssec);
+    }
+    BOOST_FOREACH(const ConstRRsetPtr& rrset, additionals) {
+        addRRset(response, Message::SECTION_ADDITIONAL, rrset, dnssec);
+    }
 }
 
-namespace isc {
-namespace auth {
-
 void
 Query::addSOA(ZoneFinder& finder) {
     ZoneFinderContextPtr soa_ctx = finder.find(finder.getOrigin(),
@@ -154,14 +158,10 @@ Query::addNXDOMAINProofByNSEC(ZoneFinder& finder, ConstRRsetPtr nsec) {
         isc_throw(BadNSEC, "Unexpected result for wildcard NXDOMAIN proof");
     }
 
-    // Add the (no-) wildcard proof only when it's different from the NSEC
-    // that proves NXDOMAIN; sometimes they can be the same.
-    // Note: name comparison is relatively expensive.  When we are at the
-    // stage of performance optimization, we should consider optimizing this
-    // for some optimized data source implementations.
-    if (nsec->getName() != fcontext->rrset->getName()) {
-        authorities_.push_back(fcontext->rrset);
-    }
+    // Add the (no-) wildcard proof.  This can be the same NSEC we already
+    // added, but we'd add it here anyway; duplicate checks will take place
+    // later in a unified manner.
+    authorities_.push_back(fcontext->rrset);
 }
 
 uint8_t
@@ -261,11 +261,8 @@ Query::addWildcardNXRRSETProof(ZoneFinder& finder, ConstRRsetPtr nsec) {
         fcontext->rrset->getRdataCount() == 0) {
         isc_throw(BadNSEC, "Unexpected result for no match QNAME proof");
     }
-   
-    if (nsec->getName() != fcontext->rrset->getName()) {
-        // one NSEC RR proves wildcard_nxrrset that no matched QNAME.
-        authorities_.push_back(fcontext->rrset);
-    }
+
+    authorities_.push_back(fcontext->rrset);
 }
 
 void
@@ -332,7 +329,7 @@ Query::addAuthAdditional(ZoneFinder& finder,
                   finder.getOrigin().toText());
     }
     authorities_.push_back(ns_context->rrset);
-    getAdditional(*qname_, *qtype_, *ns_context, additionals);
+    ns_context->getAdditional(A_AND_AAAA(), additionals);
 }
 
 namespace {
@@ -468,7 +465,7 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
             }
 
             // Retrieve additional records for the answer
-            getAdditional(*qname_, *qtype_, *db_context, additionals_);
+            db_context->getAdditional(A_AND_AAAA(), additionals_);
 
             // If apex NS records haven't been provided in the answer
             // section, insert apex NS records into the authority section
@@ -534,7 +531,8 @@ Query::process(datasrc::DataSourceClient& datasrc_client,
             break;
     }
 
-    createResponse();
+    response_creator_.create(*response_, answers_, authorities_, additionals_,
+                             dnssec_);
 }
 
 void
@@ -552,16 +550,6 @@ Query::initialize(datasrc::DataSourceClient& datasrc_client,
 }
 
 void
-Query::createResponse() {
-    for_each(answers_.begin(), answers_.end(),
-             RRsetInserter(*response_, Message::SECTION_ANSWER, dnssec_));
-    for_each(authorities_.begin(), authorities_.end(),
-             RRsetInserter(*response_, Message::SECTION_AUTHORITY, dnssec_));
-    for_each(additionals_.begin(), additionals_.end(),
-             RRsetInserter(*response_, Message::SECTION_ADDITIONAL, dnssec_));
-}
-
-void
 Query::reset() {
     datasrc_client_ = NULL;
     qname_ = NULL;
@@ -570,6 +558,7 @@ Query::reset() {
     answers_.clear();
     authorities_.clear();
     additionals_.clear();
+    response_creator_.clear();
 }
 
 bool
@@ -601,7 +590,8 @@ Query::processDSAtChild() {
         }
     }
 
-    createResponse();
+    response_creator_.create(*response_, answers_, authorities_, additionals_,
+                             dnssec_);
     return (true);
 }
 

src/bin/auth/query.h

@@ -20,6 +20,7 @@
 
 #include <boost/noncopyable.hpp>
 
+#include <functional>
 #include <vector>
 
 namespace isc {
@@ -36,13 +37,6 @@ class DataSourceClient;
 
 namespace auth {
 
-/// \brief Initial reserved size for the vectors in Query
-///
-/// The value is larger than we expect the vectors to even become, and
-/// has been chosen arbitrarily. The reason to set them quite high is
-/// to prevent reallocation on addition.
-const size_t RESERVE_RRSETS = 64;
-
 /// The \c Query class represents a standard DNS query that encapsulates
 /// processing logic to answer the query.
 ///
@@ -75,6 +69,12 @@ const size_t RESERVE_RRSETS = 64;
 /// we keep this name at the moment.
 class Query : boost::noncopyable {
 private:
+    /// \brief Initial reserved size for the vectors in Query
+    ///
+    /// The value is larger than we expect the vectors to even become, and
+    /// has been chosen arbitrarily. The reason to set them quite high is
+    /// to prevent reallocation on addition.
+    static const size_t RESERVE_RRSETS = 64;
 
     /// \brief Adds a SOA.
     ///
@@ -251,19 +251,6 @@ private:
                     const isc::dns::Name& qname, const isc::dns::RRType& qtype,
                     isc::dns::Message& response, bool dnssec = false);
 
-    /// \brief Fill in the response sections
-    ///
-    /// This is the final step of the process() method, and within
-    /// that method, it should be called before it returns (if any
-    /// response data is to be added)
-    ///
-    /// This will take each RRset collected in answers_, authorities_, and
-    /// additionals_, and add them to their corresponding sections in
-    /// the response message.
-    ///
-    /// After they are added, the vectors are cleared.
-    void createResponse();
-
     /// \brief Resets any partly built response data, and internal pointers
     ///
     /// Called by the QueryCleaner object upon its destruction
@@ -282,6 +269,12 @@ private:
         isc::auth::Query& query_;
     };
 
+protected:
+    // Following methods declared protected so they can be accessed
+    // by unit tests.
+
+    void createResponse();
+
 public:
     /// Default constructor.
     ///
@@ -289,8 +282,8 @@ public:
     ///
     Query() :
         datasrc_client_(NULL), qname_(NULL), qtype_(NULL),
-        response_(NULL), dnssec_(false),
-        dnssec_opt_(isc::datasrc::ZoneFinder::FIND_DEFAULT)
+        dnssec_(false), dnssec_opt_(isc::datasrc::ZoneFinder::FIND_DEFAULT),
+        response_(NULL)
     {
         answers_.reserve(RESERVE_RRSETS);
         authorities_.reserve(RESERVE_RRSETS);
@@ -402,14 +395,102 @@ public:
         {}
     };
 
+    /// \brief Response Creator Class
+    ///
+    /// This is a helper class of Query, and is expected to be used during the
+    /// construction of the response message. This class performs the
+    /// duplicate RRset detection check.  It keeps a list of RRsets added
+    /// to the message and does not add an RRset if it is the same as one
+    /// already added.
+    ///
+    /// This class is essentially private to Query, but is visible to public
+    /// for testing purposes.  It's not expected to be used from a normal
+    /// application.
+    class ResponseCreator {
+    public:
+        /// \brief Constructor
+        ///
+        /// Reserves space for the list of RRsets.  Although the
+        /// ResponseCreator will be used to create a message from the
+        /// contents of the Query object's answers_, authorities_ and
+        /// additionals_ elements, and each of these are sized to
+        /// RESERVE_RRSETS, it is _extremely_ unlikely that all three will be
+        /// filled to capacity.  So we reserve more elements than in each of
+        /// these components, but not three times the amount.
+        ///
+        /// As with the answers_, authorities_ and additionals_ elements, the
+        /// reservation is made in the constructor to avoid dynamic allocation
+        /// of memory.  The ResponseCreator is a member variable of the Query
+        /// object so is constructed once and lasts as long as that object.
+        /// Internal state is cleared through the clear() method.
+        ResponseCreator() {
+            added_.reserve(2 * RESERVE_RRSETS);
+        }
+
+        /// \brief Reset internal state
+        void clear() {
+            added_.clear();
+        }
+
+        /// \brief Complete the response message with filling in the
+        /// response sections.
+        ///
+        /// This is the final step of the Query::process() method, and within
+        /// that method, it should be called before it returns (if any
+        /// response data is to be added)
+        ///
+        /// This will take a message to build and each RRsets for the answer,
+        /// authority, and additional sections, and add them to their
+        /// corresponding sections in the given message.  The RRsets are
+        /// filtered such that a particular RRset appears only once in the
+        /// message.
+        ///
+        /// If \c dnssec is true, it tells the message to include any RRSIGs
+        /// attached to the RRsets.
+        void create(
+            isc::dns::Message& message,
+            const std::vector<isc::dns::ConstRRsetPtr>& answers_,
+            const std::vector<isc::dns::ConstRRsetPtr>& authorities_,
+            const std::vector<isc::dns::ConstRRsetPtr>& additionals_,
+            const bool dnssec);
+
+    private:
+        // \brief RRset comparison functor.
+        struct IsSameKind : public std::binary_function<
+                            const isc::dns::AbstractRRset*,
+                            const isc::dns::AbstractRRset*,
+                            bool> {
+            bool operator()(const isc::dns::AbstractRRset* r1,
+                            const isc::dns::AbstractRRset* r2) const {
+                return (r1->isSameKind(*r2));
+            }
+        };
+
+        /// Insertion operation
+        ///
+        /// \param message Message to which the RRset is to be added
+        /// \param section Section of the message in which the RRset is put
+        /// \param rrset Pointer to RRset to be added to the message
+        /// \param dnssec Whether RRSIG records should be added as well
+        void addRRset(isc::dns::Message& message,
+                      const isc::dns::Message::Section section,
+                      const isc::dns::ConstRRsetPtr& rrset, const bool dnssec);
+
+
+    private:
+        /// List of RRsets already added to the message
+        std::vector<const isc::dns::AbstractRRset*> added_;
+    };
+
 private:
     const isc::datasrc::DataSourceClient* datasrc_client_;
     const isc::dns::Name* qname_;
     const isc::dns::RRType* qtype_;
-    isc::dns::Message* response_;
     bool dnssec_;
     isc::datasrc::ZoneFinder::FindOptions dnssec_opt_;
+    ResponseCreator response_creator_;
 
+    isc::dns::Message* response_;
     std::vector<isc::dns::ConstRRsetPtr> answers_;
     std::vector<isc::dns::ConstRRsetPtr> authorities_;
     std::vector<isc::dns::ConstRRsetPtr> additionals_;

src/bin/auth/tests/query_unittest.cc

@@ -12,12 +12,13 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <map>
 #include <sstream>
 #include <vector>
-#include <map>
 
 #include <boost/bind.hpp>
 #include <boost/scoped_ptr.hpp>
+#include <boost/static_assert.hpp>
 
 #include <exceptions/exceptions.h>
 
@@ -2367,4 +2368,145 @@ TEST_F(QueryTest, emptyNameWithNSEC3) {
     EXPECT_TRUE(result->isNSEC3Signed());
     EXPECT_FALSE(result->isWildcard());
 }
+
+// Vector of RRsets used for the test.   Having this external to functions and
+// classes used for the testing simplifies the code.
+std::vector<RRsetPtr> rrset_vector;
+
+// Callback function for masterLoad.
+void
+loadRRsetVectorCallback(RRsetPtr rrsetptr) {
+    rrset_vector.push_back(rrsetptr);
+}
+
+// Load a set of RRsets into a vector for use in the duplicate RRset test.
+// They don't make a lot of sense as a zone, they are just different.  The
+// variables used in the stringstream input have been chosen so that each
+// represents just one RRset.
+void
+loadRRsetVector() {
+    stringstream ss;
+
+    // Comments indicate offset in the rrset_vector (when loaded) and the
+    // number of RRs in that RRset.
+    ss << soa_txt               // 0(1)
+       << zone_ns_txt           // 1(3)
+       << delegation_txt        // 2(4)
+       << delegation_ds_txt     // 3(1)
+       << mx_txt                // 4(3)
+       << www_a_txt             // 5(1)
+       << cname_txt             // 6(1)
+       << cname_nxdom_txt       // 7(1)
+       << cname_out_txt;        // 8(1)
+    rrset_vector.clear();
+    masterLoad(ss, Name("example.com."), RRClass::IN(),
+               loadRRsetVectorCallback);
+}
+
+TEST_F(QueryTest, DuplicateNameRemoval) {
+
+    // Load some RRsets into the master vector.
+    loadRRsetVector();
+    EXPECT_EQ(9, rrset_vector.size());
+
+    // Create an answer, authority and additional vector with some overlapping
+    // entries.  The following indicate which elements from rrset_vector
+    // go into each section vector.  (The values have been separated to show
+    // the overlap.)
+    //
+    // answer     = 0 1 2 3
+    // authority  =     2 3 4 5 6 7...
+    //                     ...5 (duplicate in the same section)
+    // additional = 0     3       7 8
+    //
+    // If the duplicate removal works, we should end up with the following in
+    // the message created from the three vectors:
+    //
+    // answer     = 0 1 2 3
+    // authority  =         4 5 6 7
+    // additional =                 8
+    //
+    // The expected section into which each RRset is placed is indicated in the
+    // array below.
+    const Message::Section expected_section[] = {
+        Message::SECTION_ANSWER,
+        Message::SECTION_ANSWER,
+        Message::SECTION_ANSWER,
+        Message::SECTION_ANSWER,
+        Message::SECTION_AUTHORITY,
+        Message::SECTION_AUTHORITY,
+        Message::SECTION_AUTHORITY,
+        Message::SECTION_AUTHORITY,
+        Message::SECTION_ADDITIONAL
+    };
+    EXPECT_EQ(rrset_vector.size(),
+              (sizeof(expected_section) / sizeof(Message::Section)));
+
+    // Create the vectors of RRsets (with the RRsets in a semi-random order).
+    std::vector<ConstRRsetPtr> answer;
+    answer.insert(answer.end(), rrset_vector.begin() + 2,
+                  rrset_vector.begin() + 4);
+    answer.insert(answer.end(), rrset_vector.begin() + 0,
+                  rrset_vector.begin() + 2);
+
+    std::vector<ConstRRsetPtr> authority;
+    authority.insert(authority.end(), rrset_vector.begin() + 3,
+                     rrset_vector.begin() + 8);
+    authority.push_back(rrset_vector[2]);
+    authority.push_back(rrset_vector[5]);
+
+    std::vector<ConstRRsetPtr> additional;
+    additional.insert(additional.end(), rrset_vector.begin() + 7,
+                      rrset_vector.end());
+    additional.push_back(rrset_vector[3]);
+    additional.push_back(rrset_vector[0]);
+
+    // Create the message object into which the RRsets are put
+    Message message(Message::RENDER);
+    EXPECT_EQ(0, message.getRRCount(Message::SECTION_ANSWER));
+    EXPECT_EQ(0, message.getRRCount(Message::SECTION_AUTHORITY));
+    EXPECT_EQ(0, message.getRRCount(Message::SECTION_ADDITIONAL));
+
+    // ... and fill it.
+    Query::ResponseCreator().create(message, answer, authority, additional,
+                                    false);
+
+    // Check counts in each section.  Note that these are RR counts,
+    // not RRset counts.
+    EXPECT_EQ(9, message.getRRCount(Message::SECTION_ANSWER));
+    EXPECT_EQ(6, message.getRRCount(Message::SECTION_AUTHORITY));
+    EXPECT_EQ(1, message.getRRCount(Message::SECTION_ADDITIONAL));
+
+    // ... and check that the RRsets are in the correct section
+    BOOST_STATIC_ASSERT(Message::SECTION_QUESTION == 0);
+    BOOST_STATIC_ASSERT(Message::SECTION_ANSWER == 1);
+    BOOST_STATIC_ASSERT(Message::SECTION_AUTHORITY == 2);
+    BOOST_STATIC_ASSERT(Message::SECTION_ADDITIONAL == 3);
+    Message::Section sections[] = {
+        Message::SECTION_QUESTION,
+        Message::SECTION_ANSWER,
+        Message::SECTION_AUTHORITY,
+        Message::SECTION_ADDITIONAL
+    };
+    for (int section = 1; section <= 3; ++section) {
+        for (int vecindex = 0; vecindex < rrset_vector.size(); ++vecindex) {
+            // Prepare error message in case an assertion fails (as the default
+            // message will only refer to the loop indexes).
+            stringstream ss;
+            ss << "section " << section << ", name "
+               << rrset_vector[vecindex]->getName();
+            SCOPED_TRACE(ss.str());
+
+            // Check RRset is in the right section and not in the wrong
+            // section.
+            if (sections[section] == expected_section[vecindex]) {
+                EXPECT_TRUE(message.hasRRset(sections[section],
+                            rrset_vector[vecindex]));
+            } else {
+                EXPECT_FALSE(message.hasRRset(sections[section],
+                             rrset_vector[vecindex]));
+            }
+        }
+    }
+}
 }

src/bin/bind10/bind10.xml

@@ -45,6 +45,7 @@
     <cmdsynopsis>
       <command>bind10</command>
       <arg><option>-c <replaceable>config-filename</replaceable></option></arg>
+      <arg><option>-i</option></arg>
       <arg><option>-m <replaceable>file</replaceable></option></arg>
       <arg><option>-n</option></arg>
       <arg><option>-p <replaceable>data_path</replaceable></option></arg>
@@ -56,6 +57,7 @@
       <arg><option>--data-path</option> <replaceable>directory</replaceable></arg>
       <arg><option>--msgq-socket-file <replaceable>file</replaceable></option></arg>
       <arg><option>--no-cache</option></arg>
+      <arg><option>--no-kill</option></arg>
       <arg><option>--pid-file</option> <replaceable>filename</replaceable></arg>
       <arg><option>--pretty-name <replaceable>name</replaceable></option></arg>
       <arg><option>--user <replaceable>user</replaceable></option></arg>
@@ -156,6 +158,16 @@
       </varlistentry>
 
       <varlistentry>
+        <term><option>-i</option>, <option>--no-kill</option></term>
+        <listitem>
+	  <para>When this option is passed, <command>bind10</command>
+	  does not send SIGTERM and SIGKILL signals to modules during
+	  shutdown. (This option was introduced for use during
+	  testing.)</para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><option>-u</option> <replaceable>user</replaceable>, <option>--user</option> <replaceable>name</replaceable></term>
 <!-- TODO: example more detail. -->
         <listitem>

src/bin/bind10/bind10_src.py.in

@@ -167,8 +167,9 @@ class BoB:
     """Boss of BIND class."""
     
     def __init__(self, msgq_socket_file=None, data_path=None,
-    config_filename=None, nocache=False, verbose=False, setuid=None,
-    username=None, cmdctl_port=None, wait_time=10):
+                 config_filename=None, clear_config=False, nocache=False,
+                 verbose=False, nokill=False, setuid=None, username=None,
+                 cmdctl_port=None, wait_time=10):
         """
             Initialize the Boss of BIND. This is a singleton (only one can run).
         
@@ -208,8 +209,10 @@ class BoB:
         self.uid = setuid
         self.username = username
         self.verbose = verbose
+        self.nokill = nokill
         self.data_path = data_path
         self.config_filename = config_filename
+        self.clear_config = clear_config
         self.cmdctl_port = cmdctl_port
         self.wait_time = wait_time
         self._component_configurator = isc.bind10.component.Configurator(self,
@@ -465,6 +468,8 @@ class BoB:
             args.append("--data-path=" + self.data_path)
         if self.config_filename is not None:
             args.append("--config-filename=" + self.config_filename)
+        if self.clear_config:
+            args.append("--clear-config")
         bind_cfgd = ProcessInfo("b10-cfgmgr", args,
                                 self.c_channel_env)
         bind_cfgd.spawn()
@@ -702,32 +707,36 @@ class BoB:
         # still not enough.
         time.sleep(1)
         self.reap_children()
-        # next try sending a SIGTERM
-        components_to_stop = list(self.components.values())
-        for component in components_to_stop:
-            logger.info(BIND10_SEND_SIGTERM, component.name(), component.pid())
-            try:
-                component.kill()
-            except OSError:
-                # ignore these (usually ESRCH because the child
-                # finally exited)
-                pass
-        # finally, send SIGKILL (unmaskable termination) until everybody dies
-        while self.components:
-            # XXX: some delay probably useful... how much is uncertain
-            time.sleep(0.1)  
-            self.reap_children()
+
+        # Send TERM and KILL signals to modules if we're not prevented
+        # from doing so
+        if not self.nokill:
+            # next try sending a SIGTERM
             components_to_stop = list(self.components.values())
             for component in components_to_stop:
-                logger.info(BIND10_SEND_SIGKILL, component.name(),
-                            component.pid())
+                logger.info(BIND10_SEND_SIGTERM, component.name(), component.pid())
                 try:
-                    component.kill(True)
+                    component.kill()
                 except OSError:
                     # ignore these (usually ESRCH because the child
                     # finally exited)
                     pass
-        logger.info(BIND10_SHUTDOWN_COMPLETE)
+            # finally, send SIGKILL (unmaskable termination) until everybody dies
+            while self.components:
+                # XXX: some delay probably useful... how much is uncertain
+                time.sleep(0.1)
+                self.reap_children()
+                components_to_stop = list(self.components.values())
+                for component in components_to_stop:
+                    logger.info(BIND10_SEND_SIGKILL, component.name(),
+                                component.pid())
+                    try:
+                        component.kill(True)
+                    except OSError:
+                        # ignore these (usually ESRCH because the child
+                        # finally exited)
+                        pass
+            logger.info(BIND10_SHUTDOWN_COMPLETE)
 
     def _get_process_exit_status(self):
         return os.waitpid(-1, os.WNOHANG)
@@ -1043,6 +1052,8 @@ def parse_args(args=sys.argv[1:], Parser=OptionParser):
                       help="UNIX domain socket file the b10-msgq daemon will use")
     parser.add_option("-n", "--no-cache", action="store_true", dest="nocache",
                       default=False, help="disable hot-spot cache in authoritative DNS server")
+    parser.add_option("-i", "--no-kill", action="store_true", dest="nokill",
+                      default=False, help="do not send SIGTERM and SIGKILL signals to modules during shutdown")
     parser.add_option("-u", "--user", dest="user", type="string", default=None,
                       help="Change user after startup (must run as root)")
     parser.add_option("-v", "--verbose", dest="verbose", action="store_true",
@@ -1053,6 +1064,10 @@ def parse_args(args=sys.argv[1:], Parser=OptionParser):
     parser.add_option("-c", "--config-file", action="store",
                       dest="config_file", default=None,
                       help="Configuration database filename")
+    parser.add_option("--clear-config", action="store_true",
+                      dest="clear_config", default=False,
+                      help="Create backup of the configuration file and " +
+                           "start with a clean configuration")
     parser.add_option("-p", "--data-path", dest="data_path",
                       help="Directory to search for configuration files",
                       default=None)
@@ -1165,9 +1180,10 @@ def main():
     try:
         # Go bob!
         boss_of_bind = BoB(options.msgq_socket_file, options.data_path,
-                           options.config_file, options.nocache,
-                           options.verbose, setuid, username,
-                           options.cmdctl_port, options.wait_time)
+                           options.config_file, options.clear_config,
+                           options.nocache, options.verbose, options.nokill,
+                           setuid, username, options.cmdctl_port,
+                           options.wait_time)
         startup_result = boss_of_bind.startup()
         if startup_result:
             logger.fatal(BIND10_STARTUP_ERROR, startup_result)

src/bin/bind10/tests/bind10_test.py.in

@@ -1012,6 +1012,22 @@ class TestParseArgs(unittest.TestCase):
         options = parse_args(['--config-file=config-file'], TestOptParser)
         self.assertEqual('config-file', options.config_file)
 
+    def test_clear_config(self):
+        options = parse_args([], TestOptParser)
+        self.assertEqual(False, options.clear_config)
+        options = parse_args(['--clear-config'], TestOptParser)
+        self.assertEqual(True, options.clear_config)
+
+    def test_nokill(self):
+        options = parse_args([], TestOptParser)
+        self.assertEqual(False, options.nokill)
+        options = parse_args(['--no-kill'], TestOptParser)
+        self.assertEqual(True, options.nokill)
+        options = parse_args([], TestOptParser)
+        self.assertEqual(False, options.nokill)
+        options = parse_args(['-i'], TestOptParser)
+        self.assertEqual(True, options.nokill)
+
     def test_cmdctl_port(self):
         """
         Test it can parse the command control port.
@@ -1194,7 +1210,60 @@ class TestBossComponents(unittest.TestCase):
         bob.shutdown()
 
         self.assertTrue(bob.ccs.stopped)
+
+        # Here, killed is an array where False is added if SIGTERM
+        # should be sent, or True if SIGKILL should be sent, in order in
+        # which they're sent.
         self.assertEqual([False, True], killed)
+
+        self.assertTrue(self.__called)
+
+        bob._component_configurator.shutdown = orig
+
+    def test_nokill(self):
+        """
+        Test that the boss *doesn't* kill components which don't want to
+        stop, when asked not to (by passing the --no-kill option which
+        sets bob.nokill to True).
+        """
+        bob = MockBob()
+        bob.nokill = True
+
+        killed = []
+        class ImmortalComponent:
+            """
+            An immortal component. It does not stop when it is told so
+            (anyway it is not told so). It does not die if it is killed
+            the first time. It dies only when killed forcefully.
+            """
+            def kill(self, forceful=False):
+                killed.append(forceful)
+                if forceful:
+                    bob.components = {}
+            def pid(self):
+                return 1
+            def name(self):
+                return "Immortal"
+        bob.components = {}
+        bob.register_process(1, ImmortalComponent())
+
+        # While at it, we check the configurator shutdown is actually called
+        orig = bob._component_configurator.shutdown
+        bob._component_configurator.shutdown = self.__nullary_hook
+        self.__called = False
+
+        bob.ccs = MockModuleCCSession()
+        self.assertFalse(bob.ccs.stopped)
+
+        bob.shutdown()
+
+        self.assertTrue(bob.ccs.stopped)
+
+        # Here, killed is an array where False is added if SIGTERM
+        # should be sent, or True if SIGKILL should be sent, in order in
+        # which they're sent.
+        self.assertEqual([], killed)
+
         self.assertTrue(self.__called)
 
         bob._component_configurator.shutdown = orig

src/bin/bindctl/bindcmd.py

@@ -319,6 +319,8 @@ class BindCmdInterpreter(Cmd):
                                   param_spec = arg)
                 if ("item_default" in arg):
                     param.default = arg["item_default"]
+                if ("item_description" in arg):
+                    param.desc = arg["item_description"]
                 cmd.add_param(param)
             module.add_command(cmd)
         self.add_module_info(module)
@@ -361,12 +363,12 @@ class BindCmdInterpreter(Cmd):
                 if type(name) == int:
                     # lump all extraneous arguments together as one big final one
                     # todo: check if last param type is a string?
-                    if (param_count > 2):
-                        while (param_count > len(command_info.params) - 1):
-                            params[param_count - 2] += params[param_count - 1]
-                            del(params[param_count - 1])
-                            param_count = len(params)
-                            cmd.params = params.copy()
+                    while (param_count > 2 and
+                           param_count > len(command_info.params) - 1):
+                        params[param_count - 2] += " " + params[param_count - 1]
+                        del(params[param_count - 1])
+                        param_count = len(params)
+                        cmd.params = params.copy()
 
                     # (-1, help is always in the all_params list)
                     if name >= len(all_params) - 1:

src/bin/bindctl/moduleinfo.py

@@ -57,8 +57,12 @@ class ParamInfo:
     def __str__(self):        
         return str("\t%s <type: %s> \t(%s)" % (self.name, self.type, self.desc))
 
-    def get_name(self):
-        return "%s <type: %s>" % (self.name, self.type)
+    def get_basic_info(self):
+        if self.is_optional:
+            opt_str = "optional"
+        else:
+            opt_str = "mandatory"
+        return "%s (%s, %s)" % (self.name, self.type, opt_str)
 
     def get_desc(self):
         return self.desc
@@ -155,37 +159,24 @@ class CommandInfo:
         """Prints the help info for this command to stdout"""
         print("Command ", self)
         print("\t\thelp (Get help for command)")
-                
+
         params = self.params.copy()
         del params["help"]
 
         if len(params) == 0:
-            print("No parameters for the command")
+            print("This command has no parameters")
             return
-        
-        print("\nMandatory parameters:")
-        mandatory_infos = []
-        for info in params.values():            
-            if not info.is_optional:
-                print("    %s" % info.get_name())
-                print(textwrap.fill(info.get_desc(),
-                      initial_indent="        ",
-                      subsequent_indent="        ",
-                      width=70))
-                mandatory_infos.append(info)
 
-        optional_infos = [info for info in params.values() 
-                          if info not in mandatory_infos]
-        if len(optional_infos) > 0:
-            print("\nOptional parameters:")      
-            for info in optional_infos:
-                print("    %s" % info.get_name())
-                print(textwrap.fill(info.get_desc(),
+        print("Parameters:")
+        for info in params.values():
+            print("    %s" % info.get_basic_info())
+            description = info.get_desc()
+            if description != "":
+                print(textwrap.fill(description,
                       initial_indent="        ",
                       subsequent_indent="        ",
                       width=70))
 
-
 class ModuleInfo:
     """Define the information of one module, include module name, 
     module supporting commands.

src/bin/bindctl/tests/bindctl_test.py

@@ -180,12 +180,10 @@ class TestCmdSyntax(unittest.TestCase):
         self.my_assert_raise(isc.cc.data.DataTypeError, "zone set zone_name ='cn', port='cn'")
         self.no_assert_raise("zone reload_all")
 
-
     def testCmdUnknownModuleSyntaxError(self):
         self.my_assert_raise(CmdUnknownModuleSyntaxError, "zoned d")
         self.my_assert_raise(CmdUnknownModuleSyntaxError, "dd dd  ")
 
-
     def testCmdUnknownCmdSyntaxError(self):
         self.my_assert_raise(CmdUnknownCmdSyntaxError, "zone dd")
 
@@ -198,6 +196,7 @@ class TestCmdSyntax(unittest.TestCase):
     def testCmdUnknownParamSyntaxError(self):
         self.my_assert_raise(CmdUnknownParamSyntaxError, "zone load zone_d='cn'")
         self.my_assert_raise(CmdUnknownParamSyntaxError, "zone reload_all zone_name = 'cn'")
+        self.my_assert_raise(CmdUnknownParamSyntaxError, "zone help a b c")
 
 class TestModuleInfo(unittest.TestCase):
 

src/bin/cfgmgr/b10-cfgmgr.py.in

@@ -49,6 +49,10 @@ def parse_options(args=sys.argv[1:], Parser=OptionParser):
                       help="Configuration database filename " +
                       "(default=" + DEFAULT_CONFIG_FILE + ")",
                       default=DEFAULT_CONFIG_FILE)
+    parser.add_option("--clear-config", action="store_true",
+                      dest="clear_config", default=False,
+                      help="Back up the configuration file and start with " +
+                           "a clean one")
     (options, args) = parser.parse_args(args)
     if args:
         parser.error("No non-option arguments allowed")
@@ -85,7 +89,8 @@ def main():
     options = parse_options()
     global cm
     try:
-        cm = ConfigManager(options.data_path, options.config_file)
+        cm = ConfigManager(options.data_path, options.config_file,
+                           None, options.clear_config)
         signal.signal(signal.SIGINT, signal_handler)
         signal.signal(signal.SIGTERM, signal_handler)
         cm.read_config()

src/bin/cfgmgr/tests/b10-cfgmgr_test.py.in

@@ -24,12 +24,13 @@ import bind10_config
 from isc.testutils.parse_args import OptsError, TestOptParser
 
 class MyConfigManager:
-    def __init__(self, path, filename):
+    def __init__(self, path, filename, session=None, rename_config_file=False):
         self._path = path
         self.read_config_called = False
         self.notify_boss_called = False
         self.run_called = False
         self.write_config_called = False
+        self.rename_config_called = False
         self.running = True
         self.virtual_modules = []
 
@@ -45,6 +46,9 @@ class MyConfigManager:
     def write_config(self):
         self.write_config_called = True
 
+    def rename_config_file(self, ofile, nfile):
+        self.rename_config_called = True
+
     def set_virtual_module(self, spec, function):
         self.virtual_modules.append((spec, function))
 
@@ -90,6 +94,7 @@ class TestConfigManagerStartup(unittest.TestCase):
         self.assertTrue(self.loaded_plugins)
         # if there are no changes, config is not written
         self.assertFalse(b.cm.write_config_called)
+        self.assertFalse(b.cm.rename_config_called)
 
         self.assertTrue(b.cm.running)
         b.signal_handler(None, None)
@@ -187,6 +192,14 @@ class TestParseArgs(unittest.TestCase):
         self.assertRaises(OptsError, b.parse_options, ['--config-filename'],
                           TestOptParser)
 
+    def test_clear_config(self):
+        b = __import__("b10-cfgmgr")
+        parsed = b.parse_options([], TestOptParser)
+        self.assertFalse(parsed.clear_config)
+        parsed = b.parse_options(['--clear-config'], TestOptParser)
+        self.assertTrue(parsed.clear_config)
+        
+
 if __name__ == '__main__':
     unittest.main()
 

src/bin/dbutil/Makefile.am

@@ -0,0 +1,38 @@
+SUBDIRS = . tests
+
+bin_SCRIPTS = b10-dbutil
+man_MANS = b10-dbutil.8
+
+nodist_pylogmessage_PYTHON = $(PYTHON_LOGMSGPKG_DIR)/work/dbutil_messages.py
+pylogmessagedir = $(pyexecdir)/isc/log_messages/
+
+EXTRA_DIST = $(man_MANS) b10-dbutil.xml dbutil_messages.mes
+
+noinst_SCRIPTS = run_dbutil.sh
+
+CLEANFILES = b10-dbutil b10-dbutil.pyc
+CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/dbutil_messages.py
+CLEANFILES += $(PYTHON_LOGMSGPKG_DIR)/work/dbutil_messages.pyc
+
+if ENABLE_MAN
+
+b10-dbutil.8: b10-dbutil.xml
+	xsltproc --novalid --xinclude --nonet -o $@ http://docbook.sourceforge.net/release/xsl/current/manpages/docbook.xsl $(srcdir)/b10-dbutil.xml
+
+endif
+
+# Define rule to build logging source files from message file
+$(PYTHON_LOGMSGPKG_DIR)/work/dbutil_messages.py : dbutil_messages.mes
+	$(top_builddir)/src/lib/log/compiler/message \
+	-d $(PYTHON_LOGMSGPKG_DIR)/work -p $(srcdir)/dbutil_messages.mes
+
+b10-dbutil: dbutil.py $(PYTHON_LOGMSGPKG_DIR)/work/dbutil_messages.py
+	$(SED) -e "s|@@PYTHONPATH@@|@pyexecdir@|" \
+	       -e "s|@@SYSCONFDIR@@|@sysconfdir@|" \
+	       -e "s|@@LIBEXECDIR@@|$(pkglibexecdir)|" dbutil.py >$@
+	chmod a+x $@
+
+CLEANDIRS = __pycache__
+
+clean-local:
+	rm -rf $(CLEANDIRS)

src/bin/dbutil/b10-dbutil.xml

@@ -0,0 +1,192 @@
+<!DOCTYPE book PUBLIC "-//OASIS//DTD DocBook XML V4.2//EN"
+               "http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd"
+	       [<!ENTITY mdash "&#8212;">]>
+<!--
+ - Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+ -
+ - Permission to use, copy, modify, and/or distribute this software for any
+ - purpose with or without fee is hereby granted, provided that the above
+ - copyright notice and this permission notice appear in all copies.
+ -
+ - THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+ - REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+ - AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+ - INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+ - LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+ - OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+ - PERFORMANCE OF THIS SOFTWARE.
+-->
+
+<refentry>
+
+  <refentryinfo>
+    <date>March 20, 2012</date>
+  </refentryinfo>
+
+  <refmeta>
+    <refentrytitle>b10-dbutil</refentrytitle>
+    <manvolnum>8</manvolnum>
+    <refmiscinfo>BIND10</refmiscinfo>
+  </refmeta>
+
+  <refnamediv>
+    <refname>b10-dbutil</refname>
+    <refpurpose>Zone Database Maintenance Utility</refpurpose>
+  </refnamediv>
+
+  <docinfo>
+    <copyright>
+      <year>2012</year>
+      <holder>Internet Systems Consortium, Inc. ("ISC")</holder>
+    </copyright>
+  </docinfo>
+
+  <refsynopsisdiv>
+    <cmdsynopsis>
+      <command>b10-dbutil --check</command>
+        <arg>--verbose</arg>
+        <arg>--quiet</arg>
+        <arg><replaceable choice='req'>dbfile</replaceable></arg>
+    </cmdsynopsis>
+    <cmdsynopsis>
+      <command>b10-dbutil --upgrade</command>
+        <arg>--noconfirm</arg>
+        <arg>--verbose</arg>
+        <arg>--quiet</arg>
+        <arg><replaceable choice='req'>dbfile</replaceable></arg>
+    </cmdsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>DESCRIPTION</title>
+    <para>
+      The <command>b10-dbutil</command> utility is a general administration
+      utility for SQL databases. (Currently only SQLite is supported by
+      BIND 10.)  It can report the current verion of the schema, and upgrade
+      an existing database to the latest version of the schema.
+    </para>
+
+    <para>
+      <command>b10-dbutil</command> operates in one of two modes, check mode
+      or upgrade mode.
+    </para>
+
+    <para>
+      In check mode (<command>b10-dbutil --check</command>), the
+      utility reads the version of the database schema from the database
+      and prints it.  It will tell you whether the schema is at the latest
+      version supported by BIND 10. Exit status is 0 if the schema is at
+      the correct version, 1 if the schema is at an older version, 2 if
+      the schema is at a version not yet supported by this version of
+      b10-dbutil. Any higher value indicates an error during command-line
+      parsing or execution.
+    </para>
+
+    <para>
+      When the upgrade function is selected
+      (<command>b10-dbutil --upgrade</command>), the
+      utility takes a copy of the database, then upgrades it to the latest
+      version of the schema.  The contents of the database remain intact.
+      (The backup file is a file in the same directory as the database
+      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
+      aborted by the user, and non-zero if there is an error.
+    </para>
+
+    <para>
+    When upgrading the database, it is <emphasis>strongly</emphasis>
+    recommended that BIND 10 not be running while the upgrade is in
+    progress.
+    </para>
+
+  </refsect1>
+
+  <refsect1>
+    <title>ARGUMENTS</title>
+
+    <para>The arguments are as follows:</para>
+
+    <variablelist>
+      <varlistentry>
+        <term>
+         <option>--check</option>
+        </term>
+        <listitem>
+          <para>Selects the version check function, which reports the
+          current version of the database.  This is incompatible
+          with the --upgrade option.
+          </para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term>
+         <option>--noconfirm</option>
+        </term>
+        <listitem>
+          <para>Only valid with --upgrade, this disables the prompt.
+          Normally the utility will print a warning that an upgrade is
+          about to take place and request that you type "Yes" to continue.
+          If this switch is given on the command line, no prompt will
+          be issued: the utility will just perform the upgrade.
+          </para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term>
+         <option>--upgrade</option>
+        </term>
+        <listitem>
+          <para>Selects the upgrade function, which upgrades the database
+          to the latest version of the schema.  This is incompatible
+          with the --upgrade option.
+          </para>
+          <para>
+          The upgrade function will upgrade a BIND 10 database - no matter how
+          old the schema - preserving all data.  A backup file is created
+          before the upgrade (with the same name as the database, but with
+          ".backup" suffixed to it).  If the upgrade fails, this file can
+          be copied back to restore the original database.
+          </para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term>
+         <option>--verbose</option>
+        </term>
+        <listitem>
+          <para>Enable verbose mode.  Each SQL command issued by the
+          utility will be printed to stderr before it is executed.</para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term>
+         <option>--quiet</option>
+        </term>
+        <listitem>
+          <para>Enable quiet mode. No output is printed, except errors during
+            command-line argument parsing, or the user confirmation dialog.
+          </para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term>
+        <option><replaceable choice='req'>dbfile</replaceable></option>
+        </term>
+        <listitem>
+          <para>
+          Name of the database file to check of upgrade.
+          </para>
+        </listitem>
+      </varlistentry>
+
+
+    </variablelist>
+  </refsect1>
+</refentry>

src/bin/dbutil/dbutil.py.in

@@ -0,0 +1,608 @@
+#!@PYTHON@
+
+# Copyright (C) 2012  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+"""
+@file Dabase Utilities
+
+This file holds the "dbutil" program, a general utility program for doing
+management of the BIND 10 database.  There are two modes of operation:
+
+      b10-dbutil --check [--verbose] database
+      b10-dbutil --upgrade [--noconfirm] [--verbose] database
+
+The first form checks the version of the given database.  The second form
+upgrades the database to the latest version of the schema, omitting the
+warning prompt if --noconfirm is given.
+
+For maximum safety, prior to the upgrade a backup database is created.
+The is the database name with ".backup" appended to it (or ".backup-n" if
+".backup" already exists).  This is used to restore the database if the
+upgrade fails.
+"""
+
+# Exit codes
+# These are defined here because one of them is already used before most
+# of the import statements.
+EXIT_SUCCESS = 0
+EXIT_NEED_UPDATE = 1
+EXIT_VERSION_TOO_HIGH = 2
+EXIT_COMMAND_ERROR = 3
+EXIT_READ_ERROR = 4
+EXIT_UPGRADE_ERROR = 5
+EXIT_UNCAUGHT_EXCEPTION = 6
+
+import sys; sys.path.append("@@PYTHONPATH@@")
+
+# Normally, python exits with a status code of 1 on uncaught exceptions
+# Since we reserve exit status 1 for 'database needs upgrade', we
+# override the excepthook to exit with a different status
+def my_except_hook(a, b, c):
+    sys.__excepthook__(a,b,c)
+    sys.exit(EXIT_UNCAUGHT_EXCEPTION)
+sys.excepthook = my_except_hook
+
+import os, sqlite3, shutil
+from optparse import OptionParser
+import isc.util.process
+import isc.log
+from isc.log_messages.dbutil_messages import *
+
+isc.log.init("b10-dbutil")
+logger = isc.log.Logger("dbutil")
+isc.util.process.rename()
+
+TRACE_BASIC = logger.DBGLVL_TRACE_BASIC
+
+
+# @brief Version String
+# This is the version displayed to the user.  It comprises the module name,
+# the module version number, and the overall BIND 10 version number (set in
+# configure.ac)
+VERSION = "b10-dbutil 20120319 (BIND 10 @PACKAGE_VERSION@)"
+
+# @brief Statements to Update the Database
+# These are in the form of a list of dictionaries, each of which contains the
+# information to perform an incremental upgrade from one version of the
+# database to the next.  The information is:
+#
+# a) from: (major, minor) version that the database is expected to be at
+#    to perform this upgrade.
+# b) to: (major, minor) version of the database to which this set of statements
+#    upgrades the database to.  (This is used for documentation purposes,
+#    and to update the schema_version table when the upgrade is complete.)
+# c) statements: List of SQL statments to perform the upgrade.
+#
+# The incremental upgrades are performed one after the other.  If the version
+# of the database does not exactly match that required for the incremental
+# upgrade, the upgrade is skipped.  For this reason, the list must be in
+# ascending order (e.g. upgrade 1.0 to 2.0, 2.0 to 2.1, 2.1 to 2.2 etc.).
+#
+# Note that apart from the 1.0 to 2.0 upgrade, no upgrade need alter the
+# schema_version table: that is done by the upgrade process using the
+# information in the "to" field.
+UPGRADES = [
+    {'from': (1, 0), 'to': (2, 0),
+        'statements': [
+
+            # Move to the latest "V1" state of the database if not there
+            # already.
+            "CREATE TABLE IF NOT EXISTS diffs (" +
+                "id INTEGER PRIMARY KEY, " +
+                "zone_id INTEGER NOT NULL," +
+                "version INTEGER NOT NULL, " +
+                "operation INTEGER NOT NULL, " +
+                "name STRING NOT NULL COLLATE NOCASE, " +
+                "rrtype STRING NOT NULL COLLATE NOCASE, " +
+                "ttl INTEGER NOT NULL, " +
+                "rdata STRING NOT NULL)",
+
+            # Within SQLite with can only rename tables and add columns; we
+            # can't drop columns nor can we alter column characteristics.
+            # So the strategy is to rename the table, create the new table,
+            # then copy all data across.  This means creating new indexes
+            # as well; these are created after the data has been copied.
+
+            # zones table
+            "DROP INDEX zones_byname",
+            "ALTER TABLE zones RENAME TO old_zones",
+            "CREATE TABLE zones (" +
+                "id INTEGER PRIMARY KEY, " +
+                "name TEXT NOT NULL COLLATE NOCASE, " +
+                "rdclass TEXT NOT NULL COLLATE NOCASE DEFAULT 'IN', " +
+                "dnssec BOOLEAN NOT NULL DEFAULT 0)",
+            "INSERT INTO ZONES " +
+                "SELECT id, name, rdclass, dnssec FROM old_zones",
+            "CREATE INDEX zones_byname ON zones (name)",
+            "DROP TABLE old_zones",
+
+            # records table
+            "DROP INDEX records_byname",
+            "DROP INDEX records_byrname",
+            "ALTER TABLE records RENAME TO old_records",
+            "CREATE TABLE records (" +
+                "id INTEGER PRIMARY KEY, " +
+                "zone_id INTEGER NOT NULL, " +
+                "name TEXT NOT NULL COLLATE NOCASE, " +
+                "rname TEXT NOT NULL COLLATE NOCASE, " +
+                "ttl INTEGER NOT NULL, " +
+                "rdtype TEXT NOT NULL COLLATE NOCASE, " +
+                "sigtype TEXT COLLATE NOCASE, " +
+                "rdata TEXT NOT NULL)",
+            "INSERT INTO records " +
+                "SELECT id, zone_id, name, rname, ttl, rdtype, sigtype, " +
+                    "rdata FROM old_records",
+            "CREATE INDEX records_byname ON records (name)",
+            "CREATE INDEX records_byrname ON records (rname)",
+            "CREATE INDEX records_bytype_and_rname ON records (rdtype, rname)",
+            "DROP TABLE old_records",
+
+            # nsec3 table
+            "DROP INDEX nsec3_byhash",
+            "ALTER TABLE nsec3 RENAME TO old_nsec3",
+            "CREATE TABLE nsec3 (" +
+                "id INTEGER PRIMARY KEY, " +
+                "zone_id INTEGER NOT NULL, " +
+                "hash TEXT NOT NULL COLLATE NOCASE, " +
+                "owner TEXT NOT NULL COLLATE NOCASE, " +
+                "ttl INTEGER NOT NULL, " +
+                "rdtype TEXT NOT NULL COLLATE NOCASE, " +
+                "rdata TEXT NOT NULL)",
+            "INSERT INTO nsec3 " +
+                "SELECT id, zone_id, hash, owner, ttl, rdtype, rdata " +
+                    "FROM old_nsec3",
+            "CREATE INDEX nsec3_byhash ON nsec3 (hash)",
+            "DROP TABLE old_nsec3",
+
+            # diffs table
+            "ALTER TABLE diffs RENAME TO old_diffs",
+            "CREATE TABLE diffs (" +
+                "id INTEGER PRIMARY KEY, " +
+                "zone_id INTEGER NOT NULL, " +
+                "version INTEGER NOT NULL, " +
+                "operation INTEGER NOT NULL, " +
+                "name TEXT NOT NULL COLLATE NOCASE, " +
+                "rrtype TEXT NOT NULL COLLATE NOCASE, " +
+                "ttl INTEGER NOT NULL, " +
+                "rdata TEXT NOT NULL)",
+            "INSERT INTO diffs " +
+                "SELECT id, zone_id, version, operation, name, rrtype, " +
+                    "ttl, rdata FROM old_diffs",
+            "DROP TABLE old_diffs",
+
+            # Schema table.  This is updated to include a second column for
+            # future changes.  The idea is that if a version of BIND 10 is
+            # written for schema M.N, it should be able to work for all
+            # versions of N; if not, M must be incremented.
+            #
+            # For backwards compatibility, the column holding the major
+            # version number is left named "version".
+            "ALTER TABLE schema_version " +
+                "ADD COLUMN minor INTEGER NOT NULL DEFAULT 0"
+        ]
+    }
+
+# To extend this, leave the above statements in place and add another
+# dictionary to the list.  The "from" version should be (2, 0), the "to" 
+# version whatever the version the update is to, and the SQL statements are
+# the statements required to perform the upgrade.  This way, the upgrade
+# program will be able to upgrade both a V1.0 and a V2.0 database.
+]
+
+class DbutilException(Exception):
+    """
+    @brief Exception class to indicate error exit
+    """
+    pass
+
+class Database:
+    """
+    @brief Database Encapsulation
+
+    Encapsulates the SQL database, both the connection and the cursor.  The
+    methods will cause a program exit on any error.
+    """
+    def __init__(self, db_file):
+        """
+        @brief Constructor
+
+        @param db_file Name of the database file
+        """
+        self.connection = None
+        self.cursor = None
+        self.db_file = db_file
+        self.backup_file = None
+
+    def open(self):
+        """
+        @brief Open Database
+
+        Opens the passed file as an sqlite3 database and stores a connection
+        and a cursor.
+        """
+        if not os.path.exists(self.db_file):
+            raise DbutilException("database " + self.db_file +
+                                 " does not exist");
+
+        try:
+            self.connection = sqlite3.connect(self.db_file)
+            self.connection.isolation_level = None  # set autocommit
+            self.cursor = self.connection.cursor()
+        except sqlite3.OperationalError as ex:
+            raise DbutilException("unable to open " + self.db_file +
+                                  " - " + str(ex))
+
+    def close(self):
+        """
+        @brief Closes the database
+        """
+        if self.connection is not None:
+            self.connection.close()
+
+    def execute(self, statement):
+        """
+        @brief Execute Statement
+
+        Executes the given statement, exiting the program on error.
+
+        @param statement SQL statement to execute
+        """
+        logger.debug(TRACE_BASIC, DBUTIL_EXECUTE, statement)
+
+        try:
+            self.cursor.execute(statement)
+        except Exception as ex:
+            logger.error(DBUTIL_STATEMENT_ERROR, statement, ex)
+            raise DbutilException(str(ex))
+
+    def result(self):
+        """
+        @brief Return result of last execute
+
+        Returns a single row that is the result of the last "execute".
+        """
+        return self.cursor.fetchone()
+
+    def backup(self):
+        """
+        @brief Backup Database
+
+        Attempts to copy the given database file to a backup database, the
+        backup database file being the file name with ".backup" appended.
+        If the ".backup" file exists, a new name is constructed by appending
+        ".backup-n" (n starting at 1) and the action repeated until an
+        unused filename is found.
+
+        @param db_file Database file to backup
+        """
+        if not os.path.exists(self.db_file):
+            raise DbutilException("database " + self.db_file +
+                                  " does not exist");
+
+        self.backup_file = self.db_file + ".backup"
+        count = 0
+        while os.path.exists(self.backup_file):
+            count = count + 1
+            self.backup_file = self.db_file + ".backup-" + str(count)
+
+        # Do the backup
+        shutil.copyfile(self.db_file, self.backup_file)
+        logger.info(DBUTIL_BACKUP, self.db_file, self.backup_file)
+
+def prompt_user():
+    """
+    @brief Prompt the User
+
+    Explains about the upgrade and requests authorisation to continue.
+
+    @return True if user entered 'Yes', False if 'No'
+    """
+    sys.stdout.write(
+"""You have selected the upgrade option.  This will upgrade the schema of the
+selected BIND 10 zone database to the latest version.
+
+The utility will take a copy of the zone database file before executing so, in
+the event of a problem, you will be able to restore the zone database from
+the backup.  To ensure that the integrity of this backup, please ensure that
+BIND 10 is not running before continuing.
+""")
+    yes_entered = False
+    no_entered = False
+    while (not yes_entered) and (not no_entered):
+        sys.stdout.write("Enter 'Yes' to proceed with the upgrade, " +
+                         "'No' to exit the program: \n")
+        response = sys.stdin.readline()
+        if response.lower() == "yes\n":
+            yes_entered = True
+        elif response.lower() == "no\n":
+            no_entered = True
+        else:
+            sys.stdout.write("Please enter 'Yes' or 'No'\n")
+
+    return yes_entered
+
+
+def version_string(version):
+    """
+    @brief Format Database Version
+
+    Converts a (major, minor) tuple into a 'Vn.m' string.
+
+    @param version Version tuple to convert
+
+    @return Version string
+    """
+    return "V" + str(version[0]) + "." + str(version[1])
+
+
+def compare_versions(first, second):
+    """
+    @brief Compare Versions
+
+    Compares two database version numbers.
+
+    @param first First version number to check (in the form of a
+           "(major, minor)" tuple).
+    @param second Second version number to check (in the form of a
+           "(major, minor)" tuple).
+
+    @return -1, 0, +1 if "first" is <, ==, > "second"
+    """
+    if first == second:
+        return 0
+
+    elif ((first[0] < second[0]) or
+          ((first[0] == second[0]) and (first[1] < second[1]))):
+        return -1
+
+    else:
+        return 1
+
+
+def get_latest_version():
+    """
+    @brief Returns the version to which this utility can upgrade the database
+
+    This is the 'to' version held in the last element of the upgrades list
+    """
+    return UPGRADES[-1]['to']
+
+
+def get_version(db):
+    """
+    @brief Return version of database
+
+    @return Version of database in form (major version, minor version)
+    """
+
+    # Get the version information.
+    db.execute("SELECT * FROM schema_version")
+    result = db.result()
+    if result is None:
+        raise DbutilException("nothing in schema_version table")
+
+    major = result[0]
+    if (major == 1):
+        # If the version number is 1, there will be no "minor" column, so
+        # assume a minor version number of 0.
+        minor = 0
+    else:
+        minor = result[1]
+
+    result = db.result()
+    if result is not None:
+        raise DbutilException("too many rows in schema_version table")
+
+    return (major, minor)
+
+
+def check_version(db):
+    """
+    @brief Check the version
+
+    Checks the version of the database and the latest version, and advises if
+    an upgrade is needed.
+
+    @param db Database object
+
+    returns 0 if the database is up to date
+    returns EXIT_NEED_UPDATE if the database needs updating
+    returns EXIT_VERSION_TOO_HIGH if the database is at a later version
+            than this program knows about
+    These return values are intended to be passed on to sys.exit.
+    """
+    current = get_version(db)
+    latest = get_latest_version()
+
+    match = compare_versions(current, latest)
+    if match == 0:
+        logger.info(DBUTIL_VERSION_CURRENT, version_string(current))
+        logger.info(DBUTIL_CHECK_OK)
+        return EXIT_SUCCESS
+    elif match < 0:
+        logger.info(DBUTIL_VERSION_LOW, version_string(current),
+                    version_string(latest))
+        logger.info(DBUTIL_CHECK_UPGRADE_NEEDED)
+        return EXIT_NEED_UPDATE
+    else:
+        logger.warn(DBUTIL_VERSION_HIGH, version_string(current),
+                    version_string(get_latest_version()))
+        logger.info(DBUTIL_UPGRADE_DBUTIL)
+        return EXIT_VERSION_TOO_HIGH
+
+def perform_upgrade(db, upgrade):
+    """
+    @brief Perform upgrade
+
+    Performs the upgrade.  At the end of the upgrade, updates the schema_version
+    table with the expected version.
+
+    @param db Database object
+    @param upgrade Upgrade dictionary, holding "from", "to" and "statements".
+    """
+    logger.info(DBUTIL_UPGRADING, version_string(upgrade['from']),
+         version_string(upgrade['to']))
+    for statement in upgrade['statements']:
+        db.execute(statement)
+
+    # Update the version information
+    db.execute("DELETE FROM schema_version")
+    db.execute("INSERT INTO schema_version VALUES (" +
+                    str(upgrade['to'][0]) + "," + str(upgrade['to'][1]) + ")")
+
+
+def perform_all_upgrades(db):
+    """
+    @brief Performs all the upgrades
+
+    @brief db Database object
+
+    For each upgrade, checks that the database is at the expected version.
+    If so, calls perform_upgrade to update the database.
+    """
+    match = compare_versions(get_version(db), get_latest_version())
+    if match == 0:
+        logger.info(DBUTIL_UPGRADE_NOT_NEEDED)
+
+    elif match > 0:
+        logger.warn(DBUTIL_UPGRADE_NOT_POSSIBLE)
+
+    else:
+        # Work our way through all upgrade increments
+        count = 0
+        for upgrade in UPGRADES:
+            if compare_versions(get_version(db), upgrade['from']) == 0:
+                perform_upgrade(db, upgrade)
+                count = count + 1
+
+        if count > 0:
+            logger.info(DBUTIL_UPGRADE_SUCCESFUL)
+        else:
+            # Should not get here, as we established earlier that the database
+            # was not at the latest version so we should have upgraded.
+            raise DbutilException("internal error in upgrade tool - no " +
+                                  "upgrade was performed on an old version " +
+                                  "the database")
+
+
+def parse_command():
+    """
+    @brief Parse Command
+
+    Parses the command line and sets the global command options.
+
+    @return Tuple of parser options and parser arguments
+    """
+    usage = ("usage: %prog --check [options] db_file\n" +
+             "       %prog --upgrade [--noconfirm] [options] db_file")
+    parser = OptionParser(usage = usage, version = VERSION)
+    parser.add_option("-c", "--check", action="store_true",
+                      dest="check", default=False,
+                      help="Print database version and check if it " +
+                           "needs upgrading")
+    parser.add_option("-n", "--noconfirm", action="store_true",
+                      dest="noconfirm", default=False,
+                      help="Do not prompt for confirmation before upgrading")
+    parser.add_option("-u", "--upgrade", action="store_true",
+                      dest="upgrade", default=False,
+                      help="Upgrade the database file to the latest version")
+    parser.add_option("-v", "--verbose", action="store_true",
+                      dest="verbose", default=False,
+                      help="Print SQL statements as they are executed")
+    parser.add_option("-q", "--quiet", action="store_true",
+                      dest="quiet", default=False,
+                      help="Don't print any info, warnings or errors")
+    (options, args) = parser.parse_args()
+
+    # Set the database file on which to operate
+    if (len(args) > 1):
+        logger.error(DBUTIL_TOO_MANY_ARGUMENTS)
+        parser.print_usage()
+        sys.exit(EXIT_COMMAND_ERROR)
+    elif len(args) == 0:
+        logger.error(DBUTIL_NO_FILE)
+        parser.print_usage()
+        sys.exit(EXIT_COMMAND_ERROR)
+
+    # Check for conflicting options.  If some are found, output a suitable
+    # error message and print the usage.
+    if options.check and options.upgrade:
+        logger.error(DBUTIL_COMMAND_UPGRADE_CHECK)
+    elif (not options.check) and (not options.upgrade):
+        logger.error(DBUTIL_COMMAND_NONE)
+    elif (options.check and options.noconfirm):
+        logger.error(DBUTIL_CHECK_NOCONFIRM)
+    else:
+        return (options, args)
+
+    # Only get here on conflicting options
+    parser.print_usage()
+    sys.exit(EXIT_COMMAND_ERROR)
+
+
+if __name__ == "__main__":
+    (options, args) = parse_command()
+
+    if options.verbose:
+        isc.log.init("b10-dbutil", "DEBUG", 99)
+        logger = isc.log.Logger("dbutil")
+    elif options.quiet:
+        # We don't use FATAL, so setting the logger to use
+        # it should essentially make it silent.
+        isc.log.init("b10-dbutil", "FATAL")
+        logger = isc.log.Logger("dbutil")
+
+    db = Database(args[0])
+    exit_code = EXIT_SUCCESS
+
+    logger.info(DBUTIL_FILE, args[0])
+    if options.check:
+        # Check database - open, report, and close
+        try:
+            db.open()
+            exit_code = check_version(db)
+            db.close()
+        except Exception as ex:
+            logger.error(DBUTIL_CHECK_ERROR, ex)
+            exit_code = EXIT_READ_ERROR
+
+    elif options.upgrade:
+        # Upgrade.  Check if this is what they really want to do
+        if not options.noconfirm:
+            proceed = prompt_user()
+            if not proceed:
+                logger.info(DBUTIL_UPGRADE_CANCELED)
+                sys.exit(EXIT_SUCCESS)
+
+        # It is.  Do a backup then do the upgrade.
+        in_progress = False
+        try:
+            db.backup()
+            db.open()
+            in_progress = True
+            perform_all_upgrades(db)
+            db.close()
+        except Exception as ex:
+            if in_progress:
+                logger.error(DBUTIL_UPGRADE_FAILED, ex)
+                logger.warn(DBUTIL_DATABASE_MAY_BE_CORRUPT, db.db_file,
+                            db.backup_file)
+            else:
+                logger.error(DBUTIL_UPGRADE_PREPARATION_FAILED, ex)
+                logger.info(DBUTIL_UPGRADE_NOT_ATTEMPTED)
+            exit_code = EXIT_UPGRADE_ERROR
+
+    sys.exit(exit_code)

src/bin/dbutil/dbutil_messages.mes

@@ -0,0 +1,114 @@
+# Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+#
+# Permission to use, copy, modify, and/or distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+# REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+# AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+# LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+# OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+# PERFORMANCE OF THIS SOFTWARE.
+
+# No namespace declaration - these constants go in the global namespace
+# of the ddns messages python module.
+
+# When you add a message to this file, it is a good idea to run
+# <topsrcdir>/tools/reorder_message_file.py to make sure the
+# messages are in the correct order.
+
+% DBUTIL_BACKUP created backup of %1 in %2
+A backup for the given database file was created. Same of original file and
+backup are given in the output message.
+
+% DBUTIL_CHECK_ERROR unable to check database version: %1
+There was an error while trying to check the current version of the database
+schema. The error is shown in the message.
+
+% DBUTIL_CHECK_NOCONFIRM --noconfirm is not compatible with --check
+b10-dbutil was called with --check and --noconfirm. --noconfirm only has
+meaning with --upgrade, so this is considered an error.
+
+% DBUTIL_CHECK_OK this is the latest version of the database schema. No upgrade is required
+The database schema version has been checked, and is up to date.
+No action is required.
+
+% DBUTIL_CHECK_UPGRADE_NEEDED re-run this program with the --upgrade switch to upgrade
+The database schema version is not up to date, and an update is required.
+Please run the dbutil tool again, with the --upgrade argument.
+
+% DBUTIL_COMMAND_NONE must select one of --check or --upgrade
+b10-dbutil was called with neither --check nor --upgrade. One action must be
+provided.
+
+% DBUTIL_COMMAND_UPGRADE_CHECK --upgrade is not compatible with --check
+b10-dbutil was called with both the commands --upgrade and --check. Only one
+action can be performed at a time.
+
+% DBUTIL_DATABASE_MAY_BE_CORRUPT database file %1 may be corrupt, restore it from backup (%2)
+The upgrade failed while it was in progress; the database may now be in an
+inconsistent state, and it is advised to restore it from the backup that was
+created when b10-dbutil started.
+
+% DBUTIL_EXECUTE Executing SQL statement: %1
+Debug message; the given SQL statement is executed
+
+% DBUTIL_FILE Database file: %1
+The database file that is being checked.
+
+% DBUTIL_NO_FILE must supply name of the database file to upgrade
+b10-dbutil was called without a database file. Currently, it cannot find this
+file on its own, and it must be provided.
+
+% DBUTIL_STATEMENT_ERROR failed to execute %1: %2
+The given database statement failed to execute. The error is shown in the
+message.
+
+% DBUTIL_TOO_MANY_ARGUMENTS too many arguments to the command, maximum of one expected
+There were too many command-line arguments to b10-dbutil
+
+% DBUTIL_UPGRADE_CANCELED upgrade canceled; database has not been changed
+The user aborted the upgrade, and b10-dbutil will now exit.
+
+% DBUTIL_UPGRADE_DBUTIL please get the latest version of b10-dbutil and re-run
+A database schema was found that was newer than this version of dbutil, which
+is apparently out of date and should be upgraded itself.
+
+% DBUTIL_UPGRADE_FAILED upgrade failed: %1
+While the upgrade was in progress, an unexpected error occurred. The error
+is shown in the message.
+
+% DBUTIL_UPGRADE_NOT_ATTEMPTED database upgrade was not attempted
+Due to the earlier failure, the database schema upgrade was not attempted,
+and b10-dbutil will now exit.
+
+% DBUTIL_UPGRADE_NOT_NEEDED database already at latest version, no upgrade necessary
+b10-dbutil was told to upgrade the database schema, but it is already at the
+latest version.
+
+% DBUTIL_UPGRADE_NOT_POSSIBLE database at a later version than this utility can support
+b10-dbutil was told to upgrade the database schema, but it is at a higher
+version than this tool currently supports. Please update b10-dbutil and try
+again.
+
+% DBUTIL_UPGRADE_PREPARATION_FAILED upgrade preparation failed: %1
+An unexpected error occurred while b10-dbutil was preparing to upgrade the
+database schema. The error is shown in the message
+
+% DBUTIL_UPGRADE_SUCCESFUL database upgrade successfully completed
+The database schema update was completed successfully.
+
+% DBUTIL_UPGRADING upgrading database from %1 to %2
+An upgrade is in progress, the versions of the current upgrade action are shown.
+
+% DBUTIL_VERSION_CURRENT database version %1
+The current version of the database schema.
+
+% DBUTIL_VERSION_HIGH database is at a later version (%1) than this program can cope with (%2)
+The database schema is at a higher version than b10-dbutil knows about.
+
+% DBUTIL_VERSION_LOW database version %1, latest version is %2.
+The database schema is not up to date, the current version and the latest
+version are in the message.

src/bin/dbutil/run_dbutil.sh.in

@@ -0,0 +1,40 @@
+#! /bin/sh
+
+# Copyright (C) 2010  Internet Systems Consortium.
+#
+# Permission to use, copy, modify, and distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SYSTEMS CONSORTIUM
+# DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL
+# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL
+# INTERNET SYSTEMS CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING
+# FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+# NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+# WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+
+PYTHON_EXEC=${PYTHON_EXEC:-@PYTHON@}
+export PYTHON_EXEC
+
+DBUTIL_PATH=@abs_top_builddir@/src/bin/dbutil
+
+PYTHONPATH=@abs_top_srcdir@/src/bin:@abs_top_builddir@/src/lib/python/isc/log_messages:@abs_top_builddir@/src/lib/python:@abs_top_builddir@/src/bin:@abs_top_srcdir@/src/lib/python
+export PYTHONPATH
+
+# If necessary (rare cases), explicitly specify paths to dynamic libraries
+# required by loadable python modules.
+SET_ENV_LIBRARY_PATH=@SET_ENV_LIBRARY_PATH@
+if test $SET_ENV_LIBRARY_PATH = yes; then
+	@ENV_LIBRARY_PATH@=@abs_top_builddir@/src/lib/dns/.libs:@abs_top_builddir@/src/lib/dns/python/.libs:@abs_top_builddir@/src/lib/cryptolink/.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/util/io/.libs:@abs_top_builddir@/src/lib/exceptions/.libs:@abs_top_builddir@/src/lib/datasrc/.libs:$@ENV_LIBRARY_PATH@
+	export @ENV_LIBRARY_PATH@
+fi
+
+B10_FROM_SOURCE=@abs_top_srcdir@
+export B10_FROM_SOURCE
+
+BIND10_MSGQ_SOCKET_FILE=@abs_top_builddir@/msgq_socket
+export BIND10_MSGQ_SOCKET_FILE
+
+exec ${PYTHON_EXEC} -O ${DBUTIL_PATH}/b10-dbutil "$@"

src/bin/dbutil/tests/Makefile.am

@@ -0,0 +1,6 @@
+SUBDIRS = . testdata
+
+# Tests of the update script.
+
+check-local:
+	$(SHELL) $(abs_builddir)/dbutil_test.sh

src/bin/dbutil/tests/dbutil_test.sh.in

@@ -0,0 +1,482 @@
+#!/bin/sh
+# Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+#
+# Permission to use, copy, modify, and/or distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+# REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+# AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+# INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+# LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+# OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+# PERFORMANCE OF THIS SOFTWARE.
+
+# Checks that the logger will limit the output of messages less severe than
+# the severity/debug setting.
+
+testname="Database Upgrade Test"
+echo $testname
+
+failcount=0
+tempfile=@abs_builddir@/dbutil_test_tempfile_$$
+backupfile=${tempfile}.backup
+testdata=@abs_srcdir@/testdata
+verfile=@abs_builddir@/dbutil_test_verfile_$$
+
+# @brief Record a success
+succeed() {
+    echo "--- PASS"
+}
+
+
+# @brief Record a fail
+#
+# @param $1 Optional additional reason to output
+fail() {
+    if [ "$1" != "" ]
+    then
+        echo "ERROR: $1"
+    fi
+    echo "*** FAIL"
+    failcount=`expr $failcount + 1`
+}
+
+
+# @brief Record a pass if the argument is zero
+#
+# @param $1 Value to test
+passzero() {
+    if [ $1 -eq 0 ]; then
+        succeed
+    else
+        fail
+    fi
+}
+
+
+# @brief Record a fail if the argument is non-zero
+#
+# @param $1 Value to test
+failzero() {
+    if [ $1 -ne 0 ]; then
+        succeed
+    else
+        fail
+    fi
+}
+
+
+# @brief Copy File
+#
+# Executes a "cp" operation followed by a "chmod" to make the target writeable.
+#
+# @param $1 Source file
+# @param $2 Target file
+copy_file () {
+    cp $1 $2
+    chmod a+w $2
+}
+
+
+
+# @brief Check backup file
+#
+# Record a failure if the backup file does not exist or if it is different
+# to the data file. (N.B. No success is recorded if they are the same.)
+#
+# @param $1 Source database file
+# @param $2 Backup file
+check_backup() {
+    if [ ! -e $1 ]
+    then
+        fail "database file $1 not found"
+
+    elif [ ! -e $2 ]
+    then
+        fail "backup file $2 not found"
+
+    else
+        diff $1 $2 > /dev/null
+        if [ $? -ne 0 ]
+        then
+            fail "database file $1 different to backup file $2"
+        fi
+    fi
+}
+
+
+# @brief Check No Backup File
+#
+# Record a failure if the backup file exists.  (N.B. No success is recorded if
+# it does not.)
+#
+# @param $1 Source database file (unused, present for symmetry)
+# @param $2 Backup file
+check_no_backup() {
+    if [ -e $2 ]
+    then
+        fail "backup of database $2 exists when it should not"
+    fi
+}
+
+
+# @brief Get Database Schema
+#
+# As the schema stored in the database is format-dependent - how it is printed
+# depends on how the commands were entered (on one line, split across two
+# lines etc.) - comparing schema is awkward.
+#
+# The function sets the local variable db_schema to the output of the
+# .schema command, with spaces removed and upper converted to lowercase.
+#
+# The database is copied before the schema is taken (and removed after)
+# as SQLite3 assummes a writeable database, which may not be the case if
+# getting the schema from a reference copy.
+#
+# @param $1 Database for which the schema is required
+get_schema() {
+    db1=@abs_builddir@/dbutil_test_schema_$$
+    copy_file $1 $db1
+
+    db_schema=`sqlite3 $db1 '.schema' | \
+               awk '{line = line $0} END {print line}' | \
+               sed -e 's/ //g' | \
+               tr [:upper:] [:lower:]`
+    rm -f $db1
+}
+
+
+# @brief Successful Schema Upgrade Test
+#
+# This test is done where the upgrade is expected to be successful - when
+# the end result of the test is that the test database is upgraded to a
+# database of the expected schema.
+#
+# Note: the caller must ensure that $tempfile and $backupfile do not exist
+#       on entry, and is responsible for removing them afterwards.
+#
+# @param $1 Database to upgrade
+# @param $2 Expected backup file
+upgrade_ok_test() {
+    copy_file $1 $tempfile
+    ../run_dbutil.sh --upgrade --noconfirm $tempfile
+    if [ $? -eq 0 ]
+    then
+        # Compare schema with the reference
+        get_schema $testdata/v2_0.sqlite3
+        expected_schema=$db_schema
+        get_schema $tempfile
+        actual_schema=$db_schema
+        if [ "$expected_schema" = "$actual_schema" ]
+        then
+            succeed
+        else
+            fail "upgraded schema not as expected"
+        fi
+
+        # Check the version is set correctly
+        check_version $tempfile "V2.0"
+
+        # Check that a backup was made
+        check_backup $1 $2
+    else
+        # Error should have been output already
+        fail
+    fi
+}
+
+
+# @brief Unsuccessful Upgrade Test
+#
+# Checks that an upgrade of the specified database fails.
+#
+# Note: the caller must ensure that $tempfile and $backupfile do not exist
+#       on entry, and is responsible for removing them afterwards.
+#
+# @param $1 Database to upgrade
+# @param $2 Expected backup file
+upgrade_fail_test() {
+    copy_file $1 $tempfile
+    ../run_dbutil.sh --upgrade --noconfirm $tempfile
+    failzero $?
+    check_backup $1 $backupfile
+}
+
+
+# @brief Record Count Test
+#
+# Checks that the count of records in each table is preserved in the upgrade.
+#
+# Note 1: This test assumes that the "diffs" table is present.
+# Note 2: The caller must ensure that $tempfile and $backupfile do not exist
+#         on entry, and is responsible for removing them afterwards.
+#
+# @brief $1 Database to upgrade
+record_count_test() {
+    copy_file $1 $tempfile
+
+    diffs_count=`sqlite3 $tempfile 'select count(*) from diffs'`
+    nsec3_count=`sqlite3 $tempfile 'select count(*) from nsec3'`
+    records_count=`sqlite3 $tempfile 'select count(*) from records'`
+    zones_count=`sqlite3 $tempfile 'select count(*) from zones'`
+
+    ../run_dbutil.sh --upgrade --noconfirm $tempfile
+    if [ $? -ne 0 ]
+    then
+        # Reason for failure should already have been output
+        fail
+    else
+        new_diffs_count=`sqlite3 $tempfile 'select count(*) from diffs'`
+        new_nsec3_count=`sqlite3 $tempfile 'select count(*) from nsec3'`
+        new_records_count=`sqlite3 $tempfile 'select count(*) from records'`
+        new_zones_count=`sqlite3 $tempfile 'select count(*) from zones'`
+
+        if [ $diffs_count -ne $new_diffs_count ]
+        then
+            fail "diffs table was not completely copied"
+        fi
+
+        if [ $nsec3_count -ne $new_nsec3_count ]
+        then
+            fail "nsec3 table was not completely copied"
+        fi
+
+        if [ $records_count -ne $new_records_count ]
+        then
+            fail "records table was not completely copied"
+        fi
+
+        if [ $zones_count -ne $new_zones_count ]
+        then
+            fail "zones table was not completely copied"
+        fi
+
+        # As an extra check, test that the backup was successful
+        check_backup $1 $backupfile
+    fi
+}
+
+
+# @brief Version Check
+#
+# Checks that the database is at the specified version (and so checks the
+# --check function).  On success, a pass is recorded.
+#
+# @param $1 Database to check
+# @param $2 Expected version string
+check_version() {
+    copy_file $1 $verfile
+    ../run_dbutil.sh --check $verfile
+    if [ $? -gt 2 ]
+    then
+        fail "version check failed on database $1; return code $?"
+    else
+        ../run_dbutil.sh --check $verfile 2>&1 | grep "$2" > /dev/null
+        if [ $? -ne 0 ]
+        then
+            fail "database $1 not at expected version $2 (output: $?)"
+        else
+            succeed
+        fi
+    fi
+    rm -f $verfile
+}
+
+
+# @brief Version Check Fail
+#
+# Does a version check but expected the check to fail
+#
+# @param $1 Database to check
+# @param $2 Backup file
+check_version_fail() {
+    copy_file $1 $verfile
+    ../run_dbutil.sh --check $verfile
+    failzero $?
+    check_no_backup $tempfile $backupfile
+}
+
+
+# Main test sequence
+
+rm -f $tempfile $backupfile
+
+# Test 1 - check that the utility fails if the database does not exist
+echo "1.1. Non-existent database - check"
+../run_dbutil.sh --check $tempfile
+failzero $?
+check_no_backup $tempfile $backupfile
+
+echo "1.2. Non-existent database - upgrade"
+../run_dbutil.sh --upgrade --noconfirm $tempfile
+failzero $?
+check_no_backup $tempfile $backupfile
+rm -f $tempfile $backupfile
+
+
+# Test 2 - should fail to check an empty file and fail to upgrade it
+echo "2.1. Database is an empty file - check"
+touch $tempfile
+check_version_fail $tempfile $backupfile
+rm -f $tempfile $backupfile
+
+echo "2.2. Database is an empty file - upgrade"
+touch $tempfile
+../run_dbutil.sh --upgrade --noconfirm $tempfile
+failzero $?
+# A backup is performed before anything else, so the backup should exist.
+check_backup $tempfile $backupfile
+rm -f $tempfile $backupfile
+
+
+echo "3.1. Database is not an SQLite file - check"
+echo "This is not an sqlite3 database" > $tempfile
+check_version_fail $tempfile $backupfile
+rm -f $tempfile $backupfile
+
+echo "3.2. Database is not an SQLite file - upgrade"
+echo "This is not an sqlite3 database" > $tempfile
+../run_dbutil.sh --upgrade --noconfirm $tempfile
+failzero $?
+# ...and as before, a backup should have been created
+check_backup $tempfile $backupfile
+rm -f $tempfile $backupfile
+
+
+echo "4.1. Database is an SQLite3 file without the schema table - check"
+check_version_fail $testdata/no_schema.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+echo "4.1. Database is an SQLite3 file without the schema table - upgrade"
+upgrade_fail_test $testdata/no_schema.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+
+echo "5.1. Database is an old V1 database - check"
+check_version $testdata/old_v1.sqlite3 "V1.0"
+check_no_backup $tempfile $backupfile
+rm -f $tempfile $backupfile
+
+echo "5.2. Database is an old V1 database - upgrade"
+upgrade_ok_test $testdata/old_v1.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+
+echo "6.1. Database is new V1 database - check"
+check_version $testdata/new_v1.sqlite3 "V1.0"
+check_no_backup $tempfile $backupfile
+rm -f $tempfile $backupfile
+
+echo "6.2. Database is a new V1 database - upgrade"
+upgrade_ok_test $testdata/new_v1.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+
+echo "7.1. Database is V2.0 database - check"
+check_version $testdata/v2_0.sqlite3 "V2.0"
+check_no_backup $tempfile $backupfile
+rm -f $tempfile $backupfile
+
+echo "7.2. Database is a V2.0 database - upgrade"
+upgrade_ok_test $testdata/v2_0.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+
+echo "8.1. Database is V2.0 database with empty schema table - check"
+check_version_fail $testdata/empty_version.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+echo "8.2. Database is V2.0 database with empty schema table - upgrade"
+upgrade_fail_test $testdata/empty_version.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+
+echo "9.1. Database is V2.0 database with over-full schema table - check"
+check_version_fail $testdata/too_many_version.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+echo "9.2. Database is V2.0 database with over-full schema table - upgrade"
+upgrade_fail_test $testdata/too_many_version.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+
+echo "10.0. Upgrade corrupt database"
+upgrade_fail_test $testdata/corrupt.sqlite3 $backupfile
+rm -f $tempfile $backupfile
+
+
+echo "11. Record count test"
+record_count_test $testdata/new_v1.sqlite3
+rm -f $tempfile $backupfile
+
+
+echo "12. Backup file already exists"
+touch $backupfile
+touch ${backupfile}-1
+upgrade_ok_test $testdata/v2_0.sqlite3 ${backupfile}-2
+rm -f $tempfile $backupfile ${backupfile}-1 ${backupfile}-2
+
+
+echo "13.1 Command-line errors"
+copy_file $testdata/old_v1.sqlite3 $tempfile
+../run_dbutil.sh $tempfile
+failzero $?
+../run_dbutil.sh --upgrade --check $tempfile
+failzero $?
+../run_dbutil.sh --noconfirm --check $tempfile
+failzero $?
+../run_dbutil.sh --check
+failzero $?
+../run_dbutil.sh --upgrade --noconfirm
+failzero $?
+../run_dbutil.sh --check $tempfile $backupfile
+failzero $?
+../run_dbutil.sh --upgrade --noconfirm $tempfile $backupfile
+failzero $?
+rm -f $tempfile $backupfile
+
+echo "13.2 verbose flag"
+copy_file $testdata/old_v1.sqlite3 $tempfile
+../run_dbutil.sh --upgrade --noconfirm --verbose $tempfile
+passzero $?
+rm -f $tempfile $backupfile
+
+echo "13.3 quiet flag"
+copy_file $testdata/old_v1.sqlite3 $tempfile
+../run_dbutil.sh --check --quiet $tempfile 2>&1 | grep .
+failzero $?
+rm -f $tempfile $backupfile
+
+echo "13.3 Interactive prompt - yes"
+copy_file $testdata/old_v1.sqlite3 $tempfile
+../run_dbutil.sh --upgrade $tempfile << .
+Yes
+.
+passzero $?
+check_version $tempfile "V2.0"
+rm -f $tempfile $backupfile
+
+echo "13.4 Interactive prompt - no"
+copy_file $testdata/old_v1.sqlite3 $tempfile
+../run_dbutil.sh --upgrade $tempfile << .
+no
+.
+passzero $?
+diff $testdata/old_v1.sqlite3 $tempfile > /dev/null
+passzero $?
+rm -f $tempfile $backupfile
+
+
+# Report the result
+if [ $failcount -eq 0 ]; then
+    echo "PASS: $testname"
+elif [ $failcount -eq 1 ]; then
+    echo "FAIL: $testname - 1 test failed"
+else
+    echo "FAIL: $testname - $failcount tests failed"
+fi
+
+# Exit with appropriate error status
+exit $failcount

src/bin/dbutil/tests/testdata/Makefile.am

@@ -0,0 +1,12 @@
+EXTRA_DIST =
+EXTRA_DIST += corrupt.sqlite3
+EXTRA_DIST += empty_schema.sqlite3
+EXTRA_DIST += empty_v1.sqlite3
+EXTRA_DIST += empty_version.sqlite3
+EXTRA_DIST += invalid_v1.sqlite3
+EXTRA_DIST += new_v1.sqlite3
+EXTRA_DIST += no_schema.sqlite3
+EXTRA_DIST += old_v1.sqlite3
+EXTRA_DIST += README
+EXTRA_DIST += too_many_version.sqlite3
+EXTRA_DIST += v2_0.sqlite3

src/bin/dbutil/tests/testdata/README

@@ -0,0 +1,41 @@
+The versioning of BIND 10 databases to date has not been the best:
+
+The original database is known here as the "old V1" schema.  It had a
+schema_version table, with the single "version" value set to 1.
+
+The schema was then updated with a "diffs" table.  This is referred to
+here as the "new V1" schema.
+
+The Spring 2012 release of BIND 10 modified the schema.  The
+schema_version table was updated to include a "minor" column, holding the
+minor version number. Other changes to the database included redefining
+"STRING" columns as "TEXT" columns.  This is referred to as the "V2.0
+schema".
+
+The following test data files are present:
+
+empty_schema.sqlite3: A database conforming to the new V1 schema.
+However, there is nothing in the schema_version table.
+
+empty_v1.sqlite3: A database conforming to the new V1 schema.
+The database is empty, except for the schema_version table, where the
+"version" column is set to 1.
+
+empty_version.sqlite3: A database conforming to the V2.0 schema but without
+anything in the schema_version table.
+
+no_schema.sqlite3: A valid SQLite3 database, but without a schema_version
+table.
+
+old_v1.sqlite3: A valid SQLite3 database conforming to the old V1 schema.
+It does not have a diffs table.
+
+invalid_v1.sqlite3: A valid SQLite3 database that, although the schema
+is marked as V1, does not have the nsec3 table.
+
+new_v1.sqlite3: A valid SQLite3 database with data in all the tables
+(although the single rows in both the nsec3 and diffs table make no
+sense, but are valid).
+
+too_many_version.sqlite3: A database conforming to the V2.0 schema but with
+too many rows of data.

src/bin/xfrout/b10-xfrout.8

@@ -9,6 +9,15 @@
 .\"
 .TH "B10\-XFROUT" "8" "March 16\&. 2012" "BIND10" "BIND10"
 .\" -----------------------------------------------------------------
+.\" * Define some portability stuff
+.\" -----------------------------------------------------------------
+.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.\" http://bugs.debian.org/507673
+.\" http://lists.gnu.org/archive/html/groff/2009-02/msg00013.html
+.\" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.ie \n(.g .ds Aq \(aq
+.el       .ds Aq '
+.\" -----------------------------------------------------------------
 .\" * set default formatting
 .\" -----------------------------------------------------------------
 .\" disable hyphenation

src/bin/zonemgr/tests/.gitignore

@@ -1 +1,2 @@
+/initdb.file
 /zonemgr_test

src/lib/datasrc/memory_datasrc.cc

@@ -93,6 +93,11 @@ const DomainNode::Flags WILD = DomainNode::FLAG_USER1;
 // realistic to keep this flag update for all affected nodes, and we may
 // have to reconsider the mechanism.
 const DomainNode::Flags GLUE = DomainNode::FLAG_USER2;
+
+// This flag indicates the node is generated as a result of wildcard
+// expansion.  In this implementation, this flag can be set only in
+// the separate auxiliary tree of ZoneData (see the structure description).
+const DomainNode::Flags WILD_EXPANDED = DomainNode::FLAG_USER3;
 };
 
 // Separate storage for NSEC3 RRs (and their RRSIGs).  It's an STL map
@@ -121,6 +126,31 @@ struct ZoneData {
     // The main data (name + RRsets)
     DomainTree domains_;
 
+    // An auxiliary tree for wildcard expanded data used in additional data
+    // processing.  It contains names like "ns.wild.example" in the following
+    // example:
+    // child.wild.example. NS ns.wild.example.
+    // *.wild.example IN AAAA 2001:db8::1234
+    // (and there's no exact ns.wild.example. in the zone).  This tree contains
+    // such names with a copy of the RRsets of the matching wildcard name
+    // with its owner name expanded, e.g.:
+    // ns.wild.example. IN AAAA 2001:db8::1234
+    // In theory, this tree could have many such wildcard-expandable names,
+    // each of which has a copy of the original list of RRsets.  In practice,
+    // however, it should be very rare that names for additional section
+    // processing are subject to wildcard expansion, so in most cases this tree
+    // should be even empty, and even if it has content it should be very
+    // small.
+private:
+    scoped_ptr<DomainTree> aux_wild_domains_;
+public:
+    DomainTree& getAuxWildDomains() {
+        if (!aux_wild_domains_) {
+            aux_wild_domains_.reset(new DomainTree);
+        }
+        return (*aux_wild_domains_);
+    }
+
     // Shortcut to the origin node, which should always exist
     DomainNode* origin_data_;
 
@@ -136,9 +166,255 @@ struct ZoneData {
         const scoped_ptr<NSEC3Hash> hash_; // hash parameter/calculator
     };
     scoped_ptr<NSEC3Data> nsec3_data_; // non NULL only when it's NSEC3 signed
+
+    // This templated structure encapsulates the find result of findNode()
+    // method (also templated) below.
+    // The template parameter is expected to be either 'const DomainNode' or
+    // 'DomainNode' (to avoid misuse the template definition itself is kept
+    // private - we only expose expected typedefs).  The former is expected
+    // to be used for lookups, and the latter is expected to be used for
+    // constructing the zone.
+private:
+    template <typename NodeType>
+    struct FindNodeResultBase {
+        // Bitwise flags to represent supplemental information of the
+        // search result:
+        // Search resulted in a wildcard match.
+        static const unsigned int FIND_WILDCARD = 1;
+        // Search encountered a zone cut due to NS but continued to look for
+        // a glue.
+        static const unsigned int FIND_ZONECUT = 2;
+
+        FindNodeResultBase(ZoneFinder::Result code_param,
+                           NodeType* node_param,
+                           ConstRBNodeRRsetPtr rrset_param,
+                           unsigned int flags_param = 0) :
+            code(code_param), node(node_param), rrset(rrset_param),
+            flags(flags_param)
+        {}
+        const ZoneFinder::Result code;
+        NodeType* const node;
+        ConstRBNodeRRsetPtr const rrset;
+        const unsigned int flags;
+    };
+public:
+    typedef FindNodeResultBase<const DomainNode> FindNodeResult;
+    typedef FindNodeResultBase<DomainNode> FindMutableNodeResult;
+
+    // Identify the RBTree node that best matches the given name.
+    // See implementation notes below.
+    template <typename ResultType>
+    ResultType findNode(const Name& name,
+                        ZoneFinder::FindOptions options) const;
 };
+
+/// Maintain intermediate data specific to the search context used in
+/// \c find().
+///
+/// It will be passed to \c cutCallback() (see below) and record a possible
+/// zone cut node and related RRset (normally NS or DNAME).
+struct FindState {
+    FindState(bool glue_ok) :
+        zonecut_node_(NULL),
+        dname_node_(NULL),
+        glue_ok_(glue_ok)
+    {}
+
+    // These will be set to a domain node of the highest delegation point,
+    // if any.  In fact, we could use a single variable instead of both.
+    // But then we would need to distinquish these two cases by something
+    // else and it seemed little more confusing when this was written.
+    const DomainNode* zonecut_node_;
+    const DomainNode* dname_node_;
+
+    // Delegation RRset (NS or DNAME), if found.
+    ConstRBNodeRRsetPtr rrset_;
+
+    // Whether to continue search below a delegation point.
+    // Set at construction time.
+    const bool glue_ok_;
+};
+
+// A callback called from possible zone cut nodes and nodes with DNAME.
+// This will be passed from findNode() to \c RBTree::find().
+bool cutCallback(const DomainNode& node, FindState* state) {
+    // We need to look for DNAME first, there's allowed case where
+    // DNAME and NS coexist in the apex. DNAME is the one to notice,
+    // the NS is authoritative, not delegation (corner case explicitly
+    // allowed by section 3 of 2672)
+    const Domain::const_iterator found_dname(node.getData()->find(
+                                                 RRType::DNAME()));
+    if (found_dname != node.getData()->end()) {
+        LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_MEM_DNAME_ENCOUNTERED);
+        state->dname_node_ = &node;
+        state->rrset_ = found_dname->second;
+        // No more processing below the DNAME (RFC 2672, section 3
+        // forbids anything to exist below it, so there's no need
+        // to actually search for it). This is strictly speaking
+        // a different way than described in 4.1 of that RFC,
+        // but because of the assumption in section 3, it has the
+        // same behaviour.
+        return (true);
+    }
+
+    // Look for NS
+    const Domain::const_iterator found_ns(node.getData()->find(RRType::NS()));
+    if (found_ns != node.getData()->end()) {
+        // We perform callback check only for the highest zone cut in the
+        // rare case of nested zone cuts.
+        if (state->zonecut_node_ != NULL) {
+            return (false);
+        }
+
+        LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_MEM_NS_ENCOUNTERED);
+
+        // BIND 9 checks if this node is not the origin.  That's probably
+        // because it can support multiple versions for dynamic updates
+        // and IXFR, and it's possible that the callback is called at
+        // the apex and the DNAME doesn't exist for a particular version.
+        // It cannot happen for us (at least for now), so we don't do
+        // that check.
+        state->zonecut_node_ = &node;
+        state->rrset_ = found_ns->second;
+
+        // Unless glue is allowed the search stops here, so we return
+        // false; otherwise return true to continue the search.
+        return (!state->glue_ok_);
+    }
+
+    // This case should not happen because we enable callback only
+    // when we add an RR searched for above.
+    assert(0);
+    // This is here to avoid warning (therefore compilation error)
+    // in case assert is turned off. Otherwise we could get "Control
+    // reached end of non-void function".
+    return (false);
 }
 
+// Implementation notes: this method identifies an RBT node that best matches
+// the give name in terms of DNS query handling.  In many cases,
+// DomainTree::find() will result in EXACTMATCH or PARTIALMATCH (note that
+// the given name is generally expected to be contained in the zone, so
+// even if it doesn't exist, it should at least match the zone origin).
+// If it finds an exact match, that's obviously the best one.  The partial
+// match case is more complicated.
+//
+// We first need to consider the case where search hits a delegation point,
+// either due to NS or DNAME.  They are indicated as either dname_node_ or
+// zonecut_node_ being non NULL.  Usually at most one of them will be
+// something else than NULL (it might happen both are NULL, in which case we
+// consider it NOT FOUND). There's one corner case when both might be
+// something else than NULL and it is in case there's a DNAME under a zone
+// cut and we search in glue OK mode ‒ in that case we don't stop on the
+// domain with NS and ignore it for the answer, but it gets set anyway. Then
+// we find the DNAME and we need to act by it, therefore we first check for
+// DNAME and then for NS. In all other cases it doesn't matter, as at least
+// one of them is NULL.
+//
+// Next, we need to check if the RBTree search stopped at a node for a
+// subdomain of the search name (so the comparison result that stopped the
+// search is "SUPERDOMAIN"), it means the stopping node is an empty
+// non-terminal node.  In this case the search name is considered to exist
+// but no data should be found there.
+//
+// If none of above is the case, we then consider whether there's a matching
+// wildcard.  DomainTree::find() records the node if it encounters a
+// "wildcarding" node, i.e., the immediate ancestor of a wildcard name
+// (e.g., wild.example.com for *.wild.example.com), and returns it if it
+// doesn't find any node that better matches the query name.  In this case
+// we'll check if there's indeed a wildcard below the wildcarding node.
+//
+// Note, first, that the wildcard is checked after the empty
+// non-terminal domain case above, because if that one triggers, it
+// means we should not match according to 4.3.3 of RFC 1034 (the query
+// name is known to exist).
+//
+// Before we try to find a wildcard, we should check whether there's
+// an existing node that would cancel the wildcard match.  If
+// DomainTree::find() stopped at a node which has a common ancestor
+// with the query name, it might mean we are comparing with a
+// non-wildcard node. In that case, we check which part is common. If
+// we have something in common that lives below the node we got (the
+// one above *), then we should cancel the match according to section
+// 4.3.3 of RFC 1034 (as the name between the wildcard domain and the
+// query name is known to exist).
+//
+// If there's no node below the wildcarding node that shares a common ancestor
+// of the query name, we can conclude the wildcard is the best match.
+// We'll then identify the wildcard node via an incremental search.  Note that
+// there's no possibility that the query name is at an empty non terminal
+// node below the wildcarding node at this stage; that case should have been
+// caught above.
+//
+// If none of the above succeeds, we conclude the name doesn't exist in
+// the zone.
+template <typename ResultType>
+ResultType
+ZoneData::findNode(const Name& name, ZoneFinder::FindOptions options) const {
+    DomainNode* node = NULL;
+    RBTreeNodeChain<Domain> node_path;
+    FindState state((options & ZoneFinder::FIND_GLUE_OK) != 0);
+
+    const DomainTree::Result result =
+        domains_.find(name, &node, node_path, cutCallback, &state);
+    const unsigned int zonecut_flag =
+        (state.zonecut_node_ != NULL) ? FindNodeResult::FIND_ZONECUT : 0;
+    if (result == DomainTree::EXACTMATCH) {
+        return (ResultType(ZoneFinder::SUCCESS, node, state.rrset_,
+                           zonecut_flag));
+    }
+    if (result == DomainTree::PARTIALMATCH) {
+        assert(node != NULL);
+        if (state.dname_node_ != NULL) { // DNAME
+            LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DNAME_FOUND).
+                arg(state.rrset_->getName());
+            return (ResultType(ZoneFinder::DNAME, NULL, state.rrset_));
+        }
+        if (state.zonecut_node_ != NULL) { // DELEGATION due to NS
+            LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DELEG_FOUND).
+                arg(state.rrset_->getName());
+            return (ResultType(ZoneFinder::DELEGATION, NULL, state.rrset_));
+        }
+        if (node_path.getLastComparisonResult().getRelation() ==
+            NameComparisonResult::SUPERDOMAIN) { // empty node, so NXRRSET
+            LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_SUPER_STOP).arg(name);
+            return (ResultType(ZoneFinder::NXRRSET, node,
+                               ConstRBNodeRRsetPtr()));
+        }
+        if (node->getFlag(domain_flag::WILD)) { // maybe a wildcard
+            if (node_path.getLastComparisonResult().getRelation() ==
+                NameComparisonResult::COMMONANCESTOR &&
+                node_path.getLastComparisonResult().getCommonLabels() > 1) {
+                // Wildcard canceled.  Treat it as NXDOMAIN.
+                // Note: Because the way the tree stores relative names, we
+                // will have exactly one common label (the ".") in case we have
+                // nothing common under the node we got, and we will get
+                // more common labels otherwise (yes, this relies on the
+                // internal RBTree structure, which leaks out through this
+                // little bit).
+                LOG_DEBUG(logger, DBG_TRACE_DATA,
+                          DATASRC_MEM_WILDCARD_CANCEL).arg(name);
+                return (ResultType(ZoneFinder::NXDOMAIN, NULL,
+                                   ConstRBNodeRRsetPtr()));
+            }
+            // Now the wildcard should be the best match.
+            const Name wildcard(Name("*").concatenate(
+                                    node_path.getAbsoluteName()));
+            DomainTree::Result result = domains_.find(wildcard, &node);
+            // Otherwise, why would the domain_flag::WILD be there if
+            // there was no wildcard under it?
+            assert(result == DomainTree::EXACTMATCH);
+            return (ResultType(ZoneFinder::SUCCESS, node, state.rrset_,
+                               FindNodeResult::FIND_WILDCARD |
+                               zonecut_flag));
+        }
+    }
+    // Nothing really matched.  The name may even be out-of-bailiwick.
+    LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_NOT_FOUND).arg(name);
+    return (ResultType(ZoneFinder::NXDOMAIN, node, state.rrset_));
+}
+} // unnamed namespace
+
 namespace internal {
 
 /// \brief An encapsulation type for a pointer of an additional node
@@ -148,7 +424,7 @@ namespace internal {
 /// in rbnode_rrset.h; this is essentially a pointer to \c DomainNode.
 /// In future, however, this structure may have other attributes.
 struct AdditionalNodeInfo {
-    AdditionalNodeInfo(DomainNode* node) : node_(node) {}
+    explicit AdditionalNodeInfo(DomainNode* node) : node_(node) {}
     DomainNode* node_;
 };
 
@@ -310,6 +586,53 @@ RBNodeRRset::copyAdditionalNodes(RBNodeRRset& dst) const {
 } // end of internal
 
 namespace {
+/*
+ * Prepares a rrset to be return as a result.
+ *
+ * If rename is false, it returns the one provided. If it is true, it
+ * creates a new rrset with the same data but with provided name.
+ * In addition, if DNSSEC records are required by the original caller of
+ * find(), it also creates expanded RRSIG based on the RRSIG of the
+ * wildcard RRset.
+ * It is designed for wildcard case, where we create the rrsets
+ * dynamically.
+ */
+ConstRBNodeRRsetPtr
+prepareRRset(const Name& name, const ConstRBNodeRRsetPtr& rrset, bool rename,
+             ZoneFinder::FindOptions options)
+{
+    if (rename) {
+        LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_MEM_RENAME).
+            arg(rrset->getName()).arg(name);
+        RRsetPtr result_base(new RRset(name, rrset->getClass(),
+                                       rrset->getType(), rrset->getTTL()));
+        for (RdataIteratorPtr i(rrset->getRdataIterator()); !i->isLast();
+             i->next()) {
+            result_base->addRdata(i->getCurrent());
+        }
+        if ((options & ZoneFinder::FIND_DNSSEC) != 0) {
+            ConstRRsetPtr sig_rrset = rrset->getRRsig();
+            if (sig_rrset) {
+                RRsetPtr result_sig(new RRset(name, sig_rrset->getClass(),
+                                              RRType::RRSIG(),
+                                              sig_rrset->getTTL()));
+                for (RdataIteratorPtr i(sig_rrset->getRdataIterator());
+                     !i->isLast();
+                     i->next())
+                {
+                    result_sig->addRdata(i->getCurrent());
+                }
+                result_base->addRRsig(result_sig);
+            }
+        }
+        RBNodeRRsetPtr result(new RBNodeRRset(result_base));
+        rrset->copyAdditionalNodes(*result);
+        return (result);
+    } else {
+        return (rrset);
+    }
+}
+
 // Specialized version of ZoneFinder::ResultContext, which specifically
 // holds rrset in the form of RBNodeRRset.
 struct RBNodeResultContext {
@@ -393,12 +716,22 @@ private:
             if (!glue_ok && additional.node_->getFlag(domain_flag::GLUE)) {
                 continue;
             }
+            const bool wild_expanded =
+                additional.node_->getFlag(domain_flag::WILD_EXPANDED);
             BOOST_FOREACH(const RRType& rrtype, requested_types) {
                 Domain::const_iterator found =
                     additional.node_->getData()->find(rrtype);
                 if (found != additional.node_->getData()->end()) {
-                    // TODO: wildcard consideration
-                    result.push_back(found->second);
+                    // If the additional node was generated as a result of
+                    // wildcard expansion, we return the underlying RRset,
+                    // in case the caller has the same RRset but as a result
+                    // of normal find() and needs to know they are of the same
+                    // kind; otherwise we simply use the stored RBNodeRRset.
+                    if (wild_expanded) {
+                        result.push_back(found->second->getUnderlyingRRset());
+                    } else {
+                        result.push_back(found->second);
+                    }
                 }
             }
         }
@@ -838,129 +1171,6 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         }
     }
 
-    // Maintain intermediate data specific to the search context used in
-    /// \c find().
-    ///
-    /// It will be passed to \c zonecutCallback() and record a possible
-    /// zone cut node and related RRset (normally NS or DNAME).
-    struct FindState {
-        FindState(FindOptions options) :
-            zonecut_node_(NULL),
-            dname_node_(NULL),
-            options_(options)
-        {}
-        const DomainNode* zonecut_node_;
-        const DomainNode* dname_node_;
-        ConstRBNodeRRsetPtr rrset_;
-        const FindOptions options_;
-    };
-
-    // A callback called from possible zone cut nodes and nodes with DNAME.
-    // This will be passed from the \c find() method to \c RBTree::find().
-    static bool cutCallback(const DomainNode& node, FindState* state) {
-        // We need to look for DNAME first, there's allowed case where
-        // DNAME and NS coexist in the apex. DNAME is the one to notice,
-        // the NS is authoritative, not delegation (corner case explicitly
-        // allowed by section 3 of 2672)
-        const Domain::const_iterator foundDNAME(node.getData()->find(
-            RRType::DNAME()));
-        if (foundDNAME != node.getData()->end()) {
-            LOG_DEBUG(logger, DBG_TRACE_DETAILED,
-                      DATASRC_MEM_DNAME_ENCOUNTERED);
-            state->dname_node_ = &node;
-            state->rrset_ = foundDNAME->second;
-            // No more processing below the DNAME (RFC 2672, section 3
-            // forbids anything to exist below it, so there's no need
-            // to actually search for it). This is strictly speaking
-            // a different way than described in 4.1 of that RFC,
-            // but because of the assumption in section 3, it has the
-            // same behaviour.
-            return (true);
-        }
-
-        // Look for NS
-        const Domain::const_iterator foundNS(node.getData()->find(
-            RRType::NS()));
-        if (foundNS != node.getData()->end()) {
-            // We perform callback check only for the highest zone cut in the
-            // rare case of nested zone cuts.
-            if (state->zonecut_node_ != NULL) {
-                return (false);
-            }
-
-            LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_MEM_NS_ENCOUNTERED);
-
-            // BIND 9 checks if this node is not the origin.  That's probably
-            // because it can support multiple versions for dynamic updates
-            // and IXFR, and it's possible that the callback is called at
-            // the apex and the DNAME doesn't exist for a particular version.
-            // It cannot happen for us (at least for now), so we don't do
-            // that check.
-            state->zonecut_node_ = &node;
-            state->rrset_ = foundNS->second;
-
-            // Unless glue is allowed the search stops here, so we return
-            // false; otherwise return true to continue the search.
-            return ((state->options_ & FIND_GLUE_OK) == 0);
-        }
-
-        // This case should not happen because we enable callback only
-        // when we add an RR searched for above.
-        assert(0);
-        // This is here to avoid warning (therefore compilation error)
-        // in case assert is turned off. Otherwise we could get "Control
-        // reached end of non-void function".
-        return (false);
-    }
-
-    /*
-     * Prepares a rrset to be return as a result.
-     *
-     * If rename is false, it returns the one provided. If it is true, it
-     * creates a new rrset with the same data but with provided name.
-     * In addition, if DNSSEC records are required by the original caller of
-     * find(), it also creates expanded RRSIG based on the RRSIG of the
-     * wildcard RRset.
-     * It is designed for wildcard case, where we create the rrsets
-     * dynamically.
-     */
-    static ConstRBNodeRRsetPtr prepareRRset(const Name& name,
-                                            const ConstRBNodeRRsetPtr& rrset,
-                                            bool rename, FindOptions options)
-    {
-        if (rename) {
-            LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_MEM_RENAME).
-                arg(rrset->getName()).arg(name);
-            RRsetPtr result_base(new RRset(name, rrset->getClass(),
-                                           rrset->getType(),
-                                           rrset->getTTL()));
-            for (RdataIteratorPtr i(rrset->getRdataIterator()); !i->isLast();
-                 i->next()) {
-                result_base->addRdata(i->getCurrent());
-            }
-            if ((options & FIND_DNSSEC) != 0) {
-                ConstRRsetPtr sig_rrset = rrset->getRRsig();
-                if (sig_rrset) {
-                    RRsetPtr result_sig(new RRset(name, sig_rrset->getClass(),
-                                                  RRType::RRSIG(),
-                                                  sig_rrset->getTTL()));
-                    for (RdataIteratorPtr i(sig_rrset->getRdataIterator());
-                         !i->isLast();
-                         i->next())
-                    {
-                        result_sig->addRdata(i->getCurrent());
-                    }
-                    result_base->addRRsig(result_sig);
-                }
-            }
-            RBNodeRRsetPtr result(new RBNodeRRset(result_base));
-            rrset->copyAdditionalNodes(*result);
-            return (result);
-        } else {
-            return (rrset);
-        }
-    }
-
     // Set up FindContext object as a return value of find(), taking into
     // account wildcard matches and DNSSEC information.  We set the NSEC/NSEC3
     // flag when applicable regardless of the find option; the caller would
@@ -991,130 +1201,20 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
     {
         LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_FIND).arg(name).
             arg(type);
-        // Get the node
-        DomainNode* node(NULL);
-        FindState state(options);
-        RBTreeNodeChain<Domain> node_path;
-        bool rename(false);
-        switch (zone_data_->domains_.find(name, &node, node_path, cutCallback,
-                                          &state)) {
-            case DomainTree::PARTIALMATCH:
-                /*
-                 * In fact, we could use a single variable instead of
-                 * dname_node_ and zonecut_node_. But then we would need
-                 * to distinquish these two cases by something else and
-                 * it seemed little more confusing to me when I wrote it.
-                 *
-                 * Usually at most one of them will be something else than
-                 * NULL (it might happen both are NULL, in which case we
-                 * consider it NOT FOUND). There's one corner case when
-                 * both might be something else than NULL and it is in case
-                 * there's a DNAME under a zone cut and we search in
-                 * glue OK mode ‒ in that case we don't stop on the domain
-                 * with NS and ignore it for the answer, but it gets set
-                 * anyway. Then we find the DNAME and we need to act by it,
-                 * therefore we first check for DNAME and then for NS. In
-                 * all other cases it doesn't matter, as at least one of them
-                 * is NULL.
-                 */
-                if (state.dname_node_ != NULL) {
-                    LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DNAME_FOUND).
-                        arg(state.rrset_->getName());
-                    // We were traversing a DNAME node (and wanted to go
-                    // lower below it), so return the DNAME
-                    return (createFindResult(DNAME,
-                                             prepareRRset(name, state.rrset_,
-                                                          false, options)));
-                }
-                if (state.zonecut_node_ != NULL) {
-                    LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DELEG_FOUND).
-                        arg(state.rrset_->getName());
-                    return (createFindResult(DELEGATION,
-                                             prepareRRset(name, state.rrset_,
-                                                          false, options)));
-                }
-
-                // If the RBTree search stopped at a node for a super domain
-                // of the search name, it means the search name exists in
-                // the zone but is empty.  Treat it as NXRRSET.
-                if (node_path.getLastComparisonResult().getRelation() ==
-                    NameComparisonResult::SUPERDOMAIN) {
-                    LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_SUPER_STOP).
-                        arg(name);
-                    return (createFindResult(NXRRSET, ConstRBNodeRRsetPtr()));
-                }
-
-                /*
-                 * No redirection anywhere. Let's try if it is a wildcard.
-                 *
-                 * The wildcard is checked after the empty non-terminal domain
-                 * case above, because if that one triggers, it means we should
-                 * not match according to 4.3.3 of RFC 1034 (the query name
-                 * is known to exist).
-                 */
-                if (node->getFlag(domain_flag::WILD)) {
-                    /* Should we cancel this match?
-                     *
-                     * If we compare with some node and get a common ancestor,
-                     * it might mean we are comparing with a non-wildcard node.
-                     * In that case, we check which part is common. If we have
-                     * something in common that lives below the node we got
-                     * (the one above *), then we should cancel the match
-                     * according to section 4.3.3 of RFC 1034 (as the name
-                     * between the wildcard domain and the query name is known
-                     * to exist).
-                     *
-                     * Because the way the tree stores relative names, we will
-                     * have exactly one common label (the ".") in case we have
-                     * nothing common under the node we got and we will get
-                     * more common labels otherwise (yes, this relies on the
-                     * internal RBTree structure, which leaks out through this
-                     * little bit).
-                     *
-                     * If the empty non-terminal node actually exists in the
-                     * tree, then this cancellation is not needed, because we
-                     * will not get here at all.
-                     */
-                    if (node_path.getLastComparisonResult().getRelation() ==
-                        NameComparisonResult::COMMONANCESTOR && node_path.
-                        getLastComparisonResult().getCommonLabels() > 1) {
-                        LOG_DEBUG(logger, DBG_TRACE_DATA,
-                                     DATASRC_MEM_WILDCARD_CANCEL).arg(name);
-                        return (createFindResult(NXDOMAIN,
-                                                 ConstRBNodeRRsetPtr(),
-                                                 false));
-                    }
-                    const Name wildcard(Name("*").concatenate(
-                        node_path.getAbsoluteName()));
-                    DomainTree::Result result =
-                        zone_data_->domains_.find(wildcard, &node);
-                    /*
-                     * Otherwise, why would the domain_flag::WILD be there if
-                     * there was no wildcard under it?
-                     */
-                    assert(result == DomainTree::EXACTMATCH);
-                    /*
-                     * We have the wildcard node now. Jump below the switch,
-                     * where handling of the common (exact-match) case is.
-                     *
-                     * However, rename it to the searched name.
-                     */
-                    rename = true;
-                    break;
-                }
 
-                // fall through
-            case DomainTree::NOTFOUND:
-                LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_NOT_FOUND).
-                    arg(name);
-                return (createFindResult(NXDOMAIN, ConstRBNodeRRsetPtr(),
-                                         false));
-            case DomainTree::EXACTMATCH: // This one is OK, handle it
-                break;
-            default:
-                assert(0);
+        // Get the node.  All other cases than an exact match are handled
+        // in findNode().  We simply construct a result structure and return.
+        const ZoneData::FindNodeResult node_result =
+            zone_data_->findNode<ZoneData::FindNodeResult>(name, options);
+        if (node_result.code != SUCCESS) {
+            return (createFindResult(node_result.code, node_result.rrset));
         }
+
+        // We've found an exact match, may or may not be a result of wildcard.
+        const DomainNode* node = node_result.node;
         assert(node != NULL);
+        const bool rename = ((node_result.flags &
+                              ZoneData::FindNodeResult::FIND_WILDCARD) != 0);
 
         // If there is an exact match but the node is empty, it's equivalent
         // to NXRRSET.
@@ -1184,7 +1284,8 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
     }
 };
 
-InMemoryZoneFinder::InMemoryZoneFinder(const RRClass& zone_class, const Name& origin) :
+InMemoryZoneFinder::InMemoryZoneFinder(const RRClass& zone_class,
+                                       const Name& origin) :
     impl_(new InMemoryZoneFinderImpl(zone_class, origin))
 {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_CREATE).arg(origin).
@@ -1323,28 +1424,24 @@ getAdditionalName(RRType rrtype, const rdata::Rdata& rdata) {
     }
 }
 
-bool
-checkZoneCut(const DomainNode& node, pair<bool, bool>* arg) {
-    // We are only interested in the highest zone cut information.
-    // Ignore others and continue the search.
-    if (arg->first) {
-        return (false);
-    }
-    // Once we encounter a delegation point due to a DNAME, anything under it
-    // should be hidden.
-    if (node.getData()->find(RRType::DNAME()) != node.getData()->end()) {
-        return (true);
-    } else if (node.getData()->find(RRType::NS()) != node.getData()->end()) {
-        arg->first = true;
-        arg->second = true;
-        return (false);
-    }
-    return (false);
+void
+convertAndInsert(const DomainPair& rrset_item, DomainPtr dst_domain,
+                 const Name* dstname)
+{
+    // We copy RRSIGs, too, if they are attached in case we need it in
+    // getAdditional().
+    dst_domain->insert(DomainPair(rrset_item.first,
+                                  prepareRRset(*dstname, rrset_item.second,
+                                               true,
+                                               ZoneFinder::FIND_DNSSEC)));
 }
 
 void
-addAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
+addAdditional(RBNodeRRset* rrset, ZoneData* zone_data,
+              vector<RBNodeRRset*>* wild_rrsets)
+{
     RdataIteratorPtr rdata_iterator = rrset->getRdataIterator();
+    bool match_wild = false;    // will be true if wildcard match is found
     for (; !rdata_iterator->isLast(); rdata_iterator->next()) {
         // For each domain name that requires additional section processing
         // in each RDATA, search the tree for the name and remember it if
@@ -1352,31 +1449,91 @@ addAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
         // child zone), mark the node as "GLUE", so we can selectively
         // include/exclude them when we use it.
 
-        // TODO: wildcard
-        RBTreeNodeChain<Domain> node_path;
-        DomainNode* node = NULL;
-        // The callback argument is a pair of bools: the first is a flag to
-        // only check the highest cut; the second one records whether the
-        // search goes under a zone cut.
-        pair<bool, bool> callback_arg(false, false);
-        const DomainTree::Result result =
-            zone_data->domains_.find(
-                getAdditionalName(rrset->getType(),
-                                  rdata_iterator->getCurrent()),
-                &node, node_path, checkZoneCut, &callback_arg);
-        if (result == DomainTree::EXACTMATCH) {
-            assert(node != NULL);
-            if (callback_arg.second ||
-                (node->getFlag(DomainNode::FLAG_CALLBACK) &&
-                 node->getData()->find(RRType::NS()) !=
-                 node->getData()->end())) {
-                // The node is under or at a zone cut; mark it as a glue.
-                node->setFlag(domain_flag::GLUE);
+        const Name& name = getAdditionalName(rrset->getType(),
+                                             rdata_iterator->getCurrent());
+        const ZoneData::FindMutableNodeResult result =
+            zone_data->findNode<ZoneData::FindMutableNodeResult>(
+                name, ZoneFinder::FIND_GLUE_OK);
+        if (result.code != ZoneFinder::SUCCESS) {
+            // We are not interested in anything but a successful match.
+            continue;
+        }
+        DomainNode* node = result.node;
+        assert(node != NULL);
+        if ((result.flags & ZoneData::FindNodeResult::FIND_ZONECUT) != 0 ||
+            (node->getFlag(DomainNode::FLAG_CALLBACK) &&
+             node->getData()->find(RRType::NS()) != node->getData()->end())) {
+            // The node is under or at a zone cut; mark it as a glue.
+            node->setFlag(domain_flag::GLUE);
+        }
+
+        // A rare case: the additional name may have to be expanded with a
+        // wildcard.  We'll store the name in a separate auxiliary tree,
+        // copying all RRsets of the original wildcard node with expanding
+        // the owner name.  This is costly in terms of memory, but this case
+        // should be pretty rare.  On the other hand we won't have to worry
+        // about wildcard expansion in getAdditional, which is quite
+        // performance sensitive.
+        DomainNode* wildnode = NULL;
+        if ((result.flags & ZoneData::FindNodeResult::FIND_WILDCARD) != 0) {
+            // Wildcard and glue shouldn't coexist.  Make it sure here.
+            assert(!node->getFlag(domain_flag::GLUE));
+
+            if (zone_data->getAuxWildDomains().insert(name, &wildnode)
+                == DomainTree::SUCCESS) {
+                // If we first insert the node, copy the RRsets.  If the
+                // original node was empty, we add empty data so
+                // addWildAdditional() can get an exactmatch for this name.
+                DomainPtr dst_domain(new Domain);
+                if (!node->isEmpty()) {
+                    for_each(node->getData()->begin(), node->getData()->end(),
+                             boost::bind(convertAndInsert, _1, dst_domain,
+                                         &name));
+                }
+                wildnode->setData(dst_domain);
+                // Mark the node as "wildcard expanded" so it can be
+                // distinguished at lookup time.
+                wildnode->setFlag(domain_flag::WILD_EXPANDED);
             }
+            match_wild = true;
+            node = wildnode;
+        }
+
+        // If this name wasn't subject to wildcard substitution, we can add
+        // the additional information to the RRset now; otherwise I'll defer
+        // it until the entire auxiliary tree is built (pointers may be
+        // invalidated as we build it).
+        if (wildnode == NULL) {
             // Note that node may be empty.  We should keep it in the list
             // in case we dynamically update the tree and it becomes non empty
             // (which is not supported yet)
-            rrset->addAdditionalNode(node);
+            rrset->addAdditionalNode(AdditionalNodeInfo(node));
+        }
+    }
+
+    if (match_wild) {
+        wild_rrsets->push_back(rrset);
+    }
+}
+
+void
+addWildAdditional(RBNodeRRset* rrset, ZoneData* zone_data) {
+    // Similar to addAdditional(), but due to the first stage we know that
+    // the rrset should contain a name stored in the auxiliary trees, and
+    // that it should be found as an exact match.  The RRset may have other
+    // names that didn't require wildcard expansion, but we can simply ignore
+    // them in this context.  (Note that if we find an exact match in the
+    // auxiliary tree, it shouldn't be in the original zone; otherwise it
+    // shouldn't have resulted in wildcard in the first place).
+
+    RdataIteratorPtr rdata_iterator = rrset->getRdataIterator();
+    for (; !rdata_iterator->isLast(); rdata_iterator->next()) {
+        const Name& name = getAdditionalName(rrset->getType(),
+                                             rdata_iterator->getCurrent());
+        DomainNode* wildnode = NULL;
+        if (zone_data->getAuxWildDomains().find(name, &wildnode) ==
+            DomainTree::EXACTMATCH) {
+            rrset->addAdditionalNode(AdditionalNodeInfo(wildnode));
         }
     }
 }
@@ -1398,8 +1555,14 @@ InMemoryZoneFinder::load(const string& filename) {
 
     // For each RRset in need_additionals, identify the corresponding
     // RBnode for additional processing and associate it in the RRset.
+    // If some additional names in an RRset RDATA as additional need wildcard
+    // expansion, we'll remember them in a separate vector, and handle them
+    // with addWildAdditional.
+    vector<RBNodeRRset*> wild_additionals;
     for_each(need_additionals.begin(), need_additionals.end(),
-             boost::bind(addAdditional, _1, tmp.get()));
+             boost::bind(addAdditional, _1, tmp.get(), &wild_additionals));
+    for_each(wild_additionals.begin(), wild_additionals.end(),
+             boost::bind(addWildAdditional, _1, tmp.get()));
 
     // If the zone is NSEC3-signed, check if it has NSEC3PARAM
     if (tmp->nsec3_data_) {

src/lib/datasrc/rbtree.h

@@ -124,7 +124,8 @@ public:
     enum Flags {
         FLAG_CALLBACK = 1, ///< Callback enabled. See \ref callback
         FLAG_USER1 = 0x80000000U, ///< Application specific flag
-        FLAG_USER2 = 0x40000000U  ///< Application specific flag
+        FLAG_USER2 = 0x40000000U, ///< Application specific flag
+        FLAG_USER3 = 0x20000000U  ///< Application specific flag
     };
 private:
     // Some flag values are expected to be used for internal purposes
@@ -133,7 +134,7 @@ private:
     // explicitly defined in \c Flags.  This constant represents all
     // such flags.
     static const uint32_t SETTABLE_FLAGS = (FLAG_CALLBACK | FLAG_USER1 |
-                                            FLAG_USER2);
+                                            FLAG_USER2 | FLAG_USER3);
 
 public:
 
@@ -1161,6 +1162,10 @@ RBTree<T>::insert(const isc::dns::Name& target_name, RBNode<T>** new_node) {
 }
 
 
+// Note: when we redesign this (still keeping the basic concept), we should
+// change this part so the newly created node will be used for the inserted
+// name (and therefore the name for the existing node doesn't change).
+// Otherwise, things like shortcut links between nodes won't work.
 template <typename T>
 void
 RBTree<T>::nodeFission(RBNode<T>& node, const isc::dns::Name& base_name) {

src/lib/datasrc/tests/rbnode_rrset_unittest.cc

@@ -12,16 +12,17 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <stdexcept>
-
 #include <exceptions/exceptions.h>
 #include <dns/rdataclass.h>
 #include <datasrc/rbnode_rrset.h>
 #include <testutils/dnsmessage_test.h>
 
+#include <dns/tests/unittest_util.h>
+
 #include <gtest/gtest.h>
 
-#include <dns/tests/unittest_util.h>
+#include <sstream>
+#include <stdexcept>
 
 using isc::UnitTestUtil;
 

src/lib/datasrc/tests/testdata/contexttest.zone

@@ -1,7 +1,7 @@
 ;; test zone file used for ZoneFinderContext tests.
 ;; RRSIGs are (obviouslly) faked ones for testing.
 
-example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 56 3600 300 3600000 3600
+example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 67 3600 300 3600000 3600
 example.org.			      3600 IN NS	ns1.example.org.
 example.org.			      3600 IN NS	ns2.example.org.
 example.org.			      3600 IN MX	1 mx1.example.org.
@@ -57,9 +57,15 @@ d.example.org. 	      	      3600 IN NS	ns1.example.org.
 foo.ns.empty.example.org.     3600 IN A		192.0.2.13
 bar.ns.empty.example.org.     3600 IN A		192.0.2.14
 
-;; delegation; the NS name matches a wildcard (and there's no exact match)
+;; delegation; the NS name matches a wildcard (and there's no exact
+;; match).  One of the NS names matches an empty wildcard node, for
+;; which no additional record should be provided (or any other
+;; disruption should happen).
 e.example.org. 	      	      3600 IN NS	ns.wild.example.org.
+e.example.org. 	      	      3600 IN NS	ns.emptywild.example.org.
+e.example.org. 	      	      3600 IN NS	ns2.example.org.
 *.wild.example.org.	      3600 IN A		192.0.2.15
+a.*.emptywild.example.org.    3600 IN AAAA	2001:db8::2
 
 ;; additional for an answer RRset (MX) as a result of wildcard
 ;; expansion

src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -262,18 +262,28 @@ TEST_P(ZoneFinderContextTest, getAdditionalDelegationWithEmptyName) {
 }
 
 TEST_P(ZoneFinderContextTest, getAdditionalDelegationWithWild) {
-    // The NS name needs to be expanded by a wildcard.  Currently it doesn't
-    // work for the optimized in-memory version.
-    if (GetParam() == createInMemoryClient) {
-        return;
-    }
-
+    // An NS name needs to be expanded by a wildcard.  Another NS name
+    // also matches a wildcard, but it's an empty node, so there's no
+    // corresponding additional RR.  The other NS name isn't subject to
+    // wildcard expansion, which shouldn't cause any disruption.
     ZoneFinderContextPtr ctx = finder_->find(Name("www.e.example.org"),
                                              RRType::AAAA());
     EXPECT_EQ(ZoneFinder::DELEGATION, ctx->code);
     ctx->getAdditional(REQUESTED_BOTH, result_sets_);
-    rrsetsCheck("ns.wild.example.org. 3600 IN A 192.0.2.15\n",
+    rrsetsCheck("ns.wild.example.org. 3600 IN A 192.0.2.15\n"
+                "ns2.example.org. 3600 IN A 192.0.2.2\n",
                 result_sets_.begin(), result_sets_.end());
+
+    // ns.wild.example.org/A (expanded from a wildcard) should be considered
+    // the same kind, whether it's a direct result of find() or a result of
+    // getAdditional().
+    ctx = finder_->find(Name("ns.wild.example.org"), RRType::A());
+    EXPECT_EQ(ZoneFinder::SUCCESS, ctx->code);
+    for (vector<ConstRRsetPtr>::const_iterator it = result_sets_.begin();
+         it != result_sets_.end(); ++it) {
+        const bool same_kind = (*it)->isSameKind(*ctx->rrset);
+        EXPECT_EQ((*it)->getName() == ctx->rrset->getName(), same_kind);
+    }
 }
 
 TEST_P(ZoneFinderContextTest, getAdditionalDelegationForWild) {

src/lib/dns/rrset.h

@@ -482,8 +482,8 @@ public:
     /// \param other Pointer to another AbstractRRset to compare
     ///              against.
     virtual bool isSameKind(const AbstractRRset& other) const;
-
     //@}
+
 };
 
 /// \brief The \c RdataIterator class is an abstract base class that

src/lib/dns/tests/rrset_unittest.cc

@@ -12,8 +12,6 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <stdexcept>
-
 #include <util/buffer.h>
 #include <dns/messagerenderer.h>
 #include <dns/name.h>
@@ -24,9 +22,12 @@
 #include <dns/rrttl.h>
 #include <dns/rrset.h>
 
+#include <dns/tests/unittest_util.h>
+
 #include <gtest/gtest.h>
 
-#include <dns/tests/unittest_util.h>
+#include <stdexcept>
+#include <sstream>
 
 using isc::UnitTestUtil;
 

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

@@ -72,7 +72,7 @@ class Session:
         self._lname = None
         self._closed = True
 
-    def sendmsg(self, env, msg = None):
+    def sendmsg(self, env, msg=None):
         with self._lock:
             if self._closed:
                 raise SessionError("Session has been closed.")
@@ -82,15 +82,24 @@ class Session:
                 raise ProtocolError("Envelope too large")
             if type(msg) == dict:
                 msg = isc.cc.message.to_wire(msg)
-            self._socket.setblocking(1)
             length = 2 + len(env);
-            if msg:
+            if msg is not None:
                 length += len(msg)
-            self._socket.send(struct.pack("!I", length))
-            self._socket.send(struct.pack("!H", len(env)))
-            self._socket.send(env)
-            if msg:
-                self._socket.send(msg)
+
+            # Build entire message.
+            data = struct.pack("!I", length)
+            data += struct.pack("!H", len(env))
+            data += env
+            if msg is not None:
+                data += msg
+
+            # Send it in the blocking mode.  On some systems send() may
+            # actually send only part of the data, so we need to repeat it
+            # until all data have been sent out.
+            self._socket.setblocking(1)
+            while len(data) > 0:
+                cc = self._socket.send(data)
+                data = data[cc:]
 
     def recvmsg(self, nonblock = True, seq = None):
         """Reads a message. If nonblock is true, and there is no

src/lib/python/isc/cc/tests/session_test.py

@@ -29,6 +29,7 @@ class MySocket():
         self.recvqueue = bytearray()
         self.sendqueue = bytearray()
         self._blocking = True
+        self.send_limit = None
 
     def connect(self, to):
         pass
@@ -40,7 +41,14 @@ class MySocket():
         self._blocking = val
 
     def send(self, data):
-        self.sendqueue.extend(data);
+        # If the upper limit is specified, only "send" up to the specified
+        # limit
+        if self.send_limit is not None and len(data) > self.send_limit:
+            self.sendqueue.extend(data[0:self.send_limit])
+            return self.send_limit
+        else:
+            self.sendqueue.extend(data)
+            return len(data)
 
     def readsent(self, length):
         if length > len(self.sendqueue):
@@ -101,6 +109,17 @@ class MySocket():
     def gettimeout(self):
         return 0
 
+    def set_send_limit(self, limit):
+        '''Specify the upper limit of the transmittable data at once.
+
+        By default, the send() method of this class "sends" all given data.
+        If this method is called and the its parameter is not None,
+        subsequent calls to send() will only transmit the specified amount
+        of data.  This can be used to emulate the situation where send()
+        on a real socket object results in partial write.
+        '''
+        self.send_limit = limit
+
 #
 # We subclass the Session class we're testing here, only
 # to override the __init__() method, which wants a socket,
@@ -157,6 +176,16 @@ class testSession(unittest.TestCase):
         #print(sent)
         #self.assertRaises(SessionError, sess.sendmsg, {}, {"hello": "a"})
 
+    def test_session_sendmsg_shortwrite(self):
+        sess = MySession()
+        # Specify the upper limit of the size that can be transmitted at
+        # a single send() call on the faked socket (10 is an arbitrary choice,
+        # just reasonably small).
+        sess._socket.set_send_limit(10)
+        sess.sendmsg({'to': 'someone', 'reply': 1}, {"hello": "a"})
+        # The complete message should still have been transmitted in the end.
+        sent = sess._socket.readsentmsg();
+
     def recv_and_compare(self, session, bytes, env, msg):
         """Adds bytes to the recvqueue (which will be read by the
            session object, and compare the resultinv env and msg to

src/lib/python/isc/config/cfgmgr.py

@@ -148,6 +148,27 @@ class ConfigManagerData:
             # Ok if we really can't delete it anymore, leave it
             pass
 
+    def rename_config_file(self, old_file_name=None, new_file_name=None):
+        """Renames the given configuration file to the given new file name,
+           if it exists. If it does not exist, nothing happens.
+           If old_file_name is None (default), the file used in
+           read_from_file is used. If new_file_name is None (default), the
+           file old_file_name appended with .bak is used. If that file exists
+           already, .1 is appended. If that file exists, .2 is appended, etc.
+        """
+        if old_file_name is None:
+            old_file_name = self.db_filename
+        if new_file_name is None:
+            new_file_name = old_file_name + ".bak"
+        if os.path.exists(new_file_name):
+            i = 1
+            while os.path.exists(new_file_name + "." + str(i)):
+                i += 1
+            new_file_name = new_file_name + "." + str(i)
+        if os.path.exists(old_file_name):
+            logger.info(CFGMGR_RENAMED_CONFIG_FILE, old_file_name, new_file_name)
+            os.rename(old_file_name, new_file_name)
+
     def __eq__(self, other):
         """Returns True if the data contained is equal. data_path and
            db_filename may be different."""
@@ -163,14 +184,16 @@ class ConfigManager:
        channel session. If not, a new session will be created.
        The ability to specify a custom session is for testing purposes
        and should not be needed for normal usage."""
-    def __init__(self, data_path, database_filename, session=None):
+    def __init__(self, data_path, database_filename, session=None,
+                 clear_config=False):
         """Initialize the configuration manager. The data_path string
            is the path to the directory where the configuration is
            stored (in <data_path>/<database_filename> or in
            <database_filename>, if it is absolute). The dabase_filename
            is the config file to load. Session is an optional
            cc-channel session. If this is not given, a new one is
-           created."""
+           created. If clear_config is True, the configuration file is
+           renamed and a new one is created."""
         self.data_path = data_path
         self.database_filename = database_filename
         self.module_specs = {}
@@ -179,6 +202,8 @@ class ConfigManager:
         # of some other process
         self.virtual_modules = {}
         self.config = ConfigManagerData(data_path, database_filename)
+        if clear_config:
+            self.config.rename_config_file()
         if session:
             self.cc = session
         else:

src/lib/python/isc/config/cfgmgr_messages.mes

@@ -51,7 +51,11 @@ error is given. The most likely cause is that the system does not have
 write access to the configuration database file. The updated
 configuration is not stored.
 
+% CFGMGR_RENAMED_CONFIG_FILE renamed configuration file %1 to %2, will create new %1
+BIND 10 has been started with the command to clear the configuration file.
+The existing file is backed up to the given file name, so that data is not
+immediately lost if this was done by accident.
+
 % CFGMGR_STOPPED_BY_KEYBOARD keyboard interrupt, shutting down
 There was a keyboard interrupt signal to stop the cfgmgr daemon. The
 daemon will now shut down.
-

src/lib/python/isc/config/config_data.py

@@ -109,14 +109,17 @@ def convert_type(spec_part, value):
 
             return ret
         elif data_type == "map":
-            map = ast.literal_eval(value)
-            if type(map) == dict:
-                # todo: check types of map contents too
-                return map
-            else:
-                raise isc.cc.data.DataTypeError(
-                           "Value in convert_type not a string "
-                           "specifying a dict")
+            try:
+                map = ast.literal_eval(value)
+                if type(map) == dict:
+                    # todo: check types of map contents too
+                    return map
+                else:
+                    raise isc.cc.data.DataTypeError(
+                               "Value in convert_type not a string "
+                               "specifying a dict")
+            except SyntaxError as se:
+                raise isc.cc.data.DataTypeError("Error parsing map: " + str(se))
         else:
             return value
     except ValueError as err:

src/lib/python/isc/config/tests/cfgmgr_test.py

@@ -74,6 +74,60 @@ class TestConfigManagerData(unittest.TestCase):
         self.assertEqual(self.config_manager_data, new_config)
         os.remove(output_file_name)
 
+    def check_existence(self, files, should_exist=[], should_not_exist=[]):
+        """Helper function for test_rename_config_file.
+           Arguments:
+           files: array of file names to check.
+           should_exist: array of indices, the files in 'files' with these
+                         indices should exist.
+           should_not_exist: array of indices, the files in 'files' with
+                             these indices should not exist."""
+        for n in should_exist:
+            self.assertTrue(os.path.exists(files[n]))
+        for n in should_not_exist:
+            self.assertFalse(os.path.exists(files[n]))
+
+    def test_rename_config_file(self):
+        # test file names, put in array for easy cleanup
+        filenames = [ "b10-config-rename-test",
+                      "b10-config-rename-test.bak",
+                      "b10-config-rename-test.bak.1",
+                      "b10-config-rename-test.bak.2" ]
+
+        for filename in filenames:
+            if os.path.exists(filename):
+                os.remove(filename)
+
+        # The original does not exist, so the new one should not be created
+        self.config_manager_data.rename_config_file(filenames[0])
+        self.check_existence(filenames, [], [0, 1, 2, 3])
+
+        # now create a file to rename, and call rename again
+        self.config_manager_data.write_to_file(filenames[0])
+        self.config_manager_data.rename_config_file(filenames[0])
+        self.check_existence(filenames, [1], [0, 2, 3])
+
+        # If backup already exists, give it a new name automatically
+        self.config_manager_data.write_to_file(filenames[0])
+        self.config_manager_data.rename_config_file(filenames[0])
+        self.check_existence(filenames, [1, 2], [0, 3])
+
+        # If backup already exists, give it a new name automatically with
+        # increasing postfix
+        self.config_manager_data.write_to_file(filenames[0])
+        self.config_manager_data.rename_config_file(filenames[0])
+        self.check_existence(filenames, [1, 2, 3], [0])
+
+        # Test with explicit renamed file argument
+        self.config_manager_data.rename_config_file(filenames[1],
+                                                    filenames[0])
+        self.check_existence(filenames, [0, 2, 3], [1])
+
+        # clean up again to be nice
+        for filename in filenames:
+            if os.path.exists(filename):
+                os.remove(filename)
+
     def test_equality(self):
         # tests the __eq__ function. Equality is only defined
         # by equality of the .data element. If data_path or db_filename
@@ -570,5 +624,6 @@ if __name__ == '__main__':
     if not 'CONFIG_TESTDATA_PATH' in os.environ or not 'CONFIG_WR_TESTDATA_PATH' in os.environ:
         print("You need to set the environment variable CONFIG_TESTDATA_PATH and CONFIG_WR_TESTDATA_PATH to point to the directory containing the test data files")
         exit(1)
+    isc.log.init("unittests")
+    isc.log.resetUnitTestRootLogger()
     unittest.main()
-

src/lib/python/isc/config/tests/config_data_test.py

@@ -157,6 +157,7 @@ class TestConfigData(unittest.TestCase):
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "a", "b" ])
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, [ "1", "b" ])
         self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, { "a": 1 })
+        self.assertRaises(isc.cc.data.DataTypeError, convert_type, spec_part, "\"{ \"a\": 1 }\"")
 
         spec_part = find_spec_part(config_spec, "value7")
         self.assertEqual(['1', '2'], convert_type(spec_part, '1, 2'))

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

@@ -14,6 +14,7 @@ EXTRA_DIST += config_messages.py
 EXTRA_DIST += notify_out_messages.py
 EXTRA_DIST += libxfrin_messages.py
 EXTRA_DIST += server_common_messages.py
+EXTRA_DIST += dbutil_messages.py
 
 CLEANFILES = __init__.pyc
 CLEANFILES += bind10_messages.pyc
@@ -29,6 +30,7 @@ CLEANFILES += config_messages.pyc
 CLEANFILES += notify_out_messages.pyc
 CLEANFILES += libxfrin_messages.pyc
 CLEANFILES += server_common_messages.pyc
+CLEANFILES += dbutil_messages.pyc
 
 CLEANDIRS = __pycache__
 

src/lib/python/isc/log_messages/dbutil_messages.py

@@ -0,0 +1 @@
+from work.dbutil_messages import *

tests/lettuce/features/nsec3_auth.feature

@@ -160,45 +160,41 @@ Feature: NSEC3 Authoritative service
     # Below are additional tests, not explicitely stated in RFC5155
     #
 
-    # THIS TEST CURRENTLY FAILS: An NSEC3 record is added twice
-    # See ticket #1688
-    #Scenario: 7.2.2 other; Name Error where one NSEC3 covers multiple parts of proof (closest encloser)
-    #    Given I have bind10 running with configuration nsec3/nsec3_auth.config
-    #    A dnssec query for b.x.w.example. should have rcode NXDOMAIN
-    #    The last query response should have flags qr aa rd
-    #    The last query response should have edns_flags do
-    #    The last query response should have ancount 0
-    #    The last query response should have nscount 6
-    #    The last query response should have adcount 1
-    #    The authority section of the last query response should be
-    #    """
-    #    example.	3600	IN	SOA	ns1.example. bugs.x.w.example. 1 3600 300 3600000 3600
-    #    example.	3600	IN	RRSIG	SOA 7 1 3600 20150420235959 20051021000000 40430 example. Hu25UIyNPmvPIVBrldN+9Mlp9Zql39qaUd8iq4ZLlYWfUUbbAS41pG+6 8z81q1xhkYAcEyHdVI2LmKusbZsT0Q==
-    #    b4um86eghhds6nea196smvmlo4ors995.example.	3600	IN	NSEC3	1 1 12 aabbccdd  gjeqe526plbf1g8mklp59enfd789njgi MX RRSIG 
-    #    b4um86eghhds6nea196smvmlo4ors995.example.	3600	IN	RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. ZkPG3M32lmoHM6pa3D6gZFGB/rhL//Bs3Omh5u4m/CUiwtblEVOaAKKZ d7S959OeiX43aLX3pOv0TSTyiTxIZg==
-    #    35mthgpgcu1qg68fab165klnsnk3dpvl.example.	3600	IN	NSEC3	1 1 12 aabbccdd  b4um86eghhds6nea196smvmlo4ors995 NS DS RRSIG 
-    #    35mthgpgcu1qg68fab165klnsnk3dpvl.example.	3600	IN	RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. g6jPUUpduAJKRljUsN8gB4UagAX0NxY9shwQAynzo8EUWH+z6hEIBlUT PGj15eZll6VhQqgZXtAIR3chwgW+SA==
-    #    """
+    Scenario: 7.2.2 other; Name Error where one NSEC3 covers multiple parts of proof (closest encloser)
+        Given I have bind10 running with configuration nsec3/nsec3_auth.config
+        A dnssec query for b.x.w.example. should have rcode NXDOMAIN
+        The last query response should have flags qr aa rd
+        The last query response should have edns_flags do
+        The last query response should have ancount 0
+        The last query response should have nscount 6
+        The last query response should have adcount 1
+        The authority section of the last query response should be
+        """
+        example.	3600	IN	SOA	ns1.example. bugs.x.w.example. 1 3600 300 3600000 3600
+        example.	3600	IN	RRSIG	SOA 7 1 3600 20150420235959 20051021000000 40430 example. Hu25UIyNPmvPIVBrldN+9Mlp9Zql39qaUd8iq4ZLlYWfUUbbAS41pG+6 8z81q1xhkYAcEyHdVI2LmKusbZsT0Q==
+        b4um86eghhds6nea196smvmlo4ors995.example.	3600	IN	NSEC3	1 1 12 aabbccdd  gjeqe526plbf1g8mklp59enfd789njgi MX RRSIG 
+        b4um86eghhds6nea196smvmlo4ors995.example.	3600	IN	RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. ZkPG3M32lmoHM6pa3D6gZFGB/rhL//Bs3Omh5u4m/CUiwtblEVOaAKKZ d7S959OeiX43aLX3pOv0TSTyiTxIZg==
+        35mthgpgcu1qg68fab165klnsnk3dpvl.example.	3600	IN	NSEC3	1 1 12 aabbccdd  b4um86eghhds6nea196smvmlo4ors995 NS DS RRSIG 
+        35mthgpgcu1qg68fab165klnsnk3dpvl.example.	3600	IN	RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. g6jPUUpduAJKRljUsN8gB4UagAX0NxY9shwQAynzo8EUWH+z6hEIBlUT PGj15eZll6VhQqgZXtAIR3chwgW+SA==
+        """
 
-    # THIS TEST CURRENTLY FAILS: An NSEC3 record is added twice
-    # See ticket #1688
-    #Scenario: 7.2.2 other; Name Error where one NSEC3 covers multiple parts of proof (wildcard)
-    #    Given I have bind10 running with configuration nsec3/nsec3_auth.config
-    #    A dnssec query for a.w.example. should have rcode NXDOMAIN
-    #    The last query response should have flags qr aa rd
-    #    The last query response should have edns_flags do
-    #    The last query response should have ancount 0
-    #    The last query response should have nscount 6
-    #    The last query response should have adcount 1
-    #    The authority section of the last query response should be
-    #    """
-    #    example.		3600	IN	SOA	ns1.example. bugs.x.w.example. 1 3600 300 3600000 3600
-    #    example.		3600	IN	RRSIG	SOA 7 1 3600 20150420235959 20051021000000 40430 example. Hu25UIyNPmvPIVBrldN+9Mlp9Zql39qaUd8iq4ZLlYWfUUbbAS41pG+6 8z81q1xhkYAcEyHdVI2LmKusbZsT0Q==
-    #    k8udemvp1j2f7eg6jebps17vp3n8i58h.example. 3600 IN NSEC3	1 1 12 AABBCCDD KOHAR7MBB8DC2CE8A9QVL8HON4K53UHI
-    #    k8udemvp1j2f7eg6jebps17vp3n8i58h.example. 3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. FtXGbvF0+wf8iWkyo73enAuVx03klN+pILBKS6qCcftVtfH4yVzsEZqu J27NHR7ruxJWDNMtOtx7w9WfcIg62A==
-    #    r53bq7cc2uvmubfu5ocmm6pers9tk9en.example. 3600 IN NSEC3	1 1 12 AABBCCDD T644EBQK9BIBCNA874GIVR6JOJ62MLHV MX RRSIG
-    #    r53bq7cc2uvmubfu5ocmm6pers9tk9en.example. 3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. aupviViruXs4bDg9rCbezzBMf9h1ZlDvbW/CZFKulIGXXLj8B/fsDJar XVDA9bnUoRhEbKp+HF1FWKW7RIJdtQ==
-    #    """
+    Scenario: 7.2.2 other; Name Error where one NSEC3 covers multiple parts of proof (wildcard)
+        Given I have bind10 running with configuration nsec3/nsec3_auth.config
+        A dnssec query for a.w.example. should have rcode NOERROR
+        The last query response should have flags qr aa rd
+        The last query response should have edns_flags do
+        The last query response should have ancount 0
+        The last query response should have nscount 6
+        The last query response should have adcount 1
+        The authority section of the last query response should be
+        """
+        example.		3600	IN	SOA	ns1.example. bugs.x.w.example. 1 3600 300 3600000 3600
+        example.		3600	IN	RRSIG	SOA 7 1 3600 20150420235959 20051021000000 40430 example. Hu25UIyNPmvPIVBrldN+9Mlp9Zql39qaUd8iq4ZLlYWfUUbbAS41pG+6 8z81q1xhkYAcEyHdVI2LmKusbZsT0Q==
+        k8udemvp1j2f7eg6jebps17vp3n8i58h.example. 3600 IN NSEC3	1 1 12 AABBCCDD KOHAR7MBB8DC2CE8A9QVL8HON4K53UHI
+        k8udemvp1j2f7eg6jebps17vp3n8i58h.example. 3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. FtXGbvF0+wf8iWkyo73enAuVx03klN+pILBKS6qCcftVtfH4yVzsEZqu J27NHR7ruxJWDNMtOtx7w9WfcIg62A==
+        r53bq7cc2uvmubfu5ocmm6pers9tk9en.example. 3600 IN NSEC3	1 1 12 AABBCCDD T644EBQK9BIBCNA874GIVR6JOJ62MLHV MX RRSIG
+        r53bq7cc2uvmubfu5ocmm6pers9tk9en.example. 3600 IN RRSIG	NSEC3 7 2 3600 20150420235959 20051021000000 40430 example. aupviViruXs4bDg9rCbezzBMf9h1ZlDvbW/CZFKulIGXXLj8B/fsDJar XVDA9bnUoRhEbKp+HF1FWKW7RIJdtQ==
+        """
 
     Scenario: Wildcard other: Wildcard name itself
         Given I have bind10 running with configuration nsec3/nsec3_auth.config