BIND 10 trac2377, updated. 80de8b943cba0596032073ed443c573b341106af [2377] docs: Add forgotten update to the addRRCallback
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Dec 7 17:02:33 UTC 2012
The branch, trac2377 has been updated
via 80de8b943cba0596032073ed443c573b341106af (commit)
via 2f505102370abac68e4b5dd52c39aecbfda41852 (commit)
via 63cce9fa516b2a36e13515013d45e95407ffce36 (commit)
via e6898f852cbff7dc11eecb74bac36d632628c639 (commit)
via ec661eb924e484b0591e155302b1cca5ccda3234 (commit)
via ab0db5a96545dece318f354fcb3317d1358060eb (commit)
via 2edd97ec54d5a3df9e11e5e74188558606e2e3bd (commit)
via b70d80fca2146f8adca9b5c36e0806911097d2d2 (commit)
via 3a2ccbf78e869817a411d735e28425713c220ffa (commit)
via cc62eeca9e859daf93603bc2421f3f2988870d1c (commit)
via e5fb484fbabf454dc0fea14ed4564061555af275 (commit)
from 03911f1000649e0db11d1bbd956b2df5732a5a44 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 80de8b943cba0596032073ed443c573b341106af
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 18:01:23 2012 +0100
[2377] docs: Add forgotten update to the addRRCallback
commit 2f505102370abac68e4b5dd52c39aecbfda41852
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 17:58:26 2012 +0100
[2377] Keep a boolean variable for option
The option would be used often. While the produced code would probably
be the same, because the compiler can see it's equivalent, this is more
convenient to use.
commit 63cce9fa516b2a36e13515013d45e95407ffce36
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 17:49:15 2012 +0100
[2377] Minor code simplification
commit e6898f852cbff7dc11eecb74bac36d632628c639
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 17:46:48 2012 +0100
[2377] Don't accept quoted TTLs and such
commit ec661eb924e484b0591e155302b1cca5ccda3234
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 17:45:08 2012 +0100
[2377] Small optimizations
commit ab0db5a96545dece318f354fcb3317d1358060eb
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 17:29:33 2012 +0100
[2377] Move a method outside of the class definition
Just to make it easier for some editors to parse.
commit 2edd97ec54d5a3df9e11e5e74188558606e2e3bd
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 17:22:56 2012 +0100
[2377] Prepare MasterLoader::pushSource for future
When we support $INCLUDE, it should report reasonable place of error,
not :0.
commit b70d80fca2146f8adca9b5c36e0806911097d2d2
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 17:12:35 2012 +0100
[2377] #include just to make sure
commit 3a2ccbf78e869817a411d735e28425713c220ffa
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 17:10:27 2012 +0100
[2377] Report error on !MANY_ERRORS as exception
In this case, the user should be aware of the error as much as possible.
In case of MANY_ERRORS, we can't do this, exception is able to handle
just one error.
commit cc62eeca9e859daf93603bc2421f3f2988870d1c
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 16:01:22 2012 +0100
[2377] docs: Reference note about Lenient mode
commit e5fb484fbabf454dc0fea14ed4564061555af275
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 7 15:52:24 2012 +0100
[2377] Provide MasterLoader constructor from stream
Instead of file name. And use it for some of the tests, to avoid
juggling with files.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dns/master_loader.cc | 254 ++++++++++++++++-----------
src/lib/dns/master_loader.h | 36 +++-
src/lib/dns/master_loader_callbacks.h | 12 +-
src/lib/dns/tests/master_loader_unittest.cc | 68 ++++---
4 files changed, 244 insertions(+), 126 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc
index 581640b..ed5f019 100644
--- a/src/lib/dns/master_loader.cc
+++ b/src/lib/dns/master_loader.cc
@@ -20,6 +20,8 @@
#include <dns/rrtype.h>
#include <dns/rdata.h>
+#include <string>
+
using std::string;
namespace isc {
@@ -42,119 +44,51 @@ public:
master_file_(master_file),
initialized_(false),
ok_(true),
+ many_errors_((options & MANY_ERRORS) != 0),
complete_(false)
{}
+ void reportError(const std::string& filename, size_t line,
+ const std::string& reason)
+ {
+ callbacks_.error(filename, line, reason);
+ if (!many_errors_) {
+ // In case we don't have the lenient mode, every error is fatal
+ // and we throw
+ ok_ = false;
+ complete_ = true;
+ isc_throw(MasterLoaderError, reason.c_str());
+ }
+ }
+
void pushSource(const std::string& filename) {
std::string error;
if (!lexer_.pushSource(filename.c_str(), &error)) {
- ok_ = false;
- callbacks_.error("", 0, error);
+ if (initialized_) {
+ // $INCLUDE file
+ reportError(lexer_.getSourceName(), lexer_.getSourceLine(),
+ error);
+ } else {
+ // Top-level file
+ reportError("", 0, error);
+ ok_ = false;
+ }
}
+ initialized_ = true;
+ }
+
+ void pushStreamSource(std::istream& stream) {
+ lexer_.pushSource(stream);
+ initialized_ = true;
}
// Get a string token. Handle it as error if it is not string.
const string getString() {
- return (lexer_.getNextToken(MasterToken::QSTRING).getString());
+ lexer_.getNextToken(MasterToken::STRING).getString(string_token_);
+ return (string_token_);
}
- bool loadIncremental(size_t count_limit) {
- if (complete_) {
- isc_throw(isc::InvalidOperation,
- "Trying to load when already loaded");
- }
- if (!initialized_) {
- pushSource(master_file_);
- initialized_ = true;
- }
- size_t count = 0;
- while (ok_ && count < count_limit) {
- try {
- // Skip all EOLNs (empty lines) and finish on EOF
- bool empty = true;
- do {
- const MasterToken& empty_token(lexer_.getNextToken());
- if (empty_token.getType() == MasterToken::END_OF_FILE) {
- // TODO: Check if this is the last source, possibly pop
- return (true);
- }
- empty = empty_token.getType() == MasterToken::END_OF_LINE;
- } while (empty);
- // Return the last token, as it was not empty
- lexer_.ungetToken();
-
- const MasterToken::StringRegion
- name_string(lexer_.getNextToken(MasterToken::QSTRING).
- getStringRegion());
- // TODO $ handling
- const Name name(name_string.beg, name_string.len,
- &zone_origin_);
- // TODO: Some more flexibility. We don't allow omitting
- // anything yet
-
- // The parameters
- const RRTTL ttl(getString());
- const RRClass rrclass(getString());
- const RRType rrtype(getString());
-
- // TODO: Some more validation?
- if (rrclass != zone_class_) {
- // It doesn't really matter much what type of exception
- // we throw, we catch it just below.
- isc_throw(isc::BadValue, "Class mismatch: " << rrclass <<
- "vs. " << zone_class_);
- }
-
- const rdata::RdataPtr data(rdata::createRdata(rrtype, rrclass,
- lexer_,
- &zone_origin_,
- options_,
- callbacks_));
- // In case we get NULL, it means there was error creating
- // the Rdata. The errors should have been reported by
- // callbacks_ already. We need to decide if we want to continue
- // or not.
- if (data != rdata::RdataPtr()) {
- add_callback_(name, rrclass, rrtype, ttl, data);
-
- // Good, we loaded another one
- ++count;
- } else if ((options_ & MANY_ERRORS) == 0) {
- return (true);
- }
- } catch (const isc::Exception& e) {
- // TODO: Do we want to list expected exceptions here instead?
- callbacks_.error(lexer_.getSourceName(),
- lexer_.getSourceLine(),
- e.what());
- if ((options_ & MANY_ERRORS) != 0) {
- // We want to continue. Try to read until the end of line
- bool end = false;
- do {
- const MasterToken& token(lexer_.getNextToken());
- switch (token.getType()) {
- case MasterToken::END_OF_FILE:
- // TODO: Try pop in case this is not the only
- // source
- return (true);
- case MasterToken::END_OF_LINE:
- end = true;
- break;
- default:
- // Do nothing. This is just to make compiler
- // happy
- break;
- }
- } while (!end);
- } else {
- // We abort on first error. We are therefore done.
- return (true);
- }
- }
- }
- // When there was a fatal error and ok is false, we say we are done.
- return (!ok_);
- }
+ bool loadIncremental(size_t count_limit);
private:
MasterLexer lexer_;
@@ -164,12 +98,117 @@ private:
AddRRCallback add_callback_;
MasterLoader::Options options_;
const std::string master_file_;
+ std::string string_token_;
bool initialized_;
bool ok_;
+ const bool many_errors_;
public:
bool complete_;
};
+bool
+MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
+ if (complete_) {
+ isc_throw(isc::InvalidOperation,
+ "Trying to load when already loaded");
+ }
+ if (!initialized_) {
+ pushSource(master_file_);
+ }
+ size_t count = 0;
+ while (ok_ && count < count_limit) {
+ try {
+ // Skip all EOLNs (empty lines) and finish on EOF
+ bool empty = true;
+ do {
+ const MasterToken& empty_token(lexer_.getNextToken());
+ if (empty_token.getType() == MasterToken::END_OF_FILE) {
+ // TODO: Check if this is the last source, possibly pop
+ return (true);
+ }
+ empty = empty_token.getType() == MasterToken::END_OF_LINE;
+ } while (empty);
+ // Return the last token, as it was not empty
+ lexer_.ungetToken();
+
+ const MasterToken::StringRegion&
+ name_string(lexer_.getNextToken(MasterToken::QSTRING).
+ getStringRegion());
+ // TODO $ handling
+ const Name name(name_string.beg, name_string.len,
+ &zone_origin_);
+ // TODO: Some more flexibility. We don't allow omitting
+ // anything yet
+
+ // The parameters
+ const RRTTL ttl(getString());
+ const RRClass rrclass(getString());
+ const RRType rrtype(getString());
+
+ // TODO: Some more validation?
+ if (rrclass != zone_class_) {
+ // It doesn't really matter much what type of exception
+ // we throw, we catch it just below.
+ isc_throw(isc::BadValue, "Class mismatch: " << rrclass <<
+ "vs. " << zone_class_);
+ }
+
+ const rdata::RdataPtr data(rdata::createRdata(rrtype, rrclass,
+ lexer_,
+ &zone_origin_,
+ options_,
+ callbacks_));
+ // In case we get NULL, it means there was error creating
+ // the Rdata. The errors should have been reported by
+ // callbacks_ already. We need to decide if we want to continue
+ // or not.
+ if (data) {
+ add_callback_(name, rrclass, rrtype, ttl, data);
+
+ // Good, we loaded another one
+ ++count;
+ } else if (!many_errors_) {
+ ok_ = false;
+ complete_ = true;
+ // We don't have the exact error here, but it was reported
+ // by the error callback.
+ isc_throw(MasterLoaderError, "Invalid RR data");
+ }
+ } catch (const MasterLoaderError&) {
+ // This is a hack. We exclude the MasterLoaderError from the
+ // below case. Once we restrict the below to some smaller
+ // exception, we should remove this.
+ throw;
+ } catch (const isc::Exception& e) {
+ // TODO: Once we do #2518, catch only the DNSTextError here,
+ // not isc::Exception. The rest should be just simply
+ // propagated.
+ reportError(lexer_.getSourceName(), lexer_.getSourceLine(),
+ e.what());
+ // We want to continue. Try to read until the end of line
+ bool end = false;
+ do {
+ const MasterToken& token(lexer_.getNextToken());
+ switch (token.getType()) {
+ case MasterToken::END_OF_FILE:
+ // TODO: Try pop in case this is not the only
+ // source
+ return (true);
+ case MasterToken::END_OF_LINE:
+ end = true;
+ break;
+ default:
+ // Do nothing. This is just to make compiler
+ // happy
+ break;
+ }
+ } while (!end);
+ }
+ }
+ // When there was a fatal error and ok is false, we say we are done.
+ return (!ok_);
+}
+
MasterLoader::MasterLoader(const char* master_file,
const Name& zone_origin,
const RRClass& zone_class,
@@ -184,6 +223,21 @@ MasterLoader::MasterLoader(const char* master_file,
zone_class, callbacks, add_callback, options);
}
+MasterLoader::MasterLoader(std::istream& stream,
+ const Name& zone_origin,
+ const RRClass& zone_class,
+ const MasterLoaderCallbacks& callbacks,
+ const AddRRCallback& add_callback,
+ Options options)
+{
+ if (add_callback.empty()) {
+ isc_throw(isc::InvalidParameter, "Empty add RR callback");
+ }
+ impl_ = new MasterLoaderImpl("", zone_origin, zone_class, callbacks,
+ add_callback, options);
+ impl_->pushStreamSource(stream);
+}
+
MasterLoader::~MasterLoader() {
delete impl_;
}
diff --git a/src/lib/dns/master_loader.h b/src/lib/dns/master_loader.h
index bbe4ac1..659e885 100644
--- a/src/lib/dns/master_loader.h
+++ b/src/lib/dns/master_loader.h
@@ -25,6 +25,15 @@ namespace dns {
class Name;
class RRClass;
+/// \brief Error while loading by MasterLoader without specifying the
+/// MANY_ERRORS option.
+class MasterLoaderError : public isc::Exception {
+public:
+ MasterLoaderError(const char* file, size_t line, const char* what) :
+ Exception(file, line, what)
+ {}
+};
+
/// \brief A class able to load DNS master files
///
/// This class is able to produce a stream of RRs from a master file.
@@ -37,7 +46,8 @@ public:
/// \brief Options how the parsing should work.
enum Options {
DEFAULT = 0, ///< Nothing special.
- MANY_ERRORS = 1 ///< Lenient mode.
+ MANY_ERRORS = 1 ///< Lenient mode (see documentation of MasterLoader
+ /// constructor).
};
/// \brief Constructor
@@ -56,6 +66,10 @@ public:
/// \param zone_class The class of zone to be expected inside the
/// master file.
/// \param callbacks The callbacks by which it should report problems.
+ /// Usually, the callback carries a filename and line number of the
+ /// input where the problem happens. There's a special case of empty
+ /// filename and zero line in case the opening of the top-level master
+ /// file fails.
/// \param add_callback The callback which would be called with each
/// loaded RR.
/// \param options Options for the parsing, which is bitwise-or of
@@ -71,6 +85,20 @@ public:
const AddRRCallback& add_callback,
Options options = DEFAULT);
+ /// \brief Constructor from a stream
+ ///
+ /// This is a constructor very similar to the previous one. The only
+ /// difference is it doesn't take a filename, but an input stream
+ /// to read the data from. It is expected to be mostly used in tests,
+ /// but it is public as it may possibly be useful for other currently
+ /// unknown purposes.
+ MasterLoader(std::istream& input,
+ const Name& zone_origin,
+ const RRClass& zone_class,
+ const MasterLoaderCallbacks& callbacks,
+ const AddRRCallback& add_callback,
+ Options options = DEFAULT);
+
/// \brief Destructor
~MasterLoader();
@@ -85,6 +113,9 @@ public:
/// It returns true if the loading is done.
/// \throw isc::InvalidOperation when called after loading was done
/// already.
+ /// \throw MasterLoaderError when there's an error in the input master
+ /// file and the MANY_ERRORS is not specified. It never throws this
+ /// in case MANY_ERRORS is specified.
bool loadIncremental(size_t count_limit);
/// \brief Load everything
@@ -92,6 +123,9 @@ public:
/// This simply calls loadIncremental until the loading is done.
/// \throw isc::InvalidOperation when called after loading was done
/// already.
+ /// \throw MasterLoaderError when there's an error in the input master
+ /// file and the MANY_ERRORS is not specified. It never throws this
+ /// in case MANY_ERRORS is specified.
void load() {
while (!loadIncremental(1000)) { // 1000 = arbitrary largish number
// Body intentionally left blank
diff --git a/src/lib/dns/master_loader_callbacks.h b/src/lib/dns/master_loader_callbacks.h
index 44ddbfa..f572194 100644
--- a/src/lib/dns/master_loader_callbacks.h
+++ b/src/lib/dns/master_loader_callbacks.h
@@ -32,15 +32,17 @@ class Rdata;
typedef boost::shared_ptr<Rdata> RdataPtr;
}
-/// \brief Type of callback to add a RRset.
+/// \brief Type of callback to add a RR.
///
/// This type of callback is used by the loader to report another loaded
-/// RRset. The RRset is no longer preserved by the loader and is fully
+/// RR. The Rdata is no longer preserved by the loader and is fully
/// owned by the callback.
///
-/// \param RRset The rrset to add. It does not contain the accompanying
-/// RRSIG (if the zone is signed), they are reported with separate
-/// calls to the callback.
+/// \param name The domain name where the RR belongs.
+/// \param rrclass The class of the RR.
+/// \param rrtype Type of the RR.
+/// \param rrttl Time to live of the RR.
+/// \param rdata The actual carried data of the RR.
typedef boost::function<void(const Name& name, const RRClass& rrclass,
const RRType& rrtype, const RRTTL& rrttl,
const rdata::RdataPtr& rdata)>
diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc
index 8598b47..1fe9a02 100644
--- a/src/lib/dns/tests/master_loader_unittest.cc
+++ b/src/lib/dns/tests/master_loader_unittest.cc
@@ -26,13 +26,13 @@
#include <string>
#include <vector>
#include <list>
-#include <fstream>
+#include <sstream>
using namespace isc::dns;
using std::vector;
using std::string;
using std::list;
-using std::ofstream;
+using std::stringstream;
using std::endl;
namespace {
@@ -67,8 +67,8 @@ public:
rrsets_.push_back(rrset);
}
- void setLoader(const char* file, const Name& origin, const RRClass rrclass,
- const MasterLoader::Options options)
+ void setLoader(const char* file, const Name& origin,
+ const RRClass& rrclass, const MasterLoader::Options options)
{
loader_.reset(new MasterLoader(file, origin, rrclass, callbacks_,
boost::bind(&MasterLoaderTest::addRRset,
@@ -76,15 +76,22 @@ public:
options));
}
- void prepareBrokenZone(const string& filename, const string& line) {
- ofstream out(filename.c_str(),
- std::ios_base::out | std::ios_base::trunc);
- ASSERT_FALSE(out.fail());
- out << "example.org. 3600 IN SOA ns1.example.org. "
- "admin.example.org. 1234 3600 1800 2419200 7200" << endl;
- out << line << endl;
- out << "correct 3600 IN A 192.0.2.2" << endl;
- out.close();
+ void setLoader(std::istream& stream, const Name& origin,
+ const RRClass& rrclass, const MasterLoader::Options options)
+ {
+ loader_.reset(new MasterLoader(stream, origin, rrclass, callbacks_,
+ boost::bind(&MasterLoaderTest::addRRset,
+ this, _1, _2, _3, _4, _5),
+ options));
+ }
+
+ string prepareZone(const string& line) {
+ string result;
+ result += "example.org. 3600 IN SOA ns1.example.org. "
+ "admin.example.org. 1234 3600 1800 2419200 7200\n";
+ result += line + "\n";
+ result += "correct 3600 IN A 192.0.2.2\n";
+ return (result);
}
void clear() {
@@ -131,6 +138,21 @@ TEST_F(MasterLoaderTest, basicLoad) {
checkRR("www.example.org", RRType::A(), "192.0.2.1");
}
+// Check it works the same when created based on a stream, not filename
+TEST_F(MasterLoaderTest, streamConstructor) {
+ stringstream zone_stream(prepareZone(""));
+ setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+ MasterLoader::MANY_ERRORS);
+
+ loader_->load();
+
+ EXPECT_TRUE(errors_.empty());
+ EXPECT_TRUE(warnings_.empty());
+ checkRR("example.org", RRType::SOA(), "ns1.example.org. "
+ "admin.example.org. 1234 3600 1800 2419200 7200");
+ checkRR("correct.example.org", RRType::A(), "192.0.2.2");
+}
+
// Try loading data incrementally.
TEST_F(MasterLoaderTest, incrementalLoad) {
setLoader(TEST_DATA_SRCDIR "/example.org", Name("example.org."),
@@ -193,18 +215,18 @@ struct ErrorCase {
// Test a broken zone is handled properly. We test several problems,
// both in strict and lenient mode.
TEST_F(MasterLoaderTest, brokenZone) {
- const string filename(TEST_DATA_BUILDDIR "/broken.zone");
for (const ErrorCase* ec = error_cases; ec->line != NULL; ++ec) {
SCOPED_TRACE(ec->problem);
- prepareBrokenZone(filename, ec->line);
+ const string zone(prepareZone(ec->line));
{
SCOPED_TRACE("Strict mode");
clear();
- setLoader(filename.c_str(), Name("example.org."), RRClass::IN(),
+ stringstream zone_stream(zone);
+ setLoader(zone_stream, Name("example.org."), RRClass::IN(),
MasterLoader::DEFAULT);
- loader_->load();
- EXPECT_EQ(1, errors_.size());
+ EXPECT_THROW(loader_->load(), MasterLoaderError);
+ EXPECT_EQ(1, errors_.size()) << errors_[0];
EXPECT_TRUE(warnings_.empty());
checkRR("example.org", RRType::SOA(), "ns1.example.org. "
@@ -217,9 +239,10 @@ TEST_F(MasterLoaderTest, brokenZone) {
{
SCOPED_TRACE("Lenient mode");
clear();
- setLoader(filename.c_str(), Name("example.org."), RRClass::IN(),
+ stringstream zone_stream(zone);
+ setLoader(zone_stream, Name("example.org."), RRClass::IN(),
MasterLoader::MANY_ERRORS);
- loader_->load();
+ EXPECT_NO_THROW(loader_->load());
EXPECT_EQ(1, errors_.size());
EXPECT_TRUE(warnings_.empty());
checkRR("example.org", RRType::SOA(), "ns1.example.org. "
@@ -236,6 +259,11 @@ TEST_F(MasterLoaderTest, emptyCallback) {
EXPECT_THROW(MasterLoader(TEST_DATA_SRCDIR "/example.org",
Name("example.org"), RRClass::IN(), callbacks_,
AddRRCallback()), isc::InvalidParameter);
+ // And the same with the second constructor
+ stringstream ss("");
+ EXPECT_THROW(MasterLoader(ss, Name("example.org"), RRClass::IN(),
+ callbacks_, AddRRCallback()),
+ isc::InvalidParameter);
}
// Check it throws when we try to load after loading was complete.
More information about the bind10-changes
mailing list