[master] Merge branch 'trac3340'

Corrects DHO_HOST_OPTION bug in b10-dhcp4.

src/bin/dhcp4/dhcp4_srv.cc

@@ -703,12 +703,11 @@ Dhcpv4Srv::processClientName(const Pkt4Ptr& query, Pkt4Ptr& answer) {
             processClientFqdnOption(fqdn, answer);
 
         } else {
-            OptionCustomPtr hostname = boost::dynamic_pointer_cast<OptionCustom>
+            OptionStringPtr hostname = boost::dynamic_pointer_cast<OptionString>
                 (query->getOption(DHO_HOST_NAME));
             if (hostname) {
                 processHostnameOption(hostname, answer);
             }
-
         }
     } catch (const Exception& ex) {
         // In some rare cases it is possible that the client's name processing
@@ -761,7 +760,7 @@ Dhcpv4Srv::processClientFqdnOption(const Option4ClientFqdnPtr& fqdn,
 }
 
 void
-Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
+Dhcpv4Srv::processHostnameOption(const OptionStringPtr& opt_hostname,
                                  Pkt4Ptr& answer) {
     // Fetch D2 configuration.
     D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr();
@@ -771,7 +770,7 @@ Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
         return;
     }
 
-    std::string hostname = isc::util::str::trim(opt_hostname->readString());
+    std::string hostname = isc::util::str::trim(opt_hostname->getValue());
     unsigned int label_count = OptionDataTypeUtil::getLabelCount(hostname);
     // The hostname option sent by the client should be at least 1 octet long.
     // If it isn't we ignore this option. (Per RFC 2131, section 3.14)
@@ -785,7 +784,7 @@ Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
     // possible that we will use the hostname option provided by the client
     // to perform the DNS update and we will send the same option to him to
     // indicate that we accepted this hostname.
-    OptionCustomPtr opt_hostname_resp(new OptionCustom(*opt_hostname));
+    OptionStringPtr opt_hostname_resp(new OptionString(*opt_hostname));
 
     // The hostname option may be unqualified or fully qualified. The lab_count
     // holds the number of labels for the name. The number of 1 means that
@@ -802,12 +801,14 @@ Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
     /// conversion if needed and possible.
     if ((d2_mgr.getD2ClientConfig()->getReplaceClientName()) ||
         (label_count < 2)) {
-        opt_hostname_resp->writeString("");
+        // Set to root domain to signal later on that we should replace it.
+        // DHO_HOST_NAME is a string option which cannot be empty.
+        opt_hostname_resp->setValue(".");
     } else if (label_count == 2) {
         // If there are two labels, it means that the client has specified
         // the unqualified name. We have to concatenate the unqalified name
         // with the domain name.
-        opt_hostname_resp->writeString(d2_mgr.qualifyName(hostname));
+        opt_hostname_resp->setValue(d2_mgr.qualifyName(hostname));
     }
 
     answer->addOption(opt_hostname_resp);
@@ -958,7 +959,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     std::string hostname;
     bool fqdn_fwd = false;
     bool fqdn_rev = false;
-    OptionCustomPtr opt_hostname;
+    OptionStringPtr opt_hostname;
     Option4ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<
         Option4ClientFqdn>(answer->getOption(DHO_FQDN));
     if (fqdn) {
@@ -966,10 +967,17 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
         fqdn_fwd = fqdn->getFlag(Option4ClientFqdn::FLAG_S);
         fqdn_rev = !fqdn->getFlag(Option4ClientFqdn::FLAG_N);
     } else {
-        opt_hostname = boost::dynamic_pointer_cast<OptionCustom>
+        opt_hostname = boost::dynamic_pointer_cast<OptionString>
             (answer->getOption(DHO_HOST_NAME));
         if (opt_hostname) {
-            hostname = opt_hostname->readString();
+            hostname = opt_hostname->getValue();
+            // DHO_HOST_NAME is string option which cannot be blank,
+            // we use "." to know we should replace it with a fully
+            // generated name. The local string variable needs to be
+            // blank in logic below.
+            if (hostname == ".") {
+                hostname = "";
+            }
             /// @todo It could be configurable what sort of updates the
             /// server is doing when Hostname option was sent.
             fqdn_fwd = true;
@@ -1023,7 +1031,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
                     fqdn->setDomainName(lease->hostname_,
                                         Option4ClientFqdn::FULL);
                 } else if (opt_hostname) {
-                    opt_hostname->writeString(lease->hostname_);
+                    opt_hostname->setValue(lease->hostname_);
                 }
 
             } catch (const Exception& ex) {

src/bin/dhcp4/dhcp4_srv.h

@@ -18,6 +18,7 @@
 #include <dhcp/dhcp4.h>
 #include <dhcp/pkt4.h>
 #include <dhcp/option.h>
+#include <dhcp/option_string.h>
 #include <dhcp/option4_client_fqdn.h>
 #include <dhcp/option_custom.h>
 #include <dhcp_ddns/ncr_msg.h>
@@ -460,10 +461,10 @@ private:
     /// prepare the Hostname option to be sent back to the client in the
     /// server's response.
     ///
-    /// @param opt_hostname An @c OptionCustom object encapsulating the Hostname
+    /// @param opt_hostname An @c OptionString object encapsulating the Hostname
     /// %Option.
     /// @param [out] answer A response message to be sent to a client.
-    void processHostnameOption(const OptionCustomPtr& opt_hostname,
+    void processHostnameOption(const OptionStringPtr& opt_hostname,
                                Pkt4Ptr& answer);
 
 protected:

src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -118,11 +118,11 @@ public:
    }
 
     // Create an instance of the Hostname option.
-    OptionCustomPtr
+    OptionStringPtr
     createHostname(const std::string& hostname) {
-        OptionDefinition def("hostname", DHO_HOST_NAME, "string");
-        OptionCustomPtr opt_hostname(new OptionCustom(def, Option::V4));
-        opt_hostname->writeString(hostname);
+        OptionStringPtr opt_hostname(new OptionString(Option::V4,
+                                                      DHO_HOST_NAME,
+                                                      hostname));
         return (opt_hostname);
     }
 
@@ -147,9 +147,9 @@ public:
     }
 
     // get the Hostname option from the given message.
-    OptionCustomPtr getHostnameOption(const Pkt4Ptr& pkt) {
+    OptionStringPtr getHostnameOption(const Pkt4Ptr& pkt) {
         return (boost::dynamic_pointer_cast<
-                OptionCustom>(pkt->getOption(DHO_HOST_NAME)));
+                OptionString>(pkt->getOption(DHO_HOST_NAME)));
     }
 
     // Create a message holding DHCPv4 Client FQDN Option.
@@ -264,7 +264,7 @@ public:
     // the hostname option which would be sent to the client. It will
     // throw NULL pointer if the hostname option is not to be included
     // in the response.
-    OptionCustomPtr processHostname(const Pkt4Ptr& query) {
+    OptionStringPtr processHostname(const Pkt4Ptr& query) {
         if (!getHostnameOption(query)) {
             ADD_FAILURE() << "Hostname option not carried in the query";
         }
@@ -279,7 +279,7 @@ public:
         }
         srv_->processClientName(query, answer);
 
-        OptionCustomPtr hostname = getHostnameOption(answer);
+        OptionStringPtr hostname = getHostnameOption(answer);
         return (hostname);
 
     }
@@ -571,29 +571,20 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateHostname) {
     Pkt4Ptr query;
     ASSERT_NO_THROW(query = generatePktWithHostname(DHCPREQUEST,
                                                     "myhost.example.com."));
-    OptionCustomPtr hostname;
+    OptionStringPtr hostname;
     ASSERT_NO_THROW(hostname = processHostname(query));
 
     ASSERT_TRUE(hostname);
-    EXPECT_EQ("myhost.example.com.", hostname->readString());
+    EXPECT_EQ("myhost.example.com.", hostname->getValue());
 
 }
 
