[svn] commit: r1228 - in /trunk/src/lib/auth: data_source_sqlite3.cc data_source_sqlite3.h data_source_sqlite3_unittest.cc testdata/root.zone testdata/test-root.sqlite3
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Mar 9 08:59:49 UTC 2010
Author: jinmei
Date: Tue Mar 9 08:59:49 2010
New Revision: 1228
Log:
fixed infinite loop bug when Sqlite3DataSrc::findClosest() failed
(a regression of rev1222).
refactored findClosest() to avoid having a character-by-character loop,
which is error prone. this may make the code a bit slower, but IMO accuracy
and better understandability are more important in this stage.
added a test case that rev1222 seemingly tried to address.
Added:
trunk/src/lib/auth/testdata/root.zone
trunk/src/lib/auth/testdata/test-root.sqlite3 (with props)
Modified:
trunk/src/lib/auth/data_source_sqlite3.cc
trunk/src/lib/auth/data_source_sqlite3.h
trunk/src/lib/auth/data_source_sqlite3_unittest.cc
Modified: trunk/src/lib/auth/data_source_sqlite3.cc
==============================================================================
--- trunk/src/lib/auth/data_source_sqlite3.cc (original)
+++ trunk/src/lib/auth/data_source_sqlite3.cc Tue Mar 9 08:59:49 2010
@@ -185,10 +185,15 @@
RRsetList& target, const Name* zonename,
const Mode mode, uint32_t& flags) const
{
- const string s_name = name.toText();
- const char* const c_name = s_name.c_str();
+ flags = 0;
+ int zone_id = (zonename == NULL) ? findClosest(name, NULL) :
+ findClosest(*zonename, NULL);
+ if (zone_id < 0) {
+ flags = NO_SUCH_ZONE;
+ return (0);
+ }
+
sqlite3_stmt* query;
-
switch (mode) {
case ADDRESS:
query = q_addrs;
@@ -204,15 +209,6 @@
}
break;
}
-
- flags = 0;
-
- int zone_id = (zonename == NULL) ? findClosest(c_name, NULL) :
- findClosest(zonename->toText().c_str(), NULL);
- if (zone_id < 0) {
- flags = NO_SUCH_ZONE;
- return (0);
- }
sqlite3_reset(query);
sqlite3_clear_bindings(query);
@@ -222,7 +218,8 @@
if (rc != SQLITE_OK) {
throw("Could not bind 1 (query)");
}
- rc = sqlite3_bind_text(query, 2, c_name, -1, SQLITE_STATIC);
+ const string s_name = name.toText();
+ rc = sqlite3_bind_text(query, 2, s_name.c_str(), -1, SQLITE_STATIC);
if (rc != SQLITE_OK) {
throw("Could not bind 2 (query)");
}
@@ -282,29 +279,22 @@
// longest match found.
//
int
-Sqlite3DataSrc::findClosest(const char* const name,
- const char** position) const
-{
- const char* current = name;
-
- while (*current != 0) {
- const int rc = hasExactZone(current);
+Sqlite3DataSrc::findClosest(const Name& name, unsigned int* position) const {
+ const unsigned int nlabels = name.getLabelCount();
+ for (unsigned int i = 0; i < nlabels; ++i) {
+ Name matchname(name.split(i, nlabels - i));
+ const int rc = hasExactZone(matchname.toText().c_str());
if (rc >= 0) {
if (position != NULL) {
- *position = current;
+ *position = i;
}
return (rc);
}
- while (*current != '.' && *current != 0) {
- ++current;
- }
- if (*current == '.' && *(current + 1) != '\0') {
- ++current;
- }
}
return (-1);
}
+
void
Sqlite3DataSrc::loadVersion(void) {
@@ -498,18 +488,20 @@
void
Sqlite3DataSrc::findClosestEnclosure(NameMatch& match,
- const RRClass& qclass) const {
- const char* position = NULL;
-
+ const RRClass& qclass) const
+{
if (qclass != getClass()) {
return;
}
- if (findClosest(match.qname().toText().c_str(), &position) == -1) {
+ unsigned int position;
+ if (findClosest(match.qname(), &position) == -1) {
return;
}
- match.update(*this, Name(position));
+ match.update(*this, match.qname().split(position,
+ match.qname().getLabelCount() -
+ position));
}
DataSrc::Result
@@ -519,8 +511,8 @@
const Name* zonename) const
{
int zone_id = (zonename == NULL) ?
- findClosest(qname.toText().c_str(), NULL) :
- findClosest(zonename->toText().c_str(), NULL);
+ findClosest(qname, NULL) :
+ findClosest(*zonename, NULL);
if (zone_id < 0) {
return (ERROR);
@@ -557,7 +549,7 @@
string& hashstr,
RRsetList& target) const
{
- int zone_id = findClosest(zonename.toText().c_str(), NULL);
+ int zone_id = findClosest(zonename, NULL);
if (zone_id < 0) {
return (ERROR);
}
Modified: trunk/src/lib/auth/data_source_sqlite3.h
==============================================================================
--- trunk/src/lib/auth/data_source_sqlite3.h (original)
+++ trunk/src/lib/auth/data_source_sqlite3.h Tue Mar 9 08:59:49 2010
@@ -121,7 +121,7 @@
int findRecords(const isc::dns::Name& name, const isc::dns::RRType& rdtype,
isc::dns::RRsetList& target, const isc::dns::Name* zonename,
const Mode mode, uint32_t& flags) const;
- int findClosest(const char *name, const char **position) const;
+ int findClosest(const isc::dns::Name& name, unsigned int* position) const;
void loadVersion(void);
void setupPreparedStatements(void);
void execSetupQuery(const char *query);
Modified: trunk/src/lib/auth/data_source_sqlite3_unittest.cc
==============================================================================
--- trunk/src/lib/auth/data_source_sqlite3_unittest.cc (original)
+++ trunk/src/lib/auth/data_source_sqlite3_unittest.cc Tue Mar 9 08:59:49 2010
@@ -46,7 +46,9 @@
"{ \"database_file\": \"testdata/test.sqlite3\"}");
static ElementPtr SQLITE_DBFILE_EXAMPLE2 = Element::createFromString(
"{ \"database_file\": \"testdata/test2.sqlite3\"}");
-// The following file must be non existent and mutt be "creatable";
+static ElementPtr SQLITE_DBFILE_EXAMPLE_ROOT = Element::createFromString(
+ "{ \"database_file\": \"testdata/test-root.sqlite3\"}");
+// The following file must be non existent and must be non"creatable";
// the sqlite3 library will try to create a new DB file if it doesn't exist,
// so to test a failure case the create operation should also fail.
// The "nodir", a non existent directory, is inserted for this purpose.
@@ -342,9 +344,9 @@
answers.push_back(&expected_data);
signatures.push_back(expected_sig_data);
- checkFind(mode, data_source, query, expected_name, zone_name, expected_class,
- expected_type, ttls, expected_flags, types, answers,
- signatures);
+ checkFind(mode, data_source, query, expected_name, zone_name,
+ expected_class, expected_type, ttls, expected_flags, types,
+ answers, signatures);
}
TEST_F(Sqlite3DataSourceTest, close) {
@@ -355,7 +357,6 @@
// Replace the data with a totally different zone. This should succeed,
// and shouldn't match any names in the previously managed domains.
EXPECT_EQ(DataSrc::SUCCESS, data_source.close());
-
EXPECT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_EXAMPLE2));
NameMatch name_match(www_name);
@@ -371,8 +372,18 @@
TEST_F(Sqlite3DataSourceTest, findClosestEnclosure) {
NameMatch name_match(www_name);
- data_source.findClosestEnclosure(name_match, RRClass::IN());
+ data_source.findClosestEnclosure(name_match, rrclass);
EXPECT_EQ(zone_name, *name_match.closestName());
+ EXPECT_EQ(&data_source, name_match.bestDataSrc());
+}
+
+TEST_F(Sqlite3DataSourceTest, findClosestEnclosureMatchRoot) {
+ EXPECT_EQ(DataSrc::SUCCESS, data_source.close());
+ EXPECT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_EXAMPLE_ROOT));
+
+ NameMatch name_match(Name("org."));
+ data_source.findClosestEnclosure(name_match, rrclass);
+ EXPECT_EQ(Name("."), *name_match.closestName());
EXPECT_EQ(&data_source, name_match.bestDataSrc());
}
@@ -380,14 +391,14 @@
// The search name exists both in the parent and child zones, but
// child has a better match.
NameMatch name_match(child_name);
- data_source.findClosestEnclosure(name_match, RRClass::IN());
+ data_source.findClosestEnclosure(name_match, rrclass);
EXPECT_EQ(child_name, *name_match.closestName());
EXPECT_EQ(&data_source, name_match.bestDataSrc());
}
TEST_F(Sqlite3DataSourceTest, findClosestEnclosureNoMatch) {
NameMatch name_match(nomatch_name);
- data_source.findClosestEnclosure(name_match, RRClass::IN());
+ data_source.findClosestEnclosure(name_match, rrclass);
EXPECT_EQ(NULL, name_match.closestName());
EXPECT_EQ(NULL, name_match.bestDataSrc());
}
More information about the bind10-changes
mailing list