BIND 10 master, updated. 5d3f0570a21d753d681d170622df02ef39ebb70d [master] Fix invalid zone data in DataSrcClientsBuilderTest.loadZoneSQLite3

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Jan 22 11:45:14 UTC 2013


The branch, master has been updated
       via  5d3f0570a21d753d681d170622df02ef39ebb70d (commit)
       via  b5b877d1ec728e2dd0d8992a825dd2145c1277a9 (commit)
       via  c4a5a733f2e6729f4d91bc7c1117f4ff0fa70e1c (commit)
       via  f4a107a8f8775523e32d8ba0bc3e28e2f26eff3a (commit)
       via  c0b406791e7d3be3fb7962d52f6b6a320b4a7736 (commit)
       via  dc2aa27ea58ec3be03e742f2f4dd123a2c044200 (commit)
       via  671b6ae757f8a4f22bec449afa9b743c4ec2d719 (commit)
       via  4d44f0bf193dac303be3a15258fe8faaf857c6f9 (commit)
       via  58c958d4503a9a45be0267dc833c6e3be178dc14 (commit)
       via  45466d27ef5c4f095c7d3bf23b4d0a4ad30a8bcb (commit)
      from  2cbabde53f226cb4fa4883b9578268719f28732b (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 5d3f0570a21d753d681d170622df02ef39ebb70d
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Jan 22 17:10:53 2013 +0530

    [master] Fix invalid zone data in DataSrcClientsBuilderTest.loadZoneSQLite3

commit b5b877d1ec728e2dd0d8992a825dd2145c1277a9
Merge: c4a5a73 2cbabde
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Jan 22 17:00:03 2013 +0530

    Merge branch 'master' into trac2499

-----------------------------------------------------------------------

Summary of changes:
 .../auth/tests/datasrc_clients_builder_unittest.cc |    3 +-
 src/lib/datasrc/memory/Makefile.am                 |    1 +
 src/lib/datasrc/memory/memory_messages.mes         |   13 ++
 src/lib/datasrc/memory/rrset_collection.cc         |   57 ++++++++
 src/lib/datasrc/memory/rrset_collection.h          |   85 ++++++++++++
 src/lib/datasrc/memory/zone_data_loader.cc         |   32 ++++-
 src/lib/datasrc/memory/zone_data_loader.h          |    8 +-
 src/lib/datasrc/tests/client_list_unittest.cc      |   70 ++++++----
 src/lib/datasrc/tests/memory/Makefile.am           |    1 +
 .../datasrc/tests/memory/memory_client_unittest.cc |   43 ++++++-
 .../tests/memory/rrset_collection_unittest.cc      |   88 +++++++++++++
 src/lib/datasrc/tests/memory/testdata/Makefile.am  |    1 +
 .../testdata/example.org-duplicate-type.zone       |    3 +-
 .../tests/memory/testdata/example.org-empty.zone   |    3 +-
 .../memory/testdata/example.org-multiple.zone      |    3 +-
 .../example.org-rrsig-follows-nothing.zone         |    3 +-
 .../tests/memory/testdata/example.org-rrsigs.zone  |    3 +-
 .../tests/memory/testdata/rrset-collection.zone    |    4 +
 src/lib/datasrc/tests/zone_loader_unittest.cc      |  136 ++++++++++++++++----
 19 files changed, 483 insertions(+), 74 deletions(-)
 create mode 100644 src/lib/datasrc/memory/rrset_collection.cc
 create mode 100644 src/lib/datasrc/memory/rrset_collection.h
 create mode 100644 src/lib/datasrc/tests/memory/rrset_collection_unittest.cc
 create mode 100644 src/lib/datasrc/tests/memory/testdata/rrset-collection.zone

-----------------------------------------------------------------------
diff --git a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
index 838e032..b0c06d4 100644
--- a/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
+++ b/src/bin/auth/tests/datasrc_clients_builder_unittest.cc
@@ -328,7 +328,8 @@ TEST_F(DataSrcClientsBuilderTest,
 {
     // Prepare the database first
     const std::string test_db = TEST_DATA_BUILDDIR "/auth_test.sqlite3.copied";
-    std::stringstream ss("example.org. 3600 IN SOA . . 0 0 0 0 0\n");
+    std::stringstream ss("example.org. 3600 IN SOA . . 0 0 0 0 0\n"
+                         "example.org. 3600 IN NS ns1.example.org.");
     createSQLite3DB(rrclass, Name("example.org"), test_db.c_str(), ss);
     // This describes the data source in the configuration
     const ConstElementPtr config(Element::fromJSON("{"
diff --git a/src/lib/datasrc/memory/Makefile.am b/src/lib/datasrc/memory/Makefile.am
index 72b3273..c0ee688 100644
--- a/src/lib/datasrc/memory/Makefile.am
+++ b/src/lib/datasrc/memory/Makefile.am
@@ -15,6 +15,7 @@ libdatasrc_memory_la_SOURCES += rdataset.h rdataset.cc
 libdatasrc_memory_la_SOURCES += treenode_rrset.h treenode_rrset.cc
 libdatasrc_memory_la_SOURCES += rdata_serialization.h rdata_serialization.cc
 libdatasrc_memory_la_SOURCES += zone_data.h zone_data.cc
+libdatasrc_memory_la_SOURCES += rrset_collection.h rrset_collection.cc
 libdatasrc_memory_la_SOURCES += segment_object_holder.h
 libdatasrc_memory_la_SOURCES += logger.h logger.cc
 libdatasrc_memory_la_SOURCES += zone_table.h zone_table.cc
diff --git a/src/lib/datasrc/memory/memory_messages.mes b/src/lib/datasrc/memory/memory_messages.mes
index 1b67093..81ac7f6 100644
--- a/src/lib/datasrc/memory/memory_messages.mes
+++ b/src/lib/datasrc/memory/memory_messages.mes
@@ -88,3 +88,16 @@ The software refuses to load NS records into a wildcard domain.  It isn't
 explicitly forbidden, but the protocol is ambiguous about how this should
 behave and BIND 9 refuses that as well. Please describe your intention using
 different tools.
+
+% DATASRC_MEMORY_CHECK_ERROR post-load check of zone %1/%2 failed: %3
+The zone was loaded into the data source successfully, but the content fails
+basic sanity checks. See the message if you want to know what exactly is wrong
+with the data. The data can not be used and previous version, if any, will be
+preserved.
+
+% DATASRC_MEMORY_CHECK_WARNING %1/%2: %3
+The zone was loaded into the data source successfully, but there's some problem
+with the content. The problem does not stop the new version from being used
+(though there may be other problems that do, see DATASRC_MEMORY_CHECK_ERROR),
+but it should still be checked and fixed. See the message to know what exactly
+is wrong with the data.
diff --git a/src/lib/datasrc/memory/rrset_collection.cc b/src/lib/datasrc/memory/rrset_collection.cc
new file mode 100644
index 0000000..92319f6
--- /dev/null
+++ b/src/lib/datasrc/memory/rrset_collection.cc
@@ -0,0 +1,57 @@
+// Copyright (C) 2013  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.
+
+#include <datasrc/memory/rrset_collection.h>
+#include <datasrc/memory/treenode_rrset.h>
+
+#include <exceptions/exceptions.h>
+
+using namespace isc;
+using namespace isc::dns;
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+
+ConstRRsetPtr
+RRsetCollection::find(const isc::dns::Name& name,
+                      const isc::dns::RRClass& rrclass,
+                      const isc::dns::RRType& rrtype) const
+{
+    if (rrclass != rrclass_) {
+        // We could throw an exception here, but RRsetCollection is
+        // expected to support an arbitrary collection of RRsets, and it
+        // can be queried just as arbitrarily. So we just return nothing
+        // here.
+        return (ConstRRsetPtr());
+    }
+
+    const ZoneTree& tree = zone_data_.getZoneTree();
+    const ZoneNode *node = NULL;
+    ZoneTree::Result result = tree.find(name, &node);
+    if (result != ZoneTree::EXACTMATCH) {
+        return (ConstRRsetPtr());
+    }
+
+    const RdataSet* rdataset = RdataSet::find(node->getData(), rrtype);
+    if (rdataset == NULL) {
+        return (ConstRRsetPtr());
+    }
+
+    return (ConstRRsetPtr(new TreeNodeRRset(rrclass_, node, rdataset, true)));
+}
+
+} // end of namespace memory
+} // end of namespace datasrc
+} // end of namespace isc
diff --git a/src/lib/datasrc/memory/rrset_collection.h b/src/lib/datasrc/memory/rrset_collection.h
new file mode 100644
index 0000000..e5edd64
--- /dev/null
+++ b/src/lib/datasrc/memory/rrset_collection.h
@@ -0,0 +1,85 @@
+// Copyright (C) 2013  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.
+
+#ifndef RRSET_COLLECTION_DATASRC_MEMORY_H
+#define RRSET_COLLECTION_DATASRC_MEMORY_H 1
+
+#include <dns/rrset_collection_base.h>
+#include <dns/rrclass.h>
+
+#include <datasrc/memory/zone_data.h>
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+
+/// \brief In-memory derivation of \c isc::dns::RRsetCollectionBase.
+class RRsetCollection : public isc::dns::RRsetCollectionBase {
+public:
+    /// \brief Constructor.
+    ///
+    /// No reference (count via \c shared_ptr) to the \c ZoneData is
+    /// acquired. The RRsetCollection must not be used after its
+    /// \c ZoneData has been destroyed.
+    ///
+    /// \param zone_data The ZoneData to wrap around.
+    /// \param rrclass The RRClass of the records in the zone.
+    RRsetCollection(ZoneData& zone_data, const isc::dns::RRClass& rrclass) :
+        zone_data_(zone_data),
+        rrclass_(rrclass)
+    {}
+
+    /// \brief Destructor
+    virtual ~RRsetCollection() {}
+
+    /// \brief Find a matching RRset in the collection.
+    ///
+    /// Returns the RRset in the collection that exactly matches the
+    /// given \c name, \c rrclass and \c rrtype.  If no matching RRset
+    /// is found, \c NULL is returned.
+    ///
+    /// \throw isc::dns::RRsetCollectionError if \c find() results in
+    /// some underlying error.
+    ///
+    /// \param name The name of the RRset to search for.
+    /// \param rrclass The class of the RRset to search for.
+    /// \param rrtype The type of the RRset to search for.
+    /// \returns The RRset if found, \c NULL otherwise.
+    virtual isc::dns::ConstRRsetPtr find(const isc::dns::Name& name,
+                                         const isc::dns::RRClass& rrclass,
+                                         const isc::dns::RRType& rrtype) const;
+
+protected:
+    virtual RRsetCollectionBase::IterPtr getBeginning() {
+        isc_throw(NotImplemented, "This method is not implemented.");
+    }
+
+    virtual RRsetCollectionBase::IterPtr getEnd() {
+        isc_throw(NotImplemented, "This method is not implemented.");
+    }
+
+private:
+    ZoneData& zone_data_;
+    isc::dns::RRClass rrclass_;
+};
+
+} // end of namespace memory
+} // end of namespace datasrc
+} // end of namespace isc
+
+#endif  // RRSET_COLLECTION_DATASRC_MEMORY_H
+
+// Local Variables:
+// mode: c++
+// End:
diff --git a/src/lib/datasrc/memory/zone_data_loader.cc b/src/lib/datasrc/memory/zone_data_loader.cc
index e224224..e3c9c3f 100644
--- a/src/lib/datasrc/memory/zone_data_loader.cc
+++ b/src/lib/datasrc/memory/zone_data_loader.cc
@@ -18,11 +18,13 @@
 #include <datasrc/memory/logger.h>
 #include <datasrc/memory/segment_object_holder.h>
 #include <datasrc/memory/util_internal.h>
