BIND 10 trac2317, updated. c4904fef78c9bae9fdff9aa9852d7b8dca3a9ba8 [2317] More strict checks for option name format.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Jan 9 10:18:47 UTC 2013
The branch, trac2317 has been updated
via c4904fef78c9bae9fdff9aa9852d7b8dca3a9ba8 (commit)
from e39b61088924933e1335760d21cfa10e0cfd5425 (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 c4904fef78c9bae9fdff9aa9852d7b8dca3a9ba8
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Jan 9 11:18:35 2013 +0100
[2317] More strict checks for option name format.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/option_definition.cc | 31 +++++++++++---
src/lib/dhcp/tests/option_definition_unittest.cc | 50 ++++++++++++++++------
2 files changed, 63 insertions(+), 18 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index d97ca71..6db9e4b 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -23,6 +23,8 @@
#include <dhcp/option_int_array.h>
#include <util/encode/hex.h>
#include <util/strutil.h>
+#include <boost/algorithm/string/classification.hpp>
+#include <boost/algorithm/string/predicate.hpp>
using namespace std;
using namespace isc::util;
@@ -207,16 +209,29 @@ OptionDefinition::sanityCheckUniverse(const Option::Universe expected_universe,
void
OptionDefinition::validate() const {
+
+ using namespace boost::algorithm;
+
std::ostringstream err_str;
- if (name_.empty()) {
- // Option name must not be empty.
- err_str << "option name must not be empty.";
- } else if (name_.find(" ") != string::npos) {
- // Option name must not contain spaces.
- err_str << "option name must not contain spaces.";
+
+ // Allowed characters in the option name are: lower or
+ // upper case letters, digits, underscores and hyphens.
+ // Empty option spaces are not allowed.
+ if (!all(name_, boost::is_from_range('a', 'z') ||
+ boost::is_from_range('A', 'Z') ||
+ boost::is_digit() ||
+ boost::is_any_of("-_")) ||
+ name_.empty() ||
+ // Hyphens and underscores are not allowed at the beginning
+ // and at the end of the option name.
+ all(find_head(name_, 1), boost::is_any_of("-_")) ||
+ all(find_tail(name_, 1), boost::is_any_of("-_"))) {
+ err_str << "invalid option name '" << name_ << "'";
+
} else if (type_ >= OPT_UNKNOWN_TYPE) {
// Option definition must be of a known type.
err_str << "option type value " << type_ << " is out of range.";
+
} else if (array_type_) {
if (type_ == OPT_STRING_TYPE) {
// Array of strings is not allowed because there is no way
@@ -225,9 +240,12 @@ OptionDefinition::validate() const {
err_str << "array of strings is not a valid option definition.";
} else if (type_ == OPT_BINARY_TYPE) {
err_str << "array of binary values is not a valid option definition.";
+
} else if (type_ == OPT_EMPTY_TYPE) {
err_str << "array of empty value is not a valid option definition.";
+
}
+
} else if (type_ == OPT_RECORD_TYPE) {
// At least two data fields should be added to the record. Otherwise
// non-record option definition could be used.
@@ -235,6 +253,7 @@ OptionDefinition::validate() const {
err_str << "invalid number of data fields: " << getRecordFields().size()
<< " specified for the option of type 'record'. Expected at"
<< " least 2 fields.";
+
} else {
// If the number of fields is valid we have to check if their order
// is valid too. We check that string or binary data fields are not
diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc
index deb61b5..310c7bf 100644
--- a/src/lib/dhcp/tests/option_definition_unittest.cc
+++ b/src/lib/dhcp/tests/option_definition_unittest.cc
@@ -164,29 +164,55 @@ TEST_F(OptionDefinitionTest, validate) {
EXPECT_THROW(opt_def5.validate(), MalformedOptionDefinition);
// Option name must not contain spaces.
- OptionDefinition opt_def6("OPTION CLIENTID", D6O_CLIENTID, "string", true);
+ OptionDefinition opt_def6("OPTION CLIENTID", D6O_CLIENTID, "string");
EXPECT_THROW(opt_def6.validate(), MalformedOptionDefinition);
+ // Option name may contain lower case letters.
+ OptionDefinition opt_def7("option_clientid", D6O_CLIENTID, "string");
+ EXPECT_NO_THROW(opt_def7.validate());
+
+ // Using digits in option name is legal.
+ OptionDefinition opt_def8("option_123", D6O_CLIENTID, "string");
+ EXPECT_NO_THROW(opt_def8.validate());
+
+ // Using hyphen is legal.
+ OptionDefinition opt_def9("option-clientid", D6O_CLIENTID, "string");
+ EXPECT_NO_THROW(opt_def9.validate());
+
+ // Using hyphen or undescore at the beginning or at the end
+ // of the option name is not allowed.
+ OptionDefinition opt_def10("-option-clientid", D6O_CLIENTID, "string");
+ EXPECT_THROW(opt_def10.validate(), MalformedOptionDefinition);
+
+ OptionDefinition opt_def11("_option-clientid", D6O_CLIENTID, "string");
+ EXPECT_THROW(opt_def11.validate(), MalformedOptionDefinition);
+
+ OptionDefinition opt_def12("option-clientid_", D6O_CLIENTID, "string");
+ EXPECT_THROW(opt_def12.validate(), MalformedOptionDefinition);
+
+ OptionDefinition opt_def13("option-clientid-", D6O_CLIENTID, "string");
+ EXPECT_THROW(opt_def13.validate(), MalformedOptionDefinition);
+
// Having array of strings does not make sense because there is no way
// to determine string's length.
- OptionDefinition opt_def7("OPTION_CLIENTID", D6O_CLIENTID, "string", true);
- EXPECT_THROW(opt_def7.validate(), MalformedOptionDefinition);
+ OptionDefinition opt_def14("OPTION_CLIENTID", D6O_CLIENTID, "string", true);
+ EXPECT_THROW(opt_def14.validate(), MalformedOptionDefinition);
// It does not make sense to have string field within the record before
// other fields because there is no way to determine the length of this
// string and thus there is no way to determine where the other field
// begins.
- OptionDefinition opt_def8("OPTION_STATUS_CODE", D6O_STATUS_CODE,
- "record");
- opt_def8.addRecordField("string");
- opt_def8.addRecordField("uint16");
- EXPECT_THROW(opt_def8.validate(), MalformedOptionDefinition);
+ OptionDefinition opt_def15("OPTION_STATUS_CODE", D6O_STATUS_CODE,
+ "record");
+ opt_def15.addRecordField("string");
+ opt_def15.addRecordField("uint16");
+ EXPECT_THROW(opt_def15.validate(), MalformedOptionDefinition);
// ... but it is ok if the string value is the last one.
- OptionDefinition opt_def9("OPTION_STATUS_CODE", D6O_STATUS_CODE,
- "record");
- opt_def9.addRecordField("uint8");
- opt_def9.addRecordField("string");
+ OptionDefinition opt_def16("OPTION_STATUS_CODE", D6O_STATUS_CODE,
+ "record");
+ opt_def16.addRecordField("uint8");
+ opt_def16.addRecordField("string");
}
More information about the bind10-changes
mailing list