Browse Source

[master] Merge branch 'trac851'

JINMEI Tatuya 14 years ago
parent
commit
2463b96680
2 changed files with 29 additions and 8 deletions
  1. 13 8
      src/lib/datasrc/data_source.cc
  2. 16 0
      src/lib/datasrc/tests/datasrc_unittest.cc

+ 13 - 8
src/lib/datasrc/data_source.cc

@@ -354,7 +354,19 @@ doQueryTask(QueryTask& task, ZoneInfo& zoneinfo, RRsetList& target) {
     HotCache& cache = task.q.getCache();
     HotCache& cache = task.q.getCache();
     RRsetPtr rrset;
     RRsetPtr rrset;
 
 
-    // First, check the cache for matching data
+    // First off, make sure at least we have a matching zone in some data
+    // source.  We must do this before checking the cache, because it can
+    // happen that the matching zone has been removed after an RRset of that
+    // zone is cached.  Such inconsistency will cause various problems,
+    // including a crash.
+    const DataSrc* ds = zoneinfo.getDataSource();
+    const Name* const zonename = zoneinfo.getEnclosingZone();
+    if (ds == NULL) {
+        task.flags |= DataSrc::NO_SUCH_ZONE;
+        return (DataSrc::SUCCESS);
+    }
+
+    // Then check the cache for matching data
     if (checkCache(task, target)) {
     if (checkCache(task, target)) {
         return (DataSrc::SUCCESS);
         return (DataSrc::SUCCESS);
     }
     }
@@ -362,13 +374,6 @@ doQueryTask(QueryTask& task, ZoneInfo& zoneinfo, RRsetList& target) {
     // Requested data weren't in the cache (or were, but had expired),
     // Requested data weren't in the cache (or were, but had expired),
     // so now we proceed with the low-level data source lookup, and cache
     // so now we proceed with the low-level data source lookup, and cache
     // whatever we find.
     // whatever we find.
-    const DataSrc* ds = zoneinfo.getDataSource();
-    const Name* const zonename = zoneinfo.getEnclosingZone();
-
-    if (ds == NULL) {
-        task.flags |= DataSrc::NO_SUCH_ZONE;
-        return (DataSrc::SUCCESS);
-    }
 
 
     DataSrc::Result result;
     DataSrc::Result result;
     switch (task.op) {
     switch (task.op) {

+ 16 - 0
src/lib/datasrc/tests/datasrc_unittest.cc

@@ -1092,6 +1092,22 @@ TEST_F(DataSrcTest, DISABLED_synthesizedCnameTooLong) {
         RRClass::IN(), RRType::A());
         RRClass::IN(), RRType::A());
 }
 }
 
 
+TEST_F(DataSrcTest, cacheDataInNonexistentZone) {
+    // This test emulates the situation where an RRset in some zone of some
+    // data source is cached and then the zone is removed from the data source.
+    // When there is such a substantial inconsistency between the cache and
+    // the real data source, we should honor the latter.  More important,
+    // the inconsistency shouldn't cause any disruption such as a crash.
+
+    const Name qname("nosuchzone.example");
+    RRsetPtr rrset(new RRset(qname, RRClass::IN(), RRType::A(), RRTTL(0)));
+    cache.addPositive(rrset, DataSrc::REFERRAL);
+
+    createAndProcessQuery(qname, RRClass::IN(), RRType::A(), false);
+    headerCheck(msg, qid, Rcode::REFUSED(), opcodeval, QR_FLAG | RD_FLAG,
+                1, 0, 0, 0);
+}
+
 // Tests of the DataSrcMatch class start here
 // Tests of the DataSrcMatch class start here
 class DataSrcMatchTest : public ::testing::Test {
 class DataSrcMatchTest : public ::testing::Test {
 protected:
 protected: