Browse Source

[2380] make sure the new origin for $INCLUDE doesn't change post-include.

this seems to be the actual intent of the RFC, and it's compatible with
BIND 9, too.  This fix will resolve the remaining regression for the
old loadzone tests.
JINMEI Tatuya 12 years ago
parent
commit
37cc046b9f
2 changed files with 17 additions and 7 deletions
  1. 13 4
      src/lib/dns/master_loader.cc
  2. 4 3
      src/lib/dns/tests/master_loader_unittest.cc

+ 13 - 4
src/lib/dns/master_loader.cc

@@ -79,7 +79,7 @@ public:
         warn_rfc1035_ttl_(true)
         warn_rfc1035_ttl_(true)
     {}
     {}
 
 
-    void pushSource(const std::string& filename) {
+    void pushSource(const std::string& filename, const Name& current_origin) {
         std::string error;
         std::string error;
         if (!lexer_.pushSource(filename.c_str(), &error)) {
         if (!lexer_.pushSource(filename.c_str(), &error)) {
             if (initialized_) {
             if (initialized_) {
@@ -91,7 +91,7 @@ public:
             }
             }
         }
         }
         // Store the current status, so we can recover it upon popSource
         // Store the current status, so we can recover it upon popSource
-        include_info_.push_back(IncludeInfo(active_origin_, last_name_));
+        include_info_.push_back(IncludeInfo(current_origin, last_name_));
         initialized_ = true;
         initialized_ = true;
         previous_name_ = false;
         previous_name_ = false;
     }
     }
@@ -182,9 +182,18 @@ private:
             filename(lexer_.getNextToken(MasterToken::QSTRING).getString());
             filename(lexer_.getNextToken(MasterToken::QSTRING).getString());
 
 
         // There optionally can be an origin, that applies before the include.
         // There optionally can be an origin, that applies before the include.
+        // We need to save the currently active origin before calling
+        // doOrigin(), because it would update active_origin_ while we need
+        // to pass the active origin before recognizing the new origin to
+        // pushSource.  Note: RFC 1035 is not really clear on this: it reads
+        // "regardless of changes... within the included file", but the new
+        // origin is not really specified "within the included file".
+        // Nevertheless, this behavior is probably more likely to be the
+        // intent of the RFC, and it's compatible with BIND 9.
+        const Name current_origin = active_origin_;
         doOrigin(true);
         doOrigin(true);
 
 
-        pushSource(filename);
+        pushSource(filename, current_origin);
     }
     }
 
 
     // A helper method for loadIncremental(). It parses part of an RR
     // A helper method for loadIncremental(). It parses part of an RR
@@ -512,7 +521,7 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
                   "Trying to load when already loaded");
                   "Trying to load when already loaded");
     }
     }
     if (!initialized_) {
     if (!initialized_) {
-        pushSource(master_file_);
+        pushSource(master_file_, active_origin_);
     }
     }
     size_t count = 0;
     size_t count = 0;
     while (ok_ && count < count_limit) {
     while (ok_ && count < count_limit) {

+ 4 - 3
src/lib/dns/tests/master_loader_unittest.cc

@@ -544,7 +544,7 @@ TEST_F(MasterLoaderTest, includeAndOrigin) {
         "@  1H  IN  A   192.0.2.1\n"
         "@  1H  IN  A   192.0.2.1\n"
         // Then include the file with data and switch origin back
         // Then include the file with data and switch origin back
         "$INCLUDE " TEST_DATA_SRCDIR "/example.org example.org.\n"
         "$INCLUDE " TEST_DATA_SRCDIR "/example.org example.org.\n"
-        // Another RR to see the switch survives after we exit include
+        // Another RR to see we fall back to the previous origin.
         "www    1H  IN  A   192.0.2.1\n";
         "www    1H  IN  A   192.0.2.1\n";
     stringstream ss(include_string);
     stringstream ss(include_string);
     setLoader(ss, Name("example.org"), RRClass::IN(),
     setLoader(ss, Name("example.org"), RRClass::IN(),
@@ -557,7 +557,7 @@ TEST_F(MasterLoaderTest, includeAndOrigin) {
     // And check it's the correct data
     // And check it's the correct data
     checkARR("www.example.org");
     checkARR("www.example.org");
     checkBasicRRs();
     checkBasicRRs();
-    checkARR("www.example.org");
+    checkARR("www.www.example.org");
 }
 }
 
 
 // Like above, but the origin after include is bogus. The whole line should
 // Like above, but the origin after include is bogus. The whole line should
@@ -582,7 +582,8 @@ TEST_F(MasterLoaderTest, includeAndBadOrigin) {
 
 
 // Check the origin doesn't get outside of the included file.
 // Check the origin doesn't get outside of the included file.
 TEST_F(MasterLoaderTest, includeOriginRestore) {
 TEST_F(MasterLoaderTest, includeOriginRestore) {
-    const string include_string = "$INCLUDE " TEST_DATA_SRCDIR "/origincheck.txt\n"
+    const string include_string =
+        "$INCLUDE " TEST_DATA_SRCDIR "/origincheck.txt\n"
         "@  1H  IN  A   192.0.2.1\n";
         "@  1H  IN  A   192.0.2.1\n";
     stringstream ss(include_string);
     stringstream ss(include_string);
     setLoader(ss, Name("example.org"), RRClass::IN(),
     setLoader(ss, Name("example.org"), RRClass::IN(),