Commit 65b09778 authored by JINMEI Tatuya's avatar JINMEI Tatuya
Browse files

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.


git-svn-id: svn://bind10.isc.org/svn/bind10/trunk@1228 e5f2f494-b856-4b98-b285-d166d9295462
parent 6bc6159e
......@@ -185,10 +185,15 @@ Sqlite3DataSrc::findRecords(const Name& name, const RRType& rdtype,
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();
sqlite3_stmt* query;
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 @@ Sqlite3DataSrc::findRecords(const Name& name, const RRType& rdtype,
}
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 @@ Sqlite3DataSrc::findRecords(const Name& name, const RRType& rdtype,
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,30 +279,23 @@ Sqlite3DataSrc::findRecords(const Name& name, const RRType& rdtype,
// 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) {
sqlite3_stmt* prepared = prepare("SELECT version FROM schema_version");
......@@ -498,18 +488,20 @@ Sqlite3DataSrc::init(const isc::data::ElementPtr config) {
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 @@ Sqlite3DataSrc::findPreviousName(const Query& q,
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 @@ Sqlite3DataSrc::findCoveringNSEC3(const Query& q,
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);
}
......
......@@ -121,7 +121,7 @@ private:
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);
......
......@@ -46,7 +46,9 @@ static ElementPtr SQLITE_DBFILE_EXAMPLE = Element::createFromString(
"{ \"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 @@ checkFind(FindMode mode, const Sqlite3DataSrc& data_source, const Query& query,
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 @@ TEST_F(Sqlite3DataSourceTest, reOpen) {
// 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,23 +372,33 @@ TEST_F(Sqlite3DataSourceTest, openFail) {
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());
}
TEST_F(Sqlite3DataSourceTest, findClosestEnclosureAtDelegation) {
// 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());
}
......
. 86400 IN SOA a.root-servers.net. nstld.verisign-grs.com. 2010030802 1800 900 604800 86400
. 518400 IN NS a.root-servers.net.
. 518400 IN NS e.root-servers.net.
. 518400 IN NS g.root-servers.net.
. 518400 IN NS l.root-servers.net.
. 518400 IN NS f.root-servers.net.
. 518400 IN NS k.root-servers.net.
. 518400 IN NS m.root-servers.net.
. 518400 IN NS d.root-servers.net.
. 518400 IN NS h.root-servers.net.
. 518400 IN NS c.root-servers.net.
. 518400 IN NS b.root-servers.net.
. 518400 IN NS i.root-servers.net.
. 518400 IN NS j.root-servers.net.
a.root-servers.net. 3600000 IN A 198.41.0.4
a.root-servers.net. 3600000 IN AAAA 2001:503:ba3e::2:30
b.root-servers.net. 3600000 IN A 192.228.79.201
c.root-servers.net. 3600000 IN A 192.33.4.12
d.root-servers.net. 3600000 IN A 128.8.10.90
e.root-servers.net. 3600000 IN A 192.203.230.10
f.root-servers.net. 3600000 IN A 192.5.5.241
f.root-servers.net. 3600000 IN AAAA 2001:500:2f::f
g.root-servers.net. 3600000 IN A 192.112.36.4
h.root-servers.net. 3600000 IN A 128.63.2.53
h.root-servers.net. 3600000 IN AAAA 2001:500:1::803f:235
i.root-servers.net. 3600000 IN A 192.36.148.17
j.root-servers.net. 3600000 IN A 192.58.128.30
j.root-servers.net. 3600000 IN AAAA 2001:503:c27::2:30
k.root-servers.net. 3600000 IN A 193.0.14.129
k.root-servers.net. 3600000 IN AAAA 2001:7fd::1
l.root-servers.net. 3600000 IN A 199.7.83.42
l.root-servers.net. 3600000 IN AAAA 2001:500:3::42
m.root-servers.net. 3600000 IN A 202.12.27.33
m.root-servers.net. 3600000 IN AAAA 2001:dc3::35
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment