[#2181] created RAII ScopedOptionsCopy to restore pkt options

doc/sphinx/arm/hooks.rst

@@ -1312,7 +1312,7 @@ Examples:
 
     {
         "request-parser-format": "ifelse(pkt6.msgtype == 8 or pkt6.msgtype == 9, ifelse(option[3].option[5].exists, 'Address: ' + addrtotext(substring(option[3].option[5].hex, 0, 16)) + ' has been released from a device with DUID: ' + hexstring(option[1].hex, ':') + ifelse(relay6[0].peeraddr == '', '', ' connected via relay at address: ' + addrtotext(relay6[0].peeraddr) + ' for client on link address: ' + addrtotext(relay6[0].linkaddr) + ifelse(relay6[0].option[37].exists, ', remote-id: ' + hexstring(relay6[0].option[37].hex, ':'), '') + ifelse(relay6[0].option[38].exists, ', subscriber-id: ' + hexstring(relay6[0].option[38].hex, ':'), '') + ifelse(relay6[0].option[18].exists, ', connected at location interface-id: ' + hexstring(relay6[0].option[18].hex, ':'), '')), '') + ifelse(option[25].option[26].exists, 'Prefix: ' + addrtotext(substring(option[25].option[26].hex, 9, 16)) + '/' + uint8totext(substring(option[25].option[26].hex, 8, 1)) + ' has been released from a device with DUID: ' + hexstring(option[1].hex, ':') + ifelse(relay6[0].peeraddr == '', '', ' connected via relay at address: ' + addrtotext(relay6[0].peeraddr) + ' for client on link address: ' + addrtotext(relay6[0].linkaddr) + ifelse(relay6[0].option[37].exists, ', remote-id: ' + hexstring(relay6[0].option[37].hex, ':'), '') + ifelse(relay6[0].option[38].exists, ', subscriber-id: ' + hexstring(relay6[0].option[38].hex, ':'), '') + ifelse(relay6[0].option[18].exists, ', connected at location interface-id: ' + hexstring(relay6[0].option[18].hex, ':'), '')), ''), '')",
-        "response-parser-format": "ifelse(pkt6.msgtype == 7, ifelse(option[3].option[5].exists, 'Address: ' + addrtotext(substring(option[3].option[5].hex, 0, 16)) + ' has been assigned for ' + uint32totext(substring(option[3].option[5].hex, 20, 4)) + ' seconds to a device with DUID: ' + hexstring(option[1].hex, ':') + ifelse(relay6[0].peeraddr == '', '', ' connected via relay at address: ' + addrtotext(relay6[0].peeraddr) + ' for client on link address: ' + addrtotext(relay6[0].linkaddr) + ifelse(relay6[0].option[37].exists, ', remote-id: ' + hexstring(relay6[0].option[37].hex, ':'), '') + ifelse(relay6[0].option[38].exists, ', subscriber-id: ' + hexstring(relay6[0].option[38].hex, ':'), '') + ifelse(relay6[0].option[18].exists, ', connected at location interface-id: ' + hexstring(relay6[0].option[18].hex, ':'), '')), '') + ifelse(option[25].option[26].exists, 'Prefix: ' + addrtotext(substring(option[25].option[26].hex, 9, 16)) + '/' + uint8totext(substring(option[25].option[26].hex, 8, 1)) + ' has been assigned for ' + uint32totext(substring(option[25].option[26].hex, 4, 4)) + ' seconds to a device with DUID: ' + hexstring(option[1].hex, ':') + ifelse(relay6[0].peeraddr == '', '', ' connected via relay at address: ' + addrtotext(relay6[0].peeraddr) + ' for client on link address: ' + addrtotext(relay6[0].linkaddr) + ifelse(relay6[0].option[37].exists, ', remote-id: ' + hexstring(relay6[0].option[37].hex, ':'), '') + ifelse(relay6[0].option[38].exists, ', subscriber-id: ' + hexstring(relay6[0].option[38].hex, ':'), '') + ifelse(relay6[0].option[18].exists, ', connected at location interface-id: ' + hexstring(relay6[0].option[18].hex, ':'), '')), ''), '')"
+        "response-parser-format": "ifelse(pkt6.msgtype == 7, ifelse(option[3].option[5].exists and not (substring(option[3].option[5].hex, 20, 4) == 0), 'Address: ' + addrtotext(substring(option[3].option[5].hex, 0, 16)) + ' has been assigned for ' + uint32totext(substring(option[3].option[5].hex, 20, 4)) + ' seconds to a device with DUID: ' + hexstring(option[1].hex, ':') + ifelse(relay6[0].peeraddr == '', '', ' connected via relay at address: ' + addrtotext(relay6[0].peeraddr) + ' for client on link address: ' + addrtotext(relay6[0].linkaddr) + ifelse(relay6[0].option[37].exists, ', remote-id: ' + hexstring(relay6[0].option[37].hex, ':'), '') + ifelse(relay6[0].option[38].exists, ', subscriber-id: ' + hexstring(relay6[0].option[38].hex, ':'), '') + ifelse(relay6[0].option[18].exists, ', connected at location interface-id: ' + hexstring(relay6[0].option[18].hex, ':'), '')), '') + ifelse(option[25].option[26].exists and not (substring(option[25].option[26].hex, 4, 4) == 0), 'Prefix: ' + addrtotext(substring(option[25].option[26].hex, 9, 16)) + '/' + uint8totext(substring(option[25].option[26].hex, 8, 1)) + ' has been assigned for ' + uint32totext(substring(option[25].option[26].hex, 4, 4)) + ' seconds to a device with DUID: ' + hexstring(option[1].hex, ':') + ifelse(relay6[0].peeraddr == '', '', ' connected via relay at address: ' + addrtotext(relay6[0].peeraddr) + ' for client on link address: ' + addrtotext(relay6[0].linkaddr) + ifelse(relay6[0].option[37].exists, ', remote-id: ' + hexstring(relay6[0].option[37].hex, ':'), '') + ifelse(relay6[0].option[38].exists, ', subscriber-id: ' + hexstring(relay6[0].option[38].hex, ':'), '') + ifelse(relay6[0].option[18].exists, ', connected at location interface-id: ' + hexstring(relay6[0].option[18].hex, ':'), '')), ''), '')"
     }
 
 .. raw:: html
@@ -1354,7 +1354,7 @@ Examples:
                 '')",
         "response-parser-format":
             "ifelse(pkt6.msgtype == 7,
