BIND 10 master, updated. 9c7390b2e75ff45d42f18e8775998ec5513c0053 Merge #2427

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Dec 18 13:08:32 UTC 2012


The branch, master has been updated
       via  9c7390b2e75ff45d42f18e8775998ec5513c0053 (commit)
       via  b5904bd8eed8e36d1d6fd6d4305f687f53fe6733 (commit)
       via  bdc390376f18c7a858744eb9fb11827e792c6243 (commit)
       via  eda75ca3700979153bcdee08b7d97a10f14519f0 (commit)
       via  625f08940bce63bd70bd9d8dac67d003519a61f1 (commit)
       via  a435de0e81e1cde5ad40f5a8699252d31a2a39b7 (commit)
       via  f74f25e7b04afb0bb116fcb6023837e47f471a77 (commit)
       via  f8f73869c12d0cc0007618e91b943feab7073840 (commit)
       via  90094d0add9f2047b2dbd307f800d4fb9ccccd73 (commit)
       via  dc3f31bea1c264125464906dae050a995829e816 (commit)
       via  471c6f2d6c1ad9b71422f1d19b6223492c5b6023 (commit)
       via  3adc5a448824ec9a819c52c89869855cf1de8d0c (commit)
       via  694dda8c2b939cf29d8949693f40dd12439fab0c (commit)
       via  23a5c8f5a97538d51389303ebef46164f95cd317 (commit)
       via  8e839a08dc0c54d88c6e5a93b2be00f8eb81c2f9 (commit)
       via  79b7e9ce53c8bf6fc47285795599630cc9dc3ef8 (commit)
       via  13f6a9e896862d5ca4aab33386a4c47c6636fe4d (commit)
       via  d0f80f2a66545a7247e97e7a17c26ae812349d26 (commit)
       via  a651d9b90dc0bc9737e6f104d1fde7f4b7c2110e (commit)
       via  a8c08c6be23f58204300bb2c916fcda7aef7b4c1 (commit)
       via  e47b08e8a356c1bf498aa5a615fcce86958f1fe0 (commit)
       via  8fd5cc452a30a35df0a24b7bd1c30e76ea6bb54d (commit)
      from  55e0fb24720f98d51eeff77af6bdc0dff8649fa4 (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 9c7390b2e75ff45d42f18e8775998ec5513c0053
Merge: 55e0fb2 b5904bd
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Dec 18 13:32:55 2012 +0100

    Merge #2427
    
    The $ORIGIN handling.
    
    Conflicts:
    	src/lib/dns/master_loader.cc
    	src/lib/dns/tests/master_loader_unittest.cc

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dns/master_lexer.cc                      |    4 +-
 src/lib/dns/master_lexer.h                       |    3 +-
 src/lib/dns/master_loader.cc                     |  226 ++++++++++++++-----
 src/lib/dns/tests/master_lexer_state_unittest.cc |    5 +-
 src/lib/dns/tests/master_lexer_unittest.cc       |   20 +-
 src/lib/dns/tests/master_loader_unittest.cc      |  250 ++++++++++++++++++++--
 src/lib/dns/tests/testdata/Makefile.am           |    2 +
 src/lib/dns/tests/testdata/example.org           |    2 +
 src/lib/dns/tests/testdata/omitcheck.txt         |    1 +
 src/lib/dns/tests/testdata/origincheck.txt       |    5 +
 10 files changed, 432 insertions(+), 86 deletions(-)
 create mode 100644 src/lib/dns/tests/testdata/omitcheck.txt
 create mode 100644 src/lib/dns/tests/testdata/origincheck.txt

-----------------------------------------------------------------------
diff --git a/src/lib/dns/master_lexer.cc b/src/lib/dns/master_lexer.cc
index e6e2c78..e4ddedc 100644
--- a/src/lib/dns/master_lexer.cc
+++ b/src/lib/dns/master_lexer.cc
@@ -37,7 +37,7 @@ using namespace master_lexer_internal;
 
 struct MasterLexer::MasterLexerImpl {
     MasterLexerImpl() : source_(NULL), token_(MasterToken::NOT_STARTED),
-                        paren_count_(0), last_was_eol_(false),
+                        paren_count_(0), last_was_eol_(true),
                         has_previous_(false),
                         previous_paren_count_(0),
                         previous_was_eol_(false)
@@ -127,6 +127,7 @@ MasterLexer::pushSource(const char* filename, std::string* error) {
 
     impl_->source_ = impl_->sources_.back().get();
     impl_->has_previous_ = false;
+    impl_->last_was_eol_ = true;
     return (true);
 }
 
@@ -135,6 +136,7 @@ MasterLexer::pushSource(std::istream& input) {
     impl_->sources_.push_back(InputSourcePtr(new InputSource(input)));
     impl_->source_ = impl_->sources_.back().get();
     impl_->has_previous_ = false;
+    impl_->last_was_eol_ = true;
 }
 
 void
diff --git a/src/lib/dns/master_lexer.h b/src/lib/dns/master_lexer.h
index cdf2866..1aa4255 100644
--- a/src/lib/dns/master_lexer.h
+++ b/src/lib/dns/master_lexer.h
@@ -54,7 +54,8 @@ public:
         END_OF_LINE, ///< End of line detected
         END_OF_FILE, ///< End of file detected
         INITIAL_WS,  ///< White spaces at the beginning of a line after an
-                     ///< end of line (if asked for detecting it)
+                     ///< end of line or at the beginning of file (if asked
+                     //   for detecting it)
         NOVALUE_TYPE_MAX = INITIAL_WS, ///< Max integer corresponding to
                                        /// no-value (type only) types.
                                        /// Mainly for internal use.
diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc
index 247b4c1..257e841 100644
--- a/src/lib/dns/master_loader.cc
+++ b/src/lib/dns/master_loader.cc
@@ -26,10 +26,16 @@
 
 #include <string>
 #include <memory>
+#include <vector>
+#include <boost/algorithm/string/predicate.hpp> // for iequals
+#include <boost/shared_ptr.hpp>
 
 using std::string;
 using std::auto_ptr;
+using std::vector;
+using std::pair;
 using boost::algorithm::iequals;
+using boost::shared_ptr;
 
 namespace isc {
 namespace dns {
@@ -58,6 +64,7 @@ public:
                      MasterLoader::Options options) :
         lexer_(),
         zone_origin_(zone_origin),
+        active_origin_(zone_origin),
         zone_class_(zone_class),
         callbacks_(callbacks),
         add_callback_(add_callback),
@@ -66,6 +73,7 @@ public:
         initialized_(false),
         ok_(true),
         many_errors_((options & MANY_ERRORS) != 0),
+        previous_name_(false),
         complete_(false),
         seen_error_(false),
         warn_rfc1035_ttl_(true)
@@ -82,7 +90,10 @@ public:
                 ok_ = false;
             }
         }
+        // Store the current status, so we can recover it upon popSource
+        include_info_.push_back(IncludeInfo(active_origin_, last_name_));
         initialized_ = true;
+        previous_name_ = false;
     }
 
     void pushStreamSource(std::istream& stream) {
@@ -112,6 +123,16 @@ private:
             return (false);
         }
         lexer_.popSource();
