[3977] Addressed review comments.

src/bin/dhcp4/dhcp4_srv.cc

@@ -774,51 +774,6 @@ Dhcpv4Srv::srvidToString(const OptionPtr& srvid) {
     return (addrs[0].toText());
 }
 
-isc::dhcp_ddns::D2Dhcid
-Dhcpv4Srv::computeDhcid(const Lease4Ptr& lease) {
-    if (!lease) {
-        isc_throw(DhcidComputeError, "a pointer to the lease must be not"
-                  " NULL to compute DHCID");
-
-    } else if (lease->hostname_.empty()) {
-        isc_throw(DhcidComputeError, "unable to compute the DHCID for the"
-                  " lease which has empty hostname set");
-
-    }
-
-    // In order to compute DHCID the client's hostname must be encoded in
-    // canonical wire format. It is unlikely that the FQDN is malformed
-    // because it is validated by the classes which encapsulate options
-    // carrying client FQDN. However, if the client name was carried in the
-    // Hostname option it is more likely as it carries the hostname as a
-    // regular string.
-    std::vector<uint8_t> fqdn_wire;
-    try {
-        OptionDataTypeUtil::writeFqdn(lease->hostname_, fqdn_wire, true);
-
-    } catch (const Exception& ex) {
-        isc_throw(DhcidComputeError, "unable to compute DHCID because the"
-                  " hostname: " << lease->hostname_ << " is invalid");
-
-    }
-
-    // Prefer client id to HW address to compute DHCID. If Client Id is
-    // NULL, use HW address.
-    try {
-        if (lease->client_id_) {
-            return (D2Dhcid(lease->client_id_->getClientId(), fqdn_wire));
-
-        } else {
-            return (D2Dhcid(lease->hwaddr_, fqdn_wire));
-        }
-    } catch (const Exception& ex) {
-        isc_throw(DhcidComputeError, "unable to compute DHCID: "
-                  << ex.what());
-
-    }
-
-}
-
 void
 Dhcpv4Srv::appendServerID(Dhcpv4Exchange& ex) {
     // The source address for the outbound message should have been set already.

src/bin/dhcp4/dhcp4_srv.h

@@ -44,13 +44,6 @@
 namespace isc {
 namespace dhcp {
 
-/// @brief Exception thrown when DHCID computation failed.
-class DhcidComputeError : public isc::Exception {
-public:
-    DhcidComputeError(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) { };
-};
-
 /// @brief DHCPv4 message exchange.
 ///
 /// This class represents the DHCPv4 message exchange. The message exchange
@@ -666,28 +659,6 @@ protected:
     /// @return string representation
     static std::string srvidToString(const OptionPtr& opt);
 
-    /// @brief Computes DHCID from a lease.
-    ///
-    /// This method creates an object which represents DHCID. The DHCID value
-    /// is computed as described in RFC4701. The section 3.3. of RFC4701
-    /// specifies the DHCID RR Identifier Type codes:
-    /// - 0x0000 The 1 octet htype followed by glen octets of chaddr
-    /// - 0x0001 The data octets from the DHCPv4 client's Client Identifier
-    /// option.
-    /// - 0x0002 The client's DUID.
-    ///
-    /// Currently this function supports first two of these identifiers.
-    /// The 0x0001 is preferred over the 0x0000 - if the client identifier
-    /// option is present, the former is used. If the client identifier
-    /// is absent, the latter is used.
-    ///
-    /// @todo Implement support for 0x0002 DHCID identifier type.
-    ///
-    /// @param lease A pointer to the structure describing a lease.
-    /// @return An object encapsulating DHCID to be used for DNS updates.
-    /// @throw DhcidComputeError If the computation of the DHCID failed.
-    static isc::dhcp_ddns::D2Dhcid computeDhcid(const Lease4Ptr& lease);
-
     /// @brief Selects a subnet for a given client's packet.
     ///
     /// @param query client's message

src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -198,7 +198,6 @@ public:
     using Dhcpv4Srv::processDecline;
     using Dhcpv4Srv::processInform;
     using Dhcpv4Srv::processClientName;
-    using Dhcpv4Srv::computeDhcid;
     using Dhcpv4Srv::createNameChangeRequests;
     using Dhcpv4Srv::acceptServerId;
     using Dhcpv4Srv::sanityCheck;

src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -489,63 +489,6 @@ public:
     }
 };
 
-// Test that the exception is thrown if lease pointer specified as the argument
-// of computeDhcid function is NULL.
-TEST_F(NameDhcpv4SrvTest, dhcidNullLease) {
-    Lease4Ptr lease;
-    EXPECT_THROW(srv_->computeDhcid(lease), isc::dhcp::DhcidComputeError);
-
-}
-
-// Test that the appropriate exception is thrown if the lease object used
-// to compute DHCID comprises wrong hostname.
-TEST_F(NameDhcpv4SrvTest, dhcidWrongHostname) {
-    // First, make sure that the lease with the correct hostname is accepted.
-    Lease4Ptr lease = createLease(IOAddress("192.0.2.3"),
-                                  "myhost.example.com.", true, true);
-    ASSERT_NO_THROW(srv_->computeDhcid(lease));
-
-    // Now, use the wrong hostname. It should result in the exception.
-    lease->hostname_ = "myhost...example.com.";
-    EXPECT_THROW(srv_->computeDhcid(lease), isc::dhcp::DhcidComputeError);
-
-    // Also, empty hostname is wrong.
-    lease->hostname_ = "";
-    EXPECT_THROW(srv_->computeDhcid(lease), isc::dhcp::DhcidComputeError);
-}
-
-// Test that the DHCID is computed correctly, when the lease holds
-// correct hostname and non-NULL client id.
-TEST_F(NameDhcpv4SrvTest, dhcidComputeFromClientId) {
-    Lease4Ptr lease = createLease(IOAddress("192.0.2.3"),
-                                  "myhost.example.com.",
-                                  true, true);
-    isc::dhcp_ddns::D2Dhcid dhcid;
-    ASSERT_NO_THROW(dhcid = srv_->computeDhcid(lease));
-
-    // Make sure that the computed DHCID is valid.
-    std::string dhcid_ref = "00010132E91AA355CFBB753C0F0497A5A9404"
-        "36965B68B6D438D98E680BF10B09F3BCF";
-    EXPECT_EQ(dhcid_ref, dhcid.toStr());
-}
-
-// Test that the DHCID is computed correctly, when the lease holds correct
-// hostname and NULL client id.
-TEST_F(NameDhcpv4SrvTest, dhcidComputeFromHWAddr) {
-    Lease4Ptr lease = createLease(IOAddress("192.0.2.3"),
-                                  "myhost.example.com.",
-                                  true, true);
-    lease->client_id_.reset();
-
-    isc::dhcp_ddns::D2Dhcid dhcid;
-    ASSERT_NO_THROW(dhcid = srv_->computeDhcid(lease));
-
-    // Make sure that the computed DHCID is valid.
-    std::string dhcid_ref = "0000012247F6DC4423C3E8627434A9D6868609"
-        "D88948F78018B215EDCAA30C0C135035";
-    EXPECT_EQ(dhcid_ref, dhcid.toStr());
-}
-
 // Tests the following scenario:
 //  - Updates are enabled
 //  - All overrides are off

src/bin/dhcp6/tests/fqdn_unittest.cc

@@ -738,8 +738,8 @@ TEST_F(FqdnDhcpv6SrvTest, createRemovalNameChangeRequestFwdRev) {
 
 }
 
-// Checks that NameChangeRequests to remove entries are not created
-// when ddns updates are disabled.
+// Checks that calling queueNCR would not result in error if DDNS updates are
+// disabled. 
 TEST_F(FqdnDhcpv6SrvTest, noRemovalsWhenDisabled) {
     // Disable DDNS updates.
     disableD2();
@@ -751,7 +751,10 @@ TEST_F(FqdnDhcpv6SrvTest, noRemovalsWhenDisabled) {
     // as if we typed domain-name in lower case.
     lease_->hostname_ = "MYHOST.example.com.";
 
-    // When DDNS is disabled an attempt to send a request will throw.
+    // When DDNS is disabled an attempt to send a request should not throw, but
+    // nothing is generated. Unfortunately, we can't see if anything get
+    // generated because getting anything from the queue when DDNS is disabled
+    // would result in exception.
     ASSERT_NO_THROW(queueNCR(CHG_REMOVE, lease_));
 }
 

src/lib/dhcp/pkt6.h

@@ -178,7 +178,6 @@ public:
     /// This variant of the method does not include transaction id.
     ///
     /// @param duid Pointer to the client identifier or NULL.
-    /// @param transid Numeric transaction id to include in the string.
     /// @param hwaddr Hardware address to include in the string or NULL.
     ///
     /// @return String with text representation of the packet identifiers.

src/lib/dhcp/tests/pkt4_unittest.cc

@@ -1035,10 +1035,14 @@ TEST_F(Pkt4Test, makeLabelWithoutTransactionId) {
     EXPECT_EQ("[hwtype=123 01:02:03:04:05:06], cid=[no info]",
               Pkt4::makeLabel(hwaddr, ClientIdPtr()));
 
-    // Test non-null client identifier.
+    // Test non-null client identifier and non-null hardware address.
     ClientIdPtr cid = ClientId::fromText("01:02:03:04");
     EXPECT_EQ("[hwtype=123 01:02:03:04:05:06], cid=[01:02:03:04]",
               Pkt4::makeLabel(hwaddr, cid));
+
+    // Test non-nnull client identifier and null hardware address.
+    EXPECT_EQ("[no hwaddr info], cid=[01:02:03:04]",
+              Pkt4::makeLabel(HWAddrPtr(), cid));
 }
 
 // Tests that the correct DHCPv4 message name is returned for various

src/lib/dhcpsrv/alloc_engine_messages.mes

@@ -350,7 +350,7 @@ This debug message is issued when the server begins reclamation of the
 expired DHCPv6 lease. The reclaimed lease may either be an address lease
 or delegated prefix. The first argument provides the client identification
 information. The other arguments specify the prefix and the prefix length
-for the lease. The prefix length for address lease is equal to.
+for the lease. The prefix length for address lease is equal to 128.
 
 % ALLOC_ENGINE_V6_LEASE_RECLAMATION_FAILED failed to reclaim the lease %1: %2
 This error message is logged when the allocation engine fails to

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -589,16 +589,24 @@ lease from the PostgreSQL database for the specified address.
 
 % DHCPSRV_QUEUE_NCR %1: name change request to %2 DNS entry queued: %3
 A debug message which is logged when the NameChangeRequest to add or remove
-a DNS entries for a particular lease has been queued. The first parameter
-includes the client identification information. The second parameter of
-indicates whether the DNS entry is to be added or removed. The second
-parameter carries the details of the NameChangeRequest.
+a DNS entries for a particular lease has been queued. The first argument
+includes the client identification information. The second argument
+indicates whether the DNS entry is to be added or removed. The third
+argument carries the details of the NameChangeRequest.
 
-% DHCPSRV_QUEUE_NCR_FAILED queueing %1 name change request failed for lease %2: %3
+% DHCPSRV_QUEUE_NCR_FAILED %1: queueing %2 name change request failed for lease %3: %4
 This error message is logged when sending a name change request
-to DHCP DDNS failed. The type of the name change request is specified
-as first argument. The remaining arguments provide the details of the
-lease and the reson for failure.
+to DHCP DDNS failed. The first argument includes the client identification
+information. The second argument indicates whether the DNS entry is to be
+added or removed. The third argument specifies the leased address. The
+last argument provides the reason for failure.
+
+% DHCPSRV_QUEUE_NCR_SKIP %1: skip queueing name change request for lease: %2
+This debug message is issued when the server decides to not queue the name
+change request because the lease doesn't include the FQDN, the forward and
+reverse update is disabled for this lease or the DNS updates are disabled
+in the configuration. The first argument includes the client identification
+information. The second argument includes the leased address.
 
 % DHCPSRV_TIMERMGR_CALLBACK_FAILED running handler for timer %1 caused exception: %2
 This error message is emitted when the timer elapsed and the

src/lib/dhcpsrv/ncr_generator.cc

@@ -44,8 +44,13 @@ void queueNCRCommon(const NameChangeType& chg_type, const LeasePtrType& lease,
                     const IdentifierType& identifier, const std::string& label) {
 
     // Check if there is a need for update.
-    if (!lease || lease->hostname_.empty() || (!lease->fqdn_fwd_ && !lease->fqdn_rev_)
+    if (lease->hostname_.empty() || (!lease->fqdn_fwd_ && !lease->fqdn_rev_)
         || !CfgMgr::instance().getD2ClientMgr().ddnsEnabled()) {
+        LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
+                  DHCPSRV_QUEUE_NCR_SKIP)
+            .arg(label)
+            .arg(lease->addr_.toText());
+            
         return;
     }
 
@@ -72,6 +77,7 @@ void queueNCRCommon(const NameChangeType& chg_type, const LeasePtrType& lease,
 
     } catch (const std::exception& ex) {
         LOG_ERROR(dhcpsrv_logger, DHCPSRV_QUEUE_NCR_FAILED)
+            .arg(label)
             .arg(chg_type == CHG_ADD ? "add" : "remove")
             .arg(lease->addr_.toText())
             .arg(ex.what());

src/lib/dhcpsrv/tests/alloc_engine_expiration_unittest.cc

@@ -1216,7 +1216,10 @@ ExpirationAllocEngine6Test::createLeases() {
                                    20, SubnetID(1), true, true,
                                    generateHostnameForLeaseIndex(i)));
         leases_.push_back(lease);
-        LeaseMgrFactory::instance().addLease(lease);
+        // Copy the lease before adding it to the lease manager. We want to
+        // make sure that modifications to the leases held in the leases_
+        // container doesn't affect the leases in the lease manager.
+        LeaseMgrFactory::instance().addLease(Lease6Ptr(new Lease6(*lease)));
 
         // Note in the statistics that this lease has been added.
         StatsMgr& stats_mgr = StatsMgr::instance();
@@ -1324,7 +1327,7 @@ ExpirationAllocEngine6Test::testReclaimExpiredLeasesStats() {
 void
 ExpirationAllocEngine6Test::testReclaimReusedLeases(const uint16_t msg_type,
                                                     const bool use_reclaimed) {
-    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 0xFFFF);
+    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 1000);
 
     for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
         // Depending on the parameter, mark leases 'expired-reclaimed' or
@@ -1368,7 +1371,7 @@ ExpirationAllocEngine6Test::testReclaimReusedLeases(const uint16_t msg_type,
             ASSERT_NO_THROW(engine_->renewLeases6(ctx));
 
         } else {
-            ASSERT_NO_THROW(engine_->renewLeases6(ctx));
+            ASSERT_NO_THROW(engine_->allocateLeases6(ctx));
         }
     }
 
@@ -1632,7 +1635,8 @@ public:
     /// @param client_renews A boolean value which indicates if the test should
     /// simulate renewals of leases (if true) or reusing expired leases which
     /// belong to different clients (if false).
-    /// @param use_reclaimed Boolean parameter indicating if the leases
+    /// @param use_reclaimed Boolean parameter indicating if the leases being
+    /// reused should initially be reclaimed.
     void testReclaimReusedLeases(const uint8_t msg_type, const bool client_renews,
                                  const bool use_reclaimed);
 
@@ -1666,7 +1670,10 @@ ExpirationAllocEngine4Test::createLeases() {
                                    time(NULL), SubnetID(1), true, true,
                                    generateHostnameForLeaseIndex(i)));
         leases_.push_back(lease);
-        LeaseMgrFactory::instance().addLease(lease);
+        // Copy the lease before adding it to the lease manager. We want to
+        // make sure that modifications to the leases held in the leases_
+        // container doesn't affect the leases in the lease manager.
+        LeaseMgrFactory::instance().addLease(Lease4Ptr(new Lease4(*lease)));
 
         // Note in the statistics that this lease has been added.
         StatsMgr& stats_mgr = StatsMgr::instance();
@@ -1865,8 +1872,8 @@ void
 ExpirationAllocEngine4Test::testReclaimReusedLeases(const uint8_t msg_type,
                                                     const bool client_renews,
                                                     const bool use_reclaimed) {
-    // Let's restrict the number of leases to /16.
-    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 0xFFFF);
+    // Let's restrict the number of leases.
+    BOOST_STATIC_ASSERT(TEST_LEASES_NUM < 1000);
 
     for (unsigned int i = 0; i < TEST_LEASES_NUM; ++i) {
         // Depending on the parameter, mark leases 'expired-reclaimed' or

src/lib/dhcpsrv/tests/ncr_generator_unittest.cc

@@ -232,6 +232,16 @@ public:
                                 lease_->cltt_ + lease_->valid_lft_,
                                 lease_->valid_lft_);
     }
