Browse Source

[master] Merge branch 'trac3292'

Marcin Siodelski 11 years ago
parent
commit
7c4c0514ed

+ 38 - 16
doc/guide/bind10-guide.xml

@@ -7,7 +7,7 @@
 ]>
 
 <!--
- - Copyright (C) 2010-2013  Internet Systems Consortium, Inc. ("ISC")
+ - Copyright (C) 2010-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
@@ -3928,7 +3928,7 @@ Dhcp4/subnet4	[]	list	(default)
       <para>
         The following commands override the global
         DNS servers option for a particular subnet, setting a single DNS
-        server with address 2001:db8:1::3.
+        server with address 192.0.2.3.
         <screen>
 &gt; <userinput>config add Dhcp4/subnet4[0]/option-data</userinput>
 &gt; <userinput>config set Dhcp4/subnet4[0]/option-data[0]/name "domain-name-servers"</userinput>
@@ -4172,10 +4172,10 @@ Dhcp4/subnet4	[]	list	(default)
       primitives (uint8, string, ipv4-address etc): it is possible to
       define an option comprising a number of existing primitives.
       </para>
-      <para>Assume we
-      want to define a new option that will consist of an IPv4
-      address, followed by unsigned 16 bit integer, followed by a text
-      string. Such an option could be defined in the following way:
+      <para>Assume we want to define a new option that will consist of
+      an IPv4 address, followed by unsigned 16 bit integer, followed by
+      a boolean value, followed by a text string. Such an option could
+      be defined in the following way:
 <screen>
 &gt; <userinput>config add Dhcp4/option-def</userinput>
 &gt; <userinput>config set Dhcp4/option-def[0]/name "bar"</userinput>
@@ -4183,7 +4183,7 @@ Dhcp4/subnet4	[]	list	(default)
 &gt; <userinput>config set Dhcp4/option-def[0]/space "dhcp4"</userinput>
 &gt; <userinput>config set Dhcp4/option-def[0]/type "record"</userinput>
 &gt; <userinput>config set Dhcp4/option-def[0]/array false</userinput>
-&gt; <userinput>config set Dhcp4/option-def[0]/record-types "ipv4-address, uint16, string"</userinput>
+&gt; <userinput>config set Dhcp4/option-def[0]/record-types "ipv4-address, uint16, boolean, string"</userinput>
 &gt; <userinput>config set Dhcp4/option-def[0]/encapsulate ""</userinput>
 </screen>
       The "type" is set to "record" to indicate that the option contains
@@ -4198,12 +4198,23 @@ Dhcp4/subnet4	[]	list	(default)
 &gt; <userinput>config set Dhcp4/option-data[0]/space "dhcp4"</userinput>
 &gt; <userinput>config set Dhcp4/option-data[0]/code 223</userinput>
 &gt; <userinput>config set Dhcp4/option-data[0]/csv-format true</userinput>
-&gt; <userinput>config set Dhcp4/option-data[0]/data "192.0.2.100, 123, Hello World"</userinput>
+&gt; <userinput>config set Dhcp4/option-data[0]/data "192.0.2.100, 123, true, Hello World"</userinput>
 &gt; <userinput>config commit</userinput></screen>
-      </para>
       "csv-format" is set "true" to indicate that the "data" field comprises a command-separated
       list of values.  The values in the "data" must correspond to the types set in
       the "record-types" field of the option definition.
+     </para>
+     <note>
+       <para>
+         It is recommended that boolean values are specified using "true" and "false"
+         strings. This helps to prevent errors when typing multiple comma separated
+         values, as it make it easier to identify the type of the value being typed,
+         and compare it with the order of data fields. Nevetheless, it is possible
+         to use integer values: "1" and "0", instead of "true" and "false"
+         accordingly. If other integer value is specified, the configuration is
+         rejected.
+       </para>
+     </note>
     </section>
 
     <section id="dhcp4-vendor-opts">
@@ -5067,10 +5078,10 @@ Dhcp6/subnet6/	list
       define an option comprising a number of existing primitives.
       </para>
       <para>
-      Assume we
-      want to define a new option that will consist of an IPv6
-      address, followed by unsigned 16 bit integer, followed by a text
-      string. Such an option could be defined in the following way:
+      Assume we want to define a new option that will consist of an IPv6
+      address, followed by an unsigned 16 bit integer, followed by a
+      boolean value, followed by a text string. Such an option could
+      be defined in the following way:
 <screen>
 &gt; <userinput>config add Dhcp6/option-def</userinput>
 &gt; <userinput>config set Dhcp6/option-def[0]/name "bar"</userinput>
@@ -5078,7 +5089,7 @@ Dhcp6/subnet6/	list
 &gt; <userinput>config set Dhcp6/option-def[0]/space "dhcp6"</userinput>
 &gt; <userinput>config set Dhcp6/option-def[0]/type "record"</userinput>
 &gt; <userinput>config set Dhcp6/option-def[0]/array false</userinput>
-&gt; <userinput>config set Dhcp6/option-def[0]/record-types "ipv6-address, uint16, string"</userinput>
+&gt; <userinput>config set Dhcp6/option-def[0]/record-types "ipv6-address, uint16, boolean, string"</userinput>
 &gt; <userinput>config set Dhcp6/option-def[0]/encapsulate ""</userinput>
 </screen>
       The "type" is set to "record" to indicate that the option contains
