merge from trac69

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

ChangeLog

@@ -1,20 +1,26 @@
+  66.  [bug]        each
+	Check for duplicate RRsets before inserting data into a message
+	section; this, among other things, will prevent multiple copies
+	of the same CNAME from showing up when there's a loop.  (Trac #69,
+	svn r2349)
+    
   65.  [func]       shentingting
-    Added verbose options to exactly what is happening with loadzone.
+	Added verbose options to exactly what is happening with loadzone.
 	Added loadzone test suite of different file formats to load.
 	(Trac #197, #199, #244, #161, #198, #174, #175, svn r2340)
 
   64.  [func]       jerry
-    Added python logging framework. It is for testing and experimenting
+	Added python logging framework. It is for testing and experimenting
 	with logging ideas. Currently, it supports three channels(file, 
 	syslog and stderr) and five levels(debug, info, warning, error and
 	critical).
 	(Trac #176, svn r2338)
 
   63.  [func]       shane
-    Added initial support for setuid(), using the "-u" flag. This will
+	Added initial support for setuid(), using the "-u" flag. This will
-    be replaced in the future, but for now provides a reasonable 
+	be replaced in the future, but for now provides a reasonable 
-    starting point.
+	starting point.
-    (Trac #180, svn r2330)
+	(Trac #180, svn r2330)
 
   62.  [func]		jelte
 	bin/xfrin: Use the database_file as configured in Auth to transfers

src/lib/datasrc/data_source.cc

@@ -166,6 +166,25 @@ doQueryTask(const DataSrc* ds, const Name* zonename, QueryTask& task,
     return (DataSrc::ERROR);
 }
 
+// Add an RRset (and its associated RRSIG) to a message section,
+// checking first to ensure that there isn't already an RRset with
+// the same name and type.
+inline void
+addToMessage(Query& q, const Section& sect, RRsetPtr rrset,
+             bool no_dnssec = false)
+{
+    Message& m = q.message();
+    if (no_dnssec) {
+        if (rrset->getType() == RRType::RRSIG() || !m.hasRRset(sect, rrset)) {
+            m.addRRset(sect, rrset, false);
+        }
+    } else {
+        if (!m.hasRRset(sect, rrset)) {
+            m.addRRset(sect, rrset, q.wantDnssec());
+        }
+    }
+}
+
 // Copy referral information into the authority section of a message
 inline void
 copyAuth(Query& q, RRsetList& auth) {
@@ -176,7 +195,7 @@ copyAuth(Query& q, RRsetList& auth) {
         if (rrset->getType() == RRType::DS() && !q.wantDnssec()) {
             continue;
         }
-        q.message().addRRset(Section::AUTHORITY(), rrset, q.wantDnssec());
+        addToMessage(q, Section::AUTHORITY(), rrset);
         getAdditional(q, rrset);
     }
 }
@@ -232,14 +251,12 @@ hasDelegation(const DataSrc* ds, const Name* zonename, Query& q,
             RRsetPtr r = ref.findRRset(RRType::DNAME(), q.qclass());
             if (r != NULL) {
                 RRsetList syn;
-                q.message().addRRset(Section::ANSWER(), r, q.wantDnssec());
+                addToMessage(q, Section::ANSWER(), r);
                 q.message().setHeaderFlag(MessageFlag::AA());
                 synthesizeCname(task, r, syn);
                 if (syn.size() == 1) {
-                    q.message().addRRset(Section::ANSWER(),
+                    addToMessage(q, Section::ANSWER(),
-                                         syn.findRRset(RRType::CNAME(),
+                                 syn.findRRset(RRType::CNAME(), q.qclass()));
-                                                       q.qclass()),
-                                         q.wantDnssec());
                     chaseCname(q, task, syn.findRRset(RRType::CNAME(),
                                                       q.qclass()));
                     return (true);
@@ -264,7 +281,6 @@ hasDelegation(const DataSrc* ds, const Name* zonename, Query& q,
 
 inline DataSrc::Result
 addSOA(Query& q, const Name* zonename, const DataSrc* ds) {
-    Message& m = q.message();
     RRsetList soa;
 
     QueryTask newtask(*zonename, q.qclass(), RRType::SOA(),
@@ -274,8 +290,8 @@ addSOA(Query& q, const Name* zonename, const DataSrc* ds) {
         return (DataSrc::ERROR);
     }
 
-    m.addRRset(Section::AUTHORITY(), soa.findRRset(RRType::SOA(), q.qclass()),
+    addToMessage(q, Section::AUTHORITY(),
-               q.wantDnssec());
+                 soa.findRRset(RRType::SOA(), q.qclass()));
     return (DataSrc::SUCCESS);
 }
 
@@ -284,14 +300,13 @@ addNSEC(Query& q, const QueryTaskPtr task, const Name& name,
         const Name& zonename, const DataSrc* ds)
 {
     RRsetList nsec;
-    Message& m = q.message();
 
     QueryTask newtask(name, task->qclass, RRType::NSEC(),
                       QueryTask::SIMPLE_QUERY); 
     RETERR(doQueryTask(ds, &zonename, newtask, nsec));
     if (newtask.flags == 0) {
-        m.addRRset(Section::AUTHORITY(), nsec.findRRset(RRType::NSEC(),
+        addToMessage(q, Section::AUTHORITY(),
-                                                        q.qclass()), true);
+                     nsec.findRRset(RRType::NSEC(), q.qclass()));
     }
 
     return (DataSrc::SUCCESS);
@@ -344,14 +359,13 @@ inline DataSrc::Result
 proveNX(Query& q, QueryTaskPtr task, const DataSrc* ds,
         const Name& zonename, const bool wildcard)
 {
-    Message& m = q.message();
     ConstNsec3ParamPtr nsec3 = getNsec3Param(q, ds, zonename);
     if (nsec3 != NULL) {
         // Attach the NSEC3 record covering the QNAME
         RRsetPtr rrset;
         string hash1(nsec3->getHash(task->qname));
         RETERR(getNsec3(ds, zonename, q.qclass(), hash1, rrset));
-        m.addRRset(Section::AUTHORITY(), rrset, true);
+        addToMessage(q, Section::AUTHORITY(), rrset);
 
         // If this is an NXRRSET or NOERROR/NODATA, we're done
         if ((task->flags & DataSrc::TYPE_NOT_FOUND) != 0) {
@@ -376,7 +390,7 @@ proveNX(Query& q, QueryTaskPtr task, const DataSrc* ds,
             // we don't want to use one until we find an exact match
             RETERR(getNsec3(ds, zonename, q.qclass(), hash2, rrset));
             if (hash2 == nodehash) {
-                m.addRRset(Section::AUTHORITY(), rrset, true);
+                addToMessage(q, Section::AUTHORITY(), rrset);
                 break;
             }
         }
@@ -391,7 +405,7 @@ proveNX(Query& q, QueryTaskPtr task, const DataSrc* ds,
         string hash3(nsec3->getHash(Name("*").concatenate(enclosure)));
         RETERR(getNsec3(ds, zonename, q.qclass(), hash3, rrset));
         if (hash3 != hash1 && hash3 != hash2) {
-            m.addRRset(Section::AUTHORITY(), rrset, true);
+            addToMessage(q, Section::AUTHORITY(), rrset);
         }
     } else {
         Name nsecname(task->qname);
@@ -487,13 +501,13 @@ tryWildcard(Query& q, QueryTaskPtr task, const DataSrc* ds,
             RRsetPtr rrset = wild.findRRset(RRType::CNAME(), q.qclass());
             if (rrset != NULL) {
                 rrset->setName(task->qname);
-                m.addRRset(Section::ANSWER(), rrset, q.wantDnssec());
+                addToMessage(q, Section::ANSWER(), rrset);
                 chaseCname(q, task, rrset);
             }
         } else {
             BOOST_FOREACH (RRsetPtr rrset, wild) {
                 rrset->setName(task->qname);
-                m.addRRset(Section::ANSWER(), rrset, q.wantDnssec());
+                addToMessage(q, Section::ANSWER(), rrset);
             }
 
             RRsetList auth;
@@ -597,7 +611,7 @@ DataSrc::doQuery(Query& q) {
             case QueryTask::GETANSWER:
             case QueryTask::FOLLOWCNAME:
                 BOOST_FOREACH(RRsetPtr rrset, data) {
-                    m.addRRset(task->section, rrset, q.wantDnssec());
+                    addToMessage(q, task->section, rrset);
                     if (q.tasks().empty()) {
                         need_auth = true;
                     }
@@ -650,7 +664,7 @@ DataSrc::doQuery(Query& q) {
             // queue to look up its target.
             RRsetPtr rrset = data.findRRset(RRType::CNAME(), q.qclass());
             if (rrset != NULL) {
-                m.addRRset(task->section, rrset, q.wantDnssec());
+                addToMessage(q, task->section, rrset);
                 chaseCname(q, task, rrset);
             }
             continue;
@@ -666,12 +680,12 @@ DataSrc::doQuery(Query& q) {
                 }
                 BOOST_FOREACH (RRsetPtr rrset, auth) {
                     if (rrset->getType() == RRType::NS()) {
-                        m.addRRset(Section::AUTHORITY(), rrset, q.wantDnssec());
+                        addToMessage(q, Section::AUTHORITY(), rrset);
                     } else if (rrset->getType() == task->qtype) {
-                        m.addRRset(Section::ANSWER(), rrset, q.wantDnssec());
+                        addToMessage(q, Section::ANSWER(), rrset);
                     } else if (rrset->getType() == RRType::DS() &&
                                q.wantDnssec()) {
-                        m.addRRset(Section::AUTHORITY(), rrset, true);
+                        addToMessage(q, Section::AUTHORITY(), rrset);
                     }
                     getAdditional(q, rrset);
                 }
@@ -739,13 +753,13 @@ DataSrc::doQuery(Query& q) {
     // space, signatures in additional section are
     // optional.)
     BOOST_FOREACH(RRsetPtr rrset, additional) {
-        m.addRRset(Section::ADDITIONAL(), rrset, false);
+        addToMessage(q, Section::ADDITIONAL(), rrset, true);
     }
 
     if (q.wantDnssec()) {
         BOOST_FOREACH(RRsetPtr rrset, additional) {
             if (rrset->getRRsig()) {
-                m.addRRset(Section::ADDITIONAL(), rrset->getRRsig(), false);
+                addToMessage(q, Section::ADDITIONAL(), rrset->getRRsig(), true);
             }
         }
     }

src/lib/datasrc/tests/datasrc_unittest.cc

@@ -772,6 +772,12 @@ TEST_F(DataSrcTest, DS) {
 TEST_F(DataSrcTest, CNAMELoop) {
     createAndProcessQuery(Name("one.loop.example"), RRClass::IN(),
                           RRType::A());
+    EXPECT_EQ(Rcode::NOERROR(), msg.getRcode());
+
+    // one.loop.example points to two.loop.example, which points back
+    // to one.loop.example, so there should be exactly two CNAME records
+    // in the answer.
+    EXPECT_EQ(2, msg.getRRCount(Section::ANSWER()));
 }
 
 // NSEC query for the name of a zone cut for non-secure delegation.

src/lib/dns/message.cc

@@ -21,6 +21,7 @@
 #include <sstream>
 #include <vector>
 
+#include <boost/foreach.hpp>
 #include <boost/lexical_cast.hpp>
 #include <boost/shared_ptr.hpp>
 
@@ -370,7 +371,6 @@ Message::addRRset(const Section& section, RRsetPtr rrset, const bool sign) {
                   "addRRset performed in non-render mode");
     }
 
-    // Note: should check duplicate (TBD)
     impl_->rrsets_[sectionCodeToId(section)].push_back(rrset);
     impl_->counts_[section.getCode()] += rrset->getRdataCount();
 
@@ -381,6 +381,20 @@ Message::addRRset(const Section& section, RRsetPtr rrset, const bool sign) {
     }
 }
 
+bool
+Message::hasRRset(const Section& section, RRsetPtr rrset) {
+    BOOST_FOREACH(RRsetPtr r, impl_->rrsets_[sectionCodeToId(section)]) {
+        if (r->getType() == rrset->getType() &&
+            r->getName() == rrset->getName())
+        {
+            return (true);
+
+        }
+    }
+
+    return (false);
+}
+
 void
 Message::addQuestion(const QuestionPtr question) {
     if (impl_->mode_ != Message::RENDER) {

src/lib/dns/message.h

@@ -739,8 +739,16 @@ public:
     /// \c rrset.  This interface design needs to be revisited later.
     ///
     /// Only allowed in the \c RENDER mode.
+    ///
+    /// Note that addRRset() does not currently check for duplicate
+    /// data before inserting RRsets.  The caller is responsible for
+    /// checking for these (see hasRRset() below).
     void addRRset(const Section& section, RRsetPtr rrset, bool sign = false);
 
+    /// \brief Determine whether the given section already has an RRset
+    /// matching the name and type of this one
+    bool hasRRset(const Section& section, RRsetPtr rrset);
+
     // The following methods are not currently implemented.
     //void removeQuestion(QuestionPtr question);
     //void removeRRset(const Section& section, RRsetPtr rrset);