From: Marcin Siodelski Date: Wed, 9 Jan 2013 10:18:35 +0000 (+0100) Subject: [2317] More strict checks for option name format. X-Git-Tag: bind10-1.0.0-rc-release~100^2~2^2~18 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c4904fef78c9bae9fdff9aa9852d7b8dca3a9ba8;p=thirdparty%2Fkea.git [2317] More strict checks for option name format. --- diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc index d97ca71d0b..6db9e4bd10 100644 --- a/src/lib/dhcp/option_definition.cc +++ b/src/lib/dhcp/option_definition.cc @@ -23,6 +23,8 @@ #include #include #include +#include +#include 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 deb61b586b..310c7bfb31 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"); }