[svn] commit: r946 - in /branches/each-ds/src/lib: auth/cpp/data_source.cc auth/cpp/query.h dns/cpp/rrset.h

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Feb 24 07:53:45 UTC 2010


Author: each
Date: Wed Feb 24 07:53:45 2010
New Revision: 946

Log:
Followed a suggestion from Michael, and shortened doQuery() (and
hopefully improved readability) by moving some code into helper functions
instead of keeping them in the main body of doQuery().

Modified:
    branches/each-ds/src/lib/auth/cpp/data_source.cc
    branches/each-ds/src/lib/auth/cpp/query.h
    branches/each-ds/src/lib/dns/cpp/rrset.h

Modified: branches/each-ds/src/lib/auth/cpp/data_source.cc
==============================================================================
--- branches/each-ds/src/lib/auth/cpp/data_source.cc (original)
+++ branches/each-ds/src/lib/auth/cpp/data_source.cc Wed Feb 24 07:53:45 2010
@@ -17,6 +17,8 @@
 namespace isc {
 namespace auth {
 
+// Add a task to the query task queue to look up additional data
+// (i.e., address records for the names included in NS or MX records)
 static void
 getAdditional(Query& q, RRsetPtr rrset) {
     if (!q.wantAdditional()) {
@@ -47,6 +49,8 @@
     }
 }
 
+// Synthesize a CNAME answer, for the benefit of clients that don't
+// understand DNAME
 static void
 synthesizeCname(Query& q, QueryTask& task, RRsetPtr rrset, RRsetList& target) {
     RdataIteratorPtr it;
@@ -76,6 +80,8 @@
     } catch (...) {}
 }
 
+// Add a task to the query task queue to look up the data pointed
+// to by a CNAME record
 static void
 chaseCname(Query& q, QueryTask& task, RRsetPtr rrset) {
     RdataIteratorPtr it;
@@ -97,6 +103,7 @@
     q.tasks().push(*t);
 }
 
+// Perform the query specified in a QueryTask object
 DataSrc::Result
 doQueryTask(const DataSrc* ds, Query& q, QueryTask& task, RRsetList& target) {
     switch (task.op) {
@@ -122,10 +129,167 @@
     return (DataSrc::ERROR);
 }
 
+// Copy referral information into the authority section of a message
+static inline void
+copyAuth(Query& q, const RRsetList& auth) {
+    Message& m = q.message();
+    BOOST_FOREACH(RRsetPtr rrset, auth) {
+        if (rrset->getType() == RRType::DNAME()) {
+            continue;
+        }
+        m.addRRset(Section::AUTHORITY(), rrset, q.wantDnssec());
+        getAdditional(q, rrset);
+    }
+}
+
+// Query for referrals (i.e., NS/DS or DNAME) at a given name
+static inline bool
+refQuery(const Name& name, Query& q, const DataSrc* ds, RRsetList& target) {
+    QueryTask t(name, q.qclass(), QueryTask::REF_QUERY);
+    DataSrc::Result result = doQueryTask(ds, q, t, target);
+    if (result == DataSrc::SUCCESS && t.flags == 0) {
+        return true;
+    }
+    return false;
+}
+
+// Match downward, from the zone apex to the query name, looking for
+// referrals.
+static inline bool
+hasDelegation(const DataSrc* ds, Query& q, QueryTask& task) {
+    Message& m = q.message();
+    int nlen = task.qname.getLabelCount();
+    int diff = nlen - task.zone->getLabelCount();
+    if (diff > 1) {
+        bool found = false;
+        RRsetList ref;
+        for(int i = diff; i > 1; i--) {
+            Name sub(task.qname.split(i - 1, nlen - i));
+            if (refQuery(sub, q, ds, ref)) {
+                found = true;
+                break;
+            }
+        }
+
+        // Found a referral while getting additional data
+        // for something other than NS; we skip it.
+        if (found && task.op == QueryTask::NOGLUE_QUERY) {
+            return (true);
+        }
+
+        // Found a referral while getting answer data;
+        // send a delegation.
+        if (found) {
+            if (RRsetPtr r = ref[RRType::DNAME()]) {
+                RRsetList syn;
+                m.addRRset(Section::ANSWER(), r, q.wantDnssec());
+                synthesizeCname(q, task, r, syn);
+                if (syn.size() == 1) {
+                    m.addRRset(Section::ANSWER(),
+                               syn[RRType::CNAME()],
+                               q.wantDnssec());
+                    chaseCname(q, task, syn[RRType::CNAME()]);
+                    return (true);
+                }
+            }
+
+            copyAuth(q, ref);
+            return (true);
+        }
+    }
+
+    // We appear to have authoritative data; set the header
+    // flag.  (We may clear it later if we find a referral
+    // at the actual qname node.)
+    if (task.op == QueryTask::AUTH_QUERY &&
+        task.state == QueryTask::GETANSWER) {
+        m.setHeaderFlag(MessageFlag::AA());
+    }
+
+    return (false);
+}
+
+// Attempt a wildcard lookup
+static inline DataSrc::Result
+tryWildcard(Query& q, QueryTask& task, const DataSrc* ds, bool& found) {
+    Message& m = q.message();
+    DataSrc::Result result;
+    found = false;
+
+    if ((task.flags & DataSrc::NAME_NOT_FOUND) == 0 || 
+        (task.state != QueryTask::GETANSWER &&
+         task.state != QueryTask::FOLLOWCNAME)) {
+        return (DataSrc::SUCCESS);
+    }
+
+    int nlen = task.qname.getLabelCount();
+    int diff = nlen - task.zone->getLabelCount();
+    if (diff < 1) {
+        return (DataSrc::SUCCESS);
+    }
+
+    RRsetList wild;
+    Name star("*");
+    uint32_t rflags = 0;
+
+    for(int i = 1; i <= diff; i++) {
+        const Name& wname(star.concatenate(task.qname.split(i, nlen - i)));
+        QueryTask t(wname, task.qclass, task.qtype,
+                    QueryTask::SIMPLE_QUERY); 
+        result = doQueryTask(ds, q, t, wild);
+        if (result == DataSrc::SUCCESS &&
+            (t.flags == 0 || (t.flags & DataSrc::CNAME_FOUND))) {
+            rflags = t.flags;
+            found = true;
+            break;
+        }
+    }
+
+    // A wildcard was found.  Add the data to the answer
+    // section (but with the name changed to match the
+    // qname), and then continue as if this were a normal
+    // answer: if a CNAME, chase the target, otherwise
+    // add authority.
+    if (found) {
+        if (rflags & DataSrc::CNAME_FOUND) {
+            if (RRsetPtr rrset = wild[RRType::CNAME()]) {
+                rrset->setName(task.qname);
+                m.addRRset(Section::ANSWER(), rrset, q.wantDnssec());
+                chaseCname(q, task, rrset);
+            }
+        } else {
+            BOOST_FOREACH (RRsetPtr rrset, wild) {
+                rrset->setName(task.qname);
+                m.addRRset(Section::ANSWER(), rrset, q.wantDnssec());
+            }
+
+            RRsetList auth;
+            if (! refQuery(Name(*task.zone), q, ds, auth)) {
+                return (DataSrc::ERROR);
+            }
+
+            copyAuth(q, auth);
+        }
+    } else if (q.wantDnssec()) {
+        // No wildcard found; add an NSEC to prove it
+        RRsetList nsec;
+        QueryTask t = QueryTask(*task.zone, task.qclass, RRType::NSEC(),
+                                QueryTask::SIMPLE_QUERY); 
+        result = doQueryTask(ds, q, t, nsec);
+        if (result != DataSrc::SUCCESS) {
+            return (DataSrc::ERROR);
+        }
+
+        if (t.flags == 0) {
+            m.addRRset(Section::AUTHORITY(), nsec[RRType::NSEC()], true);
+        }
+    }
+
+    return (DataSrc::SUCCESS);
+}
+
 //
 // doQuery: Processes a query.
-// On success, the query status is set to Query::SUCCESS and
-// on failure, Query::FAILURE.
 // 
 void
 DataSrc::doQuery(Query q) {
@@ -139,8 +303,8 @@
 
     m.clearHeaderFlag(MessageFlag::AA());
     while (!q.tasks().empty()) {
-        RdataIteratorPtr it;
         RRsetList data;
+
         QueryTask task = q.tasks().front();
         q.tasks().pop();
 
@@ -148,7 +312,6 @@
         if (task.op == QueryTask::SIMPLE_QUERY ||
             task.op == QueryTask::REF_QUERY) {
             m.setRcode(Rcode::SERVFAIL());
-            q.setStatus(Query::FAILURE);
             return;
         }
 
@@ -161,81 +324,28 @@
         } else {
             search = task.qname;
         }
+
         NameMatch match(search);
         findClosestEnclosure(match);
         const DataSrc* ds = match.bestDataSrc();
         const Name* zone = match.closestName();
 
-        if (ds == NULL) {
-            task.flags = NO_SUCH_ZONE;
-        } else {
+        if (ds != NULL) {
             task.zone = new Name(*zone);
 
             // For these query task types, if there is more than
             // one level between the zone name and qname, we need to
             // check the intermediate nodes for referrals.
-            if (task.op == QueryTask::AUTH_QUERY ||
-                task.op == QueryTask::NOGLUE_QUERY) {
-                int nlen = task.qname.getLabelCount();
-                int diff = nlen - zone->getLabelCount();
-                if (diff > 1) {
-                    bool found = false;
-                    RRsetList ref;
-                    for(int i = diff; i > 1; i--) {
-                        Name sub(task.qname.split(i - 1, nlen - i));
-                        QueryTask t(sub, task.qclass, QueryTask::REF_QUERY);
-                        result = doQueryTask(ds, q, t, ref);
-                        if (result == SUCCESS && t.flags == 0) {
-                            found = true;
-                            break;
-                        }
-                    }
-
-                    // Found a referral while getting additional data
-                    // for something other than NS; we skip it.
-                    if (found && task.op == QueryTask::NOGLUE_QUERY) {
-                        continue;
-                    }
-
-                    // Found a referral while getting answer data;
-                    // send a delegation.
-                    if (found) {
-                        if (RRsetPtr r = ref[RRType::DNAME()]) {
-                            RRsetList syn;
-                            m.addRRset(Section::ANSWER(), r,
-                                       q.wantDnssec());
-                            synthesizeCname(q, task, r, syn);
-                            if (syn.size() == 1) {
-                                m.addRRset(Section::ANSWER(), syn[RRType::CNAME()],
-                                           q.wantDnssec());
-                                chaseCname(q, task, syn[RRType::CNAME()]);
-                                continue;
-                            }
-                        }
-                        BOOST_FOREACH (RRsetPtr r, ref) {
-                            if (r->getType() != RRType::DNAME()) {
-                                m.addRRset(Section::AUTHORITY(), r,
-                                           q.wantDnssec());
-                                getAdditional(q, r);
-                            }
-                        }
-                        continue;
-                    }
-                }
-
-                // We appear to have authoritative data; set the header
-                // flag.  (We may clear it later if we find a referral
-                // at the actual qname node.)
-                if (task.op == QueryTask::AUTH_QUERY &&
-                    task.state == QueryTask::GETANSWER) {
-                    m.setHeaderFlag(MessageFlag::AA());
-                }
+            if ((task.op == QueryTask::AUTH_QUERY ||
+                 task.op == QueryTask::NOGLUE_QUERY) &&
+                hasDelegation(ds, q, task))
+            {
+                continue;
             }
 
             result = doQueryTask(ds, q, task, data);
             if (result != SUCCESS) {
                 m.setRcode(Rcode::SERVFAIL());
-                q.setStatus(Query::FAILURE);
                 return;
             }
 
@@ -249,6 +359,8 @@
                  task.qtype == RRType::DNAME())) {
                 task.flags &= ~REFERRAL;
             }
+        } else {
+            task.flags = NO_SUCH_ZONE;
         }
 
         if (result == SUCCESS && task.flags == 0) {
@@ -268,33 +380,23 @@
                 }
                 q.setStatus(Query::ANSWERED);
                 if (need_auth && !have_ns) {
-                // Data found, no additional processing needed.
-                // Add the NS records for the enclosing zone to
-                // the authority section.
+                    // Data found, no additional processing needed.
+                    // Add the NS records for the enclosing zone to
+                    // the authority section.
                     RRsetList auth;
-                    QueryTask t(Name(*zone), task.qclass,
-                                QueryTask::REF_QUERY); 
-                    result = doQueryTask(ds, q, t, auth);
-                    if (result != SUCCESS) {
+                    if (! refQuery(Name(*zone), q, ds, auth)) {
                         m.setRcode(Rcode::SERVFAIL());
-                        q.setStatus(Query::FAILURE);
                         return;
                     }
 
-                    BOOST_FOREACH(RRsetPtr rrset, auth) {
-                        if (rrset->getType() == RRType::DNAME()) {
-                            continue;
-                        }
-                        m.addRRset(Section::AUTHORITY(), rrset,
-                                   q.wantDnssec());
-                        getAdditional(q, rrset);
-                    }
+                    copyAuth(q, auth);
                 }
                 continue;
 
             case QueryTask::GETADDITIONAL:
                 // Got additional data.  Do not add it to the message
-                // yet; instead store it and copy it in at the end.
+                // yet; instead store it and copy it in at the end
+                // (this allow RRSIGs to be omitted if necessary).
                 BOOST_FOREACH(RRsetPtr rrset, data) {
                     if (q.status() == Query::ANSWERED &&
                         rrset->getName() == q.qname() &&
@@ -310,7 +412,6 @@
             }
         } else if (result == ERROR || result == NOT_IMPLEMENTED) {
             m.setRcode(Rcode::SERVFAIL());
-            q.setStatus(Query::FAILURE);
             return;
         } else if (task.flags & CNAME_FOUND) {
             // The qname node contains a CNAME.  Add a new task to the
@@ -325,11 +426,8 @@
             if (task.state == QueryTask::GETANSWER) {
                 RRsetList auth;
                 m.clearHeaderFlag(MessageFlag::AA());
-                QueryTask t(task.qname, task.qclass, QueryTask::REF_QUERY);
-                result = doQueryTask(ds, q, t, auth);
-                if (result != SUCCESS) {
+                if (! refQuery(task.qname, q, ds, auth)) {
                     m.setRcode(Rcode::SERVFAIL());
-                    q.setStatus(Query::FAILURE);
                     return;
                 }
                 BOOST_FOREACH (RRsetPtr rrset, auth) {
@@ -352,7 +450,6 @@
             // REFUSED.
             if (task.state == QueryTask::GETANSWER) {
                 m.setRcode(Rcode::REFUSED());
-                q.setStatus(Query::FAILURE);
                 return;
             }
             continue;
@@ -361,91 +458,15 @@
             // If we were looking for answer data, not additional,
             // and the name was not found, we need to find out whether
             // there are any relevant wildcards.
-            if (task.flags & NAME_NOT_FOUND && 
-                (task.state == QueryTask::GETANSWER ||
-                 task.state == QueryTask::FOLLOWCNAME)) {
-                Result r;
-                int nlen = task.qname.getLabelCount();
-                int diff = nlen - zone->getLabelCount();
-                if (diff >= 1) {
-                    bool found = false;
-                    RRsetList wild;
-                    Name star("*");
-                    uint32_t rflags = 0;
-
-                    for(int i = 1; i <= diff; i++) {
-                        const Name& wname(star.concatenate(task.qname.split(i,
-                                                           nlen - i)));
-                        QueryTask t(wname, task.qclass, task.qtype,
-                                    QueryTask::SIMPLE_QUERY); 
-                        r = doQueryTask(ds, q, t, wild);
-                        if (r == SUCCESS &&
-                            (t.flags == 0 || (t.flags & CNAME_FOUND))) {
-                            rflags = t.flags;
-                            found = true;
-                            break;
-                        }
-                    }
-
-                    // A wildcard was found.  Add the data to the answer
-                    // section (but with the name changed to match the
-                    // qname), and then continue as if this were a normal
-                    // answer: if a CNAME, chase the target, otherwise
-                    // add authority.
-                    if (found) {
-                        if (rflags & CNAME_FOUND) {
-                            if (RRsetPtr rrset = wild[RRType::CNAME()]) {
-                                rrset->setName(task.qname);
-                                m.addRRset(Section::ANSWER(), rrset,
-                                           q.wantDnssec());
-                                chaseCname(q, task, rrset);
-                            }
-                        } else {
-                            BOOST_FOREACH (RRsetPtr rrset, wild) {
-                                rrset->setName(task.qname);
-                                m.addRRset(Section::ANSWER(), rrset,
-                                           q.wantDnssec());
-                            }
-
-                            RRsetList auth;
-                            QueryTask t(Name(*zone), task.qclass,
-                                        QueryTask::REF_QUERY); 
-                            r = doQueryTask(ds, q, t, auth);
-                            if (r != SUCCESS || t.flags != 0) {
-                                m.setRcode(Rcode::SERVFAIL());
-                                q.setStatus(Query::FAILURE);
-                                return;
-                            }
-
-                            BOOST_FOREACH (RRsetPtr rrset, auth) {
-                                if (rrset->getType() == RRType::DNAME()) {
-                                    continue;
-                                }
-                                m.addRRset(Section::AUTHORITY(), rrset,
-                                           q.wantDnssec());
-                                getAdditional(q, rrset);
-                            }
-                        }
-                        continue;
-                    } else if (q.wantDnssec()) {
-                        // No wildcard found; add an NSEC to prove it
-                        RRsetList nsec;
-                        QueryTask t = QueryTask(*task.zone, task.qclass,
-                                                RRType::NSEC(),
-                                                QueryTask::SIMPLE_QUERY); 
-                        result = doQueryTask(ds, q, t, nsec);
-                        if (result != SUCCESS) {
-                            m.setRcode(Rcode::SERVFAIL());
-                            q.setStatus(Query::FAILURE);
-                            return;
-                        }
-
-                        if (t.flags == 0) {
-                            m.addRRset(Section::AUTHORITY(), nsec[RRType::NSEC()], true);
-                        }
-
-                    }
-                }
+            bool wildcard_found;
+            result = tryWildcard(q, task, ds, wildcard_found);
+            if (result != SUCCESS) {
+                m.setRcode(Rcode::SERVFAIL());
+                return;
+            }
+
+            if (wildcard_found) {
+                continue;
             }
 
             // If we've reached this point, there is definitely no answer.
@@ -461,7 +482,9 @@
             }
 
             if (task.state == QueryTask::GETANSWER) {
-                m.setRcode(Rcode::NXDOMAIN());
+                if (task.flags & NAME_NOT_FOUND) {
+                    m.setRcode(Rcode::NXDOMAIN());
+                }
 
                 RRsetList soa;
                 QueryTask t(Name(*zone), task.qclass, RRType::SOA(), 
@@ -469,11 +492,11 @@
                 result = doQueryTask(ds, q, t, soa);
                 if (result != SUCCESS || t.flags != 0) {
                     m.setRcode(Rcode::SERVFAIL());
-                    q.setStatus(Query::FAILURE);
                     return;
                 }
 
-                m.addRRset(Section::AUTHORITY(), soa[RRType::SOA()], q.wantDnssec());
+                m.addRRset(Section::AUTHORITY(), soa[RRType::SOA()],
+                           q.wantDnssec());
             }
 
             if (q.wantDnssec()) {
@@ -484,22 +507,19 @@
                 result = doQueryTask(ds, q, t, nsec);
                 if (result != SUCCESS) {
                     m.setRcode(Rcode::SERVFAIL());
-                    q.setStatus(Query::FAILURE);
                     return;
                 }
 
                 if (t.flags == 0) {
-                    // There should only be one NSEC record
-                    m.addRRset(Section::AUTHORITY(), nsec[RRType::NSEC()], true);
-                }
-            }
-
-            q.setStatus(Query::FAILURE);
+                    m.addRRset(Section::AUTHORITY(), nsec[RRType::NSEC()],
+                               true);
+                }
+            }
+
             return;
         } else {
             // Should never be reached!
             m.setRcode(Rcode::SERVFAIL());
-            q.setStatus(Query::FAILURE);
             return;
         }
     }
@@ -511,16 +531,15 @@
     BOOST_FOREACH(RRsetPtr rrset, additional) {
         m.addRRset(Section::ADDITIONAL(), rrset, false);
     }
+
     if (q.wantDnssec()) {
         BOOST_FOREACH(RRsetPtr rrset, additional) {
             if (rrset->getRRsig()) {
-                m.addRRset(Section::ADDITIONAL(),
-                           rrset->getRRsig(), false);
-            }
-        }
-    }
-    q.setStatus(Query::SUCCESS);
-}
-
-}
-}
+                m.addRRset(Section::ADDITIONAL(), rrset->getRRsig(), false);
+            }
+        }
+    }
+}
+
+}
+}

