Parcourir la source

[master] Merge branch 'trac3467'

Marcin Siodelski il y a 10 ans
Parent
commit
7bf8cef161

+ 2 - 0
doc/Makefile.am

@@ -4,8 +4,10 @@ EXTRA_DIST = version.ent.in differences.txt Doxyfile Doxyfile-xml
 
 nobase_dist_doc_DATA  = examples/kea4/single-subnet.json
 nobase_dist_doc_DATA += examples/kea4/several-subnets.json
+nobase_dist_doc_DATA += examples/kea4/multiple-options.json
 nobase_dist_doc_DATA += examples/kea6/simple.json
 nobase_dist_doc_DATA += examples/kea6/several-subnets.json
+nobase_dist_doc_DATA += examples/kea6/multiple-options.json
 nobase_dist_doc_DATA += examples/ddns/sample1.json
 nobase_dist_doc_DATA += examples/ddns/template.json
 

+ 74 - 0
doc/examples/kea4/multiple-options.json

@@ -0,0 +1,74 @@
+# This is an example configuration file for the DHCPv4 server in Kea.
+# It demonstrates simple configuration of the options for a subnet.
+
+{ "Dhcp4":
+
+{
+# Kea is told to listen on eth0 interface only.
+  "interfaces": [ "eth0" ],
+
+# We need to specify lease type. As of May 2014, three backends are supported:
+# memfile, mysql and pgsql. We'll just use memfile, because it doesn't require
+# any prior set up.
+  "lease-database": {
+    "type": "memfile"
+  },
+
+# Addresses will be assigned with valid lifetimes being 4000. Client
+# is told to start renewing after 1000 seconds. If the server does not respond
+# after 2000 seconds since the lease was granted, client is supposed
+# to start REBIND procedure (emergency renewal that allows switching
+# to a different server).
+  "valid-lifetime": 4000,
+
+# Renew and rebind timers are commented out. This implies that options
+# 58 and 59 will not be sent to the client. In this case it is up to
+# the client to pick the timer values according to RFC2131. Uncomment the
+# timers to send these options to the client.
+#  "renew-timer": 1000,
+#  "rebind-timer": 2000,
+
+# Defining a subnet. There are 3 DHCP options returned to the
+# clients connected to this subnet. The first two options are
+# identified by the name. The third option is identified by the
+# option code.
+  "subnet4": [
+    {
+       "pools": [ { "pool":  "192.0.2.10 - 192.0.2.200" } ],
+       "subnet": "192.0.2.0/24",
+       "interface": "eth0",
+       "option-data": [
+         {
+             "name": "domain-name-servers",
+             "data": "192.0.2.1, 192.0.2.2"
+         },
+         {
+             "name": "routers",
+             "data": "192.0.2.1"
+         },
+         {
+             "code": 15,
+             "data": "example.org"
+         }
+       ]
+    } 
+  ]
+},
+
+# The following configures logging. It assumes that messages with at least
+# informational level (info, warn, error) will will be logged to stdout.
+"Logging": {
+    "loggers": [
+        {
+            "name": "kea-dhcp4",
+            "output_options": [
+                {
+                    "output": "stdout"
+                }
+            ],
+            "severity": "INFO"
+        }
+    ]
+}
+
+}

+ 68 - 0
doc/examples/kea6/multiple-options.json

@@ -0,0 +1,68 @@
+# This is an example configuration file for DHCPv6 server in Kea.
+# It demonstrates simple configuration of the options for a subnet.
+
+{ "Dhcp6":
+
+{
+# Kea is told to listen on eth0 interface only.
+  "interfaces": [ "eth0" ],
+
+# We need to specify lease type. As of May 2014, three backends are supported:
+# memfile, mysql and pgsql. We'll just use memfile, because it doesn't require
+# any prior set up.
+  "lease-database": {
+    "type": "memfile"
+  },
+
+# Addresses will be assigned with preferred and valid lifetimes
+# being 3000 and 4000, respectively. Client is told to start
+# renewing after 1000 seconds. If the server does not repond
+# after 2000 seconds since the lease was granted, client is supposed
+# to start REBIND procedure (emergency renewal that allows switching
+# to a different server).
+  "preferred-lifetime": 3000,
+  "valid-lifetime": 4000,
+  "renew-timer": 1000,
+  "rebind-timer": 2000,
+
+# Defining a subnet. There are 2 DHCP options returned to the
+# clients connected to this subnet. The first option is identified
+# by the name. The second option is identified by the code.
+  "subnet6": [
+    {
+      "pools": [ { "pool": "2001:db8:1::/80" } ],
+      "subnet": "2001:db8:1::/64",
+      "interface": "eth0",
+      "option-data": [
+        {
+            "name": "dns-servers",
+            "data": "2001:db8:2::45, 2001:db8:2::100"
+        },
+        {
+            "code": 12,
+            "data": "2001:db8:1:0:ff00::1"
+        },
+      ]
+    }
+  ]
+},
+
+# The following configures logging. Kea will log all debug messages
+# to /var/log/kea-debug.log file.
+"Logging": {
+    "loggers": [
+        {
+            "name": "kea-dhcp6",
+            "output_options": [
+                {
+                    "output": "/var/log/kea-debug.log"
+                }
+            ],
+            "debuglevel": 99,
+            "severity": "DEBUG"
+        }
+    ]
+}
+
+}
+

+ 76 - 9
doc/guide/dhcp4-srv.xml

@@ -553,6 +553,12 @@ temporarily override a list of interface names and listen on all interfaces.
       </para>
 
       <para>
+        Most of the parameters in the "option-data" structure are optional and
+        can be omitted in some circumstances as discussed in the
+        <xref linkend="dhcp4-option-data-defaults"/>.
+      </para>
+
+      <para>
         It is possible to specify or override options on a per-subnet basis.  If
         clients connected to most of your subnets are expected to get the
         same values of a given option, you should use global options: you
@@ -589,15 +595,6 @@ temporarily override a list of interface names and listen on all interfaces.
 </screen>
       </para>
 
-    <note>
-      <para>
-        In future versions of Kea, it will not be necessary to specify
-        the <command>code</command>, <command>space</command>
-        and <command>csv-format</command> fields, as they will
-        be set automatically.
-      </para>
-    </note>
-
       <para>
         The currently supported standard DHCPv4 options are
         listed in <xref linkend="dhcp4-std-options-list"/>
@@ -1099,6 +1096,76 @@ temporarily override a list of interface names and listen on all interfaces.
     </para>
     </section>
 
