[4301] Addressed review comments.

src/lib/dhcpsrv/host.cc

@@ -125,6 +125,23 @@ Host::getIdentifierType() const {
     return (identifier_type_);
 }
 
+Host::IdentifierType
+Host::getIdentifierType(const std::string& identifier_name) {
+    if (identifier_name == "hw-address") {
+        return (IDENT_HWADDR);
+
+    } else if (identifier_name == "duid") {
+        return (IDENT_DUID);
+
+    } else if (identifier_name == "circuit-id") {
+        return (IDENT_CIRCUIT_ID);
+
+    } else {
+        isc_throw(isc::BadValue, "invalid client identifier type '"
+                  << identifier_name << "'");
+    }
+}
+
 HWAddrPtr
 Host::getHWAddress() const {
     return ((identifier_type_ == IDENT_HWADDR) ?
@@ -201,53 +218,35 @@ Host::setIdentifier(const uint8_t* identifier, const size_t len,
 
 void
 Host::setIdentifier(const std::string& identifier, const std::string& name) {
-    // HW address and DUID are special cases because they are typically
-    // specified as values with colons between consecutive octets. Thus,
-    // we use the HWAddr and DUID classes to validate them and to
-    // convert them into binary format.
-    if (name == "hw-address") {
-        HWAddr hwaddr(HWAddr::fromText(identifier));
-        identifier_type_= IDENT_HWADDR;
-        identifier_value_ = hwaddr.hwaddr_;
-
-    } else if (name == "duid") {
-        identifier_type_ = IDENT_DUID;
-        DUID duid(DUID::fromText(identifier));
-        identifier_value_ = duid.getDuid();
-
-    } else {
-        if (name == "circuit-id") {
-            identifier_type_ = IDENT_CIRCUIT_ID;
+    // Empty identifier is not allowed.
+    if (identifier.empty()) {
+        isc_throw(isc::BadValue, "empty host identifier used");
+    }
 
-        } else {
-            isc_throw(isc::BadValue, "invalid client identifier type '"
-                      << name << "' when creating host instance");
-        }
+    // Set identifier type.
+    identifier_type_ = getIdentifierType(name);
 
-        // Here we're converting values other than DUID and HW address. These
-        // values can either be specified as strings of hexadecimal digits or
-        // strings in quotes. The latter are copied to a vector excluding quote
-        // characters.
+    // Idetifier value can either be specified as string of hexadecimal
+    // digits or a string in quotes. The latter is copied to a vector excluding
+    // quote characters.
 
-        // Try to convert the values in quotes into a vector of ASCII codes.
-        // If the identifier lacks opening and closing quote, this will return
-        // an empty value, in which case we'll try to decode it as a string of
-        // hexadecimal digits.
+    // Try to convert the values in quotes into a vector of ASCII codes.
+    // If the identifier lacks opening and closing quote, this will return
+    // an empty value, in which case we'll try to decode it as a string of
+    // hexadecimal digits.
+    try {
         std::vector<uint8_t> binary = util::str::quotedStringToBinary(identifier);
         if (binary.empty()) {
-            try {
-                util::encode::decodeHex(identifier, binary);
-
-            } catch (...) {
-                // The string doesn't match any known pattern, so we have to
-                // report an error at this point.
-                isc_throw(isc::BadValue, "invalid host identifier value '"
-                          << identifier << "'");
-            }
+            util::str::decodeFormattedHexString(identifier, binary);
         }
-
         // Successfully decoded the identifier, so let's use it.
         identifier_value_.swap(binary);
+
+    } catch (...) {
+        // The string doesn't match any known pattern, so we have to
+        // report an error at this point.
+        isc_throw(isc::BadValue, "invalid host identifier value '"
+                      << identifier << "'");
     }
 }
 

src/lib/dhcpsrv/host.h

@@ -221,17 +221,18 @@ public:
     ///
     /// Creates @c Host object using an identifier in a textual format. This
     /// is useful in cases when the reservation is specified in the server
-    /// configuration file, where:
-    /// - MAC address is specified as: "01:02:03:04:05:06"
-    /// - DUID can be specified as: "01:02:03:04:05:06:ab:cd" or "010203040506abcd".
-    /// - Other identifiers are specified as: "010203040506abcd" or as
-    /// "'some identfier'".
-    ///
-    /// In case of identifiers other than HW address and DUID it is possible to use
-    /// textual representation, e.g. 'some identifier', which is converted to a
-    /// vector of ASCII codes representing characters in a given string, excluding
-    /// quotes. This is useful in cases when specific identifiers, e.g. circuit-id
-    /// are manually assigned user friendly values.
+    /// configuration file. Identifiers can be specified in the following
+    /// formats:
+    /// - "yy:yy:yy:yy:yy:yy"
+    /// - "yyyyyyyyyy",
+    /// - "0xyyyyyyyyyy",
+    /// - "'some identfier'".
+    /// where y is a hexadecimal digit.
+    ///
+    /// Note that it is possible to use textual representation, e.g. 'some identifier',
+    /// which is converted to a vector of ASCII codes representing characters in a
+    /// given string, excluding quotes. This is useful in cases when specific
+    /// identifiers, e.g. circuit-id are manually assigned user friendly values.
     ///
     /// @param identifier Identifier in the textual format. The expected formats
     /// for the hardware address and other identifiers are provided above.
@@ -304,6 +305,12 @@ public:
     ///
     IdentifierType getIdentifierType() const;
 
+    /// @brief Converts identifier name to identifier type.
+    ///
+    /// @param identifier_name Identifier name.
+    /// @return Identifier type.
+    static IdentifierType getIdentifierType(const std::string& identifier_name);
+
     /// @brief Returns host identifier in a textual form.
     ///
     /// @return Identifier in the form of <type>=<value>.

src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc

@@ -29,6 +29,11 @@ using namespace isc::dhcp;
 
 namespace {
 
+/// @brief Holds a type of the last identifier in @c IdentifierType enum.
+///
+/// This value must be updated when new identifiers are added to the enum.
+const Host::IdentifierType LAST_IDENTIFIER_TYPE = Host::IDENT_CIRCUIT_ID;
+
 /// @brief Test fixture class for @c HostReservationParser.
 class HostReservationParserTest : public ::testing::Test {
 protected:
@@ -93,10 +98,6 @@ protected:
         return (OptionPtr());
     }
 
-    void
-    expectFailure(const HostReservationParser& parser,
-                  const std::string& config) const;
-
     /// @brief This test verifies that it is possible to specify an empty list
     /// of options for a host.
     ///
@@ -208,49 +209,23 @@ HostReservationParserTest::TearDown() {
 // This test verfies that the parser can parse the reservation entry for
 // which hw-address is a host identifier.
 TEST_F(HostReservationParserTest, dhcp4HWaddr) {
-    std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\","
-        "\"ip-address\": \"192.0.2.134\","
-        "\"hostname\": \"foo.example.com\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(1));
-    ASSERT_NO_THROW(parser.build(config_element));
-
-    CfgHostsPtr cfg_hosts = CfgMgr::instance().getStagingCfg()->getCfgHosts();
-    HostCollection hosts;
-    ASSERT_NO_THROW(hosts = cfg_hosts->getAll(hwaddr_, DuidPtr()));
-
-    ASSERT_EQ(1, hosts.size());
-
-    EXPECT_EQ(1, hosts[0]->getIPv4SubnetID());
-    EXPECT_EQ(0, hosts[0]->getIPv6SubnetID());
-    EXPECT_EQ("192.0.2.134", hosts[0]->getIPv4Reservation().toText());
-    EXPECT_EQ("foo.example.com", hosts[0]->getHostname());
+    testIdentifier4("hw-address", "1:2:3:4:5:6", Host::IDENT_HWADDR,
+                    hwaddr_->hwaddr_);
 }
 
-// This test verfies that the parser can parse the reservation entry for
+// This test verifies that the parser can parse the reservation entry for
 // which DUID is a host identifier.
 TEST_F(HostReservationParserTest, dhcp4DUID) {
-    std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
-        "\"ip-address\": \"192.0.2.112\","
-        "\"hostname\": \"\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(10));
-    ASSERT_NO_THROW(parser.build(config_element));
-
-    CfgHostsPtr cfg_hosts = CfgMgr::instance().getStagingCfg()->getCfgHosts();
-    HostCollection hosts;
-    ASSERT_NO_THROW(hosts = cfg_hosts->getAll(HWAddrPtr(), duid_));
-
-    ASSERT_EQ(1, hosts.size());
+    testIdentifier4("duid", "01:02:03:04:05:06:07:08:09:0A",
+                    Host::IDENT_DUID, duid_->getDuid());
+}
 
-    EXPECT_EQ(10, hosts[0]->getIPv4SubnetID());
-    EXPECT_EQ(0, hosts[0]->getIPv6SubnetID());
-    EXPECT_EQ("192.0.2.112", hosts[0]->getIPv4Reservation().toText());
-    EXPECT_TRUE(hosts[0]->getHostname().empty());
+// This test verifies that the parser can parse the reservation entry for
+// which DUID specified as a string of hexadecimal digits with '0x' prefix
+// is a host identifier
+TEST_F(HostReservationParserTest, dhcp4DUIDWithPrefix) {
+    testIdentifier4("duid", "0x0102030405060708090A",
+                    Host::IDENT_DUID, duid_->getDuid());
 }
 
 // This test verifies that the parser can parse a reservation entry for
@@ -269,6 +244,14 @@ TEST_F(HostReservationParserTest, dhcp4CircuitIdHex) {
                     circuit_id_);
 }
 
+// This test verifies that the parser can parse a reservation entry for
+// which circuit-id is an identifier. The circuit-id is specified in
+// hexadecimal format with a '0x' prefix.
+TEST_F(HostReservationParserTest, dhcp4CircuitIdHexWithPrefix) {
+    testIdentifier4("circuit-id", "0x686F776479", Host::IDENT_CIRCUIT_ID,
+                    circuit_id_);
+}
+
 // This test verifies that the parser can parse the reservation entry
 // when IPv4 address is specified, but hostname is not.
 TEST_F(HostReservationParserTest, dhcp4NoHostname) {
@@ -299,11 +282,7 @@ TEST_F(HostReservationParserTest, dhcp4NoHostname) {
 TEST_F(HostReservationParserTest, dhcp4IPv6Address) {
     std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\","
         "\"ip-address\": \"2001:db8:1::1\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(10));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser4>(config);
 }
 
 // This test verifies that the configuration parser for host reservations
@@ -311,22 +290,14 @@ TEST_F(HostReservationParserTest, dhcp4IPv6Address) {
 TEST_F(HostReservationParserTest, noIdentifier) {
     std::string config = "{ \"ip-address\": \"192.0.2.112\","
         "\"hostname\": \"\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(10));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser4>(config);
 }
 
 // This test verifies  that the configuration parser for host reservations
 // throws an exception when neither ip address nor hostname is specified.
 TEST_F(HostReservationParserTest, noResource) {
     std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(10));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser4>(config);
 }
 
 // This test verifies that the parser can parse the reservation entry
@@ -358,11 +329,7 @@ TEST_F(HostReservationParserTest, noIPAddress) {
 TEST_F(HostReservationParserTest, emptyHostname) {
     std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\","
         "\"hostname\": \"\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(10));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser4>(config);
 }
 
 // This test verifies that the configuration parser for host reservations
@@ -370,11 +337,7 @@ TEST_F(HostReservationParserTest, emptyHostname) {
 TEST_F(HostReservationParserTest, malformedAddress) {
     std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\","
         "\"ip-address\": \"192.0.2.bogus\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(10));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser4>(config);
 }
 
 // This test verifies that the configuration parser for host reservations
@@ -382,11 +345,7 @@ TEST_F(HostReservationParserTest, malformedAddress) {
 TEST_F(HostReservationParserTest, zeroAddress) {
     std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\","
         "\"ip-address\": \"0.0.0.0\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(10));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser4>(config);
 }
 
 // This test verifies that the configuration parser for host reservations
@@ -394,11 +353,7 @@ TEST_F(HostReservationParserTest, zeroAddress) {
 TEST_F(HostReservationParserTest, bcastAddress) {
     std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\","
         "\"ip-address\": \"255.255.255.255\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(10));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser4>(config);
 }
 
 // This test verifies that the configuration parser for host reservations
@@ -410,11 +365,7 @@ TEST_F(HostReservationParserTest, invalidParameterName) {
     std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\","
         "\"hostname\": \"foo.bar.isc.org\","
         "\"ip-addresses\": \"2001:db8:1::1\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser4 parser(SubnetID(10));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser4>(config);
 }
 
 // This test verfies that the parser can parse the IPv6 reservation entry for
@@ -511,12 +462,7 @@ TEST_F(HostReservationParserTest, dhcp6CircuitId) {
         "\"ip-addresses\": [ \"2001:db8:1::100\", \"2001:db8:1::200\" ],"
         "\"prefixes\": [ ],"
         "\"hostname\": \"foo.example.com\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    // The parser should throw exception.
-    HostReservationParser6 parser(SubnetID(12));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser6>(config);
 }
 
 // This test verfies that the parser can parse the IPv6 reservation entry
@@ -563,11 +509,7 @@ TEST_F(HostReservationParserTest, dhcp6IPv4Address) {
     std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
         "\"ip-addresses\": [ \"192.0.2.3\", \"2001:db8:1::200\" ],"
         "\"prefixes\": [ ] }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser6 parser(SubnetID(12));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser6>(config);
 }
 
 // This test verifies that the configuration parser throws an exception
@@ -576,11 +518,7 @@ TEST_F(HostReservationParserTest, dhcp6NullAddress) {
     std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
         "\"ip-addresses\": [ \"\" ],"
         "\"prefixes\": [ ] }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser6 parser(SubnetID(12));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser6>(config);
 }
 
 // This test verifies that the configuration parser throws an exception
@@ -588,11 +526,7 @@ TEST_F(HostReservationParserTest, dhcp6NullAddress) {
 TEST_F(HostReservationParserTest, dhcp6InvalidPrefixLength) {
     std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
         "\"prefixes\": [ \"2001:db8:1::/abc\" ] }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser6 parser(SubnetID(12));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser6>(config);
 }
 
 // This test verifies that the configuration parser throws an exception
@@ -600,11 +534,7 @@ TEST_F(HostReservationParserTest, dhcp6InvalidPrefixLength) {
 TEST_F(HostReservationParserTest, dhcp6NullPrefix) {
     std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
         "\"prefixes\": [ \"/64\" ] }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser6 parser(SubnetID(12));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser6>(config);
 }
 
 // This test verifies that the configuration parser throws an exception
@@ -612,11 +542,7 @@ TEST_F(HostReservationParserTest, dhcp6NullPrefix) {
 TEST_F(HostReservationParserTest, dhcp6NullPrefix2) {
     std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
         "\"prefixes\": [ \"/\" ] }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser6 parser(SubnetID(12));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser6>(config);
 }
 
 // This test verifies that the configuration parser throws an exception
@@ -624,11 +550,7 @@ TEST_F(HostReservationParserTest, dhcp6NullPrefix2) {
 TEST_F(HostReservationParserTest, dhcp6DuplicatedAddress) {
     std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
         "\"ip-addresses\": [ \"2001:db8:1::1\", \"2001:db8:1::1\" ] }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser6 parser(SubnetID(12));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser6>(config);
 }
 
 // This test verifies that the configuration parser throws an exception
@@ -636,14 +558,9 @@ TEST_F(HostReservationParserTest, dhcp6DuplicatedAddress) {
 TEST_F(HostReservationParserTest, dhcp6DuplicatedPrefix) {
     std::string config = "{ \"duid\": \"01:02:03:04:05:06:07:08:09:0A\","
         "\"prefixes\": [ \"2001:db8:0101::/64\", \"2001:db8:0101::/64\" ] }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser6 parser(SubnetID(12));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser6>(config);
 }
 
-
 // This test verifies that the configuration parser for host reservations
 // throws an exception when unsupported parameter is specified.
 TEST_F(HostReservationParserTest, dhcp6invalidParameterName) {
@@ -653,11 +570,7 @@ TEST_F(HostReservationParserTest, dhcp6invalidParameterName) {
     std::string config = "{ \"hw-address\": \"01:02:03:04:05:06\","
         "\"hostname\": \"foo.bar.isc.org\","
         "\"ip-address\": \"192.0.2.3\" }";
-
-    ElementPtr config_element = Element::fromJSON(config);
-
-    HostReservationParser6 parser(SubnetID(10));
-    EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    testInvalidConfig<HostReservationParser6>(config);
 }
 
 // This test verifies that it is possible to specify DHCPv4 options for
@@ -821,5 +734,54 @@ TEST_F(HostReservationParserTest, options6InvalidOptionSpace) {
     testInvalidConfig<HostReservationParser6>(config);
 }
 
+// This test verifies that host identifiers for DHCPv4 are mutually exclusive.
+TEST_F(HostReservationParserTest, mutuallyExclusiveIdentifiers4) {
+    std::vector<std::string> identifiers;
+    identifiers.push_back("hw-address");
+    identifiers.push_back("duid");
+    identifiers.push_back("circuit-id");
+
+    for (unsigned int i = 0; i < identifiers.size(); ++i) {
+        // j points to an index of the next identifier. If it
+        // overflows, we set it to 0.
+        unsigned int j = (i + 1) % (identifiers.size());
+        Host::IdentifierType first = static_cast<Host::IdentifierType>(i);
+        Host::IdentifierType second = static_cast<Host::IdentifierType>(j);
+
+        SCOPED_TRACE("Using identifiers " + Host::getIdentifierName(first)
+                     + " and " + Host::getIdentifierName(second));
+
+        // Create configuration with two different identifiers.
+        std::ostringstream config;
+        config << "{ \"" << Host::getIdentifierName(first) << "\": \"121314151617\","
+            "\"" << Host::getIdentifierName(second) << "\": \"0A0B0C0D0E0F\","
+            "\"ip-address\": \"192.0.2.3\" }";
+        testInvalidConfig<HostReservationParser4>(config.str());
+    }
+}
+
+// This test verifies that host identifiers for DHCPv6 are mutually exclusive.
+TEST_F(HostReservationParserTest, mutuallyExclusiveIdentifiers6) {
+    std::vector<std::string> identifiers;
+    identifiers.push_back("hw-address");
+    identifiers.push_back("duid");
+
+    for (unsigned int i = 0; i < identifiers.size(); ++i) {
+        // j points to an index of the next identifier. If it
+        // overflows, we set it to 0.
+        unsigned int j = (i + 1) % (identifiers.size());
+
+        SCOPED_TRACE("Using identifiers " + identifiers[i] + " and "
+                     + identifiers[j]);
+
+        // Create configuration with two different identifiers.
+        std::ostringstream config;
+        config << "{ \"" << identifiers[i] << "\": \"121314151617\","
+            "\"" << identifiers[j] << "\": \"0A0B0C0D0E0F\","
+            "\"ip-addresses\": \"2001:db8:1::1\" }";
+        testInvalidConfig<HostReservationParser6>(config.str());
+    }
+}
+
 
 } // end of anonymous namespace

src/lib/dhcpsrv/tests/host_reservations_list_parser_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -16,6 +16,9 @@
 #include <dhcpsrv/parsers/host_reservation_parser.h>
 #include <dhcpsrv/parsers/host_reservations_list_parser.h>
 #include <gtest/gtest.h>
+#include <sstream>
+#include <string>
+#include <vector>
 
 using namespace isc::data;
 using namespace isc::dhcp;
@@ -107,6 +110,39 @@ TEST_F(HostReservationsListParserTest, ipv4Reservations) {
     EXPECT_EQ("bar.example.com", hosts[0]->getHostname());
 }
 
+// This test verifies that an attempt to add two reservations with the
+// same identifier value will return an error.
+TEST_F(HostReservationsListParserTest, duplicatedIdentifierValue4) {
+    std::vector<std::string> identifiers;
+    identifiers.push_back("hw-address");
+    identifiers.push_back("duid");
+    identifiers.push_back("circuit-id");
+
+    for (unsigned int i = 0; i < identifiers.size(); ++i) {
+        SCOPED_TRACE("Using identifier " + identifiers[i]);
+
+        std::ostringstream config;
+        config <<
+            "[ "
+            "  { "
+            "   \"" << identifiers[i] << "\": \"010203040506\","
+            "   \"ip-address\": \"192.0.2.134\","
+            "   \"hostname\": \"foo.example.com\""
+            "  }, "
+            "  { "
+            "   \"" << identifiers[i] << "\": \"010203040506\","
+            "   \"ip-address\": \"192.0.2.110\","
+            "   \"hostname\": \"bar.example.com\""
+            "  } "
+            "]";
+
+        ElementPtr config_element = Element::fromJSON(config.str());
+
+        HostReservationsListParser<HostReservationParser4> parser(SubnetID(1));
+        EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    }
+}
+
 // This test verifies that the parser for the list of the host reservations
 // parses IPv6 reservations correctly.
 TEST_F(HostReservationsListParserTest, ipv6Reservations) {
@@ -167,4 +203,38 @@ TEST_F(HostReservationsListParserTest, ipv6Reservations) {
     EXPECT_EQ(80, prefixes.first->second.getPrefixLen());
 }
 
+// This test verifies that an attempt to add two reservations with the
+// same identifier value will return an error.
+TEST_F(HostReservationsListParserTest, duplicatedIdentifierValue6) {
+    std::vector<std::string> identifiers;
+    identifiers.push_back("hw-address");
+    identifiers.push_back("duid");
+
+    for (unsigned int i = 0; i < identifiers.size(); ++i) {
+        SCOPED_TRACE("Using identifier " + identifiers[i]);
+
+        std::ostringstream config;
+        config <<
+            "[ "
+            "  { "
+            "   \"" << identifiers[i] << "\": \"010203040506\","
+            "   \"ip-addresses\": [ \"2001:db8:1::123\" ],"
+            "   \"hostname\": \"foo.example.com\""
+            "  }, "
+            "  { "
+            "   \"" << identifiers[i] << "\": \"010203040506\","
+            "   \"ip-addresses\": [ \"2001:db8:1::123\" ],"
+            "   \"hostname\": \"bar.example.com\""
+            "  } "
+            "]";
+
+        ElementPtr config_element = Element::fromJSON(config.str());
+
+        HostReservationsListParser<HostReservationParser6> parser(SubnetID(1));
+        EXPECT_THROW(parser.build(config_element), DhcpConfigError);
+    }
+}
+
+
+
 } // end of anonymous namespace

src/lib/dhcpsrv/tests/host_unittest.cc

@@ -173,6 +173,17 @@ public:
     }
 };
 
+// This test verifies that correct identifier name is returned for
+// a given identifier name and that an error is reported for an
+// unsupported identifier name.
+TEST_F(HostTest, getIdentifier) {
+    EXPECT_EQ(Host::IDENT_HWADDR, Host::getIdentifierType("hw-address"));
+    EXPECT_EQ(Host::IDENT_DUID, Host::getIdentifierType("duid"));
+    EXPECT_EQ(Host::IDENT_CIRCUIT_ID, Host::getIdentifierType("circuit-id"));
+
+    EXPECT_THROW(Host::getIdentifierType("unuspported"), isc::BadValue);
+}
+
 // This test verfies that it is possible to create a Host object
 // using hardware address in the textual format.
 TEST_F(HostTest, createFromHWAddrString) {
@@ -200,7 +211,7 @@ TEST_F(HostTest, createFromHWAddrString) {
                  isc::BadValue);
 
     // Use invalid HW address.
-    EXPECT_THROW(Host("010203040506", "hw-address", SubnetID(1), SubnetID(2),
+    EXPECT_THROW(Host("01:0203040506", "hw-address", SubnetID(1), SubnetID(2),
                       IOAddress("192.0.2.3"), "somehost.example.org"),
                  isc::BadValue);
 }