BIND 10 master, updated. af808fad3a5d1de608b799a0e7491ac8eacac8ff [master] Merge branch 'trac2267'

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Sep 20 16:28:53 UTC 2012


The branch, master has been updated
       via  af808fad3a5d1de608b799a0e7491ac8eacac8ff (commit)
       via  d2a8a10ffc18ab3c57e4008d72fe505b62c25432 (commit)
       via  b117859663685e898c085750bc7a0908dc42e489 (commit)
       via  5784101c08e8318a6e88bea598cf2a7ac5a389c3 (commit)
       via  fee99a5bb5b064ee62866f93b75b22c99905dc05 (commit)
       via  9eeacfa616c813189a516f343c320d65ed9bfc7d (commit)
       via  9d974a35bb76a63c7d8749d5728612ab0ef83c98 (commit)
       via  71bfb14495829b1e03c79a03e6a1cd44c4d3a1b9 (commit)
       via  f0d4fcf168a840786ff9172ae23402920c85f3de (commit)
       via  dfc2f2b4dd181378d493aad1c53c8fcc53ba8152 (commit)
       via  5a493720575e76a80ed6bba5cfcd2fac01364ce2 (commit)
       via  869d1d20492f5221714d2fbf7c39bef800a28e60 (commit)
       via  a65bda660f985ff422b1528b1f7654ac254c6a79 (commit)
       via  5ffdbdff90641201be36ab0ee65773df2ef6e0dd (commit)
       via  b90b048ba805efc450026c5192bb09aa7668d52f (commit)
       via  eb64f1c38d521886fef599582b4c80e892e31341 (commit)
       via  6f4b4413fad5433ff41cd2b51f4784cbedfa859b (commit)
       via  55f047a3e275e54e45a027631d1bd7e900b661f7 (commit)
       via  7e5dbd9146f8c9a27937b733a63116d25865cf45 (commit)
       via  4a78f86fffe147322f65b581a89d834b50999f39 (commit)
       via  ac96bd4de08738f779e79cc9cd15c88a63fb32bf (commit)
       via  91a32d4c23fdb0780a78cae0fe34cec2ef64cffb (commit)
      from  8bc72c0e77a786901f61f3fab7aa0074aa1c5b70 (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 af808fad3a5d1de608b799a0e7491ac8eacac8ff
Merge: 8bc72c0 d2a8a10
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Sep 20 09:20:59 2012 -0700

    [master] Merge branch 'trac2267'

commit d2a8a10ffc18ab3c57e4008d72fe505b62c25432
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Thu Sep 20 09:19:36 2012 -0700

    [2267] cleanup: removed a redundant 'return'

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

Summary of changes:
 src/lib/datasrc/memory/memory_client.cc            |  363 ++++++++++----------
 src/lib/datasrc/memory/memory_client.h             |    4 +
 .../datasrc/memory/tests/memory_client_unittest.cc |  121 ++++---
 src/lib/datasrc/memory/tests/testdata/Makefile.am  |    2 -
 .../testdata/example.org-rrsig-name-unmatched.zone |    6 -
 .../testdata/example.org-rrsig-type-unmatched.zone |    6 -
 6 files changed, 255 insertions(+), 247 deletions(-)
 delete mode 100644 src/lib/datasrc/memory/tests/testdata/example.org-rrsig-name-unmatched.zone
 delete mode 100644 src/lib/datasrc/memory/tests/testdata/example.org-rrsig-type-unmatched.zone

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/memory_client.cc b/src/lib/datasrc/memory/memory_client.cc
index d70f345..66822d2 100644
--- a/src/lib/datasrc/memory/memory_client.cc
+++ b/src/lib/datasrc/memory/memory_client.cc
@@ -41,6 +41,7 @@
 #include <boost/scoped_ptr.hpp>
 #include <boost/bind.hpp>
 #include <boost/foreach.hpp>
+#include <boost/noncopyable.hpp>
 
 #include <algorithm>
 #include <map>
@@ -109,7 +110,6 @@ public:
     unsigned int zone_count_;
     ZoneTable* zone_table_;
     FileNameTree* file_name_tree_;
-    ConstRRsetPtr last_rrset_;
 
     // Common process for zone load.
     // rrset_installer is a functor that takes another functor as an argument,
@@ -171,7 +171,8 @@ public:
      * If such condition is found, it throws AddError.
      */
     void contextCheck(const Name& zone_name, const AbstractRRset& rrset,
-                      const RdataSet* set) const {
+                      const RdataSet* set) const
+    {
         // Ensure CNAME and other type of RR don't coexist for the same
         // owner name except with NSEC, which is the only RR that can coexist
         // with CNAME (and also RRSIG, which is handled separately)
@@ -247,7 +248,23 @@ public:
                       << rrset->getName() << " which isn't supported");
         }
 
