From: Thomas Markwalder <tmark@isc.org>
Date: Mon, 6 Oct 2025 13:44:31 +0000 (-0400)
Subject: [#4142] Fix qualifyName, add v6 UT
X-Git-Tag: Kea-3.1.3~12
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=8ac09c737c6b09bb3688683d9a5e889e1810e3d9;p=thirdparty%2Fkea.git

[#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 4bb5dc18d4..18e4c6d4b9 100644
--- 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 ca51856e67..7891c1f5e6 100644
--- 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 84ee11d9fb..10e44140bc 100644
--- 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 520acbbb88..15dbed9eb5 100644
--- 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