From: Marcin Siodelski Date: Thu, 30 Jun 2016 15:25:53 +0000 (+0200) Subject: [4497] Implemented copying options on retrieval from packet. X-Git-Tag: trac4551_base~23^2~13 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fbd175e09f2d2a4e93e5e6ad593a3121b2145ada;p=thirdparty%2Fkea.git [4497] Implemented copying options on retrieval from packet. --- diff --git a/src/lib/dhcp/pkt.cc b/src/lib/dhcp/pkt.cc index 9bc1dd2a2f..a8bd0deec4 100644 --- a/src/lib/dhcp/pkt.cc +++ b/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 } diff --git a/src/lib/dhcp/pkt.h b/src/lib/dhcp/pkt.h index 75f70f978c..1b234e9251 100644 --- a/src/lib/dhcp/pkt.h +++ b/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_; diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc index 10542496b6..8a2bf7590e 100644 --- a/src/lib/dhcp/pkt4.cc +++ b/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."); } diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc index 393cece498..bf09eaf016 100644 --- a/src/lib/dhcp/pkt6.cc +++ b/src/lib/dhcp/pkt6.cc @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -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 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 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) { diff --git a/src/lib/dhcp/pkt6.h b/src/lib/dhcp/pkt6.h index 44eed2b320..c1b446b9a4 100644 --- a/src/lib/dhcp/pkt6.h +++ b/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 /// diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc index 4fab1a8b55..2c61cfea08 100644 --- a/src/lib/dhcp/tests/pkt4_unittest.cc +++ b/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) { diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index 21e4de0183..5bd03d80d7 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -20,15 +20,16 @@ #include #include #include - #include #include #include #include #include +#include #include #include +#include #include @@ -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 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(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(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(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(2, opt3)); + EXPECT_TRUE(opt_it != options.end()); + + opt_it = std::find(options.begin(), options.end(), + std::pair(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(1, opt1)); + EXPECT_TRUE(opt_it == options.end()); + + opt_it = std::find(options.begin(), options.end(), + std::pair(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(2, opt3)); + EXPECT_TRUE(opt_it != options.end()); + + opt_it = std::find(options.begin(), options.end(), + std::pair(2, opt4)); + EXPECT_TRUE(opt_it != options.end()); +} + TEST_F(Pkt6Test, Timestamp) { boost::scoped_ptr 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(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 msg(new Pkt6(DHCPV6_ADVERTISE, 0x020304)); + boost::scoped_ptr 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. diff --git a/src/lib/eval/evaluate.cc b/src/lib/eval/evaluate.cc index 630b227310..bb947f1991 100644 --- a/src/lib/eval/evaluate.cc +++ b/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) { diff --git a/src/lib/eval/evaluate.h b/src/lib/eval/evaluate.h index 0dbf788a3a..3564b90092 100644 --- a/src/lib/eval/evaluate.h +++ b/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 diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index f0e74a04d5..c9f5c969b7 100644 --- a/src/lib/eval/token.cc +++ b/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 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(pkt); + Pkt6& pkt6 = dynamic_cast(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 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 binary; string type_str; diff --git a/src/lib/eval/token.h b/src/lib/eval/token.h index d51ed6d1ec..4b7812e664 100644 --- a/src/lib/eval/token.h +++ b/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 ///