[master] Merge branch 'trac1535'

src/lib/datasrc/database.cc

@@ -865,7 +865,7 @@ DatabaseClient::Finder::findInternal(const Name& name, const RRType& type,
         name.compare(getOrigin()).getRelation();
     if (reln != NameComparisonResult::SUBDOMAIN &&
         reln != NameComparisonResult::EQUAL) {
-        return (ResultContext(NXDOMAIN, ConstRRsetPtr()));
+        isc_throw(OutOfZone, name.toText() << " not in " << getOrigin());
     }
 
     // First, go through all superdomains from the origin down, searching for

src/lib/datasrc/memory_datasrc.cc

@@ -362,8 +362,7 @@ ZoneData::findNode(const Name& name, ZoneFinder::FindOptions options) const {
     if (result == DomainTree::EXACTMATCH) {
         return (ResultType(ZoneFinder::SUCCESS, node, state.rrset_,
                            zonecut_flag));
-    }
-    if (result == DomainTree::PARTIALMATCH) {
+    } else if (result == DomainTree::PARTIALMATCH) {
         assert(node != NULL);
         if (state.dname_node_ != NULL) { // DNAME
             LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_DNAME_FOUND).
@@ -408,10 +407,15 @@ ZoneData::findNode(const Name& name, ZoneFinder::FindOptions options) const {
                                FindNodeResult::FIND_WILDCARD |
                                zonecut_flag));
         }
+        // Nothing really matched.
+        LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_NOT_FOUND).arg(name);
+        return (ResultType(ZoneFinder::NXDOMAIN, node, state.rrset_));
+    } else {
+        // If the name is neither an exact or partial match, it is
+        // out of bailiwick, which is considered an error.
+        isc_throw(OutOfZone, name.toText() << " not in " <<
+                             origin_data_->getName());
     }
-    // Nothing really matched.  The name may even be out-of-bailiwick.
-    LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_NOT_FOUND).arg(name);
-    return (ResultType(ZoneFinder::NXDOMAIN, node, state.rrset_));
 }
 } // unnamed namespace
 