-        NameComparisonResult compare(zone_name.compare(rrset->getName()));
+        // For RRSIGs, check consistency of the type covered.
+        // We know the RRset isn't empty, so the following check is safe.
+        if (rrset->getType() == RRType::RRSIG()) {
+            RdataIteratorPtr rit = rrset->getRdataIterator();
+            const RRType covered = dynamic_cast<const generic::RRSIG&>(
+                rit->getCurrent()).typeCovered();
+            for (rit->next(); !rit->isLast(); rit->next()) {
+                if (dynamic_cast<const generic::RRSIG&>(
+                        rit->getCurrent()).typeCovered() != covered) {
+                    isc_throw(AddError, "RRSIG contains mixed covered types: "
+                              << rrset->toText());
+                }
+            }
+        }
+
+        const NameComparisonResult compare =
+            zone_name.compare(rrset->getName());
         if (compare.getRelation() != NameComparisonResult::SUPERDOMAIN &&
             compare.getRelation() != NameComparisonResult::EQUAL)
         {
@@ -262,10 +279,9 @@ public:
         // Even though the protocol specifically doesn't completely ban such
         // usage, we refuse to load a zone containing such RR in order to
         // keep the lookup logic simpler and more predictable.
-        // See RFC4592 and (for DNAME) draft-ietf-dnsext-rfc2672bis-dname
-        // for more technical background.  Note also that BIND 9 refuses
-        // NS at a wildcard, so in that sense we simply provide compatible
-        // behavior.
+        // See RFC4592 and (for DNAME) RFC6672 for more technical background.
+        // Note also that BIND 9 refuses NS at a wildcard, so in that sense
+        // we simply provide compatible behavior.
         if (rrset->getName().isWildcard()) {
             if (rrset->getType() == RRType::NS()) {
                 LOG_ERROR(logger, DATASRC_MEMORY_MEM_WILDCARD_NS).
@@ -299,7 +315,8 @@ public:
 
     void addNSEC3(const ConstRRsetPtr rrset,
                   const ConstRRsetPtr rrsig,
-                  ZoneData& zone_data) {
+                  ZoneData& zone_data)
+    {
         // We know rrset has exactly one RDATA
         const generic::NSEC3& nsec3_rdata =
             dynamic_cast<const generic::NSEC3&>(
@@ -343,96 +360,57 @@ public:
     }
 
     void addRdataSet(const Name& zone_name, ZoneData& zone_data,
-                     const ConstRRsetPtr rrset, const ConstRRsetPtr rrsig) {
-        // Only one of these can be passed at a time.
-        assert(!(rrset && rrsig));
-
-        // If rrsig is passed, validate it against the last-saved rrset.
-        if (rrsig) {
-            // The covered RRset should have been saved by now.
-            if (!last_rrset_) {
-                isc_throw(AddError,
-                          "RRSIG is being added, "
-                          "but doesn't follow its covered RR: "
-                          << rrsig->getName());
-            }
-
-            if (rrsig->getName() != last_rrset_->getName()) {
-                isc_throw(AddError,
-                          "RRSIG is being added, "
-                          "but doesn't match the last RR's name: "
-                          << last_rrset_->getName() << " vs. "
-                          << rrsig->getName());
-            }
-
-            // Consistency of other types in rrsig are checked in addRRsig().
-            RdataIteratorPtr rit = rrsig->getRdataIterator();
-            const RRType covered = dynamic_cast<const generic::RRSIG&>(
-                rit->getCurrent()).typeCovered();
-
-            if (covered != last_rrset_->getType()) {
-                isc_throw(AddError,
-                          "RRSIG is being added, "
-                          "but doesn't match the last RR's type: "
-                          << last_rrset_->getType() << " vs. "
-                          << covered);
-            }
-        }
-
-        if (!last_rrset_) {
-            last_rrset_ = rrset;
-            return;
-        }
-
-        if (last_rrset_->getType() == RRType::NSEC3()) {
-            addNSEC3(last_rrset_, rrsig, zone_data);
+                     const ConstRRsetPtr rrset, const ConstRRsetPtr rrsig)
+    {
+        if (rrset->getType() == RRType::NSEC3()) {
+            addNSEC3(rrset, rrsig, zone_data);
         } else {
             ZoneNode* node;
-            zone_data.insertName(mem_sgmt_, last_rrset_->getName(), &node);
+            zone_data.insertName(mem_sgmt_, rrset->getName(), &node);
 
-            RdataSet* set = node->getData();
+            RdataSet* rdataset_head = node->getData();
 
             // Checks related to the surrounding data.
             // Note: when the check 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(zone_name, *last_rrset_, set);
+            contextCheck(zone_name, *rrset, rdataset_head);
 
-            if (RdataSet::find(set, last_rrset_->getType()) != NULL) {
+            if (RdataSet::find(rdataset_head, rrset->getType()) != NULL) {
                 isc_throw(AddError,
                           "RRset of the type already exists: "
-                          << last_rrset_->getName() << " (type: "
-                          << last_rrset_->getType() << ")");
+                          << rrset->getName() << " (type: "
+                          << rrset->getType() << ")");
             }
 
             RdataEncoder encoder;
-            RdataSet *new_set = RdataSet::create(mem_sgmt_, encoder,
-                                                 last_rrset_, rrsig);
-            new_set->next = set;
-            node->setData(new_set);
+            RdataSet* rdataset = RdataSet::create(mem_sgmt_, encoder, rrset,
+                                                  rrsig);
+            rdataset->next = rdataset_head;
+            node->setData(rdataset);
 
             // 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 (last_rrset_->getType() == RRType::NS() &&
-                last_rrset_->getName() != zone_name) {
+            if (rrset->getType() == RRType::NS() &&
+                rrset->getName() != zone_name) {
                 node->setFlag(ZoneNode::FLAG_CALLBACK);
                 // If it is DNAME, we have a callback as well here
-            } else if (last_rrset_->getType() == RRType::DNAME()) {
+            } else if (rrset->getType() == 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 (last_rrset_->getType() == RRType::NSEC3PARAM() &&
-                last_rrset_->getName() == zone_name) {
+            if (rrset->getType() == RRType::NSEC3PARAM() &&
+                rrset->getName() == zone_name) {
                 // We know rrset has exactly one RDATA
                 const generic::NSEC3PARAM& param =
                     dynamic_cast<const generic::NSEC3PARAM&>
-                      (last_rrset_->getRdataIterator()->getCurrent());
+                      (rrset->getRdataIterator()->getCurrent());
 
                 NSEC3Data* nsec3_data = zone_data.getNSEC3Data();
                 if (nsec3_data == NULL) {
@@ -450,91 +428,149 @@ public:
                                      salt_data, salt_len) != 0)) {
                         isc_throw(AddError,
                                   "NSEC3PARAM with inconsistent parameters: "
-                                  << last_rrset_->toText());
+                                  << rrset->toText());
                     }
                 }
-            } else if (last_rrset_->getType() == RRType::NSEC()) {
-                // If it is NSEC signed zone, so we put a flag there
-                // (flag is enough)
+            } else if (rrset->getType() == 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 "NSEC
+                // signed")
                 zone_data.setSigned(true);
             }
         }
-
-        last_rrset_ = rrset;
-    }
-
-    result::Result addRRsig(const ConstRRsetPtr sig_rrset,
-                            const Name& zone_name, ZoneData& zone_data)
-    {
-        // Check consistency of the type covered.
-        // We know the RRset isn't empty, so the following check is safe.
-        RdataIteratorPtr rit = sig_rrset->getRdataIterator();
-        const RRType covered = dynamic_cast<const generic::RRSIG&>(
-            rit->getCurrent()).typeCovered();
-        for (rit->next(); !rit->isLast(); rit->next()) {
-            if (dynamic_cast<const generic::RRSIG&>(
-                    rit->getCurrent()).typeCovered() != covered) {
-                isc_throw(AddError, "RRSIG contains mixed covered types: "
-                          << sig_rrset->toText());
-            }
-        }
-
-        addRdataSet(zone_name, zone_data, ConstRRsetPtr(), sig_rrset);
-        return (result::SUCCESS);
     }
 
-    /*
-     * Implementation of longer methods. We put them here, because the
-     * access is without the impl_-> and it will get inlined anyway.
-     */
-
     // Implementation of InMemoryClient::add()
-    result::Result add(const ConstRRsetPtr& rrset,
-                       const Name& zone_name, ZoneData& zone_data)
+    void add(const ConstRRsetPtr& rrset, const ConstRRsetPtr& sig_rrset,
+             const Name& zone_name, ZoneData& zone_data)
     {
         // Sanitize input.  This will cause an exception to be thrown
         // if the input RRset is empty.
         addValidation(zone_name, rrset);
+        if (sig_rrset) {
+            addValidation(zone_name, 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);
 
-        if (rrset->getType() == RRType::NSEC3()) {
-            addRdataSet(zone_name, zone_data, rrset, ConstRRsetPtr());
-            return (result::SUCCESS);
+        // 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(zone_name, zone_data, rrset->getName());
         }
 
-        // RRSIGs are special in various points, so we handle it in a
-        // separate dedicated method.
-        if (rrset->getType() == RRType::RRSIG()) {
-            return (addRRsig(rrset, zone_name, zone_data));
+        addRdataSet(zone_name, zone_data, rrset, sig_rrset);
+    }
+};
+
+// A helper internal class for load().  make it non-copyable to avoid
+// accidental copy.
+//
+// The current internal implementation expects that both a normal
+// (non RRSIG) RRset and (when signed) its RRSIG are added at once.
+// Also in the current implementation, the input sequence of RRsets
+// are grouped with their owner name (so once a new owner name is encountered,
+// no subsequent RRset has the previous owner name), but the ordering
+// in the same group is not fixed.  So we hold all RRsets of the same
+// owner name in node_rrsets_ and node_rrsigsets_, and add the matching
+// pairs of RRsets to the zone when we see a new owner name.
+//
+// The caller is responsible for adding the RRsets of the last group
+// in the input sequence by explicitly calling flushNodeRRsets() at the
+// end.  It's cleaner and more robust if we let the destructor of this class
+// do it, but since we cannot guarantee the adding operation is exception free,
+// we don't choose that option to maintain the common expectation for
+// destructors.
+class InMemoryClient::Loader : boost::noncopyable {
+    typedef std::map<RRType, ConstRRsetPtr> NodeRRsets;
+    typedef NodeRRsets::value_type NodeRRsetsVal;
+public:
+    Loader(InMemoryClientImpl* client_impl, const Name& zone_name,
+           ZoneData& zone_data) :
+        client_impl_(client_impl), zone_name_(zone_name), zone_data_(zone_data)
+    {}
+    void addFromLoad(const ConstRRsetPtr& rrset) {
+        // If we see a new name, flush the temporary holders, adding the
+        // pairs of RRsets and RRSIGs of the previous name to the zone.
+        if ((!node_rrsets_.empty() || !node_rrsigsets_.empty()) &&
+            getCurrentName() != rrset->getName()) {
+            flushNodeRRsets();
         }
 
-        // Add wildcards possibly contained in the owner name to the domain
-        // tree.
-        // Note: this can throw an exception, breaking strong exception
-        // guarantee.  (see also the note for contextCheck() below).
-        addWildcards(zone_name, zone_data, rrset->getName());
+        // Store this RRset until it can be added to the zone.  The current
+        // implementation requires RRs of the same RRset should be added at
+        // once, so we check the "duplicate" here.
+        const bool is_rrsig = rrset->getType() == RRType::RRSIG();
+        NodeRRsets& node_rrsets = is_rrsig ? node_rrsigsets_ : node_rrsets_;
+        const RRType& rrtype = is_rrsig ?
+            getCoveredType(rrset) : rrset->getType();
+        if (!node_rrsets.insert(NodeRRsetsVal(rrtype, rrset)).second) {
+            isc_throw(AddError,
+                      "Duplicate add of the same type of"
+                      << (is_rrsig ? " RRSIG" : "") << " RRset: "
+                      << rrset->getName() << "/" << rrtype);
+        }
+    }
+    void flushNodeRRsets() {
+        BOOST_FOREACH(NodeRRsetsVal val, node_rrsets_) {
+            // Identify the corresponding RRSIG for the RRset, if any.
+            // If found add both the RRset and its RRSIG at once.
+            ConstRRsetPtr sig_rrset;
+            NodeRRsets::iterator sig_it =
+                node_rrsigsets_.find(val.first);
+            if (sig_it != node_rrsigsets_.end()) {
+                sig_rrset = sig_it->second;
+                node_rrsigsets_.erase(sig_it);
+            }
+            client_impl_->add(val.second, sig_rrset, zone_name_, zone_data_);
+        }
 
-        addRdataSet(zone_name, zone_data, rrset, ConstRRsetPtr());
+        // Right now, we don't accept RRSIG without covered RRsets (this
+        // should eventually allowed, but to do so we'll need to update the
+        // finder).
+        if (!node_rrsigsets_.empty()) {
+            isc_throw(AddError, "RRSIG is added without covered RRset for "
+                      << getCurrentName());
+        }
 
-        return (result::SUCCESS);
+        node_rrsets_.clear();
+        node_rrsigsets_.clear();
     }
-
-    /*
-     * Wrapper around above.
-     */
-    void addFromLoad(const ConstRRsetPtr& set,
-                     const Name& zone_name, ZoneData* zone_data)
-    {
-        switch (add(set, zone_name, *zone_data)) {
-        case result::SUCCESS:
-            return;
-        default:
-            assert(0);
+private:
+    // A helper to identify the covered type of an RRSIG.
+    static 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());
     }
+    const Name& getCurrentName() const {
+        if (!node_rrsets_.empty()) {
+            return (node_rrsets_.begin()->second->getName());
+        }
+        assert(!node_rrsigsets_.empty());
+        return (node_rrsigsets_.begin()->second->getName());
+    }
+
+private:
+    InMemoryClientImpl* client_impl_;
+    const Name& zone_name_;
+    ZoneData& zone_data_;
+    NodeRRsets node_rrsets_;
+    NodeRRsets node_rrsigsets_;
 };
 
 result::Result
@@ -544,31 +580,17 @@ InMemoryClient::InMemoryClientImpl::load(
     boost::function<void(LoadCallback)> rrset_installer)
 {
     SegmentObjectHolder<ZoneData, RRClass> holder(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, zone_name),
-        rrclass_);
-
-    assert(!last_rrset_);
-
-    try {
-        rrset_installer(boost::bind(&InMemoryClientImpl::addFromLoad, this,
-                                    _1, zone_name, holder.get()));
-        // Add any last RRset that was left
-        addRdataSet(zone_name, *holder.get(),
-                    ConstRRsetPtr(), ConstRRsetPtr());
-    } catch (...) {
-        last_rrset_ = ConstRRsetPtr();
-        throw;
-    }
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zone_name), rrclass_);
 
-    assert(!last_rrset_);
+    Loader loader(this, zone_name, *holder.get());
+    rrset_installer(boost::bind(&Loader::addFromLoad, &loader, _1));
+    // Add any last RRsets that were left
+    loader.flushNodeRRsets();
 
     const ZoneNode* origin_node = holder.get()->getOriginNode();
     const RdataSet* set = origin_node->getData();
     // If the zone is NSEC3-signed, check if it has NSEC3PARAM
     if (holder.get()->isNSEC3Signed()) {
-        // Note: origin_data_ is set on creation of ZoneData, and the load
-        // process only adds new nodes (and their data), so this assertion
-        // should hold.
         if (RdataSet::find(set, RRType::NSEC3PARAM()) == NULL) {
             LOG_WARN(logger, DATASRC_MEMORY_MEM_NO_NSEC3PARAM).
                 arg(zone_name).arg(rrclass_);
@@ -639,30 +661,8 @@ masterLoadWrapper(const char* const filename, const Name& origin,
 void
 generateRRsetFromIterator(ZoneIterator* iterator, LoadCallback callback) {
     ConstRRsetPtr rrset;
-    vector<ConstRRsetPtr> rrsigs; // placeholder for RRSIGs until "commitable".
-
-    // The current internal implementation assumes an RRSIG is always added
-    // after the RRset they cover.  So we store any RRSIGs in 'rrsigs' until
-    // it's safe to add them; based on our assumption if the owner name
-    // changes, all covered RRsets of the previous name should have been
-    // installed and any pending RRSIGs can be added at that point.  RRSIGs
-    // of the last name from the iterator must be added separately.
     while ((rrset = iterator->getNextRRset()) != NULL) {
-        if (!rrsigs.empty() && rrset->getName() != rrsigs[0]->getName()) {
-            BOOST_FOREACH(ConstRRsetPtr sig_rrset, rrsigs) {
-                callback(sig_rrset);
-            }
-            rrsigs.clear();
-        }
-        if (rrset->getType() == RRType::RRSIG()) {
-            rrsigs.push_back(rrset);
-        } else {
-            callback(rrset);
-        }
-    }
-
-    BOOST_FOREACH(ConstRRsetPtr sig_rrset, rrsigs) {
-        callback(sig_rrset);
+        callback(rrset);
     }
 }
 }
@@ -736,29 +736,20 @@ InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
 
 result::Result
 InMemoryClient::add(const isc::dns::Name& zone_name,
-                    const ConstRRsetPtr& rrset) {
-    assert(!impl_->last_rrset_);
-
-    ZoneTable::FindResult result(impl_->zone_table_->findZone(zone_name));
+                    const ConstRRsetPtr& rrset)
+{
+    const ZoneTable::FindResult result =
+        impl_->zone_table_->findZone(zone_name);
     if (result.code != result::SUCCESS) {
         isc_throw(DataSourceError, "No such zone: " + zone_name.toText());
     }
 
-    result::Result ret(impl_->add(rrset, zone_name, *result.zone_data));
-    // Add any associated RRSIG too. This has to be done here, as both
-    // the RRset and its RRSIG have to be passed when constructing an
-    // RdataSet.
-    if ((ret == result::SUCCESS) && rrset->getRRsig()) {
-        impl_->add(rrset->getRRsig(), zone_name, *result.zone_data);
-    }
-
-    // Add any last RRset that was left
-    impl_->addRdataSet(zone_name, *result.zone_data,
-                       ConstRRsetPtr(), ConstRRsetPtr());
-
-    assert(!impl_->last_rrset_);
+    const ConstRRsetPtr sig_rrset =
+        rrset ? rrset->getRRsig() : ConstRRsetPtr();
+    impl_->add(rrset, sig_rrset, zone_name, *result.zone_data);
 
-    return (ret);
+    // add() doesn't allow duplicate add, so we always return SUCCESS.
+    return (result::SUCCESS);
 }
 
 namespace {
diff --git a/src/lib/datasrc/memory/memory_client.h b/src/lib/datasrc/memory/memory_client.h
index d2ec4c3..4c3ad27 100644
--- a/src/lib/datasrc/memory/memory_client.h
+++ b/src/lib/datasrc/memory/memory_client.h
@@ -244,6 +244,10 @@ private:
     // directly any more (it should be handled through DataSourceClient)?
     class InMemoryClientImpl;
     InMemoryClientImpl* impl_;
+
+    // A helper internal class used by load().  It maintains some intermediate
+    // states while loading RRs of the zone.
+    class Loader;
 };
 
 } // namespace memory
diff --git a/src/lib/datasrc/memory/tests/memory_client_unittest.cc b/src/lib/datasrc/memory/tests/memory_client_unittest.cc
index d5aa431..fcee0be 100644
--- a/src/lib/datasrc/memory/tests/memory_client_unittest.cc
+++ b/src/lib/datasrc/memory/tests/memory_client_unittest.cc
@@ -34,6 +34,8 @@
 
 #include <testutils/dnsmessage_test.h>
 
+#include "memory_segment_test.h"
+
 #include <gtest/gtest.h>
 
 #include <new>                  // for bad_alloc
@@ -45,42 +47,53 @@ using namespace isc::datasrc::memory;
 using namespace isc::testutils;
 
 namespace {
-// Memory segment specified for tests.  It normally behaves like a "local"
-// memory segment.  If "throw count" is set to non 0 via setThrowCount(),
-// it continues the normal behavior up to the specified number of calls to
-// allocate(), and throws an exception at the next call.
-class TestMemorySegment : public isc::util::MemorySegmentLocal {
-public:
-    TestMemorySegment() : throw_count_(0) {}
-    virtual void* allocate(size_t size) {
-        if (throw_count_ > 0) {
-            if (--throw_count_ == 0) {
-                throw std::bad_alloc();
-            }
-        }
-        return (isc::util::MemorySegmentLocal::allocate(size));
-    }
-    void setThrowCount(size_t count) { throw_count_ = count; }
 
-private:
-    size_t throw_count_;
+const char* rrset_data[] = {
+    "example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. "
+    "68 3600 300 3600000 3600",
+    "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 "
+    "40430 example.org. FAKEFAKE",
+    "a.example.org. 3600 IN MX 10 mail.example.org.",
+    "a.example.org. 3600 IN RRSIG MX 5 3 3600 20150420235959 20051021000000 "
+    "40430 example.org. FAKEFAKEFAKE",
+    NULL
 };
 
-const char* rrset_data[] = {
-    "example.org. 3600 IN SOA   ns1.example.org. bugs.x.w.example.org. 68 3600 300 3600000 3600",
-    "a.example.org.		   	 3600 IN A	192.168.0.1",
-    "a.example.org.		   	 3600 IN MX	10 mail.example.org.",
+// RRsets that emulate the "separate RRs" mode.
+const char* rrset_data_separated[] = {
+    "example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. "
+    "68 3600 300 3600000 3600",
+    "a.example.org. 3600 IN A 192.168.0.1", // these two belong to the same
+    "a.example.org. 3600 IN A 192.168.0.2", // RRset, but are separated.
+    NULL
+};
+
+// Similar to the previous one, but with separated RRSIGs
+const char* rrset_data_sigseparated[] = {
+    "example.org. 3600 IN SOA ns1.example.org. bugs.x.w.example.org. "
+    "68 3600 300 3600000 3600",
+    "a.example.org. 3600 IN A 192.168.0.1",
+    "a.example.org. 3600 IN RRSIG A 5 3 3600 20150420235959 20051021000000 "
+    "40430 example.org. FAKEFAKE",
+    "a.example.org. 3600 IN RRSIG A 5 3 3600 20150420235959 20051021000000 "
+    "53535 example.org. FAKEFAKE",
     NULL
 };
 
 class MockIterator : public ZoneIterator {
 private:
-    MockIterator() :
-        rrset_data_ptr_(rrset_data)
+    MockIterator(const char** rrset_data_ptr, bool pass_empty_rrsig) :
+        rrset_data_ptr_(rrset_data_ptr),
+        pass_empty_rrsig_(pass_empty_rrsig)
     {
     }
 
     const char** rrset_data_ptr_;
+    // If true, emulate an unexpected bogus case where an RRSIG RRset is
+    // returned without the RDATA.  For brevity allow tests tweak it directly.
+    bool pass_empty_rrsig_;
 
 public:
     virtual ConstRRsetPtr getNextRRset() {
@@ -88,9 +101,13 @@ public:
              return (ConstRRsetPtr());
         }
 
-        RRsetPtr result(textToRRset(*rrset_data_ptr_,
-                                    RRClass::IN(), Name("example.org")));
-        rrset_data_ptr_++;
+        ConstRRsetPtr result(textToRRset(*rrset_data_ptr_,
+                                         RRClass::IN(), Name("example.org")));
+        if (pass_empty_rrsig_ && result->getType() == RRType::RRSIG()) {
+            result.reset(new RRset(result->getName(), result->getClass(),
+                                   result->getType(), result->getTTL()));
+        }
+        ++rrset_data_ptr_;
 
         return (result);
     }
@@ -99,8 +116,11 @@ public:
         isc_throw(isc::NotImplemented, "Not implemented");
     }
 
-    static ZoneIteratorPtr makeIterator(void) {
-        return (ZoneIteratorPtr(new MockIterator()));
+    static ZoneIteratorPtr makeIterator(const char** rrset_data_ptr,
+                                        bool pass_empty_rrsig = false)
+    {
+        return (ZoneIteratorPtr(new MockIterator(rrset_data_ptr,
+                                                 pass_empty_rrsig)));
     }
 };
 
@@ -120,7 +140,7 @@ protected:
         EXPECT_TRUE(mem_sgmt_.allMemoryDeallocated()); // catch any leak here.
     }
     const RRClass zclass_;
-    TestMemorySegment mem_sgmt_;
+    test::MemorySegmentTest mem_sgmt_;
     InMemoryClient* client_;
 };
 
@@ -184,7 +204,7 @@ TEST_F(MemoryClientTest, load) {
 
 TEST_F(MemoryClientTest, loadFromIterator) {
     client_->load(Name("example.org"),
-                  *MockIterator::makeIterator());
+                  *MockIterator::makeIterator(rrset_data));
 
     ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
 
@@ -197,17 +217,38 @@ TEST_F(MemoryClientTest, loadFromIterator) {
     rrset = iterator->getNextRRset();
     EXPECT_TRUE(rrset);
     EXPECT_EQ(RRType::MX(), rrset->getType());
+    EXPECT_EQ(1, rrset->getRRsigDataCount()); // this RRset is signed
 
     // RRType::A() RRset
     rrset = iterator->getNextRRset();
     EXPECT_TRUE(rrset);
     EXPECT_EQ(RRType::A(), rrset->getType());
+    EXPECT_EQ(1, rrset->getRRsigDataCount()); // also signed
 
     // There's nothing else in this iterator
     EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
 
     // Iterating past the end should result in an exception
     EXPECT_THROW(iterator->getNextRRset(), isc::Unexpected);
+
+    // Loading the zone with an iterator separating RRs of the same RRset
+    // will fail because the resulting sequence doesn't meet assumptions of
+    // the (current) in-memory implementation.
+    EXPECT_THROW(client_->load(Name("example.org"),
+                               *MockIterator::makeIterator(
+                                   rrset_data_separated)),
+                 InMemoryClient::AddError);
+
+    // Similar to the previous case, but with separated RRSIGs.
+    EXPECT_THROW(client_->load(Name("example.org"),
+                               *MockIterator::makeIterator(
+                                   rrset_data_sigseparated)),
+                 InMemoryClient::AddError);
+
+    // Emulating bogus iterator implementation that passes empty RRSIGs.
+    EXPECT_THROW(client_->load(Name("example.org"),
+                               *MockIterator::makeIterator(rrset_data, true)),
+                 isc::Unexpected);
 }
 
 TEST_F(MemoryClientTest, loadMemoryAllocationFailures) {
@@ -465,6 +506,8 @@ TEST_F(MemoryClientTest, loadDNAMEAndNSNonApex2) {
 }
 
 TEST_F(MemoryClientTest, loadRRSIGFollowsNothing) {
+    // This causes the situation where an RRSIG is added without a covered
+    // RRset.  Such cases are currently rejected.
     EXPECT_THROW(client_->load(Name("example.org"),
                                TEST_DATA_DIR
                                "/example.org-rrsig-follows-nothing.zone"),
@@ -472,22 +515,6 @@ TEST_F(MemoryClientTest, loadRRSIGFollowsNothing) {
     // Teardown checks for memory segment leaks
 }
 
-TEST_F(MemoryClientTest, loadRRSIGNameUnmatched) {
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-rrsig-name-unmatched.zone"),
-                 InMemoryClient::AddError);
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, loadRRSIGTypeUnmatched) {
-    EXPECT_THROW(client_->load(Name("example.org"),
-                               TEST_DATA_DIR
-                               "/example.org-rrsig-type-unmatched.zone"),
-                 InMemoryClient::AddError);
-    // Teardown checks for memory segment leaks
-}
-
 TEST_F(MemoryClientTest, loadRRSIGs) {
     client_->load(Name("example.org"),
                   TEST_DATA_DIR "/example.org-rrsigs.zone");
diff --git a/src/lib/datasrc/memory/tests/testdata/Makefile.am b/src/lib/datasrc/memory/tests/testdata/Makefile.am
index 9fb9986..2d92266 100644
--- a/src/lib/datasrc/memory/tests/testdata/Makefile.am
+++ b/src/lib/datasrc/memory/tests/testdata/Makefile.am
@@ -24,8 +24,6 @@ EXTRA_DIST += example.org-nsec3-signed-no-param.zone
 EXTRA_DIST += example.org-nsec3-signed.zone
 EXTRA_DIST += example.org-out-of-zone.zone
 EXTRA_DIST += example.org-rrsig-follows-nothing.zone
-EXTRA_DIST += example.org-rrsig-name-unmatched.zone
-EXTRA_DIST += example.org-rrsig-type-unmatched.zone
 EXTRA_DIST += example.org-rrsigs.zone
 EXTRA_DIST += example.org-wildcard-dname.zone
 EXTRA_DIST += example.org-wildcard-ns.zone
diff --git a/src/lib/datasrc/memory/tests/testdata/example.org-rrsig-name-unmatched.zone b/src/lib/datasrc/memory/tests/testdata/example.org-rrsig-name-unmatched.zone
deleted file mode 100644
index dc1d728..0000000
--- a/src/lib/datasrc/memory/tests/testdata/example.org-rrsig-name-unmatched.zone
+++ /dev/null
@@ -1,6 +0,0 @@
-;; 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. 70 3600 300 3600000 3600
-ns1.example.org.		      3600 IN A		192.0.2.1
-ns2.example.org.		      3600 IN RRSIG	A 7 3 3600 20150420235959 20051021000000 40430 example.org. FAKEFAKE
diff --git a/src/lib/datasrc/memory/tests/testdata/example.org-rrsig-type-unmatched.zone b/src/lib/datasrc/memory/tests/testdata/example.org-rrsig-type-unmatched.zone
deleted file mode 100644
index 300e124..0000000
--- a/src/lib/datasrc/memory/tests/testdata/example.org-rrsig-type-unmatched.zone
+++ /dev/null
@@ -1,6 +0,0 @@
-;; 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. 72 3600 300 3600000 3600
-ns1.example.org.		      3600 IN AAAA	2001:db8::1
-ns1.example.org.		      3600 IN RRSIG	A 7 3 3600 20150420235959 20051021000000 40430 example.org. FAKEFAKE



More information about the bind10-changes mailing list