BIND 10 bind10-1.1.0-release, updated. a66bdc21c873982f907eb5edb87f2113536eb384 [master] Added ChangeLog entry for #2786. (cherry picked from commit 9346c7a0f4d9dce50b673e66b5cc992f9b32d18b)

BIND 10 source code commits bind10-changes at lists.isc.org
Wed May 29 14:55:19 UTC 2013


The branch, bind10-1.1.0-release has been updated
       via  a66bdc21c873982f907eb5edb87f2113536eb384 (commit)
       via  3b2bf5fdb7a3e834da57a862639a6797d26d87c7 (commit)
      from  515cf0ab42d1aae7fa9db5140ebdef8d889d970a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit a66bdc21c873982f907eb5edb87f2113536eb384
Author: Jeremy C. Reed <jreed at isc.org>
Date:   Wed May 29 10:17:43 2013 -0400

    [master] Added ChangeLog entry for #2786.
    (cherry picked from commit 9346c7a0f4d9dce50b673e66b5cc992f9b32d18b)
    
    Conflicts:
    
    	ChangeLog
    
    remove unrelated after cherry-pick conflict

commit 3b2bf5fdb7a3e834da57a862639a6797d26d87c7
Author: Jeremy C. Reed <jreed at isc.org>
Date:   Wed May 29 10:16:50 2013 -0400

    [master] Merge branch 'trac2786'
    (cherry picked from commit 96b1a7eb31b16bf9b270ad3d82873c0bd86a3530)
    
    Conflicts:
    
    	src/lib/dhcp/tests/pkt4_unittest.cc
    
    manually removed the head version for that conflict

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog                                        |    8 ++
 src/bin/dhcp6/dhcp6_srv.cc                       |    7 +-
 src/lib/dhcp/Makefile.am                         |    1 +
 src/lib/dhcp/option.cc                           |   13 +-
 src/lib/dhcp/option.h                            |   11 +-
 src/lib/dhcp/option_custom.cc                    |   38 ++---
 src/lib/dhcp/option_custom.h                     |    9 +-
 src/lib/dhcp/option_definition.cc                |    6 +-
 src/lib/dhcp/option_string.cc                    |   86 ++++++++++++
 src/lib/dhcp/option_string.h                     |  114 +++++++++++++++
 src/lib/dhcp/std_option_defs.h                   |    2 +-
 src/lib/dhcp/tests/Makefile.am                   |    1 +
 src/lib/dhcp/tests/libdhcp++_unittest.cc         |   62 +++++----
 src/lib/dhcp/tests/option_custom_unittest.cc     |    4 +-
 src/lib/dhcp/tests/option_definition_unittest.cc |   10 +-
 src/lib/dhcp/tests/option_string_unittest.cc     |  160 ++++++++++++++++++++++
 src/lib/dhcp/tests/pkt4_unittest.cc              |   27 ++--
 17 files changed, 468 insertions(+), 91 deletions(-)
 create mode 100644 src/lib/dhcp/option_string.cc
 create mode 100644 src/lib/dhcp/option_string.h
 create mode 100644 src/lib/dhcp/tests/option_string_unittest.cc

-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index 990b1b6..2119212 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+617.	[bug]		marcin
+	b10-dhcp4: Fixed a bug whereby the domain-name option was encoded
+	as FQDN (using technique described in RFC1035) instead of a string.
+	Also, created new class which represents an option carrying a single
+	string value. This class is now used for all standard options of
+	this kind.
+	(Trac #2786, git 96b1a7eb31b16bf9b270ad3d82873c0bd86a3530)
+
 615.	[bug]		jinmei
 	b10-auth: Avoid referencing to a freed object when authoritative
 	server addresses are reconfigured.  It caused a crash on a busy
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 75a5337..6354302 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -458,10 +458,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).
diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am
index 1e292bd..f9a2881 100644
--- a/src/lib/dhcp/Makefile.am
+++ b/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
diff --git a/src/lib/dhcp/option.cc b/src/lib/dhcp/option.cc
index e06b163..cd6e313 100644
--- a/src/lib/dhcp/option.cc
+++ b/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()) );
diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h
index 28e65a9..9abb4b3 100644
--- a/src/lib/dhcp/option.h
+++ b/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();
diff --git a/src/lib/dhcp/option_custom.cc b/src/lib/dhcp/option_custom.cc
index e807928..12145fc 100644
--- a/src/lib/dhcp/option_custom.cc
+++ b/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) {
diff --git a/src/lib/dhcp/option_custom.h b/src/lib/dhcp/option_custom.h
index 5766cb4..e0314ae 100644
--- a/src/lib/dhcp/option_custom.h
+++ b/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_;
 
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index 7f0d578..9db99f4 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/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());
diff --git a/src/lib/dhcp/option_string.cc b/src/lib/dhcp/option_string.cc
new file mode 100644
index 0000000..6a8001a
--- /dev/null
+++ b/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
diff --git a/src/lib/dhcp/option_string.h b/src/lib/dhcp/option_string.h
new file mode 100644
index 0000000..3d0aa9a
--- /dev/null
+++ b/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
diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h
index 839a5d9..9bd19a1 100644
--- a/src/lib/dhcp/std_option_defs.h
+++ b/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,
diff --git a/src/lib/dhcp/tests/Makefile.am b/src/lib/dhcp/tests/Makefile.am
index c868553..0432ca5 100644
--- a/src/lib/dhcp/tests/Makefile.am
+++ b/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
diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc
index dbab492..d3bbd08 100644
--- a/src/lib/dhcp/tests/libdhcp++_unittest.cc
+++ b/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>
@@ -373,7 +374,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
@@ -404,18 +405,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(
@@ -424,38 +425,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
@@ -533,7 +539,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));
@@ -569,25 +575,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));
@@ -653,7 +659,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));
@@ -675,7 +681,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));
@@ -702,7 +708,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>));
@@ -720,7 +726,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));
@@ -909,10 +915,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>));
diff --git a/src/lib/dhcp/tests/option_custom_unittest.cc b/src/lib/dhcp/tests/option_custom_unittest.cc
index 2a9f771..c04bbc2 100644
--- a/src/lib/dhcp/tests/option_custom_unittest.cc
+++ b/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());
diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc
index 174bafb..f7602a6 100644
--- a/src/lib/dhcp/tests/option_definition_unittest.cc
+++ b/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
diff --git a/src/lib/dhcp/tests/option_string_unittest.cc b/src/lib/dhcp/tests/option_string_unittest.cc
new file mode 100644
index 0000000..0be3c3d
--- /dev/null
+++ b/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
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index 49588e1..da3c3ad 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/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>
@@ -573,17 +574,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



More information about the bind10-changes mailing list