[2940] Fixed handling of the NULL client identifier in the Memfile backend

src/lib/dhcpsrv/lease.cc

@@ -74,6 +74,15 @@ Lease4::Lease4(const Lease4& other)
     }
 }
 
+std::vector<uint8_t>
+Lease4::getClientIdVector() const {
+    if(!client_id_) {
+        return std::vector<uint8_t>();
+    }
+
+    return (client_id_->getClientId());
+}
+
 Lease4&
 Lease4::operator=(const Lease4& other) {
     if (this != &other) {

src/lib/dhcpsrv/lease.h

@@ -209,6 +209,12 @@ struct Lease4 : public Lease {
     /// @param other the @c Lease4 object to be copied.
     Lease4(const Lease4& other);
 
+    /// @brief Returns a client identifier.
+    ///
+    /// @return A client identifier as vector, or an empty vector if client
+    /// identifier is NULL.
+    std::vector<uint8_t> getClientIdVector() const;
+
     /// @brief Assignment operator.
     ///
     /// @param other the @c Lease4 object to be assigned.

src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -80,7 +80,7 @@ Memfile_LeaseMgr::getLease4(const HWAddr& hwaddr) const {
         lease != idx.end(); ++lease) {
 
         // Every Lease4 has a hardware address, so we can compare it
-        if((* lease)->hwaddr_ == hwaddr.hwaddr_) {
+        if((*lease)->hwaddr_ == hwaddr.hwaddr_) {
             collection.push_back((* lease));
         }
     }
@@ -113,9 +113,9 @@ Memfile_LeaseMgr::getLease4(const HWAddr& hwaddr, SubnetID subnet_id) const {
 }
 
 Lease4Collection
-Memfile_LeaseMgr::getLease4(const ClientId& clientid) const {
+Memfile_LeaseMgr::getLease4(const ClientId& client_id) const {
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
-              DHCPSRV_MEMFILE_GET_CLIENTID).arg(clientid.toText());
+              DHCPSRV_MEMFILE_GET_CLIENTID).arg(client_id.toText());
     typedef Memfile_LeaseMgr::Lease4Storage::nth_index<0>::type SearchIndex;
     Lease4Collection collection;
     const SearchIndex& idx = storage4_.get<0>();
@@ -124,8 +124,8 @@ Memfile_LeaseMgr::getLease4(const ClientId& clientid) const {
 
         // client-id is not mandatory in DHCPv4. There can be a lease that does
         // not have a client-id. Dereferencing null pointer would be a bad thing
-        if((*lease)->client_id_ && *(*lease)->client_id_ == clientid) {
-            collection.push_back((* lease));
+        if((*lease)->client_id_ && *(*lease)->client_id_ == client_id) {
+            collection.push_back((*lease));
         }
     }
 

src/lib/dhcpsrv/memfile_lease_mgr.h

@@ -330,20 +330,8 @@ protected:
                 // lease: client id and subnet id.
                 boost::multi_index::composite_key<
                     Lease4,
-                    // The client id value is not directly accessible through the
-                    // Lease4 object as it is wrapped with the ClientIdPtr object.
-                    // Therefore we use the KeyFromKeyExtractor class to access
-                    // client id through this cascaded structure. The client id
-                    // is used as an index value.
-                    KeyFromKeyExtractor<
-                        // Specify that the vector holding client id value can be obtained
-                        // from the ClientId object.
-                        boost::multi_index::const_mem_fun<ClientId, std::vector<uint8_t>,
-                                                          &ClientId::getClientId>,
-                        // Specify that the ClientId object (actually pointer to it) can
-                        // be accessed by the client_id_ member of Lease4 class.
-                        boost::multi_index::member<Lease4, ClientIdPtr, &Lease4::client_id_>
-                    >,
+                    boost::multi_index::const_mem_fun<Lease4, std::vector<uint8_t>,
+                                                      &Lease4::getClientIdVector>,
                     // The subnet id is accessed through the subnet_id_ member.
                     boost::multi_index::member<Lease, uint32_t, &Lease::subnet_id_>
                 >
@@ -355,20 +343,8 @@ protected:
                 // lease: client id and subnet id.
                 boost::multi_index::composite_key<
                     Lease4,
-                    // The client id value is not directly accessible through the
-                    // Lease4 object as it is wrapped with the ClientIdPtr object.
-                    // Therefore we use the KeyFromKeyExtractor class to access
-                    // client id through this cascaded structure. The client id
-                    // is used as an index value.
-                    KeyFromKeyExtractor<
-                        // Specify that the vector holding client id value can be obtained
-                        // from the ClientId object.
-                        boost::multi_index::const_mem_fun<ClientId, std::vector<uint8_t>,
-                                                          &ClientId::getClientId>,
-                        // Specify that the ClientId object (actually pointer to it) can
-                        // be accessed by the client_id_ member of Lease4 class.
-                        boost::multi_index::member<Lease4, ClientIdPtr, &Lease4::client_id_>
-                    >,
+                    boost::multi_index::const_mem_fun<Lease4, std::vector<uint8_t>,
+                                                      &Lease4::getClientIdVector>,
                     // The hardware address is held in the hwaddr_ member of the
                     // Lease4 object.
                     boost::multi_index::member<Lease4, std::vector<uint8_t>,

src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc

@@ -164,14 +164,17 @@ TEST_F(MemfileLeaseMgrTest, getLease4NullClientId) {
     // Let's initialize a specific lease ... But this time
     // We keep its client id for further lookup and
     // We clearly 'reset' it ...
-    Lease4Ptr lease = initializeLease4(straddress4_[4]);
-    ClientIdPtr client_id = lease->client_id_;
-    lease->client_id_ = ClientIdPtr();
-    EXPECT_TRUE(lease_mgr->addLease(lease));
+    Lease4Ptr leaseA = initializeLease4(straddress4_[4]);
+    ClientIdPtr client_id = leaseA->client_id_;
+    leaseA->client_id_ = ClientIdPtr();
+    EXPECT_TRUE(lease_mgr->addLease(leaseA));
 
     Lease4Collection returned = lease_mgr->getLease4(*client_id);
     // Shouldn't have our previous lease ...
     ASSERT_EQ(0, returned.size());
+    Lease4Ptr leaseB = initializeLease4(straddress4_[5]);
+    // Shouldn't throw any null pointer exception
+    EXPECT_NO_THROW(lease_mgr->addLease(leaseB));
 }
 
 // Checks lease4 retrieval through HWAddr
@@ -216,4 +219,20 @@ TEST_F(MemfileLeaseMgrTest, getLease4ClientIdHWAddrSubnetId) {
     EXPECT_TRUE(lease == Lease4Ptr());
 }
 
+TEST_F(MemfileLeaseMgrTest, getLease4ClientIdVector) {
+    const LeaseMgr::ParameterMap pmap;
+    boost::scoped_ptr<Memfile_LeaseMgr> lease_mgr(new Memfile_LeaseMgr(pmap));
+
+    const std::vector<uint8_t> vec;
+    Lease4Ptr lease = initializeLease4(straddress4_[7]);
+    // Check that this lease has null client-id
+    ASSERT_TRUE(lease->client_id_ == ClientIdPtr());
+    // Check that this return empty vector
+    ASSERT_TRUE(lease->getClientIdVector() == vec);
+    // Let's take a lease with client-id not null
+    lease = initializeLease4(straddress4_[6]);
+    // Check that they return same client-id value
+    ASSERT_TRUE(lease->client_id_->getClientId() == lease->getClientIdVector());
+}
+
 }; // end of anonymous namespace