[master] Merge branch 'trac3624'

src/lib/dhcp/option4_client_fqdn.cc

@@ -446,8 +446,10 @@ Option4ClientFqdn::packDomainName(isc::util::OutputBuffer& buf) const {
         }
 
     } else {
-        std::string domain_name = impl_->domain_name_->toText();
-        buf.writeData(&domain_name[0], domain_name.size());
+        std::string domain_name = getDomainName();
+        if (!domain_name.empty()) {
+            buf.writeData(&domain_name[0], domain_name.size());
+        }
 
     }
 }
@@ -512,11 +514,24 @@ Option4ClientFqdn::toText(int indent) {
 uint16_t
 Option4ClientFqdn::len() {
     uint16_t domain_name_length = 0;
-    // If domain name is partial, the NULL terminating character
-    // is not included and the option length have to be adjusted.
+    // Try to calculate the length of the domain name only if there is
+    // any domain name specified.
     if (impl_->domain_name_) {
-        domain_name_length = impl_->domain_name_type_ == FULL ?
-            impl_->domain_name_->getLength() : impl_->domain_name_->getLength() - 1;
+        // If the E flag is specified, the domain name is encoded in the
+        // canonical format. The length of the domain name depends on
+        // whether it is a partial or fully qualified names. For the
+        // former the length is 1 octet lesser because it lacks terminating
+        // zero.
+        if (getFlag(FLAG_E)) {
+            domain_name_length = impl_->domain_name_type_ == FULL ?
+                impl_->domain_name_->getLength() :
+                impl_->domain_name_->getLength() - 1;
+
+        // ASCII encoded domain name. Its length is just a length of the
+        // string holding the name.
+        } else {
+            domain_name_length = getDomainName().length();
+        }
     }
 
     return (getHeaderLen() + FIXED_FIELDS_LEN + domain_name_length);

src/lib/dhcp/tests/option4_client_fqdn_unittest.cc

@@ -694,7 +694,7 @@ TEST(Option4ClientFqdnTest, packASCII) {
 
     // Prepare reference data.
     const uint8_t ref_data[] = {
-        81, 23,                               // header
+        81, 22,                               // header
         Option4ClientFqdn::FLAG_S,            // flags
         0,                                    // RCODE1
         0,                                    // RCODE2
@@ -955,6 +955,81 @@ TEST(Option4ClientFqdnTest, len) {
     );
     ASSERT_TRUE(option);
     EXPECT_EQ(12, option->len());
-    }
+
+    // Another test for partial domain name but this time the domain name
+    // contains two labels.
+    ASSERT_NO_THROW(
+        option.reset(new Option4ClientFqdn(Option4ClientFqdn::FLAG_E,
+                                           Option4ClientFqdn::RCODE_CLIENT(),
+                                           "myhost.example",
+                                           Option4ClientFqdn::PARTIAL))
+    );
+    ASSERT_TRUE(option);
+    EXPECT_EQ(20, option->len());
+
+}
+
+// This test verifies that the correct length of the option in on-wire
+// format is returned when ASCII encoding for FQDN is in use.
+TEST(Option4ClientFqdnTest, lenAscii) {
+    // Create option instance. Check that constructor doesn't throw.
+    boost::scoped_ptr<Option4ClientFqdn> option;
+    ASSERT_NO_THROW(
+        option.reset(new Option4ClientFqdn(0, Option4ClientFqdn::RCODE_CLIENT(),
+                                           "myhost.example.com"))
+    );
+    ASSERT_TRUE(option);
+
+    // This option comprises a header (2 octets), flag field (1 octet),
+    // RCODE1 and RCODE2 (2 octets) and the domain name in the ASCII format.
+    // The length of the domain name in the ASCII format is 19 - length
+    // of the string plus terminating dot.
+    EXPECT_EQ(24, option->len());
+
+    // Let's change the domain name and see if the length is different.
+    ASSERT_NO_THROW(
+        option.reset(new Option4ClientFqdn(0, Option4ClientFqdn::RCODE_CLIENT(),
+                                           "example.com"))
+    );
+    ASSERT_TRUE(option);
+
+    EXPECT_EQ(17, option->len());
+
+    // Let's test the length of the option when the partial domain name is
+    // specified.
+    ASSERT_NO_THROW(
+        option.reset(new Option4ClientFqdn(0, Option4ClientFqdn::RCODE_CLIENT(),
+                                           "myhost",
+                                           Option4ClientFqdn::PARTIAL))
+    );
+    ASSERT_TRUE(option);
+
+    // For partial names, there is no terminating dot, so the length of the
+    // domain name is equal to the length of the "myhost".
+    EXPECT_EQ(11, option->len());
+
+    // Another check for partial domain name but this time the domain name
+    // contains two labels.
+    ASSERT_NO_THROW(
+        option.reset(new Option4ClientFqdn(0, Option4ClientFqdn::RCODE_CLIENT(),
+                                           "myhost.example",
+                                           Option4ClientFqdn::PARTIAL))
+    );
+    ASSERT_TRUE(option);
+
+    EXPECT_EQ(19, option->len());
+
+
+    // A special case is an empty domain name for which the returned length
+    // should be a sum of the header length, RCODE1, RCODE2 and flag fields
+    // length.
+    ASSERT_NO_THROW(
+        option.reset(new Option4ClientFqdn(0, Option4ClientFqdn::RCODE_CLIENT(),
+                                           "", Option4ClientFqdn::PARTIAL))
+    );
+    ASSERT_TRUE(option);
+
+    EXPECT_EQ(5, option->len());
+}
 
 } // anonymous namespace

src/lib/dhcp/tests/option6_client_fqdn_unittest.cc

@@ -191,17 +191,16 @@ TEST(Option6ClientFqdnTest, constructFromWireTooLongLabel) {
 // is over 255.
 TEST(Option6ClientFqdnTest, constructFromWireTooLongDomainName) {
     OptionBuffer in_buf(Option6ClientFqdn::FLAG_S);
+    // Construct the FQDN from 26 labels, each having a size of 10.
     for (int i = 0; i < 26;  ++i) {
+        // Append the length of each label.
         in_buf.push_back(10);
+        // Append the actual label.
         in_buf.insert(in_buf.end(), 10, 109);
     }
+    // Terminate FQDN with a dot.
     in_buf.push_back(0);
 
-    try {
-        Option6ClientFqdn(in_buf.begin(), in_buf.end());
-    } catch (const Exception& ex) {
-        std::cout << ex.what() << std::endl;
-    }
     EXPECT_THROW(Option6ClientFqdn(in_buf.begin(), in_buf.end()),
                  InvalidOption6FqdnDomainName);
 }
@@ -791,6 +790,14 @@ TEST(Option6ClientFqdnTest, len) {
     // length of the string representation of the domain name + 1).
     EXPECT_EQ(25, option->len());
 
+    // Use different domain name to check if the length also changes
+    // as expected.
+    ASSERT_NO_THROW(
+        option.reset(new Option6ClientFqdn(0, "example.com"))
+    );
+    ASSERT_TRUE(option);
+    EXPECT_EQ(18, option->len());
+
     // Let's check that the size will change when domain name of a different
     // size is used.
     ASSERT_NO_THROW(
@@ -805,6 +812,16 @@ TEST(Option6ClientFqdnTest, len) {
     );
     ASSERT_TRUE(option);
     EXPECT_EQ(12, option->len());
+
+    // Another test for partial domain name but this time using
+    // two labels.
+    ASSERT_NO_THROW(
+        option.reset(new Option6ClientFqdn(0, "myhost.example",
+                                           Option6ClientFqdn::PARTIAL))
+    );
+    ASSERT_TRUE(option);
+    EXPECT_EQ(20, option->len());
+
 }
 
 } // anonymous namespace