+#include <datasrc/memory/rrset_collection.h>
 
 #include <dns/master_loader.h>
 #include <dns/rrcollator.h>
 #include <dns/rdataclass.h>
 #include <dns/rrset.h>
+#include <dns/zone_checker.h>
 
 #include <boost/foreach.hpp>
 #include <boost/bind.hpp>
@@ -148,6 +150,22 @@ ZoneDataLoader::getCurrentName() const {
     return (node_rrsigsets_.begin()->second->getName());
 }
 
+void
+logWarning(const dns::Name* zone_name, const dns::RRClass* rrclass,
+           const std::string& reason)
+{
+    LOG_WARN(logger, DATASRC_MEMORY_CHECK_WARNING).arg(*zone_name).
+        arg(*rrclass).arg(reason);
+}
+
+void
+logError(const dns::Name* zone_name, const dns::RRClass* rrclass,
+         const std::string& reason)
+{
+    LOG_ERROR(logger, DATASRC_MEMORY_CHECK_ERROR).arg(*zone_name).arg(*rrclass).
+        arg(reason);
+}
+
 ZoneData*
 loadZoneDataInternal(util::MemorySegment& mem_sgmt,
                      const isc::dns::RRClass& rrclass,
@@ -172,12 +190,14 @@ loadZoneDataInternal(util::MemorySegment& mem_sgmt,
         }
     }
 