+    <section id="dhcp4-option-data-defaults">
+      <title>Unspecified parameters for DHCPv4 option configuration</title>
+      <para>In many cases it is not required to specify all parameters for
+      an option configuration and the default values may be used. However, it is
+      important to understand the implications of not specifing some of them
+      as it may result in configuration errors. The list below explains
+      the behavior of the server when a particular parameter is not explicitly
+      specified:
+
+      <itemizedlist>
+        <listitem>
+          <simpara><command>name</command> - the server requires an option name or
+          option code to identify an option. If this parameter is unspecified, the
+          option code must be specified.
+          </simpara>
+        </listitem>
+
+        <listitem>
+          <simpara><command>code</command> - the server requires an option name or
+          option code to identify an option. This parameter may be left unspecified if
+          the <command>name</command> parameter is specified. However, this also
+          requires that the particular option has its definition (it is either a
+          standard option or an administrator created a definition for the option
+          using an 'option-def' structure), as the option definition associates an
+          option with a particular name. It is possible to configure an option
+          for which there is no definition (unspecified option format).
+          Configuration of such options requires the use of option code.
+          </simpara>
+        </listitem>
+
+        <listitem>
+          <simpara><command>space</command> - if the option space is unspecified it
+          will default to 'dhcp4' which is an option space holding DHCPv4 standard
+          options.
+          </simpara>
+        </listitem>
+
+        <listitem>
+          <simpara><command>data</command> - if the option data is unspecified it
+          defaults to an empty value. The empty value is mostly used for the
+          options which have no payload (boolean options), but it is legal to specify
+          empty values for some options which carry variable length data and which
+          spec allows for the length of 0. For such options, the data parameter
+          may be omitted in the configuration.</simpara>
+        </listitem>
+
+        <listitem>
+          <simpara><command>csv-format</command> - if this value is not specified
+          and the definition for the particular option exists, the server will assume
+          that the option data is specified as a list of comma separated values to be
+          assigned to individual fields of the DHCP option. If the definition
+          does not exist for this option, the server will assume that the data
+          parameter contains the option payload in the binary format (represented
+          as a string of hexadecimal digits). Note that not specifying this
+          parameter doesn't imply that it defaults to a fixed value, but
+          the configuration data interpretation also depends on the presence
+          of the option definition. An administrator must be aware if the
+          definition for the particular option exists when this parameter
+          is not specified. It is generally recommended to not specify this
+          parameter only for the options for which the definition exists, e.g.
+          standard options. Setting <command>csv-format</command> to an explicit
+          value will cause the server to strictly check the format of the option
+          data specified.
+          </simpara>
+        </listitem>
+      </itemizedlist>
+      </para>
+
+    </section>
+
     <section id="dhcp4-stateless-configuration">
       <title>Stateless Configuration of DHCPv4 clients</title>
       <para>The DHCPv4 server supports the stateless client configuration whereby the

+ 77 - 10
doc/guide/dhcp6-srv.xml

@@ -647,6 +647,13 @@ temporarily override a list of interface names and listen on all interfaces.
        to validate data correctness in hexadecimal is limited.
       </para>
 
+      <para>
+        Most of the parameters in the "option-data" structure are optional and
+        can be omitted in some circumstances as discussed in the
+        <xref linkend="dhcp6-option-data-defaults"/>.
+      </para>
+
+
     <para>
       It is possible to override options on a per-subnet basis.  If
       clients connected to most of your subnets are expected to get the
@@ -684,16 +691,6 @@ temporarily override a list of interface names and listen on all interfaces.
 </screen>
     </para>
 
-    <note>
-      <para>
-        In future versions of Kea, it will not be necessary to specify
-        the <command>code</command>, <command>space</command>
-        and <command>csv-format</command> fields, as they will
-        be set automatically.
-      </para>
-    </note>
-
-
     <para>
       The currently supported standard DHCPv6 options are
       listed in <xref linkend="dhcp6-std-options-list"/>.
@@ -1093,6 +1090,76 @@ should include options from the isc option space:
     </para>
     </section>
 
+    <section id="dhcp6-option-data-defaults">
+      <title>Unspecified parameters for DHCPv6 option configuration</title>
+      <para>In many cases it is not required to specify all parameters for
+      an option configuration and the default values can be used. However, it is
+      important to understand the implications of not specifing some of them
+      as it may result in configuration errors. The list below explains
+      the behavior of the server when a particular parameter is not explicitly
+      specified:
+
+      <itemizedlist>
+        <listitem>
+          <simpara><command>name</command> - the server requires an option name or
+          option code to identify an option. If this parameter is unspecified, the
+          option code must be specified.
+          </simpara>
+        </listitem>
+
+        <listitem>
+          <simpara><command>code</command> - the server requires an option name or
+          option code to identify an option. This parameter may be left unspecified if
+          the <command>name</command> parameter is specified. However, this also
+          requires that the particular option has its definition (it is either a
+          standard option or an administrator created a definition for the option
+          using an 'option-def' structure), as the option definition associates an
+          option with a particular name. It is possible to configure an option
+          for which there is no definition (unspecified option format).
+          Configuration of such options requires the use of option code.
+          </simpara>
+        </listitem>
+
+        <listitem>
+          <simpara><command>space</command> - if the option space is unspecified it
+          will default to 'dhcp6' which is an option space holding DHCPv6 standard
+          options.
+          </simpara>
+        </listitem>
+
+        <listitem>
+          <simpara><command>data</command> - if the option data is unspecified it
+          defaults to an empty value. The empty value is mostly used for the
+          options which have no payload (boolean options), but it is legal to specify
+          empty values for some options which carry variable length data and which
+          spec allows for the length of 0. For such options, the data parameter
+          may be omitted in the configuration.</simpara>
+        </listitem>
+
+        <listitem>
+          <simpara><command>csv-format</command> - if this value is not specified
+          and the definition for the particular option exists, the server will assume
+          that the option data is specified as a list of comma separated values to be
+          assigned to individual fields of the DHCP option. If the definition
+          does not exist for this option, the server will assume that the data
+          parameter contains the option payload in the binary format (represented
+          as a string of hexadecimal digits). Note that not specifying this
+          parameter doesn't imply that it defaults to a fixed value, but
+          the configuration data interpretation also depends on the presence
+          of the option definition. An administrator must be aware if the
+          definition for the particular option exists when this parameter
+          is not specified. It is generally recommended to not specify this
+          parameter only for the options for which the definition exists, e.g.
+          standard options. Setting <command>csv-format</command> to an explicit
+          value will cause the server to strictly check the format of the option
+          data specified.
+          </simpara>
+        </listitem>
+      </itemizedlist>
+      </para>
+
+    </section>
+
     <section id="dhcp6-config-subnets">
       <title>IPv6 Subnet Selection</title>
       <para>

+ 35 - 0
src/lib/dhcp/libdhcp++.cc

@@ -130,6 +130,41 @@ LibDHCP::getOptionDef(const Option::Universe u, const uint16_t code) {
 }
 
 OptionDefinitionPtr
+LibDHCP::getOptionDef(const Option::Universe u, const std::string& name) {
+    const OptionDefContainer& defs = getOptionDefs(u);
+    const OptionDefContainerNameIndex& idx = defs.get<2>();
+    const OptionDefContainerNameRange& range = idx.equal_range(name);
+    if (range.first != range.second) {
+        return (*range.first);
+    }
+    return (OptionDefinitionPtr());
+
+}
+
+
+OptionDefinitionPtr
+LibDHCP::getVendorOptionDef(const Option::Universe u, const uint32_t vendor_id,
+                            const std::string& name) {
+    const OptionDefContainer* defs = NULL;
+    if (u == Option::V4) {
+        defs = getVendorOption4Defs(vendor_id);
+    } else if (u == Option::V6) {
+        defs = getVendorOption6Defs(vendor_id);
+    }
+
+    if (!defs) {
+        return (OptionDefinitionPtr());
+    }
+
+    const OptionDefContainerNameIndex& idx = defs->get<2>();
+    const OptionDefContainerNameRange& range = idx.equal_range(name);
+    if (range.first != range.second) {
+        return (*range.first);
+    }
+    return (OptionDefinitionPtr());
+}
+
+OptionDefinitionPtr
 LibDHCP::getVendorOptionDef(const Option::Universe u, const uint32_t vendor_id,
                             const uint16_t code) {
     const OptionDefContainer* defs = NULL;

+ 25 - 1
src/lib/dhcp/libdhcp++.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -20,6 +20,7 @@
 #include <util/buffer.h>
 
 #include <iostream>
+#include <string>
 
 namespace isc {
 namespace dhcp {
@@ -55,6 +56,16 @@ public:
     static OptionDefinitionPtr getOptionDef(const Option::Universe u,
                                             const uint16_t code);
 
+    /// @brief Return the definition of option having a specified name.
+    ///
+    /// @param u universe (v4 or V6)
+    /// @param name Option name.
+    ///
+    /// @return Pointer to the option definition or NULL pointer if option
+    /// definition has not been found.
+    static OptionDefinitionPtr getOptionDef(const Option::Universe u,
+                                            const std::string& name);
+
     /// @brief Returns vendor option definition for a given vendor-id and code
     ///
     /// @param u universe (V4 or V6)
@@ -66,6 +77,19 @@ public:
                                                   const uint32_t vendor_id,
                                                   const uint16_t code);
 
+    /// @brief Returns vendor option definition for a given vendor-id and
+    /// option name.
+    ///
+    /// @param u Universe (V4 or V6)
+    /// @param vendor_id Enterprise-id for a given vendor
+    /// @param name Option name.
+    ///
+    /// @return A pointer to an option definition or NULL pointer if
+    /// no option definition found.
+    static OptionDefinitionPtr getVendorOptionDef(const Option::Universe u,
+                                                  const uint32_t vendor_id,
+                                                  const std::string& name);
+
     /// @brief Check if the specified option is a standard option.
     ///
     /// @param u universe (V4 or V6)

+ 5 - 2
src/lib/dhcp/option6_addrlst.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2012, 2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -17,7 +17,7 @@
 
 #include <asiolink/io_address.h>
 #include <dhcp/option.h>
-
+#include <boost/shared_ptr.hpp>
 #include <vector>
 
 namespace isc {
@@ -94,6 +94,9 @@ protected:
     AddressContainer addrs_;
 };
 
+/// @brief Pointer to the @c Option6AddrLst object.
+typedef boost::shared_ptr<Option6AddrLst> Option6AddrLstPtr;
+
 } // isc::dhcp namespace
 } // isc namespace
 

