BIND 10 trac2490, updated. e77b710633d5cf1d34dea5153a0f77b7d1f111b5 [2490] Use option definitions to unpack options from received packets.

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Nov 16 21:21:24 UTC 2012


The branch, trac2490 has been updated
       via  e77b710633d5cf1d34dea5153a0f77b7d1f111b5 (commit)
      from  b4b3814079cf33af52d9ab257464ef0685894b82 (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 e77b710633d5cf1d34dea5153a0f77b7d1f111b5
Author: Marcin Siodelski <marcin at isc.org>
Date:   Fri Nov 16 22:21:02 2012 +0100

    [2490] Use option definitions to unpack options from received packets.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dhcp/libdhcp++.cc                        |   65 +++++++----
 src/lib/dhcp/libdhcp++.h                         |    4 +
 src/lib/dhcp/option_definition.cc                |   20 ++--
 src/lib/dhcp/option_definition.h                 |   10 --
 src/lib/dhcp/tests/libdhcp++_unittest.cc         |  129 ++++++++++++++--------
 src/lib/dhcp/tests/option6_ia_unittest.cc        |    2 +-
 src/lib/dhcp/tests/option_definition_unittest.cc |   41 +------
 7 files changed, 140 insertions(+), 131 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc
index 7fb88bf..32d8a03 100644
--- a/src/lib/dhcp/libdhcp++.cc
+++ b/src/lib/dhcp/libdhcp++.cc
@@ -48,8 +48,14 @@ const OptionDefContainer&
 LibDHCP::getOptionDefs(Option::Universe u) {
     switch (u) {
     case Option::V4:
+        isc_throw(isc::InvalidOperation, "DHCPv4 option definitions not initialized."
+                  << " Call initStdOptionDefs first");
         return (v4option_defs_);
     case Option::V6:
+        if (v6option_defs_.size() == 0) {
+            isc_throw(isc::InvalidOperation, "DHCPv6 option definitions not initialized."
+                      << " Call initStdOptionDefs first");
+        }
         return (v6option_defs_);
     default:
         isc_throw(isc::BadValue, "invalid universe " << u << " specified");
@@ -96,30 +102,43 @@ size_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
             // @todo: consider throwing exception here.
             return (offset);
         }
+
+        // Get the list of stdandard option definitions.
+        OptionDefContainer option_defs = LibDHCP::getOptionDefs(Option::V6);
+        // Get the search index #1. It allows to search for option definitions
+        // using option code.
+        const OptionDefContainerTypeIndex& idx = option_defs.get<1>();
+        // Get all options with the particular option code. Note that option code
+        // is non-unique within this container however at this point we expect
+        // to get one option definition with the particular code. If more are
+        // returned we report an error.
+        const OptionDefContainerTypeRange& range = idx.equal_range(opt_type);
+        // Get the number of returned option definitions for the option code.
+        size_t num_defs = distance(range.first, range.second);
         OptionPtr opt;
-        switch (opt_type) {
-        case D6O_IA_NA:
-        case D6O_IA_PD:
-            opt = OptionPtr(new Option6IA(opt_type,
-                                          buf.begin() + offset,
-                                          buf.begin() + offset + opt_len));
-            break;
-        case D6O_IAADDR:
-            opt = OptionPtr(new Option6IAAddr(opt_type,
-                                              buf.begin() + offset,
-                                              buf.begin() + offset + opt_len));
-            break;
-        case D6O_ORO:
-            opt = OptionPtr(new Option6IntArray<uint16_t>(opt_type,
-                                                          buf.begin() + offset,
-                                                          buf.begin() + offset + opt_len));
-            break;
-        default:
-            opt = OptionPtr(new Option(Option::V6,
-                                       opt_type,
+        if (num_defs > 1) {
+            // Multiple options of the same code are not supported right now!
+            isc_throw(isc::Unexpected, "Internal error: multiple option definitions"
+                      " for option type " << opt_type << " returned. Currently it is not"
+                      " supported to initialize multiple option definitions"
+                      " for the same option code. This will be supported once"
+                      " support for option spaces is implemented");
+        } else if (num_defs == 0) {
+            // Don't crash if definition does not exist because only a few
+            // option definitions are initialized right now. In the future
+            // we will initialize definitions for all options and we will
+            // remove this elseif. For now, return generic option.
+            opt = OptionPtr(new Option(Option::V6, opt_type,
                                        buf.begin() + offset,
                                        buf.begin() + offset + opt_len));
-            break;
+        } else {
+            // The option definition has been found. Use it to create
+            // the option instance from the provided buffer chunk.
+            const OptionDefinitionPtr& def = *(range.first);
+            assert(def);
+            opt = def->optionFactory(Option::V6, opt_type,
+                                     buf.begin() + offset,
+                                     buf.begin() + offset + opt_len);
         }
         // add option to options
         options.insert(std::make_pair(opt_type, opt));
@@ -266,7 +285,8 @@ LibDHCP::initStdOptionDefs6() {
         { "ELAPSED_TIME", D6O_ELAPSED_TIME, OptionDefinition::UINT16_TYPE, false },
         { "STATUS_CODE", D6O_STATUS_CODE, OptionDefinition::RECORD_TYPE, false },
         { "RAPID_COMMIT", D6O_RAPID_COMMIT, OptionDefinition::EMPTY_TYPE, false },
-        { "DNS_SERVERS", D6O_NAME_SERVERS, OptionDefinition::IPV6_ADDRESS_TYPE, true }
+        { "DNS_SERVERS", D6O_NAME_SERVERS, OptionDefinition::IPV6_ADDRESS_TYPE, true },
+        { "IA_PD", D6O_IA_PD, OptionDefinition::RECORD_TYPE, false }
     };
     const int params_size = sizeof(params) / sizeof(params[0]);
 
@@ -277,6 +297,7 @@ LibDHCP::initStdOptionDefs6() {
                                                             params[i].array));
         switch(params[i].code) {
         case D6O_IA_NA:
+        case D6O_IA_PD:
             for (int j = 0; j < 3; ++j) {
                 definition->addRecordField(OptionDefinition::UINT32_TYPE);
             }
diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h
index c773fd7..4db753c 100644
--- a/src/lib/dhcp/libdhcp++.h
+++ b/src/lib/dhcp/libdhcp++.h
@@ -34,6 +34,10 @@ public:
     /// @brief Return collection of option definitions.
     ///
     /// @param u universe of the options (V4 or V6).
+    ///
+    /// @throw isc::InvalidOperation if option definitions for a specified
+    /// universe have not been initialized yet. initStdOptionDefs must
+    /// be first called to avoid this error.
     /// @return collection of option definitions.
     static const OptionDefContainer& getOptionDefs(Option::Universe u);
 
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index 2740141..c85e60a 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -213,12 +213,8 @@ OptionDefinition::factoryAddrList6(Option::Universe u, uint16_t type,
 
 OptionPtr
 OptionDefinition::factoryEmpty(Option::Universe u, uint16_t type,
-                               OptionBufferConstIter begin,
-                               OptionBufferConstIter end) {
-    if (std::distance(begin, end) > 0) {
-        isc_throw(isc::BadValue, "input option buffer must be empty"
-                  " when creating empty option instance");
-    }
+                               OptionBufferConstIter,
+                               OptionBufferConstIter) {
     OptionPtr option(new Option(u, type));
     return (option);
 }
@@ -236,9 +232,9 @@ OptionDefinition::factoryIA6(Option::Universe u, uint16_t type,
                              OptionBufferConstIter begin,
                              OptionBufferConstIter end) {
     sanityCheckUniverse(u, Option::V6);
-    if (std::distance(begin, end) != Option6IA::OPTION6_IA_LEN) {
-        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expeted "
-                  << Option6IA::OPTION6_IA_LEN << " bytes");
+    if (std::distance(begin, end) < Option6IA::OPTION6_IA_LEN) {
+        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expected "
+                  "at least " << Option6IA::OPTION6_IA_LEN << " bytes");
     }
     boost::shared_ptr<Option6IA> option(new Option6IA(type, begin, end));
     return (option);
@@ -249,9 +245,9 @@ OptionDefinition::factoryIAAddr6(Option::Universe u, uint16_t type,
                                  OptionBufferConstIter begin,
                                  OptionBufferConstIter end) {
     sanityCheckUniverse(u, Option::V6);
-    if (std::distance(begin, end) != Option6IAAddr::OPTION6_IAADDR_LEN) {
-        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expeted "
-                  << Option6IAAddr::OPTION6_IAADDR_LEN << " bytes");
+    if (std::distance(begin, end) < Option6IAAddr::OPTION6_IAADDR_LEN) {
+        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expected "
+                  " at least " << Option6IAAddr::OPTION6_IAADDR_LEN << " bytes");
     }
     boost::shared_ptr<Option6IAAddr> option(new Option6IAAddr(type, begin, end));
     return (option);
diff --git a/src/lib/dhcp/option_definition.h b/src/lib/dhcp/option_definition.h
index b2ecbaf..9fa2e41 100644
--- a/src/lib/dhcp/option_definition.h
+++ b/src/lib/dhcp/option_definition.h
@@ -372,10 +372,6 @@ public:
     static OptionPtr factoryInteger(Option::Universe, uint16_t type,
                                     OptionBufferConstIter begin,
                                     OptionBufferConstIter end) {
-        if (std::distance(begin, end) > sizeof(T)) {
-            isc_throw(isc::OutOfRange, "provided option buffer is too large, expected: "
-                      << sizeof(T) << " bytes");
-        }
         OptionPtr option(new Option6Int<T>(type, begin, end));
         return (option);
     }
@@ -391,12 +387,6 @@ public:
     static OptionPtr factoryIntegerArray(Option::Universe, uint16_t type,
                                          OptionBufferConstIter begin,
                                          OptionBufferConstIter end) {
-        if (std::distance(begin, end) == 0) {
-            isc_throw(isc::OutOfRange, "option buffer length must be greater than zero");
-        } else if (std::distance(begin, end) % OptionDataTypes<T>::len != 0) {
-            isc_throw(isc::OutOfRange, "option buffer length must be multiple of "
-                      << OptionDataTypes<T>::len << " bytes");
-        }
         OptionPtr option(new Option6IntArray<T>(type, begin, end));
         return (option);
     }
diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc
index ba414e0..0a6c949 100644
--- a/src/lib/dhcp/tests/libdhcp++_unittest.cc
+++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc
@@ -40,6 +40,10 @@ namespace {
 class LibDhcpTest : public ::testing::Test {
 public:
     LibDhcpTest() {
+        // @todo initialize standard DHCPv4 option definitions
+
+        // Initialize DHCPv6 option definitions.
+        LibDHCP::initStdOptionDefs(Option::V6);
     }
 
     /// @brief Generic factory function to create any option.
@@ -67,11 +71,10 @@ public:
     static void testInitOptionDefs6(const uint16_t code,
                              const OptionBuffer& buf,
                              const std::type_info& expected_type) {
-        // Initialize stdandard options definitions. They are held
-        // in the static container throughout the program.
-        LibDHCP::initStdOptionDefs(Option::V6);
         // Get all option definitions, we will use them to extract
         // the definition for a particular option code.
+        // We don't have to initialize option deinitions here because they
+        // are initialized in the class'es constructor.
         OptionDefContainer options = LibDHCP::getOptionDefs(Option::V6);
         // Get the container index #1. This one allows for searching
         // option definitions using option code.
@@ -103,14 +106,14 @@ public:
 };
 
 static const uint8_t packed[] = {
-    0, 12, 0, 5, 100, 101, 102, 103, 104, // opt1 (9 bytes)
-    0, 13, 0, 3, 105, 106, 107, // opt2 (7 bytes)
-    0, 14, 0, 2, 108, 109, // opt3 (6 bytes)
-    1,  0, 0, 4, 110, 111, 112, 113, // opt4 (8 bytes)
-    1,  1, 0, 1, 114 // opt5 (5 bytes)
+    0, 1, 0, 5, 100, 101, 102, 103, 104, // CLIENT_ID (9 bytes)
+    0, 2, 0, 3, 105, 106, 107, // SERVER_ID (7 bytes)
+    0, 14, 0, 0, // RAPID_COMMIT (0 bytes)
+    0,  6, 0, 4, 108, 109, 110, 111, // ORO (8 bytes)
+    0,  8, 0, 2, 112, 113 // ELAPSED_TIME (6 bytes)
 };
 
-TEST(LibDhcpTest, optionFactory) {
+TEST_F(LibDhcpTest, optionFactory) {
     OptionBuffer buf;
     // Factory functions for specific options must be registered before
     // they can be used to create options instances. Otherwise exception
@@ -182,7 +185,7 @@ TEST(LibDhcpTest, optionFactory) {
                            opt_clientid->getData().begin()));
 }
 
-TEST(LibDhcpTest, packOptions6) {
+TEST_F(LibDhcpTest, packOptions6) {
     OptionBuffer buf(512);
     isc::dhcp::Option::OptionCollection opts; // list of options
 
@@ -191,11 +194,11 @@ TEST(LibDhcpTest, packOptions6) {
         buf[i]=i+100;
     }
 
-    OptionPtr opt1(new Option(Option::V6, 12, buf.begin() + 0, buf.begin() + 5));
-    OptionPtr opt2(new Option(Option::V6, 13, buf.begin() + 5, buf.begin() + 8));
-    OptionPtr opt3(new Option(Option::V6, 14, buf.begin() + 8, buf.begin() + 10));
-    OptionPtr opt4(new Option(Option::V6,256, buf.begin() + 10,buf.begin() + 14));
-    OptionPtr opt5(new Option(Option::V6,257, buf.begin() + 14,buf.begin() + 15));
+    OptionPtr opt1(new Option(Option::V6, 1, buf.begin() + 0, buf.begin() + 5));
+    OptionPtr opt2(new Option(Option::V6, 2, buf.begin() + 5, buf.begin() + 8));
+    OptionPtr opt3(new Option(Option::V6, 14, buf.begin() + 8, buf.begin() + 8));
+    OptionPtr opt4(new Option(Option::V6, 6, buf.begin() + 8, buf.begin() + 12));
+    OptionPtr opt5(new Option(Option::V6, 8, buf.begin() + 12, buf.begin() + 14));
 
     opts.insert(pair<int, OptionPtr >(opt1->getType(), opt1));
     opts.insert(pair<int, OptionPtr >(opt1->getType(), opt2));
@@ -206,11 +209,11 @@ TEST(LibDhcpTest, packOptions6) {
     OutputBuffer assembled(512);
 
     EXPECT_NO_THROW(LibDHCP::packOptions6(assembled, opts));
-    EXPECT_EQ(35, assembled.getLength()); // options should take 35 bytes
-    EXPECT_EQ(0, memcmp(assembled.getData(), packed, 35) );
+    EXPECT_EQ(sizeof(packed), assembled.getLength());
+    EXPECT_EQ(0, memcmp(assembled.getData(), packed, sizeof(packed)));
 }
 
-TEST(LibDhcpTest, unpackOptions6) {
+TEST_F(LibDhcpTest, unpackOptions6) {
 
     // just couple of random options
     // Option is used as a simple option implementation
@@ -219,55 +222,85 @@ TEST(LibDhcpTest, unpackOptions6) {
     isc::dhcp::Option::OptionCollection options; // list of options
 
     OptionBuffer buf(512);
-    memcpy(&buf[0], packed, 35);
+    memcpy(&buf[0], packed, sizeof(packed));
 
     EXPECT_NO_THROW ({
-        LibDHCP::unpackOptions6(OptionBuffer(buf.begin(), buf.begin()+35), options);
+            LibDHCP::unpackOptions6(OptionBuffer(buf.begin(), buf.begin() + sizeof(packed)),
+                                    options);
     });
 
     EXPECT_EQ(options.size(), 5); // there should be 5 options
 
-    isc::dhcp::Option::OptionCollection::const_iterator x = options.find(12);
+    isc::dhcp::Option::OptionCollection::const_iterator x = options.find(1);
     ASSERT_FALSE(x == options.end()); // option 1 should exist
-    EXPECT_EQ(12, x->second->getType());  // this should be option 12
+    EXPECT_EQ(1, x->second->getType());  // this should be option 1
     ASSERT_EQ(9, x->second->len()); // it should be of length 9
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+4, 5)); // data len=5
+    ASSERT_EQ(5, x->second->getData().size());
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed + 4, 5)); // data len=5
 
-    x = options.find(13);
-    ASSERT_FALSE(x == options.end()); // option 13 should exist
-    EXPECT_EQ(13, x->second->getType());  // this should be option 13
+        x = options.find(2);
+    ASSERT_FALSE(x == options.end()); // option 2 should exist
+    EXPECT_EQ(2, x->second->getType());  // this should be option 2
     ASSERT_EQ(7, x->second->len()); // it should be of length 7
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+13, 3)); // data len=3
+    ASSERT_EQ(3, x->second->getData().size());
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed + 13, 3)); // data len=3
 
     x = options.find(14);
-    ASSERT_FALSE(x == options.end()); // option 3 should exist
+    ASSERT_FALSE(x == options.end()); // option 14 should exist
     EXPECT_EQ(14, x->second->getType());  // this should be option 14
-    ASSERT_EQ(6, x->second->len()); // it should be of length 6
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+20, 2)); // data len=2
-
-    x = options.find(256);
-    ASSERT_FALSE(x == options.end()); // option 256 should exist
-    EXPECT_EQ(256, x->second->getType());  // this should be option 256
-    ASSERT_EQ(8, x->second->len()); // it should be of length 7
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+26, 4)); // data len=4
-
-    x = options.find(257);
-    ASSERT_FALSE(x == options.end()); // option 257 should exist
-    EXPECT_EQ(257, x->second->getType());  // this should be option 257
-    ASSERT_EQ(5, x->second->len()); // it should be of length 5
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+34, 1)); // data len=1
+    ASSERT_EQ(4, x->second->len()); // it should be of length 4
+    EXPECT_EQ(0, x->second->getData().size()); // data len = 0
+
+    x = options.find(6);
+    ASSERT_FALSE(x == options.end()); // option 6 should exist
+    EXPECT_EQ(6, x->second->getType());  // this should be option 6
+    ASSERT_EQ(8, x->second->len()); // it should be of length 8
+    // Option with code 6 is the OPTION_ORO. This option is
+    // represented by the Option6IntArray<uint16_t> class which
+    // comprises the set of uint16_t values. We need to cast the
+    // returned pointer to this type to get values stored in it.
+    boost::shared_ptr<Option6IntArray<uint16_t> > opt_oro =
+        boost::dynamic_pointer_cast<Option6IntArray<uint16_t> >(x->second);
+    // This value will be NULL if cast was unsuccessful. This is the case
+    // when returned option has different type than expected.
+    ASSERT_TRUE(opt_oro);
+    // Get set of uint16_t values.
+    std::vector<uint16_t> opts = opt_oro->getValues();
+    // Prepare the refrence data.
+    std::vector<uint16_t> expected_opts;
+    expected_opts.push_back(0x6C6D); // equivalent to: 108, 109
+    expected_opts.push_back(0x6E6F); // equivalent to 110, 111
+    ASSERT_EQ(expected_opts.size(), opts.size());
+    // Validated if option has been unpacked correctly.
+    EXPECT_TRUE(std::equal(expected_opts.begin(), expected_opts.end(),
+                           opts.begin()));
+
+    x = options.find(8);
+    ASSERT_FALSE(x == options.end()); // option 8 should exist
+    EXPECT_EQ(8, x->second->getType());  // this should be option 8
+    ASSERT_EQ(6, x->second->len()); // it should be of length 9
+    // Option with code 8 is OPTION_ELAPSED_TIME. This option is
+    // represented by Option6Int<uint16_t> value that holds single
+    // uint16_t value.
+    boost::shared_ptr<Option6Int<uint16_t> > opt_elapsed_time =
+        boost::dynamic_pointer_cast<Option6Int<uint16_t> >(x->second);
+    // This value will be NULL if cast was unsuccessful. This is the case
+    // when returned option has different type than expected.
+    ASSERT_TRUE(opt_elapsed_time);
+    // Returned value should be equivalent to two byte values: 112, 113
+    EXPECT_EQ(0x7071, opt_elapsed_time->getValue());
 
     x = options.find(0);
     EXPECT_TRUE(x == options.end()); // option 0 not found
 