-                ifelse(option[3].option[5].exists,
+                ifelse(option[3].option[5].exists and not (substring(option[3].option[5].hex, 20, 4) == 0),
                     'Address: ' + addrtotext(substring(option[3].option[5].hex, 0, 16)) + ' has been assigned for ' + uint32totext(substring(option[3].option[5].hex, 20, 4)) + ' seconds to a device with DUID: ' + hexstring(option[1].hex, ':') +
                     ifelse(relay6[0].peeraddr == '',
                         '',
@@ -1369,7 +1369,7 @@ Examples:
                             ', connected at location interface-id: ' + hexstring(relay6[0].option[18].hex, ':'),
                             '')),
                     '') +
-                ifelse(option[25].option[26].exists,
+                ifelse(option[25].option[26].exists and not (substring(option[25].option[26].hex, 4, 4) == 0),
                     'Prefix: ' + addrtotext(substring(option[25].option[26].hex, 9, 16)) + '/' + uint8totext(substring(option[25].option[26].hex, 8, 1)) + ' has been assigned for ' + uint32totext(substring(option[25].option[26].hex, 4, 4)) + ' seconds to a device with DUID: ' + hexstring(option[1].hex, ':') +
                     ifelse(relay6[0].peeraddr == '',
                         '',

src/lib/dhcp/pkt.h

@@ -74,10 +74,60 @@ public:
 
 private:
 
-    /// @brief Holds a pointers to the packets.
+    /// @brief Holds a pair of pointers of the packets.
     std::pair<PktTypePtr, PktTypePtr> pkts_;
 };
 
+/// @brief RAII object enabling duplication of the stored options and restoring
+/// the original options on destructor.
+///
+/// This object enables duplication of the stored options and restoring the
+/// original options on destructor. When the object goes out of scope, the
+/// initial options are restored. This is applicable in cases when the server is
+/// going to invoke a callout (hook library) where the list of options in the
+/// packet will be modified. The use of RAII object eliminates the need for
+/// explicitly copying and restoring the list of options and is safer in case of
+/// exceptions thrown by callouts and a presence of multiple exit points.
+///
+/// @tparam PktType Type of the packet, e.g. Pkt4, Pkt6, Pkt4o6.
+template<typename PktType>
+class ScopedOptionsCopy {
+public:
+
+    /// @brief Pointer to an encapsulated packet.
+    typedef boost::shared_ptr<PktType> PktTypePtr;
+
+    /// @brief Constructor.
+    ///
+    /// Creates a copy of the initial options on a packet(s).
+    ///
+    /// @param pkt Pointer to the packet.
+    ScopedOptionsCopy(const PktTypePtr& pkt)
+        : pkt_(pkt) {
+        if (pkt) {
+            options_ = pkt->options_;
+            pkt->options_ = OptionCollectionPtr(new OptionCollection(*options_));
+        }
+    }
+
+    /// @brief Destructor.
+    ///
+    /// Restores the initial options on a packet.
+    ~ScopedOptionsCopy() {
+        if (pkt_) {
+            pkt_->options_ = options_;
+        }
+    }
+
+private:
+
+    /// @brief Holds a pointer to the packet.
+    PktTypePtr pkt_;
+
+    /// @brief Holds a pointer to the initial packet options.
+    OptionCollectionPtr options_;
+};
+
 /// @brief Base class for classes representing DHCP messages.
 ///
 /// This is a base class that holds common information (e.g. source

src/lib/dhcp/tests/protocol_util_unittest.cc

@@ -381,4 +381,72 @@ TEST(ProtocolUtilTest, writeIpUdpHeader) {
     EXPECT_EQ(0x8817, udp_checksum);
 }
 
+TEST(ScopedEnableOptionsCopy, enableOptionsCopy) {
+    Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 2543));
+    OptionPtr option = Option::create(Option::V4, DHO_BOOT_FILE_NAME);
+    pkt->addOption(option);
+    ASSERT_FALSE(pkt->isCopyRetrievedOptions());
+    ASSERT_EQ(option, pkt->getOption(DHO_BOOT_FILE_NAME));
+    {
+        ScopedEnableOptionsCopy<Pkt4> oc(pkt);
+        ASSERT_TRUE(pkt->isCopyRetrievedOptions());
+        OptionPtr option_copy = pkt->getOption(DHO_BOOT_FILE_NAME);
+        ASSERT_NE(option, option_copy);
+        option = option_copy;
+    }
+    ASSERT_FALSE(pkt->isCopyRetrievedOptions());
+    ASSERT_EQ(option, pkt->getOption(DHO_BOOT_FILE_NAME));
+    {
+        try {
+            ScopedEnableOptionsCopy<Pkt4> oc(pkt);
+            ASSERT_TRUE(pkt->isCopyRetrievedOptions());
+            OptionPtr option_copy = pkt->getOption(DHO_BOOT_FILE_NAME);
+            ASSERT_NE(option, option_copy);
+            option = option_copy;
+            throw 0;
+        } catch (...) {
+            ASSERT_FALSE(pkt->isCopyRetrievedOptions());
+            ASSERT_EQ(option, pkt->getOption(DHO_BOOT_FILE_NAME));
+        }
+        ASSERT_FALSE(pkt->isCopyRetrievedOptions());
+        ASSERT_EQ(option, pkt->getOption(DHO_BOOT_FILE_NAME));
+    }
+}
+
+TEST(ScopedOptionsCopy, optionsCopy) {
+    Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 2543));
+    OptionPtr option = Option::create(Option::V4, DHO_BOOT_FILE_NAME);
+    OptionCollectionPtr options = pkt->options_;
+    pkt->addOption(option);
+    size_t count = pkt->options_->size();
+    ASSERT_NE(0, count);
+    ASSERT_EQ(option, pkt->getOption(DHO_BOOT_FILE_NAME));
+    {
+        ScopedOptionsCopy<Pkt4> oc(pkt);
+        ASSERT_NE(pkt->options_, options);
+        ASSERT_EQ(*pkt->options_, *options);
+        pkt->delOption(DHO_BOOT_FILE_NAME);
+        ASSERT_EQ(pkt->options_->size(), count - 1);
+        ASSERT_FALSE(pkt->getOption(DHO_BOOT_FILE_NAME));
+    }
+    ASSERT_EQ(pkt->options_, options);
+    ASSERT_EQ(pkt->getOption(DHO_BOOT_FILE_NAME), option);
+    {
+        try {
+            ScopedOptionsCopy<Pkt4> oc(pkt);
+            ASSERT_NE(pkt->options_, options);
+            ASSERT_EQ(*pkt->options_, *options);
+            pkt->delOption(DHO_BOOT_FILE_NAME);
+            ASSERT_EQ(pkt->options_->size(), count - 1);
+            ASSERT_FALSE(pkt->getOption(DHO_BOOT_FILE_NAME));
+            throw 0;
+        } catch (...) {
+            ASSERT_EQ(pkt->options_, options);
+            ASSERT_EQ(pkt->getOption(DHO_BOOT_FILE_NAME), option);
+        }
+        ASSERT_EQ(pkt->options_, options);
+        ASSERT_EQ(pkt->getOption(DHO_BOOT_FILE_NAME), option);
+    }
+}
+
 } // anonymous namespace