+ 20 - 0
src/lib/dhcp/option_definition.h

@@ -717,6 +717,17 @@ typedef boost::multi_index_container<
                 uint16_t,
                 &OptionDefinition::getCode
             >
+        >,
+        // Start definition of index #2
+        boost::multi_index::hashed_non_unique<
+            // Use option name as the index key. This value is
+            // returned by the @c OptionDefinition::getName
+            // method.
+            boost::multi_index::const_mem_fun<
+                OptionDefinition,
+                std::string,
+                &OptionDefinition::getName
+            >
         >
     >
 > OptionDefContainer;
@@ -736,6 +747,15 @@ typedef OptionDefContainer::nth_index<1>::type OptionDefContainerTypeIndex;
 typedef std::pair<OptionDefContainerTypeIndex::const_iterator,
                   OptionDefContainerTypeIndex::const_iterator> OptionDefContainerTypeRange;
 
+/// Type of the index #2 - option name.
+typedef OptionDefContainer::nth_index<2>::type OptionDefContainerNameIndex;
+/// Pair of iterators to represent the range of options definitions
+/// having the same option name. The first element in this pair
+/// represents the beginning of the range, the second element
+/// represents the end.
+typedef std::pair<OptionDefContainerNameIndex::const_iterator,
+                  OptionDefContainerNameIndex::const_iterator> OptionDefContainerNameRange;
+
 
 } // namespace isc::dhcp
 } // namespace isc

+ 64 - 0
src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -16,6 +16,7 @@
 
 #include <dhcp/dhcp4.h>
 #include <dhcp/dhcp6.h>
+#include <dhcp/docsis3_option_defs.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/option4_addrlst.h>
 #include <dhcp/option4_client_fqdn.h>
@@ -1141,6 +1142,69 @@ TEST_F(LibDhcpTest, stdOptionDefs6) {
                                     typeid(Option6AddrLst));
 }
 
+// This test checks if the DHCPv6 option definition can be searched by
+// an option name.
+TEST_F(LibDhcpTest, getOptionDefByName6) {
+    // Get all definitions.
+    const OptionDefContainer& defs = LibDHCP::getOptionDefs(Option::V6);
+    // For each definition try to find it using option name.
+    for (OptionDefContainer::const_iterator def = defs.begin();
+         def != defs.end(); ++def) {
+        OptionDefinitionPtr def_by_name =
+            LibDHCP::getOptionDef(Option::V6, (*def)->getName());
+        ASSERT_TRUE(def_by_name);
+        ASSERT_TRUE(**def == *def_by_name);
+    }
+}
+
+
+// This test checks if the DHCPv4 option definition can be searched by
+// an option name.
+TEST_F(LibDhcpTest, getOptionDefByName4) {
+    // Get all definitions.
+    const OptionDefContainer& defs = LibDHCP::getOptionDefs(Option::V4);
+    // For each definition try to find it using option name.
+    for (OptionDefContainer::const_iterator def = defs.begin();
+         def != defs.end(); ++def) {
+        OptionDefinitionPtr def_by_name =
+            LibDHCP::getOptionDef(Option::V4, (*def)->getName());
+        ASSERT_TRUE(def_by_name);
+        ASSERT_TRUE(**def == *def_by_name);
+    }
+}
+
+// This test checks if the definition of the DHCPv6 vendor option can
+// be searched by option name.
+TEST_F(LibDhcpTest, getVendorOptionDefByName6) {
+    const OptionDefContainer* defs =
+        LibDHCP::getVendorOption6Defs(VENDOR_ID_CABLE_LABS);
+    ASSERT_TRUE(defs != NULL);
+    for (OptionDefContainer::const_iterator def = defs->begin();
+         def != defs->end(); ++def) {
+        OptionDefinitionPtr def_by_name =
+            LibDHCP::getVendorOptionDef(Option::V6, VENDOR_ID_CABLE_LABS,
+                                        (*def)->getName());
+        ASSERT_TRUE(def_by_name);
+        ASSERT_TRUE(**def == *def_by_name);
+    }
+}
+
+// This test checks if the definition of the DHCPv4 vendor option can
+// be searched by option name.
+TEST_F(LibDhcpTest, getVendorOptionDefByName4) {
+    const OptionDefContainer* defs =
+        LibDHCP::getVendorOption4Defs(VENDOR_ID_CABLE_LABS);
+    ASSERT_TRUE(defs != NULL);
+    for (OptionDefContainer::const_iterator def = defs->begin();
+         def != defs->end(); ++def) {
+        OptionDefinitionPtr def_by_name =
+            LibDHCP::getVendorOptionDef(Option::V4, VENDOR_ID_CABLE_LABS,
+                                        (*def)->getName());
+        ASSERT_TRUE(def_by_name);
+        ASSERT_TRUE(**def == *def_by_name);
+    }
+}
+
 // tests whether v6 vendor-class option can be parsed properly.
 TEST_F(LibDhcpTest, vendorClass6) {
 

+ 21 - 1
src/lib/dhcpsrv/cfg_option_def.cc

@@ -138,7 +138,27 @@ CfgOptionDef::get(const std::string& option_space,
     return (OptionDefinitionPtr());
 }
 
-
+OptionDefinitionPtr
+CfgOptionDef::get(const std::string& option_space,
+                  const std::string& option_name) const {
+    // Get the pointer to collection of the option definitions that belong
+    // to the particular option space.
+    OptionDefContainerPtr defs = getAll(option_space);
+    // If there are any option definitions for this option space, get the
+    // one that has the specified option name.
+    if (defs && !defs->empty()) {
+        const OptionDefContainerNameIndex& idx = defs->get<2>();
+        const OptionDefContainerNameRange& range = idx.equal_range(option_name);
+        // If there is more than one definition matching the option name,
+        // return the first one. In fact, it shouldn't happen that we have
+        // more than one because we check for duplicates when we add them.
+        if (std::distance(range.first, range.second) > 0) {
+            return (*range.first);
+        }
+    }
+    // Nothing found. Return NULL pointer.
+    return (OptionDefinitionPtr());
+}
 
 } // end of namespace isc::dhcp
 } // end of namespace isc

