[#2536] Implementing DNRv4 Option with TDD

diff --git a/src/lib/dhcp/option4_dnr.cc b/src/lib/dhcp/option4_dnr.cc
index f517273d05bd2d801d1006bd2237c8b10f16afeb..ce59be639e4027d6d511ac8a5b43cd332895bb19 100644 (file)
--- a/src/lib/dhcp/option4_dnr.cc
+++ b/src/lib/dhcp/option4_dnr.cc
@@ -62,9 +62,10 @@ Option4Dnr::unpack(OptionBufferConstIter begin, OptionBufferConstIter end) {
         }
         dnr_instance.setDnrInstanceDataLength(
             readUint16(&(*(begin + offset)), dnr_instance.getDnrInstanceDataLengthSize()));
+        offset += dnr_instance.getDnrInstanceDataLengthSize();
         OptionBufferConstIter dnr_instance_end = begin + offset +
                                                  dnr_instance.getDnrInstanceDataLength();
-        offset += dnr_instance.getDnrInstanceDataLengthSize();
+
         dnr_instance.setServicePriority(
             readUint16(&(*(begin + offset)), dnr_instance.SERVICE_PRIORITY_SIZE));
         offset += dnr_instance.SERVICE_PRIORITY_SIZE;
@@ -81,15 +82,7 @@ Option4Dnr::unpack(OptionBufferConstIter begin, OptionBufferConstIter end) {
         }
 
         // Let's try to extract ADN FQDN data.
-        InputBuffer name_buf(&(*adn_tuple.getData().begin()), adn_length);
-        try {
-            auto adn = dnr_instance.getAdn();
-            adn.reset(new isc::dns::Name(name_buf, true));
-        } catch (const Exception& ex) {
-            isc_throw(InvalidOptionDnrDomainName, "failed to parse "
-                                                  "fully qualified domain-name from wire format "
-                                                  "- " << ex.what());
-        }
+        dnr_instance.unpackAdn(adn_tuple.getData().begin(), adn_length);
 
         offset += adn_tuple.getTotalLength();
 
@@ -125,17 +118,17 @@ Option4Dnr::unpack(OptionBufferConstIter begin, OptionBufferConstIter end) {
         offset += dnr_instance.getAddrLengthSize();
         OptionBufferConstIter addr_begin = begin + offset;
         OptionBufferConstIter addr_end = addr_begin + addr_length;
-        auto ip_addresses = dnr_instance.getIpAddresses();
 
         while (addr_begin != addr_end) {
             const uint8_t* ptr = &(*addr_begin);
-            ip_addresses.push_back(IOAddress(readUint32(ptr, std::distance(addr_begin, addr_end))));
+            dnr_instance.addIpAddress(IOAddress(readUint32(ptr, std::distance(addr_begin, addr_end))));
             addr_begin += V4ADDRESS_LEN;
             offset += V4ADDRESS_LEN;
         }
 
         // SvcParams (variable length) field is last.
         auto svc_params_length = std::distance(begin + offset, dnr_instance_end);
+        dnr_instance.setSvcParamsLength(svc_params_length);
         if (svc_params_length > 0) {
             std::string svc_params;
             svc_params.assign(begin + offset, dnr_instance_end);
@@ -264,6 +257,18 @@ DnrInstance::setAdn(const std::string& adn) {
     }
 }
 
+void
+DnrInstance::unpackAdn(OptionBufferConstIter begin, uint16_t adn_len) {
+    InputBuffer name_buf(&*begin, adn_len);
+    try {
+        adn_.reset(new isc::dns::Name(name_buf, true));
+    } catch (const Exception& ex) {
+        isc_throw(InvalidOptionDnrDomainName, "Failed to parse "
+                                              "fully qualified domain-name from wire format "
+                                              "- " << ex.what());
+    }
+}
+
 void
 DnrInstance::checkSvcParams(bool from_wire_data) {
     std::string svc_params = isc::util::str::trim(svc_params_);
@@ -442,5 +447,10 @@ DnrInstance::getMinimalLength() const {
     return (getDnrInstanceDataLengthSize() + SERVICE_PRIORITY_SIZE + getAdnLengthSize());
 }
 
+void
+DnrInstance::addIpAddress(const IOAddress& ip_address) {
+    ip_addresses_.push_back(ip_address);
+}
+
 }  // namespace dhcp
 }  // namespace isc

diff --git a/src/lib/dhcp/option4_dnr.h b/src/lib/dhcp/option4_dnr.h
index 6f79a07d56821368e95d30fdf4bd75176e36eb47..9c42d43b09b84442e0b2c97b787ad437a307dec9 100644 (file)
--- a/src/lib/dhcp/option4_dnr.h
+++ b/src/lib/dhcp/option4_dnr.h
@@ -69,20 +69,6 @@ public:
     /// @brief Default destructor.
     virtual ~DnrInstance() = default;
 
-    /// @brief Getter of the @c ip_addresses_.
-    ///
-    /// @return Vector container holding one or more IP addresses.
-    const AddressContainer& getIpAddresses() const {
-        return ip_addresses_;
-    }
-
-    /// @brief Getter of the @c adn_.
-    ///
-    /// @return Authentication domain name field of variable length.
-    const boost::shared_ptr<isc::dns::Name>& getAdn() const {
-        return adn_;
-    }
-
     /// @brief Getter of the @c dnr_instance_data_length_.
     ///
     /// @return Length of all following data inside this DNR instance in octets.
@@ -218,6 +204,12 @@ public:
         svc_params_ = svc_params;
     }
 