+
+    /// @brief Test that calling queueNCR for NULL lease doesn't cause
+    /// an exception.
+    ///
+    /// @param chg_type Name change type.
+    void testNullLease(const NameChangeType chg_type) {
+        lease_.reset();
+        ASSERT_NO_FATAL_FAILURE(queueNCR(chg_type, lease_));
+        EXPECT_EQ(0, d2_mgr_.getQueueSize());
+    }
 };
 
 /// @brief Test fixture class implementation for DHCPv6.
@@ -270,12 +280,29 @@ TEST_F(NCRGenerator6Test, fwdRev) {
                 "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
                 "D0ECCEA5E0DD71730F392119A");
     }
+
+    // Now try the same test with all lower case.
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, true, true, "myhost.example.com.",
+                "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
+                "D0ECCEA5E0DD71730F392119A");
+    }
+
     {
         SCOPED_TRACE("case CHG_ADD");
         testNCR(CHG_ADD, true, true, "MYHOST.example.com.",
                 "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
                 "D0ECCEA5E0DD71730F392119A");
     }
+
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, true, true, "myhost.example.com.",
+                "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
+                "D0ECCEA5E0DD71730F392119A");
+    }
+
 }
 
 // Checks that NameChangeRequests are not created when ddns updates are disabled.
@@ -292,7 +319,7 @@ TEST_F(NCRGenerator6Test, d2Disabled) {
 
 // Test creation of the NameChangeRequest for reverse mapping in the
 // given lease.
-TEST_F(NCRGenerator6Test, removeRev) {
+TEST_F(NCRGenerator6Test, revOnly) {
     {
         SCOPED_TRACE("case CHG_REMOVE");
         testNCR(CHG_REMOVE, false, true, "myhost.example.com.",
@@ -308,6 +335,25 @@ TEST_F(NCRGenerator6Test, removeRev) {
     }
 }
 
+// Test creation of the NameChangeRequest for forward mapping in the
+// given lease.
+TEST_F(NCRGenerator6Test, fwdOnly) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, true, false, "myhost.example.com.",
+                "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
+                "D0ECCEA5E0DD71730F392119A");
+    }
+
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, true, false, "myhost.example.com.",
+                "000201BE0D7A66F8AB6C4082E7F8B81E2656667A102E3"
+                "D0ECCEA5E0DD71730F392119A");
+    }
+}
+
+
 // Test that NameChangeRequest is not generated when neither forward
 // nor reverse DNS update has been performed for a lease.
 TEST_F(NCRGenerator6Test, noFwdRevUpdate) {
@@ -340,11 +386,11 @@ TEST_F(NCRGenerator6Test, noHostname) {
 TEST_F(NCRGenerator6Test, wrongHostname) {
     {
         SCOPED_TRACE("case CHG_REMOVE");
-        testNoUpdate(CHG_REMOVE, false, false, "");
+        testNoUpdate(CHG_REMOVE, false, false, "myhost...example.com.");
     }
     {
         SCOPED_TRACE("case CHG_ADD");
-        testNoUpdate(CHG_ADD, false, false, "");
+        testNoUpdate(CHG_ADD, false, false, "myhost...example.com.");
     }
 }
 
@@ -364,6 +410,20 @@ TEST_F(NCRGenerator6Test, wrongLeaseType) {
     }
 }
 
+// Test that NameChangeRequest is not generated if the lease is NULL,
+// and that the call to queueNCR doesn't cause an exception or
+// assertion.
+TEST_F(NCRGenerator6Test, nullLease) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNullLease(CHG_REMOVE);
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNullLease(CHG_ADD);
+    }
+}
+
 /// @brief Test fixture class implementation for DHCPv4.
 class NCRGenerator4Test : public NCRGeneratorTest<Lease4Ptr> {
 public:
@@ -380,7 +440,7 @@ public:
 
     /// @brief Implementation of the method creating DHCPv6 lease instance.
     virtual void initLease() {
-        lease_.reset(new Lease4(IOAddress("2001:db8:1::1"), hwaddr_, ClientIdPtr(),
+        lease_.reset(new Lease4(IOAddress("192.0.2.1"), hwaddr_, ClientIdPtr(),
                                 100, 30, 60, time(NULL), 1));
     }
 };
@@ -397,12 +457,28 @@ TEST_F(NCRGenerator4Test, fwdRev) {
                 "000001E356D43E5F0A496D65BCA24D982D646140813E3"
                 "B03AB370BFF46BFA309AE7BFD");
     }
+
+    // Now try the same with all lower case.
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, true, true, "myhost.example.com.",
+                "000001E356D43E5F0A496D65BCA24D982D646140813E3"
+                "B03AB370BFF46BFA309AE7BFD");
+    }
+
     {
         SCOPED_TRACE("case CHG_ADD");
         testNCR(CHG_ADD, true, true, "MYHOST.example.com.",
                 "000001E356D43E5F0A496D65BCA24D982D646140813E3"
                 "B03AB370BFF46BFA309AE7BFD");
     }
