From a86db3136d0449194b5f00ac4dcbc91b1f2d4953 Mon Sep 17 00:00:00 2001 From: Razvan Becheriu Date: Thu, 24 Feb 2022 18:28:45 +0200 Subject: [PATCH] [#2181] created RAII ScopedOptionsCopy to restore pkt options --- doc/sphinx/arm/hooks.rst | 6 +- src/lib/dhcp/pkt.h | 52 ++++++++++++++- src/lib/dhcp/tests/protocol_util_unittest.cc | 68 ++++++++++++++++++++ 3 files changed, 122 insertions(+), 4 deletions(-) diff --git a/doc/sphinx/arm/hooks.rst b/doc/sphinx/arm/hooks.rst index 96befa9364..0d979a6f14 100644 --- a/doc/sphinx/arm/hooks.rst +++ b/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 == '', '', diff --git a/src/lib/dhcp/pkt.h b/src/lib/dhcp/pkt.h index 4462c3cab2..8536d3c96e 100644 --- a/src/lib/dhcp/pkt.h +++ b/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 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 +class ScopedOptionsCopy { +public: + + /// @brief Pointer to an encapsulated packet. + typedef boost::shared_ptr 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 diff --git a/src/lib/dhcp/tests/protocol_util_unittest.cc b/src/lib/dhcp/tests/protocol_util_unittest.cc index 78cb99d348..94d3f7d418 100644 --- a/src/lib/dhcp/tests/protocol_util_unittest.cc +++ b/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 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 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 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 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