+        // Restore original origin and last seen name
+
+        // We move in tandem, there's an extra item included during the
+        // initialization, so we can never run out of them
+        assert(!include_info_.empty());
+        const IncludeInfo& info(include_info_.back());
+        active_origin_ = info.first;
+        last_name_ = info.second;
+        include_info_.pop_back();
+        previous_name_ = false;
         return (true);
     }
 
@@ -121,24 +142,47 @@ private:
         return (string_token_);
     }
 
-    void doInclude() {
-        // First, get the filename to include
-        const string
-            filename(lexer_.getNextToken(MasterToken::QSTRING).getString());
+    MasterToken handleInitialToken();
 
-        // There could be an origin (or maybe not). So try looking
-        const MasterToken name_tok(lexer_.getNextToken(MasterToken::QSTRING,
-                                                       true));
+    void doOrigin(bool is_optional) {
+        // Parse and create the new origin. It is relative to the previous
+        // one.
+        const MasterToken&
+            name_tok(lexer_.getNextToken(MasterToken::QSTRING, is_optional));
 
         if (name_tok.getType() == MasterToken::QSTRING ||
             name_tok.getType() == MasterToken::STRING) {
-            // TODO: Handle the origin. Once we complete #2427.
+
+            const MasterToken::StringRegion&
+                name_string(name_tok.getStringRegion());
+            active_origin_ = Name(name_string.beg, name_string.len,
+                                  &active_origin_);
+            if (name_string.len > 0 &&
+                name_string.beg[name_string.len - 1] != '.') {
+                callbacks_.warning(lexer_.getSourceName(),
+                                   lexer_.getSourceLine(),
+                                   "The new origin is relative, did you really"
+                                   " mean " + active_origin_.toText() + "?");
+            }
         } 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.
+            // If it is not optional, we must not get anything but
+            // a string token.
+            assert(is_optional);
+
+            // We return the newline there. This is because we want to
+            // behave the same if there is or isn't the name, leaving the
+            // newline there.
             lexer_.ungetToken();
         }
+    }
+
+    void doInclude() {
+        // First, get the filename to include
+        const string
+            filename(lexer_.getNextToken(MasterToken::QSTRING).getString());
+
+        // There optionally can be an origin, that applies before the include.
+        doOrigin(true);
 
         pushSource(filename);
     }
