[master] Merge branch 'trac1430'

src/lib/datasrc/database.cc

@@ -585,7 +585,8 @@ DatabaseClient::Finder::findWildcardMatch(
     WantedTypes final_types(FINAL_TYPES());
     final_types.insert(type);
 
-    for (size_t i = 1; i <= (name.getLabelCount() - dresult.last_known); ++i) {
+    const size_t remove_labels = name.getLabelCount() - dresult.last_known;
+    for (size_t i = 1; i <= remove_labels; ++i) {
 
         // Strip off the left-more label(s) in the name and replace with a "*".
         const Name superdomain(name.split(i));
@@ -840,6 +841,16 @@ DatabaseClient::Finder::findInternal(const isc::dns::Name& name,
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_FIND_RECORDS)
               .arg(accessor_->getDBName()).arg(name).arg(type).arg(getClass());
 
+    // find() variants generally expect 'name' to be included in the zone.
+    // Otherwise the search algorithm below won't work correctly, so we
+    // reject the unexpected case first.
+    const NameComparisonResult::NameRelation reln =
+        name.compare(getOrigin()).getRelation();
+    if (reln != NameComparisonResult::SUBDOMAIN &&
+        reln != NameComparisonResult::EQUAL) {
+        return (FindResult(NXDOMAIN, ConstRRsetPtr()));
+    }
+
     // First, go through all superdomains from the origin down, searching for
     // nodes that indicate a delegation (i.e. NS or DNAME, ignoring NS records
     // at the apex).  If one is found, the search stops there.

src/lib/datasrc/tests/database_unittest.cc

@@ -387,11 +387,11 @@ private:
             // 'hardcoded' names to trigger exceptions
             // On these names some exceptions are thrown, to test the robustness
             // of the find() method.
-            if (searched_name_ == "dsexception.in.search.") {
+            if (searched_name_ == "dsexception.example.org.") {
                 isc_throw(DataSourceError, "datasource exception on search");
-            } else if (searched_name_ == "iscexception.in.search.") {
+            } else if (searched_name_ == "iscexception.example.org.") {
                 isc_throw(isc::Exception, "isc exception on search");
-            } else if (searched_name_ == "basicexception.in.search.") {
+            } else if (searched_name_ == "basicexception.example.org.") {
                 throw std::exception();
             }
 
@@ -420,11 +420,12 @@ private:
         }
 
         virtual bool getNext(std::string (&columns)[COLUMN_COUNT]) {
-            if (searched_name_ == "dsexception.in.getnext.") {
+            if (searched_name_ == "dsexception.getnext.example.org.") {
                 isc_throw(DataSourceError, "datasource exception on getnextrecord");
-            } else if (searched_name_ == "iscexception.in.getnext.") {
+            } else if (searched_name_ == "iscexception.getnext.example.org.") {
                 isc_throw(isc::Exception, "isc exception on getnextrecord");
-            } else if (searched_name_ == "basicexception.in.getnext.") {
+            } else if (searched_name_ ==
+                       "basicexception.getnext.example.org.") {
                 throw std::exception();
             }
 
@@ -1731,27 +1732,27 @@ TYPED_TEST(DatabaseClientTest, find) {
 
     // Trigger the hardcoded exceptions and see if find() has cleaned up
     if (this->is_mock_) {
-        EXPECT_THROW(finder->find(isc::dns::Name("dsexception.in.search."),
+        EXPECT_THROW(finder->find(Name("dsexception.example.org."),
                                   this->qtype_,
                                   ZoneFinder::FIND_DEFAULT),
                      DataSourceError);
-        EXPECT_THROW(finder->find(isc::dns::Name("iscexception.in.search."),
+        EXPECT_THROW(finder->find(Name("iscexception.example.org."),
                                   this->qtype_,
                                   ZoneFinder::FIND_DEFAULT),
                      isc::Exception);
-        EXPECT_THROW(finder->find(isc::dns::Name("basicexception.in.search."),
+        EXPECT_THROW(finder->find(Name("basicexception.example.org."),
                                   this->qtype_,
                                   ZoneFinder::FIND_DEFAULT),
                      std::exception);
-        EXPECT_THROW(finder->find(isc::dns::Name("dsexception.in.getnext."),
+        EXPECT_THROW(finder->find(Name("dsexception.getnext.example.org"),
                                   this->qtype_,
                                   ZoneFinder::FIND_DEFAULT),
                      DataSourceError);
-        EXPECT_THROW(finder->find(isc::dns::Name("iscexception.in.getnext."),
+        EXPECT_THROW(finder->find(Name("iscexception.getnext.example.org."),
                                   this->qtype_,
                                   ZoneFinder::FIND_DEFAULT),
                      isc::Exception);
-        EXPECT_THROW(finder->find(isc::dns::Name("basicexception.in.getnext."),
+        EXPECT_THROW(finder->find(Name("basicexception.getnext.example.org."),
                                   this->qtype_,
                                   ZoneFinder::FIND_DEFAULT),
                      std::exception);
@@ -1769,6 +1770,41 @@ TYPED_TEST(DatabaseClientTest, find) {
                this->expected_rdatas_, this->expected_sig_rdatas_);
 }
 
+TYPED_TEST(DatabaseClientTest, findOutOfZone) {
+    // If the query name is out-of-zone it should result in NXDOMAIN
+    boost::shared_ptr<DatabaseClient::Finder> finder(this->getFinder());
+    vector<ConstRRsetPtr> target;
+
+    // Superdomain
+    doFindTest(*finder, Name("org"), this->qtype_, this->qtype_,
+               this->rrttl_, ZoneFinder::NXDOMAIN,
+               this->empty_rdatas_, this->empty_rdatas_);
+    EXPECT_EQ(ZoneFinder::NXDOMAIN, finder->findAll(Name("org"), target).code);
+    // sharing a common ancestor
+    doFindTest(*finder, Name("noexample.org"), this->qtype_, this->qtype_,
+               this->rrttl_, ZoneFinder::NXDOMAIN,
+               this->empty_rdatas_, this->empty_rdatas_);
+    EXPECT_EQ(ZoneFinder::NXDOMAIN, finder->findAll(Name("noexample.org"),
+                                                    target).code);
+    // totally unrelated domain, smaller number of labels
+    doFindTest(*finder, Name("com"), this->qtype_, this->qtype_,
+               this->rrttl_, ZoneFinder::NXDOMAIN,
+               this->empty_rdatas_, this->empty_rdatas_);
+    EXPECT_EQ(ZoneFinder::NXDOMAIN, finder->findAll(Name("com"), target).code);
+    // totally unrelated domain, same number of labels
+    doFindTest(*finder, Name("example.com"), this->qtype_, this->qtype_,
+               this->rrttl_, ZoneFinder::NXDOMAIN,
+               this->empty_rdatas_, this->empty_rdatas_);
+    EXPECT_EQ(ZoneFinder::NXDOMAIN, finder->findAll(Name("example.com"),
+                                                    target).code);
+    // totally unrelated domain, larger number of labels
+    doFindTest(*finder, Name("more.example.com"), this->qtype_, this->qtype_,
+               this->rrttl_, ZoneFinder::NXDOMAIN,
+               this->empty_rdatas_, this->empty_rdatas_);
+    EXPECT_EQ(ZoneFinder::NXDOMAIN, finder->findAll(Name("more.example.com"),
+                                                    target).code);
+}
+
 TYPED_TEST(DatabaseClientTest, findDelegation) {
     boost::shared_ptr<DatabaseClient::Finder> finder(this->getFinder());
 
@@ -2673,12 +2709,13 @@ TYPED_TEST(DatabaseClientTest, addDeviantRR) {
     this->expected_rdatas_.clear();
     this->expected_rdatas_.push_back("192.0.2.100");
     {
-        // Note: with the find() implementation being more strict about
-        // zone cuts, this test may fail.  Then the test should be updated.
+        // Note: find() rejects out-of-zone query name with NXDOMAIN
+        // regardless of whether adding the RR succeeded, so this check
+        // actually doesn't confirm it.
         SCOPED_TRACE("add out-of-zone RR");
         doFindTest(this->updater_->getFinder(), Name("example.com"),
                    this->qtype_, this->qtype_, this->rrttl_,
-                   ZoneFinder::SUCCESS, this->expected_rdatas_,
+                   ZoneFinder::NXDOMAIN, this->empty_rdatas_,
                    this->empty_rdatas_);
     }
 }

src/lib/datasrc/zone.h

@@ -257,6 +257,16 @@ public:
     ///   proof of the non existence of any matching wildcard or non existence
     ///   of an exact match when a wildcard match is found.
     ///
+    /// In general, \c name is expected to be included in the zone, that is,
+    /// it should be equal to or a subdomain of the zone origin.  Otherwise
+    /// this method will return \c NXDOMAIN with an empty RRset.  But such a
+    /// case should rather be considered a caller's bug.
+    ///
+    /// \note For this reason it's probably better to throw an exception
+    /// than returning \c NXDOMAIN.  This point should be revisited in a near
+    /// future version.  In any case applications shouldn't call this method
+    /// for an out-of-zone name.
+    ///
     /// \exception std::bad_alloc Memory allocation such as for constructing
     ///  the resulting RRset fails
     /// \exception DataSourceError Derived class specific exception, e.g.

src/lib/python/isc/datasrc/finder_inc.cc

@@ -91,6 +91,10 @@ Their semantics is as follows (they are or bit-field):\n\
   of the non existence of any matching wildcard or non existence of an\n\
   exact match when a wildcard match is found.\n\
 \n\
+In general, name is expected to be included in the zone, that is, it\n\
+should be equal to or a subdomain of the zone origin. Otherwise this\n\
+method will return NXDOMAIN with an empty RRset. But such a case\n\
+should rather be considered a caller's bug.\n\
 \n\
 This method raises an isc.datasrc.Error exception if there is an\n\
 internal error in the datasource.\n\