BIND 10 trac1176, updated. 04b04226b726b6e1fea6bba970556b9ed5cc3446 [1176] Cleanup of previous commit

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Sep 6 12:35:38 UTC 2011


The branch, trac1176 has been updated
  discards  30ade643ceb25a67bd2c27df2dd4eb4001fc60c8 (commit)

This update discarded existing revisions and left the branch pointing at
a previous point in the repository history.

 * -- * -- N (04b04226b726b6e1fea6bba970556b9ed5cc3446)
            \
             O -- O -- O (30ade643ceb25a67bd2c27df2dd4eb4001fc60c8)

The removed revisions are not necessarilly gone - if another reference
still refers to them they will stay in the repository.

No new revisions were added by this update.

Summary of changes:
 src/lib/datasrc/database.cc |  271 +++++++++++++++++++------------------------
 src/lib/datasrc/database.h  |   71 +++++++-----
 2 files changed, 158 insertions(+), 184 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 8571453..347e3ab 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -171,13 +171,16 @@ private:
 };
 }
 
-DatabaseClient::Finder::FoundRRsets
-DatabaseClient::Finder::getRRsets(const Name& name, const WantedTypes& types,
-                                  bool check_ns, const Name* construct_name)
+std::pair<bool, isc::dns::RRsetPtr>
+DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
+                                 const isc::dns::RRType* type,
+                                 bool want_cname, bool want_dname,
+                                 bool want_ns,
+                                 const isc::dns::Name* construct_name)
 {
     RRsigStore sig_store;
     bool records_found = false;
-    std::map<RRType, RRsetPtr> result;
+    isc::dns::RRsetPtr result_rrset;
 
     // Request the context
     DatabaseAccessor::IteratorContextPtr
@@ -192,19 +195,81 @@ DatabaseClient::Finder::getRRsets(const Name& name, const WantedTypes& types,
     if (construct_name == NULL) {
         construct_name = &name;
     }
-
-    bool seen_cname(false);
-    bool seen_other(false);
-    bool seen_ns(false);
-
     while (context->getNext(columns)) {
-        // The domain is not empty
-        records_found = true;
+        if (!records_found) {
+            records_found = true;
+        }
 
         try {
-            const RRType cur_type(columns[DatabaseAccessor::TYPE_COLUMN]);
-
-            if (cur_type == RRType::RRSIG()) {
+            const isc::dns::RRType cur_type(columns[DatabaseAccessor::
+                                            TYPE_COLUMN]);
+            const isc::dns::RRTTL cur_ttl(columns[DatabaseAccessor::
+                                          TTL_COLUMN]);
+            // Ths sigtype column was an optimization for finding the
+            // relevant RRSIG RRs for a lookup. Currently this column is
+            // not used in this revised datasource implementation. We
+            // should either start using it again, or remove it from use
+            // completely (i.e. also remove it from the schema and the
+            // backend implementation).
+            // Note that because we don't use it now, we also won't notice
+            // it if the value is wrong (i.e. if the sigtype column
+            // contains an rrtype that is different from the actual value
+            // of the 'type covered' field in the RRSIG Rdata).
+            //cur_sigtype(columns[SIGTYPE_COLUMN]);
+
+            // Check for delegations before checking for the right type.
+            // This is needed to properly delegate request for the NS
+            // record itself.
+            //
+            // This happens with NS only, CNAME must be alone and DNAME
+            // is not checked in the exact queried domain.
+            if (want_ns && cur_type == isc::dns::RRType::NS()) {
+                if (result_rrset &&
+                    result_rrset->getType() != isc::dns::RRType::NS()) {
+                    isc_throw(DataSourceError, "NS found together with data"
+                              " in non-apex domain " + name.toText());
+                }
+                addOrCreate(result_rrset, *construct_name, getClass(),
+                            cur_type, cur_ttl,
+                            columns[DatabaseAccessor::RDATA_COLUMN],
+                            *database_);
+            } else if (type != NULL && cur_type == *type) {
+                if (result_rrset &&
+                    result_rrset->getType() == isc::dns::RRType::CNAME()) {
+                    isc_throw(DataSourceError, "CNAME found but it is not "
+                              "the only record for " + name.toText());
+                } else if (result_rrset && want_ns &&
+                           result_rrset->getType() == isc::dns::RRType::NS()) {
+                    isc_throw(DataSourceError, "NS found together with data"
+                              " in non-apex domain " + name.toText());
+                }
+                addOrCreate(result_rrset, *construct_name, getClass(),
+                            cur_type, cur_ttl,
+                            columns[DatabaseAccessor::RDATA_COLUMN],
+                            *database_);
+            } else if (want_cname && cur_type == isc::dns::RRType::CNAME()) {
+                // There should be no other data, so result_rrset should
+                // be empty.
+                if (result_rrset) {
+                    isc_throw(DataSourceError, "CNAME found but it is not "
+                              "the only record for " + name.toText());
+                }
+                addOrCreate(result_rrset, *construct_name, getClass(),
+                            cur_type, cur_ttl,
+                            columns[DatabaseAccessor::RDATA_COLUMN],
+                            *database_);
+            } else if (want_dname && cur_type == isc::dns::RRType::DNAME()) {
+                // There should be max one RR of DNAME present
+                if (result_rrset &&
+                    result_rrset->getType() == isc::dns::RRType::DNAME()) {
+                    isc_throw(DataSourceError, "DNAME with multiple RRs in " +
+                              name.toText());
+                }
+                addOrCreate(result_rrset, *construct_name, getClass(),
+                            cur_type, cur_ttl,
+                            columns[DatabaseAccessor::RDATA_COLUMN],
+                            *database_);
+            } else if (cur_type == isc::dns::RRType::RRSIG()) {
                 // If we get signatures before we get the actual data, we
                 // can't know which ones to keep and which to drop...
                 // So we keep a separate store of any signature that may be
@@ -212,67 +277,27 @@ DatabaseClient::Finder::getRRsets(const Name& name, const WantedTypes& types,
                 // done.
                 // A possible optimization here is to not store them for
                 // types we are certain we don't need
-                sig_store.addSig(rdata::createRdata(cur_type, getClass(),
-                     columns[DatabaseAccessor::RDATA_COLUMN]));
-            }
-
-            if (types.find(cur_type) != types.end()) {
-                // This type is requested, so put it into result
-                const RRTTL cur_ttl(columns[DatabaseAccessor::TTL_COLUMN]);
-                // Ths sigtype column was an optimization for finding the
-                // relevant RRSIG RRs for a lookup. Currently this column is
-                // not used in this revised datasource implementation. We
-                // should either start using it again, or remove it from use
-                // completely (i.e. also remove it from the schema and the
-                // backend implementation).
-                // Note that because we don't use it now, we also won't notice
-                // it if the value is wrong (i.e. if the sigtype column
-                // contains an rrtype that is different from the actual value
-                // of the 'type covered' field in the RRSIG Rdata).
-                //cur_sigtype(columns[SIGTYPE_COLUMN]);
-                addOrCreate(result[cur_type], *construct_name, getClass(),
-                            cur_type, cur_ttl,
-                            columns[DatabaseAccessor::RDATA_COLUMN],
-                            *database_);
-            }
-
-            if (cur_type == RRType::CNAME()) {
-                seen_cname = true;
-            } else if (cur_type == RRType::NS()) {
-                seen_ns = true;
-            } else if (cur_type != RRType::RRSIG()) {//RRSIG can live anywhere
-                // FIXME: Is something else allowed in the delegation point? DS?
-                seen_other = true;
+                sig_store.addSig(isc::dns::rdata::createRdata(cur_type,
+                    getClass(), columns[DatabaseAccessor::RDATA_COLUMN]));
             }
-        } catch (const InvalidRRType& irt) {
+        } catch (const isc::dns::InvalidRRType& irt) {
             isc_throw(DataSourceError, "Invalid RRType in database for " <<
                       name << ": " << columns[DatabaseAccessor::
                       TYPE_COLUMN]);
-        } catch (const InvalidRRTTL& irttl) {
+        } catch (const isc::dns::InvalidRRTTL& irttl) {
             isc_throw(DataSourceError, "Invalid TTL in database for " <<
                       name << ": " << columns[DatabaseAccessor::
                       TTL_COLUMN]);
-        } catch (const rdata::InvalidRdataText& ird) {
+        } catch (const isc::dns::rdata::InvalidRdataText& ird) {
             isc_throw(DataSourceError, "Invalid rdata in database for " <<
                       name << ": " << columns[DatabaseAccessor::
                       RDATA_COLUMN]);
         }
     }
-    if (seen_cname && (seen_other || seen_ns)) {
-        isc_throw(DataSourceError, "CNAME shares domain " << name <<
-                  " with something else");
-    }
-    if (check_ns && seen_ns && seen_other) {
-        isc_throw(DataSourceError, "NS shares domain " << name <<
-                  " with something else");
+    if (result_rrset) {
+        sig_store.appendSignatures(result_rrset);
     }
-    // Add signatures to all found RRsets
-    for (std::map<RRType, RRsetPtr>::iterator i(result.begin());
-         i != result.end(); ++ i) {
-        sig_store.appendSignatures(i->second);
-    }
-
-    return (FoundRRsets(records_found, result));
+    return (std::pair<bool, isc::dns::RRsetPtr>(records_found, result_rrset));
 }
 
 bool
@@ -289,21 +314,6 @@ DatabaseClient::Finder::hasSubdomains(const std::string& name) {
     return (context->getNext(columns));
 }
 
-// Some manipulation with RRType sets
-namespace {
-
-std::set<RRType> empty_types;
-
-// To conveniently put the RRTypes into the sets. This is not really clean
-// design, but it is hidden inside this file and makes the calls much more
-// convenient.
-std::set<RRType> operator +(std::set<RRType> set, const RRType& type) {
-    set.insert(type);
-    return (set);
-}
-
-}
-
 ZoneFinder::FindResult
 DatabaseClient::Finder::find(const isc::dns::Name& name,
                              const isc::dns::RRType& type,
@@ -316,7 +326,7 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
     bool glue_ok(options & FIND_GLUE_OK);
     isc::dns::RRsetPtr result_rrset;
     ZoneFinder::Result result_status = SUCCESS;
-    FoundRRsets found;
+    std::pair<bool, isc::dns::RRsetPtr> found;
     logger.debug(DBG_TRACE_DETAILED, DATASRC_DATABASE_FIND_RECORDS)
         .arg(database_->getDBName()).arg(name).arg(type);
     // In case we are in GLUE_OK mode and start matching wildcards,
@@ -332,52 +342,39 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
     // This is how many labels we remove to get origin
     size_t remove_labels(current_label_count - origin_label_count);
 
-    // Type shortcut, used a lot here
-    typedef std::map<RRType, RRsetPtr>::const_iterator FoundIterator;
-
     // Now go trough all superdomains from origin down
     for (int i(remove_labels); i > 0; --i) {
         Name superdomain(name.split(i));
         // Look if there's NS or DNAME (but ignore the NS in origin)
-        static WantedTypes delegation_types(empty_types + RRType::DNAME() +
-                                            RRType::NS());
-        found = getRRsets(superdomain, delegation_types, i != remove_labels);
+        found = getRRset(superdomain, NULL, false, true,
+                         i != remove_labels && !glue_ok);
         if (found.first) {
             // It contains some RRs, so it exists.
             last_known = superdomain.getLabelCount();
-
-            const FoundIterator nsi(found.second.find(RRType::NS()));
-            const FoundIterator dni(found.second.find(RRType::DNAME()));
-            // In case we are in GLUE_OK mode, we want to store the
-            // highest encountered NS (but not apex)
-            if (glue_ok && !first_ns && i != remove_labels &&
-                nsi != found.second.end()) {
-                first_ns = nsi->second;
-            } else if (!glue_ok && i != remove_labels &&
-                       nsi != found.second.end()) {
-                // Do a NS delegation, but ignore NS in glue_ok mode. Ignore
-                // delegation in apex
+            // In case we are in GLUE_OK, we want to store the highest
+            // encountered RRset.
+            if (glue_ok && !first_ns && i != remove_labels) {
+                first_ns = getRRset(superdomain, NULL, false, false,
+                                    true).second;
+            }
+        }
+        if (found.second) {
+            // We found something redirecting somewhere else
+            // (it can be only NS or DNAME here)
+            result_rrset = found.second;
+            if (result_rrset->getType() == isc::dns::RRType::NS()) {
                 LOG_DEBUG(logger, DBG_TRACE_DETAILED,
                           DATASRC_DATABASE_FOUND_DELEGATION).
                     arg(database_->getDBName()).arg(superdomain);
-                result_rrset = nsi->second;
                 result_status = DELEGATION;
-                // No need to go lower, found
-                break;
-            } else if (dni != found.second.end()) {
-                // Very similar with DNAME
+            } else {
                 LOG_DEBUG(logger, DBG_TRACE_DETAILED,
                           DATASRC_DATABASE_FOUND_DNAME).
                     arg(database_->getDBName()).arg(superdomain);
-                result_rrset = dni->second;
                 result_status = DNAME;
-                if (result_rrset->getRdataCount() != 1) {
-                    isc_throw(DataSourceError, "DNAME at " << superdomain <<
-                              " has " << result_rrset->getRdataCount() <<
-                              " rdata, 1 expected");
-                }
-                break;
             }
+            // Don't search more
+            break;
         }
     }
 
@@ -385,37 +382,21 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
         // Try getting the final result and extract it
         // It is special if there's a CNAME or NS, DNAME is ignored here
         // And we don't consider the NS in origin
-
-        static WantedTypes final_types(empty_types + RRType::CNAME() +
-                                       RRType::NS());
-        found = getRRsets(name, final_types + type, name != origin);
+        found = getRRset(name, &type, true, false, name != origin && !glue_ok);
         records_found = found.first;
-
-        // NS records, CNAME record and Wanted Type records
-        const FoundIterator nsi(found.second.find(RRType::NS()));
-        const FoundIterator cni(found.second.find(RRType::CNAME()));
-        const FoundIterator wti(found.second.find(type));
-        if (name != origin && !glue_ok && nsi != found.second.end()) {
-            // There's a delegation at the exact node.
+        result_rrset = found.second;
+        if (result_rrset && name != origin && !glue_ok &&
+            result_rrset->getType() == isc::dns::RRType::NS()) {
             LOG_DEBUG(logger, DBG_TRACE_DETAILED,
                       DATASRC_DATABASE_FOUND_DELEGATION_EXACT).
                 arg(database_->getDBName()).arg(name);
             result_status = DELEGATION;
-            result_rrset = nsi->second;
-        } else if (type != isc::dns::RRType::CNAME() &&
-                   cni != found.second.end()) {
-            // A CNAME here
+        } else if (result_rrset && type != isc::dns::RRType::CNAME() &&
+                   result_rrset->getType() == isc::dns::RRType::CNAME()) {
             result_status = CNAME;
-            result_rrset = cni->second;
-            if (result_rrset->getRdataCount() != 1) {
-                isc_throw(DataSourceError, "CNAME with " <<
-                          result_rrset->getRdataCount() <<
-                          " rdata at " << name << ", expected 1");
-            }
-        } else if (wti != found.second.end()) {
-            // Just get the answer
-            result_rrset = wti->second;
-        } else if (!records_found) {
+        }
+
+        if (!result_rrset && !records_found) {
             // Nothing lives here.
             // But check if something lives below this
             // domain and if so, pretend something is here as well.
@@ -439,11 +420,8 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
                     Name superdomain(name.split(i));
                     Name wildcard(star.concatenate(superdomain));
                     // TODO What do we do about DNAME here?
-                    static WantedTypes wildcard_types(empty_types +
-                                                      RRType::CNAME() +
-                                                      RRType::NS());
-                    found = getRRsets(wildcard, wildcard_types + type, true,
-                                      &name);
+                    found = getRRset(wildcard, &type, true, false, true,
+                                     &name);
                     if (found.first) {
                         if (first_ns) {
                             // In case we are under NS, we don't
@@ -464,22 +442,7 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
                             // domain, but it could be empty non-terminal. In
                             // that case, we need to cancel the match.
                             records_found = true;
-                            const FoundIterator
-                                cni(found.second.find(RRType::CNAME()));
-                            const FoundIterator
-                                nsi(found.second.find(RRType::NS()));
-                            const FoundIterator wti(found.second.find(type));
-                            if (cni != found.second.end() &&
-                                type != RRType::CNAME()) {
-                                result_rrset = cni->second;
-                                result_status = CNAME;
-                            } else if (nsi != found.second.end()) {
-                                result_rrset = nsi->second;
-                                result_status = DELEGATION;
-                            } else if (wti != found.second.end()) {
-                                result_rrset = wti->second;
-                            }
-
+                            result_rrset = found.second;
                             LOG_DEBUG(logger, DBG_TRACE_DETAILED,
                                       DATASRC_DATABASE_WILDCARD).
                                 arg(database_->getDBName()).arg(wildcard).
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index 48f292e..1120a3e 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -20,9 +20,6 @@
 #include <dns/name.h>
 #include <exceptions/exceptions.h>
 
-#include <map>
-#include <set>
-
 namespace isc {
 namespace datasrc {
 
@@ -355,38 +352,52 @@ public:
         boost::shared_ptr<DatabaseAccessor> database_;
         const int zone_id_;
         const isc::dns::Name origin_;
-        /// \brief Shortcut name for the result of getRRsets
-        typedef std::pair<bool, std::map<dns::RRType, dns::RRsetPtr> >
-            FoundRRsets;
-        /// \brief Just shortcut for set of types
-        typedef std::set<dns::RRType> WantedTypes;
         /**
-         * \brief Searches the database for RRsets for the given domain.
+         * \brief Searches database for an RRset
          *
-         * It searches the given domain and provides all RRsets it can find
-         * and are listed as wanted.
+         * This method scans RRs of single domain specified by name and finds
+         * RRset with given type or any of redirection RRsets that are
+         * requested.
          *
-         * It also performs some of the checks, where one type isn't allowed
-         * to coexist with another one and throws in case they are found.
+         * This function is used internally by find(), because this part is
+         * called multiple times with slightly different parameters.
          *
-         * \param name The domain to search.
-         * \param types Set of types that should be returned, if found.
-         * \param check_ns Should it perform a check if NS lives together
-         *     with something? It should be false in apex, as the NS there
-         *     can live together with anything, but false everywhere else.
-         * \param construct_name If not null, it overrides the name for
-         *     which the result RRsets are constructed. This is useful for
-         *     wildcards.
-         * \result The first part of the pair informs wheather there are
-         *     any RRs in the domain (even ones that are not in types).
-         *     The second part is map holding found RRsets (the ones that
-         *     are in types but are not found are not present, not even as
-         *     NULL pointers).
-         * \thrown DataSourceError if two types that can't coexist are found.
+         * \param name Which domain name should be scanned.
+         * \param type The RRType which is requested. This can be NULL, in
+         *     which case the method will look for the redirections only.
+         * \param want_cname If this is true, CNAME redirection may be returned
+         *     instead of the RRset with given type. If there's CNAME and
+         *     something else or the CNAME has multiple RRs, it throws
+         *     DataSourceError.
+         * \param want_dname If this is true, DNAME redirection may be returned
+         *     instead. This is with type = NULL only and is not checked in
+         *     other circumstances. If the DNAME has multiple RRs, it throws
+         *     DataSourceError.
+         * \param want_ns This allows redirection by NS to be returned. If
+         *     any other data is met as well, DataSourceError is thrown.
+         * \param construct_name If set to non-NULL, the resulting RRset will
+         *     be constructed for this name instead of the queried one. This
+         *     is useful for wildcards.
+         * \note It may happen that some of the above error conditions are not
+         *     detected in some circumstances. The goal here is not to validate
+         *     the domain in DB, but to avoid bad behaviour resulting from
+         *     broken data.
+         * \return First part of the result tells if the domain contains any
+         *     RRs. This can be used to decide between NXDOMAIN and NXRRSET.
+         *     The second part is the RRset found (if any) with any relevant
+         *     signatures attached to it.
+         * \todo This interface doesn't look very elegant. Any better idea
+         *     would be nice.
          */
-        FoundRRsets getRRsets(const dns::Name& name, const WantedTypes& types,
-                              bool check_ns,
-                              const dns::Name* construct_name = NULL);
+        std::pair<bool, isc::dns::RRsetPtr> getRRset(const isc::dns::Name&
+                                                     name,
+                                                     const isc::dns::RRType*
+                                                     type,
+                                                     bool want_cname,
+                                                     bool want_dname,
+                                                     bool want_ns, const
+                                                     isc::dns::Name*
+                                                     construct_name = NULL);
         /**
          * \brief Checks if something lives below this domain.
          *




More information about the bind10-changes mailing list