[3292] Fixed option definition class to handle booleans carried as strings.

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
@@ -411,33 +411,59 @@ OptionDefinition::lexicalCastWithRangeCheck(const std::string& value_str)
                   "unable to do lexical cast to non-integer and"
                   << " non-boolean data type");
     }
+
+    // The lexical cast will not handle conversion of the string holding
+    // "true" or "false". Therefore we will do the conversion on our own.
+    // If the string value doesn't match any of "true" or "false", it
+    // is possible that "0" or "1" has been specified, which we are
+    // ok with. For conversion of "0" or "1" we will try lexical cast
+    // below.
+    if (OptionDataTypeTraits<T>::type == OPT_BOOLEAN_TYPE) {
+        if (value_str == "true") {
+            return (true);
+
+        } else if (value_str == "false") {
+            return (false);
+
+        }
+    }
+
     // 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
     // data type. Obviously if the value exceeds the limits of int64
     // this function will not handle that properly.
+    // Note that with the conversion below we also handle boolean
+    // values specified as "0" or "1".
     int64_t result = 0;
     try {
         result = boost::lexical_cast<int64_t>(value_str);
-    } catch (const boost::bad_lexical_cast& ex) {
+    } catch (const boost::bad_lexical_cast&) {
         // 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());
+        isc_throw(BadDataTypeCast, "unable to convert the value '"
+                  << value_str << "' to " << data_type_str
+                  << " data type");
     }
     // Perform range checks for integer values only (exclude bool values).
     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());
         }
+    // The specified value is of the boolean type and we are checking
+    // that it has successfuly converted to 0 or 1. Any other numeric
+    // value is not accepted to represent boolean value.
+    } else if (result != 1 && result != 0) {
+        isc_throw(BadDataTypeCast, "unable to convert '" << value_str
+                  << "' to boolean data type");
     }
     return (static_cast<T>(result));
 }

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,120 @@ 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(
+        option_v4 = opt_def.optionFactory(Option::V6, 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;
+    // Provide two boolean values. It is expected that only the first
+    // one will be used.
+    values.push_back("true");
+    values.push_back("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.
+    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";
+    values[1] = "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.
+    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";
+    values[1] = "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_FALSE(option_cast_v4->readBoolean());
+
+    // Swap numeric values and test if it works for "true" case.
+    values[0] = "1";
+    values[1] = "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_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";
+    values[1] = "false";
+    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";
+    values[1] = "0";
+    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 +786,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.