Merge remote-tracking branch 'origin/trac1228' into trac1238

Conflicts:
	ChangeLog

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -130,13 +130,15 @@ TEST_F(Dhcpv6SrvTest, Solicit_basic) {
     ASSERT_TRUE( tmp );
     EXPECT_EQ(clientid->getType(), tmp->getType() );
     ASSERT_EQ(clientid->len(), tmp->len() );
-    EXPECT_FALSE(memcmp(clientid->getData(), tmp->getData(), tmp->len() ) );
+
+    EXPECT_EQ( clientid->getData(), tmp->getData() );
+
     // check that server included its server-id
     tmp = reply->getOption(D6O_SERVERID);
     EXPECT_EQ(tmp->getType(), srv->getServerID()->getType() );
     ASSERT_EQ(tmp->len(),  srv->getServerID()->len() );
-    EXPECT_FALSE( memcmp(tmp->getData(), srv->getServerID()->getData(),
-                      tmp->len()) );
+
+    EXPECT_EQ(tmp->getData(), srv->getServerID()->getData());
 
     // more checks to be implemented
     delete srv;

src/lib/dhcp/libdhcp.cc

@@ -14,16 +14,17 @@
 
 #include <boost/shared_array.hpp>
 #include <boost/shared_ptr.hpp>
-#include "dhcp/libdhcp.h"
+#include <util/buffer.h>
+#include <dhcp/libdhcp.h>
 #include "config.h"
-#include "dhcp6.h"
-
-#include "option.h"
-#include "option6_ia.h"
-#include "option6_iaaddr.h"
+#include <dhcp/dhcp6.h>
+#include <dhcp/option.h>
+#include <dhcp/option6_ia.h>
+#include <dhcp/option6_iaaddr.h>
 
 using namespace std;
 using namespace isc::dhcp;
+using namespace isc::util;
 
 // static array with factories for options
 std::map<unsigned short, Option::Factory*> LibDHCP::v6factories_;
@@ -32,7 +33,7 @@ unsigned int
 LibDHCP::unpackOptions6(const boost::shared_array<uint8_t> buf,
                         unsigned int buf_len,
                         unsigned int offset, unsigned int parse_len,
-                        isc::dhcp::Option::Option6Collection& options) {
+                        isc::dhcp::Option::OptionCollection& options) {
     if (offset + parse_len > buf_len) {
         isc_throw(OutOfRange, "Option parse failed. Tried to parse "
                   << parse_len << " bytes at offset " << offset
@@ -83,13 +84,41 @@ LibDHCP::unpackOptions6(const boost::shared_array<uint8_t> buf,
     return (offset);
 }
 
+void
+LibDHCP::unpackOptions4(const std::vector<uint8_t>& buf,
+                        isc::dhcp::Option::OptionCollection& options) {
+    size_t offset = 0;
+
+    // 2 - header of DHCPv4 option
+    while (offset + 2 <= buf.size()) {
+        uint8_t opt_type = buf[offset++];
+        uint8_t opt_len =  buf[offset++];
+        if (offset + opt_len > buf.size() ) {
+            isc_throw(OutOfRange, "Option parse failed. Tried to parse "
+                      << offset + opt_len << " bytes from " << buf.size()
+                      << "-byte long buffer.");
+        }
+
+        boost::shared_ptr<Option> opt;
+        switch(opt_type) {
+        default:
+            opt = boost::shared_ptr<Option>(new Option(Option::V4, opt_type,
+                                                       buf.begin()+offset,
+                                                       buf.begin()+offset+opt_len));
+        }
+
+        options.insert(pair<int, boost::shared_ptr<Option> >(opt_type, opt));
+        offset += opt_len;
+    }
+}
+
 unsigned int
 LibDHCP::packOptions6(boost::shared_array<uint8_t> data,
                       unsigned int data_len,
                       unsigned int offset,
-                      const isc::dhcp::Option::Option6Collection& options) {
+                      const isc::dhcp::Option::OptionCollection& options) {
     try {
-        for (isc::dhcp::Option::Option6Collection::const_iterator it = options.begin();
+        for (Option::OptionCollection::const_iterator it = options.begin();
              it != options.end();
              ++it) {
             unsigned short opt_len = (*it).second->len();
@@ -97,7 +126,7 @@ LibDHCP::packOptions6(boost::shared_array<uint8_t> data,
                 isc_throw(OutOfRange, "Failed to build option " <<
                           (*it).first << ": out of buffer");
             }
-            offset = (*it).second->pack(data, data_len, offset);
+            offset = it->second->pack(data, data_len, offset);
         }
     }
     catch (const Exception& e) {
@@ -107,6 +136,17 @@ LibDHCP::packOptions6(boost::shared_array<uint8_t> data,
     return (offset);
 }
 
+void
+LibDHCP::packOptions(isc::util::OutputBuffer& buf,
+                     const Option::OptionCollection& options) {
+    for (Option::OptionCollection::const_iterator it = options.begin();
+         it != options.end();
+         ++it) {
+        it->second->pack4(buf);
+    }
+}
+
+
 bool
 LibDHCP::OptionFactoryRegister(Option::Universe u,
                                unsigned short opt_type,

src/lib/dhcp/libdhcp.h

@@ -16,7 +16,8 @@
 #define LIBDHCP_H_
 
 #include <iostream>
-#include "dhcp/pkt6.h"
+#include <util/buffer.h>
+#include <dhcp/pkt6.h>
 
 namespace isc {
 namespace dhcp {
@@ -39,8 +40,27 @@ public:
     static unsigned int
     packOptions6(boost::shared_array<uint8_t> buf, unsigned int buf_len,
                  unsigned int offset,
-                 const isc::dhcp::Option::Option6Collection& options);
+                 const isc::dhcp::Option::OptionCollection& options);
 
+
+    /// @brief Stores options in a buffer.
+    ///
+    /// Stores all options defined in options containers in a on-wire
+    /// format in output buffer specified by buf.
+    ///
+    /// May throw different exceptions if option assembly fails. There
+    /// may be different reasons (option too large, option malformed,
+    /// too many options etc.)
+    ///
+    /// @param buf
+    /// @param options
+    static void
+    packOptions(isc::util::OutputBuffer& buf,
+                const isc::dhcp::Option::OptionCollection& options);
+
+    static void
+    unpackOptions4(const std::vector<uint8_t>& buf,
+                   isc::dhcp::Option::OptionCollection& options);
     ///
     /// Parses provided buffer and creates Option objects.
     ///
@@ -57,7 +77,7 @@ public:
     static unsigned int
     unpackOptions6(const boost::shared_array<uint8_t> buf, unsigned int buf_len,
                    unsigned int offset, unsigned int parse_len,
-                   isc::dhcp::Option::Option6Collection& options_);
+                   isc::dhcp::Option::OptionCollection& options_);
 
     ///
     /// Registers factory method that produces options of specific option types.

src/lib/dhcp/option.cc

@@ -29,50 +29,117 @@ using namespace isc::dhcp;
 using namespace isc::util;
 
 Option::Option(Universe u, unsigned short type)
-    :universe_(u), type_(type), data_len_(0) {
-
+    :universe_(u), type_(type) {
 
+    if ((u == V4) && (type > 255)) {
+        isc_throw(BadValue, "Can't create V4 option of type "
+                  << type << ", V4 options are in range 0..255");
+    }
 }
 
 Option::Option(Universe u, unsigned short type,
                const boost::shared_array<uint8_t>& buf,
                unsigned int offset, unsigned int len)
-    :universe_(u), type_(type), data_(buf),
-     data_len_(len), offset_(offset)
-      {
+    :universe_(u), type_(type),
+     offset_(offset)
+{
+    uint8_t* ptr = &buf[offset];
+    data_ = std::vector<uint8_t>(ptr, ptr + len);
+
+    check();
+}
+
+Option::Option(Universe u, unsigned short type, std::vector<uint8_t>& data)
+    :universe_(u), type_(type), data_(data) {
+    check();
+}
 
-    // sanity checks
-    // TODO: universe must be in V4 and V6
+Option::Option(Universe u, uint16_t type, vector<uint8_t>::const_iterator first,
+               vector<uint8_t>::const_iterator last)
+    :universe_(u), type_(type), data_(std::vector<uint8_t>(first,last)) {
+    check();
+}
+
+void
+Option::check() {
+    if ( (universe_ != V4) && (universe_ != V6) ) {
+        isc_throw(BadValue, "Invalid universe type specified."
+                  << "Only V4 and V6 are allowed.");
+    }
+
+    if (universe_ == V4) {
+
+        if (type_ > 255) {
+            isc_throw(OutOfRange, "DHCPv4 Option type " << type_ << " is too big."
+                      << "For DHCPv4 allowed type range is 0..255");
+        } else if (data_.size() > 255) {
+            isc_throw(OutOfRange, "DHCPv4 Option " << type_ << " is too big.");
+            /// TODO Larger options can be stored as separate instances
+            /// of DHCPv4 options. Clients MUST concatenate them.
+            /// Fortunately, there are no such large options used today.
+        }
+    }
+
+    // no need to check anything for DHCPv6. It allows full range (0-64k) of
+    // both types and data size.
 }
 
 unsigned int
 Option::pack(boost::shared_array<uint8_t>& buf,
              unsigned int buf_len,
              unsigned int offset) {
+    if (universe_ != V6) {
+        isc_throw(BadValue, "Failed to pack " << type_ << " option. Do not "
+                  << "use this method for options other than DHCPv6.");
+    }
+    return pack6(buf, buf_len, offset);
+}
+
+void
+Option::pack4(isc::util::OutputBuffer& buf) {
     switch (universe_) {
-    case V4:
-        return pack4(buf, buf_len, offset);
+    case V4: {
+        if (data_.size() > 255) {
+            isc_throw(OutOfRange, "DHCPv4 Option " << type_ << " is too big."
+                      << "At most 255 bytes are supported.");
+            /// TODO Larger options can be stored as separate instances
+            /// of DHCPv4 options. Clients MUST concatenate them.
+            /// Fortunately, there are no such large options used today.
+        }
+
+        buf.writeUint8(type_);
+        buf.writeUint8(len() - getHeaderLen());
+
+        buf.writeData(&data_[0], data_.size());
+
+        LibDHCP::packOptions(buf, options_);
+        return;
+    }
     case V6:
-        return pack6(buf, buf_len, offset);
+        /// TODO: Do we need a sanity check for option size here?
+        buf.writeUint16(type_);
+        buf.writeUint16(len() - getHeaderLen());
+
+        LibDHCP::packOptions(buf, options_);
+        return;
     default:
-        isc_throw(BadValue, "Unknown universe defined for Option " << type_);
+        isc_throw(OutOfRange, "Invalid universe type" << universe_);
     }
 }
 
-
 unsigned int
 Option::pack4(boost::shared_array<uint8_t>& buf,
              unsigned int buf_len,
              unsigned int offset) {
-    if ( offset+len() > buf_len ) {
+    if (offset + len() > buf_len) {
         isc_throw(OutOfRange, "Failed to pack v4 option=" <<
-                  type_ << ",len=" << data_len_ << ": too small buffer.");
+                  type_ << ",len=" << len() << ": too small buffer.");
     }
     uint8_t *ptr = &buf[offset];
     ptr[0] = type_;
-    ptr[1] = data_len_;
+    ptr[1] = len() - getHeaderLen();
     ptr += 2;
-    memcpy(ptr, &data_[0], data_len_);
+    memcpy(ptr, &data_[0], data_.size());
 
     return offset + len();
 }
@@ -81,22 +148,22 @@ unsigned int
 Option::pack6(boost::shared_array<uint8_t>& buf,
              unsigned int buf_len,
              unsigned int offset) {
-    if ( offset+len() > buf_len ) {
+    if (offset+len() > buf_len) {
         isc_throw(OutOfRange, "Failed to pack v6 option=" <<
                   type_ << ",len=" << len() << ": too small buffer.");
     }
 
-    uint8_t * ptr = &buf[offset];
+    uint8_t* ptr = &buf[offset];
 
     ptr = writeUint16(type_, ptr);
 
     ptr = writeUint16(len() - getHeaderLen(), ptr);
 
-    if (data_len_)
-        memcpy(ptr, &data_[offset_], data_len_);
+    if (! data_.empty())
+        memcpy(ptr, &data_[0], data_.size());
 
     // end of fixed part of this option
-    offset += OPTION6_HDR_LEN + data_len_;
+    offset += OPTION6_HDR_LEN + data_.size();
 
     return LibDHCP::packOptions6(buf, buf_len, offset, options_);
 }
@@ -140,22 +207,27 @@ Option::unpack6(const boost::shared_array<uint8_t>& buf,
                   << "): too small buffer.");
     }
 
-    data_ = buf;
+    uint8_t* ptr = &buf[offset];
+    data_ = std::vector<uint8_t>(ptr, ptr + parse_len);
+
     offset_ = offset;
-    data_len_ = buf_len;
 
-    return LibDHCP::unpackOptions6(buf, buf_len, offset, parse_len,
-                                   options_);
+    return (offset+parse_len);
+
+    //return LibDHCP::unpackOptions6(buf, buf_len, offset, parse_len,
+    //                               options_);
 }
 
+/// Returns length of the complete option (data length + DHCPv4/DHCPv6
+/// option header)
 unsigned short
 Option::len() {
 
     // length of the whole option is header and data stored in this option...
-    int length = getHeaderLen() + data_len_;
+    int length = getHeaderLen() + data_.size();
 
     // ... and sum of lengths of all suboptions
-    for (Option::Option6Collection::iterator it = options_.begin();
+    for (Option::OptionCollection::iterator it = options_.begin();
          it != options_.end();
          ++it) {
         length += (*it).second->len();
@@ -177,16 +249,9 @@ Option::valid() {
     return (true);
 }
 
-void
-isc::dhcp::Option::addOption(boost::shared_ptr<isc::dhcp::Option> opt) {
-    options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(),
-                                                            opt));
-
-}
-
 boost::shared_ptr<isc::dhcp::Option>
 Option::getOption(unsigned short opt_type) {
-    isc::dhcp::Option::Option6Collection::const_iterator x =
+    isc::dhcp::Option::OptionCollection::const_iterator x =
         options_.find(opt_type);
     if ( x != options_.end() ) {
         return (*x).second;
@@ -196,7 +261,7 @@ Option::getOption(unsigned short opt_type) {
 
 bool
 Option::delOption(unsigned short opt_type) {
-    isc::dhcp::Option::Option6Collection::iterator x = options_.find(opt_type);
+    isc::dhcp::Option::OptionCollection::iterator x = options_.find(opt_type);
     if ( x != options_.end() ) {
         options_.erase(x);
         return true; // delete successful
@@ -208,22 +273,22 @@ Option::delOption(unsigned short opt_type) {
 std::string Option::toText(int indent /* =0 */ ) {
     std::stringstream tmp;
 
-    for (int i=0; i<indent; i++)
+    for (int i = 0; i < indent; i++)
         tmp << " ";
 
-    tmp << "type=" << type_ << ", len=" << data_len_ << ": ";
+    tmp << "type=" << type_ << ", len=" << len()-getHeaderLen() << ": ";
 
-    for (unsigned int i=0; i<data_len_; i++) {
+    for (unsigned int i = 0; i < data_.size(); i++) {
         if (i) {
             tmp << ":";
         }
         tmp << setfill('0') << setw(2) << hex
-            << static_cast<unsigned short>(data_[offset_+i]);
+            << static_cast<unsigned short>(data_[i]);
     }
 
     // print suboptions
-    for (Option6Collection::const_iterator opt=options_.begin();
-         opt!=options_.end();
+    for (OptionCollection::const_iterator opt = options_.begin();
+         opt != options_.end();
          ++opt) {
         tmp << (*opt).second->toText(indent+2);
     }
@@ -235,13 +300,9 @@ Option::getType() {
     return type_;
 }
 
-uint8_t*
+const std::vector<uint8_t>&
 Option::getData() {
-    if (data_len_) {
-        return (&data_[offset_]);
-    } else {
-        return (NULL);
-    }
+    return (data_);
 }
 
 unsigned short
@@ -255,6 +316,18 @@ Option::getHeaderLen() {
     return 0; // should not happen
 }
 
+void
+Option::addOption(boost::shared_ptr<Option> opt) {
+    if (universe_ == V4) {
+        // check for uniqueness (DHCPv4 options must be unique)
+        if (getOption(opt->getType())) {
+            isc_throw(BadValue, "Option " << opt->getType()
+                      << " already present in this message.");
+        }
+    }
+    options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(), opt));
+}
+
 Option::~Option() {
 
 }

src/lib/dhcp/option.h

@@ -17,8 +17,10 @@
 
 #include <string>
 #include <map>
+#include <vector>
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_array.hpp>
+#include <util/buffer.h>
 
 namespace isc {
 namespace dhcp {
@@ -34,13 +36,9 @@ public:
     /// defines option universe DHCPv4 or DHCPv6
     enum Universe { V4, V6 };
 
-    /// a collection of DHCPv4 options
-    typedef std::map<unsigned int, boost::shared_ptr<Option> >
-    Option4Collection;
-
     /// a collection of DHCPv6 options
     typedef std::multimap<unsigned int, boost::shared_ptr<Option> >
-    Option6Collection;
+    OptionCollection;
 
     /// @brief a factory function prototype
     ///
@@ -80,11 +78,55 @@ public:
            const boost::shared_array<uint8_t>& buf, unsigned int offset,
            unsigned int len);
 
-    /// @brief writes option in wire-format to buf
+    /// @brief Constructor, used for received options.
+    ///
+    /// This constructor takes vector<uint8_t>& which is used in cases
+    /// when content of the option will be copied and stored within
+    /// option object. V4 Options follow that approach already.
+    /// TODO Migrate V6 options to that approach.
+    ///
+    /// @param u specifies universe (V4 or V6)
+    /// @param type option type (0-255 for V4 and 0-65535 for V6)
+    /// @param data content of the option
+    Option(Universe u, unsigned short type, std::vector<uint8_t>& data);
+
+    /// @brief Constructor, used for received options.
+    ///
+    /// This contructor is similar to the previous one, but it does not take
+    /// the whole vector<uint8_t>, but rather subset of it.
+    ///
+    /// TODO: This can be templated to use different containers, not just
+    /// vector. Prototype should look like this:
+    /// template<typename InputIterator> Option(Universe u, uint16_t type,
+    /// InputIterator first, InputIterator last);
+    ///
+    /// vector<int8_t> myData;
+    /// Example usage: new Option(V4, 123, myData.begin()+1, myData.end()-1)
+    /// This will create DHCPv4 option of type 123 that contains data from
+    /// trimmed (first and last byte removed) myData vector.
+    ///
+    /// @param u specifies universe (V4 or V6)
+    /// @param type option type (0-255 for V4 and 0-65535 for V6)
+    /// @param first iterator to the first element that should be copied
+    /// @param last iterator to the next element after the last one
+    ///        to be copied.
+    Option(Universe u, uint16_t type,
+           std::vector<uint8_t>::const_iterator first,
+           std::vector<uint8_t>::const_iterator last);
+
+    /// @brief returns option universe (V4 or V6)
+    ///
+    /// @return universe type
+    Universe
+    getUniverse() { return universe_; };
+
+    /// @brief Writes option in wire-format to a buffer.
     ///
     /// Writes option in wire-format to buffer, returns pointer to first unused
     /// byte after stored option (that is useful for writing options one after
-    /// another)
+    /// another). Used in DHCPv6 options.
+    ///
+    /// TODO: Migrate DHCPv6 code to pack(OutputBuffer& buf) version
     ///
     /// @param buf pointer to a buffer
     /// @param buf_len length of the buffer
@@ -93,10 +135,21 @@ public:
     /// @return offset to first unused byte after stored option
     ///
     virtual unsigned int
-    pack(boost::shared_array<uint8_t>& buf,
-         unsigned int buf_len,
+    pack(boost::shared_array<uint8_t>& buf, unsigned int buf_len,
          unsigned int offset);
 
+    /// @brief Writes option in a wire-format to a buffer.
+    ///
+    /// Method will throw if option storing fails for some reason.
+    ///
+    /// TODO Once old (DHCPv6) implementation is rewritten,
+    /// unify pack4() and pack6() and rename them to just pack().
+    ///
+    /// @param buf output buffer (option will be stored there)
+    virtual void
+    pack4(isc::util::OutputBuffer& buf);
+
+
     /// @brief Parses buffer.
     ///
     /// Parses received buffer, returns offset to the first unused byte after
@@ -150,7 +203,7 @@ public:
     /// Returns pointer to actual data.
     ///
     /// @return pointer to actual data (or NULL if there is no data)
-    virtual uint8_t*
+    virtual const std::vector<uint8_t>&
     getData();
 
     /// Adds a sub-option.
@@ -242,26 +295,31 @@ protected:
             unsigned int offset,
             unsigned int parse_len);
 
+    /// @brief A private method used for option correctness.
+    ///
+    /// It is used in constructors. In there are any problems detected
+    /// (like specifying type > 255 for DHCPv4 option), it will throw
+    /// BadValue or OutOfRange exceptions.
+    void check();
+
     /// option universe (V4 or V6)
     Universe universe_;
 
     /// option type (0-255 for DHCPv4, 0-65535 for DHCPv6)
     unsigned short type_;
 
-    /// shared pointer to a buffer (usually a part of packet)
-    boost::shared_array<uint8_t> data_;
-
-    /// length of data only. Use len() if you want to
-    /// know proper length with option header overhead
-    unsigned int data_len_;
+    /// contains content of this data
+    std::vector<uint8_t> data_;
 
+    /// TODO: Remove this field. vector<uint8_t> should be used
+    /// instead.
     /// data is a shared_pointer that points out to the
     /// whole packet. offset_ specifies where data for
     /// this option begins.
     unsigned int offset_;
 
     /// collection for storing suboptions
-    Option6Collection options_;
+    OptionCollection options_;
 
     /// TODO: probably 2 different containers have to be used for v4 (unique
     /// options) and v6 (options with the same type can repeat)

src/lib/dhcp/option6_ia.cc

@@ -113,7 +113,7 @@ std::string Option6IA::toText(int indent /* = 0*/) {
     tmp << " iaid=" << iaid_ << ", t1=" << t1_ << ", t2=" << t2_
         << " " << options_.size() << " sub-options:" << endl;
 
-    for (Option6Collection::const_iterator opt=options_.begin();
+    for (OptionCollection::const_iterator opt=options_.begin();
          opt!=options_.end();
          ++opt) {
         tmp << (*opt).second->toText(indent+2);
@@ -127,7 +127,7 @@ unsigned short Option6IA::len() {
         OPTION6_IA_LEN  /* option content (12) */;
 
     // length of all suboptions
-    for (Option::Option6Collection::iterator it = options_.begin();
+    for (Option::OptionCollection::iterator it = options_.begin();
          it != options_.end();
          ++it) {
         length += (*it).second->len();

src/lib/dhcp/option6_iaaddr.cc

@@ -108,7 +108,7 @@ std::string Option6IAAddr::toText(int indent /* =0 */) {
         << ", preferred-lft=" << preferred_  << ", valid-lft="
         << valid_ << endl;
 
-    for (Option6Collection::const_iterator opt=options_.begin();
+    for (OptionCollection::const_iterator opt=options_.begin();
          opt!=options_.end();
          ++opt) {
         tmp << (*opt).second->toText(indent+2);
@@ -123,7 +123,7 @@ unsigned short Option6IAAddr::len() {
     // length of all suboptions
     // TODO implement:
     // protected: unsigned short Option::lenHelper(int header_size);
-    for (Option::Option6Collection::iterator it = options_.begin();
+    for (Option::OptionCollection::iterator it = options_.begin();
          it != options_.end();
          ++it) {
         length += (*it).second->len();

src/lib/dhcp/pkt4.cc

@@ -88,7 +88,13 @@ size_t
 Pkt4::len() {
     size_t length = DHCPV4_PKT_HDR_LEN; // DHCPv4 header
 
-    /// TODO: Include options here (ticket #1228)
+    // ... and sum of lengths of all options
+    for (Option::OptionCollection::const_iterator it = options_.begin();
+         it != options_.end();
+         ++it) {
+        length += (*it).second->len();
+    }
+
     return (length);
 }
 
@@ -109,7 +115,7 @@ Pkt4::pack() {
     bufferOut_.writeData(sname_, MAX_SNAME_LEN);
     bufferOut_.writeData(file_, MAX_FILE_LEN);
 
-    /// TODO: Options should follow here (ticket #1228)
+    LibDHCP::packOptions(bufferOut_, options_);
 
     return (true);
 }
@@ -136,7 +142,11 @@ Pkt4::unpack() {
     bufferIn_.readData(sname_, MAX_SNAME_LEN);
     bufferIn_.readData(file_, MAX_FILE_LEN);
 
-    /// TODO: Parse options here (ticket #1228)
+    size_t opts_len = bufferIn_.getLength() - bufferIn_.getPosition();
+    vector<uint8_t> optsBuffer;
+    // fist use of readVector
+    bufferIn_.readVector(optsBuffer, opts_len);
+    LibDHCP::unpackOptions4(optsBuffer, options_);
 
     return (true);
 }
@@ -220,6 +230,26 @@ Pkt4::DHCPTypeToBootpType(uint8_t dhcpType) {
     }
 }
 
+void
+Pkt4::addOption(boost::shared_ptr<Option> opt) {
+    // check for uniqueness (DHCPv4 options must be unique)
+    if (getOption(opt->getType())) {
+        isc_throw(BadValue, "Option " << opt->getType()
+                  << " already present in this message.");
+    }
+    options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(), opt));
+}
+
+boost::shared_ptr<isc::dhcp::Option>
+Pkt4::getOption(uint8_t type) {
+    Option::OptionCollection::const_iterator x = options_.find(type);
+    if (x!=options_.end()) {
+        return (*x).second;
+    }
+    return boost::shared_ptr<isc::dhcp::Option>(); // NULL
+}
+
+
 } // end of namespace isc::dhcp
 
 } // end of namespace isc

src/lib/dhcp/pkt4.h

@@ -264,7 +264,7 @@ public:
     uint8_t
     getHlen() const { return (hlen_); };
 
-    /// @brief Returns chaddr field
+    /// @brief Returns chaddr field.
     ///
     /// Note: This is 16 bytes long field. It doesn't have to be
     /// null-terminated. Do no use strlen() or similar on it.
@@ -274,7 +274,7 @@ public:
     getChaddr() const { return (chaddr_); };
 
 
-    /// Returns reference to output buffer
+    /// @brief Returns reference to output buffer.
     ///
     /// Returned buffer will contain reasonable data only for
     /// output (TX) packet and after pack() was called. This buffer
@@ -287,6 +287,22 @@ public:
     const isc::util::OutputBuffer&
     getBuffer() const { return (bufferOut_); };
 
+    /// @brief Add an option.
+    ///
+    /// Throws BadValue if option with that type is already present.
+    ///
+    /// @param opt option to be added
+    void
+    addOption(boost::shared_ptr<Option> opt);
+
+    /// @brief Returns an option of specified type.
+    ///
+    /// @return returns option of requested type (or NULL)
+    ///         if no such option is present
+
+    boost::shared_ptr<Option>
+    getOption(uint8_t opt_type);
+
 protected:
 
     /// converts DHCP message type to BOOTP op type
@@ -383,7 +399,7 @@ protected:
     uint8_t msg_type_;
 
     /// collection of options present in this message
-    isc::dhcp::Option::Option4Collection options_;
+    isc::dhcp::Option::OptionCollection options_;
 }; // Pkt4 class
 
 } // isc::dhcp namespace

src/lib/dhcp/pkt6.cc

@@ -63,7 +63,7 @@ unsigned short
 Pkt6::len() {
     unsigned int length = DHCPV6_PKT_HDR_LEN; // DHCPv6 header
 
-    for (Option::Option6Collection::iterator it = options_.begin();
+    for (Option::OptionCollection::iterator it = options_.begin();
          it != options_.end();
          ++it) {
         length += (*it).second->len();
@@ -197,7 +197,7 @@ Pkt6::toText() {
         << "]:" << remote_port_ << endl;
     tmp << "msgtype=" << msg_type_ << ", transid=0x" << hex << transid_
         << dec << endl;
-    for (isc::dhcp::Option::Option6Collection::iterator opt=options_.begin();
+    for (isc::dhcp::Option::OptionCollection::iterator opt=options_.begin();
          opt != options_.end();
          ++opt) {
         tmp << opt->second->toText() << std::endl;
@@ -207,7 +207,7 @@ Pkt6::toText() {
 
 boost::shared_ptr<isc::dhcp::Option>
 Pkt6::getOption(unsigned short opt_type) {
-    isc::dhcp::Option::Option6Collection::const_iterator x = options_.find(opt_type);
+    isc::dhcp::Option::OptionCollection::const_iterator x = options_.find(opt_type);
     if (x!=options_.end()) {
         return (*x).second;
     }
@@ -221,7 +221,7 @@ Pkt6::addOption(boost::shared_ptr<Option> opt) {
 
 bool
 Pkt6::delOption(unsigned short type) {
-    isc::dhcp::Option::Option6Collection::iterator x = options_.find(type);
+    isc::dhcp::Option::OptionCollection::iterator x = options_.find(type);
     if (x!=options_.end()) {
         options_.erase(x);
         return (true); // delete successful

src/lib/dhcp/pkt6.h

@@ -180,7 +180,7 @@ public:
     /// TODO Need to implement getOptions() as well
 
     /// collection of options present in this message
-    isc::dhcp::Option::Option6Collection options_;
+    isc::dhcp::Option::OptionCollection options_;
 
 protected:
     /// Builds on wire packet for TCP transmission.

src/lib/dhcp/tests/Makefile.am

@@ -1,8 +1,6 @@
 SUBDIRS = .
 
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/asiolink
-AM_CPPFLAGS += -I$(top_builddir)/src/lib/asiolink
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 

src/lib/dhcp/tests/libdhcp_unittest.cc

@@ -15,16 +15,16 @@
 #include <config.h>
 #include <iostream>
 #include <sstream>
-
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
-
-#include "dhcp/libdhcp.h"
+#include <util/buffer.h>
+#include <dhcp/libdhcp.h>
 #include "config.h"
 
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::util;
 
 namespace {
 class LibDhcpTest : public ::testing::Test {
@@ -41,9 +41,9 @@ static const uint8_t packed[] = {
     1,  1, 0, 1, 114 // opt5 (5 bytes)
 };
 
-TEST_F(LibDhcpTest, packOptions6) {
+TEST(LibDhcpTest, packOptions6) {
     boost::shared_array<uint8_t> buf(new uint8_t[512]);
-    isc::dhcp::Option::Option6Collection opts; // list of options
+    isc::dhcp::Option::OptionCollection opts; // list of options
 
     // generate content for options
     for (int i = 0; i < 64; i++) {
@@ -70,13 +70,13 @@ TEST_F(LibDhcpTest, packOptions6) {
     EXPECT_EQ(0, memcmp(&buf[100], packed, 35) );
 }
 
-TEST_F(LibDhcpTest, unpackOptions6) {
+TEST(LibDhcpTest, unpackOptions6) {
 
     // just couple of random options
     // Option is used as a simple option implementation
     // More advanced uses are validated in tests dedicated for
     // specific derived classes.
-    isc::dhcp::Option::Option6Collection options; // list of options
+    isc::dhcp::Option::OptionCollection options; // list of options
 
     // we can't use packed directly, as shared_array would try to
     // free it eventually
@@ -91,35 +91,35 @@ TEST_F(LibDhcpTest, unpackOptions6) {
     EXPECT_EQ(35, offset); // parsed first 35 bytes (offset 0..34)
     EXPECT_EQ(options.size(), 5); // there should be 5 options
 
-    isc::dhcp::Option::Option6Collection::const_iterator x = options.find(12);
+    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(9, x->second->len()); // it should be of length 9
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+4, 5)); // data len=5
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+4, 5)); // data len=5
 
     x = options.find(13);
     ASSERT_FALSE(x == options.end()); // option 13 should exist
     EXPECT_EQ(13, x->second->getType());  // this should be option 13
     ASSERT_EQ(7, x->second->len()); // it should be of length 7
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+13, 3)); // data len=3
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+13, 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(6, x->second->len()); // it should be of length 6
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+20, 2)); // data len=2
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+20, 2)); // data len=2
 
     x = options.find(256);
     ASSERT_FALSE(x == options.end()); // option 256 should exist
     EXPECT_EQ(256, x->second->getType());  // this should be option 256
     ASSERT_EQ(8, x->second->len()); // it should be of length 7
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+26, 4)); // data len=4
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+26, 4)); // data len=4
 
     x = options.find(257);
     ASSERT_FALSE(x == options.end()); // option 257 should exist
     EXPECT_EQ(257, x->second->getType());  // this should be option 257
     ASSERT_EQ(5, x->second->len()); // it should be of length 5
-    EXPECT_EQ(0, memcmp(x->second->getData(), packed+34, 1)); // data len=1
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+34, 1)); // data len=1
 
     x = options.find(0);
     EXPECT_TRUE(x == options.end()); // option 0 not found
@@ -134,4 +134,101 @@ TEST_F(LibDhcpTest, unpackOptions6) {
     EXPECT_TRUE(x == options.end()); // option 32000 not found
 }
 
+
+static uint8_t v4Opts[] = {
+    12,  3, 0,   1,  2,
+    13,  3, 10, 11, 12,
+    14,  3, 20, 21, 22,
+    254, 3, 30, 31, 32,
+    128, 3, 40, 41, 42
+};
+
+TEST(LibDhcpTest, packOptions4) {
+
+    vector<uint8_t> payload[5];
+    for (int i = 0; i < 5; i++) {
+        payload[i].resize(3);
+        payload[i][0] = i*10;
+        payload[i][1] = i*10+1;
+        payload[i][2] = i*10+2;
+    }
+
+    boost::shared_ptr<Option> opt1(new Option(Option::V4, 12, payload[0]));
+    boost::shared_ptr<Option> opt2(new Option(Option::V4, 13, payload[1]));
+    boost::shared_ptr<Option> opt3(new Option(Option::V4, 14, payload[2]));
+    boost::shared_ptr<Option> opt4(new Option(Option::V4,254, payload[3]));
+    boost::shared_ptr<Option> opt5(new Option(Option::V4,128, payload[4]));
+
+    isc::dhcp::Option::OptionCollection opts; // list of options
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt1));
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt2));
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt3));
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt4));
+    opts.insert(pair<int, boost::shared_ptr<Option> >(opt1->getType(), opt5));
+
+    vector<uint8_t> expVect(v4Opts, v4Opts + sizeof(v4Opts));
+
+    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)));
+
+}
+
+TEST(LibDhcpTest, unpackOptions4) {
+
+    vector<uint8_t> packed(v4Opts, v4Opts + sizeof(v4Opts));
+    isc::dhcp::Option::OptionCollection options; // list of options
+
+    ASSERT_NO_THROW(
+        LibDHCP::unpackOptions4(packed, options);
+    );
+
+    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
+
+    x = options.find(13);
+    ASSERT_FALSE(x == options.end()); // option 1 should exist
+    EXPECT_EQ(13, x->second->getType());  // this should be option 13
+    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
+
+    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
+
+    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
+
+    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
+
+    x = options.find(0);
+    EXPECT_TRUE(x == options.end()); // option 0 not found
+
+    x = options.find(1);
+    EXPECT_TRUE(x == options.end()); // option 1 not found
+
+    x = options.find(2);
+    EXPECT_TRUE(x == options.end()); // option 2 not found
+}
+
 }

src/lib/dhcp/tests/option6_addrlst_unittest.cc

@@ -15,14 +15,12 @@
 #include <config.h>
 #include <iostream>
 #include <sstream>
-
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
-
-#include "io_address.h"
-#include "dhcp/dhcp6.h"
-#include "dhcp/option.h"
-#include "dhcp/option6_addrlst.h"
+#include <asiolink/io_address.h>
+#include <dhcp/dhcp6.h>
+#include <dhcp/option.h>
+#include <dhcp/option6_addrlst.h>
 
 using namespace std;
 using namespace isc;
@@ -38,10 +36,10 @@ public:
 
 TEST_F(Option6AddrLstTest, basic) {
 
-    // limiting tests to just a 2001:db8::/32 as is *wrong*.
+    // Limiting tests to just a 2001:db8::/32 as is *wrong*.
     // Good tests check corner cases as well.
     // ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff checks
-    // for integer overflow
+    // for integer overflow.
     // ff02::face:b00c checks if multicast addresses
     // can be represented properly.
 
@@ -111,6 +109,8 @@ TEST_F(Option6AddrLstTest, basic) {
         opt1 = new Option6AddrLst(D6O_NAME_SERVERS, buf, 128, 0, 16);
     );
 
+    EXPECT_EQ(Option::V6, opt1->getUniverse());
+
     EXPECT_EQ(D6O_NAME_SERVERS, opt1->getType());
     EXPECT_EQ(20, opt1->len());
     Option6AddrLst::AddressContainer addrs = opt1->getAddresses();
@@ -178,6 +178,7 @@ TEST_F(Option6AddrLstTest, constructors) {
     EXPECT_NO_THROW(
         opt1 = new Option6AddrLst(1234, IOAddress("::1"));
     );
+    EXPECT_EQ(Option::V6, opt1->getUniverse());
     EXPECT_EQ(1234, opt1->getType());
 
     Option6AddrLst::AddressContainer addrs = opt1->getAddresses();

src/lib/dhcp/tests/option6_ia_unittest.cc

@@ -67,6 +67,7 @@ TEST_F(Option6IATest, basic) {
                                    0,
                                    12);
 
+    EXPECT_EQ(Option::V6, opt->getUniverse());
     EXPECT_EQ(D6O_IA_NA, opt->getType());
     EXPECT_EQ(0xa1a2a3a4, opt->getIAID());
     EXPECT_EQ(0x81020304, opt->getT1());
@@ -121,6 +122,7 @@ TEST_F(Option6IATest, simple) {
     ia->setT1(2345);
     ia->setT2(3456);
 
+    EXPECT_EQ(Option::V6, ia->getUniverse());
     EXPECT_EQ(D6O_IA_NA, ia->getType());
     EXPECT_EQ(1234, ia->getIAID());
     EXPECT_EQ(2345, ia->getT1());
@@ -251,7 +253,7 @@ TEST_F(Option6IATest, suboptions_unpack) {
     EXPECT_EQ(0xcafe, subopt->getType());
     EXPECT_EQ(4, subopt->len());
     // there should be no data at all
-    EXPECT_EQ(static_cast<void*>(NULL), subopt->getData());
+    EXPECT_EQ(0, subopt->getData().size());
 
     subopt = ia->getOption(1); // get option 1
     ASSERT_FALSE(subopt); // should be NULL

src/lib/dhcp/tests/option6_iaaddr_unittest.cc

@@ -75,6 +75,8 @@ TEST_F(Option6IAAddrTest, basic) {
 
     EXPECT_EQ(78, offset);
 
+    EXPECT_EQ(Option::V6, opt->getUniverse());
+
     // 4 bytes header + 4 bytes content
     EXPECT_EQ("2001:db8:1::dead:beef", opt->getAddress().toText());
     EXPECT_EQ(1000, opt->getPreferred());

src/lib/dhcp/tests/option_unittest.cc

@@ -19,6 +19,8 @@
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
 #include <boost/shared_ptr.hpp>
+#include <exceptions/exceptions.h>
+#include <util/buffer.h>
 
 #include "dhcp/dhcp6.h"
 #include "dhcp/option.h"
@@ -26,6 +28,7 @@
 using namespace std;
 using namespace isc;
 using namespace isc::dhcp;
+using namespace isc::util;
 
 namespace {
 class OptionTest : public ::testing::Test {
@@ -35,26 +38,162 @@ public:
 };
 
 // v4 is not really implemented yet. A simple test will do for now
-TEST_F(OptionTest, basic4) {
+TEST_F(OptionTest, v4_basic) {
 
-    Option* opt = new Option(Option::V4, 17);
+    Option* opt = 0;
+    EXPECT_NO_THROW(
+        opt = new Option(Option::V4, 17);
+    );
 
+    EXPECT_EQ(Option::V4, opt->getUniverse());
     EXPECT_EQ(17, opt->getType());
-    EXPECT_EQ(static_cast<uint8_t*>(NULL), opt->getData());
+    EXPECT_EQ(0, opt->getData().size());
     EXPECT_EQ(2, opt->len()); // just v4 header
 
     EXPECT_NO_THROW(
         delete opt;
     );
+    opt = 0;
+
+    // V4 options have type 0...255
+    EXPECT_THROW(
+        opt = new Option(Option::V4, 256),
+        BadValue
+    );
+    if (opt) {
+        delete opt;
+        opt = 0;
+    }
+}
+
+const uint8_t dummyPayload[] =
+{ 1, 2, 3, 4};
+
+TEST_F(OptionTest, v4_data1) {
+
+    vector<uint8_t> data(dummyPayload, dummyPayload + sizeof(dummyPayload));
+
+    Option* opt = 0;
+
+    // create DHCPv4 option of type 123
+    // that contains 4 bytes of data
+    ASSERT_NO_THROW(
+        opt= new Option(Option::V4,
+                        123, // type
+                        data);
+    );
+
+    // check that content is reported properly
+    EXPECT_EQ(123, opt->getType());
+    vector<uint8_t> optData = opt->getData();
+    ASSERT_EQ(optData.size(), data.size());
+    EXPECT_TRUE(optData == data);
+    EXPECT_EQ(2, opt->getHeaderLen());
+    EXPECT_EQ(6, opt->len());
+
+    // now store that option into a buffer
+    OutputBuffer buf(100);
+    EXPECT_NO_THROW(
+        opt->pack4(buf);
+    );
+
+    // check content of that buffer
+
+    // 2 byte header + 4 bytes data
+    ASSERT_EQ(6, buf.getLength());
+
+    // that's how this option is supposed to look like
+    uint8_t exp[] = { 123, 4, 1, 2, 3, 4 };
+
+    /// TODO: use vector<uint8_t> getData() when it will be implemented
+    EXPECT_EQ(0, memcmp(exp, buf.getData(), 6));
+
+    // check that we can destroy that option
+    EXPECT_NO_THROW(
+        delete opt;
+    );
+}
+
+// this is almost the same test as v4_data1, but it uses
+// different constructor
+TEST_F(OptionTest, v4_data2) {
+
+    vector<uint8_t> data(dummyPayload, dummyPayload + sizeof(dummyPayload));
+
+    vector<uint8_t> expData = data;
+
+    // Add fake data in front and end. Main purpose of this test is to check
+    // that only subset of the whole vector can be used for creating option.
+    data.insert(data.begin(), 56);
+    data.push_back(67);
+
+    // Data contains extra garbage at beginning and at the end. It should be
+    // ignored, as we pass interators to proper data. Only subset (limited by
+    // iterators) of the vector should be used.
+    // expData contains expected content (just valid data, without garbage).
+
+    Option* opt = 0;
+
+    // Create DHCPv4 option of type 123 that contains
+    // 4 bytes (sizeof(dummyPayload).
+    ASSERT_NO_THROW(
+        opt= new Option(Option::V4,
+                        123, // type
+                        data.begin() + 1,
+                        data.end() - 1);
+    );
+
+    // check that content is reported properly
+    EXPECT_EQ(123, opt->getType());
+    vector<uint8_t> optData = opt->getData();
+    ASSERT_EQ(optData.size(), expData.size());
+    EXPECT_TRUE(optData == expData);
+    EXPECT_EQ(2, opt->getHeaderLen());
+    EXPECT_EQ(6, opt->len());
+
+    // now store that option into a buffer
+    OutputBuffer buf(100);
+    EXPECT_NO_THROW(
+        opt->pack4(buf);
+    );
+
+    // check content of that buffer
+
+    // 2 byte header + 4 bytes data
+    ASSERT_EQ(6, buf.getLength());
+
+    // that's how this option is supposed to look like
+    uint8_t exp[] = { 123, 4, 1, 2, 3, 4 };
+
+    /// TODO: use vector<uint8_t> getData() when it will be implemented
+    EXPECT_EQ(0, memcmp(exp, buf.getData(), 6));
+
+    // check that we can destroy that option
+    EXPECT_NO_THROW(
+        delete opt;
+    );
+}
+
+TEST_F(OptionTest, v4_toText) {
+
+    vector<uint8_t> buf(3);
+    buf[0] = 0;
+    buf[1] = 0xf;
+    buf[2] = 0xff;
+
+    Option opt(Option::V4, 253, buf);
+
+    EXPECT_EQ("type=253, len=3: 00:0f:ff", opt.toText());
 }
 
 // tests simple constructor
-TEST_F(OptionTest, basic6) {
+TEST_F(OptionTest, v6_basic) {
 
     Option* opt = new Option(Option::V6, 1);
 
+    EXPECT_EQ(Option::V6, opt->getUniverse());
     EXPECT_EQ(1, opt->getType());
-    EXPECT_EQ(static_cast<uint8_t*>(NULL), opt->getData());
+    EXPECT_EQ(0, opt->getData().size());
     EXPECT_EQ(4, opt->len()); // just v6 header
 
     EXPECT_NO_THROW(
@@ -64,7 +203,7 @@ TEST_F(OptionTest, basic6) {
 
 // tests contructor used in pkt reception
 // option contains actual data
-TEST_F(OptionTest, data1) {
+TEST_F(OptionTest, v6_data1) {
     boost::shared_array<uint8_t> buf(new uint8_t[32]);
     for (int i = 0; i < 32; i++)
         buf[i] = 100+i;
@@ -73,9 +212,10 @@ TEST_F(OptionTest, data1) {
                              3, // offset
                              7); // 7 bytes of data
     EXPECT_EQ(333, opt->getType());
-    ASSERT_EQ(&buf[3], opt->getData());
+
     ASSERT_EQ(11, opt->len());
-    EXPECT_EQ(0, memcmp(&buf[3], opt->getData(), 7) );
+    ASSERT_EQ(7, opt->getData().size());
+    EXPECT_EQ(0, memcmp(&buf[3], &opt->getData()[0], 7) );
 
     int offset = opt->pack(buf, 32, 20);
     EXPECT_EQ(31, offset);
@@ -96,7 +236,7 @@ TEST_F(OptionTest, data1) {
 
 // another text that tests the same thing, just
 // with different input parameters
-TEST_F(OptionTest, data2) {
+TEST_F(OptionTest, v6_data2) {
 
     boost::shared_array<uint8_t> simple_buf(new uint8_t[128]);
     for (int i = 0; i < 128; i++)
@@ -144,7 +284,7 @@ TEST_F(OptionTest, data2) {
 //  |
 //  +----opt3
 //
-TEST_F(OptionTest, suboptions1) {
+TEST_F(OptionTest, v6_suboptions1) {
     boost::shared_array<uint8_t> buf(new uint8_t[128]);
     for (int i=0; i<128; i++)
         buf[i] = 100+i;
@@ -184,13 +324,13 @@ TEST_F(OptionTest, suboptions1) {
     );
 }
 
-// check that an option can contain 2 suboptions:
+// check that an option can contain nested suboptions:
 // opt1
 //  +----opt2
 //        |
 //        +----opt3
 //
-TEST_F(OptionTest, suboptions2) {
+TEST_F(OptionTest, v6_suboptions2) {
     boost::shared_array<uint8_t> buf(new uint8_t[128]);
     for (int i=0; i<128; i++)
         buf[i] = 100+i;
@@ -226,7 +366,7 @@ TEST_F(OptionTest, suboptions2) {
     );
 }
 
-TEST_F(OptionTest, addgetdel) {
+TEST_F(OptionTest, v6_addgetdel) {
     boost::shared_array<uint8_t> buf(new uint8_t[128]);
     for (int i=0; i<128; i++)
         buf[i] = 100+i;
@@ -266,7 +406,7 @@ TEST_F(OptionTest, addgetdel) {
 
 }
 
-TEST_F(OptionTest, toText) {
+TEST_F(OptionTest, v6_toText) {
     boost::shared_array<uint8_t> buf(new uint8_t[3]);
     buf[0] = 0;
     buf[1] = 0xf;

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -20,16 +20,17 @@
 #include <boost/static_assert.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_array.hpp>
-
-#include "io_address.h"
-#include "dhcp/pkt4.h"
-#include "dhcp/dhcp4.h"
-#include "exceptions/exceptions.h"
+#include <util/buffer.h>
+#include <asiolink/io_address.h>
+#include <dhcp/pkt4.h>
+#include <dhcp/dhcp4.h>
+#include <exceptions/exceptions.h>
 
 using namespace std;
 using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
+using namespace isc::util;
 using namespace boost;
 
 namespace {
@@ -282,6 +283,9 @@ TEST(Pkt4Test, hwAddr) {
     vector<uint8_t> mac;
     uint8_t expectedChaddr[Pkt4::MAX_CHADDR_LEN];
 
+    // We resize vector to specified length. It is more natural for fixed-length
+    // field, than clear it (shrink size to 0) and push_back each element
+    // (growing length back to MAX_CHADDR_LEN).
     mac.resize(Pkt4::MAX_CHADDR_LEN);
 
     Pkt4* pkt = 0;
@@ -433,4 +437,126 @@ TEST(Pkt4Test, file) {
 
 }
 
+static uint8_t v4Opts[] = {
+    12,  3, 0,   1,  2,
+    13,  3, 10, 11, 12,
+    14,  3, 20, 21, 22,
+    128, 3, 30, 31, 32,
+    254, 3, 40, 41, 42
+};
+
+TEST(Pkt4Test, options) {
+    Pkt4* pkt = new Pkt4(DHCPOFFER, 0);
+
+    vector<uint8_t> payload[5];
+    for (int i = 0; i < 5; i++) {
+        payload[i].push_back(i*10);
+        payload[i].push_back(i*10+1);
+        payload[i].push_back(i*10+2);
+    }
+
+    boost::shared_ptr<Option> opt1(new Option(Option::V4, 12, payload[0]));
+    boost::shared_ptr<Option> opt2(new Option(Option::V4, 13, payload[1]));
+    boost::shared_ptr<Option> opt3(new Option(Option::V4, 14, payload[2]));
+    boost::shared_ptr<Option> opt5(new Option(Option::V4,128, payload[3]));
+    boost::shared_ptr<Option> opt4(new Option(Option::V4,254, payload[4]));
+
+    pkt->addOption(opt1);
+    pkt->addOption(opt2);
+    pkt->addOption(opt3);
+    pkt->addOption(opt4);
+    pkt->addOption(opt5);
+
+    EXPECT_TRUE(pkt->getOption(12));
+    EXPECT_TRUE(pkt->getOption(13));
+    EXPECT_TRUE(pkt->getOption(14));
+    EXPECT_TRUE(pkt->getOption(128));
+    EXPECT_TRUE(pkt->getOption(254));
+    EXPECT_FALSE(pkt->getOption(127)); //  no such option
+
+    // options are unique in DHCPv4. It should not be possible
+    // to add more than one option of the same type.
+    EXPECT_THROW(
+        pkt->addOption(opt1),
+        BadValue
+    );
+
+    EXPECT_NO_THROW(
+        pkt->pack();
+    );
+
+    const OutputBuffer& buf = pkt->getBuffer();
+    // check that all options are stored, they should take sizeof(v4Opts)
+    ASSERT_EQ(static_cast<size_t>(Pkt4::DHCPV4_PKT_HDR_LEN) + sizeof(v4Opts),
+              buf.getLength());
+
+    // that that this extra data actually contain our options
+    const uint8_t* ptr = static_cast<const uint8_t*>(buf.getData());
+    ptr += Pkt4::DHCPV4_PKT_HDR_LEN; // rewind to end of fixed part
+    EXPECT_EQ(0, memcmp(ptr, v4Opts, sizeof(v4Opts)));
+
+    EXPECT_NO_THROW(
+        delete pkt;
+    );
+}
+
+TEST(Pkt4Test, unpackOptions) {
+
+    vector<uint8_t> expectedFormat = generateTestPacket2();
+
+    for (int i=0; i < sizeof(v4Opts); i++) {
+        expectedFormat.push_back(v4Opts[i]);
+    }
+
+    // now expectedFormat contains fixed format and 5 options
+
+    shared_ptr<Pkt4> pkt(new Pkt4(&expectedFormat[0],
+                                  expectedFormat.size()));
+
+    EXPECT_NO_THROW(
+        pkt->unpack()
+    );
+
+    EXPECT_TRUE(pkt->getOption(12));
+    EXPECT_TRUE(pkt->getOption(13));
+    EXPECT_TRUE(pkt->getOption(14));
+    EXPECT_TRUE(pkt->getOption(128));
+    EXPECT_TRUE(pkt->getOption(254));
+
+    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
+
+    x = pkt->getOption(13);
+    ASSERT_TRUE(x); // option 13 should exist
+    EXPECT_EQ(13, 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
+
+    x = pkt->getOption(14);
+    ASSERT_TRUE(x); // option 14 should exist
+    EXPECT_EQ(14, x->getType());  // this should be option 14
+    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+12, 3)); // data len=3
+
+    x = pkt->getOption(128);
+    ASSERT_TRUE(x); // option 3 should exist
+    EXPECT_EQ(128, x->getType());  // this should be option 254
+    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+17, 3)); // data len=3
+
+    x = pkt->getOption(254);
+    ASSERT_TRUE(x); // option 3 should exist
+    EXPECT_EQ(254, x->getType());  // this should be option 254
+    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+22, 3)); // data len=3
+}
+
 } // end of anonymous namespace

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -18,10 +18,10 @@
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
 
-#include "io_address.h"
-#include "dhcp/option.h"
-#include "dhcp/pkt6.h"
-#include "dhcp/dhcp6.h"
+#include <asiolink/io_address.h>
+#include <dhcp/option.h>
+#include <dhcp/pkt6.h>
+#include <dhcp/dhcp6.h>
 
 using namespace std;
 using namespace isc;

src/lib/util/buffer.h

@@ -207,6 +207,24 @@ public:
     }
     //@}
 
+    /// @brief Read specified number of bytes as a vector.
+    ///
+    /// If specified buffer is too short, it will be expanded
+    /// using vector::resize() method.
+    ///
+    /// @param Reference to a buffer (data will be stored there).
+    /// @param Size specified number of bytes to read in a vector.
+    ///
+    void readVector(std::vector<uint8_t>& data, size_t len)
+    {
+        if (position_ + len > len_) {
+            isc_throw(InvalidBufferPosition, "read beyond end of buffer");
+        }
+
+        data.resize(len);
+        readData(&data[0], len);
+    }
+
 private:
     size_t position_;
 
@@ -519,6 +537,6 @@ typedef boost::shared_ptr<OutputBuffer> OutputBufferPtr;
 } // namespace isc
 #endif  // __BUFFER_H
 
-// Local Variables: 
+// Local Variables:
 // mode: c++
-// End: 
+// End:

src/lib/util/tests/buffer_unittest.cc

@@ -239,4 +239,36 @@ TEST_F(BufferTest, outputBufferZeroSize) {
     });
 }
 
+TEST_F(BufferTest, readVectorAll) {
+    std::vector<uint8_t> vec;
+
+    // check that vector can read the whole buffer
+    ibuffer.readVector(vec, 5);
+
+    ASSERT_EQ(5, vec.size());
+    EXPECT_EQ(0, memcmp(&vec[0], testdata, 5));
+
+    // ibuffer is 5 bytes long. Can't read past it.
+    EXPECT_THROW(
+        ibuffer.readVector(vec, 1),
+        isc::util::InvalidBufferPosition
+    );
+}
+
+TEST_F(BufferTest, readVectorChunks) {
+    std::vector<uint8_t> vec;
+
+    // check that vector can read the whole buffer
+    ibuffer.readVector(vec, 3);
+    EXPECT_EQ(3, vec.size());
+
+    EXPECT_EQ(0, memcmp(&vec[0], testdata, 3));
+
+    EXPECT_NO_THROW(
+        ibuffer.readVector(vec, 2)
+    );
+
+    EXPECT_EQ(0, memcmp(&vec[0], testdata+3, 2));
+}
+
 }