-    // When an empty zone file is loaded, the origin doesn't even have
-    // an SOA RR. This condition should be avoided, and hence load()
-    // should throw when an empty zone is loaded.
-    if (RdataSet::find(rdataset, RRType::SOA()) == NULL) {
-        isc_throw(EmptyZone,
-                  "Won't create an empty zone for: " << zone_name);
+    RRsetCollection collection(*(holder.get()), rrclass);
+    const dns::ZoneCheckerCallbacks
+        callbacks(boost::bind(&logError, &zone_name, &rrclass, _1),
+                  boost::bind(&logWarning, &zone_name, &rrclass, _1));
+    if (!dns::checkZone(zone_name, rrclass, collection, callbacks)) {
+        isc_throw(ZoneValidationError,
+                  "Errors found when validating zone: "
+                  << zone_name << "/" << rrclass);
     }
 
     return (holder.release());
diff --git a/src/lib/datasrc/memory/zone_data_loader.h b/src/lib/datasrc/memory/zone_data_loader.h
index 6b409ec..db7ac3b 100644
--- a/src/lib/datasrc/memory/zone_data_loader.h
+++ b/src/lib/datasrc/memory/zone_data_loader.h
@@ -26,12 +26,12 @@ namespace isc {
 namespace datasrc {
 namespace memory {
 
-/// \brief Zone is empty exception.
+/// \brief Zone is invalid exception.
 ///
-/// This is thrown if an empty zone would be created during
+/// This is thrown if an invalid zone would be created during
 /// \c loadZoneData().
-struct EmptyZone : public ZoneLoaderException {
-    EmptyZone(const char* file, size_t line, const char* what) :
+struct ZoneValidationError : public ZoneLoaderException {
+    ZoneValidationError(const char* file, size_t line, const char* what) :
         ZoneLoaderException(file, line, what)
     {}
 };
diff --git a/src/lib/datasrc/tests/client_list_unittest.cc b/src/lib/datasrc/tests/client_list_unittest.cc
index 6769e9b..29a0daa 100644
--- a/src/lib/datasrc/tests/client_list_unittest.cc
+++ b/src/lib/datasrc/tests/client_list_unittest.cc
@@ -77,7 +77,7 @@ public:
     };
     class Iterator : public ZoneIterator {
     public:
-        Iterator(const Name& origin, bool include_ns) :
+        Iterator(const Name& origin, bool include_a) :
             origin_(origin),
             soa_(new RRset(origin_, RRClass::IN(), RRType::SOA(),
                            RRTTL(3600)))
@@ -89,12 +89,21 @@ public:
                                                0, 0, 0, 0, 0));
             rrsets_.push_back(soa_);
 
-            if (include_ns) {
-                ns_.reset(new RRset(origin_, RRClass::IN(), RRType::NS(),
-                                    RRTTL(3600)));
-                ns_->addRdata(rdata::generic::NS(Name::ROOT_NAME()));
-                rrsets_.push_back(ns_);
+            RRsetPtr rrset(new RRset(origin_, RRClass::IN(), RRType::NS(),
+                                     RRTTL(3600)));
+            rrset->addRdata(rdata::generic::NS(Name::ROOT_NAME()));
+            rrsets_.push_back(rrset);
+
+            if (include_a) {
+                 // Dummy A rrset. This is used for checking zone data
+                 // after reload.
+                 rrset.reset(new RRset(Name("tstzonedata").concatenate(origin_),
+                                       RRClass::IN(), RRType::A(),
+                                       RRTTL(3600)));
+                 rrset->addRdata(rdata::in::A("192.0.2.1"));
+                 rrsets_.push_back(rrset);
             }
+
             rrsets_.push_back(ConstRRsetPtr());
 
             it_ = rrsets_.begin();
@@ -110,13 +119,12 @@ public:
     private:
         const Name origin_;
         const RRsetPtr soa_;
-        RRsetPtr ns_;
         std::vector<ConstRRsetPtr> rrsets_;
         std::vector<ConstRRsetPtr>::const_iterator it_;
     };
     // Constructor from a list of zones.
     MockDataSourceClient(const char* zone_names[]) :
-        have_ns_(true), use_baditerator_(true)
+        have_a_(true), use_baditerator_(true)
     {
         for (const char** zone(zone_names); *zone; ++zone) {
             zones.insert(Name(*zone));
@@ -128,7 +136,7 @@ public:
                          const ConstElementPtr& configuration) :
         type_(type),
         configuration_(configuration),
-        have_ns_(true), use_baditerator_(true)
+        have_a_(true), use_baditerator_(true)
     {
         EXPECT_NE("MasterFiles", type) << "MasterFiles is a special case "
             "and it never should be created as a data source client";
@@ -176,19 +184,19 @@ public:
         } else {
             FindResult result(findZone(name));
             if (result.code == isc::datasrc::result::SUCCESS) {
-                return (ZoneIteratorPtr(new Iterator(name, have_ns_)));
+                return (ZoneIteratorPtr(new Iterator(name, have_a_)));
             } else {
                 isc_throw(DataSourceError, "No such zone");
             }
         }
     }
-    void disableNS() { have_ns_ = false; }
+    void disableA() { have_a_ = false; }
     void disableBadIterator() { use_baditerator_ = false; }
     const string type_;
     const ConstElementPtr configuration_;
 private:
     set<Name> zones;
-    bool have_ns_; // control the iterator behavior wrt whether to include NS
+    bool have_a_; // control the iterator behavior whether to include A record
     bool use_baditerator_; // whether to use bogus zone iterators for tests
 };
 
@@ -285,7 +293,7 @@ public:
         MockDataSourceClient mock_client(zones);
         // Disable some default features of the mock to distinguish the
         // temporary case from normal case.
-        mock_client.disableNS();
+        mock_client.disableA();
         mock_client.disableBadIterator();
 
         // Create cache from the temporary data source, and push it to the
@@ -925,14 +933,19 @@ TYPED_TEST(ReloadTest, reloadSuccess) {
     this->list_->configure(this->config_elem_zones_, true);
     const Name name("example.org");
     this->prepareCache(0, name);
-    // The cache currently contains a tweaked version of zone, which doesn't
-    // have apex NS.  So the lookup should result in NXRRSET.
-    EXPECT_EQ(ZoneFinder::NXRRSET,
-              this->list_->find(name).finder_->find(name, RRType::NS())->code);
+    // The cache currently contains a tweaked version of zone, which
+    // doesn't have "tstzonedata" A record.  So the lookup should result
+    // in NXDOMAIN.
+    EXPECT_EQ(ZoneFinder::NXDOMAIN,
+              this->list_->find(name).finder_->
+                  find(Name("tstzonedata").concatenate(name),
+                       RRType::A())->code);
     // Now reload the full zone. It should be there now.
     EXPECT_EQ(ConfigurableClientList::ZONE_SUCCESS, this->doReload(name));
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              this->list_->find(name).finder_->find(name, RRType::NS())->code);
+              this->list_->find(name).finder_->
+                  find(Name("tstzonedata").concatenate(name),
+                       RRType::A())->code);
 }
 
 // The cache is not enabled. The load should be rejected.