-// Test that the server skips processing of the empty Hostname option.
-TEST_F(NameDhcpv4SrvTest, serverUpdateEmptyHostname) {
-    Pkt4Ptr query;
-    ASSERT_NO_THROW(query = generatePktWithHostname(DHCPREQUEST, ""));
-    OptionCustomPtr hostname;
-    ASSERT_NO_THROW(hostname = processHostname(query));
-    EXPECT_FALSE(hostname);
-}
-
 // Test that the server skips processing of a wrong Hostname option.
 TEST_F(NameDhcpv4SrvTest, serverUpdateWrongHostname) {
     Pkt4Ptr query;
     ASSERT_NO_THROW(query = generatePktWithHostname(DHCPREQUEST,
                                                     "abc..example.com"));
-    OptionCustomPtr hostname;
+    OptionStringPtr hostname;
     ASSERT_NO_THROW(hostname = processHostname(query));
     EXPECT_FALSE(hostname);
 }
@@ -620,11 +611,11 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateForwardPartialNameFqdn) {
 TEST_F(NameDhcpv4SrvTest, serverUpdateUnqualifiedHostname) {
     Pkt4Ptr query;
     ASSERT_NO_THROW(query = generatePktWithHostname(DHCPREQUEST, "myhost"));
-    OptionCustomPtr hostname;
+    OptionStringPtr hostname;
     ASSERT_NO_THROW(hostname =  processHostname(query));
 
     ASSERT_TRUE(hostname);
-    EXPECT_EQ("myhost.example.com.", hostname->readString());
+    EXPECT_EQ("myhost.example.com.", hostname->getValue());
 
 }
 
@@ -818,7 +809,7 @@ TEST_F(NameDhcpv4SrvTest, processRequestEmptyDomainNameDisabled) {
 
 // Test that server generates client's hostname from the IP address assigned
 // to it when Hostname option carries the top level domain-name.
-TEST_F(NameDhcpv4SrvTest, processRequestEmptyHostname) {
+TEST_F(NameDhcpv4SrvTest, processRequestTopLevelHostname) {
     IfaceMgrTestConfig test_config(true);
     IfaceMgr::instance().openSockets4();