[5087] Done: now DHCPv4 domain-search option takes a (possibly compressed) FQDN list

6 files changed, 170 insertions, 13 deletions

diff --git a/doc/guide/dhcp4-srv.xml b/doc/guide/dhcp4-srv.xml
index c743fd007d..48dc2decca 100644
--- a/doc/guide/dhcp4-srv.xml
+++ b/doc/guide/dhcp4-srv.xml
@@ -1245,7 +1245,7 @@ It is merely echoed by the server
 <row><entry>client-ndi</entry><entry>94</entry><entry>record (uint8, uint8, uint8)</entry><entry>false</entry><entry>false</entry></row>
 <row><entry>uuid-guid</entry><entry>97</entry><entry>record (uint8, binary)</entry><entry>false</entry><entry>false</entry></row>
 <row><entry>subnet-selection</entry><entry>118</entry><entry>ipv4-address</entry><entry>false</entry><entry>false</entry></row>
-<row><entry>domain-search</entry><entry>119</entry><entry>binary</entry><entry>false</entry><entry>false</entry></row>
+<row><entry>domain-search</entry><entry>119</entry><entry>fqdn</entry><entry>true</entry><entry>false</entry></row>
 <row><entry>vivco-suboptions</entry><entry>124</entry><entry>binary</entry><entry>false</entry><entry>false</entry></row>
 <row><entry>vivso-suboptions</entry><entry>125</entry><entry>binary</entry><entry>false</entry><entry>false</entry></row>
           </tbody>
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index 9094b8fe89..f997ef2535 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -2930,6 +2930,22 @@ TEST_F(Dhcp4ParserTest, DISABLED_Uint32Parser) {
     EXPECT_TRUE(errorContainsPosition(status, "<string>"));
 }
 
