From f8c0d29e25d71f77314ffc997e678c7b24c34ca5 Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Thu, 29 Mar 2012 14:49:15 +0200 Subject: [PATCH] [1535] make ZoneFinder::find throw on out-of-zone query Added new OutOfZoneFind exception, which is thrown (instead of NXDOMAIN result) when zonefinder::find() is called for an out-of-zone name (normally this should not happen; the zonefinder object is itself the result of a call to findZone() --- src/lib/datasrc/database.cc | 3 +- src/lib/datasrc/memory_datasrc.cc | 9 ++++ src/lib/datasrc/tests/database_unittest.cc | 52 +++++++++---------- .../datasrc/tests/memory_datasrc_unittest.cc | 24 +++++---- src/lib/datasrc/zone.h | 10 ++++ src/lib/python/isc/datasrc/datasrc.cc | 4 ++ src/lib/python/isc/datasrc/finder_python.cc | 4 ++ .../python/isc/datasrc/tests/datasrc_test.py | 9 ++-- 8 files changed, 72 insertions(+), 43 deletions(-) diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc index 4e2fb157e8..6a364bdd39 100644 --- a/src/lib/datasrc/database.cc +++ b/src/lib/datasrc/database.cc @@ -865,7 +865,8 @@ 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(OutOfZoneFind, "out-of-zone find(): " << name.toText() << + " not in " << getOrigin().toText()); } // First, go through all superdomains from the origin down, searching for diff --git a/src/lib/datasrc/memory_datasrc.cc b/src/lib/datasrc/memory_datasrc.cc index a3a2110b05..70986b4d74 100644 --- a/src/lib/datasrc/memory_datasrc.cc +++ b/src/lib/datasrc/memory_datasrc.cc @@ -1202,6 +1202,15 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl { LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_FIND).arg(name). arg(type); + const NameComparisonResult::NameRelation reln = + name.compare(origin_).getRelation(); + if (reln != NameComparisonResult::SUBDOMAIN && + reln != NameComparisonResult::EQUAL) { + isc_throw(OutOfZoneFind, "out-of-zone find(): " << + name.toText() << + " not in " << origin_.toText()); + } + // Get the node. All other cases than an exact match are handled // in findNode(). We simply construct a result structure and return. const ZoneData::FindNodeResult node_result = diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc index 8ffd3c7e4d..c30aa5cf37 100644 --- a/src/lib/datasrc/tests/database_unittest.cc +++ b/src/lib/datasrc/tests/database_unittest.cc @@ -1877,33 +1877,31 @@ TYPED_TEST(DatabaseClientTest, findOutOfZone) { vector 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_, + ZoneFinder::FIND_DEFAULT), OutOfZoneFind); + EXPECT_THROW(finder->findAll(Name("org"), target), OutOfZoneFind); + // 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_, + ZoneFinder::FIND_DEFAULT), OutOfZoneFind); + EXPECT_THROW(finder->findAll(Name("noexample.org"), target), + OutOfZoneFind); + // 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_, + ZoneFinder::FIND_DEFAULT), OutOfZoneFind); + EXPECT_THROW(finder->findAll(Name("com"), target), OutOfZoneFind); + // 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_, + ZoneFinder::FIND_DEFAULT), OutOfZoneFind); + EXPECT_THROW(finder->findAll(Name("example.com"), target), OutOfZoneFind); + // 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_, + ZoneFinder::FIND_DEFAULT), OutOfZoneFind); + EXPECT_THROW(finder->findAll(Name("more.example.com"), target), + OutOfZoneFind); } TYPED_TEST(DatabaseClientTest, findDelegation) { @@ -2835,10 +2833,10 @@ TYPED_TEST(DatabaseClientTest, addDeviantRR) { // 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_, + ZoneFinder::FIND_DEFAULT), + OutOfZoneFind); } } diff --git a/src/lib/datasrc/tests/memory_datasrc_unittest.cc b/src/lib/datasrc/tests/memory_datasrc_unittest.cc index 9096a9e0fb..b85e6cbe44 100644 --- a/src/lib/datasrc/tests/memory_datasrc_unittest.cc +++ b/src/lib/datasrc/tests/memory_datasrc_unittest.cc @@ -899,8 +899,9 @@ TEST_F(InMemoryZoneFinderTest, findAny) { findAllTest(origin_, ZoneFinder::SUCCESS, expected_sets); // out zone name - findAllTest(Name("example.com"), ZoneFinder::NXDOMAIN, - vector()); + EXPECT_THROW(findAllTest(Name("example.com"), ZoneFinder::NXDOMAIN, + vector()), + OutOfZoneFind); 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(), + ZoneFinder::FIND_DEFAULT), OutOfZoneFind); } TEST_F(InMemoryZoneFinderTest, find) { @@ -1053,8 +1054,9 @@ 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(), + ZoneFinder::FIND_DEFAULT), + OutOfZoneFind); } TEST_F(InMemoryZoneFinderTest, emptyNode) { @@ -1512,14 +1514,16 @@ 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(), + ZoneFinder::FIND_DEFAULT), + OutOfZoneFind); 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(), + ZoneFinder::FIND_DEFAULT), + OutOfZoneFind); } TEST_F(InMemoryZoneFinderTest, getFileName) { diff --git a/src/lib/datasrc/zone.h b/src/lib/datasrc/zone.h index c705279a95..b1c4825b86 100644 --- a/src/lib/datasrc/zone.h +++ b/src/lib/datasrc/zone.h @@ -27,6 +27,14 @@ namespace isc { namespace datasrc { +/// \brief Thrown when ZoneFinder::find() is called for out-of-zone data +/// +class OutOfZoneFind : public Exception { +public: + OutOfZoneFind(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 +474,8 @@ public: /// /// \exception std::bad_alloc Memory allocation such as for constructing /// the resulting RRset fails + /// \throw OutOfZoneFind 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, diff --git a/src/lib/python/isc/datasrc/datasrc.cc b/src/lib/python/isc/datasrc/datasrc.cc index 8ee06b701a..8da487a407 100644 --- a/src/lib/python/isc/datasrc/datasrc.cc +++ b/src/lib/python/isc/datasrc/datasrc.cc @@ -233,6 +233,7 @@ initModulePart_ZoneJournalReader(PyObject* mod) { } PyObject* po_DataSourceError; +PyObject* po_OutOfZoneFind; 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_OutOfZoneFind = PyErr_NewException("isc.datasrc.OutOfZoneFind", + NULL, NULL); + PyObjectContainer(po_OutOfZoneFind).installToModule(mod, "OutOfZoneFind"); po_NotImplemented = PyErr_NewException("isc.datasrc.NotImplemented", NULL, NULL); PyObjectContainer(po_NotImplemented).installToModule(mod, diff --git a/src/lib/python/isc/datasrc/finder_python.cc b/src/lib/python/isc/datasrc/finder_python.cc index 33a503f419..84967d9fe1 100644 --- a/src/lib/python/isc/datasrc/finder_python.cc +++ b/src/lib/python/isc/datasrc/finder_python.cc @@ -97,6 +97,10 @@ PyObject* ZoneFinder_helper(ZoneFinder* finder, PyObject* args) { } else { return (Py_BuildValue("IOI", r, Py_None, result_flags)); } + } catch (const OutOfZoneFind& oozf) { + PyErr_SetString(getDataSourceException("OutOfZoneFind"), + oozf.what()); + return (NULL); } catch (const DataSourceError& dse) { PyErr_SetString(getDataSourceException("Error"), dse.what()); return (NULL); diff --git a/src/lib/python/isc/datasrc/tests/datasrc_test.py b/src/lib/python/isc/datasrc/tests/datasrc_test.py index a6f8f1639b..f12339b72d 100644 --- a/src/lib/python/isc/datasrc/tests/datasrc_test.py +++ b/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.OutOfZoneFind, finder.find, + isc.dns.Name("www.some.other.domain"), + isc.dns.RRType.A(), finder.FIND_DEFAULT) result, rrset, _ = finder.find(isc.dns.Name("www.example.com"), isc.dns.RRType.TXT(),