@@ -941,14 +954,18 @@ TYPED_TEST(ReloadTest, reloadNotEnabled) {
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the thing.
     this->prepareCache(0, name);
-    // See the reloadSuccess test.  This should result in NXRRSET.
-    EXPECT_EQ(ZoneFinder::NXRRSET,
-              this->list_->find(name).finder_->find(name, RRType::NS())->code);
+    // See the reloadSuccess test.  This should result in NXDOMAIN.
+    EXPECT_EQ(ZoneFinder::NXDOMAIN,
+              this->list_->find(name).finder_->
+                  find(Name("tstzonedata").concatenate(name),
+                       RRType::A())->code);
     // Now reload. It should reject it.
     EXPECT_EQ(ConfigurableClientList::CACHE_DISABLED, this->doReload(name));
     // Nothing changed here
-    EXPECT_EQ(ZoneFinder::NXRRSET,
-              this->list_->find(name).finder_->find(name, RRType::NS())->code);
+    EXPECT_EQ(ZoneFinder::NXDOMAIN,
+              this->list_->find(name).finder_->
+                  find(Name("tstzonedata").concatenate(name),
+                       RRType::A())->code);
 }
 
 // Test several cases when the zone does not exist
@@ -974,10 +991,11 @@ TYPED_TEST(ReloadTest, reloadNoSuchZone) {
               this->list_->find(Name("example.cz")).dsrc_client_);
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
               this->list_->find(Name("sub.example.com"), true).dsrc_client_);
