BIND 10 trac2312, updated. c3e696cb4ba09b3cd53842d684f98262cf0cee7a [2312] Fixed unit tests for custom option as a result of review.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Nov 28 16:45:45 UTC 2012
The branch, trac2312 has been updated
via c3e696cb4ba09b3cd53842d684f98262cf0cee7a (commit)
via d85d48eee9608db9f4c912e5cfe3ae1f2f726c05 (commit)
from ee7dbbde6311ec0e9519169871aa182dd7cc1440 (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 c3e696cb4ba09b3cd53842d684f98262cf0cee7a
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Nov 28 17:45:27 2012 +0100
[2312] Fixed unit tests for custom option as a result of review.
commit d85d48eee9608db9f4c912e5cfe3ae1f2f726c05
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Nov 28 17:04:26 2012 +0100
[2312] Changes to the code as a result of review.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/option_custom.cc | 78 ++++++-----
src/lib/dhcp/option_custom.h | 21 ++-
src/lib/dhcp/option_data_types.cc | 46 ++++---
src/lib/dhcp/option_data_types.h | 45 ++++--
src/lib/dhcp/option_definition.cc | 4 +-
src/lib/dhcp/tests/option_custom_unittest.cc | 191 ++++++++++++++++----------
6 files changed, 234 insertions(+), 151 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/option_custom.cc b/src/lib/dhcp/option_custom.cc
index bffadd2..6bc8714 100644
--- a/src/lib/dhcp/option_custom.cc
+++ b/src/lib/dhcp/option_custom.cc
@@ -99,9 +99,12 @@ OptionCustom::createBuffers() {
// Proceed to the next data field.
data += data_size;
}
- } else {
- // If option definition type is not a 'record' than definition
- // type itself indicates what type of data is being held there.
+ } else if (data_type != OPT_EMPTY_TYPE) {
+ // If data_type value is other than OPT_RECORD_TYPE, our option is
+ // empty (have no data at all) or it comprises one or more
+ // data fields of the same type. The type of those fields
+ // is held in the data_type variable so let's use it to determine
+ // a size of buffers.
int data_size = OptionDataTypeUtil::getDataTypeLen(data_type);
// The check below will fail if the input buffer is too short
// for the data size being held by this option.
@@ -122,7 +125,9 @@ OptionCustom::createBuffers() {
assert(data_size > 0);
// Get equal chunks of data and store as collection of buffers.
// Truncate any remaining part which length is not divisible by
- // data_size.
+ // data_size. Note that it is ok to truncate the data if and only
+ // if the data buffer is long enough to keep at least one value.
+ // This has been checked above already.
do {
buffers.push_back(OptionBuffer(data, data + data_size));
data += data_size;
@@ -134,9 +139,9 @@ OptionCustom::createBuffers() {
if (data_size == 0) {
data_size = std::distance(data, data_.end());
}
- if (data_size > 0 && data_type != OPT_EMPTY_TYPE) {
+ if (data_size > 0) {
buffers.push_back(OptionBuffer(data, data + data_size));
- } else if (data_type != OPT_EMPTY_TYPE) {
+ } else {
isc_throw(OutOfRange, "option buffer truncated");
}
}
@@ -148,57 +153,49 @@ OptionCustom::createBuffers() {
std::string
OptionCustom::dataFieldToText(const OptionDataType data_type,
const uint32_t index) const {
- std::ostringstream tmp;
+ std::ostringstream text;
// Get the value of the data field.
switch (data_type) {
case OPT_BINARY_TYPE:
- tmp << util::encode::encodeHex(readBinary(index));
+ text << util::encode::encodeHex(readBinary(index));
break;
case OPT_BOOLEAN_TYPE:
- tmp << (readBoolean(index) ? "true" : "false");
+ text << (readBoolean(index) ? "true" : "false");
break;
case OPT_INT8_TYPE:
- tmp << readInteger<int8_t>(index);
+ text << readInteger<int8_t>(index);
break;
case OPT_INT16_TYPE:
- tmp << readInteger<int16_t>(index);
+ text << readInteger<int16_t>(index);
break;
case OPT_INT32_TYPE:
- tmp << readInteger<int32_t>(index);
+ text << readInteger<int32_t>(index);
break;
case OPT_UINT8_TYPE:
- tmp << readInteger<uint8_t>(index);
+ text << readInteger<uint8_t>(index);
break;
case OPT_UINT16_TYPE:
- tmp << readInteger<uint16_t>(index);
+ text << readInteger<uint16_t>(index);
break;
case OPT_UINT32_TYPE:
- tmp << readInteger<uint32_t>(index);
+ text << readInteger<uint32_t>(index);
break;
case OPT_IPV4_ADDRESS_TYPE:
case OPT_IPV6_ADDRESS_TYPE:
- {
- asiolink::IOAddress address("127.0.0.1");
- readAddress(index, address);
- tmp << address.toText();
- break;
- }
+ text << readAddress(index).toText();
+ break;
case OPT_STRING_TYPE:
- {
- std::string s;
- readString(index, s);
- tmp << s;
- break;
- }
+ text << readString(index);
+ break;
default:
;
}
// Append data field type in brackets.
- tmp << " ( " << OptionDataTypeUtil::getDataTypeName(data_type) << " ) ";
+ text << " ( " << OptionDataTypeUtil::getDataTypeName(data_type) << " ) ";
- return (tmp.str());
+ return (text.str());
}
void
@@ -214,7 +211,12 @@ OptionCustom::pack4(isc::util::OutputBuffer& buf) {
// Write data from buffers.
for (std::vector<OptionBuffer>::const_iterator it = buffers_.begin();
it != buffers_.end(); ++it) {
- buf.writeData(&(*it)[0], it->size());
+ // In theory the createBuffers function should have taken
+ // care that there are no empty buffers added to the
+ // collection but it is almost always good to make sure.
+ if (it->size() > 0) {
+ buf.writeData(&(*it)[0], it->size());
+ }
}
// Write suboptions.
@@ -235,17 +237,17 @@ OptionCustom::pack6(isc::util::OutputBuffer& buf) {
LibDHCP::packOptions(buf, options_);
}
-void
-OptionCustom::readAddress(const uint32_t index, asiolink::IOAddress& address) const {
+asiolink::IOAddress
+OptionCustom::readAddress(const uint32_t index) const {
checkIndex(index);
// The address being read can be either IPv4 or IPv6. The decision
// is made based on the buffer length. If it holds 4 bytes it is IPv4
// address, if it holds 16 bytes it is IPv6.
if (buffers_[index].size() == asiolink::V4ADDRESS_LEN) {
- OptionDataTypeUtil::readAddress(buffers_[index], AF_INET, address);
+ return (OptionDataTypeUtil::readAddress(buffers_[index], AF_INET));
} else if (buffers_[index].size() == asiolink::V6ADDRESS_LEN) {
- OptionDataTypeUtil::readAddress(buffers_[index], AF_INET6, address);
+ return (OptionDataTypeUtil::readAddress(buffers_[index], AF_INET6));
} else {
isc_throw(BadDataTypeCast, "unable to read data from the buffer as"
<< " IP address. Invalid buffer length " << buffers_[index].size());
@@ -264,10 +266,10 @@ OptionCustom::readBoolean(const uint32_t index) const {
return (OptionDataTypeUtil::readBool(buffers_[index]));
}
-void
-OptionCustom::readString(const uint32_t index, std::string& value) const {
+std::string
+OptionCustom::readString(const uint32_t index) const {
checkIndex(index);
- OptionDataTypeUtil::readString(buffers_[index], value);
+ return (OptionDataTypeUtil::readString(buffers_[index]));
}
void
@@ -310,7 +312,7 @@ void OptionCustom::setData(const OptionBufferConstIter first,
createBuffers();
}
-std::string OptionCustom::toText(int indent /* = 0 */ ) {
+std::string OptionCustom::toText(int indent) {
std::stringstream tmp;
for (int i = 0; i < indent; ++i)
diff --git a/src/lib/dhcp/option_custom.h b/src/lib/dhcp/option_custom.h
index 8cefcd0..ad248c5 100644
--- a/src/lib/dhcp/option_custom.h
+++ b/src/lib/dhcp/option_custom.h
@@ -40,6 +40,12 @@ public:
/// @brief Constructor, used for options to be sent.
///
+ /// This constructor creates an instance of an option from the whole
+ /// supplied buffer. This constructor is mainly used to create an
+ /// instances of options to be stored in outgoing DHCP packets.
+ /// The buffer used to create the instance of an option can be
+ /// created from the option data specified in server's configuration.
+ ///
/// @param def option definition.
/// @param u specifies universe (V4 or V6).
/// @param data content of the option.
@@ -51,6 +57,13 @@ public:
/// @brief Constructor, used for received options.
///
+ /// This constructor creates an instance an option from the portion
+ /// of the buffer specified by iterators. This is mainly useful when
+ /// parsing received packets. Such packets are represented by a single
+ /// buffer holding option data and all sub options. Methods that are
+ /// parsing a packet, supply relevant portions of the packet buffer
+ /// to this constructor to create option instances out of it.
+ ///
/// @param def option definition.
/// @param u specifies universe (V4 or V6).
/// @param first iterator to the first element that should be copied.
@@ -71,10 +84,10 @@ public:
/// @brief Read a buffer as IP address.
///
/// @param index buffer index.
- /// @param [out] address read IP address.
///
+ /// @return IP address read from a buffer.
/// @throw isc::OutOfRange if index is out of range.
- void readAddress(const uint32_t index, asiolink::IOAddress& address) const;
+ asiolink::IOAddress readAddress(const uint32_t index) const;
/// @brief Read a buffer as binary data.
///
@@ -140,10 +153,10 @@ public:
/// @brief Read a buffer as string value.
///
/// @param index buffer index.
- /// @param [out] value read string value.
///
+ /// @return string value read from buffer.
/// @throw isc::OutOfRange if index is out of range.
- void readString(const uint32_t index, std::string& value) const;
+ std::string readString(const uint32_t index) const;
/// @brief Parses received buffer.
///
diff --git a/src/lib/dhcp/option_data_types.cc b/src/lib/dhcp/option_data_types.cc
index c469d2b..46aa663 100644
--- a/src/lib/dhcp/option_data_types.cc
+++ b/src/lib/dhcp/option_data_types.cc
@@ -48,6 +48,10 @@ OptionDataTypeUtil::OptionDataTypeUtil() {
data_type_names_[OPT_STRING_TYPE] = "string";
data_type_names_[OPT_FQDN_TYPE] = "fqdn";
data_type_names_[OPT_RECORD_TYPE] = "record";
+ // The "unknown" data type is declared here so as
+ // it can be returned by reference by a getDataTypeName
+ // function it no other type is suitable. Other than that
+ // this is unused.
data_type_names_[OPT_UNKNOWN_TYPE] = "unknown";
}
@@ -73,16 +77,21 @@ OptionDataTypeUtil::getDataTypeLen(const OptionDataType data_type) {
case OPT_INT8_TYPE:
case OPT_UINT8_TYPE:
return (1);
+
case OPT_INT16_TYPE:
case OPT_UINT16_TYPE:
return (2);
+
case OPT_INT32_TYPE:
case OPT_UINT32_TYPE:
return (4);
+
case OPT_IPV4_ADDRESS_TYPE:
return (asiolink::V4ADDRESS_LEN);
+
case OPT_IPV6_ADDRESS_TYPE:
return (asiolink::V6ADDRESS_LEN);
+
default:
;
}
@@ -110,23 +119,22 @@ OptionDataTypeUtil::instance() {
return (instance);
}
-void
+asiolink::IOAddress
OptionDataTypeUtil::readAddress(const std::vector<uint8_t>& buf,
- const short family,
- asiolink::IOAddress& address) {
+ const short family) {
using namespace isc::asiolink;
if (family == AF_INET) {
if (buf.size() < V4ADDRESS_LEN) {
isc_throw(BadDataTypeCast, "unable to read data from the buffer as"
<< " IPv4 address. Invalid buffer size: " << buf.size());
}
- address = IOAddress::fromBytes(AF_INET, &buf[0]);
- } else if (buf.size() == V6ADDRESS_LEN) {
+ return (IOAddress::fromBytes(AF_INET, &buf[0]));
+ } else if (family == AF_INET6) {
if (buf.size() < V6ADDRESS_LEN) {
isc_throw(BadDataTypeCast, "unable to read data from the buffer as"
<< " IPv6 address. Invalid buffer size: " << buf.size());
}
- address = IOAddress::fromBytes(AF_INET6, &buf[0]);
+ return (IOAddress::fromBytes(AF_INET6, &buf[0]));
} else {
isc_throw(BadDataTypeCast, "unable to read data from the buffer as"
"IP address. Invalid family: " << family);
@@ -136,6 +144,9 @@ OptionDataTypeUtil::readAddress(const std::vector<uint8_t>& buf,
void
OptionDataTypeUtil::writeAddress(const asiolink::IOAddress& address,
std::vector<uint8_t>& buf) {
+ // @todo There is a ticket 2396 submitted, which adds the
+ // functionality to return a buffer representation of
+ // IOAddress. If so, this function can be simplified.
if (address.getAddress().is_v4()) {
asio::ip::address_v4::bytes_type addr_bytes =
address.getAddress().to_v4().to_bytes();
@@ -192,28 +203,23 @@ OptionDataTypeUtil::readBool(const std::vector<uint8_t>& buf) {
void
OptionDataTypeUtil::writeBool(const bool value,
std::vector<uint8_t>& buf) {
- if (value) {
- buf.push_back(static_cast<uint8_t>(1));
- } else {
- buf.push_back(static_cast<uint8_t>(0));
- }
+ buf.push_back(static_cast<uint8_t>(value ? 1 : 0));
}
-void
-OptionDataTypeUtil::readString(const std::vector<uint8_t>& buf,
- std::string& value) {
- value.insert(value.end(), buf.begin(), buf.end());
+std::string
+OptionDataTypeUtil::readString(const std::vector<uint8_t>& buf) {
+ std::string value;
+ if (buf.size() > 0) {
+ value.insert(value.end(), buf.begin(), buf.end());
+ }
+ return (value);
}
void
OptionDataTypeUtil::writeString(const std::string& value,
std::vector<uint8_t>& buf) {
if (value.size() > 0) {
- // Increase the size of the storage by the size of the string.
- buf.resize(buf.size() + value.size());
- // Assuming that the string is already UTF8 encoded.
- std::copy_backward(value.c_str(), value.c_str() + value.size(),
- buf.end());
+ buf.insert(buf.end(), value.begin(), value.end());
}
}
diff --git a/src/lib/dhcp/option_data_types.h b/src/lib/dhcp/option_data_types.h
index cd8176d..afc6d93 100644
--- a/src/lib/dhcp/option_data_types.h
+++ b/src/lib/dhcp/option_data_types.h
@@ -41,6 +41,13 @@ public:
/// @brief Data types of DHCP option fields.
+///
+/// @warning The order of data types matters: OPT_UNKNOWN_TYPE
+/// must always be the last position. Also, OPT_RECORD_TYPE
+/// must be at last but one position. This is because some
+/// functions perform sanity checks on data type values using
+/// '>' operators, assuming that all values beyond the
+/// OPT_RECORD_TYPE are invalid.
enum OptionDataType {
OPT_EMPTY_TYPE,
OPT_BINARY_TYPE,
@@ -175,6 +182,16 @@ struct OptionDataTypeTraits<std::string> {
};
/// @brief Utility class for option data types.
+///
+/// This class provides a set of utility functions to operate on
+/// supported DHCP option data types. It includes conversion
+/// between enumerator values representing data types and data
+/// type names. It also includes a set of functions that write
+/// data into option buffers and read data from option buffers.
+/// The data being written and read are converted from/to actual
+/// data types.
+/// @note This is a singleton class but it can be accessed via
+/// static methods only.
class OptionDataTypeUtil {
public:
@@ -212,19 +229,19 @@ public:
///
/// @param buf input buffer.
/// @param family address family: AF_INET or AF_INET6.
- /// @param [out] address being read.
- static void readAddress(const std::vector<uint8_t>& buf,
- const short family,
- asiolink::IOAddress& address);
+ ///
+ /// @return address being read.
+ static asiolink::IOAddress readAddress(const std::vector<uint8_t>& buf,
+ const short family);
- /// @brief Write IPv4 or IPv6 address into a buffer.
+ /// @brief Append IPv4 or IPv6 address to a buffer.
///
/// @param address IPv4 or IPv6 address.
/// @param [out] buf output buffer.
static void writeAddress(const asiolink::IOAddress& address,
std::vector<uint8_t>& buf);
- /// @brief Write hex-encoded binary values into a buffer.
+ /// @brief Append hex-encoded binary values to a buffer.
///
/// @param hex_str string representing a binary value encoded
/// with hexadecimal digits (without 0x prefix).
@@ -238,7 +255,7 @@ public:
/// @return boolean value read from a buffer.
static bool readBool(const std::vector<uint8_t>& buf);
- /// @brief Write boolean value into a buffer.
+ /// @brief Append boolean value into a buffer.
///
/// The bool value is encoded in a buffer in such a way that
/// "1" means "true" and "0" means "false".
@@ -256,7 +273,7 @@ public:
static T readInt(const std::vector<uint8_t>& buf) {
if (!OptionDataTypeTraits<T>::integer_type) {
isc_throw(isc::dhcp::InvalidDataType, "specified data type to be returned"
- " by readInteger is not supported integer type");
+ " by readInteger is unsupported integer type");
}
assert(buf.size() == OptionDataTypeTraits<T>::len);
@@ -266,9 +283,13 @@ public:
value = *(buf.begin());
break;
case 2:
+ // Calling readUint16 works either for unsigned
+ // or signed types.
value = isc::util::readUint16(&(*buf.begin()));
break;
case 4:
+ // Calling readUint32 works either for unsigned
+ // or signed types.
value = isc::util::readUint32(&(*buf.begin()));
break;
default:
@@ -280,7 +301,7 @@ public:
return (value);
}
- /// @brief Write integer or unsigned integer value into a buffer.
+ /// @brief Append integer or unsigned integer value to a buffer.
///
/// @param value an integer value to be written into a buffer.
/// @param [out] buf output buffer.
@@ -314,9 +335,9 @@ public:
/// @brief Read string value from a buffer.
///
/// @param buf input buffer.
- /// @param [out] value string value being read.
- static void readString(const std::vector<uint8_t>& buf,
- std::string& value);
+ ///
+ /// @return string value being read.
+ static std::string readString(const std::vector<uint8_t>& buf);
/// @brief Write UTF8-encoded string into a buffer.
///
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index 20dabfa..58d0c4b 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -350,8 +350,8 @@ OptionDefinition::writeToBuffer(const std::string& value,
case OPT_IPV6_ADDRESS_TYPE:
{
asiolink::IOAddress address(value);
- if (!address.getAddress().is_v4() &&
- !address.getAddress().is_v6()) {
+ if (address.getFamily() != AF_INET &&
+ address.getFamily() != AF_INET6) {
isc_throw(BadDataTypeCast, "provided address " << address.toText()
<< " is not a valid "
<< (address.getAddress().is_v4() ? "IPv4" : "IPv6")
diff --git a/src/lib/dhcp/tests/option_custom_unittest.cc b/src/lib/dhcp/tests/option_custom_unittest.cc
index abfa9ec..34969ee 100644
--- a/src/lib/dhcp/tests/option_custom_unittest.cc
+++ b/src/lib/dhcp/tests/option_custom_unittest.cc
@@ -157,6 +157,12 @@ TEST_F(OptionCustomTest, binaryData) {
// create option instance.
ASSERT_EQ(buf_in.size(), buf_out.size());
EXPECT_TRUE(std::equal(buf_in.begin(), buf_in.end(), buf_out.begin()));
+
+ // Check that option with "no data" is rejected.
+ EXPECT_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V4, OptionBuffer())),
+ isc::OutOfRange
+ );
}
// The purpose of this test is to verify that an option definition comprising
@@ -189,6 +195,12 @@ TEST_F(OptionCustomTest, booleanData) {
// with 0. It is expected to be 'false'.
ASSERT_NO_THROW(value = option->readBoolean(0));
EXPECT_FALSE(value);
+
+ // Check that the option with "no data" is rejected.
+ EXPECT_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V6, OptionBuffer())),
+ isc::OutOfRange
+ );
}
// The purpose of this test is to verify that the option definition comprising
@@ -215,42 +227,42 @@ TEST_F(OptionCustomTest, int16Data) {
int16_t value = 0;
ASSERT_NO_THROW(value = option->readInteger<int16_t>(0));
EXPECT_EQ(-234, value);
+
+ // Check that the option is not created when a buffer is
+ // too short (1 byte instead of 2 bytes).
+ EXPECT_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 1)),
+ isc::OutOfRange
+ );
}
-// The purpose of this test is to verify that truncated option buffer
-// can't be used to form option holding an integer data.
-TEST_F(OptionCustomTest, int32DataTruncated) {
+// The purpose of this test is to verify that the option definition comprising
+// 32-bit signed integer value can be used to create an instance of custom option.
+TEST_F(OptionCustomTest, int32Data) {
OptionDefinition opt_def("OPTION_FOO", 1000, "int32");
- // Put two integer values into the buffer. One of
- // them will be ignored by the option's constructor.
OptionBuffer buf;
writeInt<int32_t>(-234, buf);
writeInt<int32_t>(100, buf);
// Create custom option.
boost::scoped_ptr<OptionCustom> option;
-
- // The length of the provided buffer is equal to 2*sizeof(uint32_t) so
- // both values will be processed by constructor. Second one should be
- // dropped.
- EXPECT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 8));
+ ASSERT_NO_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V6, buf));
);
+ ASSERT_TRUE(option);
- // The option buffer's length is sufficient to keep only one uint32_t
- // but this is ok since option needs only one value anyway.
- EXPECT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 4));
- );
+ // We should have just one data field.
+ ASSERT_EQ(1, option->getDataFieldsNum());
- // The option buffer's length is equal to sizeof(uint32_t) so it should
- // be processed successfuly.
- EXPECT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 4));
- );
+ // Initialize value to 0 explicitely to make sure that is
+ // modified by readInteger function to expected -234.
+ int32_t value = 0;
+ ASSERT_NO_THROW(value = option->readInteger<int32_t>(0));
+ EXPECT_EQ(-234, value);
- // The option bufferis now 1 byte too short. Expect exception being thrown.
+ // Check that the option is not created when a buffer is
+ // too short (3 bytes instead of 4 bytes).
EXPECT_THROW(
option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 3)),
isc::OutOfRange
@@ -278,9 +290,16 @@ TEST_F(OptionCustomTest, ipv4AddressData) {
IOAddress address("127.0.0.1");
// Read IPv4 address from using index 0.
- ASSERT_NO_THROW(option->readAddress(0, address));
+ ASSERT_NO_THROW(address = option->readAddress(0));
EXPECT_EQ("192.168.100.50", address.toText());
+
+ // Check that option is not created if the provided buffer is
+ // too short (use 3 bytes instead of 4).
+ EXPECT_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V4, buf.begin(), buf.begin() + 3)),
+ isc::OutOfRange
+ );
}
// The purpose of this test is to verify that the option definition comprising
@@ -306,9 +325,17 @@ TEST_F(OptionCustomTest, ipv6AddressData) {
// IPv6 address.
IOAddress address("::1");
// Read an address from buffer #0.
- ASSERT_NO_THROW(option->readAddress(0, address));
+ ASSERT_NO_THROW(address = option->readAddress(0));
EXPECT_EQ("2001:db8:1::100", address.toText());
+
+ // Check that option is not created if the provided buffer is
+ // too short (use 15 bytes instead of 16).
+ EXPECT_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V4, buf.begin(),
+ buf.begin() + 15)),
+ isc::OutOfRange
+ );
}
// The purpose of this test is to verify that the option definition comprising
@@ -333,9 +360,15 @@ TEST_F(OptionCustomTest, stringData) {
// Custom option should now comprise single string value that
// can be accessed using index 0.
std::string value;
- ASSERT_NO_THROW(option->readString(0, value));
+ ASSERT_NO_THROW(value = option->readString(0));
EXPECT_EQ("hello world!", value);
+
+ // Check that option will not be created if empty buffer is provided.
+ EXPECT_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V6, OptionBuffer())),
+ isc::OutOfRange
+ );
}
// The purpose of this test is to verify that the option definition comprising
@@ -383,44 +416,12 @@ TEST_F(OptionCustomTest, booleanDataArray) {
bool value4 = false;
ASSERT_NO_THROW(value4 = option->readBoolean(4));
EXPECT_TRUE(value4);
-}
-
-// The purpose of this test is to verify that truncated buffer can't
-// be used to create an instance of the option which holds an array
-// of values.
-TEST_F(OptionCustomTest, uint16DataArrayTruncated) {
- OptionDefinition opt_def("OPTION_FOO", 1000, "uint16", true);
- OptionBuffer buf;
- for (int i = 0; i < 3; ++i) {
- writeInt<uint16_t>(i, buf);
- }
-
- // Create custom option using the input buffer.
- boost::scoped_ptr<OptionCustom> option;
-
- // Provide a buffer of a length of 4. This should succeed because exactly two
- // uint16_t values fit into it.
- EXPECT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 4));
- );
-
- // Provide a buffer of a length of 3. This should succeed because one uint16_t
- // value fits into it despite the second one is truncated.
- EXPECT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 3));
- );
-
- // Provide a buffer of a length of 2. This should succeed because still
- // one uint16_t fits into it.
- EXPECT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 2));
- );
-
- // Provide truncated buffer. This should cause the exception.
+ // Check that empty buffer can't be used to create option holding
+ // array of boolean values.
EXPECT_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 1)),
- isc::OutOfRange
+ option.reset(new OptionCustom(opt_def, Option::V6, OptionBuffer())),
+ isc::OutOfRange
);
}
@@ -462,6 +463,16 @@ TEST_F(OptionCustomTest, uint32DataArray) {
ASSERT_NO_THROW(value = option->readInteger<uint32_t>(i));
EXPECT_EQ(values[i], value);
}
+
+ // Check that too short buffer can't be used to create the option.
+ // Using buffer having length of 3 bytes. The length of 4 bytes is
+ // a minimal length to create the option with single uint32_t value.
+ EXPECT_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(),
+ buf.begin() + 3)),
+ isc::OutOfRange
+ );
+
}
// The purpose of this test is to verify that the option definition comprising
@@ -484,7 +495,7 @@ TEST_F(OptionCustomTest, ipv4AddressDataArray) {
// Use the input buffer to create custom option.
boost::scoped_ptr<OptionCustom> option;
ASSERT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V4, buf.begin(), buf.begin() + 13));
+ option.reset(new OptionCustom(opt_def, Option::V4, buf));
);
ASSERT_TRUE(option);
@@ -494,9 +505,24 @@ TEST_F(OptionCustomTest, ipv4AddressDataArray) {
// We expect 3 IPv4 addresses being stored in the option.
for (int i = 0; i < 3; ++i) {
IOAddress address("10.10.10.10");
- ASSERT_NO_THROW(option->readAddress(i, address));
+ ASSERT_NO_THROW(address = option->readAddress(i));
EXPECT_EQ(addresses[i].toText(), address.toText());
}
+
+ // Check that it is ok if buffer length is not a multiple of IPv4
+ // address length. Resize it by two bytes.
+ buf.resize(buf.size() + 2);
+ EXPECT_NO_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V4, buf));
+ );
+
+ // Check that option is not created when the provided buffer
+ // is too short. At least a buffer length of 4 bytes is needed.
+ EXPECT_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V4, buf.begin(),
+ buf.begin() + 2)),
+ isc::OutOfRange
+ );
}
// The purpose of this test is to verify that the option definition comprising
@@ -519,7 +545,7 @@ TEST_F(OptionCustomTest, ipv6AddressDataArray) {
// Use the input buffer to create custom option.
boost::scoped_ptr<OptionCustom> option;
ASSERT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 50));
+ option.reset(new OptionCustom(opt_def, Option::V6, buf));
);
ASSERT_TRUE(option);
@@ -529,9 +555,24 @@ TEST_F(OptionCustomTest, ipv6AddressDataArray) {
// We expect 3 IPv6 addresses being stored in the option.
for (int i = 0; i < 3; ++i) {
IOAddress address("fe80::4");
- ASSERT_NO_THROW(option->readAddress(i, address));
+ ASSERT_NO_THROW(address = option->readAddress(i));
EXPECT_EQ(addresses[i].toText(), address.toText());
}
+
+ // Check that it is ok if buffer length is not a multiple of IPv6
+ // address length. Resize it by two bytes.
+ buf.resize(buf.size() + 2);
+ EXPECT_NO_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V6, buf));
+ );
+
+ // Check that option is not created when the provided buffer
+ // is too short. At least a buffer length of 16 bytes is needed.
+ EXPECT_THROW(
+ option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(),
+ buf.begin() + 15)),
+ isc::OutOfRange
+ );
}
// The purpose of this test is to verify that the option definition comprising
@@ -561,7 +602,7 @@ TEST_F(OptionCustomTest, recordData) {
boost::scoped_ptr<OptionCustom> option;
ASSERT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 27));
+ option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.end()));
);
ASSERT_TRUE(option);
@@ -580,17 +621,17 @@ TEST_F(OptionCustomTest, recordData) {
// Verify value in the field 2.
IOAddress value2("127.0.0.1");
- ASSERT_NO_THROW(option->readAddress(2, value2));
+ ASSERT_NO_THROW(value2 = option->readAddress(2));
EXPECT_EQ("192.168.0.1", value2.toText());
// Verify value in the field 3.
IOAddress value3("::1");
- ASSERT_NO_THROW(option->readAddress(3, value3));
+ ASSERT_NO_THROW(value3 = option->readAddress(3));
EXPECT_EQ("2001:db8:1::1", value3.toText());
// Verify value in the field 4.
std::string value4;
- ASSERT_NO_THROW(option->readString(4, value4));
+ ASSERT_NO_THROW(value4 = option->readString(4));
EXPECT_EQ("ABCD", value4);
}
@@ -730,7 +771,7 @@ TEST_F(OptionCustomTest, unpack) {
// Use the input buffer to create custom option.
boost::scoped_ptr<OptionCustom> option;
ASSERT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V4, buf.begin(), buf.begin() + 13));
+ option.reset(new OptionCustom(opt_def, Option::V4, buf.begin(), buf.end()));
);
ASSERT_TRUE(option);
@@ -740,7 +781,7 @@ TEST_F(OptionCustomTest, unpack) {
// We expect 3 IPv4 addresses being stored in the option.
for (int i = 0; i < 3; ++i) {
IOAddress address("10.10.10.10");
- ASSERT_NO_THROW(option->readAddress(i, address));
+ ASSERT_NO_THROW(address = option->readAddress(i));
EXPECT_EQ(addresses[i].toText(), address.toText());
}
@@ -767,7 +808,7 @@ TEST_F(OptionCustomTest, unpack) {
// Verify that the addresses have been overwritten.
for (int i = 0; i < 2; ++i) {
IOAddress address("10.10.10.10");
- ASSERT_NO_THROW(option->readAddress(i, address));
+ ASSERT_NO_THROW(address = option->readAddress(i));
EXPECT_EQ(addresses[i].toText(), address.toText());
}
}
@@ -792,7 +833,7 @@ TEST_F(OptionCustomTest, setData) {
// Use the input buffer to create custom option.
boost::scoped_ptr<OptionCustom> option;
ASSERT_NO_THROW(
- option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.begin() + 50));
+ option.reset(new OptionCustom(opt_def, Option::V6, buf.begin(), buf.end()));
);
ASSERT_TRUE(option);
@@ -802,7 +843,7 @@ TEST_F(OptionCustomTest, setData) {
// We expect 3 IPv6 addresses being stored in the option.
for (int i = 0; i < 3; ++i) {
IOAddress address("fe80::4");
- ASSERT_NO_THROW(option->readAddress(i, address));
+ ASSERT_NO_THROW(address = option->readAddress(i));
EXPECT_EQ(addresses[i].toText(), address.toText());
}
@@ -828,7 +869,7 @@ TEST_F(OptionCustomTest, setData) {
// Check that it has been replaced.
for (int i = 0; i < 2; ++i) {
IOAddress address("10.10.10.10");
- ASSERT_NO_THROW(option->readAddress(i, address));
+ ASSERT_NO_THROW(address = option->readAddress(i));
EXPECT_EQ(addresses[i].toText(), address.toText());
}
}
More information about the bind10-changes
mailing list