From: Jelte Jansen Date: Wed, 22 Aug 2012 07:21:59 +0000 (+0200) Subject: [2178] better error log on masterfile parse problems X-Git-Tag: trac2351_base~118^2~1^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d99645b48e66cee78ea12b8e1d62efd8f07e14bc;p=thirdparty%2Fkea.git [2178] better error log on masterfile parse problems - add zone and file names to masterload errors - catch masterload error and log it - don't quit auth completely on a single zone load failure --- diff --git a/src/lib/datasrc/client_list.cc b/src/lib/datasrc/client_list.cc index a50a752d18..e8df356d37 100644 --- a/src/lib/datasrc/client_list.cc +++ b/src/lib/datasrc/client_list.cc @@ -17,6 +17,7 @@ #include "factory.h" #include "memory_datasrc.h" #include "logger.h" +#include #include #include @@ -139,28 +140,34 @@ ConfigurableClientList::configure(const ConstElementPtr& config, client(new_data_sources.back().data_src_client_); for (vector::const_iterator it(zones_origins.begin()); it != zones_origins.end(); ++it) { - const Name origin(*it); - shared_ptr - finder(new - InMemoryZoneFinder(rrclass_, origin)); - if (type == "MasterFiles") { - finder->load(paramConf->get(*it)->stringValue()); - } else { - ZoneIteratorPtr iterator; - try { - iterator = client->getIterator(origin); + try { + const Name origin(*it); + shared_ptr + finder(new + InMemoryZoneFinder(rrclass_, origin)); + if (type == "MasterFiles") { + finder->load(paramConf->get(*it)->stringValue()); + } else { + ZoneIteratorPtr iterator; + try { + iterator = client->getIterator(origin); + } + catch (const DataSourceError&) { + isc_throw(ConfigurationError, "Unable to " + "cache non-existent zone " + << origin); + } + if (!iterator) { + isc_throw(isc::Unexpected, "Got NULL iterator " + "for zone " << origin); + } + finder->load(*iterator); } - catch (const DataSourceError&) { - isc_throw(ConfigurationError, "Unable to cache " - "non-existent zone " << origin); - } - if (!iterator) { - isc_throw(isc::Unexpected, "Got NULL iterator for " - "zone " << origin); - } - finder->load(*iterator); + cache->addZone(finder); + } catch (const isc::dns::MasterLoadError& mle) { + LOG_ERROR(logger, DATASRC_MASTERLOAD_ERROR) + .arg(mle.what()); } - cache->addZone(finder); } } } diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes index 6ae744e20e..6837476243 100644 --- a/src/lib/datasrc/datasrc_messages.mes +++ b/src/lib/datasrc/datasrc_messages.mes @@ -305,6 +305,11 @@ Therefore, the zone will not be available for this process. If this is a problem, you should move the zone to some database backend (sqlite3, for example) and use it from there. +% DATASRC_MASTERLOAD_ERROR %1 +An error was found in the zone data for a MasterFiles zone. The zone +is not loaded. The specific error is shown in the message, and should +be addressed. + % DATASRC_MEM_ADD_RRSET adding RRset '%1/%2' into zone '%3' Debug information. An RRset is being added to the in-memory data source. diff --git a/src/lib/datasrc/tests/client_list_unittest.cc b/src/lib/datasrc/tests/client_list_unittest.cc index 64eb347ea2..8b1d4746fe 100644 --- a/src/lib/datasrc/tests/client_list_unittest.cc +++ b/src/lib/datasrc/tests/client_list_unittest.cc @@ -786,6 +786,28 @@ TEST_F(ListTest, masterFiles) { EXPECT_EQ(0, list_->getDataSources().size()); } +TEST_F(ListTest, BadMasterFile) { + // Configure one zone correctly, and one with the wrong origin + // (resulting in out-of-zone data) + // Configuration should succeed, and the correct zone should + // be loaded. + const ConstElementPtr elem(Element::fromJSON("[" + "{" + " \"type\": \"MasterFiles\"," + " \"cache-enable\": true," + " \"params\": {" + " \"foo.bar.\": \"" TEST_DATA_DIR "/example.org\"," + " \".\": \"" TEST_DATA_DIR "/root.zone\"" + " }" + "}]")); + list_->configure(elem, true); + + EXPECT_TRUE(negativeResult_ == list_->find(Name("example.org."), true)); + EXPECT_TRUE(negativeResult_ == list_->find(Name("foo.bar"), true)); + positiveResult(list_->find(Name(".")), ds_[0], Name("."), true, "com", + true); +} + // Test we can reload a zone TEST_F(ListTest, reloadSuccess) { list_->configure(config_elem_zones_, true); diff --git a/src/lib/dns/masterload.cc b/src/lib/dns/masterload.cc index 3deae194af..53a2b8de8c 100644 --- a/src/lib/dns/masterload.cc +++ b/src/lib/dns/masterload.cc @@ -75,23 +75,28 @@ masterLoad(const char* const filename, const Name& origin, isc_throw(MasterLoadError, "Failed to open master file: " << filename << ": " << strerror(errno)); } - masterLoad(ifs, origin, zone_class, callback); + masterLoad(ifs, origin, zone_class, callback, filename); ifs.close(); } void masterLoad(istream& input, const Name& origin, const RRClass& zone_class, - MasterLoadCallback callback) + MasterLoadCallback callback, const char* source) { RRsetPtr rrset; ConstRdataPtr prev_rdata; // placeholder for special case of RRSIGs string line; unsigned int line_count = 1; + if (source == NULL) { + source = ""; + } + do { getline(input, line); if (input.bad() || (input.fail() && !input.eof())) { - isc_throw(MasterLoadError, "Unexpectedly failed to read a line"); + isc_throw(MasterLoadError, "Unexpectedly failed to read a line in " + << source); } // blank/comment lines should be simply skipped. @@ -102,7 +107,8 @@ masterLoad(istream& input, const Name& origin, const RRClass& zone_class, // The line shouldn't have leading space (which means omitting the // owner name). if (isspace(line[0])) { - isc_throw(MasterLoadError, "Leading space at line " << line_count); + isc_throw(MasterLoadError, "Leading space in " << source + << " at line " << line_count); } // Parse a single RR @@ -111,15 +117,15 @@ masterLoad(istream& input, const Name& origin, const RRClass& zone_class, stringbuf rdatabuf; iss >> owner_txt >> ttl_txt >> rrclass_txt >> rrtype_txt >> &rdatabuf; if (iss.bad() || iss.fail()) { - isc_throw(MasterLoadError, "Parse failure for a valid RR at line " - << line_count); + isc_throw(MasterLoadError, "Parse failure for a valid RR in " + << source << " at line " << line_count); } // This simple version doesn't support relative owner names with a // separate origin. if (owner_txt.empty() || *(owner_txt.end() - 1) != '.') { - isc_throw(MasterLoadError, "Owner name is not absolute at line " - << line_count); + isc_throw(MasterLoadError, "Owner name is not absolute in " + << source << " at line " << line_count); } // XXX: this part is a bit tricky (and less efficient). We are going @@ -151,8 +157,9 @@ masterLoad(istream& input, const Name& origin, const RRClass& zone_class, rdata = createRdata(*rrtype, *rrclass, stripLine(rdtext, ex)); } } catch (const Exception& ex) { - isc_throw(MasterLoadError, "Invalid RR text at line " << line_count - << ": " << ex.what()); + isc_throw(MasterLoadError, "Invalid RR text in " << source << + " at line " << line_count << ": " + << ex.what()); } // Origin related validation: @@ -161,20 +168,22 @@ masterLoad(istream& input, const Name& origin, const RRClass& zone_class, const NameComparisonResult cmp_result = owner->compare(origin); if (cmp_result.getRelation() != NameComparisonResult::EQUAL && cmp_result.getRelation() != NameComparisonResult::SUBDOMAIN) { - isc_throw(MasterLoadError, "Out-of-zone data at line " - << line_count); + isc_throw(MasterLoadError, "Out-of-zone data for " << origin + << "/" << rrclass_txt << " in " + << source << " at line " + << line_count); } if (*rrtype == RRType::SOA() && cmp_result.getRelation() != NameComparisonResult::EQUAL) { - isc_throw(MasterLoadError, "SOA not at top of zone at line " - << line_count); + isc_throw(MasterLoadError, "SOA not at top of zone in " + << source << " at line " << line_count); } // Reject RR class mismatching if (*rrclass != zone_class) { isc_throw(MasterLoadError, "RR class (" << rrclass_txt << ") does not match the zone class (" << zone_class - << ") at line " << line_count); + << ") in " << source << " at line " << line_count); } // Everything is okay. Now create/update RRset with the new RR. @@ -209,5 +218,6 @@ masterLoad(istream& input, const Name& origin, const RRClass& zone_class, callback(rrset); } } + } // namespace dns } // namespace isc diff --git a/src/lib/dns/masterload.h b/src/lib/dns/masterload.h index 41d145b150..d4c2447c3e 100644 --- a/src/lib/dns/masterload.h +++ b/src/lib/dns/masterload.h @@ -232,9 +232,15 @@ void masterLoad(const char* const filename, const Name& origin, /// \param zone_class The RR class of the zone. /// \param callback A callback functor or function that is to be called for /// each RRset. +/// \param source A string to use in error messages if zone content is bad +/// (e.g. the file name when reading from a file). If this value is NULL, +/// or left out, the error will use the string '' void masterLoad(std::istream& input, const Name& origin, - const RRClass& zone_class, MasterLoadCallback callback); + const RRClass& zone_class, MasterLoadCallback callback, + const char* source=NULL); } + + //@} }