Browse Source

[1063] Fix NS delegation handling

Sometimes it was in wrong order or strange data wasn't found.
Michal 'vorner' Vaner 13 years ago
parent
commit
62432e71ef
2 changed files with 25 additions and 18 deletions
  1. 19 15
      src/lib/datasrc/database.cc
  2. 6 3
      src/lib/datasrc/database.h

+ 19 - 15
src/lib/datasrc/database.cc

@@ -200,16 +200,30 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
             // of the 'type covered' field in the RRSIG Rdata).
             //cur_sigtype(columns[SIGTYPE_COLUMN]);
 
-            if (type != NULL && cur_type == *type) {
+            // 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, 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()) {
-                    // In case there's a NS, we should find the delegation, not
-                    // the data
-                    continue;
+                    isc_throw(DataSourceError, "NS found together with data"
+                              " in non-apex domain " + name.toText());
                 }
                 addOrCreate(result_rrset, name, getClass(), cur_type, cur_ttl,
                             columns[DatabaseAccessor::RDATA_COLUMN],
@@ -224,16 +238,6 @@ DatabaseClient::Finder::getRRset(const isc::dns::Name& name,
                 addOrCreate(result_rrset, name, getClass(), cur_type, cur_ttl,
                             columns[DatabaseAccessor::RDATA_COLUMN],
                             *database_);
-            } else if (want_ns && cur_type == isc::dns::RRType::NS()) {
-                if (result_rrset &&
-                    // In case we already found some data here, we should
-                    // replace it with the NS, we should delegate
-                    result_rrset->getType() != isc::dns::RRType::NS()) {
-                    result_rrset = isc::dns::RRsetPtr();
-                }
-                addOrCreate(result_rrset, 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 &&
@@ -325,7 +329,7 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
             found = getRRset(name, &type, true, false, name != origin);
             records_found = found.first;
             result_rrset = found.second;
-            if (result_rrset && type != isc::dns::RRType::NS() &&
+            if (result_rrset && name != origin &&
                 result_rrset->getType() == isc::dns::RRType::NS()) {
                 result_status = DELEGATION;
             } else if (result_rrset && type != isc::dns::RRType::CNAME() &&

+ 6 - 3
src/lib/datasrc/database.h

@@ -318,9 +318,12 @@ public:
          *     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.
-         * \todo When want_ns is true and there's another RRtype, we should
-         *     throw, but we don't yet.
+         * \param want_ns This allows redirection by NS to be returned. If
+         *     any other data is met as well, DataSourceError is thrown.
+         * \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