+// The goal of this test is to verify that the domain-search option
+// can be set using domain names
+TEST_F(Dhcp4ParserTest, domainSearchOption) {
+    // Create configuration.
+    std::map<std::string, std::string> params;
+    params["name"] = "domain-search";
+    params["space"] = DHCP4_OPTION_SPACE;
+    params["code"] = "119"; // DHO_DOMAIN_SEARCH
+    params["data"] = "mydomain.example.com, example.com";
+    params["csv-format"] = "true";
+
+    std::string config = createConfigWithOption(params);
+    EXPECT_TRUE(executeConfiguration(config, "parse configuration with a"
+                                     " domain-search option"));
+}
+
 // The goal of this test is to verify that the standard option can
 // be configured to encapsulate multiple other options.
 TEST_F(Dhcp4ParserTest, stdOptionDataEncapsulate) {
@@ -4748,7 +4764,7 @@ TEST_F(Dhcp4ParserTest, invalidPoolRange) {
 
     EXPECT_EQ(1, rcode);
     string expected = "Failed to create pool defined by: "
-	"192.0.2.1-19.2.0.200 (<string>:6:26)";
+        "192.0.2.1-19.2.0.200 (<string>:6:26)";
     EXPECT_EQ(expected, text);
 }
 
@@ -4778,9 +4794,9 @@ TEST_F(Dhcp4ParserTest, outsideSubnetPool) {
 
     EXPECT_EQ(1, rcode);
     string expected = "subnet configuration failed: "
-	"a pool of type V4, with the following address range: "
-	"192.0.2.1-192.0.2.100 does not match the prefix of a subnet: "
-	"10.0.2.0/24 to which it is being added (<string>:5:14)";
+        "a pool of type V4, with the following address range: "
+        "192.0.2.1-192.0.2.100 does not match the prefix of a subnet: "
+        "10.0.2.0/24 to which it is being added (<string>:5:14)";
     EXPECT_EQ(expected, text);
 }
 
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index 770dc12fe9..4859b80b1f 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -25,6 +25,8 @@
 #include <dhcp/option_vendor.h>
 #include <dhcp/option_vendor_class.h>
 #include <util/encode/hex.h>
+#include <dns/labelsequence.h>
+#include <dns/name.h>
 #include <util/strutil.h>
 #include <boost/algorithm/string/classification.hpp>
 #include <boost/algorithm/string/predicate.hpp>
@@ -445,6 +447,11 @@ OptionDefinition::haveOpaqueDataTuplesFormat() const {
 }
 
 bool
+OptionDefinition::haveCompressedFqdnListFormat() const {
+    return (haveType(OPT_FQDN_TYPE) && getArrayType());
+}
+
+bool
 OptionDefinition::convertToBool(const std::string& value_str) const {
     // Case-insensitive check that the input is one of: "true" or "false".
     if (boost::iequals(value_str, "true")) {
@@ -774,6 +781,34 @@ OptionDefinition::factoryOpaqueDataTuples(Option::Universe u,
 }
 
 OptionPtr
+OptionDefinition::factoryFqdnList(Option::Universe u,
+                                  OptionBufferConstIter begin,
+                                  OptionBufferConstIter end) const {
+    
+    const std::vector<uint8_t> data(begin, end);
+    InputBuffer in_buf(static_cast<const void*>(&data[0]), data.size());
+    std::vector<uint8_t> out_buf;
+    out_buf.reserve(data.size());
+    while (in_buf.getPosition() < in_buf.getLength()) {
+        // Reuse readFqdn and writeFqdn code but on the whole buffer
+        // so the DNS name code handles compression for us.
+        try {
+            isc::dns::Name name(in_buf);
+            isc::dns::LabelSequence labels(name);
+            if (labels.getDataLength() > 0) {
+                size_t read_len = 0;
+                const uint8_t* label = labels.getData(&read_len);
+                out_buf.insert(out_buf.end(), label, label + read_len);
+            }
+        } catch (const isc::Exception& ex) {
+            isc_throw(InvalidOptionValue, ex.what());
+        }
+    }
+    return OptionPtr(new OptionCustom(*this, u,
+                                      out_buf.begin(), out_buf.end()));
+}
+
+OptionPtr
 OptionDefinition::factorySpecialFormatOption(Option::Universe u,
                                              OptionBufferConstIter begin,
                                              OptionBufferConstIter end) const {
@@ -820,6 +855,9 @@ OptionDefinition::factorySpecialFormatOption(Option::Universe u,
     } else {
         if ((getCode() == DHO_FQDN) && haveFqdn4Format()) {
             return (OptionPtr(new Option4ClientFqdn(begin, end)));
+        } else if ((getCode() == DHO_DOMAIN_SEARCH) &&
+                   haveCompressedFqdnListFormat()) {
+            return (factoryFqdnList(Option::V4, begin, end));
         } else if ((getCode() == DHO_VIVCO_SUBOPTIONS) &&
                    haveVendorClass4Format()) {
             // V-I Vendor Class (option code 124).
diff --git a/src/lib/dhcp/option_definition.h b/src/lib/dhcp/option_definition.h
index 3871f042c3..d96b8d129e 100644
--- a/src/lib/dhcp/option_definition.h
+++ b/src/lib/dhcp/option_definition.h
@@ -374,6 +374,9 @@ public:
     /// @return true if option has the format of OpaqueDataTuples type options.
     bool haveOpaqueDataTuplesFormat() const;
 
+    /// @brief Check if the option has format of CompressedFqdnList options.
+    bool haveCompressedFqdnListFormat() const;
+
     /// @brief Option factory.
     ///
     /// This function creates an instance of DHCP option using
@@ -578,6 +581,19 @@ public:
 
 private:
 
+    /// @brief Factory function to create option with a compressed FQDN list.
+    ///
+    /// @param u universe (V4 or V6).
+    /// @param type option type.
+    /// @param begin iterator pointing to the beginning of the buffer.
+    /// @param end iterator pointing to the end of the buffer.
+    ///
+    /// @return instance of the DHCP option where FQDNs are uncompressed.
+    /// @throw InvalidOptionValue if data for the option is invalid.
+    OptionPtr factoryFqdnList(Option::Universe u,
+                              OptionBufferConstIter begin,
+                              OptionBufferConstIter end) const;
+
     /// @brief Creates an instance of an option having special format.
     ///
     /// The option with special formats are encapsulated by the dedicated
diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h
index ca81779666..ddc917ec19 100644
--- a/src/lib/dhcp/std_option_defs.h
+++ b/src/lib/dhcp/std_option_defs.h
@@ -192,12 +192,7 @@ const OptionDefParams STANDARD_V4_OPTION_DEFINITIONS[] = {
     { "uuid-guid", DHO_UUID_GUID, OPT_RECORD_TYPE, false, RECORD_DEF(UUID_GUID_RECORDS), "" },
     { "subnet-selection", DHO_SUBNET_SELECTION,
       OPT_IPV4_ADDRESS_TYPE, false, NO_RECORD_DEF, "" },
-    // The following options need a special encoding of data
-    // being carried by them. Therefore, there is no way they can
-    // be handled by OptionCustom. We may need to implement
-    // dedicated classes to handle them. Until that happens
-    // let's treat them as 'binary' options.
-    { "domain-search", DHO_DOMAIN_SEARCH, OPT_BINARY_TYPE, false, NO_RECORD_DEF, "" },
+    { "domain-search", DHO_DOMAIN_SEARCH, OPT_FQDN_TYPE, true, NO_RECORD_DEF, "" },
     { "vivco-suboptions", DHO_VIVCO_SUBOPTIONS, OPT_RECORD_TYPE,
       false, RECORD_DEF(VIVCO_RECORDS), "" },
     // Vendor-Identifying Vendor Specific Information option payload begins with a
diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc
index e5532b2d97..f22a9aefa6 100644
--- a/src/lib/dhcp/tests/libdhcp++_unittest.cc
+++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc
@@ -1319,8 +1319,21 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
     LibDhcpTest::testStdOptionDefs4(DHO_UUID_GUID, begin, begin + 17,
                                     typeid(OptionCustom));
 
-    LibDhcpTest::testStdOptionDefs4(DHO_DOMAIN_SEARCH, begin, end,
-                                    typeid(Option));
+    // Prepare buffer holding an array of FQDNs.
+    const char fqdn_data[] = {
+        8, 109, 121, 100, 111, 109, 97, 105, 110, // "mydomain"
+        7, 101, 120, 97, 109, 112, 108, 101,      // "example"
+        3, 99, 111, 109,                          // "com"
+        0,
+        7, 101, 120, 97, 109, 112, 108, 101,      // "example"
+        3, 99, 111, 109,                          // "com"
+        0
+    };
+    // Initialize a vector with the FQDN data.
+    std::vector<uint8_t> fqdn_buf(fqdn_data, fqdn_data + sizeof(fqdn_data));
+
+    LibDhcpTest::testStdOptionDefs4(DHO_DOMAIN_SEARCH, fqdn_buf.begin(),
+                                    fqdn_buf.end(), typeid(OptionCustom));
 
     // V-I Vendor option requires specially crafted data.
     const char vivco_data[] = {
@@ -1654,6 +1667,85 @@ TEST_F(LibDhcpTest, getVendorOptionDefByName4) {
     }
 }
 
+// This test checks handling of compressed FQDN list.
+TEST_F(LibDhcpTest, fqdnList) {
+    OptionDefinitionPtr def = LibDHCP::getOptionDef(DHCP4_OPTION_SPACE,
+                                                    DHO_DOMAIN_SEARCH);
+    ASSERT_TRUE(def);
+
+    // Prepare buffer holding an array of FQDNs.
+    const uint8_t fqdn[] = {
+        8, 109, 121, 100, 111, 109, 97, 105, 110, // "mydomain"
+        7, 101, 120, 97, 109, 112, 108, 101,      // "example"
+        3, 99, 111, 109,                          // "com"
+        0,
+        7, 101, 120, 97, 109, 112, 108, 101,      // "example"
+        3, 99, 111, 109,                          // "com"
+        0,
+        3, 99, 111, 109,                          // "com"
+        0
+    };
+    // Initialize a vector with the FQDN data.
+    std::vector<uint8_t> fqdn_buf(fqdn, fqdn + sizeof(fqdn));
+
+    OptionPtr option;
+    ASSERT_NO_THROW(option = def->optionFactory(Option::V4,
+                                                DHO_DOMAIN_SEARCH,
+                                                fqdn_buf.begin(),
+                                                fqdn_buf.end()));
+    ASSERT_TRUE(option);
+    OptionCustomPtr names = boost::dynamic_pointer_cast<OptionCustom>(option);
+    ASSERT_TRUE(names);
+    EXPECT_EQ(sizeof(fqdn), names->len() - names->getHeaderLen());
+    ASSERT_EQ(3, names->getDataFieldsNum());
+    EXPECT_EQ("mydomain.example.com.", names->readFqdn(0));
+    EXPECT_EQ("example.com.", names->readFqdn(1));
+    EXPECT_EQ("com.", names->readFqdn(2));
+
+    LibDhcpTest::testStdOptionDefs4(DHO_DOMAIN_SEARCH, fqdn_buf.begin(),
+                                    fqdn_buf.end(), typeid(OptionCustom));
+
+    const uint8_t compressed[] = {
+        8, 109, 121, 100, 111, 109, 97, 105, 110, // "mydomain"
+        7, 101, 120, 97, 109, 112, 108, 101,      // "example"
+        3, 99, 111, 109,                          // "com"
+        0,
+        192, 9,                                   // pointer to example.com
+        192, 17                                   // pointer to com
+    };
+    std::vector<uint8_t> compressed_buf(compressed,
+                                        compressed + sizeof(compressed));
+
+    ASSERT_NO_THROW(option = def->optionFactory(Option::V4,
+                                                DHO_DOMAIN_SEARCH,
+                                                compressed_buf.begin(),
+                                                compressed_buf.end()));
+    ASSERT_TRUE(option);
+    names = boost::dynamic_pointer_cast<OptionCustom>(option);
+    ASSERT_TRUE(names);
+    EXPECT_EQ(sizeof(fqdn), names->len() - names->getHeaderLen());
+    ASSERT_EQ(3, names->getDataFieldsNum());
+    EXPECT_EQ("mydomain.example.com.", names->readFqdn(0));
+    EXPECT_EQ("example.com.", names->readFqdn(1));
+    EXPECT_EQ("com.", names->readFqdn(2));
+
+    const uint8_t bad[] = {
+        8, 109, 121, 100, 111, 109, 97, 105, 110, // "mydomain"
+        7, 101, 120, 97, 109, 112, 108, 101,      // "example"
+        3, 99, 111, 109,                          // "com"
+        0,
+        192, 80,                                  // too big/forward pointer
+        192, 11                                   // pointer to com
+    };
+    std::vector<uint8_t> bad_buf(bad, bad + sizeof(bad));
+
+    EXPECT_THROW(option = def->optionFactory(Option::V4, 
+                                             DHO_DOMAIN_SEARCH,
+                                             bad_buf.begin(),
+                                             bad_buf.end()),
+                 InvalidOptionValue);
+}
+
 // tests whether v6 vendor-class option can be parsed properly.
 TEST_F(LibDhcpTest, vendorClass6) {