+    /// @brief Setter of the @c svc_params_length_ field.
+    /// @param svc_params_length len to be set
+    void setSvcParamsLength(uint16_t svc_params_length) {
+        svc_params_length_ = svc_params_length;
+    }
+
     /// @brief Writes the ADN FQDN in the wire format into a buffer.
     ///
     /// The Authentication Domain Name - fully qualified domain name of the encrypted
@@ -242,6 +234,11 @@ public:
     /// @param [out] buf buffer where SvcParams will be written.
     void packSvcParams(isc::util::OutputBuffer& buf) const;
 
+    /// @brief Unpacks the ADN from given wire data buffer and stores it in @c adn_ field.
+    /// @param begin beginning of the buffer from which the ADN will be read
+    /// @param adn_len length of the ADN to be read
+    void unpackAdn(OptionBufferConstIter begin, uint16_t adn_len);
+
     /// @brief Checks SvcParams field if encoded correctly and throws in case of issue found.
     ///
     /// The field should be encoded following the rules in
@@ -253,6 +250,10 @@ public:
     /// Fields lengths are also calculated and saved to member variables.
     void checkFields();
 
+    /// @brief Adds IP address to @c ip_addresses_ container.
+    /// @param ip_address IP address to be added
+    void addIpAddress(const asiolink::IOAddress& ip_address);
+
 protected:
     /// @brief Either V4 or V6 Option universe.
     Option::Universe universe_;

diff --git a/src/lib/dhcp/option6_dnr.cc b/src/lib/dhcp/option6_dnr.cc
index 8dfbac510d7d0bf16dffd0643b337745dce7f90e..3e20d6bcdd638d2e6cf40bbeacb898a5e731553d 100644 (file)
--- a/src/lib/dhcp/option6_dnr.cc
+++ b/src/lib/dhcp/option6_dnr.cc
@@ -75,14 +75,7 @@ Option6Dnr::unpack(OptionBufferConstIter begin, OptionBufferConstIter end) {
     }
 
     // Let's try to extract ADN FQDN data.
-    isc::util::InputBuffer name_buf(&(*adn_tuple.getData().begin()), adn_length_);
-    try {
-        adn_.reset(new isc::dns::Name(name_buf, true));
-    } catch (const Exception& ex) {
-        isc_throw(InvalidOptionDnrDomainName, "failed to parse "
-                                              "fully qualified domain-name from wire format "
-                                              "- " << ex.what());
-    }
+    unpackAdn(adn_tuple.getData().begin(), adn_length_);
 
     begin += adn_tuple.getTotalLength();
 

