[master] Merge branch 'trac4301'

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -3454,7 +3454,7 @@ TEST_F(Dhcp4ParserTest, reservations) {
         "        ]"
         "      },"
         "      {"
-        "        \"hw-address\": \"06:05:04:03:02:01\","
+        "        \"circuit-id\": \"060504030201\","
         "        \"ip-address\": \"192.0.4.102\","
         "        \"hostname\": \"\""
         "      }"
@@ -3528,16 +3528,19 @@ TEST_F(Dhcp4ParserTest, reservations) {
     ASSERT_TRUE(opt_ttl);
     EXPECT_EQ(32, static_cast<int>(opt_ttl->getValue()));
 
-    // The HW address used for one of the reservations in the subnet 542
+    // The circuit-id used for one of the reservations in the subnet 542
     // consists of numbers from 6 to 1. So, let's just reverse the order
     // of the address from the previous test.
-    hwaddr->hwaddr_.assign(hwaddr_vec.rbegin(), hwaddr_vec.rend());
-    host = hosts_cfg->get4(542, hwaddr);
+    std::vector<uint8_t> circuit_id(hwaddr_vec.rbegin(), hwaddr_vec.rend());
+    host = hosts_cfg->get4(542, Host::IDENT_CIRCUIT_ID, &circuit_id[0],
+                           circuit_id.size());
     EXPECT_TRUE(host);
     EXPECT_EQ("192.0.4.102", host->getIPv4Reservation().toText());
     // This reservation must not belong to other subnets.
-    EXPECT_FALSE(hosts_cfg->get4(123, hwaddr));
-    EXPECT_FALSE(hosts_cfg->get4(234, hwaddr));
+    EXPECT_FALSE(hosts_cfg->get4(123, Host::IDENT_CIRCUIT_ID,
+                                 &circuit_id[0], circuit_id.size()));
+    EXPECT_FALSE(hosts_cfg->get4(234, Host::IDENT_CIRCUIT_ID,
+                                 &circuit_id[0], circuit_id.size()));
 
     // Repeat the test for the DUID based reservation in this subnet.
     duid.reset(new DUID(std::vector<uint8_t>(duid_vec.rbegin(),

src/lib/dhcp/duid.cc

@@ -6,11 +6,8 @@
 
 #include <dhcp/duid.h>
 #include <exceptions/exceptions.h>
-#include <util/encode/hex.h>
 #include <util/io_utilities.h>
-#include <boost/algorithm/string/classification.hpp>
-#include <boost/algorithm/string/constants.hpp>
-#include <boost/algorithm/string/split.hpp>
+#include <util/strutil.h>
 #include <iomanip>
 #include <cctype>
 #include <sstream>
@@ -42,57 +39,6 @@ DUID::DUID(const uint8_t* data, size_t len) {
     duid_ = std::vector<uint8_t>(data, data + len);
 }
 
-std::vector<uint8_t>
-DUID::decode(const std::string& text) {
-    /// @todo optimize stream operations here.
-    std::vector<std::string> split_text;
-    boost::split(split_text, text, boost::is_any_of(":"),
-                 boost::algorithm::token_compress_off);
-
-    std::ostringstream s;
-    for (size_t i = 0; i < split_text.size(); ++i) {
-        // Check that only hexadecimal digits are used.
-        size_t ch_index = 0;
-        while (ch_index < split_text[i].length()) {
-            if (!isxdigit(split_text[i][ch_index])) {
-                isc_throw(isc::BadValue, "invalid value '"
-                          << split_text[i][ch_index] << "' in"
-                          << " DUID '" << text << "'");
-            }
-            ++ch_index;
-        }
-
-        if (split_text.size() > 1) {
-            // If there are multiple tokens and the current one is empty, it
-            // means that two consecutive colons were specified. This is not
-            // allowed for client identifier.
-            if (split_text[i].empty()) {
-                isc_throw(isc::BadValue, "invalid identifier '"
-                          << text << "': tokens must be"
-                          " separated with a single colon");
-            } else if (split_text[i].size() > 2) {
-                isc_throw(isc::BadValue, "invalid identifier '"
-                          << text << "'");
-            }
-        }
-
-        if (split_text[i].size() % 2) {
-                s << "0";
-        }
-
-        s << split_text[i];
-    }
-
-    std::vector<uint8_t> binary;
-    try {
-        util::encode::decodeHex(s.str(), binary);
-    } catch (const Exception& ex) {
-        isc_throw(isc::BadValue, "failed to create identifier from text '"
-                  << text << "': " << ex.what());
-    }
-    return (binary);
-}
-
 const std::vector<uint8_t>& DUID::getDuid() const {
     return (duid_);
 }
@@ -111,8 +57,9 @@ DUID::DUIDType DUID::getType() const {
 
 DUID
 DUID::fromText(const std::string& text) {
-    std::vector<uint8_t> binary = decode(text);
-    return DUID(binary);
+    std::vector<uint8_t> binary;
+    util::str::decodeFormattedHexString(text, binary);
+    return (DUID(binary));
 }
 
 DuidPtr
@@ -185,7 +132,8 @@ std::string ClientId::toText() const {
 
 ClientIdPtr
 ClientId::fromText(const std::string& text) {
-    std::vector<uint8_t> binary = decode(text);
+    std::vector<uint8_t> binary;
+    util::str::decodeFormattedHexString(text, binary);
     return (ClientIdPtr(new ClientId(binary)));
 }
 

src/lib/dhcp/duid.h

@@ -79,7 +79,6 @@ class DUID {
     /// @brief Create DUID from the textual format.
     ///
     /// This static function parses a DUID specified in the textual format.
-    /// Internally it uses @c DUID::decode to parse the DUID.
     ///
     /// @param text DUID in the hexadecimal format with digits representing
     /// individual bytes separated by colons.
@@ -98,22 +97,6 @@ class DUID {
 
  protected:
 
-    /// @brief Decodes the textual format of the DUID.
-    ///
-    /// The format being parsed should match the DUID representation returned
-    /// by the @c DUID::toText method, i.e. the pairs of hexadecimal digits
-    /// representing bytes of DUID must be separated by colons. Usually the
-    /// single byte is represented by two hexadecimal digits. However, this
-    /// function allows one digit per byte. In this case, a zero is prepended
-    /// before the conversion. For example, a DUID 0:1:2:3:4:5 equals to
-    /// 00:01:02:03:04:05.
-    ///
-    /// @param text DUID in the hexadecimal format with digits representing
-    /// individual bytes separated by colons.
-    ///
-    /// @throw isc::BadValue if parsing the DUID failed.
-    static std::vector<uint8_t> decode(const std::string& text);
-
     /// The actual content of the DUID
     std::vector<uint8_t> duid_;
 };

src/lib/dhcp/hwaddr.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-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
@@ -7,10 +7,7 @@
 #include <dhcp/hwaddr.h>
 #include <dhcp/dhcp4.h>
 #include <exceptions/exceptions.h>
-#include <util/encode/hex.h>
-#include <boost/algorithm/string/classification.hpp>
-#include <boost/algorithm/string/constants.hpp>
-#include <boost/algorithm/string/split.hpp>
+#include <util/strutil.h>
 #include <iomanip>
 #include <sstream>
 #include <vector>
@@ -69,37 +66,8 @@ std::string HWAddr::toText(bool include_htype) const {
 
 HWAddr
 HWAddr::fromText(const std::string& text, const uint16_t htype) {
-    /// @todo optimize stream operations here.
-    std::vector<std::string> split_text;
-    boost::split(split_text, text, boost::is_any_of(":"),
-                 boost::algorithm::token_compress_off);
-
-    std::ostringstream s;
-    for (size_t i = 0; i < split_text.size(); ++i) {
-        // If there are multiple tokens and the current one is empty, it
-        // means that two consecutive colons were specified. This is not
-        // allowed for hardware address.
-        if ((split_text.size() > 1) && split_text[i].empty()) {
-            isc_throw(isc::BadValue, "failed to create hardware address"
-                      " from text '" << text << "': tokens of the hardware"
-                      " address must be separated with a single colon");
-
-        } else  if (split_text[i].size() == 1) {
-            s << "0";
-
-        } else if (split_text[i].size() > 2) {
-            isc_throw(isc::BadValue, "invalid hwaddr '" << text << "'");
-        }
-        s << split_text[i];
-    }
-
     std::vector<uint8_t> binary;
-    try {
-        util::encode::decodeHex(s.str(), binary);
-    } catch (const Exception& ex) {
-        isc_throw(isc::BadValue, "failed to create hwaddr from text '"
-                  << text << "': " << ex.what());
-    }
+    util::str::decodeColonSeparatedHexString(text, binary);
     return (HWAddr(binary, htype));
 }
 

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/parsers/host_reservation_parser.cc

@@ -12,6 +12,7 @@
 #include <boost/foreach.hpp>
 #include <boost/lexical_cast.hpp>
 #include <sys/socket.h>
+#include <sstream>
 #include <string>
 
 using namespace isc::asiolink;
@@ -23,36 +24,62 @@ namespace {
 ///
 /// This function returns the set of supported parameters for
 /// host reservation in DHCPv4.
-const std::set<std::string>& getSupportedParams4() {
+///
+/// @param identifiers_only Indicates if the function should only
+/// return supported host identifiers (if true) or all supported
+/// parameters (if false).
+const std::set<std::string>&
+getSupportedParams4(const bool identifiers_only = false) {
+    // Holds set of host identifiers.
+    static std::set<std::string> identifiers_set;
+    // Holds set of all supported parameters, including identifiers.
     static std::set<std::string> params_set;
+    // If this is first execution of this function, we need
+    // to initialize the set.
+    if (identifiers_set.empty()) {
+        identifiers_set.insert("duid");
+        identifiers_set.insert("hw-address");
+        identifiers_set.insert("circuit-id");
+    }
+    // Copy identifiers and add all other parameters.
     if (params_set.empty()) {
-        const char* params[] = {
-            "duid", "hw-address", "hostname", "ip-address",
-            "option-data", NULL
-        };
-        for (int i = 0; params[i] != NULL; ++i) {
-            params_set.insert(std::string(params[i]));
-        }
+        params_set = identifiers_set;
+        params_set.insert("hostname");
+        params_set.insert("ip-address");
+        params_set.insert("option-data");
     }
-    return (params_set);
+    return (identifiers_only ? identifiers_set : params_set);
 }
 
-/// @brief Returns set of the supported parameters for DHCPv4.
+/// @brief Returns set of the supported parameters for DHCPv6.
 ///
 /// This function returns the set of supported parameters for
 /// host reservation in DHCPv6.
-const std::set<std::string>& getSupportedParams6() {
+///
+/// @param identifiers_only Indicates if the function should only
+/// return supported host identifiers (if true) or all supported
+/// parameters (if false).
+const std::set<std::string>&
+getSupportedParams6(const bool identifiers_only = false) {
+    // Holds set of host identifiers.
+    static std::set<std::string> identifiers_set;
+    // Holds set of all supported parameters, including identifiers.
     static std::set<std::string> params_set;
+    // If this is first execution of this function, we need
+    // to initialize the set.
+    if (identifiers_set.empty()) {
+        identifiers_set.insert("duid");
+        identifiers_set.insert("hw-address");
+    }
+    // Copy identifiers and add all other parameters.
     if (params_set.empty()) {
-        const char* params[] = {
-            "duid", "hw-address", "hostname", "ip-addresses", "prefixes",
-            "option-data", NULL
-        };
-        for (int i = 0; params[i] != NULL; ++i) {
-            params_set.insert(std::string(params[i]));
-        }
+        params_set = identifiers_set;
+        params_set.insert("hostname");
+        params_set.insert("ip-addresses");
+        params_set.insert("prefixes");
+        params_set.insert("option-data");
     }
-    return (params_set);
+    return (identifiers_only ? identifiers_set : params_set);
 }
 
 }
@@ -80,10 +107,11 @@ HostReservationParser::build(isc::data::ConstElementPtr reservation_data) {
                           " parameter '" << element.first << "'");
             }
 
-            if (element.first == "hw-address" || element.first == "duid") {
-                if (!identifier_name.empty()) {
-                    isc_throw(DhcpConfigError, "the 'hw-address' and 'duid'"
-                              " parameters are mutually exclusive");
+            if (isIdentifierParameter(element.first)) {
+                if (!identifier.empty()) {
+                    isc_throw(DhcpConfigError, "the '" << element.first
+                              << "' and '" << identifier_name
+                              << "' are mutually exclusive");
                 }
                 identifier = element.second->stringValue();
                 identifier_name = element.first;
@@ -100,10 +128,22 @@ HostReservationParser::build(isc::data::ConstElementPtr reservation_data) {
     }
 
     try {
-        // hw-address or duid is a must.
+        // Host identifier is a must.
         if (identifier_name.empty()) {
-            isc_throw(DhcpConfigError, "'hw-address' or 'duid' is a required"
-                      " parameter for host reservation");
+            // If there is no identifier specified, we have to display an
+            // error message and include the information what identifiers
+            // are supported.
+            std::ostringstream s;
+            BOOST_FOREACH(std::string param_name, getSupportedParameters(true)) {
+                if (s.tellp() != std::streampos(0)) {
+                    s << ", ";
+                }
+                s << param_name;
+            }
+            isc_throw(DhcpConfigError, "one of the supported identifiers must"
+                      " be specified for host reservation: "
+                      << s.str());
+
         }
 
         // Create a host object from the basic parameters we already parsed.
@@ -129,6 +169,16 @@ HostReservationParser::addHost(isc::data::ConstElementPtr reservation_data) {
     }
 }
 
+bool
+HostReservationParser::isIdentifierParameter(const std::string& param_name) const {
+    return (getSupportedParameters(true).count(param_name) > 0);
+}
+
+bool
+HostReservationParser::isSupportedParameter(const std::string& param_name) const {
+    return (getSupportedParameters(false).count(param_name) > 0);
+}
+
 HostReservationParser4::HostReservationParser4(const SubnetID& subnet_id)
     : HostReservationParser(subnet_id) {
 }
@@ -167,9 +217,9 @@ HostReservationParser4::build(isc::data::ConstElementPtr reservation_data) {
     addHost(reservation_data);
 }
 
-bool
-HostReservationParser4::isSupportedParameter(const std::string& param_name) const {
-    return (getSupportedParams4().count(param_name) > 0);
+const std::set<std::string>&
+HostReservationParser4::getSupportedParameters(const bool identifiers_only) const {
+    return (getSupportedParams4(identifiers_only));
 }
 
 HostReservationParser6::HostReservationParser6(const SubnetID& subnet_id)
@@ -262,9 +312,9 @@ HostReservationParser6::build(isc::data::ConstElementPtr reservation_data) {
     addHost(reservation_data);
 }
 
-bool
-HostReservationParser6::isSupportedParameter(const std::string& param_name) const {
-    return (getSupportedParams6().count(param_name) > 0);
+const std::set<std::string>&
+HostReservationParser6::getSupportedParameters(const bool identifiers_only) const {
+    return (getSupportedParams6(identifiers_only));
 }
 
 } // end of namespace isc::dhcp

src/lib/dhcpsrv/parsers/host_reservation_parser.h

@@ -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
@@ -48,12 +48,30 @@ protected:
     /// @throw DhcpConfigError When operation to add a configured host fails.
     void addHost(isc::data::ConstElementPtr reservation_data);
 
+    /// @brief Checks if the specified parameter is a host identifier.
+    ///
+    /// @param param_name Parameter name.
+    ///
+    /// @return true if the parameter specifies host identifier, false
+    /// otherwise.
+    virtual bool isIdentifierParameter(const std::string& param_name) const;
+
     /// @brief Checks if the specified parameter is supported by the parser.
     ///
     /// @param param_name Parameter name.
     ///
     /// @return true if the parameter is supported, false otherwise.
-    virtual bool isSupportedParameter(const std::string& param_name) const = 0;
+    virtual bool isSupportedParameter(const std::string& param_name) const;
+
+    /// @brief Returns set of the supported parameters.
+    ///
+    /// @param identifiers_only Indicates if the function should only
+    /// return supported host identifiers (if true) or all supported
+    /// parameters (if false).
+    ///
+    /// @return Set of supported parameter names.
+    virtual const std::set<std::string>&
+    getSupportedParameters(const bool identifiers_only) const = 0;
 
     /// @brief Identifier of the subnet that the host is connected to.
     SubnetID subnet_id_;
@@ -84,12 +102,16 @@ public:
 
 protected:
 
-    /// @brief Checks if the specified parameter is supported by the parser.
+    /// @brief Returns set of the supported parameters for DHCPv4.
     ///
-    /// @param param_name Parameter name.
+    /// @param identifiers_only Indicates if the function should only
+    /// return supported host identifiers (if true) or all supported
+    /// parameters (if false).
     ///
-    /// @return true if the parameter is supported, false otherwise.
-    virtual bool isSupportedParameter(const std::string& param_name) const;
+    /// @return Set of supported parameter names.
+    virtual const std::set<std::string>&
+    getSupportedParameters(const bool identifiers_only) const;
+
 };
 
 /// @brief Parser for a single host reservation for DHCPv6.
@@ -112,12 +134,16 @@ public:
 
 protected:
 
-    /// @brief Checks if the specified parameter is supported by the parser.
+    /// @brief Returns set of the supported parameters for DHCPv6.
     ///
-    /// @param param_name Parameter name.
+    /// @param identifiers_only Indicates if the function should only
+    /// return supported host identifiers (if true) or all supported
+    /// parameters (if false).
     ///
-    /// @return true if the parameter is supported, false otherwise.
-    virtual bool isSupportedParameter(const std::string& param_name) const;
+    /// @return Set of supported parameter names.
+    virtual const std::set<std::string>&
+    getSupportedParameters(const bool identifiers_only) const;
+
 };
 
 

src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc

@@ -21,6 +21,7 @@
 #include <gtest/gtest.h>
 #include <iterator>
 #include <string>
+#include <vector>
 
 using namespace isc::asiolink;
 using namespace isc::data;
@@ -28,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:
@@ -92,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.
     ///
@@ -126,6 +128,40 @@ protected:
         EXPECT_TRUE(hosts[0]->getCfgOption6()->empty());
     }
 
+    /// @brief This test verfies that the parser can parse a DHCPv4
+    /// reservation configuration including a specific identifier.
+    ///
+    /// @param identifier_name Identifier name.
+    /// @param identifier_type Identifier type.
+    void testIdentifier4(const std::string& identifier_name,
+                         const std::string& identifier_value,
+                         const Host::IdentifierType& expected_identifier_type,
+                         const std::vector<uint8_t>& expected_identifier) const {
+        std::ostringstream config;
+        config << "{ \"" << identifier_name << "\": \"" << identifier_value
+               << "\","
+               << "\"ip-address\": \"192.0.2.112\","
+               << "\"hostname\": \"\" }";
+
+        ElementPtr config_element = Element::fromJSON(config.str());
+
+        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(expected_identifier_type,
+                                                  &expected_identifier[0],
+                                                  expected_identifier.size()));
+
+        ASSERT_EQ(1, hosts.size());
+
+        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());
+    }
+
     /// @brief This test verfies that the parser returns an error when
     /// configuration is invalid.
     ///
@@ -144,6 +180,8 @@ protected:
     /// @brief DUID object used by tests.
     DuidPtr duid_;
 
+    /// @brief Vector holding circuit id used by tests.
+    std::vector<uint8_t> circuit_id_;
 };
 
 void
@@ -158,6 +196,9 @@ HostReservationParserTest::SetUp() {
     const uint8_t duid_data[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
                                   0x08, 0x09, 0x0A };
     duid_ = DuidPtr(new DUID(duid_data, sizeof(duid_data)));
+
+    const std::string circuit_id_str = "howdy";
+    circuit_id_.assign(circuit_id_str.begin(), circuit_id_str.end());
 }
 
 void
@@ -168,49 +209,47 @@ 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);
+    testIdentifier4("duid", "01:02:03:04:05:06:07:08:09:0A",
+                    Host::IDENT_DUID, duid_->getDuid());
+}
 
-    HostReservationParser4 parser(SubnetID(10));
-    ASSERT_NO_THROW(parser.build(config_element));
+// 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());
+}
 
-    CfgHostsPtr cfg_hosts = CfgMgr::instance().getStagingCfg()->getCfgHosts();
-    HostCollection hosts;
-    ASSERT_NO_THROW(hosts = cfg_hosts->getAll(HWAddrPtr(), duid_));
+// This test verifies that the parser can parse a reservation entry for
+// which circuit-id is an identifier. The circuit-id is specified as
+// a string in quotes.
+TEST_F(HostReservationParserTest, dhcp4CircuitIdStringInQuotes) {
+    testIdentifier4("circuit-id", "'howdy'", Host::IDENT_CIRCUIT_ID,
+                    circuit_id_);
+}
 
-    ASSERT_EQ(1, hosts.size());
+// 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.
+TEST_F(HostReservationParserTest, dhcp4CircuitIdHex) {
+    testIdentifier4("circuit-id", "686F776479", Host::IDENT_CIRCUIT_ID,
+                    circuit_id_);
+}
 
-    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 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
@@ -243,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
@@ -255,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
@@ -302,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
@@ -314,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
@@ -326,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
@@ -338,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
@@ -354,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
@@ -447,6 +454,17 @@ TEST_F(HostReservationParserTest, dhcp6DUID) {
     ASSERT_EQ(0, std::distance(prefixes.first, prefixes.second));
 }
 
+// This test verifies that host reservation parser for DHCPv6 rejects
+// "circuit-id" as a host identifier.
+TEST_F(HostReservationParserTest, dhcp6CircuitId) {
+    // Use DHCPv4 specific identifier 'circuit-id' with DHCPv6 parser.
+    std::string config = "{ \"circuit-id\": \"'howdy'\","
+        "\"ip-addresses\": [ \"2001:db8:1::100\", \"2001:db8:1::200\" ],"
+        "\"prefixes\": [ ],"
+        "\"hostname\": \"foo.example.com\" }";
+    testInvalidConfig<HostReservationParser6>(config);
+}
+
 // This test verfies that the parser can parse the IPv6 reservation entry
 // which lacks hostname parameter.
 TEST_F(HostReservationParserTest, dhcp6NoHostname) {
@@ -491,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
@@ -504,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
@@ -516,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
@@ -528,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
@@ -540,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
@@ -552,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
@@ -564,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) {
@@ -581,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
@@ -749,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);
 }

src/lib/util/strutil.cc

@@ -4,10 +4,17 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-#include <numeric>
+#include <util/encode/hex.h>
+#include <util/strutil.h>
 
+#include <boost/algorithm/string/classification.hpp>
+#include <boost/algorithm/string/constants.hpp>
+#include <boost/algorithm/string/split.hpp>
+
+#include <numeric>
+#include <sstream>
 #include <string.h>
-#include <util/strutil.h>
+
 
 using namespace std;
 
@@ -152,6 +159,94 @@ quotedStringToBinary(const std::string& quoted_string) {
     return (binary);
 }
 
+void
+decodeColonSeparatedHexString(const std::string& hex_string,
+                              std::vector<uint8_t>& binary) {
+    std::vector<std::string> split_text;
+    boost::split(split_text, hex_string, boost::is_any_of(":"),
+                 boost::algorithm::token_compress_off);
+
+    std::vector<uint8_t> binary_vec;
+    for (size_t i = 0; i < split_text.size(); ++i) {
+
+        // If there are multiple tokens and the current one is empty, it
+        // means that two consecutive colons were specified. This is not
+        // allowed.
+        if ((split_text.size() > 1) && split_text[i].empty()) {
+            isc_throw(isc::BadValue, "two consecutive colons specified in"
+                      " a decoded string '" << hex_string << "'");
+
+        // Between a colon we expect at most two characters.
+        } else if (split_text[i].size() > 2) {
+            isc_throw(isc::BadValue, "invalid format of the decoded string"
+                      << " '" << hex_string << "'");
+
+        } else if (!split_text[i].empty()) {
+            std::stringstream s;
+            s << "0x";
+
+            for (unsigned int j = 0; j < split_text[i].length(); ++j) {
+                // Check if we're dealing with hexadecimal digit.
+                if (!isxdigit(split_text[i][j])) {
+                    isc_throw(isc::BadValue, "'" << split_text[i][j]
+                              << "' is not a valid hexadecimal digit in"
+                              << " decoded string '" << hex_string << "'");
+                }
+                s << split_text[i][j];
+            }
+
+            // The stream should now have one or two hexadecimal digits.
+            // Let's convert it to a number and store in a temporary
+            // vector.
+            unsigned int binary_value;
+            s >> std::hex >> binary_value;
+
+            binary_vec.push_back(static_cast<uint8_t>(binary_value));
+        }
+
+    }
+
+    // All ok, replace the data in the output vector with a result.
+    binary.swap(binary_vec);
+}
+
+void
+decodeFormattedHexString(const std::string& hex_string,
+                         std::vector<uint8_t>& binary) {
+    // If there is at least one colon we assume that the string
+    // comprises octets separated by colons (e.g. MAC address notation).
+    if (hex_string.find(':') != std::string::npos) {
+        decodeColonSeparatedHexString(hex_string, binary);
+
+    } else {
+        std::ostringstream s;
+
+        // If we have odd number of digits we'll have to prepend '0'.
+        if (hex_string.length() % 2 != 0) {
+            s << "0";
+        }
+
+        // It is ok to use '0x' prefix in a string.
+        if ((hex_string.length() > 2) && (hex_string.substr(0, 2) == "0x")) {
+            // Exclude '0x' from the decoded string.
+            s << hex_string.substr(2);
+
+        } else {
+            // No '0x', so decode the whole string.
+            s << hex_string;
+        }
+
+        try {
+            // Decode the hex string.
+            encode::decodeHex(s.str(), binary);
+
+        } catch (...) {
+            isc_throw(isc::BadValue, "'" << hex_string << "' is not a valid"
+                      " string of hexadecimal digits");
+        }
+    }
+}
+
 } // namespace str
 } // namespace util
 } // namespace isc

src/lib/util/strutil.h

@@ -214,6 +214,44 @@ tokenToNum(const std::string& num_token) {
 std::vector<uint8_t>
 quotedStringToBinary(const std::string& quoted_string);
 
+/// \brief Converts a string of hexadecimal digits with colons into
+///  a vector.
+///
+/// This function supports the following formats:
+/// - yy:yy:yy:yy:yy
+/// - y:y:y:y:y
+/// - y:yy:yy:y:y
+///
+/// If the decoded string doesn't match any of the supported formats,
+/// an exception is thrown.
+///
+/// \param hex_string Input string.
+/// \param binary Vector receiving converted string into binary.
+/// \throw isc::BadValue if the format of the input string is invalid.
+void
+decodeColonSeparatedHexString(const std::string& hex_string,
+                              std::vector<uint8_t>& binary);
+
+/// \brief Converts a formatted string of hexadecimal digits into
+/// a vector.
+///
+/// This function supports formats supported by
+/// @ref decodeColonSeparatedHexString and the following additional
+/// formats:
+/// - yyyyyyyyyy
+/// - 0xyyyyyyyyyy
+///
+/// If there is an odd number of hexadecimal digits in the input
+/// string, the '0' is prepended to the string before decoding.
+///
+/// \param hex_string Input string.
+/// \param binary Vector receiving converted string into binary.
+/// \throw isc::BadValue if the format of the input string is invalid.
+void
+decodeFormattedHexString(const std::string& hex_string,
+                         std::vector<uint8_t>& binary);
+
+
 } // namespace str
 } // namespace util
 } // namespace isc

