[4497] Implemented copying options on retrieval from packet.

src/lib/dhcp/pkt.cc

@@ -24,7 +24,8 @@ Pkt::Pkt(uint32_t transid, const isc::asiolink::IOAddress& local_addr,
      remote_addr_(remote_addr),
      local_port_(local_port),
      remote_port_(remote_port),
-     buffer_out_(0)
+     buffer_out_(0),
+     copy_retrieved_options_(false)
 {
 }
 
@@ -38,7 +39,8 @@ Pkt::Pkt(const uint8_t* buf, uint32_t len, const isc::asiolink::IOAddress& local
      remote_addr_(remote_addr),
      local_port_(local_port),
      remote_port_(remote_port),
-     buffer_out_(0)
+     buffer_out_(0),
+     copy_retrieved_options_(false)
 {
 
     if (len != 0) {
@@ -56,10 +58,23 @@ Pkt::addOption(const OptionPtr& opt) {
 }
 
 OptionPtr
-Pkt::getOption(uint16_t type) const {
+Pkt::getNonCopiedOption(const uint16_t type) const {
     OptionCollection::const_iterator x = options_.find(type);
     if (x != options_.end()) {
-        return (*x).second;
+        return (x->second);
+    }
+    return (OptionPtr());
+}
+
+OptionPtr
+Pkt::getOption(const uint16_t type) {
+    OptionCollection::iterator x = options_.find(type);
+    if (x != options_.end()) {
+        if (copy_retrieved_options_) {
+            OptionPtr option_copy = x->second->clone();
+            x->second = option_copy;
+        }
+        return (x->second);
     }
     return (OptionPtr()); // NULL
 }

src/lib/dhcp/pkt.h

@@ -245,18 +245,67 @@ public:
     /// data format change etc.
     OptionBuffer data_;
 
+protected:
+
+    /// @brief Returns the first option of specified type without copying.
+    ///
+    /// This method is internally used by the @ref Pkt class and derived
+    /// classes to retrieve a pointer to the specified option. This
+    /// method doesn't copy the option before returning it to the
+    /// caller.
+    ///
+    /// @param type Option type.
+    ///
+    /// @return Pointer to the option of specified type or NULL pointer
+    /// if such option is not present.
+    OptionPtr getNonCopiedOption(const uint16_t type) const;
+
+public:
+
     /// @brief Returns the first option of specified type.
     ///
     /// Returns the first option of specified type. Note that in DHCPv6 several
     /// instances of the same option are allowed (and frequently used).
-    /// Also see \ref Pkt6::getOptions().
+    /// Also see @ref Pkt6::getOptions().
     ///
     /// The options will be only returned after unpack() is called.
     ///
     /// @param type option type we are looking for
     ///
     /// @return pointer to found option (or NULL)
-    OptionPtr getOption(uint16_t type) const;
+    OptionPtr getOption(const uint16_t type);
+
+    OptionPtr getOption(const uint16_t type) const {
+        return (getNonCopiedOption(type));
+    }
+
+    /// @brief Controls whether the option retrieved by the @ref Pkt::getOption
+    /// should be copied before being returned.
+    ///
+    /// Setting this value to true enables the mechanism of copying options
+    /// retrieved from the packet to prevent accidental modifications of
+    /// options that shouldn't be modified. The typical use case for this
+    /// mechanism is to prevent hook library from modifying instance of
+    /// an option within the packet that would also affect the value for
+    /// this option within the Kea configuration structures.
+    ///
+    /// Kea doesn't copy option instances which it stores in the packet.
+    /// It merely copy pointers into the packets. Thus, any modification
+    /// to an option would change the value of this option in the
+    /// Kea configuration. To prevent this, option copying should be
+    /// enabled prior to passing the pointer to a packet to a hook library.
+    ///
+    /// Note that only only does this method causes the server to copy
+    /// an option, but the copied option also replaces the original
+    /// option within the packet. The option can be then freely modified
+    /// and the modifications will only affect the instance of this
+    /// option within the packet but not within the server configuration.
+    ///
+    /// @param copy Indicates if the options should be copied when
+    /// retrieved (if true), or not copied (if false).
+    void setCopyRetrievedOptions(const bool copy) {
+        copy_retrieved_options_ = copy;
+    }
 
     /// @brief Update packet timestamp.
     ///
@@ -615,6 +664,12 @@ protected:
     /// data format change etc.
     isc::util::OutputBuffer buffer_out_;
 
+    /// @brief Indicates if a copy of the retrieved option should be
+    /// returned when @ref Pkt::getOption is called.
+    ///
+    /// @see the documentation for @ref Pkt::setCopyRetrievedOptions.
+    bool copy_retrieved_options_;
+
     /// packet timestamp
     boost::posix_time::ptime timestamp_;
 

src/lib/dhcp/pkt4.cc

@@ -228,7 +228,7 @@ Pkt4::unpack() {
 }
 
 uint8_t Pkt4::getType() const {
-    OptionPtr generic = getOption(DHO_DHCP_MESSAGE_TYPE);
+    OptionPtr generic = getNonCopiedOption(DHO_DHCP_MESSAGE_TYPE);
     if (!generic) {
         return (DHCP_NOTYPE);
     }
@@ -245,7 +245,7 @@ uint8_t Pkt4::getType() const {
 }
 
 void Pkt4::setType(uint8_t dhcp_type) {
-    OptionPtr opt = getOption(DHO_DHCP_MESSAGE_TYPE);
+    OptionPtr opt = getNonCopiedOption(DHO_DHCP_MESSAGE_TYPE);
     if (opt) {
 
         // There is message type option already, update it. It seems that
@@ -332,7 +332,7 @@ Pkt4::getLabel() const {
     /// use the instance member rather than fetch it every time.
     std::string suffix;
     ClientIdPtr client_id;
-    OptionPtr client_opt = getOption(DHO_DHCP_CLIENT_IDENTIFIER);
+    OptionPtr client_opt = getNonCopiedOption(DHO_DHCP_CLIENT_IDENTIFIER);
     if (client_opt) {
         try {
             client_id = ClientIdPtr(new ClientId(client_opt->getData()));
@@ -546,7 +546,7 @@ Pkt4::getHlen() const {
 void
 Pkt4::addOption(const OptionPtr& opt) {
     // Check for uniqueness (DHCPv4 options must be unique)
-    if (getOption(opt->getType())) {
+    if (getNonCopiedOption(opt->getType())) {
         isc_throw(BadValue, "Option " << opt->getType()
                   << " already present in this message.");
     }

src/lib/dhcp/pkt6.cc

@@ -18,6 +18,7 @@
 #include <dhcp/duid.h>
 #include <dhcp/iface_mgr.h>
 
+#include <iterator>
 #include <iostream>
 #include <sstream>
 
@@ -57,17 +58,9 @@ size_t Pkt6::len() {
     }
 }
 
-OptionPtr Pkt6::getAnyRelayOption(uint16_t opt_type, RelaySearchOrder order) {
-
-    if (relay_info_.empty()) {
-        // There's no relay info, this is a direct message
-        return (OptionPtr());
-    }
-
-    int start = 0; // First relay to check
-    int end = 0;   // Last relay to check
-    int direction = 0; // How we going to iterate: forward or backward?
-
+void
+Pkt6:: prepareGetAnyRelayOption(const RelaySearchOrder& order,
+                                  int& start, int& end, int& direction) const {
     switch (order) {
     case RELAY_SEARCH_FROM_CLIENT:
         // Search backwards
@@ -93,6 +86,55 @@ OptionPtr Pkt6::getAnyRelayOption(uint16_t opt_type, RelaySearchOrder order) {
         end = 0;
         direction = 1;
     }
+}
+
+
+OptionPtr
+Pkt6::getNonCopiedAnyRelayOption(const uint16_t option_code,
+                                 const RelaySearchOrder& order) const {
+    if (relay_info_.empty()) {
+        // There's no relay info, this is a direct message
+        return (OptionPtr());
+    }
+
+    int start = 0; // First relay to check
+    int end = 0;   // Last relay to check
+    int direction = 0; // How we going to iterate: forward or backward?
+
+    prepareGetAnyRelayOption(order, start, end, direction);
+
+    // This is a tricky loop. It must go from start to end, but it must work in
+    // both directions (start > end; or start < end). We can't use regular
+    // exit condition, because we don't know whether to use i <= end or i >= end.
+    // That's why we check if in the next iteration we would go past the
+    // list (end + direction). It is similar to STL concept of end pointing
+    // to a place after the last element
+    for (int i = start; i != end + direction; i += direction) {
+        OptionPtr opt = getNonCopiedRelayOption(option_code, i);
+        if (opt) {
+            return (opt);
+        }
+    }
+
+    // We iterated over specified relays and haven't found what we were
+    // looking for
+    return (OptionPtr());
+}
+
+OptionPtr
+Pkt6::getAnyRelayOption(const uint16_t option_code,
+                        const RelaySearchOrder& order) {
+
+    if (relay_info_.empty()) {
+        // There's no relay info, this is a direct message
+        return (OptionPtr());
+    }
+
+    int start = 0; // First relay to check
+    int end = 0;   // Last relay to check
+    int direction = 0; // How we going to iterate: forward or backward?
+
+    prepareGetAnyRelayOption(order, start, end, direction);
 
     // This is a tricky loop. It must go from start to end, but it must work in
     // both directions (start > end; or start < end). We can't use regular
@@ -101,7 +143,7 @@ OptionPtr Pkt6::getAnyRelayOption(uint16_t opt_type, RelaySearchOrder order) {
     // list (end + direction). It is similar to STL concept of end pointing
     // to a place after the last element
     for (int i = start; i != end + direction; i += direction) {
-        OptionPtr opt = getRelayOption(opt_type, i);
+        OptionPtr opt = getRelayOption(option_code, i);
         if (opt) {
             return (opt);
         }
@@ -112,16 +154,41 @@ OptionPtr Pkt6::getAnyRelayOption(uint16_t opt_type, RelaySearchOrder order) {
     return (OptionPtr());
 }
 
-OptionPtr Pkt6::getRelayOption(uint16_t opt_type, uint8_t relay_level) const {
+OptionPtr
+Pkt6::getNonCopiedRelayOption(const uint16_t opt_type,
+                              const uint8_t relay_level) const {
     if (relay_level >= relay_info_.size()) {
-        isc_throw(OutOfRange, "This message was relayed " << relay_info_.size() << " time(s)."
-                  << " There is no info about " << relay_level + 1 << " relay.");
+        isc_throw(OutOfRange, "This message was relayed "
+                  << relay_info_.size() << " time(s)."
+                  << " There is no info about "
+                  << relay_level + 1 << " relay.");
     }
 
     OptionCollection::const_iterator x = relay_info_[relay_level].options_.find(opt_type);
     if (x != relay_info_[relay_level].options_.end()) {
-	return (*x).second;
-      }
+        return (x->second);
+    }
+
+    return (OptionPtr());
+}
+
+OptionPtr
+Pkt6::getRelayOption(const uint16_t opt_type, const uint8_t relay_level) {
+    if (relay_level >= relay_info_.size()) {
+        isc_throw(OutOfRange, "This message was relayed "
+                  << relay_info_.size() << " time(s)."
+                  << " There is no info about "
+                  << relay_level + 1 << " relay.");
+    }
+
+    OptionCollection::iterator x = relay_info_[relay_level].options_.find(opt_type);
+    if (x != relay_info_[relay_level].options_.end()) {
+        if (copy_retrieved_options_) {
+            OptionPtr relay_option_copy = x->second->clone();
+            x->second = relay_option_copy;
+        }
+        return (x->second);
+    }
 
     return (OptionPtr());
 }
@@ -451,7 +518,7 @@ Pkt6::unpackTCP() {
 HWAddrPtr
 Pkt6::getMACFromDUID() {
     HWAddrPtr mac;
-    OptionPtr opt_duid = getOption(D6O_CLIENTID);
+    OptionPtr opt_duid = getNonCopiedOption(D6O_CLIENTID);
     if (!opt_duid) {
         return (mac);
     }
@@ -554,7 +621,7 @@ Pkt6::toText() const {
 
 DuidPtr
 Pkt6::getClientId() const {
-    OptionPtr opt_duid = getOption(D6O_CLIENTID);
+    OptionPtr opt_duid = getNonCopiedOption(D6O_CLIENTID);
     try {
         // This will throw if the DUID length is larger than 128 bytes
         // or is too short.
@@ -570,16 +637,30 @@ Pkt6::getClientId() const {
 }
 
 isc::dhcp::OptionCollection
-Pkt6::getOptions(uint16_t opt_type) {
-    isc::dhcp::OptionCollection found;
+Pkt6::getNonCopiedOptions(const uint16_t opt_type) const {
+    std::pair<OptionCollection::const_iterator,
+              OptionCollection::const_iterator> range = options_.equal_range(opt_type);
+    return (OptionCollection(range.first, range.second));
+}
 
-    for (OptionCollection::const_iterator x = options_.begin();
-         x != options_.end(); ++x) {
-        if (x->first == opt_type) {
-            found.insert(make_pair(opt_type, x->second));
+isc::dhcp::OptionCollection
+Pkt6::getOptions(const uint16_t opt_type) {
+    OptionCollection options_copy;
+
+    std::pair<OptionCollection::iterator,
+              OptionCollection::iterator> range = options_.equal_range(opt_type);
+    // If options should be copied on retrieval, we should now iterate over
+    // matching options, copy them and replace the original ones with new
+    // instances.
+    if (copy_retrieved_options_) {
+        for (OptionCollection::iterator opt_it = range.first;
+             opt_it != range.second; ++opt_it) {
+            OptionPtr option_copy = opt_it->second->clone();
+            opt_it->second = option_copy;
         }
     }
-    return (found);
+    // Finally, return updated options. This can also be empty in some cases.
+    return (OptionCollection(range.first, range.second));
 }
 
 const char*
@@ -668,7 +749,7 @@ const char* Pkt6::getName() const {
 void Pkt6::copyRelayInfo(const Pkt6Ptr& question) {
 
     // We use index rather than iterator, because we need that as a parameter
-    // passed to getRelayOption()
+    // passed to getNonCopiedRelayOption()
     for (size_t i = 0; i < question->relay_info_.size(); ++i) {
         RelayInfo info;
         info.msg_type_ = DHCPV6_RELAY_REPL;
@@ -678,7 +759,7 @@ void Pkt6::copyRelayInfo(const Pkt6Ptr& question) {
 
         // Is there an interface-id option in this nesting level?
         // If there is, we need to echo it back
-        OptionPtr opt = question->getRelayOption(D6O_INTERFACE_ID, i);
+        OptionPtr opt = question->getNonCopiedRelayOption(D6O_INTERFACE_ID, i);
         // taken from question->RelayInfo_[i].options_
         if (opt) {
             info.options_.insert(make_pair(opt->getType(), opt));
@@ -735,7 +816,7 @@ HWAddrPtr
 Pkt6::getMACFromDocsisModem() {
     HWAddrPtr mac;
     OptionVendorPtr vendor = boost::dynamic_pointer_cast<
-        OptionVendor>(getOption(D6O_VENDOR_OPTS));
+        OptionVendor>(getNonCopiedOption(D6O_VENDOR_OPTS));
 
     // Check if this is indeed DOCSIS3 environment
     if (vendor && vendor->getVendorId() == VENDOR_ID_CABLE_LABS) {

src/lib/dhcp/pkt6.h

@@ -224,7 +224,28 @@ public:
     /// @return Pointer to the DUID or NULL if the option doesn't exist.
     DuidPtr getClientId() const;
 
-    /// @brief returns option inserted by relay
+
+protected:
+
+    /// @brief Returns pointer to an option inserted by relay agent.
+    ///
+    /// This is a variant of the @ref Pk6::getRelayOption function which
+    /// never copies an option returned. This method should be only used by
+    /// the @ref Pkt6 class and derived classes. Any external callers should
+    /// use @ref getRelayOption which copies the option before returning it
+    /// when the @ref Pkt::copy_retrieved_option_ flag is set to true.
+    ///
+    /// @param opt_type Code of the requested option.
+    /// @param relay_level Nesting level as described for
+    /// @ref Pkt6::getRelayOption.
+    ///
+    /// @return Pointer to the option or NULL if such option doesn't exist.
+    OptionPtr getNonCopiedRelayOption(const uint16_t opt_type,
+                                      const uint8_t relay_level) const;
+
+public:
+
+    /// @brief Returns option inserted by relay
     ///
     /// Returns an option from specified relay scope (inserted by a given relay
     /// if this is received packet or to be decapsulated by a given relay if
@@ -239,7 +260,47 @@ public:
     /// @param nesting_level see description above
     ///
     /// @return pointer to the option (or NULL if there is no such option)
-    OptionPtr getRelayOption(uint16_t option_code, uint8_t nesting_level) const;
+    OptionPtr getRelayOption(uint16_t option_code, uint8_t nesting_level);
+
+private:
+
+    /// @brief Prepares parameters for loop used in @ref getAnyRelayOption
+    /// and @ref getNonCopiedAnyRelayOption.
+    ///
+    /// The methods retrieving "any" relay option iterate over the relay
+    /// info structures to find the matching option. This method returns
+    /// the index of the first and last relay info structure to be used
+    /// for this iteration. It also returns the direction in which the
+    /// iteration should be performed.
+    ///
+    /// @param order Option search order (see @ref RelaySearchOrder).
+    /// @param [out] start Index of the relay information structure from
+    /// which the serch should be started.
+    /// @param [out] end Index of the relay information structure on which
+    /// the option searches should stop.
+    /// @param [out] direction Equals to -1 for backwards searches, and
+    /// equals to 1 for forward searches.
+    void prepareGetAnyRelayOption(const RelaySearchOrder& order,
+                                  int& start, int& end, int& direction) const;
+
+protected:
+
+    /// @brief Returns pointer to an instance of specified option.
+    ///
+    /// This is a variant of @ref getAnyRelayOption but it never copies
+    /// an option returned. This method should be only used by
+    /// the @ref Pkt6 class and derived classes. Any external callers should
+    /// use @ref getAnyRelayOption which copies the option before returning it
+    /// when the @ref Pkt::copy_retrieved_option_ flag is set to true.
+    ///
+    /// @param option_code Searched option.
+    /// @param order Option search order (see @ref RelaySearchOrder).
+    ///
+    /// @return Option pointer or NULL, if no option matches specified criteria.
+    OptionPtr getNonCopiedAnyRelayOption(const uint16_t option_code,
+                                         const RelaySearchOrder& order) const;
+
+public:
 
     /// @brief Return first instance of a specified option
     ///
@@ -251,7 +312,8 @@ public:
     /// @param option_code searched option
     /// @param order option search order (see @ref RelaySearchOrder)
     /// @return option pointer (or NULL if no option matches specified criteria)
-    OptionPtr getAnyRelayOption(uint16_t option_code, RelaySearchOrder order);
+    OptionPtr getAnyRelayOption(const uint16_t option_code,
+                                const RelaySearchOrder& order);
 
     /// @brief return the link address field from a relay option
     ///
@@ -285,7 +347,24 @@ public:
     const isc::asiolink::IOAddress&
     getRelay6PeerAddress(uint8_t relay_level) const;
 
+protected:
+
+    /// @brief Returns all option instances of specified type without
+    /// copying.
+    ///
+    /// This is a variant of @ref getOptions method, which returns a collection
+    /// of options without copying them. This method should be only used by
+    /// the @ref Pkt6 class and derived classes. Any external callers should
+    /// use @ref getOptions which copies option instances before returning them
+    /// when the @ref Pkt::copy_retrieved_option_ flag is set to true.
+    ///
+    /// @param option_code Option code.
     ///
+    /// @return Collection of options found.
+    OptionCollection getNonCopiedOptions(const uint16_t opt_type) const;
+
+public:
+
     /// @brief Returns all instances of specified type.
     ///
     /// Returns all instances of options of the specified type. DHCPv6 protocol
@@ -293,7 +372,7 @@ public:
     ///
     /// @param type option type we are looking for
     /// @return instance of option collection with requested options
-    isc::dhcp::OptionCollection getOptions(uint16_t type);
+    isc::dhcp::OptionCollection getOptions(const uint16_t type);
 
     /// @brief add information about one traversed relay
     ///

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -593,6 +593,59 @@ TEST_F(Pkt4Test, options) {
     EXPECT_NO_THROW(pkt.reset());
 }
 
+// This test verifies that it is possible to control whether a pointer
+// to an option or a pointer to a copy of an option is returned by the
+// packet object.
+TEST_F(Pkt4Test, setCopyRetrievedOptions) {
+    // Create option 1 with two sub options.
+    OptionPtr option1(new Option(Option::V4, 1));
+    OptionPtr sub1(new Option(Option::V4, 1));
+    OptionPtr sub2(new Option(Option::V4, 2));
+
+    option1->addOption(sub1);
+    option1->addOption(sub2);
+
+    // Create option 2 with two sub options.
+    OptionPtr option2(new Option(Option::V4, 2));
+    OptionPtr sub3(new Option(Option::V4, 1));
+    OptionPtr sub4(new Option(Option::V4, 2));
+
+    option2->addOption(sub3);
+    option2->addOption(sub4);
+
+    // Add both options to a packet.
+    Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1234));
+    pkt->addOption(option1);
+    pkt->addOption(option2);
+
+    // Retrieve options and make sure that the pointers to the original
+    // option instances are returned.
+    ASSERT_TRUE(option1 == pkt->getOption(1));
+    ASSERT_TRUE(option2 == pkt->getOption(2));
+
+    // Now force copying the options when they are retrieved.
+    pkt->setCopyRetrievedOptions(true);
+
+    // Option pointer returned must point to a new instance of option 2.
+    OptionPtr option2_copy = pkt->getOption(2);
+    EXPECT_FALSE(option2 == option2_copy);
+
+    // Disable copying.
+    pkt->setCopyRetrievedOptions(false);
+
+    // Expect that the original pointer is returned. This guarantees that
+    // option1 wasn't affected by copying option 2.
+    OptionPtr option1_copy = pkt->getOption(1);
+    EXPECT_TRUE(option1 == option1_copy);
+
+    // Again, enable copying options.
+    pkt->setCopyRetrievedOptions(true);
+
+    // This time a pointer to new option instance should be returned.
+    option1_copy = pkt->getOption(1);
+    EXPECT_FALSE(option1 == option1_copy);
+}
+
 // This test verifies that the options are unpacked from the packet correctly.
 TEST_F(Pkt4Test, unpackOptions) {
 

src/lib/dhcp/tests/pkt6_unittest.cc

@@ -20,15 +20,16 @@
 #include <dhcp/docsis3_option_defs.h>
 #include <dhcp/tests/pkt_captures.h>
 #include <util/range_utilities.h>
-
 #include <boost/bind.hpp>
 #include <boost/date_time/posix_time/posix_time.hpp>
 #include <boost/scoped_ptr.hpp>
 #include <util/encode/hex.h>
 #include <gtest/gtest.h>
 
+#include <algorithm>
 #include <iostream>
 #include <sstream>
+#include <utility>
 
 #include <arpa/inet.h>
 
@@ -41,6 +42,38 @@ using boost::scoped_ptr;
 
 namespace {
 
+class NakedPkt6 : public Pkt6 {
+public:
+
+    /// @brief Constructor, used in replying to a message
+    ///
+    /// @param msg_type type of message (SOLICIT=1, ADVERTISE=2, ...)
+    /// @param transid transaction-id
+    /// @param proto protocol (TCP or UDP)
+    NakedPkt6(const uint8_t msg_type, const uint32_t transid,
+              const DHCPv6Proto& proto = UDP)
+        : Pkt6(msg_type, transid, proto) {
+    }
+
+    /// @brief Constructor, used in message transmission
+    ///
+    /// Creates new message. Transaction-id will randomized.
+    ///
+    /// @param buf pointer to a buffer of received packet content
+    /// @param len size of buffer of received packet content
+    /// @param proto protocol (usually UDP, but TCP will be supported eventually)
+    NakedPkt6(const uint8_t* buf, const uint32_t len,
+              const DHCPv6Proto& proto = UDP)
+        : Pkt6(buf, len, proto) {
+    }
+
+    using Pkt6::getNonCopiedOptions;
+    using Pkt6::getNonCopiedRelayOption;
+    using Pkt6::getNonCopiedAnyRelayOption;
+};
+
+typedef boost::shared_ptr<NakedPkt6> NakedPkt6Ptr;
+
 class Pkt6Test : public ::testing::Test {
 public:
     Pkt6Test() {
@@ -205,14 +238,14 @@ Pkt6* capture2() {
     // to be OptionBuffer format)
     isc::util::encode::decodeHex(hex_string, bin);
 
-    Pkt6* pkt = new Pkt6(&bin[0], bin.size());
+    NakedPkt6* pkt = new NakedPkt6(&bin[0], bin.size());
     pkt->setRemotePort(547);
     pkt->setRemoteAddr(IOAddress("fe80::1234"));
     pkt->setLocalPort(547);
     pkt->setLocalAddr(IOAddress("ff05::1:3"));
     pkt->setIndex(2);
     pkt->setIface("eth0");
-    return (pkt);
+    return (dynamic_cast<Pkt6*>(pkt));
 }
 
 TEST_F(Pkt6Test, unpack_solicit1) {
@@ -448,6 +481,92 @@ TEST_F(Pkt6Test, addGetDelOptions) {
     EXPECT_EQ(0, options.size());
 }
 
+// Check that multiple options of the same type may be retrieved by using
+// Pkt6::getOptions or Pkt6::getNonCopiedOptions. In the former case, also
+// check that retrieved options are copied when Pkt6::setCopyRetrievedOptions
+// is enabled.
+TEST_F(Pkt6Test, getOptions) {
+    NakedPkt6 pkt(DHCPV6_SOLICIT, 1234);
+    OptionPtr opt1(new Option(Option::V6, 1));
+    OptionPtr opt2(new Option(Option::V6, 1));
+    OptionPtr opt3(new Option(Option::V6, 2));
+    OptionPtr opt4(new Option(Option::V6, 2));
+
+    pkt.addOption(opt1);
+    pkt.addOption(opt2);
+    pkt.addOption(opt3);
+    pkt.addOption(opt4);
+
+    // Retrieve options with option code 1.
+    OptionCollection options = pkt.getOptions(1);
+    ASSERT_EQ(2, options.size());
+
+    OptionCollection::const_iterator opt_it;
+
+    // Make sure that the first option is returned. We're using the pointer
+    // to opt1 to find the option.
+    opt_it = std::find(options.begin(), options.end(),
+                       std::pair<const unsigned int, OptionPtr>(1, opt1));
+    EXPECT_TRUE(opt_it != options.end());
+
+    // Make sure that the second option is returned.
+    opt_it = std::find(options.begin(), options.end(),
+                       std::pair<const unsigned int, OptionPtr>(1, opt2));
+    EXPECT_TRUE(opt_it != options.end());
+
+    // Retrieve options with option code 2.
+    options = pkt.getOptions(2);
+
+    // opt3 and opt4 should exist.
+    opt_it = std::find(options.begin(), options.end(),
+                       std::pair<const unsigned int, OptionPtr>(2, opt3));
+    EXPECT_TRUE(opt_it != options.end());
+
+    opt_it = std::find(options.begin(), options.end(),
+                       std::pair<const unsigned int, OptionPtr>(2, opt4));
+    EXPECT_TRUE(opt_it != options.end());
+
+    // Enable copying options when they are retrieved.
+    pkt.setCopyRetrievedOptions(true);
+
+    options = pkt.getOptions(1);
+    ASSERT_EQ(2, options.size());
+
+    // Both retrieved options should be copied so an attempt to find them
+    // using option pointer should fail. Original pointers should have
+    // been replaced with new instances.
+    opt_it = std::find(options.begin(), options.end(),
+                       std::pair<const unsigned int, OptionPtr>(1, opt1));
+    EXPECT_TRUE(opt_it == options.end());
+
+    opt_it = std::find(options.begin(), options.end(),
+                       std::pair<const unsigned int, OptionPtr>(1, opt2));
+    EXPECT_TRUE(opt_it == options.end());
+
+    // Return instances of options with the option code 1 and make sure
+    // that copies of the options were used to replace original options
+    // in the packet.
+    OptionCollection options_modified = pkt.getNonCopiedOptions(1);
+    for (OptionCollection::const_iterator opt_it_modified = options_modified.begin();
+         opt_it_modified != options_modified.end(); ++opt_it_modified) {
+        opt_it = std::find(options.begin(), options.end(), *opt_it_modified);
+        ASSERT_TRUE(opt_it != options.end());
+    }
+
+    // Let's check that remaining two options haven't been affected by
+    // retrieving the options with option code 1.
+    options = pkt.getNonCopiedOptions(2);
+    ASSERT_EQ(2, options.size());
+
+    opt_it = std::find(options.begin(), options.end(),
+                       std::pair<const unsigned int, OptionPtr>(2, opt3));
+    EXPECT_TRUE(opt_it != options.end());
+
+    opt_it = std::find(options.begin(), options.end(),
+                       std::pair<const unsigned int, OptionPtr>(2, opt4));
+    EXPECT_TRUE(opt_it != options.end());
+}
+
 TEST_F(Pkt6Test, Timestamp) {
     boost::scoped_ptr<Pkt6> pkt(new Pkt6(DHCPV6_SOLICIT, 0x020304));
 
@@ -750,12 +869,35 @@ TEST_F(Pkt6Test, relayPack) {
     EXPECT_EQ(0, memcmp(expected0, &binary[0], 16));
 }
 
+TEST_F(Pkt6Test, getRelayOption) {
+    NakedPkt6Ptr msg(dynamic_cast<NakedPkt6*>(capture2()));
+    ASSERT_TRUE(msg);
+
+    ASSERT_NO_THROW(msg->unpack());
+    ASSERT_EQ(2, msg->relay_info_.size());
+
+    OptionPtr opt_iface_id = msg->getNonCopiedRelayOption(D6O_INTERFACE_ID, 0);
+    ASSERT_TRUE(opt_iface_id);
 
-// This test verified that options added by relays to the message can be
+    OptionPtr opt_iface_id_returned = msg->getRelayOption(D6O_INTERFACE_ID, 0);
+    ASSERT_TRUE(opt_iface_id_returned);
+
+    EXPECT_TRUE(opt_iface_id == opt_iface_id_returned);
+
+    msg->setCopyRetrievedOptions(true);
+
+    opt_iface_id_returned = msg->getRelayOption(D6O_INTERFACE_ID, 0);
+    EXPECT_FALSE(opt_iface_id == opt_iface_id_returned);
+
+    opt_iface_id = msg->getNonCopiedRelayOption(D6O_INTERFACE_ID, 0);
+    EXPECT_TRUE(opt_iface_id == opt_iface_id_returned);
+}
+
+// This test verifies that options added by relays to the message can be
 // accessed and retrieved properly
 TEST_F(Pkt6Test, getAnyRelayOption) {
 
-    boost::scoped_ptr<Pkt6> msg(new Pkt6(DHCPV6_ADVERTISE, 0x020304));
+    boost::scoped_ptr<NakedPkt6> msg(new NakedPkt6(DHCPV6_ADVERTISE, 0x020304));
     msg->addOption(generateRandomOption(300));
 
     // generate options for relay1
@@ -813,22 +955,69 @@ TEST_F(Pkt6Test, getAnyRelayOption) {
     opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_FROM_CLIENT);
     ASSERT_TRUE(opt);
     EXPECT_TRUE(opt->equals(relay3_opt1));
+    EXPECT_TRUE(opt == relay3_opt1);
 
     // We want to ge that one inserted by relay1 (first match, starting from
     // closest to the server.
     opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_FROM_SERVER);
     ASSERT_TRUE(opt);
     EXPECT_TRUE(opt->equals(relay1_opt1));
+    EXPECT_TRUE(opt == relay1_opt1);
 
     // We just want option from the first relay (closest to the client)
     opt = msg->getAnyRelayOption(200, Pkt6::RELAY_GET_FIRST);
     ASSERT_TRUE(opt);
     EXPECT_TRUE(opt->equals(relay3_opt1));
+    EXPECT_TRUE(opt == relay3_opt1);
 
     // We just want option from the last relay (closest to the server)
     opt = msg->getAnyRelayOption(200, Pkt6::RELAY_GET_LAST);
     ASSERT_TRUE(opt);
     EXPECT_TRUE(opt->equals(relay1_opt1));
+    EXPECT_TRUE(opt == relay1_opt1);
+
+    // Enable copying options when they are retrieved and redo the tests
+    // but expect that options are still equal but different pointers
+    // are returned.
+    msg->setCopyRetrievedOptions(true);
+
+    opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_FROM_CLIENT);
+    ASSERT_TRUE(opt);
+    EXPECT_TRUE(opt->equals(relay3_opt1));
+    EXPECT_FALSE(opt == relay3_opt1);
+    // Test that option copy has replaced the original option within the
+    // packet. We achive that by calling a variant of the method which
+    // retrieved non-copied option.
+    relay3_opt1 = msg->getNonCopiedAnyRelayOption(200, Pkt6::RELAY_SEARCH_FROM_CLIENT);
+    ASSERT_TRUE(relay3_opt1);
+    EXPECT_TRUE(opt == relay3_opt1);
+
+    opt = msg->getAnyRelayOption(200, Pkt6::RELAY_SEARCH_FROM_SERVER);
+    ASSERT_TRUE(opt);
+    EXPECT_TRUE(opt->equals(relay1_opt1));
+    EXPECT_FALSE(opt == relay1_opt1);
+    relay1_opt1 = msg->getNonCopiedAnyRelayOption(200, Pkt6::RELAY_SEARCH_FROM_SERVER);
+    ASSERT_TRUE(relay1_opt1);
+    EXPECT_TRUE(opt == relay1_opt1);
+
+    opt = msg->getAnyRelayOption(200, Pkt6::RELAY_GET_FIRST);
+    ASSERT_TRUE(opt);
+    EXPECT_TRUE(opt->equals(relay3_opt1));
+    EXPECT_FALSE(opt == relay3_opt1);
+    relay3_opt1 = msg->getNonCopiedAnyRelayOption(200, Pkt6::RELAY_GET_FIRST);
+    ASSERT_TRUE(relay3_opt1);
+    EXPECT_TRUE(opt == relay3_opt1);
+
+    opt = msg->getAnyRelayOption(200, Pkt6::RELAY_GET_LAST);
+    ASSERT_TRUE(opt);
+    EXPECT_TRUE(opt->equals(relay1_opt1));
+    EXPECT_FALSE(opt == relay1_opt1);
+    relay1_opt1 = msg->getNonCopiedAnyRelayOption(200, Pkt6::RELAY_GET_LAST);
+    ASSERT_TRUE(relay1_opt1);
+    EXPECT_TRUE(opt == relay1_opt1);
+
+    // Disable copying options and continue with other tests.
+    msg->setCopyRetrievedOptions(false);
 
     // Let's try to ask for something that is inserted by the middle relay
     // only.

src/lib/eval/evaluate.cc

@@ -9,7 +9,7 @@
 namespace isc {
 namespace dhcp {
 
-bool evaluate(const Expression& expr, const Pkt& pkt) {
+bool evaluate(const Expression& expr, Pkt& pkt) {
     ValueStack values;
     for (Expression::const_iterator it = expr.begin();
          it != expr.end(); ++it) {

src/lib/eval/evaluate.h

@@ -22,7 +22,7 @@ namespace dhcp {
 ///        stack at the end of the evaluation
 /// @throw EvalTypeError if the value at the top of the stack at the
 ///        end of the evaluation is not "false" or "true"
-bool evaluate(const Expression& expr, const Pkt& pkt);
+bool evaluate(const Expression& expr, Pkt& pkt);
 
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/lib/eval/token.cc

@@ -18,7 +18,7 @@ using namespace isc::dhcp;
 using namespace std;
 
 void
-TokenString::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+TokenString::evaluate(Pkt& /*pkt*/, ValueStack& values) {
     // Literals only push, nothing to pop
     values.push(value_);
 
@@ -56,7 +56,7 @@ TokenHexString::TokenHexString(const string& str) : value_("") {
 }
 
 void
-TokenHexString::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+TokenHexString::evaluate(Pkt& /*pkt*/, ValueStack& values) {
     // Literals only push, nothing to pop
     values.push(value_);
 
@@ -82,7 +82,7 @@ TokenIpAddress::TokenIpAddress(const string& addr) : value_("") {
 }
 
 void
-TokenIpAddress::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+TokenIpAddress::evaluate(Pkt& /*pkt*/, ValueStack& values) {
     // Literals only push, nothing to pop
     values.push(value_);
 
@@ -93,12 +93,12 @@ TokenIpAddress::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
 }
 
 OptionPtr
-TokenOption::getOption(const Pkt& pkt) {
+TokenOption::getOption(Pkt& pkt) {
     return (pkt.getOption(option_code_));
 }
 
 void
-TokenOption::evaluate(const Pkt& pkt, ValueStack& values) {
+TokenOption::evaluate(Pkt& pkt, ValueStack& values) {
     OptionPtr opt = getOption(pkt);
     std::string opt_str;
     if (opt) {
@@ -141,7 +141,7 @@ TokenRelay4Option::TokenRelay4Option(const uint16_t option_code,
     :TokenOption(option_code, rep_type) {
 }
 
-OptionPtr TokenRelay4Option::getOption(const Pkt& pkt) {
+OptionPtr TokenRelay4Option::getOption(Pkt& pkt) {
 
     // Check if there is Relay Agent Option.
     OptionPtr rai = pkt.getOption(DHO_DHCP_AGENT_OPTIONS);
@@ -154,7 +154,7 @@ OptionPtr TokenRelay4Option::getOption(const Pkt& pkt) {
 }
 
 void
-TokenPkt4::evaluate(const Pkt& pkt, ValueStack& values) {
+TokenPkt4::evaluate(Pkt& pkt, ValueStack& values) {
 
     vector<uint8_t> binary;
     string type_str;
@@ -239,7 +239,7 @@ TokenPkt4::evaluate(const Pkt& pkt, ValueStack& values) {
 }
 
 void
-TokenEqual::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+TokenEqual::evaluate(Pkt& /*pkt*/, ValueStack& values) {
 
     if (values.size() < 2) {
         isc_throw(EvalBadStack, "Incorrect stack order. Expected at least "
@@ -266,7 +266,7 @@ TokenEqual::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
 }
 
 void
-TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+TokenSubstring::evaluate(Pkt& /*pkt*/, ValueStack& values) {
 
     if (values.size() < 3) {
         isc_throw(EvalBadStack, "Incorrect stack order. Expected at least "
@@ -365,7 +365,7 @@ TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
 }
 
 void
-TokenConcat::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+TokenConcat::evaluate(Pkt& /*pkt*/, ValueStack& values) {
 
     if (values.size() < 2) {
         isc_throw(EvalBadStack, "Incorrect stack order. Expected at least "
@@ -391,7 +391,7 @@ TokenConcat::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
 }
 
 void
-TokenNot::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+TokenNot::evaluate(Pkt& /*pkt*/, ValueStack& values) {
 
     if (values.size() == 0) {
         isc_throw(EvalBadStack, "Incorrect empty stack.");
@@ -414,7 +414,7 @@ TokenNot::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
 }
 
 void
-TokenAnd::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+TokenAnd::evaluate(Pkt& /*pkt*/, ValueStack& values) {
 
     if (values.size() < 2) {
         isc_throw(EvalBadStack, "Incorrect stack order. Expected at least "
@@ -442,7 +442,7 @@ TokenAnd::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
 }
 
 void
-TokenOr::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+TokenOr::evaluate(Pkt& /*pkt*/, ValueStack& values) {
 
     if (values.size() < 2) {
         isc_throw(EvalBadStack, "Incorrect stack order. Expected at least "
@@ -469,12 +469,12 @@ TokenOr::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
         .arg('\'' + values.top() + '\'');
 }
 
-OptionPtr TokenRelay6Option::getOption(const Pkt& pkt) {
+OptionPtr TokenRelay6Option::getOption(Pkt& pkt) {
 
     try {
         // Check if it's a Pkt6.  If it's not the dynamic_cast will
         // throw std::bad_cast.
-        const Pkt6& pkt6 = dynamic_cast<const Pkt6&>(pkt);
+        Pkt6& pkt6 = dynamic_cast<Pkt6&>(pkt);
 
         try {
             // Now that we have the right type of packet we can
@@ -496,7 +496,7 @@ OptionPtr TokenRelay6Option::getOption(const Pkt& pkt) {
 }
 
 void
-TokenRelay6Field::evaluate(const Pkt& pkt, ValueStack& values) {
+TokenRelay6Field::evaluate(Pkt& pkt, ValueStack& values) {
 
     vector<uint8_t> binary;
     string type_str;
@@ -549,7 +549,7 @@ TokenRelay6Field::evaluate(const Pkt& pkt, ValueStack& values) {
 }
 
 void
-TokenPkt6::evaluate(const Pkt& pkt, ValueStack& values) {
+TokenPkt6::evaluate(Pkt& pkt, ValueStack& values) {
 
     vector<uint8_t> binary;
     string type_str;

src/lib/eval/token.h

@@ -75,7 +75,7 @@ public:
     ///
     /// @param pkt - packet being classified
     /// @param values - stack of values with previously evaluated tokens
-    virtual void evaluate(const Pkt& pkt, ValueStack& values) = 0;
+    virtual void evaluate(Pkt& pkt, ValueStack& values) = 0;
 
     /// @brief Virtual destructor
     virtual ~Token() {}
@@ -124,7 +124,7 @@ public:
     ///
     /// @param pkt (ignored)
     /// @param values (represented string will be pushed here)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 
 protected:
     std::string value_; ///< Constant value
@@ -149,7 +149,7 @@ public:
     ///
     /// @param pkt (ignored)
     /// @param values (represented string will be pushed here)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 
 protected:
     std::string value_; ///< Constant value
@@ -171,7 +171,7 @@ public:
     ///
     /// @param pkt (ignored)
     /// @param values (represented IP address will be pushed here)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 
 protected:
     ///< Constant value (empty string if the IP address cannot be converted)
@@ -222,7 +222,7 @@ public:
     ///
     /// @param pkt specified option will be extracted from this packet (if present)
     /// @param values value of the option will be pushed here (or "")
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 
     /// @brief Returns option-code
     ///
@@ -254,7 +254,7 @@ protected:
     ///
     /// @param pkt the option will be retrieved from here
     /// @return option instance (or NULL if not found)
-    virtual OptionPtr getOption(const Pkt& pkt);
+    virtual OptionPtr getOption(Pkt& pkt);
 
     uint16_t option_code_; ///< Code of the option to be extracted
     RepresentationType representation_type_; ///< Representation type.
@@ -285,7 +285,7 @@ protected:
     /// @brief Attempts to obtain specified sub-option of option 82 from the packet
     /// @param pkt DHCPv4 packet (that hopefully contains option 82)
     /// @return found sub-option from option 82
-    virtual OptionPtr getOption(const Pkt& pkt);
+    virtual OptionPtr getOption(Pkt& pkt);
 };
 
 /// @brief Token that represents fields of a DHCPv4 packet.
@@ -328,7 +328,7 @@ public:
     ///
     /// @param pkt - fields will be extracted from here
     /// @param values - stack of values (1 result will be pushed)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 
     /// @brief Returns field type
     ///
@@ -364,7 +364,7 @@ public:
     /// @param pkt (unused)
     /// @param values - stack of values (2 arguments will be popped, 1 result
     ///        will be pushed)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 };
 
 /// @brief Token that represents the substring operator (returns a portion
@@ -421,7 +421,7 @@ public:
     /// @param pkt (unused)
     /// @param values - stack of values (3 arguments will be popped, 1 result
     ///        will be pushed)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 };
 
 /// @brief Token that represents concat operator (concatenates two other tokens)
@@ -444,7 +444,7 @@ public:
     /// @param pkt (unused)
     /// @param values - stack of values (2 arguments will be popped, 1 result
     ///        will be pushed)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 };
 
 /// @brief Token that represents logical negation operator
@@ -469,7 +469,7 @@ public:
     ///
     /// @param pkt (unused)
     /// @param values - stack of values (logical top value negated)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 };
 
 /// @brief Token that represents logical and operator
@@ -494,7 +494,7 @@ public:
     /// @param pkt (unused)
     /// @param values - stack of values (2 arguments will be popped, 1 result
     ///        will be pushed)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 };
 
 /// @brief Token that represents logical or operator
@@ -519,7 +519,7 @@ public:
     /// @param pkt (unused)
     /// @param values - stack of values (2 arguments will be popped, 1 result
     ///        will be pushed)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 };
 
 /// @brief Token that represents a value of an option within a DHCPv6 relay
@@ -563,7 +563,7 @@ protected:
     /// @brief Attempts to obtain specified option from the specified relay block
     /// @param pkt DHCPv6 packet that hopefully contains the proper relay block
     /// @return option instance if available
-    virtual OptionPtr getOption(const Pkt& pkt);
+    virtual OptionPtr getOption(Pkt& pkt);
 
     uint8_t nest_level_; ///< nesting level of the relay block to use
 };
@@ -606,7 +606,7 @@ public:
     ///
     /// @param pkt fields will be extracted from here
     /// @param values - stack of values (1 result will be pushed)
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 
     /// @brief Returns nest-level
     ///
@@ -666,7 +666,7 @@ public:
     ///
     /// @param pkt - packet from which to extract the fields
     /// @param values - stack of values, 1 result will be pushed
-    void evaluate(const Pkt& pkt, ValueStack& values);
+    void evaluate(Pkt& pkt, ValueStack& values);
 
     /// @brief Returns field type
     ///