BIND 10 trac1062, updated. 69642fb8f55cb4741f977d3fbaacd5d12d742625 [trac1062] some comment updates
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Aug 5 17:09:51 UTC 2011
The branch, trac1062 has been updated
via 69642fb8f55cb4741f977d3fbaacd5d12d742625 (commit)
via 86257c05755c8adbb19ce684546b718dd48a5ef8 (commit)
via 5f13949918d125f851bd2ba8ab092c301835d3ac (commit)
via 7e1e150e056d0dcf5a58b2a8036f47c2e5dac820 (commit)
from 15428e5a9c1bb01f5e7a04979c17ec5f1de9d1db (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 69642fb8f55cb4741f977d3fbaacd5d12d742625
Author: Jelte Jansen <jelte at isc.org>
Date: Fri Aug 5 19:09:36 2011 +0200
[trac1062] some comment updates
commit 86257c05755c8adbb19ce684546b718dd48a5ef8
Author: Jelte Jansen <jelte at isc.org>
Date: Fri Aug 5 18:24:20 2011 +0200
[trac1062] minor style cleanup
commit 5f13949918d125f851bd2ba8ab092c301835d3ac
Author: Jelte Jansen <jelte at isc.org>
Date: Fri Aug 5 18:02:33 2011 +0200
[trac1062] refactor and fixes
- check for bad data in the db
- work with data in 'bad' order (sigs before data for example)
commit 7e1e150e056d0dcf5a58b2a8036f47c2e5dac820
Author: Jelte Jansen <jelte at isc.org>
Date: Fri Aug 5 15:17:27 2011 +0200
[trac1062] use shared_ptr instead of auto_ptr
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/database.cc | 194 +++++++++++++-------
src/lib/datasrc/database.h | 24 ++-
src/lib/datasrc/tests/database_unittest.cc | 50 +++++-
.../datasrc/tests/sqlite3_connection_unittest.cc | 6 +-
4 files changed, 205 insertions(+), 69 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index da823a2..3002056 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -22,6 +22,8 @@
#include <datasrc/data_source.h>
+#include <boost/foreach.hpp>
+
using isc::dns::Name;
namespace isc {
@@ -66,6 +68,96 @@ DatabaseClient::Finder::Finder(boost::shared_ptr<DatabaseConnection>
zone_id_(zone_id)
{ }
+namespace {
+ // Adds the given Rdata to the given RRset
+ // If the rrset is an empty pointer, a new one is
+ // created with the given name, class, type and ttl
+ // The type is checked if the rrset exists, but the
+ // name is not.
+ //
+ // Then adds the given rdata to the set
+ //
+ // Raises a DataSourceError if the type does not
+ // match, or if the given rdata string does not
+ // parse correctly for the given type and class
+ void addOrCreate(isc::dns::RRsetPtr& rrset,
+ const isc::dns::Name& name,
+ const isc::dns::RRClass& cls,
+ const isc::dns::RRType& type,
+ const isc::dns::RRTTL& ttl,
+ const std::string& rdata_str)
+ {
+ if (!rrset) {
+ rrset.reset(new isc::dns::RRset(name, cls, type, ttl));
+ } else {
+ if (ttl < rrset->getTTL()) {
+ rrset->setTTL(ttl);
+ }
+ // make sure the type is correct
+ if (type != rrset->getType()) {
+ isc_throw(DataSourceError,
+ "attempt to add multiple types to RRset in find()");
+ }
+ }
+ if (rdata_str != "") {
+ try {
+ rrset->addRdata(isc::dns::rdata::createRdata(type, cls, rdata_str));
+ } catch (const isc::dns::rdata::InvalidRdataText& ivrt) {
+ // at this point, rrset may have been initialised for no reason,
+ // and won't be used. But the caller would drop the shared_ptr
+ // on such an error anyway, so we don't care.
+ isc_throw(DataSourceError,
+ "bad rdata in database for " << name.toText() << " "
+ << type.toText() << " " << ivrt.what());
+ }
+ }
+ }
+
+ // This class keeps a short-lived store of RRSIG records encountered
+ // during a call to find(). If the backend happens to return signatures
+ // before the actual data, we might not know which signatures we will need
+ // So if they may be relevant, we store the in this class.
+ //
+ // (If this class seems useful in other places, we might want to move
+ // it to util. That would also provide an opportunity to add unit tests)
+ class RRsigStore {
+ public:
+ // Adds the given signature Rdata to the store
+ // The signature rdata MUST be of the RRSIG rdata type
+ // (the caller must make sure of this)
+ void addSig(isc::dns::rdata::RdataPtr sig_rdata) {
+ const isc::dns::RRType& type_covered =
+ static_cast<isc::dns::rdata::generic::RRSIG*>(
+ sig_rdata.get())->typeCovered();
+ if (!haveSigsFor(type_covered)) {
+ sigs[type_covered] = std::vector<isc::dns::rdata::RdataPtr>();
+ }
+ sigs.find(type_covered)->second.push_back(sig_rdata);
+ }
+
+ // Returns true if this store contains signatures covering the
+ // given type
+ bool haveSigsFor(isc::dns::RRType type) {
+ return (sigs.count(type) > 0);
+ }
+
+ // If the store contains signatures for the type of the given
+ // rrset, they are appended to it.
+ void appendSignatures(isc::dns::RRsetPtr& rrset) {
+ if (haveSigsFor(rrset->getType())) {
+ BOOST_FOREACH(isc::dns::rdata::RdataPtr sig,
+ sigs.find(rrset->getType())->second) {
+ rrset->addRRsig(sig);
+ }
+ }
+ }
+
+ private:
+ std::map<isc::dns::RRType, std::vector<isc::dns::rdata::RdataPtr> > sigs;
+ };
+}
+
+
ZoneFinder::FindResult
DatabaseClient::Finder::find(const isc::dns::Name& name,
const isc::dns::RRType& type,
@@ -77,6 +169,7 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
bool records_found = false;
isc::dns::RRsetPtr result_rrset;
ZoneFinder::Result result_status = SUCCESS;
+ RRsigStore sig_store;
connection_->searchForRecords(zone_id_, name.toText());
@@ -91,75 +184,52 @@ DatabaseClient::Finder::find(const isc::dns::Name& name,
"Datasource backend did not return 4 columns in getNextRecord()");
}
- const isc::dns::RRType cur_type(columns[0]);
- const isc::dns::RRTTL cur_ttl(columns[1]);
- //cur_sigtype(columns[2]);
-
- if (cur_type == type) {
- if (!result_rrset) {
- result_rrset = isc::dns::RRsetPtr(new isc::dns::RRset(name,
- getClass(),
- cur_type,
- cur_ttl));
- result_status = SUCCESS;
- } else {
- // We have existing data from earlier calls, do some checks
- // and updates if necessary
- if (cur_ttl < result_rrset->getTTL()) {
- result_rrset->setTTL(cur_ttl);
- }
- }
-
- result_rrset->addRdata(isc::dns::rdata::createRdata(cur_type,
- getClass(),
- columns[3]));
- } else if (cur_type == isc::dns::RRType::CNAME()) {
- // There should be no other data, so cur_rrset should be empty,
- // except for signatures
- if (result_rrset) {
- if (result_rrset->getRdataCount() > 0) {
- isc_throw(DataSourceError, "CNAME found but it is not the only record for " + name.toText());
- }
- } else {
- result_rrset = isc::dns::RRsetPtr(new isc::dns::RRset(name,
- getClass(),
- cur_type,
- cur_ttl));
- }
- result_rrset->addRdata(isc::dns::rdata::createRdata(cur_type,
- getClass(),
- columns[3]));
- result_status = CNAME;
- } else if (cur_type == isc::dns::RRType::RRSIG()) {
- isc::dns::rdata::RdataPtr cur_rrsig(
- isc::dns::rdata::createRdata(cur_type, getClass(), columns[3]));
- const isc::dns::RRType& type_covered =
- static_cast<isc::dns::rdata::generic::RRSIG*>(
- cur_rrsig.get())->typeCovered();
- // Ignore the RRSIG data we got if it does not cover the type
- // that was requested or CNAME
- // see if we have RRset data yet, and whether it has an RRsig yet
- if (type_covered == type || type_covered == isc::dns::RRType::CNAME()) {
- if (!result_rrset) {
- // no data at all yet, assume the RRset data is coming, and
- // that the type covered will match
- result_rrset = isc::dns::RRsetPtr(new isc::dns::RRset(name,
- getClass(),
- type_covered,
- cur_ttl));
+ try {
+ const isc::dns::RRType cur_type(columns[0]);
+ const isc::dns::RRTTL cur_ttl(columns[1]);
+ //cur_sigtype(columns[2]);
+
+ if (cur_type == type) {
+ addOrCreate(result_rrset, name, getClass(), cur_type, cur_ttl, columns[3]);
+ } else if (cur_type == isc::dns::RRType::CNAME()) {
+ // There should be no other data, so cur_rrset should be empty,
+ if (result_rrset) {
+ isc_throw(DataSourceError,
+ "CNAME found but it is not the only record for " +
+ name.toText());
}
- result_rrset->addRRsig(cur_rrsig);
+ addOrCreate(result_rrset, name, getClass(), cur_type, cur_ttl, columns[3]);
+ result_status = CNAME;
+ } 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 relevant
+ // and add them to the final RRset when we are done.
+ // A possible optimization here is to not store them for types we
+ // are certain we don't need
+ isc::dns::rdata::RdataPtr cur_rrsig(
+ isc::dns::rdata::createRdata(cur_type, getClass(), columns[3]));
+ sig_store.addSig(cur_rrsig);
}
+ } catch (const isc::dns::InvalidRRType& irt) {
+ isc_throw(DataSourceError,
+ "Invalid RRType in database for " << name << ": " << columns[0]);
+ } catch (const isc::dns::InvalidRRTTL& irttl) {
+ isc_throw(DataSourceError,
+ "Invalid TTL in database for " << name << ": " << columns[1]);
}
}
- if (result_rrset) {
- return (FindResult(result_status, result_rrset));
- } else if (records_found) {
- return (FindResult(NXRRSET, isc::dns::ConstRRsetPtr()));
+ if (!result_rrset) {
+ if (records_found) {
+ result_status = NXRRSET;
+ } else {
+ result_status = NXDOMAIN;
+ }
} else {
- return (FindResult(NXDOMAIN, isc::dns::ConstRRsetPtr()));
+ sig_store.appendSignatures(result_rrset);
}
+ return (FindResult(result_status, result_rrset));
}
Name
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index 047db3d..5498694 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -158,16 +158,34 @@ public:
/**
* \brief Find an RRset in the datasource
*
- * target is unused at this point, it was used in the original
- * API to store the results for ANY queries, and we may reuse it
- * for that, but we might choose a different approach.
+ * Searches the datasource for an RRset of the given name and
+ * type. If there is a CNAME at the given name, the CNAME rrset
+ * is returned.
+ * (this implementation is not complete, and currently only
+ * does full matches, CNAMES, and the signatures for matches and
+ * CNAMEs)
+ * \note target was used in the original design to handle ANY
+ * queries. This is not implemented yet, and may use
+ * target again for that, but it might also use something
+ * different. It is left in for compatibility at the moment.
+ * \note options are ignored at this moment
*
+ * \exception DataSourceError when there is a problem reading
+ * the data from the dabase backend.
+ * This can be a connection, code, or
+ * data (parse) error.
+ *
+ * \param name The name to find
+ * \param type The RRType to find
+ * \param target Unused at this moment
+ * \param options Unused at this moment
*/
virtual FindResult find(const isc::dns::Name& name,
const isc::dns::RRType& type,
isc::dns::RRsetList* target = NULL,
const FindOptions options = FIND_DEFAULT)
const;
+
/**
* \brief The zone ID
*
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 7e80b43..76c801c 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -127,6 +127,7 @@ private:
// some DNSSEC-'signed' data
addRecord("A", "3600", "", "192.0.2.1");
addRecord("RRSIG", "3600", "", "A 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
+ addRecord("RRSIG", "3600", "", "A 5 3 3600 20000101000000 20000201000000 12346 example.org. FAKEFAKEFAKE");
addRecord("AAAA", "3600", "", "2001:db8::1");
addRecord("AAAA", "3600", "", "2001:db8::2");
addRecord("RRSIG", "3600", "", "AAAA 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
@@ -134,11 +135,18 @@ private:
addRecord("CNAME", "3600", "", "www.example.org.");
addRecord("RRSIG", "3600", "", "CNAME 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
addCurName("signedcname1.example.org.");
+ // special case might fail; sig is for cname, which isn't there (should be ignored)
+ // (ignoring of 'normal' other type is done above by www.)
+ addRecord("A", "3600", "", "192.0.2.1");
+ addRecord("RRSIG", "3600", "", "A 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
+ addRecord("RRSIG", "3600", "", "CNAME 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
+ addCurName("acnamesig1.example.org.");
// let's pretend we have a database that is not careful
// about the order in which it returns data
addRecord("RRSIG", "3600", "", "A 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
addRecord("AAAA", "3600", "", "2001:db8::2");
+ addRecord("RRSIG", "3600", "", "A 5 3 3600 20000101000000 20000201000000 12346 example.org. FAKEFAKEFAKE");
addRecord("A", "3600", "", "192.0.2.1");
addRecord("RRSIG", "3600", "", "AAAA 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
addRecord("AAAA", "3600", "", "2001:db8::1");
@@ -147,12 +155,28 @@ private:
addRecord("CNAME", "3600", "", "www.example.org.");
addCurName("signedcname2.example.org.");
+ addRecord("RRSIG", "3600", "", "CNAME 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
+ addRecord("A", "3600", "", "192.0.2.1");
+ addRecord("RRSIG", "3600", "", "A 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
+ addCurName("acnamesig2.example.org.");
+
+ addRecord("RRSIG", "3600", "", "CNAME 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
+ addRecord("RRSIG", "3600", "", "A 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
+ addRecord("A", "3600", "", "192.0.2.1");
+ addCurName("acnamesig3.example.org.");
+
// also add some intentionally bad data
cur_name.push_back(std::vector<std::string>());
addCurName("emptyvector.example.org.");
addRecord("A", "3600", "", "192.0.2.1");
addRecord("CNAME", "3600", "", "www.example.org.");
addCurName("badcname.example.org.");
+ addRecord("A", "3600", "", "bad");
+ addCurName("badrdata.example.org.");
+ addRecord("BAD_TYPE", "3600", "", "192.0.2.1");
+ addCurName("badtype.example.org.");
+ addRecord("A", "badttl", "", "192.0.2.1");
+ addCurName("badttl.example.org.");
}
};
@@ -263,7 +287,7 @@ TEST_F(DatabaseClientTest, find) {
ZoneFinder::NXDOMAIN, 0, 0);
doFindTest(finder, isc::dns::Name("signed1.example.org."),
isc::dns::RRType::A(), isc::dns::RRType::A(),
- ZoneFinder::SUCCESS, 1, 1);
+ ZoneFinder::SUCCESS, 1, 2);
doFindTest(finder, isc::dns::Name("signed1.example.org."),
isc::dns::RRType::AAAA(), isc::dns::RRType::AAAA(),
ZoneFinder::SUCCESS, 2, 1);
@@ -276,7 +300,7 @@ TEST_F(DatabaseClientTest, find) {
doFindTest(finder, isc::dns::Name("signed2.example.org."),
isc::dns::RRType::A(), isc::dns::RRType::A(),
- ZoneFinder::SUCCESS, 1, 1);
+ ZoneFinder::SUCCESS, 1, 2);
doFindTest(finder, isc::dns::Name("signed2.example.org."),
isc::dns::RRType::AAAA(), isc::dns::RRType::AAAA(),
ZoneFinder::SUCCESS, 2, 1);
@@ -287,6 +311,16 @@ TEST_F(DatabaseClientTest, find) {
isc::dns::RRType::A(), isc::dns::RRType::CNAME(),
ZoneFinder::CNAME, 1, 1);
+ doFindTest(finder, isc::dns::Name("acnamesig1.example.org."),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ ZoneFinder::SUCCESS, 1, 1);
+ doFindTest(finder, isc::dns::Name("acnamesig2.example.org."),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ ZoneFinder::SUCCESS, 1, 1);
+ doFindTest(finder, isc::dns::Name("acnamesig3.example.org."),
+ isc::dns::RRType::A(), isc::dns::RRType::A(),
+ ZoneFinder::SUCCESS, 1, 1);
+
EXPECT_THROW(finder->find(isc::dns::Name("emptyvector.example.org."),
isc::dns::RRType::A(),
NULL, ZoneFinder::FIND_DEFAULT),
@@ -295,6 +329,18 @@ TEST_F(DatabaseClientTest, find) {
isc::dns::RRType::A(),
NULL, ZoneFinder::FIND_DEFAULT),
DataSourceError);
+ EXPECT_THROW(finder->find(isc::dns::Name("badrdata.example.org."),
+ isc::dns::RRType::A(),
+ NULL, ZoneFinder::FIND_DEFAULT),
+ DataSourceError);
+ EXPECT_THROW(finder->find(isc::dns::Name("badtype.example.org."),
+ isc::dns::RRType::A(),
+ NULL, ZoneFinder::FIND_DEFAULT),
+ DataSourceError);
+ EXPECT_THROW(finder->find(isc::dns::Name("badttl.example.org."),
+ isc::dns::RRType::A(),
+ NULL, ZoneFinder::FIND_DEFAULT),
+ DataSourceError);
}
diff --git a/src/lib/datasrc/tests/sqlite3_connection_unittest.cc b/src/lib/datasrc/tests/sqlite3_connection_unittest.cc
index 1c80eb5..1279910 100644
--- a/src/lib/datasrc/tests/sqlite3_connection_unittest.cc
+++ b/src/lib/datasrc/tests/sqlite3_connection_unittest.cc
@@ -11,6 +11,7 @@
// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
+#include <memory>
#include <datasrc/sqlite3_connection.h>
#include <datasrc/data_source.h>
@@ -18,6 +19,7 @@
#include <dns/rrclass.h>
#include <gtest/gtest.h>
+#include <boost/shared_ptr.hpp>
using namespace isc::datasrc;
using isc::data::ConstElementPtr;
@@ -74,7 +76,7 @@ public:
conn.reset(new SQLite3Connection(filename, rrclass));
}
// The tested connection
- std::auto_ptr<SQLite3Connection> conn;
+ boost::shared_ptr<SQLite3Connection> conn;
};
// This zone exists in the data, so it should be found
@@ -103,7 +105,7 @@ TEST_F(SQLite3Conn, noClass) {
namespace {
// Simple function to cound the number of records for
// any name
- size_t countRecords(std::auto_ptr<SQLite3Connection>& conn,
+ size_t countRecords(boost::shared_ptr<SQLite3Connection>& conn,
int zone_id, const std::string& name) {
conn->searchForRecords(zone_id, name);
size_t count = 0;
More information about the bind10-changes
mailing list