-    x = options.find(1); // 1 is htons(256) on little endians. Worth checking
+    x = options.find(256); // 256 is htons(1) on little endians. Worth checking
     EXPECT_TRUE(x == options.end()); // option 1 not found
 
-    x = options.find(2);
+    x = options.find(7);
     EXPECT_TRUE(x == options.end()); // option 2 not found
 
     x = options.find(32000);
-    EXPECT_TRUE(x == options.end()); // option 32000 not found
+    EXPECT_TRUE(x == options.end()); // option 32000 not found */
 }
 
 
@@ -279,7 +312,7 @@ static uint8_t v4Opts[] = {
     128, 3, 40, 41, 42
 };
 
-TEST(LibDhcpTest, packOptions4) {
+TEST_F(LibDhcpTest, packOptions4) {
 
     vector<uint8_t> payload[5];
     for (int i = 0; i < 5; i++) {
@@ -311,7 +344,7 @@ TEST(LibDhcpTest, packOptions4) {
 
 }
 
-TEST(LibDhcpTest, unpackOptions4) {
+TEST_F(LibDhcpTest, unpackOptions4) {
 
     vector<uint8_t> packed(v4Opts, v4Opts + sizeof(v4Opts));
     isc::dhcp::Option::OptionCollection options; // list of options
@@ -370,7 +403,7 @@ TEST(LibDhcpTest, unpackOptions4) {
 // @todo Only limited number of option definitions are now created
 // This test have to be extended once all option definitions are
 // created.
-TEST(LibDhcpTest, initStdOptionDefs) {
+TEST_F(LibDhcpTest, initStdOptionDefs) {
     LibDhcpTest::testInitOptionDefs6(D6O_CLIENTID, OptionBuffer(14, 1),
                                      typeid(Option));
     LibDhcpTest::testInitOptionDefs6(D6O_SERVERID, OptionBuffer(14, 1),
diff --git a/src/lib/dhcp/tests/option6_ia_unittest.cc b/src/lib/dhcp/tests/option6_ia_unittest.cc
index bc72242..1ee618a 100644
--- a/src/lib/dhcp/tests/option6_ia_unittest.cc
+++ b/src/lib/dhcp/tests/option6_ia_unittest.cc
@@ -206,7 +206,7 @@ TEST_F(Option6IATest, suboptions_unpack) {
 
     Option6IA* ia = 0;
     EXPECT_NO_THROW({
-            ia = new Option6IA(D6O_IA_NA, buf_.begin() + 4, buf_.begin() + sizeof(expected));
+        ia = new Option6IA(D6O_IA_NA, buf_.begin() + 4, buf_.begin() + sizeof(expected));
     });
     ASSERT_TRUE(ia);
 
diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc
index be51a1b..0ca08ce 100644
--- a/src/lib/dhcp/tests/option_definition_unittest.cc
+++ b/src/lib/dhcp/tests/option_definition_unittest.cc
@@ -258,21 +258,12 @@ TEST_F(OptionDefinitionTest, factoryEmpty) {
     EXPECT_EQ(0, option_v6->getData().size());
 
     // Repeat the same test scenario for DHCPv4 option.
-    EXPECT_THROW(opt_def.optionFactory(Option::V4, 214, OptionBuffer(2)),
-                 isc::BadValue);
-
     OptionPtr option_v4;
     ASSERT_NO_THROW(option_v4 = opt_def.optionFactory(Option::V4, 214, OptionBuffer()));
     // Expect 'empty' DHCPv4 option.
     EXPECT_EQ(Option::V4, option_v4->getUniverse());
     EXPECT_EQ(2, option_v4->getHeaderLen());
     EXPECT_EQ(0, option_v4->getData().size());
-
-    // This factory produces empty option (consisting of option type
-    // and length). Attempt to provide some data in the buffer should
-    // result in exception.
-    EXPECT_THROW(opt_def.optionFactory(Option::V6, D6O_RAPID_COMMIT, OptionBuffer(2)),
-                 isc::BadValue);
 }
 
 TEST_F(OptionDefinitionTest, factoryBinary) {
@@ -352,17 +343,12 @@ TEST_F(OptionDefinitionTest, factoryIA6) {
         opt_def.optionFactory(Option::V4, D6O_IA_NA, OptionBuffer(option6_ia_len)),
         isc::BadValue
     );
-    // The length of the buffer must be 12 bytes.
+    // The length of the buffer must be at least 12 bytes.
     // Check too short buffer.
     EXPECT_THROW(
         opt_def.optionFactory(Option::V6, D6O_IA_NA, OptionBuffer(option6_ia_len - 1)),
         isc::OutOfRange
      );
-    // Check too long buffer.
-    EXPECT_THROW(
-        opt_def.optionFactory(Option::V6, D6O_IA_NA, OptionBuffer(option6_ia_len + 1)),
-        isc::OutOfRange
-    );
 }
 
 TEST_F(OptionDefinitionTest, factoryIAAddr6) {
@@ -401,17 +387,12 @@ TEST_F(OptionDefinitionTest, factoryIAAddr6) {
         opt_def.optionFactory(Option::V4, D6O_IAADDR, OptionBuffer(option6_iaaddr_len)),
         isc::BadValue
     );
-    // The length of the buffer must be 12 bytes.
+    // The length of the buffer must be at least 12 bytes.
     // Check too short buffer.
     EXPECT_THROW(
         opt_def.optionFactory(Option::V6, D6O_IAADDR, OptionBuffer(option6_iaaddr_len - 1)),
         isc::OutOfRange
      );
-    // Check too long buffer.
-    EXPECT_THROW(
-        opt_def.optionFactory(Option::V6, D6O_IAADDR, OptionBuffer(option6_iaaddr_len + 1)),
-        isc::OutOfRange
-    );
 }
 
 TEST_F(OptionDefinitionTest, factoryIntegerInvalidType) {
@@ -440,12 +421,6 @@ TEST_F(OptionDefinitionTest, factoryUint8) {
         boost::static_pointer_cast<Option6Int<uint8_t> >(option_v6);
     EXPECT_EQ(1, option_cast_v6->getValue());
 
-    // Try to provide too large buffer. Expect exception.
-    EXPECT_THROW(
-        option_v6 = opt_def.optionFactory(Option::V6, D6O_PREFERENCE, OptionBuffer(3)),
-        isc::OutOfRange
-    );
-
     // Try to provide zero-length buffer. Expect exception.
     EXPECT_THROW(
         option_v6 = opt_def.optionFactory(Option::V6, D6O_PREFERENCE, OptionBuffer()),
@@ -472,11 +447,6 @@ TEST_F(OptionDefinitionTest, factoryUint16) {
         boost::static_pointer_cast<Option6Int<uint16_t> >(option_v6);
     EXPECT_EQ(0x0102, option_cast_v6->getValue());
 
-    // Try to provide too large buffer. Expect exception.
-    EXPECT_THROW(
-        option_v6 = opt_def.optionFactory(Option::V6, D6O_ELAPSED_TIME, OptionBuffer(3)),
-        isc::OutOfRange
-    );
     // Try to provide zero-length buffer. Expect exception.
     EXPECT_THROW(
         option_v6 = opt_def.optionFactory(Option::V6, D6O_ELAPSED_TIME, OptionBuffer(1)),
@@ -504,12 +474,7 @@ TEST_F(OptionDefinitionTest, factoryUint32) {
         boost::static_pointer_cast<Option6Int<uint32_t> >(option_v6);
     EXPECT_EQ(0x01020304, option_cast_v6->getValue());
 
-    // Try to provide too large buffer. Expect exception.
-    EXPECT_THROW(
-        option_v6 = opt_def.optionFactory(Option::V6, D6O_CLT_TIME, OptionBuffer(5)),
-        isc::OutOfRange
-    );
-    // Try to provide zero-length buffer. Expect exception.
+    // Try to provide too short buffer. Expect exception.
     EXPECT_THROW(
         option_v6 = opt_def.optionFactory(Option::V6, D6O_CLT_TIME, OptionBuffer(2)),
         isc::OutOfRange



More information about the bind10-changes mailing list