[#4142] Fix qualifyName, add v6 UT

modified:   src/bin/dhcp4/tests/fqdn_unittest.cc
modified:   src/bin/dhcp6/tests/fqdn_unittest.cc
modified:   src/lib/dhcpsrv/d2_client_mgr.cc
modified:   src/lib/dhcpsrv/tests/d2_client_unittest.cc

diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc
index 4bb5dc18d42d1a2da885d7269229ce3894814a60..18e4c6d4b9bb8ed0d86060bfe3cf9833f1952f88 100644 (file)
--- a/src/bin/dhcp4/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp4/tests/fqdn_unittest.cc
@@ -3221,10 +3221,7 @@ TEST_F(NameDhcpv4SrvTest, hostnameScrubbedEmpty) {
     ASSERT_EQ(DHCPOFFER, static_cast<int>(resp->getType()));
 
     // Should have logged that it was scrubbed empty.
-    std::string log = "DHCP4_CLIENT_HOSTNAME_SCRUBBED_EMPTY"
-                      " [hwtype=1 67:c6:69:73:51:ff], cid=[no info], tid=0x0:"
-                      " sanitiziing client's Hostname option '___'"
-                      " yielded an empty string";
+    std::string log = "DHCP4_CLIENT_HOSTNAME_SCRUBBED_EMPTY";
     EXPECT_EQ(1, countFile(log));
 
     // Hostname should not be in the response.
@@ -3250,10 +3247,7 @@ TEST_F(NameDhcpv4SrvTest, fqdnScrubbedEmpty) {
     ASSERT_EQ(DHCPOFFER, static_cast<int>(resp->getType()));
 
     // Should have logged that it was scrubbed empty.
-    std::string log = "DHCP4_CLIENT_FQDN_SCRUBBED_EMPTY"
-                      " [hwtype=1 67:c6:69:73:51:ff], cid=[no info], tid=0x0:"
-                      " sanitiziing client's FQDN option '___'"
-                      " yielded an empty string";
+    std::string log = "DHCP4_CLIENT_FQDN_SCRUBBED_EMPTY";
     EXPECT_EQ(1, countFile(log));
 
     // Hostname should not be in the response.

diff --git a/src/bin/dhcp6/tests/fqdn_unittest.cc b/src/bin/dhcp6/tests/fqdn_unittest.cc
index ca51856e67926aa10f34294679ea8b1058c422a0..7891c1f5e6c99d00b233422083c2502a3bb207fe 100644 (file)
--- a/src/bin/dhcp6/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp6/tests/fqdn_unittest.cc
@@ -2425,4 +2425,27 @@ TEST_F(FqdnDhcpv6SrvTest, poolDdnsParametersTest) {
     }
 }
 
+// Verify an FQDN with all invalid chars is ignored.
+TEST_F(FqdnDhcpv6SrvTest, fqdnScrubbedEmpty) {
+    // Create the query.
+    Pkt6Ptr question = generateMessage(DHCPV6_SOLICIT, Option6ClientFqdn::FLAG_S,
+                                       "___" , Option6ClientFqdn::FULL, true);
+    ASSERT_TRUE(getClientFqdnOption(question));
+    subnet_->setHostnameCharReplacement("");
+
+    // Create the response with an "assigned" lease.
+    // Set the selected subnet so ddns params get returned correctly.
+    AllocEngine::ClientContext6 ctx;
+    ctx.subnet_ = subnet_;
+    Pkt6Ptr answer = generateMessageWithIds(DHCPV6_ADVERTISE);
+    addIA(1234, IOAddress("2001:db8:1::1"), answer, ctx);
+
+    // Process the client's FQDN.
+    ASSERT_NO_THROW(srv_->processClientFqdn(question, answer, ctx));
+
+    // Should not have an FQDN option in the answer.
+    EXPECT_FALSE(answer->getOption(D6O_CLIENT_FQDN));
+    countFile("DHCP6_CLIENT_FQDN_SCRUBBED_EMPTY");
+}
+
 } // end of anonymous namespace

diff --git a/src/lib/dhcpsrv/d2_client_mgr.cc b/src/lib/dhcpsrv/d2_client_mgr.cc
index 84ee11d9fbbca531208f28cbddea2b3adf95e8cd..10e44140bcbb8ef14560126ee5a8f8419041eaf3 100644 (file)
--- a/src/lib/dhcpsrv/d2_client_mgr.cc
+++ b/src/lib/dhcpsrv/d2_client_mgr.cc
@@ -189,7 +189,8 @@ D2ClientMgr::qualifyName(const std::string& partial_name,
     std::ostringstream gen_name;
     gen_name << partial_name;
     std::string suffix = ddns_params.getQualifyingSuffix();
-    if (!suffix.empty() && partial_name.back() != '.') {
+    if ((!suffix.empty()) && (!partial_name.empty())
+        && (partial_name.back() != '.')) {
         bool suffix_present = true;
         std::string str = gen_name.str();
         auto suffix_rit = suffix.rbegin();

diff --git a/src/lib/dhcpsrv/tests/d2_client_unittest.cc b/src/lib/dhcpsrv/tests/d2_client_unittest.cc
index 520acbbb88e6fc365ccc019d6f0b1f2fa62e2c3f..15dbed9eb599cc5a074fecc4d6957884025b3526 100644 (file)
--- a/src/lib/dhcpsrv/tests/d2_client_unittest.cc
+++ b/src/lib/dhcpsrv/tests/d2_client_unittest.cc
@@ -628,6 +628,11 @@ TEST_F(D2ClientMgrParamsTest, qualifyName) {
     qualified_name = mgr.qualifyName(partial_name, *ddns_params_, do_not_dot);
     EXPECT_EQ("somehost.suffix.com", qualified_name);
 
+    // Verify that an empty name does not crash.
+    partial_name = "";
+    qualified_name = mgr.qualifyName(partial_name, *ddns_params_, do_not_dot);
+    EXPECT_EQ("", qualified_name);
+
     // Verify that an empty suffix and false flag, does not change the name
     subnet_->setDdnsQualifyingSuffix("");
     partial_name = "somehost";
@@ -660,6 +665,11 @@ TEST_F(D2ClientMgrParamsTest, qualifyName) {
     // suffix is blank and trailing dot is false
     qualified_name = mgr.qualifyName("somehost.", *ddns_params_, do_not_dot);
     EXPECT_EQ("somehost", qualified_name);
+
+    // Verify that an empty name and an empty suffix does not crash.
+    partial_name = "";
+    qualified_name = mgr.qualifyName(partial_name, *ddns_params_, do_not_dot);
+    EXPECT_EQ("", qualified_name);
 }
 
 /// @brief Tests the qualifyName method's ability to avoid duplicating