-    // Not reloaded, so NS shouldn't be visible yet.
-    EXPECT_EQ(ZoneFinder::NXRRSET,
+    // Not reloaded, so A record shouldn't be visible yet.
+    EXPECT_EQ(ZoneFinder::NXDOMAIN,
               this->list_->find(Name("example.com")).finder_->
-              find(Name("example.com"), RRType::NS())->code);
+                  find(Name("tstzonedata.example.com"),
+                       RRType::A())->code);
 }
 
 // Check we gracefuly throw an exception when a zone disappeared in
diff --git a/src/lib/datasrc/tests/memory/Makefile.am b/src/lib/datasrc/tests/memory/Makefile.am
index a5d73b4..f77d212 100644
--- a/src/lib/datasrc/tests/memory/Makefile.am
+++ b/src/lib/datasrc/tests/memory/Makefile.am
@@ -32,6 +32,7 @@ run_unittests_SOURCES += ../../tests/faked_nsec3.h ../../tests/faked_nsec3.cc
 run_unittests_SOURCES += memory_segment_test.h
 run_unittests_SOURCES += segment_object_holder_unittest.cc
 run_unittests_SOURCES += memory_client_unittest.cc
+run_unittests_SOURCES += rrset_collection_unittest.cc
 run_unittests_SOURCES += zone_data_loader_unittest.cc
 run_unittests_SOURCES += zone_data_updater_unittest.cc
 run_unittests_SOURCES += zone_table_segment_test.h
