BIND 10 trac1198, updated. 52802deb18632b028815d25f19976d0576d76e1f [1198] Changes as a result of review
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Dec 7 16:27:18 UTC 2011
The branch, trac1198 has been updated
via 52802deb18632b028815d25f19976d0576d76e1f (commit)
from a60c96464c0b959492a13b10767a7d9352be060e (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 52802deb18632b028815d25f19976d0576d76e1f
Author: Stephen Morris <stephen at isc.org>
Date: Wed Dec 7 16:26:47 2011 +0000
[1198] Changes as a result of review
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/database.cc | 45 +++++++++++++---------------------
src/lib/datasrc/datasrc_messages.mes | 11 ++++----
src/lib/datasrc/zone.h | 15 +++++++----
tools/reorder_message_file.py | 10 ++++----
4 files changed, 36 insertions(+), 45 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 91566b3..dd88d51 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -409,9 +409,9 @@ DatabaseClient::Finder::findDelegationPoint(const isc::dns::Name& name,
//
// The one case where this is forbidden is when we search past the zone
// cut but the match we find for the glue is a wildcard match. In that
- // case, we return the delegation instead. To save a new search, we record
- // the location of the delegation cut when we encounter it here.
- // TODO: where does it say we can't return wildcard glue?
+ // case, we return the delegation instead (see RFC 1034, section 4.3.3).
+ // To save a new search, we record the location of the delegation cut when
+ // we encounter it here.
isc::dns::ConstRRsetPtr first_ns;
// We want to search from the apex down. We are given the full domain
@@ -536,7 +536,8 @@ DatabaseClient::Finder::findWildcardMatch(
const string wildcard("*." + superdomain.toText());
const string construct_name(name.toText());
- // TODO What do we do about DNAME here?
+ // TODO Add a check for DNAME, as DNAME wildcards are discouraged (see
+ // RFC 4592 section 4.4).
// Search for a match. The types are the same as with original query.
FoundRRsets found = getRRsets(wildcard, final_types, true,
&construct_name);
@@ -565,7 +566,6 @@ DatabaseClient::Finder::findWildcardMatch(
result_status = DELEGATION;
result_rrset = dresult.first_ns;
-
} else if (!hasSubdomains(name.split(i - 1).toText())) {
// We found a wildcard match and we are sure that the match
// is not an empty non-terminal (E.g. searching for a match
@@ -683,8 +683,7 @@ DatabaseClient::Finder::findNoNameResult(const Name& name, const RRType& type,
if (hasSubdomains(name.toText())) {
// Does the domain have a subdomain (i.e. it is an empty non-terminal)?
// If so, return NXRRSET instead of NXDOMAIN (as although the name does
- // not exist in the zone, it does exist in the DNS tree).
- // pretend something is here as well.
+ // not exist in the database, it does exist in the DNS tree).
LOG_DEBUG(logger, DBG_TRACE_DETAILED,
DATASRC_DATABASE_FOUND_EMPTY_NONTERMINAL).
arg(accessor_->getDBName()).arg(name);
@@ -693,15 +692,14 @@ DatabaseClient::Finder::findNoNameResult(const Name& name, const RRType& type,
} else if ((options & NO_WILDCARD) == 0) {
// It's not an empty non-terminal and wildcard matching is not
- // disabled, so check for wildcards.
+ // disabled, so check for wildcards. If there is a wildcard match
+ // (i.e. all results except NXDOMAIN) return it; otherwise fall
+ // through to the NXDOMAIN case below.
const ZoneFinder::FindResult wresult =
findWildcardMatch(name, type, options, dresult);
- if (wresult.code == NXDOMAIN && dnssec_data) {
- // No match on a wildcard, so return the covering NSEC if DNSSEC
- // data was requested.
- return (FindResult(NXDOMAIN, findNSECCover(name)));
+ if (wresult.code != NXDOMAIN) {
+ return (FindResult(wresult.code, wresult.rrset));
}
- return (FindResult(wresult.code, wresult.rrset));
}
// All avenues to find a match are now exhausted, return NXDOMAIN (plus
@@ -759,8 +757,8 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
// name/type/class. However, there are special cases:
// - Requested name has a singleton CNAME record associated with it
// - Requested name is a delegation point (NS only but not at the zone
- // apex - DNAME is ignored here).
- // TODO: Why is DNAME ignored?
+ // apex - DNAME is ignored here as it redirects DNS names subordinate to
+ // the owner name - the owner name itself is not redirected.)
const bool is_origin = (name == getOrigin());
WantedTypes final_types(FINAL_TYPES());
final_types.insert(type);
@@ -784,19 +782,10 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
} else if (type != RRType::CNAME() && cni != found.second.end()) {
// We are not searching for a CNAME but nevertheless we have found one
- // at the name we are searching so we return it. (A resolver could have
- // originated the query that caues this result. If so, it will restart
- // the resolution process with the name that is the target of this
- // CNAME.) First though, do a sanity check to ensure that there is
- // only one RR in the CNAME RRset.
- //
- // TODO: Check that throwing an exception here is correct.
- // Unless the exception is caught higher up (probably not, given the
- // general nature of the exception), it is probably better to log
- // and error and terminate the query with SERVFAIL instead of crashing
- // the server. Although the CNAME is a singleton and multiple RRs
- // in the RRset may indicate an error in the data, it does not mean
- // that the entire database is corrupt.
+ // at the name we are searching so we return it. (The caller may
+ // want to continue the lookup by replacing the query name with the
+ // canonical name and the original RR type.) First though, do a sanity
+ // check to ensure that there is only one RR in the CNAME RRset.
if (cni->second->getRdataCount() != 1) {
isc_throw(DataSourceError, "CNAME with " <<
cni->second->getRdataCount() << " rdata at " << name <<
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index 54fe799..f106622 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/src/lib/datasrc/datasrc_messages.mes
@@ -79,11 +79,10 @@ an error, so we set it to the lowest value we found (but we don't modify the
database). The data in database should be checked and fixed.
% DATASRC_DATABASE_FOUND_CNAME search in datasource %1 for %2/%3/%4 found CNAME
-When searching the domain for a name a CNAME was found at that name. Even
-though it was not the RR type being sought, it is returned. If the query
-of the database was issued by a result searching for the name, the return of
-the CNAME record will cause that resolver to issue another query for the
-target of the CNAME.
+When searching the domain for a name a CNAME was found at that name.
+Even though it was not the RR type being sought, it is returned. (The
+caller may want to continue the lookup by replacing the query name with
+the canonical name and restarting the query with the original RR type.)
% DATASRC_DATABASE_FOUND_DELEGATION Found delegation at %2 in %1
When searching for a domain, the program met a delegation to a different zone
@@ -116,7 +115,7 @@ name and class, but not for the given type.
% DATASRC_DATABASE_FOUND_NXRRSET_NSEC search in datasource %1 for %2/%3/%4 resulted in RRset %5
A search in the database for RRs for the specified name, type and class has
located RRs that match the name and class but not the type. DNSSEC information
-has been requested, but there is no NSEC record corresponding to the node.
+has been requested and returned.
% DATASRC_DATABASE_FOUND_RRSET search in datasource %1 resulted in RRset %5
The data returned by the database backend contained data for the given domain
diff --git a/src/lib/datasrc/zone.h b/src/lib/datasrc/zone.h
index f824636..afe06d9 100644
--- a/src/lib/datasrc/zone.h
+++ b/src/lib/datasrc/zone.h
@@ -264,12 +264,15 @@ public:
/// proof of the non existence of any matching wildcard or non existence
/// of an exact match when a wildcard match is found.
///
- /// A derived version of this method may involve internal resource
- /// allocation, especially for constructing the resulting RRset, and may
- /// throw an exception if it fails.
- /// It throws DuplicateRRset exception if there are duplicate rrsets under
- /// the same domain.
- /// It should not throw other types of exceptions.
+ /// \exception std::bad_alloc Memory allocation such as for constructing
+ /// the resulting RRset fails
+ /// \exception DataSourceError Derived class specific exception, e.g.
+ /// when encountering a bad zone configuration or database connection
+ /// failure. Although these are considered rare, exceptional events,
+ /// it can happen under relatively usual conditions (unlike memory
+ /// allocation failure). So, in general, the application is expected
+ /// to catch this exception, either specifically or as a result of
+ /// catching a base exception class, and handle it gracefully.
///
/// \param name The domain name to be searched for.
/// \param type The RR type to be searched for.
diff --git a/tools/reorder_message_file.py b/tools/reorder_message_file.py
index 3f95544..31f4941 100644
--- a/tools/reorder_message_file.py
+++ b/tools/reorder_message_file.py
@@ -26,7 +26,7 @@
import sys
-def removeEmptyLeadingTrailing(lines):
+def remove_empty_leading_trailing(lines):
"""
Removes leading and trailing empty lines.
@@ -124,7 +124,7 @@ def make_dict(lines):
while index < len(lines):
if lines[index].startswith("%"):
# Start of new message
- dictionary[message_key] = removeEmptyLeadingTrailing(message_lines)
+ dictionary[message_key] = remove_empty_leading_trailing(message_lines)
message_key = canonicalise_message_line(lines[index])
message_lines = [message_key]
else:
@@ -132,7 +132,7 @@ def make_dict(lines):
index = index + 1
- dictionary[message_key] = removeEmptyLeadingTrailing(message_lines)
+ dictionary[message_key] = remove_empty_leading_trailing(message_lines)
return dictionary
@@ -157,7 +157,7 @@ def print_dict(dictionary):
print(l.strip())
-def processFile(filename):
+def process_file(filename):
"""
Processes a file by reading it and searching for the first line starting
with the '%' sign. Everything before that line is treated as the file
@@ -193,4 +193,4 @@ if __name__ == "__main__":
if len(sys.argv) != 2:
print "Usage: python reorder.py message_file"
else:
- processFile(sys.argv[1])
+ process_file(sys.argv[1])
More information about the bind10-changes
mailing list