Browse Source

From Trac #307.
This includes revisions 2792 and 2922 plus the ChangeLog entry:

91. [bug] jinmei
lib/datasrc: A DS query could crash the library (and therefore,
e.g. the authoritative server) if some RR of the same apex name
is stored in the hot spot cache. (Trac #307, svn rTBD)



git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@2923 e5f2f494-b856-4b98-b285-d166d9295462

Jeremy C. Reed 14 years ago
parent
commit
8034b2e3dc
3 changed files with 28 additions and 0 deletions
  1. 5 0
      ChangeLog
  2. 5 0
      src/lib/datasrc/data_source.cc
  3. 18 0
      src/lib/datasrc/tests/datasrc_unittest.cc

+ 5 - 0
ChangeLog

@@ -1,3 +1,8 @@
+  91.	[bug]		jinmei
+	lib/datasrc: A DS query could crash the library (and therefore,
+	e.g. the authoritative server) if some RR of the same apex name
+	is stored in the hot spot cache.  (Trac #307, svn r2923)
+
   92.	[func]*		jelte
   92.	[func]*		jelte
 	libdns_python (the python wrappers for libdns++) has been renamed
 	libdns_python (the python wrappers for libdns++) has been renamed
 	to pydnspp (Python DNS++). Programs and libraries that used
 	to pydnspp (Python DNS++). Programs and libraries that used

+ 5 - 0
src/lib/datasrc/data_source.cc

@@ -206,6 +206,11 @@ checkCache(QueryTask& task, RRsetList& target) {
         if (!hit || !rrset || (flags & DataSrc::CNAME_FOUND) != 0) {
         if (!hit || !rrset || (flags & DataSrc::CNAME_FOUND) != 0) {
             hit = cache.retrieve(task.qname, task.qclass, RRType::CNAME(),
             hit = cache.retrieve(task.qname, task.qclass, RRType::CNAME(),
                                  rrset, flags);
                                  rrset, flags);
+            if (!rrset) {
+                // If we don't have a positive cache, forget it; otherwise the
+                // intermediate result may confuse the subsequent processing.
+                hit = false;
+            }
         }
         }
 
 
         if (hit) {
         if (hit) {

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

@@ -915,6 +915,24 @@ TEST_F(DataSrcTest, RootDSQuery) {
     headerCheck(msg, Rcode::REFUSED(), true, false, true, 0, 0, 0);
     headerCheck(msg, Rcode::REFUSED(), true, false, true, 0, 0, 0);
 }
 }
 
 
+TEST_F(DataSrcTest, DSQueryFromCache) {
+    // explicitly enable hot spot cache
+    cache.setEnabled(true);
+
+    // The first query will create a negative cache for example.org/CNAME
+    createAndProcessQuery(Name("example.org"), RRClass::IN(), RRType::SOA());
+
+    // the cached CNAME shouldn't confuse subsequent query.
+    // there may be several different possible cases that could trigger a bug,
+    // but DS query is the only known example.
+    msg.clear(Message::PARSE);
+    createAndProcessQuery(Name("example.org"), RRClass::IN(), RRType::DS());
+
+    // returning refused is probably a bad behavior, but it's a different
+    // issue -- see Trac Ticket #306.
+    headerCheck(msg, Rcode::REFUSED(), true, false, true, 0, 0, 0);
+}
+
 // Non-existent name in the "static" data source.  The purpose of this test
 // Non-existent name in the "static" data source.  The purpose of this test
 // is to check a corner case behavior when atypical RRClass (CH in this case)
 // is to check a corner case behavior when atypical RRClass (CH in this case)
 // is specified.
 // is specified.