BIND 10 trac2312, updated. f47316a1d2b49791b785cf7b8b5e11a13a7a0fbe [2312] Initialize custom option's buffers when option is not a record.
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Nov 26 21:08:54 UTC 2012
The branch, trac2312 has been updated
via f47316a1d2b49791b785cf7b8b5e11a13a7a0fbe (commit)
from 8f0e9452fd3cc94ef44352839e91e2a4a1d91c55 (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 f47316a1d2b49791b785cf7b8b5e11a13a7a0fbe
Author: Marcin Siodelski <marcin at isc.org>
Date: Mon Nov 26 22:08:43 2012 +0100
[2312] Initialize custom option's buffers when option is not a record.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/option_custom.cc | 43 ++++++++++++++-----
src/lib/dhcp/option_definition.cc | 43 +++++++++++++------
src/lib/dhcp/tests/option_custom_unittest.cc | 37 ++++++++++++++++
src/lib/dhcp/tests/option_definition_unittest.cc | 50 ----------------------
4 files changed, 100 insertions(+), 73 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/option_custom.cc b/src/lib/dhcp/option_custom.cc
index f02f6d7..3b85474 100644
--- a/src/lib/dhcp/option_custom.cc
+++ b/src/lib/dhcp/option_custom.cc
@@ -61,10 +61,10 @@ OptionCustom::createBuffers() {
definition_.validate();
std::vector<OptionBuffer> buffers;
+ OptionBuffer::iterator data = data_.begin();
if (definition_.getType() == OPT_RECORD_TYPE) {
const OptionDefinition::RecordFieldsCollection& fields =
definition_.getRecordFields();
- OptionBuffer::iterator data = data_.begin();
for (OptionDefinition::RecordFieldsConstIter field = fields.begin();
field != fields.end(); ++field) {
int data_size = OptionDataTypeUtil::getDataTypeLen(*field);
@@ -73,22 +73,45 @@ OptionCustom::createBuffers() {
data_size = asiolink::V4ADDRESS_LEN;
} else if (*field == OPT_IPV6_ADDRESS_TYPE) {
data_size = asiolink::V6ADDRESS_LEN;
- } else if (*field == OPT_STRING_TYPE ||
- *field == OPT_FQDN_TYPE ||
- *field == OPT_BINARY_TYPE) {
- data_size = std::distance(data, data_.end());
} else {
- isc_throw(InvalidDataType, "invalid option data type"
- << " used in the option record");
+ data_size = std::distance(data, data_.end());
}
}
- if (std::distance(data, data_.end()) < data_size ||
- data_size == 0) {
+ if (data_size == 0) {
isc_throw(OutOfRange, "option buffer truncated");
}
buffers_.push_back(OptionBuffer(data, data + data_size));
data += data_size;
}
+ } else {
+ OptionDataType data_type = definition_.getType();
+ int data_size = OptionDataTypeUtil::getDataTypeLen(data_type);
+ if (data_size == 0) {
+ if (data_type == OPT_IPV4_ADDRESS_TYPE) {
+ data_size = asiolink::V4ADDRESS_LEN;
+ } else if (data_type == OPT_IPV6_ADDRESS_TYPE) {
+ data_size = asiolink::V6ADDRESS_LEN;
+ }
+ }
+ if (std::distance(data, data_.end()) < data_size) {
+ isc_throw(OutOfRange, "option buffer truncated");
+ }
+ if (definition_.getArrayType()) {
+ // We don't perform other checks for data types that can't be
+ // used together with array indicator such as strings, empty field
+ // etc. This is because OptionDefinition::validate function should
+ // have checked this already.
+ assert(data_size > 0);
+ do {
+ buffers_.push_back(OptionBuffer(data, data + data_size));
+ data += data_size;
+ } while (std::distance(data, data_.end()) >= data_size);
+ } else {
+ if (data_size == 0) {
+ data_size = std::distance(data, data_.end());
+ }
+ buffers_.push_back(OptionBuffer(data, data + data_size));
+ }
}
}
@@ -105,7 +128,6 @@ OptionCustom::pack4(isc::util::OutputBuffer& buf) {
// @todo write option data here
LibDHCP::packOptions(buf, options_);
- return;
}
void
@@ -116,7 +138,6 @@ OptionCustom::pack6(isc::util::OutputBuffer& buf) {
// @todo write option data here.
LibDHCP::packOptions(buf, options_);
- return;
}
void
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index 68f1259..2b4fddb 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -209,8 +209,10 @@ OptionDefinition::addRecordField(const OptionDataType data_type) {
isc_throw(isc::InvalidOperation, "'record' option type must be used"
" to add data fields to the record");
}
- if (data_type >= OPT_UNKNOWN_TYPE) {
- isc_throw(isc::BadValue, "attempted to add invalid data type to the record");
+ if (data_type >= OPT_RECORD_TYPE ||
+ data_type == OPT_ANY_ADDRESS_TYPE ||
+ data_type == OPT_EMPTY_TYPE) {
+ isc_throw(isc::BadValue, "attempted to add invalid data type to the record.");
}
record_fields_.push_back(data_type);
}
@@ -321,18 +323,24 @@ OptionDefinition::validate() const {
std::ostringstream err_str;
if (name_.empty()) {
// Option name must not be empty.
- err_str << "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";
+ err_str << "option name must not contain spaces.";
} 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 (type_ == OPT_STRING_TYPE && array_type_) {
- // Array of strings is not allowed because there is no way
- // to determine the size of a particular string and thus there
- // it no way to tell when other data fields begin.
- err_str << "array of strings is not a valid option definition";
+ 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
+ // to determine the size of a particular string and thus there
+ // it no way to tell when other data fields begin.
+ 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.
@@ -342,8 +350,8 @@ OptionDefinition::validate() const {
<< " 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 data fields are not laid before
- // other fields. But we allow that they are laid at the end of
+ // is valid too. We check that string or binary data fields are not
+ // laid before other fields. But we allow that they are laid at the end of
// an option.
const RecordFieldsCollection& fields = getRecordFields();
for (RecordFieldsConstIter it = fields.begin();
@@ -354,6 +362,17 @@ OptionDefinition::validate() const {
<< " of other types.";
break;
}
+ if (*it == OPT_BINARY_TYPE &&
+ it < fields.end() - 1) {
+ err_str << "binary data field can't be laid before data fields"
+ << " of other types.";
+ }
+ /// Empty typy is not allowed within a record.
+ if (*it == OPT_EMPTY_TYPE) {
+ err_str << "empty data type can't be stored as a field in an"
+ << " option record.";
+ break;
+ }
}
}
diff --git a/src/lib/dhcp/tests/option_custom_unittest.cc b/src/lib/dhcp/tests/option_custom_unittest.cc
index 43fb11a..b6bfaf3 100644
--- a/src/lib/dhcp/tests/option_custom_unittest.cc
+++ b/src/lib/dhcp/tests/option_custom_unittest.cc
@@ -70,6 +70,43 @@ TEST_F(OptionCustomTest, constructor) {
ASSERT_THROW(opt_def1.validate(), isc::Exception); */
}
+TEST_F(OptionCustomTest, setData) {
+ OptionDefinition opt_def("OPTION_FOO", 1000, "string");
+
+ OptionBuffer buf;
+ writeString("hello world!", buf);
+ OptionCustom option(opt_def, Option::V6, buf.begin(), buf.end());
+ ASSERT_TRUE(option.valid());
+
+ std::string value;
+ ASSERT_NO_THROW(option.readString(0, value));
+
+ EXPECT_EQ("hello world!", value);
+}
+
+TEST_F(OptionCustomTest, setArrayData) {
+ OptionDefinition opt_def("OPTION_FOO", 1000, "uint32", true);
+
+ std::vector<uint32_t> values;
+ values.push_back(71234);
+ values.push_back(12234);
+ values.push_back(54362);
+ values.push_back(1234);
+
+ OptionBuffer buf;
+ for (int i = 0; i < values.size(); ++i) {
+ writeInt<uint32_t>(values[i], buf);
+ }
+ OptionCustom option(opt_def, Option::V6, buf.begin(), buf.begin() + 13);
+ ASSERT_TRUE(option.valid());
+
+ for (int i = 0; i < 3; ++i) {
+ uint32_t value = 0;
+ ASSERT_NO_THROW(value = option.readInteger<uint32_t>(i));
+ EXPECT_EQ(values[i], value);
+ }
+}
+
TEST_F(OptionCustomTest, setRecordData) {
OptionDefinition opt_def("OPTION_FOO", 1000, "record");
ASSERT_NO_THROW(opt_def.addRecordField("uint16"));
diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc
index ef7428a..20c87d6 100644
--- a/src/lib/dhcp/tests/option_definition_unittest.cc
+++ b/src/lib/dhcp/tests/option_definition_unittest.cc
@@ -458,56 +458,6 @@ TEST_F(OptionDefinitionTest, binary) {
buf.begin()));
}
-// The purpose of this test is to verify that definition can be
-// creates for the option that holds binary data and that the
-// binary data can be specified in 'comma separated values'
-// format.
-TEST_F(OptionDefinitionTest, binaryTokenized) {
- OptionDefinition opt_def("OPTION_FOO", 1000, "binary", true);
-
- // Prepare some dummy data (serverid): 0, 1, 2 etc.
- OptionBuffer buf(16);
- for (int i = 0; i < buf.size(); ++i) {
- buf[i] = i;
- }
- std::vector<std::string> hex_data;
- hex_data.push_back("00010203");
- hex_data.push_back("04050607");
- hex_data.push_back("08090A0B0C0D0E0F");
-
- // Create option instance with the factory function.
- // If the OptionDefinition code works properly than
- // object of the type Option should be returned.
- OptionPtr option_v6;
- ASSERT_NO_THROW(
- option_v6 = opt_def.optionFactory(Option::V6, 1000, hex_data);
- );
- // Expect base option type returned.
- ASSERT_TRUE(typeid(*option_v6) == typeid(Option));
- // Sanity check on universe, length and size. These are
- // the basic parameters identifying any option.
- EXPECT_EQ(Option::V6, option_v6->getUniverse());
- EXPECT_EQ(4, option_v6->getHeaderLen());
- ASSERT_EQ(buf.size(), option_v6->getData().size());
-
- // Get data from the option and compare against reference buffer.
- // They are expected to match.
- EXPECT_TRUE(std::equal(option_v6->getData().begin(),
- option_v6->getData().end(),
- buf.begin()));
-
- // Repeat the same test scenario for DHCPv4 option.
- OptionPtr option_v4;
- ASSERT_NO_THROW(option_v4 = opt_def.optionFactory(Option::V4, 214, hex_data));
- EXPECT_EQ(Option::V4, option_v4->getUniverse());
- EXPECT_EQ(2, option_v4->getHeaderLen());
- ASSERT_EQ(buf.size(), option_v4->getData().size());
-
- EXPECT_TRUE(std::equal(option_v6->getData().begin(),
- option_v6->getData().end(),
- buf.begin()));
-}
-
// The purpose of this test is to verify that definition can be created
// for option that comprises record of data. In this particular test
// the IA_NA option is used. This option comprises three uint32 fields.
More information about the bind10-changes
mailing list