Browse Source

[master] Merge branch 'trac2786'

Marcin Siodelski 12 years ago
parent
commit
96b1a7eb31

+ 3 - 4
src/bin/dhcp6/dhcp6_srv.cc

@@ -463,10 +463,9 @@ Dhcpv6Srv::createStatusCode(uint16_t code, const std::string& text) {
     assert(status_code_def);
 
     // As there is no dedicated class to represent Status Code
-    // the OptionCustom class should be returned here.
-    boost::shared_ptr<OptionCustom> option_status =
-        boost::dynamic_pointer_cast<
-            OptionCustom>(status_code_def->optionFactory(Option::V6, D6O_STATUS_CODE));
+    // the OptionCustom class is used here instead.
+    OptionCustomPtr option_status =
+        OptionCustomPtr(new OptionCustom(*status_code_def, Option::V6));
     assert(option_status);
 
     // Set status code to 'code' (0 - means data field #0).

+ 1 - 0
src/lib/dhcp/Makefile.am

@@ -33,6 +33,7 @@ libb10_dhcp___la_SOURCES += option_custom.cc option_custom.h
 libb10_dhcp___la_SOURCES += option_data_types.cc option_data_types.h
 libb10_dhcp___la_SOURCES += option_definition.cc option_definition.h
 libb10_dhcp___la_SOURCES += option_space.cc option_space.h
+libb10_dhcp___la_SOURCES += option_string.cc option_string.h
 libb10_dhcp___la_SOURCES += pkt6.cc pkt6.h
 libb10_dhcp___la_SOURCES += pkt4.cc pkt4.h
 libb10_dhcp___la_SOURCES += pkt_filter.h

+ 3 - 10
src/lib/dhcp/option.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 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
@@ -55,7 +55,7 @@ Option::Option(Universe u, uint16_t type, const OptionBuffer& data)
 
 Option::Option(Universe u, uint16_t type, OptionBufferConstIter first,
                OptionBufferConstIter last)
-    :universe_(u), type_(type), data_(OptionBuffer(first,last)) {
+    :universe_(u), type_(type), data_(first, last) {
     check();
 }
 
@@ -121,7 +121,7 @@ Option::packOptions(isc::util::OutputBuffer& buf) {
 
 void Option::unpack(OptionBufferConstIter begin,
                     OptionBufferConstIter end) {
-    data_ = OptionBuffer(begin, end);
+    setData(begin, end);
 }
 
 void
@@ -274,13 +274,6 @@ void Option::setUint32(uint32_t value) {
   writeUint32(value, &data_[0]);
 }
 
-void Option::setData(const OptionBufferConstIter first,
-                     const OptionBufferConstIter last) {
-    // We will copy entire option buffer, so we have to resize data_.
-    data_.resize(std::distance(first, last));
-    std::copy(first, last, data_.begin());
-}
-
 bool Option::equal(const OptionPtr& other) const {
     return ( (getType() == other->getType()) &&
              (getData() == other->getData()) );

+ 8 - 3
src/lib/dhcp/option.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2013 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
@@ -282,8 +282,13 @@ public:
     ///
     /// @param first iterator pointing to beginning of buffer to copy.
     /// @param last iterator pointing to end of buffer to copy.
-    void setData(const OptionBufferConstIter first,
-                 const OptionBufferConstIter last);
+    ///
+    /// @tparam InputIterator type of the iterator representing the
+    /// limits of the buffer to be assigned to a data_ buffer.
+    template<typename InputIterator>
+    void setData(InputIterator first, InputIterator last) {
+        data_.assign(first, last);
+    }
 
     /// just to force that every option has virtual dtor
     virtual ~Option();

+ 12 - 26
src/lib/dhcp/option_custom.cc

@@ -28,30 +28,20 @@ OptionCustom::OptionCustom(const OptionDefinition& def,
 }
 
 OptionCustom::OptionCustom(const OptionDefinition& def,
-                             Universe u,
-                             const OptionBuffer& data)
+                           Universe u,
+                           const OptionBuffer& data)
     : Option(u, def.getCode(), data.begin(), data.end()),
       definition_(def) {
-    // It is possible that no data is provided if an option
-    // is being created on a server side. In such case a bunch
-    // of buffers with default values is first created and then
-    // the values are replaced using writeXXX functions. Thus
-    // we need to detect that no data has been specified and
-    // take a different code path.
-    if (!data_.empty()) {
-        createBuffers(data_);
-    } else {
-        createBuffers();
-    }
+    createBuffers(getData());
 }
 
 OptionCustom::OptionCustom(const OptionDefinition& def,
-                             Universe u,
-                             OptionBufferConstIter first,
-                             OptionBufferConstIter last)
+                           Universe u,
+                           OptionBufferConstIter first,
+                           OptionBufferConstIter last)
     : Option(u, def.getCode(), first, last),
       definition_(def) {
-    createBuffers(data_);
+    createBuffers(getData());
 }
 
 void
@@ -517,9 +507,7 @@ OptionCustom::writeString(const std::string& text, const uint32_t index) {
 void
 OptionCustom::unpack(OptionBufferConstIter begin,
                      OptionBufferConstIter end) {
-    data_ = OptionBuffer(begin, end);
-    // Chop the buffer stored in data_ into set of sub buffers.
-    createBuffers(data_);
+    initialize(begin, end);
 }
 
 uint16_t
@@ -543,15 +531,13 @@ OptionCustom::len() {
     return (length);
 }
 
-void OptionCustom::setData(const OptionBufferConstIter first,
-                     const OptionBufferConstIter last) {
-    // We will copy entire option buffer, so we have to resize data_.
-    data_.resize(std::distance(first, last));
-    std::copy(first, last, data_.begin());
+void OptionCustom::initialize(const OptionBufferConstIter first,
+                              const OptionBufferConstIter last) {
+    setData(first, last);
 
     // Chop the data_ buffer into set of buffers that represent
     // option fields data.
-    createBuffers(data_);
+    createBuffers(getData());
 }
 
 std::string OptionCustom::toText(int indent) {

+ 7 - 2
src/lib/dhcp/option_custom.h

@@ -280,8 +280,8 @@ public:
     ///
     /// @param first iterator pointing to beginning of buffer to copy.
     /// @param last iterator pointing to end of buffer to copy.
-    void setData(const OptionBufferConstIter first,
-                 const OptionBufferConstIter last);
+    void initialize(const OptionBufferConstIter first,
+                    const OptionBufferConstIter last);
 
 private:
 
@@ -336,6 +336,11 @@ private:
     std::string dataFieldToText(const OptionDataType data_type,
                                 const uint32_t index) const;
 
+    /// Make this function private as we don't want it to be invoked
+    /// on OptionCustom object. We rather want that initialize to
+    /// be called instead.
+    using Option::setData;
+
     /// Option definition used to create an option.
     OptionDefinition definition_;
 

+ 5 - 1
src/lib/dhcp/option_definition.cc

@@ -22,6 +22,7 @@
 #include <dhcp/option_int.h>
 #include <dhcp/option_int_array.h>
 #include <dhcp/option_space.h>
+#include <dhcp/option_string.h>
 #include <util/encode/hex.h>
 #include <util/strutil.h>
 #include <boost/algorithm/string/classification.hpp>
@@ -160,6 +161,9 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
             }
             break;
 
+        case OPT_STRING_TYPE:
+            return (OptionPtr(new OptionString(u, type, begin, end)));
+
         default:
             if (u == Option::V6) {
                 if ((code_ == D6O_IA_NA || code_ == D6O_IA_PD) &&
@@ -182,7 +186,7 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
                 }
             }
         }
-        return (OptionPtr(new OptionCustom(*this, u, OptionBuffer(begin, end))));
+        return (OptionPtr(new OptionCustom(*this, u, begin, end)));
 
     } catch (const Exception& ex) {
         isc_throw(InvalidOptionValue, ex.what());

+ 86 - 0
src/lib/dhcp/option_string.cc

@@ -0,0 +1,86 @@
+// Copyright (C) 2013 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dhcp/option_string.h>
+
+namespace isc {
+namespace dhcp {
+
+OptionString::OptionString(const Option::Universe u, const uint16_t type,
+                           const std::string& value)
+    : Option(u, type) {
+    // Try to assign the provided string value. This will throw exception
+    // if the provided value is empty.
+    setValue(value);
+}
+
+OptionString::OptionString(const Option::Universe u, const uint16_t type,
+                           OptionBufferConstIter begin,
+                           OptionBufferConstIter end)
+    : Option(u, type) {
+    // Decode the data. This will throw exception if the buffer is
+    // truncated.
+    unpack(begin, end);
+}
+
+std::string
+OptionString::getValue() const {
+    const OptionBuffer& data = getData();
+    return (std::string(data.begin(), data.end()));
+}
+
+void
+OptionString::setValue(const std::string& value) {
+    // Sanity check that the string value is at least one byte long.
+    // This is a requirement for all currently defined options which
+    // carry a string value.
+    if (value.empty()) {
+        isc_throw(isc::OutOfRange, "string value carried by the option '"
+                  << getType() << "' must not be empty");
+    }
+
+    setData(value.begin(), value.end());
+}
+
+
+uint16_t
+OptionString::len() {
+    return (getHeaderLen() + getData().size());
+}
+
+void
+OptionString::pack(isc::util::OutputBuffer& buf) {
+    // Pack option header.
+    packHeader(buf);
+    // Pack data.
+    const OptionBuffer& data = getData();
+    buf.writeData(&data[0], data.size());
+
+    // That's it. We don't pack any sub-options here, because this option
+    // must not contain sub-options.
+}
+
+void
+OptionString::unpack(OptionBufferConstIter begin,
+                     OptionBufferConstIter end) {
+    if (std::distance(begin, end) == 0) {
+        isc_throw(isc::OutOfRange, "failed to parse an option '"
+                  << getType() << "' holding string value"
+                  << " - empty value is not accepted");
+    }
+    setData(begin, end);
+}
+
+} // end of isc::dhcp namespace
+} // end of isc namespace

+ 114 - 0
src/lib/dhcp/option_string.h

@@ -0,0 +1,114 @@
+// Copyright (C) 2013 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef OPTION_STRING_H
+#define OPTION_STRING_H
+
+#include <dhcp/option.h>
+#include <util/buffer.h>
+
+#include <boost/shared_ptr.hpp>
+#include <string>
+
+namespace isc {
+namespace dhcp {
+
+/// @brief Class which represents an option carrying a single string value.
+///
+/// This class represents an option carrying a single string value.
+/// Currently this class imposes that the minimal length of the carried
+/// string is 1.
+///
+/// @todo In the future this class may be extended with some more string
+/// content checks and encoding methods if required.
+class OptionString : public Option {
+public:
+
+    /// @brief Constructor, used to create options to be sent.
+    ///
+    /// This constructor creates an instance of option which carries a
+    /// string value specified as constructor's parameter. This constructor
+    /// is most often used to create an instance of an option which will
+    /// be sent in the outgoing packet.
+    ///
+    /// @param u universe (V4 or V6).
+    /// @param type option code.
+    /// @param value a string value to be carried by the option.
+    ///
+    /// @throw isc::OutOfRange if provided string is empty.
+    OptionString(const Option::Universe u, const uint16_t type,
+                 const std::string& value);
+
+    /// @brief Constructor, used for receiving options.
+    ///
+    /// This constructor creates an instance of the option from the provided
+    /// chunk of buffer. This buffer may hold the data received on the wire.
+    ///
+    /// @param u universe (V4 or V6).
+    /// @param type option code.
+    /// @param begin iterator pointing to the first byte of the buffer chunk.
+    /// @param end iterator pointing to the last byte of the buffer chunk.
+    ///
+    /// @throw isc::OutOfRange if provided buffer is truncated.
+    OptionString(const Option::Universe u, const uint16_t type,
+                 OptionBufferConstIter begin, OptionBufferConstIter end);
+
+    /// @brief Returns length of the whole option, including header.
+    ///
+    /// @return length of the whole option.
+    virtual uint16_t len();
+
+    /// @brief Returns the string value held by the option.
+    ///
+    /// @return string value held by the option.
+    std::string getValue() const;
+
+    /// @brief Sets the string value to be held by the option.
+    ///
+    /// @param value string value to be set.
+    ///
+    /// @throw isc::OutOfRange if a string value to be set is empty.
+    void setValue(const std::string& value);
+
+    /// @brief Creates on-wire format of the option.
+    ///
+    /// This function creates on-wire format of the option and appends it to
+    /// the data existing in the provided buffer. The internal buffer's pointer
+    /// is moved to the end of stored data.
+    ///
+    /// @param [out] buf output buffer where the option will be stored.
+    virtual void pack(isc::util::OutputBuffer& buf);
+
+    /// @brief Decodes option data from the provided buffer.
+    ///
+    /// This function decodes option data from the provided buffer. Note that
+    /// it does not decode the option code and length, so the iterators must
+    /// point to the begining and end of the option payload respectively.
+    /// The size of the decoded payload must be at least 1 byte.
+    ///
+    /// @param begin the iterator pointing to the option payload.
+    /// @param end the iterator pointing to the end of the option payload.
+    ///
+    /// @throw isc::OutOfRange if provided buffer is truncated.
+    virtual void unpack(OptionBufferConstIter begin, OptionBufferConstIter end);
+
+};
+
+/// Pointer to the OptionString object.
+typedef boost::shared_ptr<OptionString> OptionStringPtr;
+
+} // namespace isc::dhcp
+} // namespace isc
+
+#endif // OPTION_STRING_H

+ 1 - 1
src/lib/dhcp/std_option_defs.h

@@ -84,7 +84,7 @@ const OptionDefParams OPTION_DEF_PARAMS4[] = {
     { "host-name", DHO_HOST_NAME, OPT_STRING_TYPE, false, NO_RECORD_DEF, "" },
     { "boot-size", DHO_BOOT_SIZE, OPT_UINT16_TYPE, false, NO_RECORD_DEF, "" },
     { "merit-dump", DHO_MERIT_DUMP, OPT_STRING_TYPE, false, NO_RECORD_DEF, "" },
-    { "domain-name", DHO_DOMAIN_NAME, OPT_FQDN_TYPE, false, NO_RECORD_DEF, "" },
+    { "domain-name", DHO_DOMAIN_NAME, OPT_STRING_TYPE, false, NO_RECORD_DEF, "" },
     { "swap-server", DHO_SWAP_SERVER, OPT_IPV4_ADDRESS_TYPE, false, NO_RECORD_DEF, "" },
     { "root-path", DHO_ROOT_PATH, OPT_STRING_TYPE, false, NO_RECORD_DEF, "" },
     { "extensions-path", DHO_EXTENSIONS_PATH, OPT_STRING_TYPE,

+ 1 - 0
src/lib/dhcp/tests/Makefile.am

@@ -41,6 +41,7 @@ libdhcp___unittests_SOURCES += option_definition_unittest.cc
 libdhcp___unittests_SOURCES += option_custom_unittest.cc
 libdhcp___unittests_SOURCES += option_unittest.cc
 libdhcp___unittests_SOURCES += option_space_unittest.cc
+libdhcp___unittests_SOURCES += option_string_unittest.cc
 libdhcp___unittests_SOURCES += pkt4_unittest.cc
 libdhcp___unittests_SOURCES += pkt6_unittest.cc
 libdhcp___unittests_SOURCES += duid_unittest.cc

+ 34 - 28
src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -24,6 +24,7 @@
 #include <dhcp/option_custom.h>
 #include <dhcp/option_int.h>
 #include <dhcp/option_int_array.h>
+#include <dhcp/option_string.h>
 #include <util/buffer.h>
 
 #include <gtest/gtest.h>
@@ -372,7 +373,7 @@ TEST_F(LibDhcpTest, unpackOptions6) {
 /// is no restriction on the data length being carried by them.
 /// For simplicity, we assign data of the length 3 for each
 /// of them.
-static uint8_t v4Opts[] = {
+static uint8_t v4_opts[] = {
     12,  3, 0,   1,  2, // Hostname
     60,  3, 10, 11, 12, // Class Id
     14,  3, 20, 21, 22, // Merit Dump File
@@ -403,18 +404,18 @@ TEST_F(LibDhcpTest, packOptions4) {
     opts.insert(make_pair(opt1->getType(), opt4));
     opts.insert(make_pair(opt1->getType(), opt5));
 
-    vector<uint8_t> expVect(v4Opts, v4Opts + sizeof(v4Opts));
+    vector<uint8_t> expVect(v4_opts, v4_opts + sizeof(v4_opts));
 
     OutputBuffer buf(100);
     EXPECT_NO_THROW(LibDHCP::packOptions(buf, opts));
-    ASSERT_EQ(buf.getLength(), sizeof(v4Opts));
-    EXPECT_EQ(0, memcmp(v4Opts, buf.getData(), sizeof(v4Opts)));
+    ASSERT_EQ(buf.getLength(), sizeof(v4_opts));
+    EXPECT_EQ(0, memcmp(v4_opts, buf.getData(), sizeof(v4_opts)));
 
 }
 
 TEST_F(LibDhcpTest, unpackOptions4) {
 
-    vector<uint8_t> v4packed(v4Opts, v4Opts + sizeof(v4Opts));
+    vector<uint8_t> v4packed(v4_opts, v4_opts + sizeof(v4_opts));
     isc::dhcp::Option::OptionCollection options; // list of options
 
     ASSERT_NO_THROW(
@@ -423,38 +424,43 @@ TEST_F(LibDhcpTest, unpackOptions4) {
 
     isc::dhcp::Option::OptionCollection::const_iterator x = options.find(12);
     ASSERT_FALSE(x == options.end()); // option 1 should exist
-    EXPECT_EQ(12, x->second->getType());  // this should be option 12
-    ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
-    EXPECT_EQ(5, x->second->len()); // total option length 5
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+2, 3)); // data len=3
+    // Option 12 holds a string so let's cast it to an appropriate type.
+    OptionStringPtr option12 = boost::static_pointer_cast<OptionString>(x->second);
+    ASSERT_TRUE(option12);
+    EXPECT_EQ(12, option12->getType());  // this should be option 12
+    ASSERT_EQ(3, option12->getValue().length()); // it should be of length 3
+    EXPECT_EQ(5, option12->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&option12->getValue()[0], v4_opts + 2, 3)); // data len=3
 
     x = options.find(60);
     ASSERT_FALSE(x == options.end()); // option 2 should exist
     EXPECT_EQ(60, x->second->getType());  // this should be option 60
     ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
     EXPECT_EQ(5, x->second->len()); // total option length 5
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+7, 3)); // data len=3
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4_opts + 7, 3)); // data len=3
 
     x = options.find(14);
     ASSERT_FALSE(x == options.end()); // option 3 should exist
-    EXPECT_EQ(14, x->second->getType());  // this should be option 14
-    ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
-    EXPECT_EQ(5, x->second->len()); // total option length 5
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+12, 3)); // data len=3
+    OptionStringPtr option14 = boost::static_pointer_cast<OptionString>(x->second);
+    ASSERT_TRUE(option14);
+    EXPECT_EQ(14, option14->getType());  // this should be option 14
+    ASSERT_EQ(3, option14->getValue().length()); // it should be of length 3
+    EXPECT_EQ(5, option14->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&option14->getValue()[0], v4_opts + 12, 3)); // data len=3
 
     x = options.find(254);
     ASSERT_FALSE(x == options.end()); // option 3 should exist
     EXPECT_EQ(254, x->second->getType());  // this should be option 254
     ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
     EXPECT_EQ(5, x->second->len()); // total option length 5
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+17, 3)); // data len=3
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4_opts + 17, 3)); // data len=3
 
     x = options.find(128);
     ASSERT_FALSE(x == options.end()); // option 3 should exist
     EXPECT_EQ(128, x->second->getType());  // this should be option 254
     ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
     EXPECT_EQ(5, x->second->len()); // total option length 5
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+22, 3)); // data len=3
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4_opts + 22, 3)); // data len=3
 
     x = options.find(0);
     EXPECT_TRUE(x == options.end()); // option 0 not found