@@ -5093,12 +5104,23 @@ Dhcp6/subnet6/	list
 &gt; <userinput>config set Dhcp6/option-data[0]/space "dhcp6"</userinput>
 &gt; <userinput>config set Dhcp6/option-data[0]/code 101</userinput>
 &gt; <userinput>config set Dhcp6/option-data[0]/csv-format true</userinput>
-&gt; <userinput>config set Dhcp6/option-data[0]/data "2001:db8:1::10, 123, Hello World"</userinput>
+&gt; <userinput>config set Dhcp6/option-data[0]/data "2001:db8:1::10, 123, false, Hello World"</userinput>
 &gt; <userinput>config commit</userinput></screen>
-      </para>
       "csv-format" is set "true" to indicate that the "data" field comprises a command-separated
       list of values.  The values in the "data" must correspond to the types set in
       the "record-types" field of the option definition.
+      </para>
+      <note>
+        <para>
+          It is recommended that boolean values are specified using "true" and "false"
+          strings. This helps to prevent errors when typing multiple comma separated
+          values, as it make it easier to identify the type of the value being typed,
+          and compare it with the order of data fields. Nevetheless, it is possible
+          to use integer values: "1" and "0", instead of "true" and "false"
+          accordingly. If other integer value is specified, the configuration is
+          rejected.
+        </para>
+      </note>
     </section>
 
     <section id="dhcp6-vendor-opts">

+ 201 - 18
src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -137,19 +137,19 @@ public:
             params["name"] = param_value;
             params["space"] = "dhcp4";
             params["code"] = "56";
-            params["data"] = "AB CDEF0105";
+            params["data"] = "ABCDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "space") {
             params["name"] = "dhcp-message";
             params["space"] = param_value;
             params["code"] = "56";
-            params["data"] = "AB CDEF0105";
+            params["data"] = "ABCDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "code") {
             params["name"] = "dhcp-message";
             params["space"] = "dhcp4";
             params["code"] = param_value;
-            params["data"] = "AB CDEF0105";
+            params["data"] = "ABCDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "data") {
             params["name"] = "dhcp-message";
@@ -161,7 +161,7 @@ public:
             params["name"] = "dhcp-message";
             params["space"] = "dhcp4";
             params["code"] = "56";
-            params["data"] = "AB CDEF0105";
+            params["data"] = "ABCDEF0105";
             params["csv-format"] = param_value;
         }
         return (createConfigWithOption(params));
@@ -212,6 +212,62 @@ public:
         return (stream.str());
     }
 
+    /// @brief Returns an option from the subnet.
+    ///
+    /// This function returns an option from a subnet to which the
+    /// specified subnet address belongs. The option is identified
+    /// by its code.
+    ///
+    /// @param subnet_address Address which belongs to the subnet from
+    /// which the option is to be returned.
+    /// @param option_code Code of the option to be returned.
+    /// @param expected_options_count Expected number of options in
+    /// the particular subnet.
+    ///
+    /// @return Descriptor of the option. If the descriptor holds a
+    /// NULL option pointer, it means that there was no such option
+    /// in the subnet.
+    Subnet::OptionDescriptor
+    getOptionFromSubnet(const IOAddress& subnet_address,
+                        const uint16_t option_code,
+                        const uint16_t expected_options_count = 1) {
+        Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(subnet_address);
+        if (!subnet) {
+            /// @todo replace toText() with the use of operator <<.
+            ADD_FAILURE() << "A subnet for the specified address "
+                          << subnet_address.toText()
+                          << "does not exist in Config Manager";
+        }
+        Subnet::OptionContainerPtr options =
+            subnet->getOptionDescriptors("dhcp4");
+        if (expected_options_count != options->size()) {
+            ADD_FAILURE() << "The number of options in the subnet '"
+                          << subnet_address.toText() << "' is different "
+                " than expected number of options '"
+                          << expected_options_count << "'";
+        }
+
+        // Get the search index. Index #1 is to search using option code.
+        const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
+
+        // Get the options for specified index. Expecting one option to be
+        // returned but in theory we may have multiple options with the same
+        // code so we get the range.
+        std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+                  Subnet::OptionContainerTypeIndex::const_iterator> range =
+            idx.equal_range(option_code);
+        if (std::distance(range.first, range.second) > 1) {
+            ADD_FAILURE() << "There is more than one option having the"
+                " option code '" << option_code << "' in a subnet '"
+                          << subnet_address.toText() << "'. Expected "
+                " at most one option";
+        } else if (std::distance(range.first, range.second) == 0) {
+            return (Subnet::OptionDescriptor(OptionPtr(), false));
+        }
+
+        return (*range.first);
+    }
+
     /// @brief Test invalid option parameter value.
     ///
     /// This test function constructs the simple configuration
@@ -233,6 +289,24 @@ public:
         ASSERT_EQ(1, rcode_);
     }
 
+    /// @brief Test invalid option paramater value.
+    ///
+    /// This test function constructs the simple configuration
+    /// string and injects invalid option configuration into it.
+    /// It expects that parser will fail with provided option code.
+    ///
+    /// @param params Map of parameters defining an option.
+    void
+    testInvalidOptionParam(const std::map<std::string, std::string>& params) {
+        ConstElementPtr x;
+        std::string config = createConfigWithOption(params);
+        ElementPtr json = Element::fromJSON(config);
+        EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+        ASSERT_TRUE(x);
+        comment_ = parseAnswer(rcode_, x);
+        ASSERT_EQ(1, rcode_);
+    }
+
     /// @brief Test option against given code and data.
     ///
     /// @param option_desc option descriptor that carries the option to
