[3624] Fixed Client FQDN option length calculation for ASCII encoding.

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,46 @@ TEST(Option4ClientFqdnTest, len) {
     );
     ASSERT_TRUE(option);
     EXPECT_EQ(12, 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());
+
+    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());
+
+    // 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