BIND 10 master, updated. a7e16e85fac60062df06be2f371868571d08e5ed Merge #2428
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Dec 14 11:20:29 UTC 2012
The branch, master has been updated
via a7e16e85fac60062df06be2f371868571d08e5ed (commit)
via ea9d025cbcd9a318a2946c1a7f00283885ff381f (commit)
via 588ff1eef555b3362954296388e93cf8729f267b (commit)
via bb1f38e03fb01fb164a58d91bd96bc9fcf4c07df (commit)
via 58664e2ecc10b71b6ce0126ccf8c5f336f7cd380 (commit)
via 443cabc1e4a305f19b39bcbdb66c0bde010c93ae (commit)
via 3fda721ea7863c9e61728c6e2b7f499705ca9abf (commit)
via 041637fac582e9b2fd003b557e1d616ecbb0db9b (commit)
via 320f960c727b5785a1869fc2e9898fea6b9e14fd (commit)
via aae73017c63bcb2527bf36a3abb2c436072e3a98 (commit)
via 45e97b85271f437e9f3897fc78e82cbac8f2368a (commit)
via 33fb59eeb630ce1fb6055a6a8d26c46951fc9c78 (commit)
via de08d209347b6639bc28d1847344c80616a6cb87 (commit)
via 1b7273a397602ebd1d04d692f517941624d89839 (commit)
via c9bd919bd97e5fd2cf3f50470a29c119bd3b1151 (commit)
via e089de85aec2c705d7fee61f1a51e37aa2510756 (commit)
via 398a65223150685c4d022dd980d0292e8116afb1 (commit)
via e76b79a1b16a388113c66083f5363b085814cdab (commit)
from 8652d1d4c6e2f9538b20443eb0cd0f37672915ef (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 a7e16e85fac60062df06be2f371868571d08e5ed
Merge: 8652d1d ea9d025
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Fri Dec 14 10:36:17 2012 +0100
Merge #2428
$INCLUDE handling in the isc::dns::MasterLoader
Conflicts:
src/lib/dns/master_loader.cc
-----------------------------------------------------------------------
Summary of changes:
src/lib/dns/master_lexer.cc | 5 +
src/lib/dns/master_lexer.h | 5 +
src/lib/dns/master_loader.cc | 143 ++++++++++++++++++++++-----
src/lib/dns/tests/master_lexer_unittest.cc | 6 ++
src/lib/dns/tests/master_loader_unittest.cc | 104 +++++++++++++++++--
src/lib/dns/tests/testdata/Makefile.am | 1 +
src/lib/dns/tests/testdata/broken.zone | 3 +
7 files changed, 232 insertions(+), 35 deletions(-)
create mode 100644 src/lib/dns/tests/testdata/broken.zone
-----------------------------------------------------------------------
diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc
index b3b78c0..e6e2c78 100644
--- a/src/lib/dns/master_lexer.cc
+++ b/src/lib/dns/master_lexer.cc
@@ -149,6 +149,11 @@ MasterLexer::popSource() {
impl_->has_previous_ = false;
}
+size_t
+MasterLexer::getSourceCount() const {
+ return (impl_->sources_.size());
+}
+
std::string
MasterLexer::getSourceName() const {
if (impl_->sources_.empty()) {
diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h
index 99b433f..cdf2866 100644
--- a/src/lib/dns/master_lexer.h
+++ b/src/lib/dns/master_lexer.h
@@ -404,6 +404,11 @@ public:
/// \throw isc::InvalidOperation Called with no pushed source.
void popSource();
+ /// \brief Get number of sources inside the lexer.
+ ///
+ /// This method never throws.
+ size_t getSourceCount() const;
+
/// \brief Return the name of the current input source name.
///
/// If it's a file, it will be the C string given at the corresponding
diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc
index 941a100..0a3db39 100644
--- a/src/lib/dns/master_loader.cc
+++ b/src/lib/dns/master_loader.cc
@@ -22,13 +22,29 @@
#include <string>
#include <memory>
+#include <boost/algorithm/string/predicate.hpp> // for iequals
using std::string;
using std::auto_ptr;
+using boost::algorithm::iequals;
namespace isc {
namespace dns {
+namespace {
+
+// An internal exception, used to control the code flow in case of errors.
+// It is thrown during the loading and caught later, not to be propagated
+// outside of the file.
+class InternalException : public isc::Exception {
+public:
+ InternalException(const char* filename, size_t line, const char* what) :
+ Exception(filename, line, what)
+ {}
+};
+
+} // end unnamed namespace
+
class MasterLoader::MasterLoaderImpl {
public:
MasterLoaderImpl(const char* master_file,
@@ -69,9 +85,7 @@ public:
std::string error;
if (!lexer_.pushSource(filename.c_str(), &error)) {
if (initialized_) {
- // $INCLUDE file
- reportError(lexer_.getSourceName(), lexer_.getSourceLine(),
- error);
+ isc_throw(InternalException, error.c_str());
} else {
// Top-level file
reportError("", 0, error);
@@ -81,6 +95,14 @@ public:
initialized_ = true;
}
+ bool popSource() {
+ if (lexer_.getSourceCount() == 1) {
+ return (false);
+ }
+ lexer_.popSource();
+ return (true);
+ }
+
void pushStreamSource(std::istream& stream) {
lexer_.pushSource(stream);
initialized_ = true;
@@ -94,6 +116,74 @@ public:
bool loadIncremental(size_t count_limit);
+ void doInclude() {
+ // First, get the filename to include
+ const string
+ filename(lexer_.getNextToken(MasterToken::QSTRING).getString());
+
+ // There could be an origin (or maybe not). So try looking
+ const MasterToken name_tok(lexer_.getNextToken(MasterToken::QSTRING,
+ true));
+
+ if (name_tok.getType() == MasterToken::QSTRING ||
+ name_tok.getType() == MasterToken::STRING) {
+ // TODO: Handle the origin. Once we complete #2427.
+ } else {
+ // We return the newline there. This is because after we pop
+ // the source, we want to call eatUntilEOL and this would
+ // eat to the next one.
+ lexer_.ungetToken();
+ }
+
+ pushSource(filename);
+ }
+
+ void handleDirective(const char* directive, size_t length) {
+ if (iequals(directive, "INCLUDE")) {
+ doInclude();
+ } else if (iequals(directive, "ORIGIN")) {
+ // TODO: Implement
+ isc_throw(isc::NotImplemented,
+ "Origin directive not implemented yet");
+ } else if (iequals(directive, "TTL")) {
+ // TODO: Implement
+ isc_throw(isc::NotImplemented,
+ "TTL directive not implemented yet");
+ } else {
+ isc_throw(InternalException, "Unknown directive '" <<
+ string(directive, directive + length) << "'");
+ }
+ }
+
+ void eatUntilEOL(bool reportExtra) {
+ // We want to continue. Try to read until the end of line
+ for (;;) {
+ const MasterToken& token(lexer_.getNextToken());
+ switch (token.getType()) {
+ case MasterToken::END_OF_FILE:
+ callbacks_.warning(lexer_.getSourceName(),
+ lexer_.getSourceLine(),
+ "File does not end with newline");
+ // We don't pop here. The End of file will stay there,
+ // and we'll handle it in the next iteration of
+ // loadIncremental properly.
+ return;
+ case MasterToken::END_OF_LINE:
+ // Found the end of the line. Good.
+ return;
+ default:
+ // Some other type of token.
+ if (reportExtra) {
+ reportExtra = false;
+ reportError(lexer_.getSourceName(),
+ lexer_.getSourceLine(),
+ "Extra tokens at the end of line");
+ }
+ break;
+ }
+ }
+ }
+
private:
MasterLexer lexer_;
const Name zone_origin_;
@@ -133,8 +223,15 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
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);
+ if (!popSource()) {
+ return (true);
+ } else {
+ // We try to read a token from the popped source
+ // So retry the loop, but first, make sure the source
+ // is at EOL
+ eatUntilEOL(true);
+ continue;
+ }
}
empty = empty_token.getType() == MasterToken::END_OF_LINE;
} while (empty);
@@ -144,7 +241,19 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
const MasterToken::StringRegion&
name_string(lexer_.getNextToken(MasterToken::QSTRING).
getStringRegion());
- // TODO $ handling
+
+ if (name_string.len > 0 && name_string.beg[0] == '$') {
+ // This should have either thrown (and the error handler
+ // will read up until the end of line) or read until the
+ // end of line.
+
+ // Exclude the $ from the string on this point.
+ handleDirective(name_string.beg + 1, name_string.len - 1);
+ // So, get to the next line, there's nothing more interesting
+ // in this one.
+ continue;
+ }
+
const Name name(name_string.beg, name_string.len,
&zone_origin_);
// TODO: Some more flexibility. We don't allow omitting
@@ -199,27 +308,7 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
// 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:
- callbacks_.warning(lexer_.getSourceName(),
- lexer_.getSourceLine(),
- "File does not end with newline");
- // 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);
+ eatUntilEOL(false);
}
}
// When there was a fatal error and ok is false, we say we are done.
diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc
index b2415da..42ed3fb 100644
--- a/src/lib/dns/tests/master_lexer_unittest.cc
+++ b/src/lib/dns/tests/master_lexer_unittest.cc
@@ -60,8 +60,10 @@ TEST_F(MasterLexerTest, preOpen) {
}
TEST_F(MasterLexerTest, pushStream) {
+ EXPECT_EQ(0, lexer.getSourceCount());
lexer.pushSource(ss);
EXPECT_EQ(expected_stream_name, lexer.getSourceName());
+ EXPECT_EQ(1, lexer.getSourceCount());
// From the point of view of this test, we only have to check (though
// indirectly) getSourceLine calls InputSource::getCurrentLine. It should
@@ -70,18 +72,22 @@ TEST_F(MasterLexerTest, pushStream) {
// By popping it the stack will be empty again.
lexer.popSource();
+ EXPECT_EQ(0, lexer.getSourceCount());
checkEmptySource(lexer);
}
TEST_F(MasterLexerTest, pushFile) {
// We use zone file (-like) data, but in this test that actually doesn't
// matter.
+ EXPECT_EQ(0, lexer.getSourceCount());
EXPECT_TRUE(lexer.pushSource(TEST_DATA_SRCDIR "/masterload.txt"));
+ EXPECT_EQ(1, lexer.getSourceCount());
EXPECT_EQ(TEST_DATA_SRCDIR "/masterload.txt", lexer.getSourceName());
EXPECT_EQ(1, lexer.getSourceLine());
lexer.popSource();
checkEmptySource(lexer);
+ EXPECT_EQ(0, lexer.getSourceCount());
// If we give a non NULL string pointer, its content will be intact
// if pushSource succeeds.
diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc
index 70e3249..bfbf9fa 100644
--- a/src/lib/dns/tests/master_loader_unittest.cc
+++ b/src/lib/dns/tests/master_loader_unittest.cc
@@ -115,6 +115,14 @@ public:
compare(current->getRdataIterator()->getCurrent()));
}
+ void checkBasicRRs() {
+ checkRR("example.org", RRType::SOA(),
+ "ns1.example.org. admin.example.org. "
+ "1234 3600 1800 2419200 7200");
+ checkRR("example.org", RRType::NS(), "ns1.example.org.");
+ checkRR("www.example.org", RRType::A(), "192.0.2.1");
+ }
+
MasterLoaderCallbacks callbacks_;
boost::scoped_ptr<MasterLoader> loader_;
vector<string> errors_;
@@ -135,11 +143,60 @@ TEST_F(MasterLoaderTest, basicLoad) {
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("example.org", RRType::NS(), "ns1.example.org.");
- checkRR("www.example.org", RRType::A(), "192.0.2.1");
+ checkBasicRRs();
+}
+
+// Test the $INCLUDE directive
+TEST_F(MasterLoaderTest, include) {
+ // Test various cases of include
+ const char* includes[] = {
+ "$include",
+ "$INCLUDE",
+ "$Include",
+ "$InCluDe",
+ "\"$INCLUDE\"",
+ NULL
+ };
+ for (const char** include = includes; *include != NULL; ++include) {
+ SCOPED_TRACE(*include);
+
+ clear();
+ // Prepare input source that has the include and some more data
+ // below (to see it returns back to the original source).
+ const string include_str = string(*include) + " " +
+ TEST_DATA_SRCDIR + "/example.org\nwww 3600 IN AAAA 2001:db8::1\n";
+ stringstream ss(include_str);
+ setLoader(ss, Name("example.org."), RRClass::IN(),
+ MasterLoader::MANY_ERRORS);
+
+ loader_->load();
+ EXPECT_TRUE(loader_->loadedSucessfully());
+ EXPECT_TRUE(errors_.empty());
+ EXPECT_TRUE(warnings_.empty());
+
+ checkBasicRRs();
+ checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
+ }
+}
+
+// Test the source is correctly popped even after error
+TEST_F(MasterLoaderTest, popAfterError) {
+ const string include_str = "$include " TEST_DATA_SRCDIR
+ "/broken.zone\nwww 3600 IN AAAA 2001:db8::1\n";
+ stringstream ss(include_str);
+ // We don't test without MANY_ERRORS, we want to see what happens
+ // after the error.
+ setLoader(ss, Name("example.org."), RRClass::IN(),
+ MasterLoader::MANY_ERRORS);
+
+ loader_->load();
+ EXPECT_FALSE(loader_->loadedSucessfully());
+ EXPECT_EQ(1, errors_.size()); // For the broken RR
+ EXPECT_EQ(1, warnings_.size()); // For missing EOLN
+
+ // The included file doesn't contain anything usable, but the
+ // line after the include should be there.
+ checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
}
// Check it works the same when created based on a stream, not filename
@@ -223,6 +280,15 @@ struct ErrorCase {
{ "www 3600 IN \"A\" 192.0.2.1", "Quoted type" },
{ "unbalanced)paren 3600 IN A 192.0.2.1", "Token error 1" },
{ "www 3600 unbalanced)paren A 192.0.2.1", "Token error 2" },
+ // Check the unknown directive. The rest looks like ordinary RR,
+ // so we see the $ is actually special.
+ { "$UNKNOWN 3600 IN A 192.0.2.1", "Unknown $ directive" },
+ { "$INCLUD " TEST_DATA_SRCDIR "/example.org", "Include too short" },
+ { "$INCLUDES " TEST_DATA_SRCDIR "/example.org", "Include too long" },
+ { "$INCLUDE", "Missing include path" },
+ { "$INCLUDE /file/not/found", "Include file not found" },
+ { "$INCLUDE /file/not/found and here goes bunch of garbage",
+ "Include file not found and garbage at the end of line" },
{ NULL, NULL }
};
@@ -242,7 +308,7 @@ TEST_F(MasterLoaderTest, brokenZone) {
EXPECT_FALSE(loader_->loadedSucessfully());
EXPECT_THROW(loader_->load(), MasterLoaderError);
EXPECT_FALSE(loader_->loadedSucessfully());
- EXPECT_EQ(1, errors_.size()) << errors_[0];
+ EXPECT_EQ(1, errors_.size());
EXPECT_TRUE(warnings_.empty());
checkRR("example.org", RRType::SOA(), "ns1.example.org. "
@@ -273,15 +339,15 @@ TEST_F(MasterLoaderTest, brokenZone) {
{
SCOPED_TRACE("Error at EOF");
// This case is interesting only in the lenient mode.
- const string zoneEOF(prepareZone(ec->line, false));
clear();
+ const string zoneEOF(prepareZone(ec->line, false));
stringstream zone_stream(zoneEOF);
setLoader(zone_stream, Name("example.org."), RRClass::IN(),
MasterLoader::MANY_ERRORS);
EXPECT_FALSE(loader_->loadedSucessfully());
EXPECT_NO_THROW(loader_->load());
EXPECT_FALSE(loader_->loadedSucessfully());
- EXPECT_EQ(1, errors_.size());
+ EXPECT_EQ(1, errors_.size()) << errors_[0] << "\n" << errors_[1];
// The unexpected EOF warning
EXPECT_EQ(1, warnings_.size());
checkRR("example.org", RRType::SOA(), "ns1.example.org. "
@@ -291,6 +357,28 @@ TEST_F(MasterLoaderTest, brokenZone) {
}
}
+// Check that a garbage after the include generates an error, but not fatal
+// one (in lenient mode) and we can recover.
+TEST_F(MasterLoaderTest, includeWithGarbage) {
+ // Include an origin (example.org) because we expect it to be handled
+ // soon and we don't want it to break here.
+ const string include_str("$INCLUDE " TEST_DATA_SRCDIR
+ "/example.org example.org bunch of other stuff\n"
+ "www 3600 IN AAAA 2001:db8::1\n");
+ stringstream zone_stream(include_str);
+ setLoader(zone_stream, Name("example.org."), RRClass::IN(),
+ MasterLoader::MANY_ERRORS);
+
+ EXPECT_NO_THROW(loader_->load());
+ EXPECT_FALSE(loader_->loadedSucessfully());
+ ASSERT_EQ(1, errors_.size());
+ // It says something about extra tokens at the end
+ EXPECT_NE(string::npos, errors_[0].find("Extra"));
+ EXPECT_TRUE(warnings_.empty());
+ checkBasicRRs();
+ checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
+}
+
// Test the constructor rejects empty add callback.
TEST_F(MasterLoaderTest, emptyCallback) {
EXPECT_THROW(MasterLoader(TEST_DATA_SRCDIR "/example.org",
diff --git a/src/lib/dns/tests/testdata/Makefile.am b/src/lib/dns/tests/testdata/Makefile.am
index d312e07..9494117 100644
--- a/src/lib/dns/tests/testdata/Makefile.am
+++ b/src/lib/dns/tests/testdata/Makefile.am
@@ -171,6 +171,7 @@ EXTRA_DIST += tsig_verify4.spec tsig_verify5.spec tsig_verify6.spec
EXTRA_DIST += tsig_verify7.spec tsig_verify8.spec tsig_verify9.spec
EXTRA_DIST += tsig_verify10.spec
EXTRA_DIST += example.org
+EXTRA_DIST += broken.zone
.spec.wire:
$(PYTHON) $(top_builddir)/src/lib/util/python/gen_wiredata.py -o $@ $<
diff --git a/src/lib/dns/tests/testdata/broken.zone b/src/lib/dns/tests/testdata/broken.zone
new file mode 100644
index 0000000..70f4540
--- /dev/null
+++ b/src/lib/dns/tests/testdata/broken.zone
@@ -0,0 +1,3 @@
+; This should fail due to broken TTL
+; The file should _NOT_ end with EOLN
+broken. 3600X IN A 192.0.2.2 More data
\ No newline at end of file
More information about the bind10-changes
mailing list