[nsec_merge] Merge branch 'trac1807' into nsec_merge

Conflicts:
	src/lib/datasrc/tests/rbtree_unittest.cc

src/lib/datasrc/memory_datasrc.cc

@@ -222,7 +222,6 @@ public:
                         RBTreeNodeChain<Domain>& node_path,
                         ZoneFinder::FindOptions options) const;
 
-private:
     // A helper method for NSEC-signed zones.  It searches the zone for
     // the "closest" NSEC corresponding to the search context stored in
     // node_path (it should contain sufficient information to identify the
@@ -441,7 +440,7 @@ ZoneData::findNode(const Name& name, RBTreeNodeChain<Domain>& node_path,
             NameComparisonResult::SUPERDOMAIN) { // empty node, so NXRRSET
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_SUPER_STOP).arg(name);
             return (ResultType(ZoneFinder::NXRRSET, node,
-                               ConstRBNodeRRsetPtr()));
+                               getClosestNSEC(node_path, options)));
         }
         if (node->getFlag(domain_flag::WILD)) { // maybe a wildcard
             if (node_path.getLastComparisonResult().getRelation() ==
@@ -462,7 +461,12 @@ ZoneData::findNode(const Name& name, RBTreeNodeChain<Domain>& node_path,
             // Now the wildcard should be the best match.
             const Name wildcard(Name("*").concatenate(
                                     node_path.getAbsoluteName()));
-            DomainTree::Result result = domains_.find(wildcard, &node);
+
+            // Clear the node_path so that we don't keep incorrect (NSEC)
+            // context
+            node_path.clear();
+            DomainTree::Result result(domains_.find(wildcard, &node,
+                                                    node_path));
             // Otherwise, why would the domain_flag::WILD be there if
             // there was no wildcard under it?
             assert(result == DomainTree::EXACTMATCH);
@@ -1310,7 +1314,10 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         if (node->isEmpty()) {
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DOMAIN_EMPTY).
                 arg(name);
-            return (createFindResult(NXRRSET, ConstRBNodeRRsetPtr(), rename));
+            return (createFindResult(NXRRSET,
+                                     zone_data_->getClosestNSEC(node_path,
+                                                                options),
+                                     rename));
         }
 
         Domain::const_iterator found;
@@ -1821,8 +1828,7 @@ public:
     {
         // Find the first node (origin) and preserve the node chain for future
         // searches
-        DomainTree::Result result(tree_.find<void*>(origin, &node_, chain_,
-                                                    NULL, NULL));
+        DomainTree::Result result(tree_.find(origin, &node_, chain_));
         // It can't happen that the origin is not in there
         if (result != DomainTree::EXACTMATCH) {
             isc_throw(Unexpected,

src/lib/datasrc/rbtree.h

@@ -758,6 +758,30 @@ public:
         return (ret);
     }
 
+    /// \brief Simple find, with node_path tracking
+    ///
+    /// Acts as described in the \ref find section.
+    Result find(const isc::dns::Name& name, RBNode<T>** node,
+                RBTreeNodeChain<T>& node_path) const
+    {
+        return (find<void*>(name, node, node_path, NULL, NULL));
+    }
+
+    /// \brief Simple find returning immutable node, with node_path tracking
+    ///
+    /// Acts as described in the \ref find section, but returns immutable node
+    /// pointer.
+    Result find(const isc::dns::Name& name, const RBNode<T>** node,
+                RBTreeNodeChain<T>& node_path) const
+    {
+        RBNode<T> *target_node = NULL;
+        Result ret = (find<void*>(name, &target_node, node_path, NULL, NULL));
+        if (ret != NOTFOUND) {
+            *node = target_node;
+        }
+        return (ret);
+    }
+
     /// \brief Find with callback and node chain.
     /// \anchor callback
     ///

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -385,6 +385,10 @@ protected:
                           ZoneFinder::RESULT_DEFAULT);
     void emptyWildcardCheck(ZoneFinder::FindResultFlags expected_flags =
                             ZoneFinder::RESULT_DEFAULT);
+    void findNSECENTCheck(const Name& ent_name,
+                          ConstRRsetPtr expected_nsec,
+                          ZoneFinder::FindResultFlags expected_flags =
+                          ZoneFinder::RESULT_DEFAULT);
 
 public:
     InMemoryZoneFinderTest() :
@@ -441,8 +445,18 @@ public:
             {"0P9MHAVEQVM6T7VBL5LOP2U3T2RP3TOM.example.org. 300 IN "
              "NSEC3 1 1 12 aabbccdd 2T7B4G4VSA5SMI47K61MV5BV1A22BOJR A RRSIG",
              &rr_nsec3_},
-            {"example.org. 300 IN NSEC cname.example.org. A NS NSEC",
-             &rr_nsec_},
+            {"example.org. 300 IN NSEC wild.*.foo.example.org. "
+             "NS SOA RRSIG NSEC DNSKEY", &rr_nsec_},
+            // Together with the apex NSEC, these next NSECs make a complete
+            // chain in the case of the zone for the emptyNonterminal tests
+            // (We may want to clean up this generator code and/or masterLoad
+            // so that we can prepare conflicting datasets better)
+            {"wild.*.foo.example.org. 3600 IN NSEC ns.example.org. "
+             "A RRSIG NSEC", &rr_ent_nsec2_},
+            {"ns.example.org. 3600 IN NSEC foo.wild.example.org. A RRSIG NSEC",
+             &rr_ent_nsec3_},
+            {"foo.wild.example.org. 3600 IN NSEC example.org. A RRSIG NSEC",
+             &rr_ent_nsec4_},
             {NULL, NULL}
         };
 
@@ -508,6 +522,9 @@ public:
     RRsetPtr rr_not_wild_another_;
     RRsetPtr rr_nsec3_;
     RRsetPtr rr_nsec_;
+    RRsetPtr rr_ent_nsec2_;
+    RRsetPtr rr_ent_nsec3_;
+    RRsetPtr rr_ent_nsec4_;
 
     // A faked NSEC3 hash calculator for convenience.
     // Tests that need to use the faked hashed values should call
@@ -1039,6 +1056,57 @@ TEST_F(InMemoryZoneFinderTest, findNSECSigned) {
     findCheck(ZoneFinder::RESULT_NSEC_SIGNED);
 }
 
+// Generalized test for Empty Nonterminal (ENT) cases with NSEC
+void
+InMemoryZoneFinderTest::findNSECENTCheck(const Name& ent_name,
+    ConstRRsetPtr expected_nsec,
+    ZoneFinder::FindResultFlags expected_flags)
+{
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_emptywild_));
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_under_wild_));
+
+    // Sanity check: Should result in NXRRSET
+    findTest(ent_name, RRType::A(), ZoneFinder::NXRRSET, true,
+             ConstRRsetPtr(), expected_flags);
+    // Sanity check: No NSEC added yet
+    findTest(ent_name, RRType::A(), ZoneFinder::NXRRSET, true,
+             ConstRRsetPtr(), expected_flags,
+             NULL, ZoneFinder::FIND_DNSSEC);
+
+    // Now add the NSEC rrs making it a 'complete' zone (in terms of NSEC,
+    // there are no sigs)
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_nsec_));
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ent_nsec2_));
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ent_nsec3_));
+    EXPECT_EQ(SUCCESS, zone_finder_.add(rr_ent_nsec4_));
+
+    // Should result in NXRRSET, and RESULT_NSEC_SIGNED
+    findTest(ent_name, RRType::A(), ZoneFinder::NXRRSET, true,
+             ConstRRsetPtr(),
+             expected_flags | ZoneFinder::RESULT_NSEC_SIGNED);
+
+    // And check for the NSEC if DNSSEC_OK
+    findTest(ent_name, RRType::A(), ZoneFinder::NXRRSET, true,
+             expected_nsec, expected_flags | ZoneFinder::RESULT_NSEC_SIGNED,
+             NULL, ZoneFinder::FIND_DNSSEC);
+}
+
+TEST_F(InMemoryZoneFinderTest,findNSECEmptyNonterminal) {
+    // Non-wildcard case
+    findNSECENTCheck(Name("wild.example.org"), rr_ent_nsec3_);
+}
+
+TEST_F(InMemoryZoneFinderTest,findNSECEmptyNonterminalWildcard) {
+    // Wildcard case, above actual wildcard
+    findNSECENTCheck(Name("foo.example.org"), rr_nsec_);
+}
+
+TEST_F(InMemoryZoneFinderTest,findNSECEmptyNonterminalAtWildcard) {
+    // Wildcard case, at actual wildcard
+    findNSECENTCheck(Name("bar.foo.example.org"), rr_nsec_,
+                     ZoneFinder::RESULT_WILDCARD);
+}
+
 void
 InMemoryZoneFinderTest::emptyNodeCheck(
     ZoneFinder::FindResultFlags expected_flags)