@@ -148,14 +192,13 @@ private:
     // specified before the RR type, it also recognizes and validates
     // them.  explicit_ttl will be set to true if this method finds a
     // valid TTL field.
-    RRType parseRRParams(bool& explicit_ttl) {
+    RRType parseRRParams(bool& explicit_ttl, MasterToken rrparam_token) {
         // Find TTL, class and type.  Both TTL and class are
         // optional and may occur in any order if they exist. TTL
         // and class come before type which must exist.
         //
         // [<TTL>] [<class>] <type> <RDATA>
         // [<class>] [<TTL>] <type> <RDATA>
-        MasterToken rrparam_token = lexer_.getNextToken(MasterToken::STRING);
 
         // named-signzone outputs TTL first, so try parsing it in order
         // first.
@@ -298,9 +341,8 @@ private:
         if (iequals(directive, "INCLUDE")) {
             doInclude();
         } else if (iequals(directive, "ORIGIN")) {
-            // TODO: Implement
-            isc_throw(isc::NotImplemented,
-                      "Origin directive not implemented yet");
+            doOrigin(false);
+            eatUntilEOL(true);
         } else if (iequals(directive, "TTL")) {
             setDefaultTTL(RRTTL(getString()), false);
             eatUntilEOL(true);
@@ -318,7 +360,7 @@ private:
                 case MasterToken::END_OF_FILE:
                     callbacks_.warning(lexer_.getSourceName(),
                                        lexer_.getSourceLine(),
-                                       "Unexpected end of file");
+                                       "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.
@@ -342,6 +384,9 @@ private:
 private:
     MasterLexer lexer_;
     const Name zone_origin_;
+    Name active_origin_; // The origin used during parsing
+                         // (modifiable by $ORIGIN)
+    shared_ptr<Name> last_name_; // Last seen name (for INITAL_WS handling)
     const RRClass zone_class_;
     MasterLoaderCallbacks callbacks_;
     AddRRCallback add_callback_;
@@ -357,6 +402,13 @@ private:
     bool ok_;                   // Is it OK to continue loading?
     const bool many_errors_;    // Are many errors allowed (or should we abort
                                 // on the first)
+    // Some info about the outer files from which we include.
+    // The first one is current origin, the second is the last seen name
+    // in that file.
+    typedef pair<Name, shared_ptr<Name> > IncludeInfo;
+    vector<IncludeInfo> include_info_;
+    bool previous_name_; // True if there was a previous name in this file
+                         // (false at the beginning or after an $INCLUDE line)
 public:
     bool complete_;             // All work done.
     bool seen_error_;           // Was there at least one error during the
@@ -365,6 +417,91 @@ public:
                                 // from the previous RR is used.
 };
 
+// A helper method of loadIncremental, parsing the first token of a new line.
+// If it looks like an RR, detect its owner name and return a string token for
+// the next field of the RR.
+// Otherwise, return either END_OF_LINE or END_OF_FILE token depending on
+// whether the loader continues to the next line or completes the load,
+// respectively.  Other corner cases including $-directive handling is done
+// here.
+// For unexpected errors, it throws an exception, which will be handled in
+// loadIncremental.
+MasterToken
+MasterLoader::MasterLoaderImpl::handleInitialToken() {
+    const MasterToken& initial_token =
+        lexer_.getNextToken(MasterLexer::QSTRING | MasterLexer::INITIAL_WS);
+
+    // The most likely case is INITIAL_WS, and then string/qstring.  We
+    // handle them first.
+    if (initial_token.getType() == MasterToken::INITIAL_WS) {
+        const MasterToken& next_token = lexer_.getNextToken();
+        if (next_token.getType() == MasterToken::END_OF_LINE) {
+            return (next_token); // blank line
+        } else if (next_token.getType() == MasterToken::END_OF_FILE) {
+            lexer_.ungetToken(); // handle it in the next iteration.
+            eatUntilEOL(true);  // effectively warn about the unexpected EOF.
+            return (MasterToken(MasterToken::END_OF_LINE));
+        }
+
+        // This means the same name as previous.
+        if (last_name_.get() == NULL) {
+            isc_throw(InternalException, "No previous name to use in "
+                      "place of initial whitespace");
+        } else if (!previous_name_) {
+            callbacks_.warning(lexer_.getSourceName(), lexer_.getSourceLine(),
+                               "Owner name omitted around $INCLUDE, the result "
+                               "might not be as expected");
+        }
+        return (next_token);
+    } else if (initial_token.getType() == MasterToken::STRING ||
+               initial_token.getType() == MasterToken::QSTRING) {
+        // If it is name (or directive), handle it.
+        const MasterToken::StringRegion&
+            name_string(initial_token.getStringRegion());
+
+        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.
+            return (MasterToken(MasterToken::END_OF_LINE));
+        }
+
+        // This should be an RR, starting with an owner name.  Construct the
+        // name, and some string token should follow.
+        last_name_.reset(new Name(name_string.beg, name_string.len,
+                                  &active_origin_));
+        previous_name_ = true;
+        return (lexer_.getNextToken(MasterToken::STRING));
+    }
+
+    switch (initial_token.getType()) { // handle less common cases
+    case MasterToken::END_OF_FILE:
+        if (!popSource()) {
+            return (initial_token);
+        } else {
+            // We try to read a token from the popped source
+            // So continue to the next line of that source, but first, make
+            // sure the source is at EOL
+            eatUntilEOL(true);
+            return (MasterToken(MasterToken::END_OF_LINE));
+        }
+    case MasterToken::END_OF_LINE:
+        return (initial_token); // empty line
+    case MasterToken::ERROR:
+        // Error token here.
+        isc_throw(InternalException, initial_token.getErrorText());
+    default:
+        // Some other token (what could that be?)
+        isc_throw(InternalException, "Parser got confused (unexpected "
+                  "token " << initial_token.getType() << ")");
+    }
+}
+
 bool
 MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
     if (count_limit == 0) {
@@ -380,61 +517,32 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
     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) {
-                    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);
-            // Return the last token, as it was not empty
-            lexer_.ungetToken();
-
-            const MasterToken::StringRegion&
-                name_string(lexer_.getNextToken(MasterToken::QSTRING).
-                            getStringRegion());
-
-            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 MasterToken next_token = handleInitialToken();
+            if (next_token.getType() == MasterToken::END_OF_FILE) {
+                return (true);  // we are done
+            } else if (next_token.getType() == MasterToken::END_OF_LINE) {
+                continue;       // nothing more to do in this line
             }
+            // We are going to parse an RR, have known the owner name,
+            // and are now seeing the next string token in the rest of the RR.
+            assert(next_token.getType() == MasterToken::STRING);
 
-            const Name name(name_string.beg, name_string.len, &zone_origin_);
-
-            // The parameters
             bool explicit_ttl = false;
-            const RRType rrtype = parseRRParams(explicit_ttl);
+            const RRType rrtype = parseRRParams(explicit_ttl, next_token);
             // TODO: Check if it is SOA, it should be at the origin.
 
             const rdata::RdataPtr rdata =
-                rdata::createRdata(rrtype, zone_class_, lexer_, &zone_origin_,
-                                   options_, callbacks_);
+                rdata::createRdata(rrtype, zone_class_, lexer_,
+                                   &active_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 (rdata) {
-                add_callback_(name, zone_class_, rrtype,
+                add_callback_(*last_name_, zone_class_, rrtype,
                               getCurrentTTL(explicit_ttl, rrtype, rdata),
                               rdata);
-
                 // Good, we loaded another one
                 ++count;
             } else {
diff --git a/src/lib/dns/tests/master_lexer_state_unittest.cc b/src/lib/dns/tests/master_lexer_state_unittest.cc
index 846c4c2..2be43db 100644
--- a/src/lib/dns/tests/master_lexer_state_unittest.cc
+++ b/src/lib/dns/tests/master_lexer_state_unittest.cc
@@ -190,13 +190,16 @@ TEST_F(MasterLexerStateTest, unbalancedParentheses) {
 }
 
 TEST_F(MasterLexerStateTest, startToComment) {
-    // Begin with 'start', skip space, then encounter a comment.  Skip
+    // Begin with 'start', detect space, then encounter a comment.  Skip
     // the rest of the line, and recognize the new line.  Note that the
     // second ';' is simply ignored.
     ss << "  ;a;\n";
     ss << ";a;";           // Likewise, but the comment ends with EOF.
     lexer.pushSource(ss);
 
+    // Initial whitespace (asked for in common_options)
+    EXPECT_EQ(s_null, State::start(lexer, common_options));
+    EXPECT_EQ(Token::INITIAL_WS, s_crlf.getToken(lexer).getType());
     // Comment ending with EOL
     EXPECT_EQ(s_null, State::start(lexer, common_options));
     EXPECT_EQ(Token::END_OF_LINE, s_crlf.getToken(lexer).getType());
diff --git a/src/lib/dns/tests/master_lexer_unittest.cc b/src/lib/dns/tests/master_lexer_unittest.cc
index 42ed3fb..62ad79d 100644
--- a/src/lib/dns/tests/master_lexer_unittest.cc
+++ b/src/lib/dns/tests/master_lexer_unittest.cc
@@ -238,10 +238,8 @@ TEST_F(MasterLexerTest, ungetToken) {
 // Check ungetting token without overriding the start method. We also
 // check it works well with changing options between the calls.
 TEST_F(MasterLexerTest, ungetRealOptions) {
-    ss << "\n    \n";
+    ss << "    \n";
     lexer.pushSource(ss);
-    // Skip the first newline
-    EXPECT_EQ(MasterToken::END_OF_LINE, lexer.getNextToken().getType());
 
     // If we call it the usual way, it skips up to the newline and returns
     // it
@@ -254,6 +252,22 @@ TEST_F(MasterLexerTest, ungetRealOptions) {
               lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
 }
 
+// Check the initial whitespace is found even in the first line of included
+// file
+TEST_F(MasterLexerTest, includeAndInitialWS) {
+    ss << "    \n";
+    lexer.pushSource(ss);
+
+    stringstream ss2;
+    ss2 << "    \n";
+
+    EXPECT_EQ(MasterToken::INITIAL_WS,
+              lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
+    lexer.pushSource(ss2);
+    EXPECT_EQ(MasterToken::INITIAL_WS,
+              lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
+}
+
 // Test only one token can be ungotten
 TEST_F(MasterLexerTest, ungetTwice) {
     ss << "\n";
diff --git a/src/lib/dns/tests/master_loader_unittest.cc b/src/lib/dns/tests/master_loader_unittest.cc
index 8c5f14d..667706a 100644
--- a/src/lib/dns/tests/master_loader_unittest.cc
+++ b/src/lib/dns/tests/master_loader_unittest.cc
@@ -50,6 +50,11 @@ public:
                                &warnings_, _1, _2, _3))
     {}
 
+    void TearDown() {
+        // Check there are no more RRs we didn't expect
+        EXPECT_TRUE(rrsets_.empty());
+    }
+
     /// Concatenate file, line, and reason, and add it to either errors
     /// or warnings
     void callback(vector<string>* target, const std::string& file, size_t line,
@@ -127,6 +132,11 @@ public:
                 "1234 3600 1800 2419200 7200");
         checkRR("example.org", RRType::NS(), "ns1.example.org.");
         checkRR("www.example.org", RRType::A(), "192.0.2.1");
+        checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
+    }
+
+    void checkARR(const string& name) {
+        checkRR(name, RRType::A(), "192.0.2.1");
     }
 
     MasterLoaderCallbacks callbacks_;
@@ -185,6 +195,66 @@ TEST_F(MasterLoaderTest, include) {
     }
 }
 
+// A commonly used helper to check callback message.
+void
+checkCallbackMessage(const string& actual_msg, const string& expected_msg,
+                     size_t expected_line) {
+    // The actual message should begin with the expected message.
+    EXPECT_EQ(0, actual_msg.find(expected_msg)) << "actual message: " <<
+                                                actual_msg << " expected: " <<
+                                                expected_msg;
+
+    // and it should end with "...:<line_num>]"
+    const string line_desc = ":" + lexical_cast<string>(expected_line) + "]";
+    EXPECT_EQ(actual_msg.size() - line_desc.size(),
+              actual_msg.find(line_desc)) << "Expected on line " <<
+        expected_line;
+}
+
+TEST_F(MasterLoaderTest, origin) {
+    // Various forms of the directive
+    const char* origins[] = {
+        "$origin",
+        "$ORIGIN",
+        "$Origin",
+        "$OrigiN",
+        "\"$ORIGIN\"",
+        NULL
+    };
+    for (const char** origin = origins; *origin != NULL; ++origin) {
+        SCOPED_TRACE(*origin);
+
+        clear();
+        const string directive = *origin;
+        const string input =
+            "@  1H  IN  A   192.0.2.1\n" +
+            directive + " sub.example.org.\n"
+            "\"www\"    1H  IN  A   192.0.2.1\n" +
+            // Relative name in the origin
+            directive + " relative\n"
+            "@  1H  IN  A   192.0.2.1\n"
+            // Origin is _not_ used here (absolute name)
+            "noorigin.example.org.  60M IN  A   192.0.2.1\n";
+        stringstream ss(input);
+        setLoader(ss, Name("example.org."), RRClass::IN(),
+                  MasterLoader::MANY_ERRORS);
+
+        loader_->load();
+        EXPECT_TRUE(loader_->loadedSucessfully());
+        EXPECT_TRUE(errors_.empty());
+        // There's a relative origin in it, we warn about that.
+        EXPECT_EQ(1, warnings_.size());
+        checkCallbackMessage(warnings_.at(0),
+                             "The new origin is relative, did you really mean "
+                             "relative.sub.example.org.?", 4);
+
+        checkARR("example.org");
+        checkARR("www.sub.example.org");
+        checkARR("relative.sub.example.org");
+        checkARR("noorigin.example.org");
+    }
+}
+
 // Test the source is correctly popped even after error
 TEST_F(MasterLoaderTest, popAfterError) {
     const string include_str = "$include " TEST_DATA_SRCDIR
@@ -250,6 +320,7 @@ TEST_F(MasterLoaderTest, incrementalLoad) {
     EXPECT_TRUE(warnings_.empty());
 
     checkRR("www.example.org", RRType::A(), "192.0.2.1");
+    checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
 }
 
 // Try loading from file that doesn't exist. There should be single error
@@ -330,12 +401,24 @@ struct ErrorCase {
     // 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", NULL, "Unknown $ directive" },
-    { "$INCLUD " TEST_DATA_SRCDIR "/example.org", NULL, "Include too short" },
-    { "$INCLUDES " TEST_DATA_SRCDIR "/example.org", NULL, "Include too long" },
-    { "$INCLUDE", NULL, "Missing include path" },
+    { "$INCLUD " TEST_DATA_SRCDIR "/example.org", "Unknown directive 'INCLUD'",
+        "Include too short" },
+    { "$INCLUDES " TEST_DATA_SRCDIR "/example.org",
+        "Unknown directive 'INCLUDES'", "Include too long" },
+    { "$INCLUDE", "unexpected end of input", "Missing include path" },
+    // The following two error messages are system dependant, omitting
     { "$INCLUDE /file/not/found", NULL, "Include file not found" },
-    { "$INCLUDE /file/not/found and here goes bunch of garbage", NULL,
-        "Include file not found and garbage at the end of line" },
+    { "$INCLUDE /file/not/found example.org. and here goes bunch of garbage",
+        NULL, "Include file not found and garbage at the end of line" },
+    { "$ORIGIN", "unexpected end of input", "Missing origin name" },
+    { "$ORIGIN invalid...name", "duplicate period in invalid...name",
+        "Invalid name for origin" },
+    { "$ORIGIN )brokentoken", "unbalanced parentheses",
+        "Broken token in origin" },
+    { "$ORIGIN example.org. garbage", "Extra tokens at the end of line",
+        "Garbage after origin" },
+    { "$ORIGI name.", "Unknown directive 'ORIGI'", "$ORIGIN too short" },
+    { "$ORIGINAL name.", "Unknown directive 'ORIGINAL'", "$ORIGIN too long" },
     { "$TTL 100 extra-garbage", "Extra tokens at the end of line",
       "$TTL with extra token" },
     { "$TTL", "unexpected end of input", "missing TTL" },
@@ -346,19 +429,6 @@ struct ErrorCase {
     { NULL, NULL, NULL }
 };
 
-// A commonly used helper to check callback message.
-void
-checkCallbackMessage(const string& actual_msg, const string& expected_msg,
-                     size_t expected_line) {
-    // The actual message should begin with the expected message.
-    EXPECT_EQ(0, actual_msg.find(expected_msg)) << "actual message: "
-                                                << actual_msg;
-
-    // and it should end with "...:<line_num>]"
-    const string line_desc = ":" + lexical_cast<string>(expected_line) + "]";
-    EXPECT_EQ(actual_msg.size() - line_desc.size(), actual_msg.find(line_desc));
-}
-
 // Test a broken zone is handled properly. We test several problems,
 // both in strict and lenient mode.
 TEST_F(MasterLoaderTest, brokenZone) {
@@ -433,7 +503,7 @@ 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"
+                             "/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(),
@@ -442,6 +512,7 @@ TEST_F(MasterLoaderTest, includeWithGarbage) {
     EXPECT_NO_THROW(loader_->load());
     EXPECT_FALSE(loader_->loadedSucessfully());
     ASSERT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "Extra tokens at the end of line", 1);
     // It says something about extra tokens at the end
     EXPECT_NE(string::npos, errors_[0].find("Extra"));
     EXPECT_TRUE(warnings_.empty());
@@ -449,6 +520,105 @@ TEST_F(MasterLoaderTest, includeWithGarbage) {
     checkRR("www.example.org", RRType::AAAA(), "2001:db8::1");
 }
 
+// Check we error about garbage at the end of $ORIGIN line (but the line
+// works).
+TEST_F(MasterLoaderTest, originWithGarbage) {
+    const string origin_str = "$ORIGIN www.example.org. More garbage here\n"
+        "@  1H  IN  A   192.0.2.1\n";
+    stringstream ss(origin_str);
+    setLoader(ss, Name("example.org."), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    EXPECT_NO_THROW(loader_->load());
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    ASSERT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "Extra tokens at the end of line", 1);
+    EXPECT_TRUE(warnings_.empty());
+    checkARR("www.example.org");
+}
+
+// Test we can pass both file to include and the origin to switch
+TEST_F(MasterLoaderTest, includeAndOrigin) {
+    // First, switch origin to something else, so we can check it is
+    // switched back.
+    const string include_string = "$ORIGIN www.example.org.\n"
+        "@  1H  IN  A   192.0.2.1\n"
+        // Then include the file with data and switch origin back
+        "$INCLUDE " TEST_DATA_SRCDIR "/example.org example.org.\n"
+        // Another RR to see the switch survives after we exit include
+        "www    1H  IN  A   192.0.2.1\n";
+    stringstream ss(include_string);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    // Successfully load the data
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_TRUE(warnings_.empty());
+    // And check it's the correct data
+    checkARR("www.example.org");
+    checkBasicRRs();
+    checkARR("www.example.org");
+}
+
+// Like above, but the origin after include is bogus. The whole line should
+// be rejected.
+TEST_F(MasterLoaderTest, includeAndBadOrigin) {
+    const string include_string =
+        "$INCLUDE " TEST_DATA_SRCDIR "/example.org example..org.\n"
+        // Another RR to see the switch survives after we exit include
+        "www    1H  IN  A   192.0.2.1\n";
+    stringstream ss(include_string);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    loader_->load();
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    EXPECT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "duplicate period in example..org.",
+                         1);
+    EXPECT_TRUE(warnings_.empty());
+    // And check it's the correct data
+    checkARR("www.example.org");
+}
+
+// Check the origin doesn't get outside of the included file.
+TEST_F(MasterLoaderTest, includeOriginRestore) {
+    const string include_string = "$INCLUDE " TEST_DATA_SRCDIR "/origincheck.txt\n"
+        "@  1H  IN  A   192.0.2.1\n";
+    stringstream ss(include_string);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    // Successfully load the data
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_TRUE(warnings_.empty());
+    // And check it's the correct data
+    checkARR("www.example.org");
+    checkARR("example.org");
+}
+
+// Check we restore the last name for initial whitespace when returning from
+// include. But we do produce a warning if there's one just ofter the include.
+TEST_F(MasterLoaderTest, includeAndInitialWS) {
+    const string include_string = "xyz  1H  IN  A   192.0.2.1\n"
+        "$INCLUDE " TEST_DATA_SRCDIR "/example.org\n"
+        "   1H  IN  A   192.0.2.1\n";
+    stringstream ss(include_string);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    // Successfully load the data
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_TRUE(errors_.empty());
+    EXPECT_EQ(1, warnings_.size());
+    checkCallbackMessage(warnings_.at(0),
+                         "Owner name omitted around $INCLUDE, the result might "
+                         "not be as expected", 3);
+    checkARR("xyz.example.org");
+    checkBasicRRs();
+    checkARR("xyz.example.org");
+}
+
 // Test for "$TTL"
 TEST_F(MasterLoaderTest, ttlDirective) {
     stringstream zone_stream;
@@ -598,7 +768,8 @@ TEST_F(MasterLoaderTest, ttlUnknownAndEOF) {
     // RDATA implementation can complain about it, too.  To be independent of
     // its details, we focus on the very last warning.
     EXPECT_FALSE(warnings_.empty());
-    checkCallbackMessage(*warnings_.rbegin(), "Unexpected end of file", 1);
+    checkCallbackMessage(*warnings_.rbegin(), "File does not end with newline",
+                         1);
 }
 
 TEST_F(MasterLoaderTest, ttlOverflow) {
@@ -650,6 +821,9 @@ TEST_F(MasterLoaderTest, loadTwice) {
 
     loader_->load();
     EXPECT_THROW(loader_->load(), isc::InvalidOperation);
+    // Don't check them, they are not interesting, so suppress the error
+    // at TearDown
+    rrsets_.clear();
 }
 
 // Load 0 items should be rejected
@@ -671,10 +845,44 @@ TEST_F(MasterLoaderTest, noEOLN) {
 
     loader_->load();
     EXPECT_TRUE(loader_->loadedSucessfully());
-    EXPECT_TRUE(errors_.empty()) << errors_[0];
+    EXPECT_TRUE(errors_.empty());
     // There should be one warning about the EOLN
     EXPECT_EQ(1, warnings_.size());
     checkRR("example.org", RRType::SOA(), "ns1.example.org. "
             "admin.example.org. 1234 3600 1800 2419200 7200");
 }
+
+// Test it rejects when we don't have the previous name to use in place of
+// initial whitespace
+TEST_F(MasterLoaderTest, noPreviousName) {
+    const string input("    1H  IN  A   192.0.2.1\n");
+    stringstream ss(input);
+    setLoader(ss, Name("example.org."), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    loader_->load();
+    EXPECT_FALSE(loader_->loadedSucessfully());
+    EXPECT_EQ(1, errors_.size());
+    checkCallbackMessage(errors_.at(0), "No previous name to use in place of "
+                         "initial whitespace", 1);
+    EXPECT_TRUE(warnings_.empty());
+}
+
+// Check we warn if the first RR in an included file has omitted name
+TEST_F(MasterLoaderTest, previousInInclude) {
+    const string input("www 1H  IN  A   192.0.2.1\n"
+                       "$INCLUDE " TEST_DATA_SRCDIR "/omitcheck.txt\n");
+    stringstream ss(input);
+    setLoader(ss, Name("example.org"), RRClass::IN(),
+              MasterLoader::MANY_ERRORS);
+    loader_->load();
+    EXPECT_TRUE(loader_->loadedSucessfully());
+    EXPECT_TRUE(errors_.empty());
+    // There should be one warning about the EOLN
+    EXPECT_EQ(1, warnings_.size());
+    checkCallbackMessage(warnings_.at(0), "Owner name omitted around "
+                         "$INCLUDE, the result might not be as expected", 1);
+    checkARR("www.example.org");
+    checkARR("www.example.org");
+}
+
 }
diff --git a/src/lib/dns/tests/testdata/Makefile.am b/src/lib/dns/tests/testdata/Makefile.am
index 9494117..52acb7c 100644
--- a/src/lib/dns/tests/testdata/Makefile.am
+++ b/src/lib/dns/tests/testdata/Makefile.am
@@ -172,6 +172,8 @@ EXTRA_DIST += tsig_verify7.spec tsig_verify8.spec tsig_verify9.spec
 EXTRA_DIST += tsig_verify10.spec
 EXTRA_DIST += example.org
 EXTRA_DIST += broken.zone
+EXTRA_DIST += origincheck.txt
+EXTRA_DIST += omitcheck.txt
 
 .spec.wire:
 	$(PYTHON) $(top_builddir)/src/lib/util/python/gen_wiredata.py -o $@ $<
diff --git a/src/lib/dns/tests/testdata/example.org b/src/lib/dns/tests/testdata/example.org
index 2f2edf3..4163fc0 100644
--- a/src/lib/dns/tests/testdata/example.org
+++ b/src/lib/dns/tests/testdata/example.org
@@ -13,3 +13,5 @@ example.org.        3600    IN  SOA ( ; The SOA, split across lines for testing
 
 ; Some empty lines here. They are to make sure the loader can skip them.
 www                 3600    IN  A 192.0.2.1 ; Test a relative name as well.
+                    3600    IN  AAAA    2001:db8::1 ; And initial whitespace hanling
+         ; Here be just some space, no RRs
diff --git a/src/lib/dns/tests/testdata/omitcheck.txt b/src/lib/dns/tests/testdata/omitcheck.txt
new file mode 100644
index 0000000..580cab4
--- /dev/null
+++ b/src/lib/dns/tests/testdata/omitcheck.txt
@@ -0,0 +1 @@
+    1H  IN  A   192.0.2.1
diff --git a/src/lib/dns/tests/testdata/origincheck.txt b/src/lib/dns/tests/testdata/origincheck.txt
new file mode 100644
index 0000000..c370ed2
--- /dev/null
+++ b/src/lib/dns/tests/testdata/origincheck.txt
@@ -0,0 +1,5 @@
+; We change the origin here. We want to check it is not propagated
+; outside of the included file.
+$ORIGIN www.example.org.
+
+@   1H  IN  A   192.0.2.1



More information about the bind10-changes mailing list