+ 10 - 0
src/lib/dhcpsrv/cfg_option_def.h

@@ -110,6 +110,16 @@ public:
     OptionDefinitionPtr get(const std::string& option_space,
                             const uint16_t option_code) const;
 
+    /// @brief Return option definition for the particular option space and name.
+    ///
+    /// @param option_space pption space.
+    /// @param option_name option name.
+    ///
+    /// @return An option definition or NULL pointer if option definition
+    /// has not been found.
+    OptionDefinitionPtr get(const std::string& option_space,
+                            const std::string& option_name) const;
+
 private:
 
     /// @brief A collection of option definitions.

+ 197 - 124
src/lib/dhcpsrv/dhcp_parsers.cc

@@ -33,6 +33,7 @@ using namespace std;
 using namespace isc::asiolink;
 using namespace isc::data;
 using namespace isc::hooks;
+using namespace isc::util;
 
 namespace isc {
 namespace dhcp {
@@ -342,128 +343,203 @@ OptionDataParser::commit() {
     // Does nothing
 }
 
-OptionDefinitionPtr
-OptionDataParser::findServerSpaceOptionDefinition(const std::string& option_space,
-                                                  const uint32_t option_code) const {
-    const Option::Universe u = address_family_ == AF_INET ?
-        Option::V4 : Option::V6;
-
-    if ((option_space == DHCP4_OPTION_SPACE) && (u == Option::V6)) {
-        isc_throw(DhcpConfigError, "'" << DHCP4_OPTION_SPACE
-                  << "' option space name is reserved for DHCPv4 server");
-    } else if ((option_space == DHCP6_OPTION_SPACE) && (u == Option::V4)) {
-        isc_throw(DhcpConfigError, "'" << DHCP6_OPTION_SPACE
-                  << "' option space name is reserved for DHCPv6 server");
-    }
-
-    OptionDefinitionPtr def;
-    if (((option_space == DHCP4_OPTION_SPACE) || (option_space == DHCP6_OPTION_SPACE)) &&
-        LibDHCP::isStandardOption(u, option_code)) {
-        def = LibDHCP::getOptionDef(u, option_code);
-
-    } else {
-        // Check if this is a vendor-option. If it is, get vendor-specific
-        // definition.
-        uint32_t vendor_id = CfgOption::optionSpaceToVendorId(option_space);
-        if (vendor_id) {
-            def = LibDHCP::getVendorOptionDef(u, vendor_id, option_code);
-        }
-    }
-    return (def);
-}
-
-
-void
-OptionDataParser::createOption(ConstElementPtr option_data) {
-    const Option::Universe universe = address_family_ == AF_INET ?
-        Option::V4 : Option::V6;
-    // Check if mandatory parameters are specified.
+OptionalValue<uint32_t>
+OptionDataParser::extractCode(ConstElementPtr parent) const {
     uint32_t code;
-    std::string name;
-    std::string data;
     try {
         code = uint32_values_->getParam("code");
-        name = string_values_->getParam("name");
-        data = string_values_->getParam("data");
-    } catch (const std::exception& ex) {
-        isc_throw(DhcpConfigError,
-                  ex.what() << "(" << option_data->getPosition() << ")");
+
+    } catch (const exception& ex) {
+        // The code parameter was not found. Return an unspecified
+        // value.
+        return (OptionalValue<uint32_t>());
     }
-    // Check parameters having default values.
-    std::string space = string_values_->getOptionalParam("space", universe == Option::V4 ?
-                                                         "dhcp4" : "dhcp6");
-    bool csv_format = boolean_values_->getOptionalParam("csv-format", false);
 
-    // Option code is held in the uint32_t storage but is supposed to
-    // be uint16_t value. We need to check that value in the configuration
-    // does not exceed range of uint8_t for DHCPv4, uint16_t for DHCPv6 and
-    // is not zero.
     if (code == 0) {
         isc_throw(DhcpConfigError, "option code must not be zero "
-                  "(" << uint32_values_->getPosition("code") << ")");
+                  "(" << uint32_values_->getPosition("code", parent) << ")");
 
-    } else if (universe == Option::V4 &&
+    } else if (address_family_ == AF_INET &&
                code > std::numeric_limits<uint8_t>::max()) {
         isc_throw(DhcpConfigError, "invalid option code '" << code
-                << "', it must not exceed '"
+                << "', it must not be greater than '"
                   << static_cast<int>(std::numeric_limits<uint8_t>::max())
-                  << "' (" << uint32_values_->getPosition("code") << ")");
+                  << "' (" << uint32_values_->getPosition("code", parent)
+                  << ")");
 
-    } else if (universe == Option::V6 &&
+    } else if (address_family_ == AF_INET6 &&
                code > std::numeric_limits<uint16_t>::max()) {
         isc_throw(DhcpConfigError, "invalid option code '" << code
                 << "', it must not exceed '"
                   << std::numeric_limits<uint16_t>::max()
-                  << "' (" << uint32_values_->getPosition("code") << ")");
+                  << "' (" << uint32_values_->getPosition("code", parent)
+                  << ")");
+
+    }
+
+    return (OptionalValue<uint32_t>(code, OptionalValueState(true)));
+}
 
