From: JINMEI Tatuya Date: Tue, 21 May 2013 00:56:49 +0000 (-0700) Subject: [2905] handle the empty-zone case in InMemoryClient. X-Git-Tag: bind10-1.2.0beta1-release~434^2~13 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0ae219d8e8bfeacdfe026ad1e3576ab401d79724;p=thirdparty%2Fkea.git [2905] handle the empty-zone case in InMemoryClient. --- diff --git a/src/lib/datasrc/client.h b/src/lib/datasrc/client.h index ba10a2cb27..41457005e3 100644 --- a/src/lib/datasrc/client.h +++ b/src/lib/datasrc/client.h @@ -187,8 +187,12 @@ public: /// - \c result::PARTIALMATCH: A zone whose origin is a /// super domain of \c name is found (but there is no exact match) /// - \c result::NOTFOUND: For all other cases. + /// - \c flags: usually FLAGS_DEFAULT, but if the zone data are not + /// available (possibly because an error was detected at load time) + /// the ZONE_EMPTY flag is set. /// - \c zone_finder: Pointer to a \c ZoneFinder object for the found zone - /// if one is found; otherwise \c NULL. + /// if one is found and is not empty (flags doesn't have ZONE_EMPTY); + /// otherwise \c NULL. /// /// A specific derived version of this method may throw an exception. /// This interface does not specify which exceptions can happen (at least @@ -218,6 +222,9 @@ public: /// \throw Others Possibly implementation specific exceptions (it is /// not fixed if a concrete implementation of this method can throw /// anything else.) + /// \throw EmptyZone the zone is supposed to exist in the data source, + /// but its content is not available. This generally means there's an + /// error in the content. /// /// \param name The name of zone apex to be traversed. It doesn't do /// nearest match as findZone. diff --git a/src/lib/datasrc/exceptions.h b/src/lib/datasrc/exceptions.h index d3b25da95d..f3bc06f4db 100644 --- a/src/lib/datasrc/exceptions.h +++ b/src/lib/datasrc/exceptions.h @@ -56,6 +56,17 @@ public: DataSourceError(file, line, what) {} }; +/// \brief An error indicating a zone is recognized but its content is not +/// available. +/// +/// This generally indicates a condition that there's an error in the zone +/// content and it's not successfully loaded. +class EmptyZone : public DataSourceError { +public: + EmptyZone(const char* file, size_t line, const char* what) : + DataSourceError(file, line, what) {} +}; + /// Base class for a number of exceptions that are thrown while working /// with zones. struct ZoneException : public Exception { diff --git a/src/lib/datasrc/memory/memory_client.cc b/src/lib/datasrc/memory/memory_client.cc index a6342ded5e..0b0c827a8c 100644 --- a/src/lib/datasrc/memory/memory_client.cc +++ b/src/lib/datasrc/memory/memory_client.cc @@ -69,11 +69,11 @@ InMemoryClient::findZone(const isc::dns::Name& zone_name) const { const ZoneTable::FindResult result(zone_table->findZone(zone_name)); ZoneFinderPtr finder; - if (result.code != result::NOTFOUND) { + if (result.code != result::NOTFOUND && result.zone_data) { finder.reset(new InMemoryZoneFinder(*result.zone_data, getClass())); } - return (DataSourceClient::FindResult(result.code, finder)); + return (DataSourceClient::FindResult(result.code, finder, result.flags)); } const ZoneData* @@ -242,7 +242,12 @@ InMemoryClient::getIterator(const Name& name, bool separate_rrs) const { const ZoneTable* zone_table = ztable_segment_->getHeader().getTable(); const ZoneTable::FindResult result(zone_table->findZone(name)); if (result.code != result::SUCCESS) { - isc_throw(NoSuchZone, "No such zone: " + name.toText()); + isc_throw(NoSuchZone, "no such zone for in-memory iterator: " + << name.toText()); + } + if (!result.zone_data) { + isc_throw(EmptyZone, "empty zone for in-memory iterator: " + << name.toText()); } return (ZoneIteratorPtr(new MemoryIterator( diff --git a/src/lib/datasrc/memory/zone_table.cc b/src/lib/datasrc/memory/zone_table.cc index 638448815c..776a760723 100644 --- a/src/lib/datasrc/memory/zone_table.cc +++ b/src/lib/datasrc/memory/zone_table.cc @@ -162,9 +162,10 @@ ZoneTable::findZone(const Name& name) const { // Can Not Happen (remember, NOTFOUND is handled). node should also have // data because the tree is constructed in the way empty nodes would // be "invisible" for find(). - assert(node != NULL && node->getData()); + assert(node != NULL); const ZoneData* zone_data = node->getData(); + assert(zone_data); const result::ResultFlags flags = zone_data->isEmpty() ? result::ZONE_EMPTY : result::FLAGS_DEFAULT; return (FindResult(my_result, zone_data->isEmpty() ? NULL : zone_data, diff --git a/src/lib/datasrc/tests/memory/memory_client_unittest.cc b/src/lib/datasrc/tests/memory/memory_client_unittest.cc index 5f52a338d8..73862e3a8c 100644 --- a/src/lib/datasrc/tests/memory/memory_client_unittest.cc +++ b/src/lib/datasrc/tests/memory/memory_client_unittest.cc @@ -694,6 +694,15 @@ TEST_F(MemoryClientTest, getIterator) { EXPECT_THROW(iterator->getNextRRset(), isc::Unexpected); } +TEST_F(MemoryClientTest, getIteratorForEmptyZone) { + // trying to load a broken zone (zone file not existent). It's internally + // stored an empty zone. + loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_, + TEST_DATA_DIR "/no-such-file.zone", true); + // Then getIterator will result in an exception. + EXPECT_THROW(client_->getIterator(Name("example.org")), EmptyZone); +} + TEST_F(MemoryClientTest, getIteratorSeparateRRs) { loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_, TEST_DATA_DIR "/example.org-multiple.zone"); @@ -791,6 +800,35 @@ TEST_F(MemoryClientTest, addEmptyRRsetThrows) { // Teardown checks for memory segment leaks } +TEST_F(MemoryClientTest, findEmptyZone) { + // trying to load a broken zone (zone file not existent). It's internally + // stored an empty zone. + loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_, + TEST_DATA_DIR "/no-such-file.zone", true); + + using namespace isc::datasrc::result; + + // findZone() returns the match, with NULL zone finder and the result + // flag indicating it's empty. + const DataSourceClient::FindResult result = + client_->findZone(Name("example.org")); + EXPECT_EQ(SUCCESS, result.code); + EXPECT_EQ(ZONE_EMPTY, result.flags); + EXPECT_FALSE(result.zone_finder); + + // Same for the case of subdomain match + const DataSourceClient::FindResult result_sub = + client_->findZone(Name("www.example.org")); + EXPECT_EQ(PARTIALMATCH, result_sub.code); + EXPECT_EQ(ZONE_EMPTY, result_sub.flags); + EXPECT_FALSE(result_sub.zone_finder); + + // findZoneData() will simply NULL (this is for testing only anyway, + // so any result would be okay as long as it doesn't cause disruption). + EXPECT_EQ(static_cast(NULL), + client_->findZoneData(Name("example.org"))); +} + TEST_F(MemoryClientTest, findZoneData) { loadZoneIntoTable(*ztable_segment_, Name("example.org"), zclass_, TEST_DATA_DIR "/example.org-rrsigs.zone"); diff --git a/src/lib/datasrc/tests/memory/zone_loader_util.cc b/src/lib/datasrc/tests/memory/zone_loader_util.cc index 24b2373de3..155871d56c 100644 --- a/src/lib/datasrc/tests/memory/zone_loader_util.cc +++ b/src/lib/datasrc/tests/memory/zone_loader_util.cc @@ -33,7 +33,8 @@ namespace test { void loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname, - const dns::RRClass& zclass, const std::string& zone_file) + const dns::RRClass& zclass, const std::string& zone_file, + bool load_error_ok) { const isc::datasrc::internal::CacheConfig cache_conf( "MasterFiles", NULL, *data::Element::fromJSON( @@ -41,7 +42,7 @@ loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname, " \"params\": {\"" + zname.toText() + "\": \"" + zone_file + "\"}}"), true); memory::ZoneWriter writer(zt_sgmt, cache_conf.getLoadAction(zclass, zname), - zname, zclass, false); + zname, zclass, load_error_ok); writer.load(); writer.install(); writer.cleanup(); diff --git a/src/lib/datasrc/tests/memory/zone_loader_util.h b/src/lib/datasrc/tests/memory/zone_loader_util.h index 06eba87f64..6d1f764269 100644 --- a/src/lib/datasrc/tests/memory/zone_loader_util.h +++ b/src/lib/datasrc/tests/memory/zone_loader_util.h @@ -33,9 +33,14 @@ namespace test { /// This function does nothing special, simply provides a shortcut for commonly /// used pattern that would be used in tests with a ZoneTableSegment loading /// a zone from file into it. +/// +/// If the optional load_error_ok parameter is set to true, it will create +/// an internal empty zone in the table when it encounters a loading error. +/// Otherwise ZoneLoaderException will be thrown in such cases. void loadZoneIntoTable(ZoneTableSegment& zt_sgmt, const dns::Name& zname, - const dns::RRClass& zclass, const std::string& zone_file); + const dns::RRClass& zclass, const std::string& zone_file, + bool load_error_ok = false); /// \brief A shortcut utility to load a specified zone into ZoneTableSegment /// from a zone iterator.