[2420] supported the case for adding RRSIG-only

except for RRSIG for NSEC3, which is a bit trickier and will be handled
separately.

src/lib/datasrc/memory/zone_data_updater.cc

@@ -12,12 +12,16 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <exceptions/exceptions.h>
+
 #include <datasrc/memory/zone_data_updater.h>
 #include <datasrc/memory/logger.h>
 #include <datasrc/zone.h>
 
 #include <dns/rdataclass.h>
 
+#include <cassert>
+
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 
@@ -99,9 +103,7 @@ ZoneDataUpdater::contextCheck(const AbstractRRset& rrset,
 
 void
 ZoneDataUpdater::validate(const isc::dns::ConstRRsetPtr rrset) const {
-    if (!rrset) {
-        isc_throw(NullRRset, "The rrset provided is NULL");
-    }
+    assert(rrset);
 
     if (rrset->getRdataCount() == 0) {
         isc_throw(AddError,
@@ -258,14 +260,15 @@ ZoneDataUpdater::addNSEC3(const ConstRRsetPtr rrset, const ConstRRsetPtr rrsig)
 }
 
 void
-ZoneDataUpdater::addRdataSet(const ConstRRsetPtr rrset,
+ZoneDataUpdater::addRdataSet(const Name& name, const RRType& rrtype,
+                             const ConstRRsetPtr rrset,
                              const ConstRRsetPtr rrsig)
 {
-    if (rrset->getType() == RRType::NSEC3()) {
-        addNSEC3(rrset, rrsig);
+    if (rrtype == RRType::NSEC3()) {
+        addNSEC3(rrset, rrsig); // TBD: check RRSIG only case
     } else {
         ZoneNode* node;
-        zone_data_.insertName(mem_sgmt_, rrset->getName(), &node);
+        zone_data_.insertName(mem_sgmt_, name, &node);
 
         RdataSet* rdataset_head = node->getData();
 
@@ -273,13 +276,14 @@ ZoneDataUpdater::addRdataSet(const ConstRRsetPtr rrset,
         // fails and the exception is thrown, it may break strong
         // exception guarantee.  At the moment we prefer code simplicity
         // and don't bother to introduce complicated recovery code.
-        contextCheck(*rrset, rdataset_head);
+        if (rrset) { // this check is only for covered RRset, not RRSIG
+            contextCheck(*rrset, rdataset_head);
+        }
 
-        if (RdataSet::find(rdataset_head, rrset->getType()) != NULL) {
+        if (RdataSet::find(rdataset_head, rrtype) != NULL) {
             isc_throw(AddError,
                       "RRset of the type already exists: "
-                      << rrset->getName() << " (type: "
-                      << rrset->getType() << ")");
+                      << name << " (type: " << rrtype << ")");
         }
 
         RdataSet* rdataset_new = RdataSet::create(mem_sgmt_, encoder_,
@@ -290,22 +294,21 @@ ZoneDataUpdater::addRdataSet(const ConstRRsetPtr rrset,
         // Ok, we just put it in.
 
         // If this RRset creates a zone cut at this node, mark the node
-        // indicating the need for callback in find().
-        if (rrset->getType() == RRType::NS() &&
-            rrset->getName() != zone_name_) {
+        // indicating the need for callback in find().  Note that we do this
+        // only when non RRSIG RRset of that type is added.
+        if (rrset && rrtype == RRType::NS() && name != zone_name_) {
             node->setFlag(ZoneNode::FLAG_CALLBACK);
             // If it is DNAME, we have a callback as well here
-        } else if (rrset->getType() == RRType::DNAME()) {
+        } else if (rrset && rrtype == RRType::DNAME()) {
             node->setFlag(ZoneNode::FLAG_CALLBACK);
         }
 
         // If we've added NSEC3PARAM at zone origin, set up NSEC3
         // specific data or check consistency with already set up
         // parameters.
-        if (rrset->getType() == RRType::NSEC3PARAM() &&
-            rrset->getName() == zone_name_) {
+        if (rrset && rrtype == RRType::NSEC3PARAM() && name == zone_name_) {
             setupNSEC3<generic::NSEC3PARAM>(rrset);
-        } else if (rrset->getType() == RRType::NSEC()) {
+        } else if (rrset && rrtype == RRType::NSEC()) {
             // If it is NSEC signed zone, we mark the zone as signed
             // (conceptually "signed" is a broader notion but our
             // current zone finder implementation regards "signed" as
@@ -315,31 +318,57 @@ ZoneDataUpdater::addRdataSet(const ConstRRsetPtr rrset,
     }
 }
 
+namespace {
+RRType
+getCoveredType(const ConstRRsetPtr& sig_rrset) {
+    RdataIteratorPtr it = sig_rrset->getRdataIterator();
+    // Empty RRSIG shouldn't be passed either via a master file or
+    // another data source iterator, but it could still happen if the
+    // iterator has a bug.  We catch and reject such cases.
+    if (it->isLast()) {
+        isc_throw(isc::Unexpected,
+                  "Empty RRset is passed in-memory loader, name: "
+                  << sig_rrset->getName());
+    }
+    return (dynamic_cast<const generic::RRSIG&>(it->getCurrent()).
+            typeCovered());
+}
+}
+
 void
 ZoneDataUpdater::add(const ConstRRsetPtr& rrset,
                      const ConstRRsetPtr& sig_rrset)
 {
-    // Validate input.  This will cause an exception to be thrown if the
-    // input RRset is empty.
-    validate(rrset);
+    // Validate input.
+    if (!rrset && !sig_rrset) {
+        isc_throw(NullRRset,
+                  "ZoneDataUpdater::add is given 2 NULL pointers");
+    }
+    if (rrset) {
+        validate(rrset);
+    }
     if (sig_rrset) {
         validate(sig_rrset);
     }
 
+    const Name& name = rrset ? rrset->getName() : sig_rrset->getName();
+    const RRType& rrtype = rrset ? rrset->getType() :
+        getCoveredType(sig_rrset);
+
     // OK, can add the RRset.
     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEMORY_MEM_ADD_RRSET).
-        arg(rrset->getName()).arg(rrset->getType()).arg(zone_name_);
+        arg(name).arg(rrtype).arg(zone_name_);
 
     // Add wildcards possibly contained in the owner name to the domain
     // tree.  This can only happen for the normal (non-NSEC3) tree.
     // Note: this can throw an exception, breaking strong exception
     // guarantee.  (see also the note for the call to contextCheck()
     // above).
-    if (rrset->getType() != RRType::NSEC3()) {
-        addWildcards(rrset->getName());
+    if (rrtype != RRType::NSEC3()) {
+        addWildcards(name);
     }
 
-    addRdataSet(rrset, sig_rrset);
+    addRdataSet(name, rrtype, rrset, sig_rrset);
 }
 
 } // namespace memory

src/lib/datasrc/memory/zone_data_updater.h

@@ -114,10 +114,16 @@ public:
     /// populated with the record data and added to the ZoneData for the
     /// name in the RRset.
     ///
-    /// This method throws an \c NullRRset exception (see above) if
-    /// \c rrset is empty. It throws \c AddError if any of a variety of
-    /// validation checks fail for the \c rrset and its associated
-    /// \c sig_rrset.
+    /// At least one of \c rrset or \c sig_rrset must be non NULL.
+    /// \c sig_rrset can be reasonably NULL when \c rrset is not signed in
+    /// the zone; it's unusual that \c rrset is NULL, but is still possible
+    /// if these RRsets are given separately to the loader, or if even the
+    /// zone is half broken and really contains an RRSIG that doesn't have
+    /// any covered RRset.  This implementation supports these cases.
+    ///
+    /// \throw NullRRset Both \c rrset and sig_rrset is NULL
+    /// \throw AddError any of a variety of validation checks fail for the
+    /// \c rrset and its associated \c sig_rrset.
     ///
     /// \param rrset The RRset to be added.
     /// \param sig_rrset An associated RRSIG RRset for the \c rrset. It
@@ -158,7 +164,9 @@ private:
     void setupNSEC3(const isc::dns::ConstRRsetPtr rrset);
     void addNSEC3(const isc::dns::ConstRRsetPtr rrset,
                   const isc::dns::ConstRRsetPtr rrsig);
-    void addRdataSet(const isc::dns::ConstRRsetPtr rrset,
+    void addRdataSet(const isc::dns::Name& name,
+                     const isc::dns::RRType& rrtype,
+                     const isc::dns::ConstRRsetPtr rrset,
                      const isc::dns::ConstRRsetPtr rrsig);
 
     util::MemorySegment& mem_sgmt_;

src/lib/datasrc/tests/memory/Makefile.am

@@ -33,6 +33,7 @@ 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 += zone_data_loader_unittest.cc
+run_unittests_SOURCES += zone_data_updater_unittest.cc
 run_unittests_SOURCES += zone_table_segment_test.h
 run_unittests_SOURCES += zone_table_segment_unittest.cc
 run_unittests_SOURCES += zone_writer_unittest.cc

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

@@ -0,0 +1,139 @@
+// 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.
+
+#include <testutils/dnsmessage_test.h>
+
+#include <exceptions/exceptions.h>
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
+#include <dns/rrset.h>
+
+#include <datasrc/memory/rdataset.h>
+#include <datasrc/memory/zone_data.h>
+#include <datasrc/memory/zone_data_updater.h>
+
+#include "memory_segment_test.h"
+
+#include <gtest/gtest.h>
+
+using isc::testutils::textToRRset;
+using namespace isc::dns;
+using namespace isc::datasrc::memory;
+
+namespace {
+
+class ZoneDataUpdaterTest : public ::testing::Test {
+protected:
+    ZoneDataUpdaterTest() :
+        zname_("example.org"), zclass_(RRClass::IN()),
+        zone_data_(ZoneData::create(mem_sgmt_, zname_)),
+        updater_(mem_sgmt_, zclass_, zname_, *zone_data_)
+    {}
+    ~ZoneDataUpdaterTest() {
+        // Make sure zone data is destroyed even if a test results in exception
+        if (zone_data_ != NULL) {
+            ZoneData::destroy(mem_sgmt_, zone_data_, zclass_);
+        }
+    }
+
+    void TearDown() {
+        if (zone_data_ != NULL) {
+            ZoneData::destroy(mem_sgmt_, zone_data_, zclass_);
+            zone_data_ = NULL;
+        }
+        EXPECT_TRUE(mem_sgmt_.allMemoryDeallocated()); // catch any leak here.
+    }
+    const Name zname_;
+    const RRClass zclass_;
+    test::MemorySegmentTest mem_sgmt_;
+    ZoneData* zone_data_;
+    ZoneDataUpdater updater_;
+};
+
+TEST_F(ZoneDataUpdaterTest, bothNull) {
+    // At least either covered RRset or RRSIG must be non NULL.
+    EXPECT_THROW(updater_.add(ConstRRsetPtr(), ConstRRsetPtr()),
+                 ZoneDataUpdater::NullRRset);
+}
+
+ZoneNode*
+getNode(isc::util::MemorySegment& mem_sgmt, const Name& name,
+        ZoneData* zone_data)
+{
+    ZoneNode* node = NULL;
+    zone_data->insertName(mem_sgmt, name, &node);
+    EXPECT_NE(static_cast<ZoneNode*>(NULL), node);
+    return (node);
+}
+
+TEST_F(ZoneDataUpdaterTest, rrisgOnly) {
+    // RRSIG that doesn't have covered RRset can be added.  The resulting
+    // rdataset won't have "normal" RDATA but sig RDATA.
+    updater_.add(ConstRRsetPtr(), textToRRset(
+                     "www.example.org. 3600 IN RRSIG A 5 3 3600 20150420235959"
+                     " 20051021000000 1 example.org. FAKE"));
+    ZoneNode* node = getNode(mem_sgmt_, Name("www.example.org"), zone_data_);
+    const RdataSet* rdset = node->getData();
+    ASSERT_NE(static_cast<RdataSet*>(NULL), rdset);
+    rdset = RdataSet::find(rdset, RRType::A());
+    ASSERT_NE(static_cast<RdataSet*>(NULL), rdset);
+    EXPECT_EQ(0, rdset->getRdataCount());
+    EXPECT_EQ(1, rdset->getSigRdataCount());
+
+    // The RRSIG covering A prohibits an actual A RRset from being added.
+    // This should be loosened in future version, but we check the current
+    // behavior.
+    EXPECT_THROW(updater_.add(
+                     textToRRset("www.example.org. 3600 IN A 192.0.2.1"),
+                     ConstRRsetPtr()), ZoneDataUpdater::AddError);
+
+    // The special "wildcarding" node mark should be added for the RRSIG-only
+    // case, too.
+    updater_.add(ConstRRsetPtr(), textToRRset(
+                     "*.wild.example.org. 3600 IN RRSIG A 5 3 3600 "
+                     "20150420235959 20051021000000 1 example.org. FAKE"));
+    node = getNode(mem_sgmt_, Name("wild.example.org"), zone_data_);
+    EXPECT_TRUE(node->getFlag(ZoneData::WILDCARD_NODE));
+
+    // Simply adding RRSIG covering (delegating NS) shouldn't enable callback
+    // in search.
+    updater_.add(ConstRRsetPtr(), textToRRset(
+                     "child.example.org. 3600 IN RRSIG NS 5 3 3600 "
+                     "20150420235959 20051021000000 1 example.org. FAKE"));
+    node = getNode(mem_sgmt_, Name("child.example.org"), zone_data_);
+    EXPECT_FALSE(node->getFlag(ZoneNode::FLAG_CALLBACK));
+
+    // Same for DNAME
+    updater_.add(ConstRRsetPtr(), textToRRset(
+                     "dname.example.org. 3600 IN RRSIG DNAME 5 3 3600 "
+                     "20150420235959 20051021000000 1 example.org. FAKE"));
+    node = getNode(mem_sgmt_, Name("dname.example.org"), zone_data_);
+    EXPECT_FALSE(node->getFlag(ZoneNode::FLAG_CALLBACK));
+
+    // Likewise, RRSIG for NSEC3PARAM alone shouldn't make the zone
+    // "NSEC3-signed".
+    updater_.add(ConstRRsetPtr(), textToRRset(
+                     "example.org. 3600 IN RRSIG NSEC3PARAM 5 3 3600 "
+                     "20150420235959 20051021000000 1 example.org. FAKE"));
+    EXPECT_FALSE(zone_data_->isNSEC3Signed());
+
+    // And same for (RRSIG for) NSEC and "is signed".
+    updater_.add(ConstRRsetPtr(), textToRRset(
+                     "example.org. 3600 IN RRSIG NSEC 5 3 3600 "
+                     "20150420235959 20051021000000 1 example.org. FAKE"));
+    EXPECT_FALSE(zone_data_->isSigned());
+}
+
+}