Browse Source

[2427] Warn on ambiguous initial whitespace name

Which happens after the $INCLUDE directive.
Michal 'vorner' Vaner 12 years ago
parent
commit
471c6f2d6c
2 changed files with 31 additions and 1 deletions
  1. 11 0
      src/lib/dns/master_loader.cc
  2. 20 1
      src/lib/dns/tests/master_loader_unittest.cc

+ 11 - 0
src/lib/dns/master_loader.cc

@@ -69,6 +69,7 @@ public:
         initialized_(false),
         initialized_(false),
         ok_(true),
         ok_(true),
         many_errors_((options & MANY_ERRORS) != 0),
         many_errors_((options & MANY_ERRORS) != 0),
+        previous_name_(false),
         complete_(false),
         complete_(false),
         seen_error_(false)
         seen_error_(false)
     {}
     {}
@@ -101,6 +102,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(active_origin_, last_name_));
         initialized_ = true;
         initialized_ = true;
+        previous_name_ = false;
     }
     }
 
 
     bool popSource() {
     bool popSource() {
@@ -117,6 +119,7 @@ public:
         active_origin_ = info.first;
         active_origin_ = info.first;
         last_name_ = info.second;
         last_name_ = info.second;
         include_info_.pop_back();
         include_info_.pop_back();
+        previous_name_ = false;
         return (true);
         return (true);
     }
     }
 
 
@@ -238,6 +241,8 @@ private:
     // in that file.
     // in that file.
     typedef pair<Name, shared_ptr<Name> > IncludeInfo;
     typedef pair<Name, shared_ptr<Name> > IncludeInfo;
     vector<IncludeInfo> include_info_;
     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:
 public:
     bool complete_;             // All work done.
     bool complete_;             // All work done.
     bool seen_error_;           // Was there at least one error during the
     bool seen_error_;           // Was there at least one error during the
@@ -312,11 +317,17 @@ MasterLoader::MasterLoaderImpl::loadIncremental(size_t count_limit) {
 
 
                 last_name_.reset(new Name(name_string.beg, name_string.len,
                 last_name_.reset(new Name(name_string.beg, name_string.len,
                                           &active_origin_));
                                           &active_origin_));
+                previous_name_ = true;
             } else if (initial_token.getType() == MasterToken::INITIAL_WS) {
             } else if (initial_token.getType() == MasterToken::INITIAL_WS) {
                 // This means the same name as previous.
                 // This means the same name as previous.
                 if (last_name_.get() == NULL) {
                 if (last_name_.get() == NULL) {
                     isc_throw(InternalException, "No previous name to use in "
                     isc_throw(InternalException, "No previous name to use in "
                               "place of initial whitespace");
                               "place of initial whitespace");
+                } else if (!previous_name_) {
+                    callbacks_.warning(lexer_.getSourceName(),
+                                       lexer_.getSourceLine(),
+                                       "Ambiguous previous name previous name "
+                                       "for initial whitespace");
                 }
                 }
             } else if (initial_token.getType() == MasterToken::ERROR) {
             } else if (initial_token.getType() == MasterToken::ERROR) {
                 // Error token here.
                 // Error token here.

+ 20 - 1
src/lib/dns/tests/master_loader_unittest.cc

@@ -478,13 +478,32 @@ TEST_F(MasterLoaderTest, includeOriginRestore) {
     // Successfully load the data
     // Successfully load the data
     loader_->load();
     loader_->load();
     EXPECT_TRUE(loader_->loadedSucessfully());
     EXPECT_TRUE(loader_->loadedSucessfully());
-    EXPECT_TRUE(errors_.empty()) << errors_[0];
+    EXPECT_TRUE(errors_.empty());
     EXPECT_TRUE(warnings_.empty());
     EXPECT_TRUE(warnings_.empty());
     // And check it's the correct data
     // And check it's the correct data
     checkARR("www.example.org");
     checkARR("www.example.org");
     checkARR("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());
+    checkARR("xyz.example.org");
+    checkBasicRRs();
+    checkARR("xyz.example.org");
+}
+
 // Test the constructor rejects empty add callback.
 // Test the constructor rejects empty add callback.
 TEST_F(MasterLoaderTest, emptyCallback) {
 TEST_F(MasterLoaderTest, emptyCallback) {
     EXPECT_THROW(MasterLoader(TEST_DATA_SRCDIR "/example.org",
     EXPECT_THROW(MasterLoader(TEST_DATA_SRCDIR "/example.org",