+OptionalValue<std::string>
+OptionDataParser::extractName(ConstElementPtr parent) const {
+    std::string name;
+    try {
+        name = string_values_->getParam("name");
+
+    } catch (...) {
+        return (OptionalValue<std::string>());
     }
 
-    // Check that the option name is non-empty and does not contain spaces
-    if (name.empty()) {
-        isc_throw(DhcpConfigError, "name of the option with code '"
-                  << code << "' is empty ("
-                  << string_values_->getPosition("name") << ")");
-    } else if (name.find(" ") != std::string::npos) {
+    if (name.find(" ") != std::string::npos) {
         isc_throw(DhcpConfigError, "invalid option name '" << name
                   << "', space character is not allowed ("
-                  << string_values_->getPosition("name") << ")");
+                  << string_values_->getPosition("name", parent) << ")");
     }
 
-    if (!OptionSpace::validateName(space)) {
-        isc_throw(DhcpConfigError, "invalid option space name '"
-                << space << "' specified for option '"
-                << name << "', code '" << code
-                  << "' (" << string_values_->getPosition("space") << ")");
+    return (OptionalValue<std::string>(name, OptionalValueState(true)));
+}
+
+std::string
+OptionDataParser::extractData() const {
+    std::string data;
+    try {
+        data = string_values_->getParam("data");
+
+    } catch (...) {
+        // The "data" parameter was not found. Return an empty value.
+        return (data);
     }
 
-    // Find the Option Definition for the option by its option code.
-    // findOptionDefinition will throw if not found, no need to test.
-    // Find the definition for the option by its code. This function
-    // may throw so we catch exceptions to log the culprit line of the
-    // configuration.
-    OptionDefinitionPtr def;
+    return (data);
+}
+
+OptionalValue<bool>
+OptionDataParser::extractCSVFormat() const {
+    bool csv_format = true;
     try {
-        def = findServerSpaceOptionDefinition(space, code);
+        csv_format = boolean_values_->getParam("csv-format");
 
-    } catch (const std::exception& ex) {
-        isc_throw(DhcpConfigError, ex.what()
-                  << " (" << string_values_->getPosition("space") << ")");
+    } catch (...) {
+        return (OptionalValue<bool>(csv_format));
+    }
+
+    return (OptionalValue<bool>(csv_format, OptionalValueState(true)));
+}
+
+std::string
+OptionDataParser::extractSpace() const {
+    std::string space = address_family_ == AF_INET ? "dhcp4" : "dhcp6";
+    try {
+        space = string_values_->getParam("space");
+
+    } catch (...) {
+        return (space);
+    }
+
+    try {
+        if (!OptionSpace::validateName(space)) {
+            isc_throw(DhcpConfigError, "invalid option space name '"
+                      << space << "'");
+        }
+
+        if ((space == DHCP4_OPTION_SPACE) && (address_family_ == AF_INET6)) {
+            isc_throw(DhcpConfigError, "'" << DHCP4_OPTION_SPACE
+                      << "' option space name is reserved for DHCPv4 server");
+
+        } else if ((space == DHCP6_OPTION_SPACE) &&
+                   (address_family_ == AF_INET)) {
+            isc_throw(DhcpConfigError, "'" << DHCP6_OPTION_SPACE
+                      << "' option space name is reserved for DHCPv6 server");
+        }
+
+    } catch (std::exception& ex) {
+        // Append position of the option space parameter. Note, that in the case
+        // when 'space' was not specified a default value will be used and we
+        // should never get here. Therefore, it is ok to call getPosition for
+        // the space parameter here as this parameter will always be specified.
+        isc_throw(DhcpConfigError, ex.what() << " ("
+                  << string_values_->getPosition("space") << ")");
+    }
+
+    return (space);
+}
+
+template<typename SearchKey>
+OptionDefinitionPtr
+OptionDataParser::findOptionDefinition(const std::string& option_space,
+                                       const SearchKey& search_key) const {
+    const Option::Universe u = address_family_ == AF_INET ?
+        Option::V4 : Option::V6;
+    OptionDefinitionPtr def;
+
+    if ((option_space == DHCP4_OPTION_SPACE) ||
+        (option_space == DHCP6_OPTION_SPACE)) {
+        def = LibDHCP::getOptionDef(u, search_key);
+
+    }
+
+    if (!def) {
+        // Check if this is a vendor-option. If it is, get vendor-specific
+        // definition.
+        uint32_t vendor_id = CfgOption::optionSpaceToVendorId(option_space);
+        if (vendor_id) {
+            def = LibDHCP::getVendorOptionDef(u, vendor_id, search_key);
+        }
     }
+
     if (!def) {
-        // If we are not dealing with a standard option then we
-        // need to search for its definition among user-configured
-        // options. They are expected to be in the global storage
-        // already.
-        def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()->get(space, code);
-
-        // It's ok if we don't have option format if the option is
-        // specified as hex
-        if (!def && csv_format) {
+        // Check if this is an option specified by a user.
+        def = CfgMgr::instance().getStagingCfg()->getCfgOptionDef()
+            ->get(option_space, search_key);
+    }
+
+    return (def);
+}
+
+void
+OptionDataParser::createOption(ConstElementPtr option_data) {
+    const Option::Universe universe = address_family_ == AF_INET ?
+        Option::V4 : Option::V6;
+
+    OptionalValue<uint32_t> code_param =  extractCode(option_data);
+    OptionalValue<std::string> name_param = extractName(option_data);
+    OptionalValue<bool> csv_format_param = extractCSVFormat();
+    std::string data_param = extractData();
+    std::string space_param = extractSpace();
+
+    // Require that option code or option name is specified.
+    if (!code_param.isSpecified() && !name_param.isSpecified()) {
+        isc_throw(DhcpConfigError, "option data configuration requires one of"
+                  " 'code' or 'name' parameters to be specified"
+                  << " (" << option_data->getPosition() << ")");
+    }
+
+    // Try to find a corresponding option definition using option code or
+    // option name.
+    OptionDefinitionPtr def = code_param.isSpecified() ?
+        findOptionDefinition(space_param, code_param) :
+        findOptionDefinition(space_param, name_param);
+
+    // If there is no definition, the user must not explicitly enable the
+    // use of csv-format.
+    if (!def) {
+        // If explicitly requested that the CSV format is to be used,
+        // the option definition is a must.
+        if (csv_format_param.isSpecified() && csv_format_param) {
             isc_throw(DhcpConfigError, "definition for the option '"
-                      << space << "." << name
-                      << "' having code '" << code
+                      << space_param << "." << name_param
+                      << "' having code '" << code_param
                       << "' does not exist ("
-                      << string_values_->getPosition("name") << ")");
+                      << string_values_->getPosition("name", option_data)
+                      << ")");
+
+        // If there is no option definition and the option code is not specified
+        // we have no means to find the option code.
+        } else if (name_param.isSpecified() && !code_param.isSpecified()) {
+            isc_throw(DhcpConfigError, "definition for the option '"
+                      << space_param << "." << name_param
+                      << " does not exist ("
+                      << string_values_->getPosition("name", option_data));
         }
     }
 
@@ -471,12 +547,15 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
     std::vector<uint8_t> binary;
     std::vector<std::string> data_tokens;
 
-    if (csv_format) {
+    // If the definition is available and csv-format hasn't been explicitly
+    // disabled, we will parse the data as comma separated values.
+    if (def && (!csv_format_param.isSpecified() || csv_format_param)) {
         // If the option data is specified as a string of comma
         // separated values then we need to split this string into
         // individual values - each value will be used to initialize
         // one data field of an option.
-        data_tokens = isc::util::str::tokens(data, ",");
+        data_tokens = isc::util::str::tokens(data_param, ",");
+
     } else {
         // Otherwise, the option data is specified as a string of
         // hexadecimal digits that we have to turn into binary format.
@@ -484,76 +563,70 @@ OptionDataParser::createOption(ConstElementPtr option_data) {
             // The decodeHex function expects that the string contains an
             // even number of digits. If we don't meet this requirement,
             // we have to insert a leading 0.
-            if (!data.empty() && data.length() % 2) {
-                data = data.insert(0, "0");
+            if (!data_param.empty() && data_param.length() % 2) {
+                data_param = data_param.insert(0, "0");
             }
-            util::encode::decodeHex(data, binary);
+            util::encode::decodeHex(data_param, binary);
         } catch (...) {
             isc_throw(DhcpConfigError, "option data is not a valid"
-                      << " string of hexadecimal digits: " << data
-                      << " (" << string_values_->getPosition("data") << ")");
+                      << " string of hexadecimal digits: " << data_param
+                      << " ("
+                      << string_values_->getPosition("data", option_data)
+                      << ")");
         }
     }
 
     OptionPtr option;
     if (!def) {
-        if (csv_format) {
-            isc_throw(DhcpConfigError, "the CSV option data format can be"
-                      " used to specify values for an option that has a"
-                      " definition. The option with code " << code
-                      << " does not have a definition ("
-                      << boolean_values_->getPosition("csv-format") << ")");
-        }
-
         // @todo We have a limited set of option definitions initalized at
         // the moment.  In the future we want to initialize option definitions
         // for all options.  Consequently an error will be issued if an option
         // definition does not exist for a particular option code. For now it is
         // ok to create generic option if definition does not exist.
-        OptionPtr option(new Option(universe,
-                                    static_cast<uint16_t>(code), binary));
+        OptionPtr option(new Option(universe, static_cast<uint16_t>(code_param),
+                                    binary));
         // The created option is stored in option_descriptor_ class member
         // until the commit stage when it is inserted into the main storage.
         // If an option with the same code exists in main storage already the
         // old option is replaced.
         option_descriptor_.option_ = option;
         option_descriptor_.persistent_ = false;
+
     } else {
 
-        // Option name should match the definition. The option name
-        // may seem to be redundant but in the future we may want
-        // to reference options and definitions using their names
-        // and/or option codes so keeping the option name in the
-        // definition of option value makes sense.
-        if (def->getName() != name) {
+        // Option name is specified it should match the name in the definition.
+        if (name_param.isSpecified() && (def->getName() != name_param.get())) {
             isc_throw(DhcpConfigError, "specified option name '"
-                      << name << "' does not match the "
-                      << "option definition: '" << space
+                      << name_param << "' does not match the "
+                      << "option definition: '" << space_param
                       << "." << def->getName() << "' ("
-                      << string_values_->getPosition("name") << ")");
+                      << string_values_->getPosition("name", option_data)
+                      << ")");
         }
 
         // Option definition has been found so let's use it to create
         // an instance of our option.
         try {
-            OptionPtr option = csv_format ?
-                def->optionFactory(universe, code, data_tokens) :
-                def->optionFactory(universe, code, binary);
+            OptionPtr option =
+                !csv_format_param.isSpecified() || csv_format_param ?
+                def->optionFactory(universe, def->getCode(), data_tokens) :
+                def->optionFactory(universe, def->getCode(), binary);
             OptionDescriptor desc(option, false);
             option_descriptor_.option_ = option;
             option_descriptor_.persistent_ = false;
 
         } catch (const isc::Exception& ex) {
             isc_throw(DhcpConfigError, "option data does not match"
-                      << " option definition (space: " << space
-                      << ", code: " << code << "): "
+                      << " option definition (space: " << space_param
+                      << ", code: " << def->getCode() << "): "
                       << ex.what() << " ("
-                      << string_values_->getPosition("data") << ")");
+                      << string_values_->getPosition("data", option_data)
+                      << ")");
         }
     }
 
     // All went good, so we can set the option space name.
-    option_space_ = space;
+    option_space_ = space_param;
 }
 
 // **************************** OptionDataListParser *************************

+ 61 - 11
src/lib/dhcpsrv/dhcp_parsers.h

@@ -25,6 +25,7 @@
 #include <dhcpsrv/option_space_container.h>
 #include <dhcpsrv/subnet.h>
 #include <exceptions/exceptions.h>
+#include <util/optional_value.h>
 
 #include <boost/shared_ptr.hpp>
 
@@ -109,15 +110,20 @@ public:
     /// element holding a particular value is located.
     ///
     /// @param name is the name of the parameter which position is desired.
+    /// @param parent Pointer to a data element which position should be
+    /// returned when position of the specified parameter is not found.
     ///
     /// @return Position of the data element or the position holding empty
     /// file name and two zeros if the position hasn't been specified for the
     /// particular value.
-    const data::Element::Position& getPosition(const std::string& name) const {
+    const data::Element::Position&
+    getPosition(const std::string& name, const data::ConstElementPtr parent =
+                data::ConstElementPtr()) const {
         typename std::map<std::string, data::Element::Position>::const_iterator
             pos = positions_.find(name);
         if (pos == positions_.end()) {
-            return (data::Element::ZERO_POSITION());
+            return (parent ? parent->getPosition() :
+                    data::Element::ZERO_POSITION());
         }
 
         return (pos->second);
@@ -538,24 +544,26 @@ public:
 
     /// @brief virtual destructor to ensure orderly destruction of derivations.
     virtual ~OptionDataParser(){};
+private:
 
-protected:
-    /// @brief Finds an option definition within the server's option space
+    /// @brief Finds an option definition within an option space
     ///
     /// Given an option space and an option code, find the correpsonding
-    /// option defintion within the server's option defintion storage.
+    /// option defintion within the option defintion storage.
     ///
     /// @param option_space name of the parameter option space
-    /// @param option_code numeric value of the parameter to find
+    /// @param search_key an option code or name to be used to lookup the
+    /// option definition.
+    /// @tparam A numeric type for searching using an option code or the
+    /// string for searching using the option name.
+    ///
     /// @return OptionDefintionPtr of the option defintion or an
     /// empty OptionDefinitionPtr if not found.
     /// @throw DhcpConfigError if the option space requested is not valid
     /// for this server.
-    virtual OptionDefinitionPtr
-    findServerSpaceOptionDefinition(const std::string& option_space,
-                                    const uint32_t option_code) const;
-
-private:
+    template<typename SearchKey>
+    OptionDefinitionPtr findOptionDefinition(const std::string& option_space,
+                                             const SearchKey& search_key) const;
 
     /// @brief Create option instance.
     ///
@@ -574,6 +582,48 @@ private:
     /// are invalid.
     void createOption(isc::data::ConstElementPtr option_data);
 
+    /// @brief Retrieves parsed option code as an optional value.
+    ///
+    /// @param parent A data element holding full option data configuration.
+    /// It is used here to log a position if the element holding a code
+    /// is not specified and its position is therefore unavailable.
+    ///
+    /// @return Option code, possibly unspecified.
+    /// @throw DhcpConfigError if option code is invalid.
+    util::OptionalValue<uint32_t>
+    extractCode(data::ConstElementPtr parent) const;
+
+    /// @brief Retrieves parsed option name as an optional value.
+    ///
+    /// @param parent A data element holding full option data configuration.
+    /// It is used here to log a position if the element holding a name
+    /// is not specified and its position is therefore unavailable.
+    ///
+    /// @return Option name, possibly unspecified.
+    /// @throw DhcpConfigError if option name is invalid.
+    util::OptionalValue<std::string>
+    extractName(data::ConstElementPtr parent) const;
+
+    /// @brief Retrieves csv-format parameter as an optional value.
+    ///
+    /// @return Value of the csv-format parameter, possibly unspecified.
+    util::OptionalValue<bool> extractCSVFormat() const;
+
+    /// @brief Retrieves option data as a string.
+    ///
+    /// @return Option data as a string. It will return empty string if
+    /// option data is unspecified.
+    std::string extractData() const;
+
+    /// @brief Retrieves option space name.
+    ///
+    /// If option space name is not specified in the configuration the
+    /// 'dhcp4' or 'dhcp6' option space name is returned, depending on
+    /// the universe specified in the parser context.
+    ///
+    /// @return Option space name.
+    std::string extractSpace() const;
+
     /// Storage for boolean values.
     BooleanStoragePtr boolean_values_;
 

+ 12 - 1
src/lib/dhcpsrv/tests/cfg_option_def_unittest.cc

@@ -122,7 +122,7 @@ TEST(CfgOptionDefTest, getAll) {
 }
 
 // This test verifies that single option definition is correctly
-// returned with getOptionDef function.
+// returned with get function.
 TEST(CfgOptionDefTest, get) {
     CfgOptionDef cfg;
     // Create a set of option definitions with codes between 100 and 109.
@@ -160,6 +160,12 @@ TEST(CfgOptionDefTest, get) {
         option_name << "option-" << code;
         EXPECT_EQ(option_name.str(), def->getName());
         EXPECT_EQ(code, def->getCode());
+
+        // Try to get the same option definition using an option name as
+        // a key.
+        def = cfg.get("isc", option_name.str());
+        ASSERT_TRUE(def);
+        EXPECT_EQ(code, def->getCode());
     }
 
     // Check that the option codes are valid.
@@ -172,7 +178,12 @@ TEST(CfgOptionDefTest, get) {
         std::ostringstream option_name;
         option_name << "option-other-" << code;
         EXPECT_EQ(option_name.str(), def->getName());
+        EXPECT_EQ(code, def->getCode());
 
+        // Try to get the same option definition using an option name as
+        // a key.
+        def = cfg.get("abcde", option_name.str());
+        ASSERT_TRUE(def);
         EXPECT_EQ(code, def->getCode());
     }
 

+ 287 - 1
src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -18,6 +18,7 @@
 #include <dhcp/option.h>
 #include <dhcp/option_custom.h>
 #include <dhcp/option_int.h>
+#include <dhcp/option6_addrlst.h>
 #include <dhcp/tests/iface_mgr_test_config.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/subnet.h>
@@ -361,9 +362,11 @@ public:
     /// @throw throws NotImplemented if element name isn't supported.
     ParserPtr createConfigParser(const std::string& config_id) {
         ParserPtr parser;
+        int family = parser_context_->universe_ == Option::V4 ?
+            AF_INET : AF_INET6;
         if (config_id.compare("option-data") == 0) {
             parser.reset(new OptionDataListParser(config_id, CfgOptionPtr(),
-                                                  AF_INET));
+                                                  family));
 
         } else if (config_id.compare("option-def") == 0) {
             parser.reset(new OptionDefListParser(config_id,
@@ -551,6 +554,289 @@ TEST_F(ParseConfigTest, basicOptionDataTest) {
     EXPECT_EQ(val, opt_ptr->toText());
 }
 
+// This test checks behavior of the configuration parser for option data
+// for different values of csv-format parameter and when there is an option
+// definition present.
+TEST_F(ParseConfigTest, optionDataCSVFormatWithOptionDef) {
+    std::string config =
+        "{ \"option-data\": [ {"
+        "    \"name\": \"swap-server\","
+        "    \"space\": \"dhcp4\","
+        "    \"code\": 16,"
+        "    \"data\": \"192.0.2.0\""
+        " } ]"
+        "}";
+
+    // The default universe is V6. We need to change it to use dhcp4 option
+    // space.
+    parser_context_->universe_ = Option::V4;
+    int rcode = 0;
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_EQ(0, rcode);
+
+    // Verify that the option data is correct.
+    OptionCustomPtr addr_opt = boost::dynamic_pointer_cast<
+        OptionCustom>(getOptionPtr("dhcp4", 16));
+    ASSERT_TRUE(addr_opt);
+    EXPECT_EQ("192.0.2.0", addr_opt->readAddress().toText());
+
+    // Explicitly enable csv-format.
+    CfgMgr::instance().clear();
+    config =
+        "{ \"option-data\": [ {"
+        "    \"name\": \"swap-server\","
+        "    \"space\": \"dhcp4\","
+        "    \"code\": 16,"
+        "    \"csv-format\": True,"
+        "    \"data\": \"192.0.2.0\""
+        " } ]"
+        "}";
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_EQ(0, rcode);
+
+    // Verify that the option data is correct.
+    addr_opt = boost::dynamic_pointer_cast<
+        OptionCustom>(getOptionPtr("dhcp4", 16));
+    ASSERT_TRUE(addr_opt);
+    EXPECT_EQ("192.0.2.0", addr_opt->readAddress().toText());
+
+    // Explicitly disable csv-format and use hex instead.
+    CfgMgr::instance().clear();
+    config =
+        "{ \"option-data\": [ {"
+        "    \"name\": \"swap-server\","
+        "    \"space\": \"dhcp4\","
+        "    \"code\": 16,"
+        "    \"csv-format\": False,"
+        "    \"data\": \"C0000200\""
+        " } ]"
+        "}";
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_EQ(0, rcode);
+
+    // Verify that the option data is correct.
+    addr_opt = boost::dynamic_pointer_cast<
+        OptionCustom>(getOptionPtr("dhcp4", 16));
+    ASSERT_TRUE(addr_opt);
+    EXPECT_EQ("192.0.2.0", addr_opt->readAddress().toText());
+}
+
+// This test checks behavior of the configuration parser for option data
+// for different values of csv-format parameter and when there is no
+// option definition.
+TEST_F(ParseConfigTest, optionDataCSVFormatNoOptionDef) {
+    // This option doesn't have any definition. It is ok to use such
+    // an option but the data should be specified in hex, not as CSV.
+    // Note that the parser will by default use the CSV format for the
+    // data but only in case there is a suitable option definition.
+    std::string config =
+        "{ \"option-data\": [ {"
+        "    \"name\": \"foo-name\","
+        "    \"space\": \"dhcp6\","
+        "    \"code\": 25000,"
+        "    \"data\": \"1, 2, 5\""
+        " } ]"
+        "}";
+    int rcode = 0;
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    EXPECT_NE(0, rcode);
+
+    CfgMgr::instance().clear();
+    // The data specified here will work both for CSV format and hex format.
+    // What we want to test here is that when the csv-format is enforced, the
+    // parser will fail because of lack of an option definition.
+    config =
+        "{ \"option-data\": [ {"
+        "    \"name\": \"foo-name\","
+        "    \"space\": \"dhcp6\","
+        "    \"code\": 25000,"
+        "    \"csv-format\": True,"
+        "    \"data\": \"0\""
+        " } ]"
+        "}";
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    EXPECT_NE(0, rcode);
+
+    CfgMgr::instance().clear();
+    // The same test case as above, but for the data specified in hex should
+    // be successful.
+    config =
+        "{ \"option-data\": [ {"
+        "    \"name\": \"foo-name\","
+        "    \"space\": \"dhcp6\","
+        "    \"code\": 25000,"
+        "    \"csv-format\": False,"
+        "    \"data\": \"0\""
+        " } ]"
+        "}";
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    ASSERT_EQ(0, rcode);
+    OptionPtr opt = getOptionPtr("dhcp6", 25000);
+    ASSERT_TRUE(opt);
+    ASSERT_EQ(1, opt->getData().size());
+    EXPECT_EQ(0, opt->getData()[0]);
+
+    CfgMgr::instance().clear();
+    // When csv-format is not specified, the parser will check if the definition
+    // exists or not. Since there is no definition, the parser will accept the
+    // data in hex.
+    config =
+        "{ \"option-data\": [ {"
+        "    \"name\": \"foo-name\","
+        "    \"space\": \"dhcp6\","
+        "    \"code\": 25000,"
+        "    \"data\": \"123456\""
+        " } ]"
+        "}";
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    EXPECT_EQ(0, rcode);
+    opt = getOptionPtr("dhcp6", 25000);
+    ASSERT_TRUE(opt);
+    ASSERT_EQ(3, opt->getData().size());
+    EXPECT_EQ(0x12, opt->getData()[0]);
+    EXPECT_EQ(0x34, opt->getData()[1]);
+    EXPECT_EQ(0x56, opt->getData()[2]);
+}
+
+// This test verifies that the option name is not mandatory, if the option
+// code has been specified.
+TEST_F(ParseConfigTest, optionDataNoName) {
+    std::string config =
+        "{ \"option-data\": [ {"
+        "    \"space\": \"dhcp6\","
+        "    \"code\": 23,"
+        "    \"csv-format\": True,"
+        "    \"data\": \"2001:db8:1::1\""
+        " } ]"
+        "}";
+    int rcode = 0;
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    EXPECT_EQ(0, rcode);
+    Option6AddrLstPtr opt = boost::dynamic_pointer_cast<
+        Option6AddrLst>(getOptionPtr("dhcp6", 23));
+    ASSERT_TRUE(opt);
+    ASSERT_EQ(1, opt->getAddresses().size());
+    EXPECT_EQ( "2001:db8:1::1", opt->getAddresses()[0].toText());
+}
+
+// This test verifies that the option code is not mandatory, if the option
+// name has been specified.
+TEST_F(ParseConfigTest, optionDataNoCode) {
+    std::string config =
+        "{ \"option-data\": [ {"
+        "    \"space\": \"dhcp6\","
+        "    \"name\": \"dns-servers\","
+        "    \"csv-format\": True,"
+        "    \"data\": \"2001:db8:1::1\""
+        " } ]"
+        "}";
+    int rcode = 0;
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    EXPECT_EQ(0, rcode);
+    Option6AddrLstPtr opt = boost::dynamic_pointer_cast<
+        Option6AddrLst>(getOptionPtr("dhcp6", 23));
+    ASSERT_TRUE(opt);
+    ASSERT_EQ(1, opt->getAddresses().size());
+    EXPECT_EQ( "2001:db8:1::1", opt->getAddresses()[0].toText());
+}
+
+// This test verifies that the option data configuration with a minimal
+// set of parameters works as expected.
+TEST_F(ParseConfigTest, optionDataMinimal) {
+    std::string config =
+        "{ \"option-data\": [ {"
+        "    \"name\": \"dns-servers\","
+        "    \"data\": \"2001:db8:1::10\""
+        " } ]"
+        "}";
+    int rcode = 0;
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    EXPECT_EQ(0, rcode);
+    Option6AddrLstPtr opt = boost::dynamic_pointer_cast<
+        Option6AddrLst>(getOptionPtr("dhcp6", 23));
+    ASSERT_TRUE(opt);
+    ASSERT_EQ(1, opt->getAddresses().size());
+    EXPECT_EQ( "2001:db8:1::10", opt->getAddresses()[0].toText());
+
+    CfgMgr::instance().clear();
+    // This time using an option code.
+    config =
+        "{ \"option-data\": [ {"
+        "    \"code\": 23,"
+        "    \"data\": \"2001:db8:1::20\""
+        " } ]"
+        "}";
+    rcode = 0;
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    EXPECT_EQ(0, rcode);
+    opt = boost::dynamic_pointer_cast<Option6AddrLst>(getOptionPtr("dhcp6",
+                                                                   23));
+    ASSERT_TRUE(opt);
+    ASSERT_EQ(1, opt->getAddresses().size());
+    EXPECT_EQ( "2001:db8:1::20", opt->getAddresses()[0].toText());
+}
+
+// This test verifies that the option data configuration with a minimal
+// set of parameters works as expected when option definition is
+// created in the configruation file.
+TEST_F(ParseConfigTest, optionDataMinimalWithOptionDef) {
+    // Configuration string.
+    std::string config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo-name\","
+        "      \"code\": 2345,"
+        "      \"type\": \"ipv6-address\","
+        "      \"array\": True,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"dhcp6\","
+        "      \"encapsulate\": \"\""
+        "  } ],"
+        "  \"option-data\": [ {"
+        "    \"name\": \"foo-name\","
+        "    \"data\": \"2001:db8:1::10, 2001:db8:1::123\""
+        " } ]"
+        "}";
+
+    int rcode = 0;
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    EXPECT_EQ(0, rcode);
+    Option6AddrLstPtr opt = boost::dynamic_pointer_cast<
+        Option6AddrLst>(getOptionPtr("dhcp6", 2345));
+    ASSERT_TRUE(opt);
+    ASSERT_EQ(2, opt->getAddresses().size());
+    EXPECT_EQ("2001:db8:1::10", opt->getAddresses()[0].toText());
+    EXPECT_EQ("2001:db8:1::123", opt->getAddresses()[1].toText());
+
+    CfgMgr::instance().clear();
+    // Do the same test but now use an option code.
+    config =
+        "{ \"option-def\": [ {"
+        "      \"name\": \"foo-name\","
+        "      \"code\": 2345,"
+        "      \"type\": \"ipv6-address\","
+        "      \"array\": True,"
+        "      \"record-types\": \"\","
+        "      \"space\": \"dhcp6\","
+        "      \"encapsulate\": \"\""
+        "  } ],"
+        "  \"option-data\": [ {"
+        "    \"code\": 2345,"
+        "    \"data\": \"2001:db8:1::10, 2001:db8:1::123\""
+        " } ]"
+        "}";
+
+    rcode = 0;
+    ASSERT_NO_THROW(rcode = parseConfiguration(config));
+    EXPECT_EQ(0, rcode);
+    opt = boost::dynamic_pointer_cast<Option6AddrLst>(getOptionPtr("dhcp6",
+                                                                   2345));
+    ASSERT_TRUE(opt);
+    ASSERT_EQ(2, opt->getAddresses().size());
+    EXPECT_EQ("2001:db8:1::10", opt->getAddresses()[0].toText());
+    EXPECT_EQ("2001:db8:1::123", opt->getAddresses()[1].toText());
+
+}
+
 };  // Anonymous namespace
 
 /// These tests check basic operation of the HooksLibrariesParser.

+ 56 - 5
src/lib/util/optional_value.h

@@ -15,9 +15,29 @@
 #ifndef OPTIONAL_VALUE_H
 #define OPTIONAL_VALUE_H
 
+#include <ostream>
+
 namespace isc {
 namespace util {
 
+/// @brief Indicate if an @c OptionalValue is is specified or not.
+///
+/// This is a simple wrapper class which holds a boolean value to indicate
+/// if the @c OptionalValue is specified or not. By using this class in the
+/// @c OptionalValue class constructor we avoid the ambiguity when the
+/// @c OptionalValue encapsulates a bool type.
+struct OptionalValueState {
+
+    /// @brief Constructor.
+    ///
+    /// @param specified A boolean value to be assigned.
+    OptionalValueState(const bool specified)
+        : specified_(specified) {
+    }
+    /// @brief A bool value encapsulated by this structure.
+    bool specified_;
+};
+
 /// @brief Simple class representing an optional value.
 ///
 /// This template class encapsulates a value of any type. An additional flag
@@ -43,16 +63,24 @@ template<typename T>
 class OptionalValue {
 public:
 
+    /// @brief Default constructor.
+    ///
+    /// Note that the type @c T must have a default constructor to use this
+    /// constructor.
+    OptionalValue()
+        : value_(T()), specified_(false) {
+    }
+
     /// @brief Constructor
     ///
     /// Creates optional value. The value defaults to "unspecified".
     ///
     /// @param value Default explicit value.
-    /// @param specified Boolean value which determines if the value is
-    /// initially specified or not (default is false).
-    OptionalValue(const T& value, const bool specified = false)
-        : value_(value),
-          specified_(specified) {
+    /// @param specified Value which determines if the value is initially
+    // specified or not (default is false).
+    explicit OptionalValue(const T& value, const OptionalValueState& state =
+                           OptionalValueState(false))
+        : value_(value), specified_(state.specified_) {
     }
 
     /// @brief Retrieves the actual value.
@@ -75,6 +103,10 @@ public:
         specify(true);
     }
 
+    void specify(const OptionalValueState& state) {
+        specified_ = state.specified_;
+    }
+
     /// @brief Sets the value to "specified" or "unspecified".
     ///
     /// It does not alter the actual value. It only marks it "specified" or
@@ -131,6 +163,25 @@ private:
     bool specified_;  ///< Flag which indicates if the value is specified.
 };
 
+/// @brief Inserts an optional value to a stream.
+///
+/// This function overloads the global operator<< to behave as in
+/// @c ostream::operator<< but applied to @c OptionalValue objects.
+///
+/// @param os A @c std::ostream object to which the value is inserted.
+/// @param optional_value An @c OptionalValue object to be inserted into
+/// a stream.
+/// @tparam Type of the value encapsulated by the @c OptionalValue object.
+///
+/// @return A reference to the stream after insertion.
+template<typename T>
+std::ostream&
+operator<<(std::ostream& os, const OptionalValue<T>& optional_value) {
+    os << optional_value.get();
+    return (os);
+}
+
+
 } // end of namespace isc::util
 } // end of namespace isc