diff --git a/src/lib/dhcp/tests/option4_dnr_unittest.cc b/src/lib/dhcp/tests/option4_dnr_unittest.cc
index c1615a25c3728aaf94dfdc7c260b7d338aa55daf..20e644f629407beff89ea5f0f77ef33703af0cbf 100644 (file)
--- a/src/lib/dhcp/tests/option4_dnr_unittest.cc
+++ b/src/lib/dhcp/tests/option4_dnr_unittest.cc
@@ -17,6 +17,7 @@
 
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::asiolink;
 using boost::scoped_ptr;
 
 namespace {
@@ -128,6 +129,63 @@ TEST(Option4DnrTest, multipleDnrOnlyModeInstances) {
               option->toText());
 }
 
+TEST(Option4DnrTest, mixedDnrInstances) {
+    // Create option instance. Check that constructor doesn't throw.
+    scoped_ptr<Option4Dnr> option;
+    EXPECT_NO_THROW(option.reset(new Option4Dnr()));
+    ASSERT_TRUE(option);
+
+    // Prepare example DNR instance to add.
+    DnrInstance::AddressContainer addresses;
+    addresses.push_back(IOAddress("192.168.0.1"));
+    addresses.push_back(IOAddress("192.168.0.2"));
+    std::string svc_params = "key123=val key234=val2 key345";
+
+    DnrInstance dnr_1 = DnrInstance(Option::V4, 1, "myhost1.example.com.");
+    DnrInstance dnr_2 = DnrInstance(Option::V4, 2, "myhost2.example.com.", addresses, svc_params);
+
+    // Add DNR instance.
+    option->addDnrInstance(dnr_1);
+    option->addDnrInstance(dnr_2);
+
+    // Check if member variables were correctly set inside DNR instances.
+    EXPECT_EQ(2, option->getDnrInstances().size());
+    EXPECT_EQ(1, option->getDnrInstances()[0].getServicePriority());
+    EXPECT_EQ(21, option->getDnrInstances()[0].getAdnLength());
+    EXPECT_EQ("myhost1.example.com.", option->getDnrInstances()[0].getAdnAsText());
+    EXPECT_EQ(2, option->getDnrInstances()[1].getServicePriority());
+    EXPECT_EQ(21, option->getDnrInstances()[1].getAdnLength());
+    EXPECT_EQ("myhost2.example.com.", option->getDnrInstances()[1].getAdnAsText());
+
+    EXPECT_EQ(0, option->getDnrInstances()[0].getAddrLength());
+    EXPECT_EQ(0, option->getDnrInstances()[0].getSvcParamsLength());
+    EXPECT_EQ(2, option->getDnrInstances()[1].getAddresses().size());
+    EXPECT_EQ(8, option->getDnrInstances()[1].getAddrLength());
+    EXPECT_EQ(29, option->getDnrInstances()[1].getSvcParamsLength());
+    EXPECT_EQ("192.168.0.1", option->getDnrInstances()[1].getAddresses()[0].toText());
+    EXPECT_EQ("192.168.0.2", option->getDnrInstances()[1].getAddresses()[1].toText());
+    EXPECT_EQ(svc_params, option->getDnrInstances()[1].getSvcParams());
+
+    // BTW let's check if len() works ok. In ADN only mode, DNR Instance Data Len
+    // is set to ADN Len (21) + 3 = 24.
+    // expected len: 1x(24 (ADN+ADN Len+Service priority) + 2 (DNR Instance Data Len)) + 2 (headers)
+    // + 24 (ADN+ADN Len+Service priority) + 2 (DNR Instance Data Len) + 1 (Addr Len)
+    // + 8 (IP addresses) + 29 (svc params)
+    // = 92
+    EXPECT_EQ(92, option->len());
+
+    // BTW let's check if toText() works ok.
+    // toText() len does not count in headers len.
+    EXPECT_EQ("type=162(V4_DNR), len=90, "
+              "DNR Instance 1(Instance len=24, service_priority=1, "
+              "adn_length=21, adn='myhost1.example.com.'), "
+              "DNR Instance 2(Instance len=62, service_priority=2, "
+              "adn_length=21, adn='myhost2.example.com.', "
+              "addr_length=8, address(es): 192.168.0.1 192.168.0.2, "
+              "svc_params='key123=val key234=val2 key345')",
+              option->toText());
+}
+
 TEST(Option4DnrTest, packOneDnrOnlyModeInstance) {
     // Create option instance. Check that constructor doesn't throw.
     scoped_ptr<Option4Dnr> option;
@@ -201,13 +259,13 @@ TEST(Option4DnrTest, packMultipleDnrOnlyModeInstances) {
         0x00,       24,                                        // DNR Instance Data Len
         0x00,       0x02,                                      // Service priority is 2 dec
         21,                                                    // ADN Length is 21 dec
-        0x07,       0x6D, 0x79, 0x68, 0x6F, 0x73, 0x74, '2',   // FQDN: myhost1.
+        0x07,       0x6D, 0x79, 0x68, 0x6F, 0x73, 0x74, '2',   // FQDN: myhost2.
         0x07,       0x65, 0x78, 0x61, 0x6D, 0x70, 0x6C, 0x65,  // example.
         0x03,       0x63, 0x6F, 0x6D, 0x00,                    // com.
         0x00,       24,                                        // DNR Instance Data Len
         0x00,       0x03,                                      // Service priority is 3 dec
         21,                                                    // ADN Length is 21 dec
-        0x07,       0x6D, 0x79, 0x68, 0x6F, 0x73, 0x74, '3',   // FQDN: myhost1.
+        0x07,       0x6D, 0x79, 0x68, 0x6F, 0x73, 0x74, '3',   // FQDN: myhost3.
         0x07,       0x65, 0x78, 0x61, 0x6D, 0x70, 0x6C, 0x65,  // example.
         0x03,       0x63, 0x6F, 0x6D, 0x00                     // com.
     };
@@ -220,4 +278,164 @@ TEST(Option4DnrTest, packMultipleDnrOnlyModeInstances) {
     EXPECT_EQ(0, memcmp(ref_data, buf.getData(), buf.getLength()));
 }
 
+TEST(Option4DnrTest, packMixedDnrInstances) {
+    // Create option instance. Check that constructor doesn't throw.
+    scoped_ptr<Option4Dnr> option;
+    EXPECT_NO_THROW(option.reset(new Option4Dnr()));
+    ASSERT_TRUE(option);
+
+    // Prepare example DNR instance to add.
+    DnrInstance::AddressContainer addresses;
+    addresses.push_back(IOAddress("192.168.0.1"));
+    addresses.push_back(IOAddress("192.168.0.2"));
+    std::string svc_params = "key123=val key234=val2 key345";
+
+    DnrInstance dnr_1 = DnrInstance(Option::V4, 1, "myhost1.example.com.");
+    DnrInstance dnr_2 = DnrInstance(Option::V4, 2, "myhost2.example.com.", addresses, svc_params);
+
+    // Add DNR instance.
+    option->addDnrInstance(dnr_1);
+    option->addDnrInstance(dnr_2);
+
+    // Prepare on-wire format of the option.
+    isc::util::OutputBuffer buf(10);
+    ASSERT_NO_THROW(option->pack(buf));
+
+    // Prepare reference data.
+    const uint8_t ref_data[] = {
+        DHO_V4_DNR,                                            // Option code
+        90,                                                    // Option len=90 dec
+        0x00,       24,                                        // DNR Instance Data Len
+        0x00,       0x01,                                      // Service priority is 1 dec
+        21,                                                    // ADN Length is 21 dec
+        0x07,       0x6D, 0x79, 0x68, 0x6F, 0x73, 0x74, '1',   // FQDN: myhost1.
+        0x07,       0x65, 0x78, 0x61, 0x6D, 0x70, 0x6C, 0x65,  // example.
+        0x03,       0x63, 0x6F, 0x6D, 0x00,                    // com.
+        0x00,       62,                                        // DNR Instance Data Len
+        0x00,       0x02,                                      // Service priority is 2 dec
+        21,                                                    // ADN Length is 21 dec
+        0x07,       0x6D, 0x79, 0x68, 0x6F, 0x73, 0x74, '2',   // FQDN: myhost2.
+        0x07,       0x65, 0x78, 0x61, 0x6D, 0x70, 0x6C, 0x65,  // example.
+        0x03,       0x63, 0x6F, 0x6D, 0x00,                    // com.
+        8,                                                     // Addr Len
+        192,        168,  0,    1,                             // IP address 1
+        192,        168,  0,    2,                             // IP address 2
+        'k',        'e',  'y',  '1',  '2',  '3',  '=',  'v',   // Svc Params
+        'a',        'l',  ' ',  'k',  'e',  'y',  '2',  '3',   // Svc Params
+        '4',        '=',  'v',  'a',  'l',  '2',  ' ',  'k',   // Svc Params
+        'e',        'y',  '3',  '4',  '5'                      // Svc Params
+    };
+
+    size_t ref_data_size = sizeof(ref_data) / sizeof(ref_data[0]);
+
+    // Check if the buffer has the same length as the reference data,
+    // so as they can be compared directly.
+    ASSERT_EQ(ref_data_size, buf.getLength());
+    EXPECT_EQ(0, memcmp(ref_data, buf.getData(), buf.getLength()));
+}
+
+TEST(Option4DnrTest, onWireDataCtor) {
+    // Prepare data to decode - ADN only mode 1 DNR instance.
+    const uint8_t buf_data[] = {
+        0x00, 24,                                        // DNR Instance Data Len
+        0x00, 0x01,                                      // Service priority is 1 dec
+        21,                                              // ADN Length is 21 dec
+        0x07, 0x6D, 0x79, 0x68, 0x6F, 0x73, 0x74, '1',   // FQDN: myhost1.
+        0x07, 0x65, 0x78, 0x61, 0x6D, 0x70, 0x6C, 0x65,  // example.
+        0x03, 0x63, 0x6F, 0x6D, 0x00                     // com.
+    };
+    OptionBuffer buf(buf_data, buf_data + sizeof(buf_data));
+    // Create option instance. Check that constructor doesn't throw.
+    scoped_ptr<Option4Dnr> option;
+    EXPECT_NO_THROW(option.reset(new Option4Dnr(buf.begin(), buf.end())));
+    ASSERT_TRUE(option);
+}
+
+TEST(Option4DnrTest, unpackOneAdnOnly) {
+    // Prepare data to decode - ADN only mode 1 DNR instance.
+    const uint8_t buf_data[] = {
+        0x00, 24,                                        // DNR Instance Data Len
+        0x00, 0x01,                                      // Service priority is 1 dec
+        21,                                              // ADN Length is 21 dec
+        0x07, 0x6D, 0x79, 0x68, 0x6F, 0x73, 0x74, '1',   // FQDN: myhost1.
+        0x07, 0x65, 0x78, 0x61, 0x6D, 0x70, 0x6C, 0x65,  // example.
+        0x03, 0x63, 0x6F, 0x6D, 0x00                     // com.
+    };
+    OptionBuffer buf(buf_data, buf_data + sizeof(buf_data));
+    // Create option instance. Check that constructor doesn't throw.
+    scoped_ptr<Option4Dnr> option;
+    EXPECT_NO_THROW(option.reset(new Option4Dnr(buf.begin(), buf.end())));
+    ASSERT_TRUE(option);
+
+    // Check if member variables were correctly set by ctor.
+    EXPECT_EQ(Option::V4, option->getUniverse());
+    EXPECT_EQ(DHO_V4_DNR, option->getType());
+
+    // Check if data was unpacked correctly from wire data.
+    EXPECT_EQ(24, option->getDnrInstances()[0].getDnrInstanceDataLength());
+    EXPECT_EQ(1, option->getDnrInstances()[0].getServicePriority());
+    EXPECT_EQ(21, option->getDnrInstances()[0].getAdnLength());
+    EXPECT_EQ("myhost1.example.com.", option->getDnrInstances()[0].getAdnAsText());
+
+    // This is ADN only mode, so Addr Length and SvcParams Length
+    // are both expected to be zero.
+    EXPECT_EQ(0, option->getDnrInstances()[0].getAddrLength());
+    EXPECT_EQ(0, option->getDnrInstances()[0].getSvcParamsLength());
+
+    // BTW let's check if len() works ok. In ADN only mode, DNR Instance Data Len
+    // is set to ADN Len (21) + 3 = 24.
+    // expected len: 1x(24 (ADN+ADN Len+Service priority) + 2 (DNR Instance Data Len)) + 2 (headers)
+    // = 28
+    EXPECT_EQ(28, option->len());
+
+    // BTW let's check if toText() works ok.
+    // toText() len does not count in headers len.
+    EXPECT_EQ("type=162(V4_DNR), len=26, "
+              "DNR Instance 1(Instance len=24, service_priority=1, "
+              "adn_length=21, adn='myhost1.example.com.')",
+              option->toText());
+}
+
+TEST(Option4DnrTest, unpackOneDnrInstance) {
+    // Prepare data to decode - 1 DNR instance.
+    const uint8_t buf_data[] = {
+        0x00, 62,                                        // DNR Instance Data Len
+        0x00, 0x01,                                      // Service priority is 1 dec
+        21,                                              // ADN Length is 21 dec
+        0x07, 0x6D, 0x79, 0x68, 0x6F, 0x73, 0x74, '1',   // FQDN: myhost1.
+        0x07, 0x65, 0x78, 0x61, 0x6D, 0x70, 0x6C, 0x65,  // example.
+        0x03, 0x63, 0x6F, 0x6D, 0x00,                    // com.
+        8,                                               // Addr Len
+        192,  168,  0,    1,                             // IP address 1
+        192,  168,  0,    2,                             // IP address 2
+        'k',  'e',  'y',  '1',  '2',  '3',  '=',  'v',   // Svc Params
+        'a',  'l',  ' ',  'k',  'e',  'y',  '2',  '3',   // Svc Params
+        '4',  '=',  'v',  'a',  'l',  '2',  ' ',  'k',   // Svc Params
+        'e',  'y',  '3',  '4',  '5'                      // Svc Params
+    };
+    OptionBuffer buf(buf_data, buf_data + sizeof(buf_data));
+    // Create option instance. Check that constructor doesn't throw.
+    scoped_ptr<Option4Dnr> option;
+    EXPECT_NO_THROW(option.reset(new Option4Dnr(buf.begin(), buf.end())));
+    ASSERT_TRUE(option);
+
+    // Check if member variables were correctly set by ctor.
+    EXPECT_EQ(Option::V4, option->getUniverse());
+    EXPECT_EQ(DHO_V4_DNR, option->getType());
+
+    // Check if data was unpacked correctly from wire data.
+    const DnrInstance& dnr_i = option->getDnrInstances()[0];
+    EXPECT_EQ(62, dnr_i.getDnrInstanceDataLength());
+    EXPECT_EQ(1, dnr_i.getServicePriority());
+    EXPECT_EQ(21, dnr_i.getAdnLength());
+    EXPECT_EQ("myhost1.example.com.", dnr_i.getAdnAsText());
+    EXPECT_EQ(8, dnr_i.getAddrLength());
+    EXPECT_EQ(29, dnr_i.getSvcParamsLength());
+    EXPECT_EQ(2, dnr_i.getAddresses().size());
+    EXPECT_EQ("192.168.0.1", dnr_i.getAddresses()[0].toText());
+    EXPECT_EQ("192.168.0.2", dnr_i.getAddresses()[1].toText());
+    EXPECT_EQ("key123=val key234=val2 key345", dnr_i.getSvcParams());
+    EXPECT_EQ(66, option->len());
+}
+
 }  // namespace
\ No newline at end of file