BIND 10 trac3035, updated. 1c9cf817f494416bfe7dba7896c4e23f331239bc [3035] Include the lease expiration time in the NameChangeRequest.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Nov 20 14:32:15 UTC 2013
The branch, trac3035 has been updated
via 1c9cf817f494416bfe7dba7896c4e23f331239bc (commit)
via 9ea3131be861042b883033530ffaec46d190dfa2 (commit)
via b7dd9c0a496dfc8cc5779f0130a5375af1e84d14 (commit)
from 90b595cca8c6b9041e9680db1d00f41f0d80423b (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 1c9cf817f494416bfe7dba7896c4e23f331239bc
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Nov 20 15:31:57 2013 +0100
[3035] Include the lease expiration time in the NameChangeRequest.
commit 9ea3131be861042b883033530ffaec46d190dfa2
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Nov 20 14:04:40 2013 +0100
[3035] Skip processing invalid Hostname options sent by a client.
commit b7dd9c0a496dfc8cc5779f0130a5375af1e84d14
Author: Marcin Siodelski <marcin at isc.org>
Date: Tue Nov 19 17:29:40 2013 +0100
[3035] Addressed "minor" issues rised in the review comments.
The minor issues addressed are:
- Missing comments
- Lack of todo
- Log messages
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/dhcp4.dox | 4 +-
src/bin/dhcp4/dhcp4_messages.mes | 28 +++---
src/bin/dhcp4/dhcp4_srv.cc | 80 ++++++++++------
src/bin/dhcp4/dhcp4_srv.h | 22 +++++
src/bin/dhcp4/tests/fqdn_unittest.cc | 109 +++++++++++++++-------
src/lib/dhcp/option_data_types.cc | 8 ++
src/lib/dhcp/option_data_types.h | 7 +-
src/lib/dhcp/tests/option_data_types_unittest.cc | 16 +++-
8 files changed, 197 insertions(+), 77 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/dhcp4.dox b/src/bin/dhcp4/dhcp4.dox
index 930ac63..fc5d3f9 100644
--- a/src/bin/dhcp4/dhcp4.dox
+++ b/src/bin/dhcp4/dhcp4.dox
@@ -96,7 +96,7 @@ client to indicate that this name will be used to perform DNS update.
The b10-dhcp-ddns process is responsible for the actual communication with the
DNS, i.e. to send DNS update messages. The b10-dhcp4 module is responsible for
-generating so called @ref isc::dhcp_ddns::NameChangeRequest and sending it to
+generating @ref isc::dhcp_ddns::NameChangeRequest and sending it to
the b10-dhcp-ddns module. The @ref isc::dhcp_ddns::NameChangeRequest object
represents changes to the DNS bindings, related to acquisition, renewal or
release of the DHCP lease. The b10-dhcp4 module implements the simple FIFO queue
@@ -139,7 +139,7 @@ change comparing to the NameChangeRequest is not generated.
The DHCPv4 Client FQDN %Option comprises flags which communicate to the server
what updates (if any) client expects the server to perform. Server may be
-configured to obey client's preference to do FQDN processing in a different way.
+configured to obey client's preference or to do FQDN processing in a different way.
If the server overrides client's preference it will communicate it by sending
the DHCPv4 Client FQDN %Option in its responses to a client, with the appropriate
flags set.
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index f666518..b4b834f 100644
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -27,6 +27,11 @@ successfully established a session with the BIND 10 control channel.
This debug message is issued just before the IPv4 DHCP server attempts
to establish a session with the BIND 10 control channel.
+% DHCP4_CLIENT_NAME_PROC_FAIL failed to process the fqdn or hostname sent by a client: %1
+This debug message is issued when the DHCP server was unable to process the
+FQDN or Hostname option sent by a client. This is likely because the client's
+name was malformed or due to internal server error.
+
% DHCP4_COMMAND_RECEIVED received command %1, arguments: %2
A debug message listing the command (and possible arguments) received
from the BIND 10 control system by the IPv4 DHCP server.
@@ -75,6 +80,11 @@ This error message is logged when the attempt to compute DHCID for a specified
lease has failed. The lease details and reason for failure is logged in the
message.
+% DHCP4_EMPTY_HOSTNAME received empty hostname from the client, skipping processing of this option
+This debug message is issued when the server received an empty Hostname option
+from a client. Server does not process empty Hostname options and therefore
+option is skipped.
+
% DHCP4_HOOK_BUFFER_RCVD_SKIP received DHCPv4 buffer was dropped because a callout set the skip flag.
This debug message is printed when a callout installed on buffer4_receive
hook point set the skip flag. For this particular hook point, the
@@ -146,8 +156,8 @@ from the acquired address. The message argument indicates the reason for the
failure.
% DHCP4_NCR_CREATION_FAILED failed to generate name change requests for DNS: %1
-This message indicates that server was unable to generate so called
-NameChangeRequests which should be sent to the b10-dhcp_ddns module to create
+This message indicates that server was unable to generate NameChangeRequests
+which should be sent to the b10-dhcp_ddns module to create
new DNS records for the lease being acquired or to update existing records
for the renewed lease. The reason for the failure is printed in the logged
message.
@@ -229,15 +239,11 @@ failure is given in the message.
% DHCP4_QUERY_DATA received packet type %1, data is <%2>
A debug message listing the data received from the client.
-% DHCP4_QUEUE_ADDITION_NCR name change request for adding new DNS entry queued: %1
-A debug message which is logged when the NameChangeRequest to add new DNS
-entries for the particular lease has been queued. The parameter of this log
-carries the details of the NameChangeRequest.
-
-% DHCP4_QUEUE_REMOVAL_NCR name change request for removing a DNS entry queued: %1
-A debug message which is logged when the NameChangeRequest to remove existing
-DNS entry for the particular lease has been queued. The parameter of this log
-carries the details of the NameChangeRequest.
+% DHCP4_QUEUE_NCR name change request to %1 DNS entry queued: %2
+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 of
+this log message indicates whether the DNS entry is to be added or removed.
+The second parameter carries the details of the NameChangeRequest.
% DHCP4_RELEASE address %1 belonging to client-id %2, hwaddr %3 was released properly.
This debug message indicates that an address was released properly. It
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 7d23c3e..873a3e7 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -33,6 +33,7 @@
#include <dhcpsrv/utils.h>
#include <hooks/callout_handle.h>
#include <hooks/hooks_manager.h>
+#include <util/strutil.h>
#include <boost/algorithm/string/erase.hpp>
@@ -78,10 +79,15 @@ namespace dhcp {
namespace {
-// The following constants describe server's behavior with respect to the
+// @todo The following constants describe server's behavior with respect to the
// DHCPv4 Client FQDN Option sent by a client. They will be removed
// when DDNS parameters for DHCPv4 are implemented with the ticket #3033.
+// @todo Additional configuration parameter which we may consider is the one
+// that controls whether the DHCP server sends the removal NameChangeRequest
+// if it discovers that the entry for the particular client exists or that
+// it always updates the DNS.
+
// Should server always include the FQDN option in its response, regardless
// if it has been requested in Parameter Request List Option (Disabled).
const bool FQDN_ALWAYS_INCLUDE = false;
@@ -742,18 +748,31 @@ Dhcpv4Srv::processClientName(const Pkt4Ptr& query, Pkt4Ptr& answer) {
// It is possible that client has sent both Client FQDN and Hostname
// option. In such case, server should prefer Client FQDN option and
// ignore the Hostname option.
- Option4ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<Option4ClientFqdn>
- (query->getOption(DHO_FQDN));
- if (fqdn) {
- processClientFqdnOption(fqdn, answer);
+ try {
+ Option4ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<Option4ClientFqdn>
+ (query->getOption(DHO_FQDN));
+ if (fqdn) {
+ processClientFqdnOption(fqdn, answer);
- } else {
- OptionCustomPtr hostname = boost::dynamic_pointer_cast<OptionCustom>
- (query->getOption(DHO_HOST_NAME));
- if (hostname) {
- processHostnameOption(hostname, answer);
- }
+ } else {
+ OptionCustomPtr hostname = boost::dynamic_pointer_cast<OptionCustom>
+ (query->getOption(DHO_HOST_NAME));
+ if (hostname) {
+ processHostnameOption(hostname, answer);
+ }
+ }
+ } catch (const Exception& ex) {
+ // In some rare cases it is possible that the client's name processing
+ // fails. For example, the Hostname option may be malformed, or there
+ // may be an error in the server's logic which would cause multiple
+ // attempts to add the same option to the response message. This
+ // error message aggregates all these errors so they can be diagnosed
+ // from the log. We don't want to throw an exception here because,
+ // it will impact the processing of the whole packet. We rather want
+ // the processing to continue, even if the client's name is wrong.
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_CLIENT_NAME_PROC_FAIL)
+ .arg(ex.what());
}
}
@@ -850,8 +869,14 @@ Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
return;
}
- std::string hostname = opt_hostname->readString();
+ std::string hostname = isc::util::str::trim(opt_hostname->readString());
unsigned int label_count = OptionDataTypeUtil::getLabelCount(hostname);
+ // The hostname option sent by the client should be at least 1 octet long.
+ // If it isn't we ignore this option.
+ if (label_count == 0) {
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_EMPTY_HOSTNAME);
+ return;
+ }
// Copy construct the hostname provided by the client. It is entirely
// possible that we will use the hostname option provided by the client
// to perform the DNS update and we will send the same option to him to
@@ -865,8 +890,12 @@ Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
// By checking the number of labels present in the hostname we may infer
// whether client has sent the fully qualified or unqualified hostname.
- // If there is only one label, it is a root. We will have to generate
- // the whole domain name for the client.
+ // @todo We may want to reconsider whether it is appropriate for the
+ // client to send a root domain name as a Hostname. There are
+ // also extensions to the auto generation of the client's name,
+ // e.g. conversion to the puny code which may be considered at some point.
+ // For now, we just remain liberal and expect that the DNS will handle
+ // conversion if needed and possible.
if (FQDN_REPLACE_CLIENT_NAME || (label_count < 2)) {
opt_hostname_resp->writeString("");
// If there are two labels, it means that the client has specified
@@ -940,31 +969,26 @@ queueNameChangeRequest(const isc::dhcp_ddns::NameChangeType chg_type,
(!lease->fqdn_rev_ && !lease->fqdn_fwd_)) {
return;
}
+
+ // Create the DHCID for the NameChangeRequest.
D2Dhcid dhcid;
try {
dhcid = computeDhcid(lease);
-
} catch (const DhcidComputeError& ex) {
LOG_ERROR(dhcp4_logger, DHCP4_DHCID_COMPUTE_ERROR)
.arg(lease->toText())
.arg(ex.what());
return;
-
}
+ // Create NameChangeRequest
NameChangeRequest ncr(chg_type, lease->fqdn_fwd_, lease->fqdn_rev_,
lease->hostname_, lease->addr_.toText(),
- dhcid, 0, lease->valid_lft_);
- if (chg_type == isc::dhcp_ddns::CHG_ADD) {
- LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA,
- DHCP4_QUEUE_ADDITION_NCR)
- .arg(ncr.toText());
-
- } else {
- LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA,
- DHCP4_QUEUE_REMOVAL_NCR)
- .arg(ncr.toText());
-
- }
+ dhcid, lease->cltt_ + lease->valid_lft_,
+ lease->valid_lft_);
+ // And queue it.
+ LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_QUEUE_NCR)
+ .arg(chg_type == CHG_ADD ? "add" : "remove")
+ .arg(ncr.toText());
name_change_reqs_.push(ncr);
}
diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index b878619..7b21b7b 100644
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -300,6 +300,12 @@ protected:
private:
/// @brief Process Client FQDN %Option sent by a client.
///
+ /// This function is called by the @c Dhcpv4Srv::processClientName when
+ /// the client has sent the FQDN option in its message to the server.
+ /// It comprises the actual logic to parse the FQDN option and prepare
+ /// the FQDN option to be sent back to the client in the server's
+ /// response.
+ ///
/// @param fqdn An DHCPv4 Client FQDN %Option sent by a client.
/// @param [out] answer A response message to be sent to a client.
void processClientFqdnOption(const Option4ClientFqdnPtr& fqdn,
@@ -433,8 +439,24 @@ protected:
/// @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.
diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc
index f4d4130..5f111f9 100644
--- a/src/bin/dhcp4/tests/fqdn_unittest.cc
+++ b/src/bin/dhcp4/tests/fqdn_unittest.cc
@@ -194,11 +194,14 @@ public:
}
- // Test that the Hostname option is returned in the server's response
- // to the message holding Hostname option sent by a client.
- void testProcessHostname(const Pkt4Ptr& query,
- const std::string& exp_hostname) {
- ASSERT_TRUE(getHostnameOption(query));
+ // Processes the Hostname option in the client's message and returns
+ // the hostname option which would be sent to the client. It will
+ // throw NULL pointer if the hostname option is not to be included
+ // in the response.
+ OptionCustomPtr processHostname(const Pkt4Ptr& query) {
+ if (!getHostnameOption(query)) {
+ ADD_FAILURE() << "Hostname option not carried in the query";
+ }
Pkt4Ptr answer;
if (query->getType() == DHCPDISCOVER) {
@@ -208,16 +211,13 @@ public:
answer.reset(new Pkt4(DHCPACK, 1234));
}
- ASSERT_NO_THROW(srv_->processClientName(query, answer));
+ srv_->processClientName(query, answer);
OptionCustomPtr hostname = getHostnameOption(answer);
- ASSERT_TRUE(hostname);
-
- EXPECT_EQ(exp_hostname, hostname->readString());
+ return (hostname);
}
-
// Test that the client message holding an FQDN is processed and the
// NameChangeRequests are generated.
void testProcessMessageWithFqdn(const uint8_t msg_type,
@@ -255,8 +255,9 @@ public:
const std::string& addr,
const std::string& fqdn,
const std::string& dhcid,
- const uint16_t expires,
- const uint16_t len) {
+ const time_t cltt,
+ const uint16_t len,
+ const bool not_strict_expire_check = false) {
NameChangeRequest ncr = srv_->name_change_reqs_.front();
EXPECT_EQ(type, ncr.getChangeType());
EXPECT_EQ(forward, ncr.isForwardChange());
@@ -268,7 +269,16 @@ public:
if (!dhcid.empty()) {
EXPECT_EQ(dhcid, ncr.getDhcid().toStr());
}
- EXPECT_EQ(expires, ncr.getLeaseExpiresOn());
+ // In some cases, the test doesn't have access to the last transmission
+ // time for the particular client. In such cases, the test can use the
+ // current time as cltt but the it may not check the lease expiration time
+ // for equality but rather check that the lease expiration time is not
+ // greater than the current time + lease lifetime.
+ if (not_strict_expire_check) {
+ EXPECT_GE(cltt + len, ncr.getLeaseExpiresOn());
+ } else {
+ EXPECT_EQ(cltt + len, ncr.getLeaseExpiresOn());
+ }
EXPECT_EQ(len, ncr.getLeaseLength());
EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr.getStatus());
srv_->name_change_reqs_.pop();
@@ -353,13 +363,40 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateForwardFqdn) {
}
// Test that server processes the Hostname option sent by a client and
-// responds with the Hostname option to confirm that the
+// responds with the Hostname option to confirm that the server has
+// taken responsibility for the update.
TEST_F(NameDhcpv4SrvTest, serverUpdateHostname) {
- Pkt4Ptr query = generatePktWithHostname(DHCPREQUEST,
- "myhost.example.com.");
- testProcessHostname(query, "myhost.example.com.");
+ Pkt4Ptr query;
+ ASSERT_NO_THROW(query = generatePktWithHostname(DHCPREQUEST,
+ "myhost.example.com."));
+ OptionCustomPtr hostname;
+ ASSERT_NO_THROW(hostname = processHostname(query));
+
+ ASSERT_TRUE(hostname);
+ EXPECT_EQ("myhost.example.com.", hostname->readString());
+
}
+// Test that the server skips processing of the empty Hostname option.
+TEST_F(NameDhcpv4SrvTest, serverUpdateEmptyHostname) {
+ Pkt4Ptr query;
+ ASSERT_NO_THROW(query = generatePktWithHostname(DHCPREQUEST, ""));
+ OptionCustomPtr hostname;
+ ASSERT_NO_THROW(hostname = processHostname(query));
+ EXPECT_FALSE(hostname);
+}
+
+// Test that the server skips processing of a wrong Hostname option.
+TEST_F(NameDhcpv4SrvTest, serverUpdateWrongHostname) {
+ Pkt4Ptr query;
+ ASSERT_NO_THROW(query = generatePktWithHostname(DHCPREQUEST,
+ "abc..example.com"));
+ OptionCustomPtr hostname;
+ ASSERT_NO_THROW(hostname = processHostname(query));
+ EXPECT_FALSE(hostname);
+}
+
+
// Test that server generates the fully qualified domain name for the client
// if client supplies the partial name.
TEST_F(NameDhcpv4SrvTest, serverUpdateForwardPartialNameFqdn) {
@@ -379,8 +416,14 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateForwardPartialNameFqdn) {
// Test that server generates the fully qualified domain name for the client
// if client supplies the unqualified name in the Hostname option.
TEST_F(NameDhcpv4SrvTest, serverUpdateUnqualifiedHostname) {
- Pkt4Ptr query = generatePktWithHostname(DHCPREQUEST, "myhost");
- testProcessHostname(query, "myhost.example.com.");
+ Pkt4Ptr query;
+ ASSERT_NO_THROW(query = generatePktWithHostname(DHCPREQUEST, "myhost"));
+ OptionCustomPtr hostname;
+ ASSERT_NO_THROW(hostname = processHostname(query));
+
+ ASSERT_TRUE(hostname);
+ EXPECT_EQ("myhost.example.com.", hostname->readString());
+
}
// Test that server sets empty domain-name in the FQDN option when client
@@ -443,7 +486,7 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNewLease) {
"192.0.2.3", "myhost.example.com.",
"00010132E91AA355CFBB753C0F0497A5A940436965"
"B68B6D438D98E680BF10B09F3BCF",
- 0, 100);
+ lease->cltt_, 100);
}
// Test that no NameChangeRequest is generated when a lease is renewed and
@@ -475,7 +518,7 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsNoUpdate) {
"192.0.2.3", "lease1.example.com.",
"0001013A5B311F5B9FB10DDF8E53689B874F25D"
"62CC147C2FF237A64C90E5A597C9B7A",
- 0, 100);
+ lease1->cltt_, 100);
lease2->hostname_ = "";
lease2->fqdn_rev_ = true;
@@ -501,13 +544,13 @@ TEST_F(NameDhcpv4SrvTest, createNameChangeRequestsRenew) {
"192.0.2.3", "lease1.example.com.",
"0001013A5B311F5B9FB10DDF8E53689B874F25D"
"62CC147C2FF237A64C90E5A597C9B7A",
- 0, 100);
+ lease1->cltt_, 100);
verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
"192.0.2.3", "lease2.example.com.",
"000101F906D2BB752E1B2EECC5FF2BF434C0B2D"
"D6D7F7BD873F4F280165DB8C9DBA7CB",
- 0, 100);
+ lease2->cltt_, 100);
}
@@ -562,7 +605,8 @@ TEST_F(NameDhcpv4SrvTest, processRequestFqdnEmptyDomainName) {
verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
reply->getYiaddr().toText(), hostname.str(),
"", // empty DHCID forces that it is not checked
- 0, subnet_->getValid());
+ time(NULL) + subnet_->getValid(),
+ subnet_->getValid(), true);
}
// Test that server generates client's hostname from the IP address assigned
@@ -583,10 +627,9 @@ TEST_F(NameDhcpv4SrvTest, processRequestEmptyHostname) {
verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
reply->getYiaddr().toText(), hostname.str(),
"", // empty DHCID forces that it is not checked
- 0, subnet_->getValid());
+ time(NULL), subnet_->getValid(), true);
}
-
// Test that client may send two requests, each carrying FQDN option with
// a different domain-name. Server should use existing lease for the second
// request but modify the DNS entries for the lease according to the contents
@@ -608,7 +651,7 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsFqdn) {
reply->getYiaddr().toText(), "myhost.example.com.",
"00010132E91AA355CFBB753C0F0497A5A940436"
"965B68B6D438D98E680BF10B09F3BCF",
- 0, subnet_->getValid());
+ time(NULL), subnet_->getValid(), true);
// Create another Request message but with a different FQDN. Server
// should generate two NameChangeRequests: one to remove existing entry,
@@ -629,14 +672,14 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsFqdn) {
"myhost.example.com.",
"00010132E91AA355CFBB753C0F0497A5A940436"
"965B68B6D438D98E680BF10B09F3BCF",
- 0, subnet_->getValid());
+ time(NULL), subnet_->getValid(), true);
verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
reply->getYiaddr().toText(),
"otherhost.example.com.",
"000101A5AEEA7498BD5AD9D3BF600E49FF39A7E3"
"AFDCE8C3D0E53F35CC584DD63C89CA",
- 0, subnet_->getValid());
+ time(NULL), subnet_->getValid(), true);
}
// Test that client may send two requests, each carrying Hostname option with
@@ -657,7 +700,7 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsHostname) {
reply->getYiaddr().toText(), "myhost.example.com.",
"00010132E91AA355CFBB753C0F0497A5A940436"
"965B68B6D438D98E680BF10B09F3BCF",
- 0, subnet_->getValid());
+ time(NULL), subnet_->getValid(), true);
// Create another Request message but with a different Hostname. Server
// should generate two NameChangeRequests: one to remove existing entry,
@@ -675,14 +718,14 @@ TEST_F(NameDhcpv4SrvTest, processTwoRequestsHostname) {
"myhost.example.com.",
"00010132E91AA355CFBB753C0F0497A5A940436"
"965B68B6D438D98E680BF10B09F3BCF",
- 0, subnet_->getValid());
+ time(NULL), subnet_->getValid(), true);
verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
reply->getYiaddr().toText(),
"otherhost.example.com.",
"000101A5AEEA7498BD5AD9D3BF600E49FF39A7E3"
"AFDCE8C3D0E53F35CC584DD63C89CA",
- 0, subnet_->getValid());
+ time(NULL), subnet_->getValid(), true);
}
// Test that when the Release message is sent for the previously acquired
@@ -705,7 +748,7 @@ TEST_F(NameDhcpv4SrvTest, processRequestRelease) {
reply->getYiaddr().toText(), "myhost.example.com.",
"00010132E91AA355CFBB753C0F0497A5A940436"
"965B68B6D438D98E680BF10B09F3BCF",
- 0, subnet_->getValid());
+ time(NULL), subnet_->getValid(), true);
// Create a Release message.
Pkt4Ptr rel = Pkt4Ptr(new Pkt4(DHCPRELEASE, 1234));
diff --git a/src/lib/dhcp/option_data_types.cc b/src/lib/dhcp/option_data_types.cc
index b2e84a3..4c29596 100644
--- a/src/lib/dhcp/option_data_types.cc
+++ b/src/lib/dhcp/option_data_types.cc
@@ -229,6 +229,14 @@ OptionDataTypeUtil::writeFqdn(const std::string& fqdn,
unsigned int
OptionDataTypeUtil::getLabelCount(const std::string& text_name) {
+ // The isc::dns::Name class doesn't accept empty names. However, in some
+ // cases we may be dealing with empty names (e.g. sent by the DHCP clients).
+ // Empty names should not be sent as hostnames but if they are, for some
+ // reason, we don't want to throw an exception from this function. We
+ // rather want to signal empty name by returning 0 number of labels.
+ if (text_name.empty()) {
+ return (0);
+ }
try {
isc::dns::Name name(text_name);
return (name.getLabelCount());
diff --git a/src/lib/dhcp/option_data_types.h b/src/lib/dhcp/option_data_types.h
index 253776f..d9d8a52 100644
--- a/src/lib/dhcp/option_data_types.h
+++ b/src/lib/dhcp/option_data_types.h
@@ -377,10 +377,13 @@ public:
/// @brief Return the number of labels in the Name.
///
+ /// If the specified name is empty the 0 is returned.
+ ///
/// @param text_name A text representation of the name.
///
- /// @return A number of labels in the provided name.
- /// @throw isc::BadCast if provided name is malformed.
+ /// @return A number of labels in the provided name or 0 if the
+ /// name string is empty.
+ /// @throw isc::dhcp::BadDataTypeCast if provided name is malformed.
static unsigned int getLabelCount(const std::string& text_name);
/// @brief Read string value from a buffer.
diff --git a/src/lib/dhcp/tests/option_data_types_unittest.cc b/src/lib/dhcp/tests/option_data_types_unittest.cc
index fd5294c..717a330 100644
--- a/src/lib/dhcp/tests/option_data_types_unittest.cc
+++ b/src/lib/dhcp/tests/option_data_types_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
@@ -62,6 +62,20 @@ public:
}
};
+// The goal of this test is to verify that the getLabelCount returns the
+// correct number of labels in the domain name specified as a string
+// parameter.
+TEST_F(OptionDataTypesTest, getLabelCount) {
+ EXPECT_EQ(0, OptionDataTypeUtil::getLabelCount(""));
+ EXPECT_EQ(1, OptionDataTypeUtil::getLabelCount("."));
+ EXPECT_EQ(2, OptionDataTypeUtil::getLabelCount("example"));
+ EXPECT_EQ(3, OptionDataTypeUtil::getLabelCount("example.com"));
+ EXPECT_EQ(3, OptionDataTypeUtil::getLabelCount("example.com."));
+ EXPECT_EQ(4, OptionDataTypeUtil::getLabelCount("myhost.example.com"));
+ EXPECT_THROW(OptionDataTypeUtil::getLabelCount(".abc."),
+ isc::dhcp::BadDataTypeCast);
+}
+
// The goal of this test is to verify that an IPv4 address being
// stored in a buffer (wire format) can be read into IOAddress
// object.
More information about the bind10-changes
mailing list