@@ -274,6 +348,39 @@ public:
                             expected_data_len));
     }
 
+    /// @brief Test option configuration.
+    ///
+    /// This function creates a configuration for a specified option using
+    /// a map of parameters specified as the argument. The map holds
+    /// name/value pairs which identifies option's configuration parameters:
+    /// - name
+    /// - space
+    /// - code
+    /// - data
+    /// - csv-format.
+    /// This function applies a new server configuration and checks that the
+    /// option being configured is inserted into CfgMgr. The raw contents of
+    /// this option are compared with the binary data specified as expected
+    /// data passed to this function.
+    ///
+    /// @param params Map of parameters defining an option.
+    /// @param option_code Option code.
+    /// @param expected_data Array containing binary data expected to be stored
+    /// in the configured option.
+    /// @param expected_data_len Length of the array holding reference data.
+    void testConfiguration(const std::map<std::string, std::string>& params,
+                           const uint16_t option_code,
+                           const uint8_t* expected_data,
+                           const size_t expected_data_len) {
+        std::string config = createConfigWithOption(params);
+        ASSERT_TRUE(executeConfiguration(config, "parse option configuration"));
+        // The subnet should now hold one option with the specified option code.
+        Subnet::OptionDescriptor desc =
+            getOptionFromSubnet(IOAddress("192.0.2.24"), option_code);
+        ASSERT_TRUE(desc.option);
+        testOption(desc, option_code, expected_data, expected_data_len);
+    }
+
     /// @brief Parse and Execute configuration
     ///
     /// Parses a configuration and executes a configuration of the server.
