BIND 10 master, updated. 1a4d0ae65b2c1012611f4c15c5e7a29d65339104 [1627] Add comment about use of try-catch blocks
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri Mar 30 09:19:33 UTC 2012
The branch, master has been updated
via 1a4d0ae65b2c1012611f4c15c5e7a29d65339104 (commit)
via f95e202d7e972f92ebf5ee9ca11c4ad846cd1524 (commit)
via 6ac392ce798a63a024a2890e3838df8a15a233d0 (commit)
via 376c01fc1b6114e6717a9f515999dd1bf67db39a (commit)
via 9d192aa44e2cb1495e78b70ad5af23c21acfe34d (commit)
via 88a80126ce73428584f8f36657cb159e581c700e (commit)
via 77eddb37c51caf6efb688a27aa60ec15fd0d3535 (commit)
via 252fc353bab9ae9eac99cc5c54b66b6e493d91b3 (commit)
via 1a46994e855e3a2a0b5b407df94e52ada3efee7e (commit)
via c80eae98a8dd78a2c0c711f642a02a4b24a3a819 (commit)
via ae9eae29057da3aa8c585b86b662967801ed8bb2 (commit)
via 48cd72d455b5bd1298485f58fc7dcf82c7e4c3cf (commit)
via 06280cc6b480f6976f25ccf8b9cedbdf7aa00e34 (commit)
via af354fd9eb8390cb388348ad66a4d8269b945ac3 (commit)
via 48cfa00bb988cfeebab4e6ac7ae440cd685137e5 (commit)
from 061f5a1ae10248ac161b9c5a489eb0405b2c8426 (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 1a4d0ae65b2c1012611f4c15c5e7a29d65339104
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Mar 30 03:47:06 2012 +0530
[1627] Add comment about use of try-catch blocks
commit f95e202d7e972f92ebf5ee9ca11c4ad846cd1524
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Mar 30 03:45:26 2012 +0530
[1627] Add a couple more testcases
* Case when no file is specified
* Case where both origin and file are not specified
commit 6ac392ce798a63a024a2890e3838df8a15a233d0
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Mar 30 03:41:23 2012 +0530
[master] Fix testcase name
commit 376c01fc1b6114e6717a9f515999dd1bf67db39a
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Mar 30 03:39:02 2012 +0530
[1627] Fix type of exception thrown in testcase
NameParserExceptions are now caught and AuthConfigError is thrown.
commit 9d192aa44e2cb1495e78b70ad5af23c21acfe34d
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri Mar 30 03:32:37 2012 +0530
[1627] Rewrite code to use local variables
commit 88a80126ce73428584f8f36657cb159e581c700e
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu Mar 29 13:59:38 2012 -0700
[1627] (editorial) folded long lines
commit 77eddb37c51caf6efb688a27aa60ec15fd0d3535
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Thu Mar 29 13:32:49 2012 -0700
[1627] (editorial) folded long lines
commit 252fc353bab9ae9eac99cc5c54b66b6e493d91b3
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue Mar 27 16:02:35 2012 +0530
[1627] Add the namestring that triggers the exception to the message
This is so that users see what name caused problems in the log.
commit 1a46994e855e3a2a0b5b407df94e52ada3efee7e
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue Mar 27 13:06:18 2012 +0530
[1627] Unify testcases for NameParserException checks
commit c80eae98a8dd78a2c0c711f642a02a4b24a3a819
Author: Mukund Sivaraman <muks at isc.org>
Date: Tue Mar 27 12:33:38 2012 +0530
[1627] Check for empty origin and file in config, and throw with more relevant error msg
commit ae9eae29057da3aa8c585b86b662967801ed8bb2
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon Mar 26 15:17:11 2012 -0700
[1627] fixed some style issues: untabify, position of '*', fold long lines.
commit 48cd72d455b5bd1298485f58fc7dcf82c7e4c3cf
Author: Mukund Sivaraman <muks at isc.org>
Date: Mon Mar 26 17:48:49 2012 +0530
bug #1627: Update error message
commit 06280cc6b480f6976f25ccf8b9cedbdf7aa00e34
Author: Mukund Sivaraman <muks at isc.org>
Date: Mon Mar 26 17:33:25 2012 +0530
bug #1627: Generate better error messages when origin is not specified
commit af354fd9eb8390cb388348ad66a4d8269b945ac3
Author: Mukund Sivaraman <muks at isc.org>
Date: Mon Mar 26 17:32:16 2012 +0530
bug #1627: Rewrite code to use NameParserException
commit 48cfa00bb988cfeebab4e6ac7ae440cd685137e5
Author: Mukund Sivaraman <muks at isc.org>
Date: Mon Mar 26 17:29:47 2012 +0530
bug #1627: Add common base class for name parser exceptions
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/auth_config.cc | 25 +++++++++++----
src/bin/auth/tests/config_unittest.cc | 18 ++++++++++-
src/lib/datasrc/database.cc | 13 ++------
src/lib/dns/name.cc | 33 ++++++++++++++------
src/lib/dns/name.h | 33 +++++++++++++-------
src/lib/dns/tests/name_unittest.cc | 54 +++++++++++++++++++-------------
6 files changed, 114 insertions(+), 62 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/auth_config.cc b/src/bin/auth/auth_config.cc
index 2ae520c..cbd4ba9 100644
--- a/src/bin/auth/auth_config.cc
+++ b/src/bin/auth/auth_config.cc
@@ -155,17 +155,30 @@ MemoryDatasourceConfig::build(ConstElementPtr config_value) {
BOOST_FOREACH(ConstElementPtr zone_config, zones_config->listValue()) {
ConstElementPtr origin = zone_config->get("origin");
- if (!origin) {
+ const string origin_txt = origin ? origin->stringValue() : "";
+ if (origin_txt.empty()) {
isc_throw(AuthConfigError, "Missing zone origin");
}
ConstElementPtr file = zone_config->get("file");
- if (!file) {
+ const string file_txt = file ? file->stringValue() : "";
+ if (file_txt.empty()) {
isc_throw(AuthConfigError, "Missing zone file for zone: "
<< origin->str());
}
- boost::shared_ptr<InMemoryZoneFinder> zone_finder(new
- InMemoryZoneFinder(rrclass_,
- Name(origin->stringValue())));
+
+ // Note: we don't want to have such small try-catch blocks for each
+ // specific error. We may eventually want to introduce some unified
+ // error handling framework as we have more configuration parameters.
+ // See bug #1627 for the relevant discussion.
+ InMemoryZoneFinder* imzf = NULL;
+ try {
+ imzf = new InMemoryZoneFinder(rrclass_, Name(origin_txt));
+ } catch (const isc::dns::NameParserException& ex) {
+ isc_throw(AuthConfigError, "unable to parse zone's origin: " <<
+ ex.what());
+ }
+
+ boost::shared_ptr<InMemoryZoneFinder> zone_finder(imzf);
const result::Result result = memory_client_->addZone(zone_finder);
if (result == result::EXIST) {
isc_throw(AuthConfigError, "zone "<< origin->str()
@@ -178,7 +191,7 @@ MemoryDatasourceConfig::build(ConstElementPtr config_value) {
* need the load method to be split into some kind of build and
* commit/abort parts.
*/
- zone_finder->load(file->stringValue());
+ zone_finder->load(file_txt);
}
}
diff --git a/src/bin/auth/tests/config_unittest.cc b/src/bin/auth/tests/config_unittest.cc
index fb5067e..96e447c 100644
--- a/src/bin/auth/tests/config_unittest.cc
+++ b/src/bin/auth/tests/config_unittest.cc
@@ -299,7 +299,7 @@ TEST_F(MemoryDatasrcConfigTest, remove) {
EXPECT_EQ(AuthSrv::InMemoryClientPtr(), server.getInMemoryClient(rrclass));
}
-TEST_F(MemoryDatasrcConfigTest, adDuplicateZones) {
+TEST_F(MemoryDatasrcConfigTest, addDuplicateZones) {
EXPECT_THROW(parser->build(
Element::fromJSON(
"[{\"type\": \"memory\","
@@ -313,6 +313,13 @@ TEST_F(MemoryDatasrcConfigTest, adDuplicateZones) {
}
TEST_F(MemoryDatasrcConfigTest, addBadZone) {
+ // origin and file are missing
+ EXPECT_THROW(parser->build(
+ Element::fromJSON(
+ "[{\"type\": \"memory\","
+ " \"zones\": [{}]}]")),
+ AuthConfigError);
+
// origin is missing
EXPECT_THROW(parser->build(
Element::fromJSON(
@@ -320,6 +327,13 @@ TEST_F(MemoryDatasrcConfigTest, addBadZone) {
" \"zones\": [{\"file\": \"example.zone\"}]}]")),
AuthConfigError);
+ // file is missing
+ EXPECT_THROW(parser->build(
+ Element::fromJSON(
+ "[{\"type\": \"memory\","
+ " \"zones\": [{\"origin\": \"example.com\"}]}]")),
+ AuthConfigError);
+
// missing zone file
EXPECT_THROW(parser->build(
Element::fromJSON(
@@ -332,7 +346,7 @@ TEST_F(MemoryDatasrcConfigTest, addBadZone) {
"[{\"type\": \"memory\","
" \"zones\": [{\"origin\": \"example..com\","
" \"file\": \"example.zone\"}]}]")),
- EmptyLabel);
+ AuthConfigError);
// bogus RR class name
EXPECT_THROW(parser->build(
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index 4e2fb15..a5a4edd 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -923,16 +923,9 @@ DatabaseClient::Finder::findPreviousName(const Name& name) const {
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");
+ catch (const isc::dns::NameParserException&) {
+ isc_throw(DataSourceError, "Bad name " + str + " from findPreviousName");
+ }
}
Name
diff --git a/src/lib/dns/name.cc b/src/lib/dns/name.cc
index b56efc4..d642e97 100644
--- a/src/lib/dns/name.cc
+++ b/src/lib/dns/name.cc
@@ -169,7 +169,8 @@ Name::Name(const std::string &namestring, bool downcase) {
//
if (c == '.') {
if (s != send) {
- isc_throw(EmptyLabel, "non terminating empty label");
+ isc_throw(EmptyLabel,
+ "non terminating empty label in " << namestring);
}
is_root = true;
} else if (c == '@' && s == send) {
@@ -197,7 +198,8 @@ Name::Name(const std::string &namestring, bool downcase) {
case ft_ordinary:
if (c == '.') {
if (count == 0) {
- isc_throw(EmptyLabel, "duplicate period");
+ isc_throw(EmptyLabel,
+ "duplicate period in " << namestring);
}
ndata.at(offsets.back()) = count;
offsets.push_back(ndata.size());
@@ -210,7 +212,8 @@ Name::Name(const std::string &namestring, bool downcase) {
state = ft_escape;
} else {
if (++count > MAX_LABELLEN) {
- isc_throw(TooLongLabel, "label is too long");
+ isc_throw(TooLongLabel,
+ "label is too long in " << namestring);
}
ndata.push_back(downcase ? maptolower[c] : c);
}
@@ -219,14 +222,16 @@ Name::Name(const std::string &namestring, bool downcase) {
if (c == '[') {
// This looks like a bitstring label, which was deprecated.
// Intentionally drop it.
- isc_throw(BadLabelType, "invalid label type");
+ isc_throw(BadLabelType,
+ "invalid label type in " << namestring);
}
state = ft_escape;
// FALLTHROUGH
case ft_escape:
if (!isdigit(c & 0xff)) {
if (++count > MAX_LABELLEN) {
- isc_throw(TooLongLabel, "label is too long");
+ isc_throw(TooLongLabel,
+ "label is too long in " << namestring);
}
ndata.push_back(downcase ? maptolower[c] : c);
state = ft_ordinary;
@@ -238,17 +243,22 @@ Name::Name(const std::string &namestring, bool downcase) {
// FALLTHROUGH
case ft_escdecimal:
if (!isdigit(c & 0xff)) {
- isc_throw(BadEscape, "mixture of escaped digit and non-digit");
+ isc_throw(BadEscape,
+ "mixture of escaped digit and non-digit in "
+ << namestring);
}
value *= 10;
value += digitvalue[c];
digits++;
if (digits == 3) {
if (value > 255) {
- isc_throw(BadEscape, "escaped decimal is too large");
+ isc_throw(BadEscape,
+ "escaped decimal is too large in "
+ << namestring);
}
if (++count > MAX_LABELLEN) {
- isc_throw(TooLongLabel, "label is too long");
+ isc_throw(TooLongLabel,
+ "label is too long in " << namestring);
}
ndata.push_back(downcase ? maptolower[value] : value);
state = ft_ordinary;
@@ -262,11 +272,14 @@ Name::Name(const std::string &namestring, bool downcase) {
if (!done) { // no trailing '.' was found.
if (ndata.size() == Name::MAX_WIRE) {
- isc_throw(TooLongName, "name is too long for termination");
+ isc_throw(TooLongName,
+ "name is too long for termination in " << namestring);
}
assert(s == send);
if (state != ft_ordinary && state != ft_at) {
- isc_throw(IncompleteName, "incomplete textual name");
+ isc_throw(IncompleteName,
+ "incomplete textual name in " <<
+ (namestring.empty() ? "<empty>" : namestring));
}
if (state == ft_ordinary) {
assert(count != 0);
diff --git a/src/lib/dns/name.h b/src/lib/dns/name.h
index ca64d69..ef32f90 100644
--- a/src/lib/dns/name.h
+++ b/src/lib/dns/name.h
@@ -32,33 +32,42 @@ namespace dns {
class AbstractMessageRenderer;
///
+/// \brief Base class for name parser exceptions.
+///
+class NameParserException : public Exception {
+public:
+ NameParserException(const char* file, size_t line, const char* what) :
+ isc::Exception(file, line, what) {}
+};
+
+///
/// \brief A standard DNS module exception that is thrown if the name parser
/// encounters an empty label in the middle of a name.
///
-class EmptyLabel : public Exception {
+class EmptyLabel : public NameParserException {
public:
EmptyLabel(const char* file, size_t line, const char* what) :
- isc::Exception(file, line, what) {}
+ NameParserException(file, line, what) {}
};
///
/// \brief A standard DNS module exception that is thrown if the name parser
/// encounters too long a name.
///
-class TooLongName : public Exception {
+class TooLongName : public NameParserException {
public:
TooLongName(const char* file, size_t line, const char* what) :
- isc::Exception(file, line, what) {}
+ NameParserException(file, line, what) {}
};
///
/// \brief A standard DNS module exception that is thrown if the name parser
/// encounters too long a label.
///
-class TooLongLabel : public Exception {
+class TooLongLabel : public NameParserException {
public:
TooLongLabel(const char* file, size_t line, const char* what) :
- isc::Exception(file, line, what) {}
+ NameParserException(file, line, what) {}
};
///
@@ -67,20 +76,20 @@ public:
/// applies to bitstring labels, which would begin with "\[". Incomplete cases
/// include an incomplete escaped sequence such as "\12".
///
-class BadLabelType : public Exception {
+class BadLabelType : public NameParserException {
public:
BadLabelType(const char* file, size_t line, const char* what) :
- isc::Exception(file, line, what) {}
+ NameParserException(file, line, what) {}
};
///
/// \brief A standard DNS module exception that is thrown if the name parser
/// fails to decode a "\"-escaped sequence.
///
-class BadEscape : public Exception {
+class BadEscape : public NameParserException {
public:
BadEscape(const char* file, size_t line, const char* what) :
- isc::Exception(file, line, what) {}
+ NameParserException(file, line, what) {}
};
///
@@ -90,10 +99,10 @@ public:
/// An attempt of constructing a name from an empty string will trigger this
/// exception.
///
-class IncompleteName : public Exception {
+class IncompleteName : public NameParserException {
public:
IncompleteName(const char* file, size_t line, const char* what) :
- isc::Exception(file, line, what) {}
+ NameParserException(file, line, what) {}
};
///
diff --git a/src/lib/dns/tests/name_unittest.cc b/src/lib/dns/tests/name_unittest.cc
index c5f3b7f..be72355 100644
--- a/src/lib/dns/tests/name_unittest.cc
+++ b/src/lib/dns/tests/name_unittest.cc
@@ -130,6 +130,15 @@ TEST_F(NameTest, nonlocalObject) {
EXPECT_EQ("\\255.example.com.", downcased_global.toText());
}
+template <typename ExceptionType>
+void
+checkBadTextName(const string& txt) {
+ // Check it results in the specified type of exception as well as
+ // NameParserException.
+ EXPECT_THROW(Name(txt, false), ExceptionType);
+ EXPECT_THROW(Name(txt, false), NameParserException);
+}
+
TEST_F(NameTest, fromText) {
vector<string> strnames;
strnames.push_back("www.example.com");
@@ -151,45 +160,46 @@ TEST_F(NameTest, fromText) {
EXPECT_EQ(Name("Www.eXample.coM", true).toText(), example_name.toText());
//
- // Tests for bogus names. These should trigger an exception.
+ // Tests for bogus names. These should trigger exceptions.
//
// empty label cannot be followed by another label
- EXPECT_THROW(Name(".a"), EmptyLabel);
+ checkBadTextName<EmptyLabel>(".a");
// duplicate period
- EXPECT_THROW(Name("a.."), EmptyLabel);
+ checkBadTextName<EmptyLabel>("a..");
// label length must be < 64
- EXPECT_THROW(Name("012345678901234567890123456789"
- "012345678901234567890123456789"
- "0123"), TooLongLabel);
+ checkBadTextName<TooLongLabel>("012345678901234567890123456789"
+ "012345678901234567890123456789"
+ "0123");
// now-unsupported bitstring labels
- EXPECT_THROW(Name("\\[b11010000011101]"), BadLabelType);
+ checkBadTextName<BadLabelType>("\\[b11010000011101]");
// label length must be < 64
- EXPECT_THROW(Name("012345678901234567890123456789"
- "012345678901234567890123456789"
- "012\\x"), TooLongLabel);
+ checkBadTextName<TooLongLabel>("012345678901234567890123456789"
+ "012345678901234567890123456789"
+ "012\\x");
// but okay as long as resulting len < 64 even if the original string is
// "too long"
EXPECT_NO_THROW(Name("012345678901234567890123456789"
"012345678901234567890123456789"
"01\\x"));
// incomplete \DDD pattern (exactly 3 D's must appear)
- EXPECT_THROW(Name("\\12abc"), BadEscape);
+ checkBadTextName<BadEscape>("\\12abc");
// \DDD must not exceed 255
- EXPECT_THROW(Name("\\256"), BadEscape);
+ checkBadTextName<BadEscape>("\\256");
// Same tests for \111 as for \\x above
- EXPECT_THROW(Name("012345678901234567890123456789"
- "012345678901234567890123456789"
- "012\\111"), TooLongLabel);
+ checkBadTextName<TooLongLabel>("012345678901234567890123456789"
+ "012345678901234567890123456789"
+ "012\\111");
EXPECT_NO_THROW(Name("012345678901234567890123456789"
"012345678901234567890123456789"
"01\\111"));
// A domain name must be 255 octets or less
- EXPECT_THROW(Name("123456789.123456789.123456789.123456789.123456789."
- "123456789.123456789.123456789.123456789.123456789."
- "123456789.123456789.123456789.123456789.123456789."
- "123456789.123456789.123456789.123456789.123456789."
- "123456789.123456789.123456789.123456789.123456789."
- "1234"), TooLongName);
+ checkBadTextName<TooLongName>("123456789.123456789.123456789.123456789."
+ "123456789.123456789.123456789.123456789."
+ "123456789.123456789.123456789.123456789."
+ "123456789.123456789.123456789.123456789."
+ "123456789.123456789.123456789.123456789."
+ "123456789.123456789.123456789.123456789."
+ "123456789.1234");
// This is a possible longest name and should be accepted
EXPECT_NO_THROW(Name("123456789.123456789.123456789.123456789.123456789."
"123456789.123456789.123456789.123456789.123456789."
@@ -198,7 +208,7 @@ TEST_F(NameTest, fromText) {
"123456789.123456789.123456789.123456789.123456789."
"123"));
// \DDD must consist of 3 digits.
- EXPECT_THROW(Name("\\12"), IncompleteName);
+ checkBadTextName<IncompleteName>("\\12");
// a name with the max number of labels. should be constructed without
// an error, and its length should be the max value.
More information about the bind10-changes
mailing list