@@ -1243,8 +1311,15 @@ InMemoryZoneFinderTest::wildcardCheck(
     // be in the wildcard (so check the wildcard isn't matched at the parent)
     {
         SCOPED_TRACE("Search at parent");
-        findTest(Name("wild.example.org"), RRType::A(), ZoneFinder::NXRRSET,
-                 true, ConstRRsetPtr(), expected_flags, NULL, find_options);
+        if ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0) {
+            findTest(Name("wild.example.org"), RRType::A(),
+                     ZoneFinder::NXRRSET, true, rr_nsec_, expected_flags,
+                     NULL, find_options);
+        } else {
+            findTest(Name("wild.example.org"), RRType::A(),
+                     ZoneFinder::NXRRSET, true, ConstRRsetPtr(),
+                     expected_flags, NULL, find_options);
+        }
     }
 
     // Search the original name of wildcard

src/lib/datasrc/tests/rbtree_unittest.cc

@@ -180,10 +180,10 @@ TEST_F(RBTreeTest, findName) {
 TEST_F(RBTreeTest, findError) {
     // For the version that takes a node chain, the chain must be empty.
     RBTreeNodeChain<int> chain;
-    EXPECT_EQ(RBTree<int>::EXACTMATCH, rbtree.find<void*>(Name("a"), &crbtnode,
-                                                          chain, NULL, NULL));
+    EXPECT_EQ(RBTree<int>::EXACTMATCH, rbtree.find(Name("a"), &crbtnode,
+                                                   chain));
     // trying to reuse the same chain.  it should result in an exception.
-    EXPECT_THROW(rbtree.find<void*>(Name("a"), &crbtnode, chain, NULL, NULL),
+    EXPECT_THROW(rbtree.find(Name("a"), &crbtnode, chain),
                  BadValue);
 }
 
@@ -280,7 +280,7 @@ TEST_F(RBTreeTest, chainLevel) {
     Name node_name(Name::ROOT_NAME());
     EXPECT_EQ(RBTree<int>::SUCCESS, tree.insert(node_name, &rbtnode));
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
-              tree.find<void*>(node_name, &crbtnode, chain, NULL, NULL));
+              tree.find(node_name, &crbtnode, chain));
     EXPECT_EQ(1, chain.getLevelCount());
 
     /*
@@ -303,8 +303,7 @@ TEST_F(RBTreeTest, chainLevel) {
         EXPECT_EQ(RBTree<int>::SUCCESS, tree.insert(node_name, &rbtnode));
         RBTreeNodeChain<int> found_chain;
         EXPECT_EQ(RBTree<int>::EXACTMATCH,
-                  tree.find<void*>(node_name, &crbtnode, found_chain,
-                                   NULL, NULL));
+                  tree.find(node_name, &crbtnode, found_chain));
         EXPECT_EQ(i, found_chain.getLevelCount());
     }
 
@@ -352,8 +351,7 @@ TEST_F(RBTreeTest, nextNode) {
     RBTreeNodeChain<int> node_path;
     const RBNode<int>* node = NULL;
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
-              rbtree.find<void*>(Name(names[0]), &node, node_path, NULL,
-                                 NULL));
+              rbtree.find(Name(names[0]), &node, node_path));
     for (int i = 0; i < name_count; ++i) {
         EXPECT_NE(static_cast<void*>(NULL), node);
         EXPECT_EQ(Name(names[i]), node_path.getAbsoluteName());
@@ -399,8 +397,7 @@ previousWalk(RBTree<int>& rbtree, const RBNode<int>* node,
             const RBNode<int>* node2(NULL);
             RBTreeNodeChain<int> node_path2;
             EXPECT_EQ(RBTree<int>::EXACTMATCH,
-                      rbtree.find<void*>(Name(names[i - 1]), &node2,
-                                         node_path2, NULL, NULL));
+                      rbtree.find(Name(names[i - 1]), &node2, node_path2));
             EXPECT_EQ(node, node2);
         }
         node = rbtree.previousNode(node_path);
@@ -420,8 +417,7 @@ TEST_F(RBTreeTest, previousNode) {
     {
         SCOPED_TRACE("Iterate through");
         EXPECT_EQ(RBTree<int>::EXACTMATCH,
-                  rbtree.find<void*>(Name(names[name_count - 1]), &node,
-                                     node_path, NULL, NULL));
+                  rbtree.find(Name(names[name_count - 1]), &node, node_path));
         previousWalk(rbtree, node, node_path, name_count, false);
         node = NULL;
         node_path.clear();
@@ -431,8 +427,7 @@ TEST_F(RBTreeTest, previousNode) {
         SCOPED_TRACE("Iterate from the middle");
         // Now, start somewhere in the middle, but within the real node.
         EXPECT_EQ(RBTree<int>::EXACTMATCH,
-                  rbtree.find<void*>(Name(names[4]), &node, node_path,
-                                     NULL, NULL));
+                  rbtree.find(Name(names[4]), &node, node_path));
         previousWalk(rbtree, node, node_path, 5, false);
         node = NULL;
         node_path.clear();
@@ -443,8 +438,7 @@ TEST_F(RBTreeTest, previousNode) {
         // If we start at the lowest (which is "a"), we get to the beginning
         // right away.
         EXPECT_EQ(RBTree<int>::EXACTMATCH,
-                  rbtree.find<void*>(Name(names[0]), &node, node_path, NULL,
-                                     NULL));
+                  rbtree.find(Name(names[0]), &node, node_path));
         EXPECT_NE(static_cast<void*>(NULL), node);
         EXPECT_EQ(static_cast<void*>(NULL), rbtree.previousNode(node_path));
         node = NULL;
@@ -465,7 +459,7 @@ TEST_F(RBTreeTest, previousNode) {
     {
         SCOPED_TRACE("Start after the last");
         EXPECT_EQ(RBTree<int>::NOTFOUND,
-                  rbtree.find<void*>(Name("z"), &node, node_path, NULL, NULL));
+                  rbtree.find(Name("z"), &node, node_path));
         previousWalk(rbtree, node, node_path, name_count, true);
         node = NULL;
         node_path.clear();
@@ -477,8 +471,7 @@ TEST_F(RBTreeTest, previousNode) {
         // we exited - 'c' (actually, we should get it by the find, as partial
         // match).
         EXPECT_EQ(RBTree<int>::PARTIALMATCH,
-                  rbtree.find<void*>(Name("b.c"), &node, node_path, NULL,
-                                     NULL));
+                  rbtree.find(Name("b.c"), &node, node_path));
         previousWalk(rbtree, node, node_path, 3, false);
         node = NULL;
         node_path.clear();
@@ -492,8 +485,7 @@ TEST_F(RBTreeTest, previousNode) {
         // The d.e.f is empty node, so it is hidden by find. Therefore NOTFOUND
         // and not PARTIALMATCH.
         EXPECT_EQ(RBTree<int>::NOTFOUND,
-                  rbtree.find<void*>(Name("xy.d.e.f"), &node, node_path,
-                                     NULL, NULL));
+                  rbtree.find(Name("xy.d.e.f"), &node, node_path));
         previousWalk(rbtree, node, node_path, 5, true);
         node = NULL;
         node_path.clear();
@@ -507,8 +499,7 @@ TEST_F(RBTreeTest, previousNode) {
         // The d.e.f is empty node, so it is hidden by find. Therefore NOTFOUND
         // and not PARTIALMATCH.
         EXPECT_EQ(RBTree<int>::NOTFOUND,
-                  rbtree.find<void*>(Name("yz.d.e.f"), &node, node_path,
-                                     NULL, NULL));
+                  rbtree.find(Name("yz.d.e.f"), &node, node_path));
         previousWalk(rbtree, node, node_path, 9, true);
         node = NULL;
         node_path.clear();
@@ -519,8 +510,7 @@ TEST_F(RBTreeTest, previousNode) {
         // The d.e.f is a single node, but we want only part of it. We
         // should start iterating before it.
         EXPECT_EQ(RBTree<int>::NOTFOUND,
-                  rbtree.find<void*>(Name("e.f"), &node, node_path,
-                                     NULL, NULL));
+                  rbtree.find(Name("e.f"), &node, node_path));
         previousWalk(rbtree, node, node_path, 3, true);
         node = NULL;
         node_path.clear();
@@ -531,8 +521,7 @@ TEST_F(RBTreeTest, previousNode) {
         // Just check it doesn't crash, etc.
         RBTree<int> empty_tree;
         EXPECT_EQ(RBTree<int>::NOTFOUND,
-                  empty_tree.find<void*>(Name("x"), &node, node_path,
-                                         NULL, NULL));
+                  empty_tree.find(Name("x"), &node, node_path));
         EXPECT_EQ(static_cast<void*>(NULL), node);
         EXPECT_EQ(static_cast<void*>(NULL),
                   empty_tree.previousNode(node_path));
@@ -576,7 +565,7 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     // A search for an empty tree should result in no 'last compared', too.
     RBTree<int> empty_tree;
     EXPECT_EQ(RBTree<int>::NOTFOUND,
-              empty_tree.find<void*>(Name("a"), &crbtnode, chain, NULL, NULL));
+              empty_tree.find(Name("a"), &crbtnode, chain));
     EXPECT_EQ(static_cast<void*>(NULL), chain.getLastComparedNode());
     chain.clear();
 
@@ -584,8 +573,7 @@ TEST_F(RBTreeTest, getLastComparedNode) {
 
     // Exact match case.  The returned node should be last compared.
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
-              tree.find<void*>(Name("x.d.e.f"), &expected_node, chain,
-                               NULL, NULL));
+              tree.find(Name("x.d.e.f"), &expected_node, chain));
     EXPECT_EQ(expected_node, chain.getLastComparedNode());
     // 2 = # labels of "x."
     comparisonChecks(chain, 0, 2, NameComparisonResult::EQUAL);
@@ -596,8 +584,7 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
               tree.find(Name("i.g.h"), &expected_node));
     EXPECT_EQ(RBTree<int>::PARTIALMATCH,
-              tree.find<void*>(Name("x.i.g.h"), &crbtnode, chain,
-                                 NULL, NULL));
+              tree.find(Name("x.i.g.h"), &crbtnode, chain));
     EXPECT_EQ(expected_node, chain.getLastComparedNode());
     // i.g.h < x.i.g.h, 2 = # labels of "i."
     comparisonChecks(chain, 1, 2, NameComparisonResult::SUBDOMAIN);
@@ -608,8 +595,7 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
               tree.find(Name("x.d.e.f"), &expected_node));
     EXPECT_EQ(RBTree<int>::PARTIALMATCH,
-              tree.find<void*>(Name("a.d.e.f"), &crbtnode, chain,
-                                 NULL, NULL));
+              tree.find(Name("a.d.e.f"), &crbtnode, chain));
     EXPECT_EQ(expected_node, chain.getLastComparedNode());
     // a < x, 1 = # labels of "." (trailing dot)
     comparisonChecks(chain, -1, 1, NameComparisonResult::COMMONANCESTOR);
@@ -620,8 +606,7 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
               tree.find(Name("z.d.e.f"), &expected_node));
     EXPECT_EQ(RBTree<int>::PARTIALMATCH,
-              tree.find<void*>(Name("zz.d.e.f"), &crbtnode, chain,
-                                 NULL, NULL));
+              tree.find(Name("zz.d.e.f"), &crbtnode, chain));
     EXPECT_EQ(expected_node, chain.getLastComparedNode());
     // zz > z, 1 = # labels of "." (trailing dot)
     comparisonChecks(chain, 1, 1, NameComparisonResult::COMMONANCESTOR);
@@ -632,8 +617,7 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
               tree.find(Name("w.y.d.e.f"), &expected_node));
     EXPECT_EQ(RBTree<int>::PARTIALMATCH,
-              tree.find<void*>(Name("y.d.e.f"), &crbtnode, chain,
-                                 NULL, NULL));
+              tree.find(Name("y.d.e.f"), &crbtnode, chain));
     EXPECT_EQ(expected_node, chain.getLastComparedNode());
     // y < w.y, 2 = # labels of "y."
     comparisonChecks(chain, -1, 2, NameComparisonResult::SUPERDOMAIN);
@@ -643,8 +627,7 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     // with the search name in the subtree below the matching node.
     // (the expected node is the same as the previous case)
     EXPECT_EQ(RBTree<int>::PARTIALMATCH,
-              tree.find<void*>(Name("z.y.d.e.f"), &crbtnode, chain,
-                                 NULL, NULL));
+              tree.find(Name("z.y.d.e.f"), &crbtnode, chain));
     EXPECT_EQ(expected_node, chain.getLastComparedNode());
     // z.y > w.y, 2 = # labels of "y."
     comparisonChecks(chain, 1, 2, NameComparisonResult::COMMONANCESTOR);
@@ -653,7 +636,7 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     // Search stops in the highest level after following a left branch.
     EXPECT_EQ(RBTree<int>::EXACTMATCH, tree.find(Name("c"), &expected_node));
     EXPECT_EQ(RBTree<int>::NOTFOUND,
-              tree.find<void*>(Name("bb"), &crbtnode, chain, NULL, NULL));
+              tree.find(Name("bb"), &crbtnode, chain));
     EXPECT_EQ(expected_node, chain.getLastComparedNode());
     // bb < c, 1 = # labels of "." (trailing dot)
     comparisonChecks(chain, -1, 1, NameComparisonResult::COMMONANCESTOR);
@@ -662,7 +645,7 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     // Search stops in the highest level after following a right branch.
     // (the expected node is the same as the previous case)
     EXPECT_EQ(RBTree<int>::NOTFOUND,
-              tree.find<void*>(Name("d"), &crbtnode, chain, NULL, NULL));
+              tree.find(Name("d"), &crbtnode, chain));
     EXPECT_EQ(expected_node, chain.getLastComparedNode());
     // d > c, 1 = # labels of "." (trailing dot)
     comparisonChecks(chain, 1, 1, NameComparisonResult::COMMONANCESTOR);