@@ -1394,7 +1501,7 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) {
         "    \"name\": \"dhcp-message\","
         "    \"space\": \"dhcp4\","
         "    \"code\": 56,"
-        "    \"data\": \"AB CDEF0105\","
+        "    \"data\": \"ABCDEF0105\","
         "    \"csv-format\": False"
         " },"
         " {"
@@ -1467,7 +1574,7 @@ TEST_F(Dhcp4ParserTest, optionDataTwoSpaces) {
         "    \"name\": \"dhcp-message\","
         "    \"space\": \"dhcp4\","
         "    \"code\": 56,"
-        "    \"data\": \"AB CDEF0105\","
+        "    \"data\": \"ABCDEF0105\","
         "    \"csv-format\": False"
         " },"
         " {"
@@ -1644,7 +1751,6 @@ TEST_F(Dhcp4ParserTest, optionDataEncapsulate) {
         " } ]"
         "}";
 
-
     json = Element::fromJSON(config);
 
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
@@ -1700,7 +1806,7 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
         "          \"name\": \"dhcp-message\","
         "          \"space\": \"dhcp4\","
         "          \"code\": 56,"
-        "          \"data\": \"AB CDEF0105\","
+        "          \"data\": \"ABCDEF0105\","
         "          \"csv-format\": False"
         "        },"
         "        {"
@@ -1751,6 +1857,89 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
     testOption(*range.first, 23, foo2_expected, sizeof(foo2_expected));
 }
 
+// The goal of this test is to check that the option carrying a boolean
+// value can be configured using one of the values: "true", "false", "0"
+// or "1".
+TEST_F(Dhcp4ParserTest, optionDataBoolean) {
+    // Create configuration. Use standard option 19 (ip-forwarding).
+    std::map<std::string, std::string> params;
+    params["name"] = "ip-forwarding";
+    params["space"] = "dhcp4";
+    params["code"] = "19";
+    params["data"] = "true";
+    params["csv-format"] = "true";
+
+    std::string config = createConfigWithOption(params);
+    ASSERT_TRUE(executeConfiguration(config, "parse configuration with a"
+                                     " boolean value"));
+
+    // The subnet should now hold one option with the code 19.
+    Subnet::OptionDescriptor desc = getOptionFromSubnet(IOAddress("192.0.2.24"),
+                                                        19);
+    ASSERT_TRUE(desc.option);
+
+    // This option should be set to "true", represented as 0x1 in the option
+    // buffer.
+    uint8_t expected_option_data[] = {
+        0x1
+    };
+    testConfiguration(params, 19, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // Configure the option with the "1" value. This should have the same
+    // effect as if "true" was specified.
+    params["data"] = "1";
+    testConfiguration(params, 19, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // The value of "1" with a few leading zeros should work too.
+    params["data"] = "00001";
+    testConfiguration(params, 19, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // Configure the option with the "false" value.
+    params["data"] = "false";
+    // The option buffer should now hold the value of 0.
+    expected_option_data[0] = 0;
+    testConfiguration(params, 19, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // Specifying "0" should have the same effect as "false".
+    params["data"] = "0";
+    testConfiguration(params, 19, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // The same effect should be for multiple 0 chars.
+    params["data"] = "00000";
+    testConfiguration(params, 19, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // Bogus values should not be accepted.
+    params["data"] = "bugus";
+    testInvalidOptionParam(params);
+
+    params["data"] = "2";
+    testInvalidOptionParam(params);
+
+    // Now let's test that it is possible to use binary format.
+    params["data"] = "0";
+    params["csv-format"] = "false";
+    testConfiguration(params, 19, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // The binary 1 should work as well.
+    params["data"] = "1";
+    expected_option_data[0] = 1;
+    testConfiguration(params, 19, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // As well as an even number of digits.
+    params["data"] = "01";
+    testConfiguration(params, 19, expected_option_data,
+                      sizeof(expected_option_data));
+
+}
+
 // Goal of this test is to verify options configuration
 // for multiple subnets.
 TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
@@ -1828,6 +2017,8 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
     testOption(*range2.first, 23, foo2_expected, sizeof(foo2_expected));
 }
 
+
+
 // Verify that empty option name is rejected in the configuration.
 TEST_F(Dhcp4ParserTest, optionNameEmpty) {
     // Empty option names not allowed.
@@ -1878,14 +2069,6 @@ TEST_F(Dhcp4ParserTest, optionDataUnexpectedPrefix) {
     testInvalidOptionParam("0x0102", "data");
 }
 
-// Verify that option data consisting od an odd number of
-// hexadecimal digits is rejected in the configuration.
-TEST_F(Dhcp4ParserTest, optionDataOddLength) {
-    // Option code 0 is reserved and should not be accepted
-    // by configuration parser.
-    testInvalidOptionParam("123", "data");
-}
-
 // Verify that either lower or upper case characters are allowed
 // to specify the option data.
 TEST_F(Dhcp4ParserTest, optionDataLowerCase) {
@@ -2201,7 +2384,7 @@ TEST_F(Dhcp4ParserTest, vendorOptionsHex) {
         "    \"name\": \"option-one\","
         "    \"space\": \"vendor-4491\"," // VENDOR_ID_CABLE_LABS = 4491
         "    \"code\": 100," // just a random code
-        "    \"data\": \"AB CDEF0105\","
+        "    \"data\": \"ABCDEF0105\","
         "    \"csv-format\": False"
         " },"
         " {"
@@ -2333,7 +2516,7 @@ buildHooksLibrariesConfig(const std::vector<std::string>& libraries) {
         "    \"name\": \"dhcp-message\","
         "    \"space\": \"dhcp4\","
         "    \"code\": 56,"
-        "    \"data\": \"AB CDEF0105\","
+        "    \"data\": \"ABCDEF0105\","
         "    \"csv-format\": False"
         " },"
         " {"

+ 215 - 16
src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 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
@@ -138,19 +138,19 @@ public:
             params["name"] = param_value;
             params["space"] = "dhcp6";
             params["code"] = "38";
-            params["data"] = "AB CDEF0105";
+            params["data"] = "ABCDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "space") {
             params["name"] = "subscriber-id";
             params["space"] = param_value;
             params["code"] = "38";
-            params["data"] = "AB CDEF0105";
+            params["data"] = "ABCDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "code") {
             params["name"] = "subscriber-id";
             params["space"] = "dhcp6";
             params["code"] = param_value;
-            params["data"] = "AB CDEF0105";
+            params["data"] = "ABCDEF0105";
             params["csv-format"] = "False";
         } else if (parameter == "data") {
             params["name"] = "subscriber-id";
@@ -162,12 +162,20 @@ public:
             params["name"] = "subscriber-id";
             params["space"] = "dhcp6";
             params["code"] = "38";
-            params["data"] = "AB CDEF0105";
+            params["data"] = "ABCDEF0105";
             params["csv-format"] = param_value;
         }
         return (createConfigWithOption(params));
     }
 
+    /// @brief Create simple configuration with single option.
+    ///
+    /// This function creates a configuration for a single option with
+    /// custom values for all parameters that describe the option.
+    ///
+    /// @params params map holding parameters and their values.
+    /// @return configuration string containing custom values of parameters
+    /// describing an option.
     std::string createConfigWithOption(const std::map<std::string,
                                        std::string>& params)
     {
@@ -176,6 +184,15 @@ public:
             "\"preferred-lifetime\": 3000,"
             "\"rebind-timer\": 2000, "
             "\"renew-timer\": 1000, "
+            "\"option-def\": [ {"
+            "  \"name\": \"bool-option\","
+            "  \"code\": 1000,"
+            "  \"type\": \"boolean\","
+            "  \"array\": False,"
+            "  \"record-types\": \"\","
+            "  \"space\": \"dhcp6\","
+            "  \"encapsulate\": \"\""
+            "} ],"
             "\"subnet6\": [ { "
             "    \"pool\": [ \"2001:db8:1::/80\" ],"
             "    \"subnet\": \"2001:db8:1::/64\", "
@@ -208,6 +225,62 @@ public:
         return (stream.str());
     }
 
+    /// @brief Returns an option from the subnet.
+    ///
+    /// This function returns an option from a subnet to which the
+    /// specified subnet address belongs. The option is identified
+    /// by its code.
+    ///
+    /// @param subnet_address Address which belongs to the subnet from
+    /// which the option is to be returned.
+    /// @param option_code Code of the option to be returned.
+    /// @param expected_options_count Expected number of options in
+    /// the particular subnet.
+    ///
+    /// @return Descriptor of the option. If the descriptor holds a
+    /// NULL option pointer, it means that there was no such option
+    /// in the subnet.
+    Subnet::OptionDescriptor
+    getOptionFromSubnet(const IOAddress& subnet_address,
+                        const uint16_t option_code,
+                        const uint16_t expected_options_count = 1) {
+        Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(subnet_address);
+        if (!subnet) {
+            /// @todo replace toText() with the use of operator <<.
+            ADD_FAILURE() << "A subnet for the specified address "
+                          << subnet_address.toText()
+                          << "does not exist in Config Manager";
+        }
+        Subnet::OptionContainerPtr options =
+            subnet->getOptionDescriptors("dhcp6");
+        if (expected_options_count != options->size()) {
+            ADD_FAILURE() << "The number of options in the subnet '"
+                          << subnet_address.toText() << "' is different "
+                " than expected number of options '"
+                          << expected_options_count << "'";
+        }
+
+        // Get the search index. Index #1 is to search using option code.
+        const Subnet::OptionContainerTypeIndex& idx = options->get<1>();
+
+        // Get the options for specified index. Expecting one option to be
+        // returned but in theory we may have multiple options with the same
+        // code so we get the range.
+        std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+                  Subnet::OptionContainerTypeIndex::const_iterator> range =
+            idx.equal_range(option_code);
+        if (std::distance(range.first, range.second) > 1) {
+            ADD_FAILURE() << "There is more than one option having the"
+                " option code '" << option_code << "' in a subnet '"
+                          << subnet_address.toText() << "'. Expected "
+                " at most one option";
+        } else if (std::distance(range.first, range.second) == 0) {
+            return (Subnet::OptionDescriptor(OptionPtr(), false));
+        }
+
+        return (*range.first);
+    }
+
     /// @brief Parse and Execute configuration
     ///
     /// Parses a configuration and executes a configuration of the server.
@@ -305,6 +378,24 @@ public:
         ASSERT_EQ(1, rcode_);
     }
 
+    /// @brief Test invalid option paramater value.
+    ///
+    /// This test function constructs the simple configuration
+    /// string and injects invalid option configuration into it.
+    /// It expects that parser will fail with provided option code.
+    ///
+    /// @param params Map of parameters defining an option.
+    void
+    testInvalidOptionParam(const std::map<std::string, std::string>& params) {
+        ConstElementPtr x;
+        std::string config = createConfigWithOption(params);
+        ElementPtr json = Element::fromJSON(config);
+        EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
+        ASSERT_TRUE(x);
+        comment_ = parseAnswer(rcode_, x);
+        ASSERT_EQ(1, rcode_);
+    }
+
     /// @brief Test option against given code and data.
     ///
     /// @param option_desc option descriptor that carries the option to
@@ -346,6 +437,39 @@ public:
                             expected_data_len));
     }
 
+    /// @brief Test option configuration.
+    ///
+    /// This function creates a configuration for a specified option using
+    /// a map of parameters specified as the argument. The map holds
+    /// name/value pairs which identifies option's configuration parameters:
+    /// - name
+    /// - space
+    /// - code
+    /// - data
+    /// - csv-format.
+    /// This function applies a new server configuration and checks that the
+    /// option being configured is inserted into CfgMgr. The raw contents of
+    /// this option are compared with the binary data specified as expected
+    /// data passed to this function.
+    ///
+    /// @param params Map of parameters defining an option.
+    /// @param option_code Option code.
+    /// @param expected_data Array containing binary data expected to be stored
+    /// in the configured option.
+    /// @param expected_data_len Length of the array holding reference data.
+    void testConfiguration(const std::map<std::string, std::string>& params,
+                           const uint16_t option_code,
+                           const uint8_t* expected_data,
+                           const size_t expected_data_len) {
+        std::string config = createConfigWithOption(params);
+        ASSERT_TRUE(executeConfiguration(config, "parse option configuration"));
+        // The subnet should now hold one option with the specified code.
+        Subnet::OptionDescriptor desc =
+            getOptionFromSubnet(IOAddress("2001:db8:1::5"), option_code);
+        ASSERT_TRUE(desc.option);
+        testOption(desc, option_code, expected_data, expected_data_len);
+    }
+
     int rcode_;          ///< Return code (see @ref isc::config::parseAnswer)
     Dhcpv6Srv srv_;      ///< Instance of the Dhcp6Srv used during tests
     ConstElementPtr comment_; ///< Comment (see @ref isc::config::parseAnswer)
@@ -1699,7 +1823,7 @@ TEST_F(Dhcp6ParserTest, optionDataDefaults) {
         "    \"name\": \"subscriber-id\","
         "    \"space\": \"dhcp6\","
         "    \"code\": 38,"
-        "    \"data\": \"AB CDEF0105\","
+        "    \"data\": \"ABCDEF0105\","
         "    \"csv-format\": False"
         " },"
         " {"
@@ -1780,7 +1904,7 @@ TEST_F(Dhcp6ParserTest, optionDataTwoSpaces) {
         "    \"name\": \"subscriber-id\","
         "    \"space\": \"dhcp6\","
         "    \"code\": 38,"
-        "    \"data\": \"AB CDEF0105\","
+        "    \"data\": \"ABCDEF0105\","
         "    \"csv-format\": False"
         " },"
         " {"
@@ -2072,6 +2196,89 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
                sizeof(user_class_expected));
 }
 
+// The goal of this test is to check that the option carrying a boolean
+// value can be configured using one of the values: "true", "false", "0"
+// or "1".
+TEST_F(Dhcp6ParserTest, optionDataBoolean) {
+    // Create configuration. Use standard option 1000.
+    std::map<std::string, std::string> params;
+    params["name"] = "bool-option";
+    params["space"] = "dhcp6";
+    params["code"] = "1000";
+    params["data"] = "true";
+    params["csv-format"] = "true";
+
+    std::string config = createConfigWithOption(params);
+    ASSERT_TRUE(executeConfiguration(config, "parse configuration with a"
+                                     " boolean value"));
+
+    // The subnet should now hold one option with the code 1000.
+    Subnet::OptionDescriptor desc =
+        getOptionFromSubnet(IOAddress("2001:db8:1::5"), 1000);
+    ASSERT_TRUE(desc.option);
+
+    // This option should be set to "true", represented as 0x1 in the option
+    // buffer.
+    uint8_t expected_option_data[] = {
+        0x1
+    };
+    testConfiguration(params, 1000, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // Configure the option with the "1" value. This should have the same
+    // effect as if "true" was specified.
+    params["data"] = "1";
+    testConfiguration(params, 1000, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // The value of "1" with a few leading zeros should work too.
+    params["data"] = "00001";
+    testConfiguration(params, 1000, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // Configure the option with the "false" value.
+    params["data"] = "false";
+    // The option buffer should now hold the value of 0.
+    expected_option_data[0] = 0;
+    testConfiguration(params, 1000, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // Specifying "0" should have the same effect as "false".
+    params["data"] = "0";
+    testConfiguration(params, 1000, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // The same effect should be for multiple 0 chars.
+    params["data"] = "00000";
+    testConfiguration(params, 1000, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // Bogus values should not be accepted.
+    params["data"] = "bugus";
+    testInvalidOptionParam(params);
+
+    params["data"] = "2";
+    testInvalidOptionParam(params);
+
+    // Now let's test that it is possible to use binary format.
+    params["data"] = "0";
+    params["csv-format"] = "false";
+    testConfiguration(params, 1000, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // The binary 1 should work as well.
+    params["data"] = "1";
+    expected_option_data[0] = 1;
+    testConfiguration(params, 1000, expected_option_data,
+                      sizeof(expected_option_data));
+
+    // As well as an even number of digits.
+    params["data"] = "01";
+    testConfiguration(params, 1000, expected_option_data,
+                      sizeof(expected_option_data));
+
+}
+
 // Verify that empty option name is rejected in the configuration.
 TEST_F(Dhcp6ParserTest, optionNameEmpty) {
     // Empty option names not allowed.
@@ -2129,14 +2336,6 @@ TEST_F(Dhcp6ParserTest, optionDataUnexpectedPrefix) {
     testInvalidOptionParam("0x0102", "data");
 }
 
-// Verify that option data consisting od an odd number of
-// hexadecimal digits is rejected in the configuration.
-TEST_F(Dhcp6ParserTest, optionDataOddLength) {
-    // Option code 0 is reserved and should not be accepted
-    // by configuration parser.
-    testInvalidOptionParam("123", "data");
-}
-
 // Verify that either lower or upper case characters are allowed
 // to specify the option data.
 TEST_F(Dhcp6ParserTest, optionDataLowerCase) {
@@ -2243,7 +2442,7 @@ TEST_F(Dhcp6ParserTest, vendorOptionsHex) {
         "    \"name\": \"option-one\","
         "    \"space\": \"vendor-4491\","
         "    \"code\": 100,"
-        "    \"data\": \"AB CDEF0105\","
+        "    \"data\": \"ABCDEF0105\","
         "    \"csv-format\": False"
         " },"
         " {"

+ 48 - 22
src/lib/dhcp/option_definition.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 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
@@ -399,18 +399,49 @@ OptionDefinition::haveVendor6Format() const {
     return  (getType() == OPT_UINT32_TYPE && !getEncapsulatedSpace().empty());
 }
 
+bool
+OptionDefinition::convertToBool(const std::string& value_str) const {
+    // Case insensitve check that the input is one of: "true" or "false".
+    if (boost::iequals(value_str, "true")) {
+        return (true);
+
+    } else if (boost::iequals(value_str, "false")) {
+        return (false);
+
+    }
+
+    // The input string is neither "true" nor "false", so let's check
+    // if it is not an integer wrapped in a string.
+    int result;
+    try {
+       result = boost::lexical_cast<int>(value_str);
+
+    } catch (const boost::bad_lexical_cast&) {
+        isc_throw(BadDataTypeCast, "unable to covert the value '"
+                  << value_str << "' to boolean data type");
+    }
+    // The boolean value is encoded in DHCP option as 0 or 1. Therefore,
+    // we only allow a user to specify those values for options which
+    // have boolean fields.
+    if (result != 1 && result != 0) {
+        isc_throw(BadDataTypeCast, "unable to convert '" << value_str
+                  << "' to boolean data type");
+    }
+    return (static_cast<bool>(result));
+}
+
 template<typename T>
 T
 OptionDefinition::lexicalCastWithRangeCheck(const std::string& value_str)
     const {
-    // Lexical cast in case of our data types make sense only
-    // for uintX_t, intX_t and bool type.
-    if (!OptionDataTypeTraits<T>::integer_type &&
-        OptionDataTypeTraits<T>::type != OPT_BOOLEAN_TYPE) {
+    // The lexical cast should be attempted when converting to an integer
+    // value only.
+    if (!OptionDataTypeTraits<T>::integer_type) {
         isc_throw(BadDataTypeCast,
-                  "unable to do lexical cast to non-integer and"
-                  << " non-boolean data type");
+                  "must not convert '" << value_str
+                  << "' to non-integer data type");
     }
+
     // We use the 64-bit value here because it has wider range than
     // any other type we use here and it allows to detect out of
     // bounds conditions e.g. negative value specified for uintX_t
@@ -419,23 +450,19 @@ OptionDefinition::lexicalCastWithRangeCheck(const std::string& value_str)
     int64_t result = 0;
     try {
         result = boost::lexical_cast<int64_t>(value_str);
-    } catch (const boost::bad_lexical_cast& ex) {
-        // Prepare error message here.
-        std::string data_type_str = "boolean";
-        if (OptionDataTypeTraits<T>::integer_type) {
-            data_type_str = "integer";
-        }
-        isc_throw(BadDataTypeCast, "unable to do lexical cast to "
-                  << data_type_str << " data type for value "
-                  << value_str << ": " << ex.what());
+
+    } catch (const boost::bad_lexical_cast&) {
+        isc_throw(BadDataTypeCast, "unable to convert the value '"
+                  << value_str << "' to integer data type");
     }
-    // Perform range checks for integer values only (exclude bool values).
+    // Perform range checks.
     if (OptionDataTypeTraits<T>::integer_type) {
         if (result > numeric_limits<T>::max() ||
             result < numeric_limits<T>::min()) {
-            isc_throw(BadDataTypeCast, "unable to do lexical cast for value "
-                      << value_str << ". This value is expected to be"
-                      << " in the range of " << numeric_limits<T>::min()
+            isc_throw(BadDataTypeCast, "unable to convert '"
+                      << value_str << "' to numeric type. This value is "
+                      " expected to be in the range of "
+                      << numeric_limits<T>::min()
                       << ".." << numeric_limits<T>::max());
         }
     }
@@ -458,8 +485,7 @@ OptionDefinition::writeToBuffer(const std::string& value,
         // That way we actually waste 7 bits but it seems to be the
         // simpler way to encode boolean.
         // @todo Consider if any other encode methods can be used.
-        OptionDataTypeUtil::writeBool(lexicalCastWithRangeCheck<bool>(value),
-                                      buf);
+        OptionDataTypeUtil::writeBool(convertToBool(value), buf);
         return;
     case OPT_INT8_TYPE:
         OptionDataTypeUtil::writeInt<uint8_t>

+ 26 - 7
src/lib/dhcp/option_definition.h

@@ -573,19 +573,38 @@ private:
         return (type == type_);
     }
 
+    /// @brief Converts a string value to a boolean value.
+    ///
+    /// This function converts the value represented as string to a boolean
+    /// value. The following conversions are acceptable:
+    /// - "true" => true
+    /// - "false" => false
+    /// - "1" => true
+    /// - "0" => false
+    /// The first two conversions are case insensitive, so as conversions from
+    /// strings such as "TRUE", "trUE" etc. will be accepted. Note that the
+    /// only acceptable integer values, carried as strings are: "0" and "1".
+    /// For other values, e.g. "2", "3" etc. an exception will be thrown
+    /// during conversion.
+    ///
+    /// @param value_str Input value.
+    ///
+    /// @return boolean representation of the string specified as the parameter.
+    /// @throw isc::dhcp::BadDataTypeCast if failed to perform the conversion.
+    bool convertToBool(const std::string& value_str) const;
+
     /// @brief Perform lexical cast of the value and validate its range.
     ///
     /// This function performs lexical cast of a string value to integer
-    /// or boolean value and checks if the resulting value is within a
-    /// range of a target type. Note that range checks are not performed
-    /// on boolean values. The target type should be one of the supported
-    /// integer types or bool.
+    /// value and checks if the resulting value is within a range of a
+    /// target type. The target type should be one of the supported
+    /// integer types.
     ///
     /// @param value_str input value given as string.
-    /// @tparam T target type for lexical cast.
+    /// @tparam T target integer type for lexical cast.
     ///
-    /// @return cast value.
-    /// @throw BadDataTypeCast if cast was not successful.
+    /// @return Integer value after conversion from the string.
+    /// @throw isc::dhcp::BadDataTypeCast if conversion was not successful.
     template<typename T>
     T lexicalCastWithRangeCheck(const std::string& value_str) const;
 

+ 109 - 2
src/lib/dhcp/tests/option_definition_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 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
@@ -663,6 +663,112 @@ TEST_F(OptionDefinitionTest, recordIAAddr6Tokenized) {
     EXPECT_EQ(5678, option_cast_v6->getValid());
 }
 
+// The purpose of this test is to verify that the definition for option
+// that comprises a boolean value can be created and that this definition
+// can be used to create and option with a single boolean value.
+TEST_F(OptionDefinitionTest, boolValue) {
+    // The IP Forwarding option comprises one boolean value.
+    OptionDefinition opt_def("ip-forwarding", DHO_IP_FORWARDING,
+                             "boolean");
+
+    OptionPtr option_v4;
+    // Use an option buffer which holds one value of 1 (true).
+    ASSERT_NO_THROW(
+        option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
+                                          OptionBuffer(1, 1));
+    );
+    ASSERT_TRUE(typeid(*option_v4) == typeid(OptionCustom));
+    // Validate parsed value in the received option.
+    boost::shared_ptr<OptionCustom> option_cast_v4 =
+        boost::static_pointer_cast<OptionCustom>(option_v4);
+    EXPECT_TRUE(option_cast_v4->readBoolean());
+
+    // Repeat the test above, but set the value to 0 (false).
+    ASSERT_NO_THROW(
+        option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
+                                          OptionBuffer(1, 0));
+    );
+    option_cast_v4 = boost::static_pointer_cast<OptionCustom>(option_v4);
+    EXPECT_FALSE(option_cast_v4->readBoolean());
+
+    // Try to provide zero-length buffer. Expect exception.
+    EXPECT_THROW(
+        opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING, OptionBuffer()),
+        InvalidOptionValue
+    );
+
+}
+
+// The purpose of this test is to verify that definition for option that
+// comprises single boolean value can be created and that this definition
+// can be used to create an option holding a single boolean value. The
+// boolean value is converted from a string which is expected to hold
+// the following values: "true", "false", "1" or "0". For all other
+// values exception should be thrown.
+TEST_F(OptionDefinitionTest, boolTokenized) {
+    OptionDefinition opt_def("ip-forwarding", DHO_IP_FORWARDING, "boolean");
+
+    OptionPtr option_v4;
+    std::vector<std::string> values;
+    // Specify a value for the option instance being created.
+    values.push_back("true");
+    ASSERT_NO_THROW(
+        option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
+                                          values);
+    );
+    ASSERT_TRUE(typeid(*option_v4) == typeid(OptionCustom));
+    // Validate the value.
+    OptionCustomPtr option_cast_v4 =
+        boost::static_pointer_cast<OptionCustom>(option_v4);
+    EXPECT_TRUE(option_cast_v4->readBoolean());
+
+    // Repeat the test but for "false" value this time.
+    values[0] = "false";
+    ASSERT_NO_THROW(
+        option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
+                                          values);
+    );
+    ASSERT_TRUE(typeid(*option_v4) == typeid(OptionCustom));
+    // Validate the value.
+    option_cast_v4 = boost::static_pointer_cast<OptionCustom>(option_v4);
+    EXPECT_FALSE(option_cast_v4->readBoolean());
+
+    // Check if that will work for numeric values.
+    values[0] = "0";
+    ASSERT_NO_THROW(
+        option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
+                                          values);
+    );
+    ASSERT_TRUE(typeid(*option_v4) == typeid(OptionCustom));
+    // Validate the value.
+    option_cast_v4 = boost::static_pointer_cast<OptionCustom>(option_v4);
+    EXPECT_FALSE(option_cast_v4->readBoolean());
+
+    // Swap numeric values and test if it works for "true" case.
+    values[0] = "1";
+    ASSERT_NO_THROW(
+        option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
+                                          values);
+    );
+    ASSERT_TRUE(typeid(*option_v4) == typeid(OptionCustom));
+    // Validate the value.
+    option_cast_v4 = boost::static_pointer_cast<OptionCustom>(option_v4);
+    EXPECT_TRUE(option_cast_v4->readBoolean());
+
+    // A conversion of non-numeric value to boolean should fail if
+    // this value is neither "true" nor "false".
+    values[0] = "garbage";
+    EXPECT_THROW(opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING, values),
+      isc::dhcp::BadDataTypeCast);
+
+    // A conversion of numeric value to boolean should fail if this value
+    // is neither "0" nor "1".
+    values[0] = "2";
+    EXPECT_THROW(opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING, values),
+      isc::dhcp::BadDataTypeCast);
+
+}
+
 // The purpose of this test is to verify that definition for option that
 // comprises single uint8 value can be created and that this definition
 // can be used to create an option with single uint8 value.
@@ -672,7 +778,8 @@ TEST_F(OptionDefinitionTest, uint8) {
     OptionPtr option_v6;
     // Try to use correct buffer length = 1 byte.
     ASSERT_NO_THROW(
-        option_v6 = opt_def.optionFactory(Option::V6, D6O_PREFERENCE, OptionBuffer(1, 1));
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_PREFERENCE,
+                                          OptionBuffer(1, 1));
     );
     ASSERT_TRUE(typeid(*option_v6) == typeid(OptionInt<uint8_t>));
     // Validate the value.

+ 23 - 5
src/lib/dhcpsrv/dhcp_parsers.cc

@@ -400,15 +400,27 @@ void
 OptionDataParser::createOption() {
     // 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 and is not zero.
+    // does not exceed range of uint8_t for DHCPv4, uint16_t for DHCPv6 and
+    // is not zero.
     uint32_t option_code = uint32_values_->getParam("code");
     if (option_code == 0) {
         isc_throw(DhcpConfigError, "option code must not be zero."
-                << " Option code '0' is reserved in DHCPv4.");
-    } else if (option_code > std::numeric_limits<uint8_t>::max()) {
+                << " Option code '0' is reserved.");
+
+    } else if (global_context_->universe_ == Option::V4 &&
+               option_code > std::numeric_limits<uint8_t>::max()) {
+        isc_throw(DhcpConfigError, "invalid option code '" << option_code
+                << "', it must not exceed '"
+                  << static_cast<int>(std::numeric_limits<uint8_t>::max())
+                  << "'");
+
+    } else if (global_context_->universe_ == Option::V6 &&
+               option_code > std::numeric_limits<uint16_t>::max()) {
         isc_throw(DhcpConfigError, "invalid option code '" << option_code
                 << "', it must not exceed '"
-                << std::numeric_limits<uint8_t>::max() << "'");
+                  << std::numeric_limits<uint16_t>::max()
+                  << "'");
+
     }
 
     // Check that the option name has been specified, is non-empty and does not
@@ -464,7 +476,7 @@ OptionDataParser::createOption() {
     }
 
     // Get option data from the configuration database ('data' field).
-    const std::string option_data = string_values_->getParam("data");
+    std::string option_data = string_values_->getParam("data");
 
     // Transform string of hexadecimal digits into binary format.
     std::vector<uint8_t> binary;
@@ -480,6 +492,12 @@ OptionDataParser::createOption() {
         // Otherwise, the option data is specified as a string of
         // hexadecimal digits that we have to turn into binary format.
         try {
+            // 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 (!option_data.empty() && option_data.length() % 2) {
+                option_data = option_data.insert(0, "0");
+            }
             util::encode::decodeHex(option_data, binary);
         } catch (...) {
             isc_throw(DhcpConfigError, "option data is not a valid"