@@ -532,7 +538,7 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
     // It will be used to create most of the options.
     std::vector<uint8_t> buf(48, 1);
     OptionBufferConstIter begin = buf.begin();
-    OptionBufferConstIter end = buf.begin();
+    OptionBufferConstIter end = buf.end();
 
     LibDhcpTest::testStdOptionDefs4(DHO_SUBNET_MASK, begin, end,
                                     typeid(OptionCustom));
@@ -568,25 +574,25 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
                                     typeid(Option4AddrLst));
 
     LibDhcpTest::testStdOptionDefs4(DHO_HOST_NAME, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs4(DHO_BOOT_SIZE, begin, begin + 2,
                                     typeid(OptionInt<uint16_t>));
 
     LibDhcpTest::testStdOptionDefs4(DHO_MERIT_DUMP, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs4(DHO_DOMAIN_NAME, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs4(DHO_SWAP_SERVER, begin, end,
                                     typeid(OptionCustom));
 
     LibDhcpTest::testStdOptionDefs4(DHO_ROOT_PATH, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs4(DHO_EXTENSIONS_PATH, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs4(DHO_IP_FORWARDING, begin, end,
                                     typeid(OptionCustom));
@@ -652,7 +658,7 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
                                     typeid(OptionCustom));
 
     LibDhcpTest::testStdOptionDefs4(DHO_NIS_DOMAIN, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs4(DHO_NIS_SERVERS, begin, end,
                                     typeid(Option4AddrLst));
@@ -674,7 +680,7 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
                                     typeid(OptionInt<uint8_t>));
 
     LibDhcpTest::testStdOptionDefs4(DHO_NETBIOS_SCOPE, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs4(DHO_FONT_SERVERS, begin, end,
                                     typeid(Option4AddrLst));
@@ -701,7 +707,7 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
                                     typeid(Option));
 
     LibDhcpTest::testStdOptionDefs4(DHO_DHCP_MESSAGE, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs4(DHO_DHCP_MAX_MESSAGE_SIZE, begin, begin + 2,
                                     typeid(OptionInt<uint16_t>));
@@ -719,7 +725,7 @@ TEST_F(LibDhcpTest, stdOptionDefs4) {
                                     typeid(Option));
 
     LibDhcpTest::testStdOptionDefs4(DHO_NWIP_DOMAIN_NAME, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs4(DHO_NWIP_SUBOPTIONS, begin, end,
                                     typeid(Option));
@@ -908,10 +914,10 @@ TEST_F(LibDhcpTest, stdOptionDefs6) {
                                     typeid(Option6AddrLst));
 
     LibDhcpTest::testStdOptionDefs6(D6O_NEW_POSIX_TIMEZONE, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs6(D6O_NEW_TZDB_TIMEZONE, begin, end,
-                                    typeid(OptionCustom));
+                                    typeid(OptionString));
 
     LibDhcpTest::testStdOptionDefs6(D6O_ERO, begin, end,
                                     typeid(OptionIntArray<uint16_t>));

+ 2 - 2
src/lib/dhcp/tests/option_custom_unittest.cc

@@ -1324,7 +1324,7 @@ TEST_F(OptionCustomTest, unpack) {
 
 // The purpose of this test is to verify that new data can be set for
 // a custom option.
-TEST_F(OptionCustomTest, setData) {
+TEST_F(OptionCustomTest, initialize) {
     OptionDefinition opt_def("OPTION_FOO", 1000, "ipv6-address", true);
 
     // Initialize reference data.
@@ -1370,7 +1370,7 @@ TEST_F(OptionCustomTest, setData) {
     }
 
     // Replace the option data.
-    ASSERT_NO_THROW(option->setData(buf.begin(), buf.end()));
+    ASSERT_NO_THROW(option->initialize(buf.begin(), buf.end()));
 
     // Now we should have only 2 data fields.
     ASSERT_EQ(2, option->getDataFieldsNum());

+ 5 - 5
src/lib/dhcp/tests/option_definition_unittest.cc

@@ -25,6 +25,7 @@
 #include <dhcp/option_definition.h>
 #include <dhcp/option_int.h>
 #include <dhcp/option_int_array.h>
+#include <dhcp/option_string.h>
 #include <exceptions/exceptions.h>
 
 #include <boost/pointer_cast.hpp>
@@ -936,11 +937,10 @@ TEST_F(OptionDefinitionTest, utf8StringTokenized) {
         option_v6 = opt_def.optionFactory(Option::V6, opt_code, values);
     );
     ASSERT_TRUE(option_v6);
-    ASSERT_TRUE(typeid(*option_v6) == typeid(OptionCustom));
-    std::vector<uint8_t> data = option_v6->getData();
-    std::vector<uint8_t> ref_data(values[0].c_str(), values[0].c_str()
-                                  + values[0].length());
-    EXPECT_TRUE(std::equal(ref_data.begin(), ref_data.end(), data.begin()));
+    ASSERT_TRUE(typeid(*option_v6) == typeid(OptionString));
+    OptionStringPtr option_v6_string =
+        boost::static_pointer_cast<OptionString>(option_v6);
+    EXPECT_TRUE(values[0] == option_v6_string->getValue());
 }
 
 // The purpose of this test is to check that non-integer data type can't

+ 160 - 0
src/lib/dhcp/tests/option_string_unittest.cc

@@ -0,0 +1,160 @@
+// Copyright (C) 2013 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <config.h>
+#include <dhcp/option_string.h>
+
+#include <boost/scoped_ptr.hpp>
+
+#include <gtest/gtest.h>
+
+using namespace isc;
+using namespace isc::dhcp;
+using namespace isc::util;
+
+namespace {
+
+/// @brief OptionString test class.
+class OptionStringTest : public ::testing::Test {
+public:
+    /// @brief Constructor.
+    ///
+    /// Initializes the test buffer with some data.
+    OptionStringTest() {
+        std::string test_string("This is a test string");
+        buf_.assign(test_string.begin(), test_string.end());
+    }
+
+    OptionBuffer buf_;
+
+};
+
+// This test verifies that the constructor which creates an option instance
+// from a string value will create it properly.
+TEST_F(OptionStringTest, constructorFromString) {
+    const std::string optv4_value = "some option";
+    OptionString optv4(Option::V4, 123, optv4_value);
+    EXPECT_EQ(Option::V4, optv4.getUniverse());
+    EXPECT_EQ(123, optv4.getType());
+    EXPECT_EQ(optv4_value, optv4.getValue());
+    EXPECT_EQ(Option::OPTION4_HDR_LEN + optv4_value.size(), optv4.len());
+
+    // Do another test with the same constructor to make sure that
+    // different set of parameters would initialize the class members
+    // to different values.
+    const std::string optv6_value = "other option";
+    OptionString optv6(Option::V6, 234, optv6_value);
+    EXPECT_EQ(Option::V6, optv6.getUniverse());
+    EXPECT_EQ(234, optv6.getType());
+    EXPECT_EQ("other option", optv6.getValue());
+    EXPECT_EQ(Option::OPTION6_HDR_LEN + optv6_value.size(), optv6.len());
+
+    // Check that an attempt to use empty string in the constructor
+    // will result in an exception.
+    EXPECT_THROW(OptionString(Option::V6, 123, ""), isc::OutOfRange);
+}
+
+// This test verifies that the constructor which creates an option instance
+// from a buffer, holding option payload, will create it properly.
+// This function calls unpack() internally thus test test is considered
+// to cover testing of unpack() functionality.
+TEST_F(OptionStringTest, constructorFromBuffer) {
+    // Attempt to create an option using empty buffer should result in
+    // an exception.
+    EXPECT_THROW(
+        OptionString(Option::V4, 234, buf_.begin(), buf_.begin()),
+        isc::OutOfRange
+    );
+
+    // Declare option as a scoped pointer here so as its scope is
+    // function wide. The initialization (constructor invocation)
+    // is pushed to the ASSERT_NO_THROW macro below, as it may
+    // throw exception if buffer is truncated.
+    boost::scoped_ptr<OptionString> optv4;
+    ASSERT_NO_THROW(
+        optv4.reset(new OptionString(Option::V4, 234, buf_.begin(), buf_.end()));
+    );
+    // Make sure that it has been initialized to non-NULL value.
+    ASSERT_TRUE(optv4);
+
+    // Test the instance of the created option.
+    const std::string optv4_value = "This is a test string";
+    EXPECT_EQ(Option::V4, optv4->getUniverse());
+    EXPECT_EQ(234, optv4->getType());
+    EXPECT_EQ(Option::OPTION4_HDR_LEN + buf_.size(), optv4->len());
+    EXPECT_EQ(optv4_value, optv4->getValue());
+
+    // Do the same test for V6 option.
+    boost::scoped_ptr<OptionString> optv6;
+    ASSERT_NO_THROW(
+        // Let's reduce the size of the buffer by one byte and see if our option
+        // will absorb this little change.
+        optv6.reset(new OptionString(Option::V6, 123, buf_.begin(), buf_.end() - 1));
+    );
+    // Make sure that it has been initialized to non-NULL value.
+    ASSERT_TRUE(optv6);
+
+    // Test the instance of the created option.
+    const std::string optv6_value = "This is a test strin";
+    EXPECT_EQ(Option::V6, optv6->getUniverse());
+    EXPECT_EQ(123, optv6->getType());
+    EXPECT_EQ(Option::OPTION6_HDR_LEN + buf_.size() - 1, optv6->len());
+    EXPECT_EQ(optv6_value, optv6->getValue());
+}
+
+// This test verifies that the current option value can be overriden
+// with new value, using setValue method.
+TEST_F(OptionStringTest, setValue) {
+    // Create an instance of the option and set some initial value.
+    OptionString optv4(Option::V4, 123, "some option");
+    EXPECT_EQ("some option", optv4.getValue());
+    // Replace the value with the new one, and make sure it has
+    // been successful.
+    EXPECT_NO_THROW(optv4.setValue("new option value"));
+    EXPECT_EQ("new option value", optv4.getValue());
+    // Try to set to an empty string. It should throw exception.
+    EXPECT_THROW(optv4.setValue(""), isc::OutOfRange);
+}
+
+// This test verifies that the pack function encodes the option in
+// a on-wire format properly.
+TEST_F(OptionStringTest, pack) {
+    // Create an instance of the option.
+    std::string option_value("sample option value");
+    OptionString optv4(Option::V4, 123, option_value);
+    // Encode the option in on-wire format.
+    OutputBuffer buf(Option::OPTION4_HDR_LEN);
+    EXPECT_NO_THROW(optv4.pack(buf));
+
+    // Sanity check the length of the buffer.
+    ASSERT_EQ(Option::OPTION4_HDR_LEN + option_value.length(),
+              buf.getLength());
+    // Copy the contents of the OutputBuffer to InputBuffer because
+    // the latter has API to read data from it.
+    InputBuffer test_buf(buf.getData(), buf.getLength());
+    // First byte holds option code.
+    EXPECT_EQ(123, test_buf.readUint8());
+    // Second byte holds option length.
+    EXPECT_EQ(option_value.size(), test_buf.readUint8());
+    // Read the option data.
+    std::vector<uint8_t> data;
+    test_buf.readVector(data, test_buf.getLength() - test_buf.getPosition());
+    // And create a string from it.
+    std::string test_string(data.begin(), data.end());
+    // This string should be equal to the string used to create
+    // option's instance. 
+    EXPECT_TRUE(option_value == test_string);
+}
+
+} // anonymous namespace

+ 18 - 9
src/lib/dhcp/tests/pkt4_unittest.cc

@@ -16,6 +16,7 @@
 
 #include <asiolink/io_address.h>
 #include <dhcp/dhcp4.h>
+#include <dhcp/option_string.h>
 #include <dhcp/pkt4.h>
 #include <exceptions/exceptions.h>
 #include <util/buffer.h>
@@ -549,17 +550,25 @@ TEST(Pkt4Test, unpackOptions) {
 
     boost::shared_ptr<Option> x = pkt->getOption(12);
     ASSERT_TRUE(x); // option 1 should exist
-    EXPECT_EQ(12, x->getType());  // this should be option 12
-    ASSERT_EQ(3, x->getData().size()); // it should be of length 3
-    EXPECT_EQ(5, x->len()); // total option length 5
-    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts + 2, 3)); // data len=3
+    // Option 12 is represented by the OptionString class so let's do
+    // the appropriate conversion.
+    OptionStringPtr option12 = boost::static_pointer_cast<OptionString>(x);
+    ASSERT_TRUE(option12);
+    EXPECT_EQ(12, option12->getType());  // this should be option 12
+    ASSERT_EQ(3, option12->getValue().length()); // it should be of length 3
+    EXPECT_EQ(5, option12->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&option12->getValue()[0], v4Opts + 2, 3)); // data len=3
 
     x = pkt->getOption(14);
-    ASSERT_TRUE(x); // option 13 should exist
-    EXPECT_EQ(14, x->getType());  // this should be option 13
-    ASSERT_EQ(3, x->getData().size()); // it should be of length 3
-    EXPECT_EQ(5, x->len()); // total option length 5
-    EXPECT_EQ(0, memcmp(&x->getData()[0], v4Opts + 7, 3)); // data len=3
+    ASSERT_TRUE(x); // option 14 should exist
+    // Option 14 is represented by the OptionString class so let's do
+    // the appropriate conversion.
+    OptionStringPtr option14 = boost::static_pointer_cast<OptionString>(x);
+    ASSERT_TRUE(option14);
+    EXPECT_EQ(14, option14->getType());  // this should be option 14
+    ASSERT_EQ(3, option14->getValue().length()); // it should be of length 3
+    EXPECT_EQ(5, option14->len()); // total option length 5
+    EXPECT_EQ(0, memcmp(&option14->getValue()[0], v4Opts + 7, 3)); // data len=3
 
     x = pkt->getOption(60);
     ASSERT_TRUE(x); // option 60 should exist