src/lib/util/tests/strutil_unittest.cc

@@ -4,18 +4,22 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-#include <stdint.h>
-
-#include <string>
+#include <exceptions/exceptions.h>
+#include <util/strutil.h>
+#include <util/encode/hex.h>
 
 #include <gtest/gtest.h>
 
-#include <util/strutil.h>
+#include <stdint.h>
+#include <string>
 
 using namespace isc;
 using namespace isc::util;
+using namespace isc::util::str;
 using namespace std;
 
+namespace {
+
 // Check for slash replacement
 
 TEST(StringUtilTest, Slash) {
@@ -297,3 +301,128 @@ TEST(StringUtilTest, quotedStringToBinary) {
     EXPECT_EQ("'", testQuoted("'''"));
     EXPECT_EQ("''", testQuoted("''''"));
 }
+
+/// @brief Test that hex string with colons can be decoded.
+///
+/// @param input Input string to be decoded.
+/// @param reference A string without colons representing the
+/// decoded data.
+void testColonSeparated(const std::string& input,
+                        const std::string& reference) {
+    // Create a reference vector.
+    std::vector<uint8_t> reference_vector;
+    ASSERT_NO_THROW(encode::decodeHex(reference, reference_vector));
+
+    // Fill the output vector with some garbage to make sure that
+    // the data is erased when a string is decoded successfully.
+    std::vector<uint8_t> decoded(1, 10);
+    ASSERT_NO_THROW(decodeColonSeparatedHexString(input, decoded));
+
+    // Get the string representation of the decoded data for logging
+    // purposes.
+    std::string encoded;
+    ASSERT_NO_THROW(encoded = encode::encodeHex(decoded));
+
+    // Check if the decoded data matches the reference.
+    EXPECT_TRUE(decoded == reference_vector)
+        << "decoded data don't match the reference, input='"
+        << input << "', reference='" << reference << "'"
+        ", decoded='" << encoded << "'";
+}
+
+TEST(StringUtilTest, decodeColonSeparatedHexString) {
+    // Test valid strings.
+    testColonSeparated("A1:02:C3:d4:e5:F6", "A102C3D4E5F6");
+    testColonSeparated("A:02:3:d:E5:F6", "0A02030DE5F6");
+    testColonSeparated("A:B:C:D", "0A0B0C0D");
+    testColonSeparated("1", "01");
+    testColonSeparated("1e", "1E");
+    testColonSeparated("", "");
+
+    // Test invalid strings.
+    std::vector<uint8_t> decoded;
+    // Whitespaces.
+    EXPECT_THROW(decodeColonSeparatedHexString("   ", decoded),
+                 isc::BadValue);
+    // Whitespace before digits.
+    EXPECT_THROW(decodeColonSeparatedHexString(" A1", decoded),
+                 isc::BadValue);
+    // Two consecutive colons.
+    EXPECT_THROW(decodeColonSeparatedHexString("A::01", decoded),
+                 isc::BadValue);
+    // Three consecutive colons.
+    EXPECT_THROW(decodeColonSeparatedHexString("A:::01", decoded),
+                 isc::BadValue);
+    // Whitespace within a string.
+    EXPECT_THROW(decodeColonSeparatedHexString("A :01", decoded),
+                 isc::BadValue);
+    // Terminating colon.
+    EXPECT_THROW(decodeColonSeparatedHexString("0A:01:", decoded),
+                 isc::BadValue);
+    // Opening colon.
+    EXPECT_THROW(decodeColonSeparatedHexString(":0A:01", decoded),
+                 isc::BadValue);
+    // Three digits before the colon.
+    EXPECT_THROW(decodeColonSeparatedHexString("0A1:B1", decoded),
+                 isc::BadValue);
+}
+
+void testFormatted(const std::string& input,
+                   const std::string& reference) {
+    // Create a reference vector.
+    std::vector<uint8_t> reference_vector;
+    ASSERT_NO_THROW(encode::decodeHex(reference, reference_vector));
+
+    // Fill the output vector with some garbage to make sure that
+    // the data is erased when a string is decoded successfully.
+    std::vector<uint8_t> decoded(1, 10);
+    ASSERT_NO_THROW(decodeFormattedHexString(input, decoded));
+
+    // Get the string representation of the decoded data for logging
+    // purposes.
+    std::string encoded;
+    ASSERT_NO_THROW(encoded = encode::encodeHex(decoded));
+
+    // Check if the decoded data matches the reference.
+    EXPECT_TRUE(decoded == reference_vector)
+        << "decoded data don't match the reference, input='"
+        << input << "', reference='" << reference << "'"
+        ", decoded='" << encoded << "'";
+}
+
+TEST(StringUtilTest, decodeFormattedHexString) {
+    // Colon separated.
+    testFormatted("1:A7:B5:4:23", "01A7B50423");
+    // No colons, even number of digits.
+    testFormatted("17a534", "17A534");
+    // Odd number of digits.
+    testFormatted("A3A6f78", "0A3A6F78");
+    // '0x' prefix.
+    testFormatted("0xA3A6f78", "0A3A6F78");
+    // '0x' prefix with a special value of 0.
+    testFormatted("0x0", "00");
+    // Empty string.
+    testFormatted("", "");
+
+    std::vector<uint8_t> decoded;
+    // Whitepspace.
+    EXPECT_THROW(decodeFormattedHexString("0a ", decoded),
+                 isc::BadValue);
+    // Whitespace within a string.
+    EXPECT_THROW(decodeFormattedHexString("01 02", decoded),
+                 isc::BadValue);
+    // '0x' prefix and colons.
+    EXPECT_THROW(decodeFormattedHexString("0x01:02", decoded),
+                 isc::BadValue);
+    // Missing colon.
+    EXPECT_THROW(decodeFormattedHexString("01:0203", decoded),
+                 isc::BadValue);
+    // Invalid prefix.
+    EXPECT_THROW(decodeFormattedHexString("x0102", decoded),
+                 isc::BadValue);
+    // Invalid prefix again.
+    EXPECT_THROW(decodeFormattedHexString("1x0102", decoded),
+                 isc::BadValue);
+}
+
+} // end of anonymous namespace