+
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, true, true, "myhost.example.com.",
+                "000001E356D43E5F0A496D65BCA24D982D646140813E3"
+                "B03AB370BFF46BFA309AE7BFD");
+    }
 }
 
 // Checks that NameChangeRequests are not created when ddns updates are disabled.
@@ -419,7 +495,7 @@ TEST_F(NCRGenerator4Test, d2Disabled) {
 
 // Test creation of the NameChangeRequest for reverse mapping in the
 // given lease.
-TEST_F(NCRGenerator4Test, removeRev) {
+TEST_F(NCRGenerator4Test, revOnly) {
     {
         SCOPED_TRACE("case CHG_REMOVE");
         testNCR(CHG_REMOVE, false, true, "myhost.example.com.",
@@ -434,6 +510,23 @@ TEST_F(NCRGenerator4Test, removeRev) {
     }
 }
 
+// Test creation of the NameChangeRequest for forward mapping in the
+// given lease.
+TEST_F(NCRGenerator4Test, fwdOnly) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNCR(CHG_REMOVE, true, false, "myhost.example.com.",
+                "000001E356D43E5F0A496D65BCA24D982D646140813E3B"
+                "03AB370BFF46BFA309AE7BFD");
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNCR(CHG_ADD, true, false, "myhost.example.com.",
+                "000001E356D43E5F0A496D65BCA24D982D646140813E3B"
+                "03AB370BFF46BFA309AE7BFD");
+    }
+}
+
 // Test that NameChangeRequest is not generated when neither forward
 // nor reverse DNS update has been performed for a lease.
 TEST_F(NCRGenerator4Test, noFwdRevUpdate) {
@@ -466,11 +559,11 @@ TEST_F(NCRGenerator4Test, noHostname) {
 TEST_F(NCRGenerator4Test, wrongHostname) {
     {
         SCOPED_TRACE("case CHG_REMOVE");
-        testNoUpdate(CHG_REMOVE, false, false, "");
+        testNoUpdate(CHG_REMOVE, false, false, "myhost...example.org.");
     }
     {
         SCOPED_TRACE("case CHG_ADD");
-        testNoUpdate(CHG_ADD, false, false, "");
+        testNoUpdate(CHG_ADD, false, false, "myhost...example.org.");
     }
 }
 
@@ -483,7 +576,7 @@ TEST_F(NCRGenerator4Test, useClientId) {
     ASSERT_EQ(1, d2_mgr_.getQueueSize());
 
     verifyNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, true, true,
-                            "2001:db8:1::1",
+                            "192.0.2.1",
                             "000101C7AA5420483BDA99C437636EA7DA2FE18"
                             "31C9679FEB031C360CA571298F3D1FA",
                             lease_->cltt_ + lease_->valid_lft_, 100);
@@ -499,7 +592,20 @@ TEST_F(NCRGenerator4Test, useClientId) {
                 "000101C7AA5420483BDA99C437636EA7DA2FE1831C9679"
                 "FEB031C360CA571298F3D1FA");
     }
+}
 
+// Test that NameChangeRequest is not generated if the lease is NULL,
+// and that the call to queueNCR doesn't cause an exception or
+// assertion.
+TEST_F(NCRGenerator4Test, nullLease) {
+    {
+        SCOPED_TRACE("case CHG_REMOVE");
+        testNullLease(CHG_REMOVE);
+    }
+    {
+        SCOPED_TRACE("case CHG_ADD");
+        testNullLease(CHG_ADD);
+    }
 }
 
 } // end of anonymous namespace