Modified: branches/each-ds/src/lib/auth/cpp/query.h
==============================================================================
--- branches/each-ds/src/lib/auth/cpp/query.h (original)
+++ branches/each-ds/src/lib/auth/cpp/query.h Wed Feb 24 07:53:45 2010
@@ -179,12 +179,10 @@
 // Data Source query
 class Query {
 public:
-    // The state of a query: running, succeeded, or failed.
+    // The state of a query: pending or answered.
     enum Status {
-        RUNNING,
-        ANSWERED,
-        SUCCESS,
-        FAILURE
+        PENDING,
+        ANSWERED
     };
 
     // Query constructor
@@ -192,7 +190,7 @@
         message_ = &m;
         want_additional = true;
         want_dnssec = dnssec;
-        status_ = RUNNING;
+        status_ = PENDING;
 
         // Check message formatting
         if (message_->getRRCount(Section::QUESTION()) != 1) {

Modified: branches/each-ds/src/lib/dns/cpp/rrset.h
==============================================================================
--- branches/each-ds/src/lib/dns/cpp/rrset.h (original)
+++ branches/each-ds/src/lib/dns/cpp/rrset.h Wed Feb 24 07:53:45 2010
@@ -179,7 +179,9 @@
 
     virtual void setName(const Name& n) {
         this->BasicRRset::setName(n);
-        rrsig_->BasicRRset::setName(n);
+        if (rrsig_) {
+            rrsig_->BasicRRset::setName(n);
+        }
     }
 
     virtual void setName(const Name* n) {




More information about the bind10-changes mailing list