BIND 10 trac1177, updated. 7a061c2e82d62e2b275cb5a8d7460dce7d36f050 [1177] More docs for DatabaseAccessor::findPreviousName
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Sep 21 11:50:00 UTC 2011
The branch, trac1177 has been updated
via 7a061c2e82d62e2b275cb5a8d7460dce7d36f050 (commit)
via a6cbb14cc9c986d109983087313225829f1c91fe (commit)
via 7cc32b7915532354ed7e2fd15f7ca5a9b9b64610 (commit)
via dd340b32df88083fdc17f682094b451f7dcdf6d6 (commit)
via 30c277567f64d09c11cadcb173eef066efdaea07 (commit)
via ec2793914d1090db8c8d94a2f9b92ed97b1a6cba (commit)
via a59c7f28a458842b4edce2d6639639b17a85eb9f (commit)
from 2a5c5383e3df0e625367bf85b740f62bf777b211 (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 7a061c2e82d62e2b275cb5a8d7460dce7d36f050
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 21 13:27:40 2011 +0200
[1177] More docs for DatabaseAccessor::findPreviousName
commit a6cbb14cc9c986d109983087313225829f1c91fe
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 21 13:21:09 2011 +0200
[1177] More tests for SQLite3Accessor NSEC
commit 7cc32b7915532354ed7e2fd15f7ca5a9b9b64610
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 21 13:09:12 2011 +0200
[1177] SQLite3Accessor NSEC cleanups
commit dd340b32df88083fdc17f682094b451f7dcdf6d6
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 21 12:55:25 2011 +0200
[1177] Empty nonterminal asterisk
commit 30c277567f64d09c11cadcb173eef066efdaea07
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 21 11:07:42 2011 +0200
[1177] Remove FIXME comments
As the concerns there seem to be false ones.
commit ec2793914d1090db8c8d94a2f9b92ed97b1a6cba
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 21 11:07:26 2011 +0200
[1177] Log when DNSSEC not supported
commit a59c7f28a458842b4edce2d6639639b17a85eb9f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Wed Sep 21 10:53:09 2011 +0200
[1177] Don't propagate name exceptions from findPreviousName
They should be turned into DataSourceError instead, as they mean bad
data in the DB.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/database.cc | 41 +++++++++++++-
src/lib/datasrc/database.h | 5 ++-
src/lib/datasrc/datasrc_messages.mes | 5 ++
src/lib/datasrc/sqlite3_accessor.cc | 58 ++++++++++----------
src/lib/datasrc/tests/database_unittest.cc | 34 ++++++++++--
src/lib/datasrc/tests/sqlite3_accessor_unittest.cc | 16 +++++-
6 files changed, 120 insertions(+), 39 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index bb5be93..3088c82 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -536,11 +536,30 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
} else if (hasSubdomains(wildcard)) {
// Empty non-terminal asterisk
records_found = true;
- get_cover = dnssec_data;
LOG_DEBUG(logger, DBG_TRACE_DETAILED,
DATASRC_DATABASE_WILDCARD_EMPTY).
arg(accessor_->getDBName()).arg(wildcard).
arg(name);
+ if (dnssec_data) {
+ // Which one should contain the NSEC record?
+ const Name
+ coverName(findPreviousName(Name(wildcard)));
+ // Get the record and copy it out
+ found = getRRsets(coverName.toText(), nsec_types,
+ true);
+ const FoundIterator
+ nci(found.second.find(RRType::NSEC()));
+ if (nci != found.second.end()) {
+ result_status = WILDCARD_NXRRSET;
+ result_rrset = nci->second;
+ } else {
+ // The previous doesn't contain NSEC, bug?
+ isc_throw(DataSourceError, "No NSEC in " +
+ coverName.toText() + ", but it was "
+ "returned as previous - "
+ "accessor error?");
+ }
+ }
break;
}
}
@@ -587,6 +606,8 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
catch (const isc::NotImplemented&) {
// Well, they want DNSSEC, but there is no available.
// So we don't provide anything.
+ LOG_INFO(logger, DATASRC_DATABASE_COVER_NSEC_UNSUPPORTED).
+ arg(accessor_->getDBName()).arg(name);
}
}
// Something is not here and we didn't decide yet what
@@ -614,8 +635,22 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
Name
DatabaseClient::Finder::findPreviousName(const Name& name) const {
- return (Name(accessor_->findPreviousName(zone_id_,
- name.reverse().toText())));
+ const string str(accessor_->findPreviousName(zone_id_,
+ name.reverse().toText()));
+ try {
+ return (Name(str));
+ }
+ /*
+ * To avoid having the same code many times, we just catch all the
+ * exceptions and handle them in a common code below
+ */
+ catch (const isc::dns::EmptyLabel&) {}
+ catch (const isc::dns::TooLongLabel&) {}
+ catch (const isc::dns::BadLabelType&) {}
+ catch (const isc::dns::BadEscape&) {}
+ catch (const isc::dns::TooLongName&) {}
+ catch (const isc::dns::IncompleteName&) {}
+ isc_throw(DataSourceError, "Bad name " + str + " from findPreviousName");
}
Name
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index c269e87..709ae77 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -487,7 +487,10 @@ public:
* and the DNSSEC order correspond (eg. org.example.a is followed
* by org.example.a.b which is followed by org.example.b, etc).
* \param zone_id The zone to look through.
- * \return The previous name.
+ * \return The previous name, or the last name in the zone, if there's
+ * no previous one (including out-of-zone cases).
+ * \note This function must return previous/last name even in case
+ * the queried rname does not exist in the zone.
*
* \throw DataSourceError if there's a problem with the database.
* \throw NotImplemented if this database doesn't support DNSSEC.
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index efb88fd..04ad610 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/src/lib/datasrc/datasrc_messages.mes
@@ -63,6 +63,11 @@ The maximum allowed number of items of the hotspot cache is set to the given
number. If there are too many, some of them will be dropped. The size of 0
means no limit.
+% DATASRC_DATABASE_COVER_NSEC_UNSUPPORTED %1 doesn't support DNSSEC when asked for NSEC data covering %2
+The datasource tried to provide an NSEC proof that the named domain does not
+exist, but the database backend doesn't support DNSSEC. No proof is included
+in the answer as a result.
+
% DATASRC_DATABASE_FIND_RECORDS looking in datasource %1 for record %2/%3
Debug information. The database data source is looking up records with the given
name and type in the database.
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index 2093dcb..ed2f84e 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -71,10 +71,16 @@ const char* const text_statements[NUM_STATEMENTS] = {
"AND rdtype=?3 AND rdata=?4",
"SELECT rdtype, ttl, sigtype, rdata, name FROM records " // ITERATE
"WHERE zone_id = ?1 ORDER BY name, rdtype",
+ /*
+ * The ones for finding previous name. The first of them takes
+ * biggest smaller than something (therefore previous to the something),
+ * the second takes biggest (used in case when there's no previous,
+ * to "wrap around").
+ */
"SELECT name FROM records " // FIND_PREVIOUS
"WHERE zone_id=?1 AND rdtype = 'NSEC' AND "
- "rname < $2 ORDER BY rname DESC LIMIT 1", // FIND_PREVIOUS_WRAP
- "SELECT name FROM records "
+ "rname < $2 ORDER BY rname DESC LIMIT 1",
+ "SELECT name FROM records " // FIND_PREVIOUS_WRAP
"WHERE zone_id = ?1 AND rdtype = 'NSEC' "
"ORDER BY rname DESC LIMIT 1"
};
@@ -403,7 +409,7 @@ namespace {
// Conversion to plain char
const char*
-convertToPlainCharInternal(const unsigned char* ucp, sqlite3 *db) {
+convertToPlainChar(const unsigned char* ucp, sqlite3 *db) {
if (ucp == NULL) {
// The field can really be NULL, in which case we return an
// empty string, or sqlite may have run out of memory, in
@@ -498,7 +504,8 @@ private:
void copyColumn(std::string (&data)[COLUMN_COUNT], int column) {
data[column] = convertToPlainChar(sqlite3_column_text(statement_,
- column));
+ column),
+ accessor_->dbparameters_->db_);
}
void bindZoneId(const int zone_id) {
@@ -525,16 +532,6 @@ private:
statement_ = NULL;
}
- // This helper method converts from the unsigned char* type (used by
- // sqlite3) to char* (wanted by std::string). Technically these types
- // might not be directly convertable
- // In case sqlite3_column_text() returns NULL, we just make it an
- // empty string, unless it was caused by a memory error
- const char* convertToPlainChar(const unsigned char* ucp) {
- return (convertToPlainCharInternal(ucp,
- accessor_->dbparameters_->db_));
- }
-
const IteratorType iterator_type_;
boost::shared_ptr<const SQLite3Accessor> accessor_;
sqlite3_stmt *statement_;
@@ -682,24 +679,24 @@ SQLite3Accessor::findPreviousName(int zone_id, const std::string& rname)
sqlite3_reset(dbparameters_->statements_[FIND_PREVIOUS]);
sqlite3_clear_bindings(dbparameters_->statements_[FIND_PREVIOUS]);
- int rc = sqlite3_bind_int(dbparameters_->statements_[FIND_PREVIOUS], 1,
- zone_id);
- if (rc != SQLITE_OK) {
+ if (sqlite3_bind_int(dbparameters_->statements_[FIND_PREVIOUS], 1,
+ zone_id) != SQLITE_OK) {
isc_throw(SQLite3Error, "Could not bind zone ID " << zone_id <<
- " to SQL statement (find previous)");
+ " to SQL statement (find previous): " <<
+ sqlite3_errmsg(dbparameters_->db_));
}
- rc = sqlite3_bind_text(dbparameters_->statements_[FIND_PREVIOUS], 2,
- rname.c_str(), -1, SQLITE_STATIC);
- if (rc != SQLITE_OK) {
+ if (sqlite3_bind_text(dbparameters_->statements_[FIND_PREVIOUS], 2,
+ rname.c_str(), -1, SQLITE_STATIC) != SQLITE_OK) {
isc_throw(SQLite3Error, "Could not bind name " << rname <<
- " to SQL statement (find previous)");
+ " to SQL statement (find previous): " <<
+ sqlite3_errmsg(dbparameters_->db_));
}
std::string result;
- rc = sqlite3_step(dbparameters_->statements_[FIND_PREVIOUS]);
+ int rc = sqlite3_step(dbparameters_->statements_[FIND_PREVIOUS]);
if (rc == SQLITE_ROW) {
// We found it
- result = convertToPlainCharInternal(sqlite3_column_text(dbparameters_->
+ result = convertToPlainChar(sqlite3_column_text(dbparameters_->
statements_[FIND_PREVIOUS], 0), dbparameters_->db_);
}
sqlite3_reset(dbparameters_->statements_[FIND_PREVIOUS]);
@@ -710,18 +707,19 @@ SQLite3Accessor::findPreviousName(int zone_id, const std::string& rname)
sqlite3_reset(dbparameters_->statements_[FIND_PREVIOUS_WRAP]);
sqlite3_clear_bindings(dbparameters_->statements_[FIND_PREVIOUS_WRAP]);
- int rc = sqlite3_bind_int(
- dbparameters_->statements_[FIND_PREVIOUS_WRAP], 1, zone_id);
- if (rc != SQLITE_OK) {
+ if (sqlite3_bind_int(
+ dbparameters_->statements_[FIND_PREVIOUS_WRAP], 1, zone_id) !=
+ SQLITE_OK) {
isc_throw(SQLite3Error, "Could not bind zone ID " << zone_id <<
- " to SQL statement (find previous wrap)");
+ " to SQL statement (find previous wrap): " <<
+ sqlite3_errmsg(dbparameters_->db_));
}
rc = sqlite3_step(dbparameters_->statements_[FIND_PREVIOUS_WRAP]);
if (rc == SQLITE_ROW) {
// We found it
result =
- convertToPlainCharInternal(sqlite3_column_text(dbparameters_->
+ convertToPlainChar(sqlite3_column_text(dbparameters_->
statements_[FIND_PREVIOUS_WRAP], 0), dbparameters_->db_);
}
sqlite3_reset(dbparameters_->statements_[FIND_PREVIOUS_WRAP]);
@@ -733,7 +731,7 @@ SQLite3Accessor::findPreviousName(int zone_id, const std::string& rname)
if (rc != SQLITE_ROW && rc != SQLITE_DONE) {
// Some kind of error
- isc_throw(SQLite3Error, "Could get data for previous name");
+ isc_throw(SQLite3Error, "Could not get data for previous name");
}
return (result);
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 36099ec..f9c81e7 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -172,6 +172,7 @@ const char* const TEST_RECORDS[][5] = {
{"*.delegatedwild.example.org.", "A", "3600", "", "192.0.2.5"},
{"wild.*.foo.example.org.", "A", "3600", "", "192.0.2.5"},
{"wild.*.foo.*.bar.example.org.", "A", "3600", "", "192.0.2.5"},
+ {"bao.example.org.", "NSEC", "3600", "", "wild.*.foo.*.bar.example.org. NSEC"},
{"*.cnamewild.example.org.", "CNAME", "3600", "", "www.example.org."},
{"*.nswild.example.org.", "NS", "3600", "", "ns.example.com."},
// For finding previous, this one is the last one in the zone
@@ -566,6 +567,10 @@ public:
return ("www.example.org.");
} else if (rname == "org.example.badnsec2.") {
return ("badnsec1.example.org.");
+ } else if (rname == "org.example.brokenname.") {
+ return ("brokenname...example.org.");
+ } else if (rname == "org.example.bar.*.") {
+ return ("bao.example.org.");
} else if (rname == "org.example.notimplnsec." ||
rname == "org.example.wild.here.") {
isc_throw(isc::NotImplemented, "Not implemented in this test");
@@ -1588,6 +1593,23 @@ TYPED_TEST(DatabaseClientTest, wildcard) {
// DNSSEC logic handle it?
}
+ const char* negative_dnssec_names[] = {
+ "a.bar.example.org.",
+ "foo.baz.bar.example.org.",
+ "a.foo.bar.example.org.",
+ NULL
+ };
+
+ this->expected_rdatas_.clear();
+ this->expected_rdatas_.push_back("wild.*.foo.*.bar.example.org. NSEC");
+ this->expected_sig_rdatas_.clear();
+ for (const char** name(negative_dnssec_names); *name != NULL; ++ name) {
+ doFindTest(*finder, isc::dns::Name(*name), this->qtype_,
+ RRType::NSEC(), this->rrttl_, ZoneFinder::WILDCARD_NXRRSET,
+ this->expected_rdatas_, this->expected_sig_rdatas_,
+ Name("bao.example.org."), ZoneFinder::FIND_DNSSEC);
+ }
+
// Some strange things in the wild node
this->expected_rdatas_.clear();
this->expected_rdatas_.push_back("www.example.org.");
@@ -1672,15 +1694,12 @@ TYPED_TEST(DatabaseClientTest, NXDOMAIN_NSEC) {
TYPED_TEST(DatabaseClientTest, emptyNonterminalNSEC) {
// Same as NXDOMAIN_NSEC, but with empty non-terminal
- //
- // FIXME: Is the nonexistence of this node really the correct proof
- // we need?
shared_ptr<DatabaseClient::Finder> finder(this->getFinder());
this->expected_rdatas_.push_back("empty.nonterminal.example.org. NSEC");
doFindTest(*finder, isc::dns::Name("nonterminal.example.org."),
isc::dns::RRType::TXT(), isc::dns::RRType::NSEC(), this->rrttl_,
- ZoneFinder::NXRRSET, // FIXME: Do we want to have specific code?
+ ZoneFinder::NXRRSET,
this->expected_rdatas_, this->expected_sig_rdatas_,
Name("l.example.org."), ZoneFinder::FIND_DNSSEC);
@@ -2347,4 +2366,11 @@ TEST_F(MockDatabaseClientTest, missingNSEC) {
DataSourceError);
}
+TEST_F(MockDatabaseClientTest, badName) {
+ shared_ptr<DatabaseClient::Finder> finder(this->getFinder());
+
+ EXPECT_THROW(finder->findPreviousName(Name("brokenname.example.org.")),
+ DataSourceError);
+}
+
}
diff --git a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
index afc1638..4bcf252 100644
--- a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
+++ b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
@@ -357,16 +357,30 @@ TEST_F(SQLite3AccessorTest, findPrevious) {
// A name that doesn't exist
EXPECT_EQ("dns01.example.com.",
accessor->findPreviousName(1, "com.example.dns01x."));
+ // Largest name
+ EXPECT_EQ("www.example.com.",
+ accessor->findPreviousName(1, "com.example.wwww"));
// Wrap around
EXPECT_EQ("www.example.com.",
accessor->findPreviousName(1, "com.example."));
+ // Out of zone before and after
+ EXPECT_EQ("www.example.com.",
+ accessor->findPreviousName(1, "bb.example."));
+ EXPECT_EQ("www.example.com.",
+ accessor->findPreviousName(1, "org.example."));
+ // Case insensitive?
+ EXPECT_EQ("dns01.example.com.",
+ accessor->findPreviousName(1, "com.exaMple.DNS02."));
+ // A name that doesn't exist
+ EXPECT_EQ("dns01.example.com.",
+ accessor->findPreviousName(1, "com.exaMple.DNS01X."));
}
TEST_F(SQLite3AccessorTest, findPreviousNoData) {
// This one doesn't hold any NSEC records, so it shouldn't work
// The underlying DB/data don't support DNSSEC, so it's not implemented
// (does it make sense? Or different exception here?)
- EXPECT_THROW(accessor->findPreviousName(3, "com.example."),
+ EXPECT_THROW(accessor->findPreviousName(3, "com.example.sql2.www."),
isc::NotImplemented);
}
More information about the bind10-changes
mailing list