@@ -1346,7 +1350,7 @@ InMemoryZoneFinder::findNSEC3(const Name& name, bool recursive) {
     const NameComparisonResult cmp_result = name.compare(impl_->origin_);
     if (cmp_result.getRelation() != NameComparisonResult::EQUAL &&
         cmp_result.getRelation() != NameComparisonResult::SUBDOMAIN) {
-        isc_throw(InvalidParameter, "findNSEC3 attempt for out-of-zone name: "
+        isc_throw(OutOfZone, "findNSEC3 attempt for out-of-zone name: "
                   << name << ", zone: " << impl_->origin_ << "/"
                   << impl_->zone_class_);
     }
@@ -1451,6 +1455,13 @@ addAdditional(RBNodeRRset* rrset, ZoneData* zone_data,
 
         const Name& name = getAdditionalName(rrset->getType(),
                                              rdata_iterator->getCurrent());
+        // if the name is not in or below this zone, skip it
+        const NameComparisonResult::NameRelation reln =
+            name.compare(zone_data->origin_data_->getName()).getRelation();
+         if (reln != NameComparisonResult::SUBDOMAIN &&
+             reln != NameComparisonResult::EQUAL) {
+            continue;
+        }
         const ZoneData::FindMutableNodeResult result =
             zone_data->findNode<ZoneData::FindMutableNodeResult>(
                 name, ZoneFinder::FIND_GLUE_OK);

src/lib/datasrc/memory_datasrc.h

@@ -125,16 +125,6 @@ public:
     ///    exists).
     result::Result add(const isc::dns::ConstRRsetPtr& rrset);
 
-    /// \brief RRSet out of zone exception.
-    ///
-    /// This is thrown if addition of an RRset that doesn't belong under the
-    /// zone's origin is requested.
-    struct OutOfZone : public InvalidParameter {
-        OutOfZone(const char* file, size_t line, const char* what) :
-            InvalidParameter(file, line, what)
-        { }
-    };
-
     /// \brief RRset is NULL exception.
     ///
     /// This is thrown if the provided RRset parameter is NULL.

src/lib/datasrc/tests/database_unittest.cc

@@ -1872,38 +1872,33 @@ TYPED_TEST(DatabaseClientTest, find) {
 }
 
 TYPED_TEST(DatabaseClientTest, findOutOfZone) {
-    // If the query name is out-of-zone it should result in NXDOMAIN
+    // If the query name is out-of-zone it should result in an exception
     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);
+    EXPECT_THROW(finder->find(Name("org"), this->qtype_), OutOfZone);
+    EXPECT_THROW(finder->findAll(Name("org"), target), OutOfZone);
+
     // 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);
+    EXPECT_THROW(finder->find(Name("noexample.org"), this->qtype_),
+                 OutOfZone);
+    EXPECT_THROW(finder->findAll(Name("noexample.org"), target),
+                 OutOfZone);
+
     // 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);
+    EXPECT_THROW(finder->find(Name("com"), this->qtype_), OutOfZone);
+    EXPECT_THROW(finder->findAll(Name("com"), target), OutOfZone);
+
     // 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);
+    EXPECT_THROW(finder->find(Name("example.com"), this->qtype_), OutOfZone);
+    EXPECT_THROW(finder->findAll(Name("example.com"), target), OutOfZone);
+
     // 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);
+    EXPECT_THROW(finder->find(Name("more.example.com"), this->qtype_),
+                 OutOfZone);
+    EXPECT_THROW(finder->findAll(Name("more.example.com"), target),
+                 OutOfZone);
 }
 
 TYPED_TEST(DatabaseClientTest, findDelegation) {
@@ -2831,14 +2826,13 @@ TYPED_TEST(DatabaseClientTest, addDeviantRR) {
     this->expected_rdatas_.clear();
     this->expected_rdatas_.push_back("192.0.2.100");
     {
-        // Note: find() rejects out-of-zone query name with NXDOMAIN
+        // Note: find() rejects out-of-zone query name with an exception
         // 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::NXDOMAIN, this->empty_rdatas_,
-                   this->empty_rdatas_);
+        EXPECT_THROW(this->updater_->getFinder().find(Name("example.com"),
+                                                      this->qtype_),
+                     OutOfZone);
     }
 }
 

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -672,7 +672,7 @@ TEST_F(InMemoryZoneFinderTest, constructor) {
  */
 TEST_F(InMemoryZoneFinderTest, add) {
     // This one does not belong to this zone
-    EXPECT_THROW(zone_finder_.add(rr_out_), InMemoryZoneFinder::OutOfZone);
+    EXPECT_THROW(zone_finder_.add(rr_out_), OutOfZone);
     // Test null pointer
     EXPECT_THROW(zone_finder_.add(ConstRRsetPtr()),
                  InMemoryZoneFinder::NullRRset);
@@ -899,8 +899,9 @@ TEST_F(InMemoryZoneFinderTest, findAny) {
     findAllTest(origin_, ZoneFinder::SUCCESS, expected_sets);
 
     // out zone name
-    findAllTest(Name("example.com"), ZoneFinder::NXDOMAIN,
-                vector<ConstRRsetPtr>());
+    EXPECT_THROW(findAllTest(Name("example.com"), ZoneFinder::NXDOMAIN,
+                             vector<ConstRRsetPtr>()),
+                 OutOfZone);
 
     expected_sets.clear();
     expected_sets.push_back(rr_child_glue_);
@@ -997,8 +998,8 @@ InMemoryZoneFinderTest::findCheck(ZoneFinder::FindResultFlags expected_flags) {
     // These domains don't exist (and one is out of the zone)
     findTest(Name("nothere.example.org"), RRType::A(), ZoneFinder::NXDOMAIN,
              true, ConstRRsetPtr(), expected_flags);
-    findTest(Name("example.net"), RRType::A(), ZoneFinder::NXDOMAIN, true,
-             ConstRRsetPtr(), expected_flags);
+    EXPECT_THROW(zone_finder_.find(Name("example.net"), RRType::A()),
+                 OutOfZone);
 }
 
 TEST_F(InMemoryZoneFinderTest, find) {
@@ -1053,8 +1054,7 @@ InMemoryZoneFinderTest::emptyNodeCheck(
     // Note: basically we don't expect such a query to be performed (the common
     // operation is to identify the best matching zone first then perform
     // search it), but we shouldn't be confused even in the unexpected case.
-    findTest(Name("org"), RRType::A(), ZoneFinder::NXDOMAIN, true,
-             ConstRRsetPtr(), expected_flags);
+    EXPECT_THROW(zone_finder_.find(Name("org"), RRType::A()), OutOfZone);
 }
 
 TEST_F(InMemoryZoneFinderTest, emptyNode) {
@@ -1512,14 +1512,12 @@ TEST_F(InMemoryZoneFinderTest, swap) {
     EXPECT_EQ(RRClass::CH(), finder1.getClass());
     EXPECT_EQ(RRClass::IN(), finder2.getClass());
     // make sure the zone data is swapped, too
-    findTest(origin_, RRType::NS(), ZoneFinder::NXDOMAIN, false,
-             ConstRRsetPtr(), ZoneFinder::RESULT_DEFAULT, &finder1);
+    EXPECT_THROW(finder1.find(origin_, RRType::NS()), OutOfZone);
     findTest(other_origin, RRType::TXT(), ZoneFinder::SUCCESS, false,
              ConstRRsetPtr(), ZoneFinder::RESULT_DEFAULT, &finder1);
     findTest(origin_, RRType::NS(), ZoneFinder::SUCCESS, false,
              ConstRRsetPtr(), ZoneFinder::RESULT_DEFAULT, &finder2);
-    findTest(other_origin, RRType::TXT(), ZoneFinder::NXDOMAIN, false,
-             ConstRRsetPtr(), ZoneFinder::RESULT_DEFAULT, &finder2);
+    EXPECT_THROW(finder2.find(other_origin, RRType::TXT()), OutOfZone);
 }
 
 TEST_F(InMemoryZoneFinderTest, getFileName) {
@@ -1971,10 +1969,8 @@ TEST_F(InMemoryZoneFinderTest, findNSEC3) {
     EXPECT_EQ(result::SUCCESS, zone_finder_.add(textToRRset(zzz_nsec3_text)));
 
     // Parameter validation: the query name must be in or below the zone
-    EXPECT_THROW(zone_finder_.findNSEC3(Name("example.com"), false),
-                 isc::InvalidParameter);
-    EXPECT_THROW(zone_finder_.findNSEC3(Name("org"), true),
-                 isc::InvalidParameter);
+    EXPECT_THROW(zone_finder_.findNSEC3(Name("example.com"), false), OutOfZone);
+    EXPECT_THROW(zone_finder_.findNSEC3(Name("org"), true), OutOfZone);
 
     // Apex name.  It should have a matching NSEC3.
     {

src/lib/datasrc/zone.h

@@ -27,6 +27,16 @@
 namespace isc {
 namespace datasrc {
 
+/// \brief Out of zone exception
+///
+/// This is thrown when a method is called for a name or RRset which
+/// is not in or below the zone.
+class OutOfZone : public Exception {
+public:
+    OutOfZone(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
 /// \brief The base class to search a zone for RRsets
 ///
 /// The \c ZoneFinder class is an abstract base class for representing
@@ -466,6 +476,8 @@ public:
     ///
     /// \exception std::bad_alloc Memory allocation such as for constructing
     ///  the resulting RRset fails
+    /// \throw OutOfZone The Name \c name is outside of the origin of the
+    /// zone of this ZoneFinder.
     /// \exception DataSourceError Derived class specific exception, e.g.
     /// when encountering a bad zone configuration or database connection
     /// failure.  Although these are considered rare, exceptional events,
@@ -589,7 +601,7 @@ public:
     /// algorithm, and salt) from the zone as noted above.  If these
     /// assumptions aren't met, \c DataSourceError exception will be thrown.
     ///
-    /// \exception InvalidParameter name is not a subdomain of the zone origin
+    /// \exception OutOfZone name is not a subdomain of the zone origin
     /// \exception DataSourceError Low-level or internal datasource errors
     /// happened, or the zone isn't properly signed with NSEC3
     /// (NSEC3 parameters cannot be found, no NSEC3s are available, etc).

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

@@ -233,6 +233,7 @@ initModulePart_ZoneJournalReader(PyObject* mod) {
 }
 
 PyObject* po_DataSourceError;
+PyObject* po_OutOfZone;
 PyObject* po_NotImplemented;
 
 PyModuleDef iscDataSrc = {
@@ -287,6 +288,9 @@ PyInit_datasrc(void) {
         po_DataSourceError = PyErr_NewException("isc.datasrc.Error", NULL,
                                                 NULL);
         PyObjectContainer(po_DataSourceError).installToModule(mod, "Error");
+        po_OutOfZone = PyErr_NewException("isc.datasrc.OutOfZone", NULL,
+                                          NULL);
+        PyObjectContainer(po_OutOfZone).installToModule(mod, "OutOfZone");
         po_NotImplemented = PyErr_NewException("isc.datasrc.NotImplemented",
                                                NULL, NULL);
         PyObjectContainer(po_NotImplemented).installToModule(mod,

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

@@ -99,10 +99,9 @@ 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\
+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 an\n\
+OutOfZoneFind exception is raised.\n\
 \n\
 Note: For this reason it's probably better to throw an exception than\n\
 returning NXDOMAIN. This point should be revisited in a near future\n\

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

@@ -97,6 +97,9 @@ PyObject* ZoneFinder_helper(ZoneFinder* finder, PyObject* args) {
             } else {
                 return (Py_BuildValue("IOI", r, Py_None, result_flags));
             }
+        } catch (const OutOfZone& ooz) {
+            PyErr_SetString(getDataSourceException("OutOfZone"), ooz.what());
+            return (NULL);
         } catch (const DataSourceError& dse) {
             PyErr_SetString(getDataSourceException("Error"), dse.what());
             return (NULL);

src/lib/python/isc/datasrc/tests/datasrc_test.py

@@ -379,11 +379,10 @@ class DataSrcClient(unittest.TestCase):
         self.assertEqual(finder.NXDOMAIN, result)
         self.assertEqual(None, rrset)
 
-        result, rrset, _ = finder.find(isc.dns.Name("www.some.other.domain"),
-                                       isc.dns.RRType.A(),
-                                       finder.FIND_DEFAULT)
-        self.assertEqual(finder.NXDOMAIN, result)
-        self.assertEqual(None, rrset)
+
+        self.assertRaises(isc.datasrc.OutOfZone, finder.find,
+                          isc.dns.Name("www.some.other.domain"),
+                          isc.dns.RRType.A())
 
         result, rrset, _ = finder.find(isc.dns.Name("www.example.com"),
                                        isc.dns.RRType.TXT(),

src/lib/python/isc/notify/notify_out.py

@@ -278,12 +278,12 @@ class NotifyOut:
         # data sources.
         datasrc_config = '{ "database_file": "' + self._db_file + '"}'
         try:
-            result, finder = DataSourceClient('sqlite3',
-                                              datasrc_config).find_zone(
-                zone_name)
+            ds_client = DataSourceClient('sqlite3', datasrc_config)
         except isc.datasrc.Error as ex:
             logger.error(NOTIFY_OUT_DATASRC_ACCESS_FAILURE, ex)
             return []
+
+        result, finder = ds_client.find_zone(zone_name)
         if result is not DataSourceClient.SUCCESS:
             logger.error(NOTIFY_OUT_DATASRC_ZONE_NOT_FOUND,
                          format_zone_str(zone_name, zone_class))
@@ -307,13 +307,17 @@ class NotifyOut:
             ns_name = Name(ns_rdata.to_text())
             if soa_mname == ns_name:
                 continue
-            result, rrset, _ = finder.find(ns_name, RRType.A())
-            if result is finder.SUCCESS and rrset is not None:
-                addrs.extend([a.to_text() for a in rrset.get_rdata()])
-
-            result, rrset, _ = finder.find(ns_name, RRType.AAAA())
-            if result is finder.SUCCESS and rrset is not None:
-                addrs.extend([aaaa.to_text() for aaaa in rrset.get_rdata()])
+            ns_result, ns_finder = ds_client.find_zone(ns_name)
+            if ns_result is DataSourceClient.SUCCESS or \
+               ns_result is DataSourceClient.PARTIALMATCH:
+                result, rrset, _ = ns_finder.find(ns_name, RRType.A())
+                if result is ns_finder.SUCCESS and rrset is not None:
+                    addrs.extend([a.to_text() for a in rrset.get_rdata()])
+
+                result, rrset, _ = ns_finder.find(ns_name, RRType.AAAA())
+                if result is ns_finder.SUCCESS and rrset is not None:
+                    addrs.extend([aaaa.to_text()
+                                    for aaaa in rrset.get_rdata()])
 
         return addrs