diff --git a/src/lib/datasrc/tests/memory/memory_client_unittest.cc b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
index 0a03645..74e7917 100644
--- a/src/lib/datasrc/tests/memory/memory_client_unittest.cc
+++ b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
@@ -59,6 +59,7 @@ namespace {
 const char* rrset_data[] = {
     "example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. "
     "68 3600 300 3600000 3600",
+    "example.org. 3600 IN NS ns1.example.org.",
     "a.example.org. 3600 IN A 192.168.0.1\n" // RRset containing 2 RRs
     "a.example.org. 3600 IN A 192.168.0.2",
     "a.example.org. 3600 IN RRSIG A 5 3 3600 20150420235959 20051021000000 "
@@ -225,7 +226,7 @@ TEST_F(MemoryClientTest, loadEmptyZoneFileThrows) {
 
     EXPECT_THROW(client_->load(Name("."),
                                TEST_DATA_DIR "/empty.zone"),
-                 EmptyZone);
+                 ZoneValidationError);
 
     EXPECT_EQ(0, client_->getZoneCount());
 
@@ -255,6 +256,11 @@ TEST_F(MemoryClientTest, loadFromIterator) {
     EXPECT_TRUE(rrset);
     EXPECT_EQ(RRType::SOA(), rrset->getType());
 
+    // RRType::NS() RRset
+    rrset = iterator->getNextRRset();
+    EXPECT_TRUE(rrset);
+    EXPECT_EQ(RRType::NS(), rrset->getType());
+
     // RRType::MX() RRset
     rrset = iterator->getNextRRset();
     EXPECT_TRUE(rrset);
@@ -377,6 +383,10 @@ TEST_F(MemoryClientTest, loadReloadZone) {
     EXPECT_EQ(RRType::SOA(), set->type);
 
     set = set->getNext();
+    EXPECT_NE(static_cast<const RdataSet*>(NULL), set);
+    EXPECT_EQ(RRType::NS(), set->type);
+
+    set = set->getNext();
     EXPECT_EQ(static_cast<const RdataSet*>(NULL), set);
 
     /* Check ns1.example.org */
@@ -402,6 +412,10 @@ TEST_F(MemoryClientTest, loadReloadZone) {
     EXPECT_EQ(RRType::SOA(), set->type);
 
     set = set->getNext();
+    EXPECT_NE(static_cast<const RdataSet*>(NULL), set);
+    EXPECT_EQ(RRType::NS(), set->type);
+
+    set = set->getNext();
     EXPECT_EQ(static_cast<const RdataSet*>(NULL), set);
 
     /* Check ns1.example.org */
@@ -636,9 +650,14 @@ TEST_F(MemoryClientTest, getIterator) {
     ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
 
     // First we have the SOA
-    ConstRRsetPtr rrset_soa(iterator->getNextRRset());
-    EXPECT_TRUE(rrset_soa);
-    EXPECT_EQ(RRType::SOA(), rrset_soa->getType());
+    ConstRRsetPtr rrset(iterator->getNextRRset());
+    EXPECT_TRUE(rrset);
+    EXPECT_EQ(RRType::SOA(), rrset->getType());
+
+    // Then the NS
+    rrset = iterator->getNextRRset();
+    EXPECT_TRUE(rrset);
+    EXPECT_EQ(RRType::NS(), rrset->getType());
 
     // There's nothing else in this iterator
     EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
@@ -659,6 +678,11 @@ TEST_F(MemoryClientTest, getIteratorSeparateRRs) {
     EXPECT_TRUE(rrset);
     EXPECT_EQ(RRType::SOA(), rrset->getType());
 
+    // Then, the NS
+    rrset = iterator->getNextRRset();
+    EXPECT_TRUE(rrset);
+    EXPECT_EQ(RRType::NS(), rrset->getType());
+
     // Only one RRType::A() RRset
     rrset = iterator->getNextRRset();
     EXPECT_TRUE(rrset);
@@ -667,7 +691,6 @@ TEST_F(MemoryClientTest, getIteratorSeparateRRs) {
     // There's nothing else in this zone
     EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
 
-
     // separate_rrs = true
     ZoneIteratorPtr iterator2(client_->getIterator(Name("example.org"), true));
 
@@ -676,6 +699,11 @@ TEST_F(MemoryClientTest, getIteratorSeparateRRs) {
     EXPECT_TRUE(rrset);
     EXPECT_EQ(RRType::SOA(), rrset->getType());
 
+    // Then, the NS
+    rrset = iterator2->getNextRRset();
+    EXPECT_TRUE(rrset);
+    EXPECT_EQ(RRType::NS(), rrset->getType());
+
     // First RRType::A() RRset
     rrset = iterator2->getNextRRset();
     EXPECT_TRUE(rrset);
@@ -751,6 +779,11 @@ TEST_F(MemoryClientTest, findZoneData) {
     EXPECT_NE(static_cast<const RdataSet*>(NULL), set);
     EXPECT_EQ(RRType::SOA(), set->type);
 
+    /* Check NS */
+    set = set->getNext();
+    EXPECT_NE(static_cast<const RdataSet*>(NULL), set);
+    EXPECT_EQ(RRType::NS(), set->type);
+
     set = set->getNext();
     EXPECT_EQ(static_cast<const RdataSet*>(NULL), set);
 
diff --git a/src/lib/datasrc/tests/memory/rrset_collection_unittest.cc b/src/lib/datasrc/tests/memory/rrset_collection_unittest.cc
new file mode 100644
index 0000000..04e285b
--- /dev/null
+++ b/src/lib/datasrc/tests/memory/rrset_collection_unittest.cc
@@ -0,0 +1,88 @@
+// Copyright (C) 2013  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.
+
+
+#include <datasrc/memory/rrset_collection.h>
+
+#include "memory_segment_test.h"
+
+#include <datasrc/memory/zone_data_loader.h>
+#include <datasrc/memory/segment_object_holder.h>
+#include <dns/rrttl.h>
+#include <dns/rdataclass.h>
+
+#include <gtest/gtest.h>
+
+using namespace isc::dns;
+using namespace isc::dns::rdata;
+using namespace std;
+using namespace isc::datasrc;
+using namespace isc::datasrc::memory;
+using namespace isc::datasrc::memory::detail;
+
+namespace {
+
+// Note: This class uses loadZoneData() to construct a ZoneData object,
+// which internally uses an RRsetCollection for validation. We assume
+// that loadZoneData() works at this point and test the RRsetCollection
+// around the ZoneData returned.
+class RRsetCollectionTest : public ::testing::Test {
+public:
+    RRsetCollectionTest() :
+        rrclass("IN"),
+        origin("example.org"),
+        zone_file(TEST_DATA_DIR "/rrset-collection.zone"),
+        zone_data_holder(mem_sgmt,
+                         loadZoneData(mem_sgmt, rrclass, origin, zone_file),
+                         rrclass),
+        collection(*zone_data_holder.get(), rrclass)
+    {}
+
+    const RRClass rrclass;
+    const Name origin;
+    std::string zone_file;
+    test::MemorySegmentTest mem_sgmt;
+    SegmentObjectHolder<ZoneData, RRClass> zone_data_holder;
+    RRsetCollection collection;
+};
+
+TEST_F(RRsetCollectionTest, find) {
+    const RRsetCollection& ccln = collection;
+    ConstRRsetPtr rrset = ccln.find(Name("www.example.org"), rrclass,
+                                    RRType::A());
+    EXPECT_TRUE(rrset);
+    EXPECT_EQ(RRType::A(), rrset->getType());
+    EXPECT_EQ(RRTTL(3600), rrset->getTTL());
+    EXPECT_EQ(RRClass("IN"), rrset->getClass());
+    EXPECT_EQ(Name("www.example.org"), rrset->getName());
+
+    // foo.example.org doesn't exist
+    rrset = ccln.find(Name("foo.example.org"), rrclass, RRType::A());
+    EXPECT_FALSE(rrset);
+
+    // www.example.org exists, but not with MX
+    rrset = ccln.find(Name("www.example.org"), rrclass, RRType::MX());
+    EXPECT_FALSE(rrset);
+
+    // www.example.org exists, with AAAA
+    rrset = ccln.find(Name("www.example.org"), rrclass, RRType::AAAA());
+    EXPECT_TRUE(rrset);
+
+    // www.example.org with AAAA does not exist in RRClass::CH()
+    rrset = ccln.find(Name("www.example.org"), RRClass::CH(),
+                            RRType::AAAA());
+    EXPECT_FALSE(rrset);
+}
+
+} // namespace
diff --git a/src/lib/datasrc/tests/memory/testdata/Makefile.am b/src/lib/datasrc/tests/memory/testdata/Makefile.am
index ef5f08b..b076837 100644
--- a/src/lib/datasrc/tests/memory/testdata/Makefile.am
+++ b/src/lib/datasrc/tests/memory/testdata/Makefile.am
@@ -29,6 +29,7 @@ EXTRA_DIST += example.org-rrsigs.zone
 EXTRA_DIST += example.org-wildcard-dname.zone
 EXTRA_DIST += example.org-wildcard-ns.zone
 EXTRA_DIST += example.org-wildcard-nsec3.zone
+EXTRA_DIST += rrset-collection.zone
 
 EXTRA_DIST += 2503-test.zone
 EXTRA_DIST += 2504-test.zone
diff --git a/src/lib/datasrc/tests/memory/testdata/example.org-duplicate-type.zone b/src/lib/datasrc/tests/memory/testdata/example.org-duplicate-type.zone
index 6e5c1b8..799b54e 100644
--- a/src/lib/datasrc/tests/memory/testdata/example.org-duplicate-type.zone
+++ b/src/lib/datasrc/tests/memory/testdata/example.org-duplicate-type.zone
@@ -1,4 +1,5 @@
-example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 76 3600 300 3600000 3600
+example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 77 3600 300 3600000 3600
+example.org. 3600 IN NS		ns1.example.org.
 ns1.example.org.		      3600 IN A	 	192.168.0.1
 ns1.example.org.		      3600 IN A	 	192.168.0.2
 ns1.example.org.		      3600 IN AAAA 	::1
diff --git a/src/lib/datasrc/tests/memory/testdata/example.org-empty.zone b/src/lib/datasrc/tests/memory/testdata/example.org-empty.zone
index f11b9b8..899a412 100644
--- a/src/lib/datasrc/tests/memory/testdata/example.org-empty.zone
+++ b/src/lib/datasrc/tests/memory/testdata/example.org-empty.zone
@@ -1,2 +1,3 @@
 ;; empty example.org zone
-example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 68 3600 300 3600000 3600
+example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 71 3600 300 3600000 3600
+example.org. 3600 IN NS		ns1.example.org.
diff --git a/src/lib/datasrc/tests/memory/testdata/example.org-multiple.zone b/src/lib/datasrc/tests/memory/testdata/example.org-multiple.zone
index f473ae6..b94d806 100644
--- a/src/lib/datasrc/tests/memory/testdata/example.org-multiple.zone
+++ b/src/lib/datasrc/tests/memory/testdata/example.org-multiple.zone
@@ -1,4 +1,5 @@
 ;; Multiple RDATA for testing separate RRs iterator
-example.org.  		   	 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 78 3600 300 3600000 3600
+example.org.  		   	 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 80 3600 300 3600000 3600
+example.org.			 3600 IN NS	ns1.example.org.
 a.example.org.		   	 3600 IN A	192.168.0.1
 a.example.org.		   	 3600 IN A	192.168.0.2
diff --git a/src/lib/datasrc/tests/memory/testdata/example.org-rrsig-follows-nothing.zone b/src/lib/datasrc/tests/memory/testdata/example.org-rrsig-follows-nothing.zone
index ef5f887..59dda88 100644
--- a/src/lib/datasrc/tests/memory/testdata/example.org-rrsig-follows-nothing.zone
+++ b/src/lib/datasrc/tests/memory/testdata/example.org-rrsig-follows-nothing.zone
@@ -1,5 +1,6 @@
 ;; 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. 68 3600 300 3600000 3600
+example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 69 3600 300 3600000 3600
+example.org. 3600 IN NS		ns1.example.org.
 ns1.example.org.		      3600 IN RRSIG	A 7 3 3600 20150420235959 20051021000000 40430 example.org. FAKEFAKE
diff --git a/src/lib/datasrc/tests/memory/testdata/example.org-rrsigs.zone b/src/lib/datasrc/tests/memory/testdata/example.org-rrsigs.zone
index 1c780b1..3ea070f 100644
--- a/src/lib/datasrc/tests/memory/testdata/example.org-rrsigs.zone
+++ b/src/lib/datasrc/tests/memory/testdata/example.org-rrsigs.zone
@@ -1,7 +1,8 @@
 ;; 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. 74 3600 300 3600000 3600
+example.org. 3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 75 3600 300 3600000 3600
+example.org. 3600 IN NS		ns1.example.org.
 ns1.example.org.		      3600 IN A	 	192.168.0.1
 ns1.example.org.		      3600 IN RRSIG	A 7 3 3600 20150420235959 20051021000000 40430 example.org. FAKEFAKE
 ns1.example.org.		      3600 IN RRSIG	A 7 2 3600 20150420235959 20051021000000 40430 example.org. FAKEFAKE
diff --git a/src/lib/datasrc/tests/memory/testdata/rrset-collection.zone b/src/lib/datasrc/tests/memory/testdata/rrset-collection.zone
new file mode 100644
index 0000000..332848f
--- /dev/null
+++ b/src/lib/datasrc/tests/memory/testdata/rrset-collection.zone
@@ -0,0 +1,4 @@
+example.org.	     3600 IN SOA	ns1.example.org. bugs.x.w.example.org. 72 3600 300 3600000 3600
+example.org.	     3600 IN NS		ns1.example.org.
+www.example.org.     3600 IN A		192.0.2.6
+www.example.org.     3600 IN AAAA	2001:db8::6
diff --git a/src/lib/datasrc/tests/zone_loader_unittest.cc b/src/lib/datasrc/tests/zone_loader_unittest.cc
index 28153e4..0f44074 100644
--- a/src/lib/datasrc/tests/zone_loader_unittest.cc
+++ b/src/lib/datasrc/tests/zone_loader_unittest.cc
@@ -21,6 +21,7 @@
 #include <dns/rrclass.h>
 #include <dns/name.h>
 #include <dns/rrset.h>
+#include <dns/rdataclass.h>
 #include <util/memory_segment_local.h>
 #include <exceptions/exceptions.h>
 
@@ -32,15 +33,11 @@
 #include <string>
 #include <vector>
 
-using isc::dns::RRClass;
-using isc::dns::Name;
-using isc::dns::RRType;
-using isc::dns::ConstRRsetPtr;
-using isc::dns::RRsetPtr;
+using namespace isc::dns;
+using namespace isc::datasrc;
+using boost::shared_ptr;
 using std::string;
 using std::vector;
-using boost::shared_ptr;
-using namespace isc::datasrc;
 
 namespace {
 
@@ -51,8 +48,97 @@ public:
         missing_zone_(false),
         rrclass_(RRClass::IN())
     {}
-    virtual FindResult findZone(const Name&) const {
-        isc_throw(isc::NotImplemented, "Method not used in tests");
+    class Finder : public ZoneFinder {
+    public:
+        Finder(const Name& origin) :
+            origin_(origin)
+        {}
+        Name getOrigin() const {
+            return (origin_);
+        }
+        RRClass getClass() const {
+            return (RRClass::IN());
+        }
+        // The rest is not to be called, so they throw.
+        shared_ptr<Context> find(const Name&, const RRType&,
+                                 const FindOptions)
+        {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        shared_ptr<Context> findAll(const Name&,
+                                    vector<ConstRRsetPtr>&,
+                                    const FindOptions)
+        {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+        FindNSEC3Result findNSEC3(const Name&, bool) {
+            isc_throw(isc::NotImplemented, "Not implemented");
+        }
+    private:
+        Name origin_;
+    };
+    class Iterator : public ZoneIterator {
+    public:
+        Iterator(const Name& origin) :
+            origin_(origin),
+            soa_(new RRset(origin_, RRClass::IN(), RRType::SOA(),
+                           RRTTL(3600)))
+        {
+            // The RData here is bogus, but it is not used to anything. There
+            // just needs to be some.
+            soa_->addRdata(rdata::generic::SOA(Name::ROOT_NAME(),
+                                               Name::ROOT_NAME(),
+                                               0, 0, 0, 0, 0));
+            rrsets_.push_back(soa_);
+
+            // There is no NS record on purpose here.
+
+            // Dummy A rrset. This is used for checking zone data after
+            // reload.
+            RRsetPtr rrset(new RRset(Name("tstzonedata").concatenate(origin_),
+                                     RRClass::IN(), RRType::A(),
+                                     RRTTL(3600)));
+            rrset->addRdata(rdata::in::A("192.0.2.1"));
+            rrsets_.push_back(rrset);
+
+            rrsets_.push_back(ConstRRsetPtr());
+
+            it_ = rrsets_.begin();
+        }
+        virtual isc::dns::ConstRRsetPtr getNextRRset() {
+            ConstRRsetPtr result = *it_;
+            ++it_;
+            return (result);
+        }
+        virtual isc::dns::ConstRRsetPtr getSOA() const {
+            return (soa_);
+        }
+    private:
+        const Name origin_;
+        const RRsetPtr soa_;
+        std::vector<ConstRRsetPtr> rrsets_;
+        std::vector<ConstRRsetPtr>::const_iterator it_;
+    };
+    virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
+                                        bool) const
+    {
+        if (name != Name("example.org")) {
+            isc_throw(DataSourceError, "No such zone");
+        }
+        return (ZoneIteratorPtr(new Iterator(Name("example.org"))));
+    }
+    virtual FindResult findZone(const Name& name) const {
+        const Name origin("example.org");
+        const ZoneFinderPtr finder(new Finder(origin));
+        NameComparisonResult compar(origin.compare(name));
+        switch (compar.getRelation()) {
+            case NameComparisonResult::EQUAL:
+                return (FindResult(result::SUCCESS, finder));
+            case NameComparisonResult::SUPERDOMAIN:
+                return (FindResult(result::PARTIALMATCH, finder));
+            default:
+                return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
+        }
     };
     virtual std::pair<ZoneJournalReader::Result, ZoneJournalReaderPtr>
         getJournalReader(const Name&, uint32_t, uint32_t) const
@@ -214,13 +300,6 @@ protected:
         source_client_(ztable_segment_, rrclass_)
     {}
     void prepareSource(const Name& zone, const char* filename) {
-        // TODO:
-        // Currently, source_client_ is of InMemoryClient and its load()
-        // uses a different code than the ZoneLoader (so we can cross-check
-        // the implementations). Currently, the load() doesn't perform any
-        // post-load checks. It will change in #2499, at which point the
-        // loading may start failing depending on details of the test data. We
-        // should prepare the data by some different method then.
         source_client_.load(zone, string(TEST_DATA_DIR) + "/" + filename);
     }
 private:
@@ -507,17 +586,6 @@ TEST_F(ZoneLoaderTest, loadCheck) {
     EXPECT_FALSE(destination_client_.commit_called_);
 }
 
-// The same test, but for copying from other data source
-TEST_F(ZoneLoaderTest, copyCheck) {
-    prepareSource(Name("example.org"), "novalidate.zone");
-    ZoneLoader loader(destination_client_, Name("example.org"),
-                      source_client_);
-
-    EXPECT_THROW(loader.loadIncremental(10), ZoneContentError);
-    // The messages go to the log. We don't have an easy way to examine them.
-    EXPECT_FALSE(destination_client_.commit_called_);
-}
-
 // Check a warning doesn't disrupt the loading of the zone
 TEST_F(ZoneLoaderTest, loadCheckWarn) {
     ZoneLoader loader(destination_client_, Name("example.org"),
@@ -541,4 +609,18 @@ TEST_F(ZoneLoaderTest, copyCheckWarn) {
 
 }
 
+// Test there's validation of the data in the zone loader when copying
+// from another data source.
+TEST_F(ZoneLoaderTest, copyCheck) {
+    // In this test, my_source_client provides a zone that does not
+    // validate (no NS).
+    MockClient my_source_client;
+    ZoneLoader loader(destination_client_, Name("example.org"),
+                      my_source_client);
+
+    EXPECT_THROW(loader.loadIncremental(10), ZoneContentError);
+    // The messages go to the log. We don't have an easy way to examine them.
+    EXPECT_FALSE(destination_client_.commit_called_);
+}
+
 }



More information about the bind10-changes mailing list