Browse Source

[master] Merge branch 'trac4300'

Marcin Siodelski 9 years ago
parent
commit
abee3e733d

+ 97 - 58
src/lib/dhcpsrv/host.cc

@@ -75,14 +75,15 @@ Host::Host(const uint8_t* identifier, const size_t identifier_len,
            const std::string& hostname,
            const std::string& dhcp4_client_classes,
            const std::string& dhcp6_client_classes)
-    : hw_address_(), duid_(), ipv4_subnet_id_(ipv4_subnet_id),
+    : identifier_type_(identifier_type),
+      identifier_value_(), ipv4_subnet_id_(ipv4_subnet_id),
       ipv6_subnet_id_(ipv6_subnet_id),
       ipv4_reservation_(asiolink::IOAddress::IPV4_ZERO_ADDRESS()),
       hostname_(hostname), dhcp4_client_classes_(dhcp4_client_classes),
       dhcp6_client_classes_(dhcp6_client_classes), host_id_(0),
       cfg_option4_(), cfg_option6_() {
 
-    // Initialize HWAddr or DUID
+    // Initialize host identifier.
     setIdentifier(identifier, identifier_len, identifier_type);
 
     if (!ipv4_reservation.isV4Zero()) {
@@ -97,14 +98,15 @@ Host::Host(const std::string& identifier, const std::string& identifier_name,
            const std::string& hostname,
            const std::string& dhcp4_client_classes,
            const std::string& dhcp6_client_classes)
-    : hw_address_(), duid_(), ipv4_subnet_id_(ipv4_subnet_id),
+    : identifier_type_(IDENT_HWADDR),
+      identifier_value_(), ipv4_subnet_id_(ipv4_subnet_id),
       ipv6_subnet_id_(ipv6_subnet_id),
       ipv4_reservation_(asiolink::IOAddress::IPV4_ZERO_ADDRESS()),
       hostname_(hostname), dhcp4_client_classes_(dhcp4_client_classes),
       dhcp6_client_classes_(dhcp6_client_classes), host_id_(0),
       cfg_option4_(new CfgOption()), cfg_option6_(new CfgOption()) {
 
-    // Initialize HWAddr or DUID
+    // Initialize host identifier.
     setIdentifier(identifier, identifier_name);
 
     if (!ipv4_reservation.isV4Zero()) {
@@ -115,51 +117,36 @@ Host::Host(const std::string& identifier, const std::string& identifier_name,
 
 const std::vector<uint8_t>&
 Host::getIdentifier() const {
-    if (hw_address_) {
-        return (hw_address_->hwaddr_);
-
-    } else if (duid_) {
-        return (duid_->getDuid());
-
-    }
-    static std::vector<uint8_t> empty_vector;
-    return (empty_vector);
+    return (identifier_value_);
 }
 
 Host::IdentifierType
 Host::getIdentifierType() const {
-    if (hw_address_) {
-        return (IDENT_HWADDR);
-    }
-    return (IDENT_DUID);
+    return (identifier_type_);
 }
 
-std::string
-Host::getIdentifierAsText() const {
-    std::string txt;
-    if (hw_address_) {
-        txt = getIdentifierAsText(IDENT_HWADDR, &hw_address_->hwaddr_[0],
-                                  hw_address_->hwaddr_.size());
-    } else if (duid_) {
-        txt = getIdentifierAsText(IDENT_DUID, &duid_->getDuid()[0],
-                                  duid_->getDuid().size());
-    } else {
-        txt = "(none)";
-    }
+HWAddrPtr
+Host::getHWAddress() const {
+    return ((identifier_type_ == IDENT_HWADDR) ?
+            HWAddrPtr(new HWAddr(identifier_value_, HTYPE_ETHER)) : HWAddrPtr());
+}
+
+DuidPtr
+Host::getDuid() const {
+    return ((identifier_type_ == IDENT_DUID) ?
+            DuidPtr(new DUID(identifier_value_)) : DuidPtr());
+}
 
-    return (txt);
 
+std::string
+Host::getIdentifierAsText() const {
+    return (getIdentifierAsText(identifier_type_, &identifier_value_[0],
+                                identifier_value_.size()));
 }
 
 std::string
 Host::getIdentifierAsText(const IdentifierType& type, const uint8_t* value,
                           const size_t length) {
-    // Length 0 doesn't make sense.
-    if (length == 0) {
-        isc_throw(BadValue, "invalid length 0 of the host identifier while"
-                  " converting the identifier to a textual form");
-    }
-
     // Convert identifier into <type>=<value> form.
     std::ostringstream s;
     switch (type) {
@@ -169,46 +156,98 @@ Host::getIdentifierAsText(const IdentifierType& type, const uint8_t* value,
     case IDENT_DUID:
         s << "duid";
         break;
+    case IDENT_CIRCUIT_ID:
+        s << "circuit-id";
+        break;
     default:
-        isc_throw(BadValue, "requested conversion of the unsupported"
-                  " identifier into textual form");
+        // This should never happen actually, unless we add new identifier
+        // and forget to add a case for it above.
+        s << "(invalid-type)";
     }
     std::vector<uint8_t> vec(value, value + length);
-    s << "=" << util::encode::encodeHex(vec);
+    s << "=" << (length > 0 ? util::encode::encodeHex(vec) : "(null)");
     return (s.str());
 }
 
+std::string
+Host::getIdentifierName(const IdentifierType& type) {
+    switch (type) {
+    case Host::IDENT_HWADDR:
+        return ("hw-address");
+
+    case Host::IDENT_DUID:
+        return ("duid");
+
+    case Host::IDENT_CIRCUIT_ID:
+        return ("circuit-id");
+
+    default:
+        ;
+    }
+    return ("(unknown)");
+}
+
 
 void
 Host::setIdentifier(const uint8_t* identifier, const size_t len,
                     const IdentifierType& type) {
-    switch (type) {
-    case IDENT_HWADDR:
-        hw_address_ = HWAddrPtr(new HWAddr(identifier, len, HTYPE_ETHER));
-        duid_.reset();
-        break;
-    case IDENT_DUID:
-        duid_ = DuidPtr(new DUID(identifier, len));
-        hw_address_.reset();
-        break;
-    default:
-        isc_throw(isc::BadValue, "invalid client identifier type '"
-                  << static_cast<int>(type) << "' when creating host "
-                  " instance");
+    if (len < 1) {
+        isc_throw(BadValue, "invalid client identifier length 0");
     }
+
+    identifier_type_ = type;
+    identifier_value_.assign(identifier, identifier + 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") {
-        hw_address_ = HWAddrPtr(new HWAddr(HWAddr::fromText(identifier)));
-        duid_.reset();
+        HWAddr hwaddr(HWAddr::fromText(identifier));
+        identifier_type_= IDENT_HWADDR;
+        identifier_value_ = hwaddr.hwaddr_;
+
     } else if (name == "duid") {
-        duid_ = DuidPtr(new DUID(DUID::fromText(identifier)));
-        hw_address_.reset();
+        identifier_type_ = IDENT_DUID;
+        DUID duid(DUID::fromText(identifier));
+        identifier_value_ = duid.getDuid();
+
     } else {
-        isc_throw(isc::BadValue, "invalid client identifier type '"
-                  << name << "' when creating host instance");
+        if (name == "circuit-id") {
+            identifier_type_ = IDENT_CIRCUIT_ID;
+
+        } else {
+            isc_throw(isc::BadValue, "invalid client identifier type '"
+                      << name << "' when creating host instance");
+        }
+
+        // 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.
+
+        // 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.
+        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 << "'");
+            }
+        }
+
+        // Successfully decoded the identifier, so let's use it.
+        identifier_value_.swap(binary);
     }
 }
 

+ 35 - 32
src/lib/dhcpsrv/host.h

@@ -179,7 +179,8 @@ public:
     /// DHCPv6 client's DUID are supported.
     enum IdentifierType {
         IDENT_HWADDR,
-        IDENT_DUID
+        IDENT_DUID,
+        IDENT_CIRCUIT_ID
     };
 
     /// @brief Constructor.
@@ -222,11 +223,19 @@ public:
     /// 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 is specified as: "010203040506abcd"
+    /// - 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.
     ///
     /// @param identifier Identifier in the textual format. The expected formats
-    /// for the hardware address and DUID have been shown above.
-    /// @param identifier_name One of "hw-address" or "duid"
+    /// for the hardware address and other identifiers are provided above.
+    /// @param identifier_name One of "hw-address", "duid", "circuit-id".
     /// @param ipv4_subnet_id Identifier of the IPv4 subnet to which the host
     /// is connected.
     /// @param ipv6_subnet_id Identifier of the IPv6 subnet to which the host
@@ -251,13 +260,10 @@ public:
 
     /// @brief Replaces currently used identifier with a new identifier.
     ///
-    /// This method initializes hardware address or DUID (@c hw_address_ or
-    /// @c duid_ respectively). The other (not initialized member) is
-    /// deallocated.
-    ///
+    /// This method sets a new identifier type and value for a host.
     /// This method is called by the @c Host constructor.
     ///
-    /// @param identifier Pointer to the new identifier in the textual format.
+    /// @param identifier Pointer to a buffer holding an identifier.
     /// @param len Length of the identifier that the @c identifier points to.
     /// @param type Identifier type.
     ///
@@ -267,14 +273,11 @@ public:
 
     /// @brief Replaces currently used identifier with a new identifier.
     ///
-    /// This method initializes hardware address or DUID (@c hw_address_ or
-    /// @c duid_ respectively). The other (not initialized member) is
-    /// deallocated.
-    ///
+    /// This method sets a new identifier type and value for a host.
     /// This method is called by the @c Host constructor.
     ///
-    /// @param identifier Pointer to the new identifier in the textual format.
-    /// @param name One of "hw-address" or "duid".
+    /// @param identifier Reference to a new identifier in the textual format.
+    /// @param name One of "hw-address", "duid", "circuit-id".
     ///
     /// @throw BadValue if the identifier is invalid.
     void setIdentifier(const std::string& identifier, const std::string& name);
@@ -283,30 +286,32 @@ public:
     ///
     /// @return Pointer to the @c HWAddr structure or null if the reservation
     /// is not associated with a hardware address.
-    HWAddrPtr getHWAddress() const {
-        return (hw_address_);
-    }
+    HWAddrPtr getHWAddress() const;
 
     /// @brief Returns DUID for which the reservations are made.
     ///
     /// @return Pointer to the @c DUID structure or null if the reservation
     /// is not associated with a DUID.
-    DuidPtr getDuid() const {
-        return (duid_);
-    }
+    DuidPtr getDuid() const;
 
-    /// @brief Returns the identifier (MAC or DUID) in binary form.
-    /// @return const reference to MAC or DUID in vector<uint8_t> form
+    /// @brief Returns the identifier in a binary form.
+    ///
+    /// @return const reference to a vector<uint8_t> holding an identifier
+    /// value.
     const std::vector<uint8_t>& getIdentifier() const;
 
     /// @brief Returns the identifier type.
-    /// @return the identifier type
+    ///
     IdentifierType getIdentifierType() const;
 
-    /// @brief Returns host identifier (mac or DUID) in printer friendly form.
-    /// @return text form of the identifier, including (duid= or mac=).
+    /// @brief Returns host identifier in a textual form.
+    ///
+    /// @return Identifier in the form of <type>=<value>.
     std::string getIdentifierAsText() const;
 
+    /// @brief Returns name of the identifier of a specified type.
+    static std::string getIdentifierName(const IdentifierType& type);
+
     /// @brief Returns host identifier in textual form.
     ///
     /// @param type Identifier type.
@@ -487,12 +492,10 @@ private:
     void addClientClassInternal(ClientClasses& classes,
                                 const std::string& class_name);
 
-    /// @brief Pointer to the hardware address associated with the reservations
-    /// for the host.
-    HWAddrPtr hw_address_;
-    /// @brief Pointer to the DUID associated with the reservations for the
-    /// host.
-    DuidPtr duid_;
+    /// @brief Identifier type.
+    IdentifierType identifier_type_;
+    /// @brief Vector holding identifier value.
+    std::vector<uint8_t> identifier_value_;
     /// @brief Subnet identifier for the DHCPv4 client.
     SubnetID ipv4_subnet_id_;
     /// @brief Subnet identifier for the DHCPv6 client.

+ 24 - 51
src/lib/dhcpsrv/mysql_host_data_source.cc

@@ -42,6 +42,11 @@ const size_t CLIENT_CLASSES_MAX_LEN = 255;
 /// in the Client FQDN %Option (see RFC4702 and RFC4704).
 const size_t HOSTNAME_MAX_LEN = 255;
 
+/// @brief Numeric value representing last supported identifier.
+///
+/// This value is used to validate whether the identifier type stored in
+/// a database is within bounds. of supported identifiers.
+const uint8_t MAX_IDENTIFIER_TYPE = static_cast<uint8_t>(Host::IDENT_CIRCUIT_ID);
 
 /// @brief Prepared MySQL statements used by the backend to insert and
 /// retrieve hosts from the database.
@@ -299,42 +304,21 @@ public:
             bind_[0].is_unsigned = MLM_TRUE;
 
             // dhcp_identifier : VARBINARY(128) NOT NULL
-            // Check which of the identifiers is used and set values accordingly
-            if (host->getDuid()) {
-                dhcp_identifier_length_ = host->getDuid()->getDuid().size();
-                bind_[1].buffer_type = MYSQL_TYPE_BLOB;
-                bind_[1].buffer = reinterpret_cast<char*>
-                    (const_cast<uint8_t*>(&(host->getDuid()->getDuid()[0])));
-                bind_[1].buffer_length = dhcp_identifier_length_;
-                bind_[1].length = &dhcp_identifier_length_;
-
-            } else if (host->getHWAddress()){
-                dhcp_identifier_length_ = host->getHWAddress()->hwaddr_.size();
-                bind_[1].buffer_type = MYSQL_TYPE_BLOB;
-                bind_[1].buffer = reinterpret_cast<char*>
-                    (&(host->getHWAddress()->hwaddr_[0]));
-                bind_[1].buffer_length = dhcp_identifier_length_;
-                bind_[1].length = &dhcp_identifier_length_;
-
-            } else {
-                isc_throw(DbOperationError, "Host object doesn't contain any"
-                          " identifier which can be used to retrieve information"
-                          " from the database about its static reservations");
-            }
+            dhcp_identifier_length_ = host->getIdentifier().size();
+            memcpy(static_cast<void*>(dhcp_identifier_buffer_),
+                   &(host->getIdentifier())[0],
+                   host->getIdentifier().size());
+
+            bind_[1].buffer_type = MYSQL_TYPE_BLOB;
+            bind_[1].buffer = dhcp_identifier_buffer_;
+            bind_[1].buffer_length = dhcp_identifier_length_;
+            bind_[1].length = &dhcp_identifier_length_;
 
             // dhcp_identifier_type : TINYINT NOT NULL
-            // Check which of the identifier types is used and set values accordingly
-            if (host->getHWAddress()) {
-                dhcp_identifier_type_ = BaseHostDataSource::ID_HWADDR; // 0
-                bind_[2].buffer_type = MYSQL_TYPE_TINY;
-                bind_[2].buffer = reinterpret_cast<char*>(&dhcp_identifier_type_);
-                bind_[2].is_unsigned = MLM_TRUE;
-            } else if (host->getDuid()) {
-                dhcp_identifier_type_ = BaseHostDataSource::ID_DUID; // 1
-                bind_[2].buffer_type = MYSQL_TYPE_TINY;
-                bind_[2].buffer = reinterpret_cast<char*>(&dhcp_identifier_type_);
-                bind_[2].is_unsigned = MLM_TRUE;
-            }
+            dhcp_identifier_type_ = static_cast<uint8_t>(host->getIdentifierType());
+            bind_[2].buffer_type = MYSQL_TYPE_TINY;
+            bind_[2].buffer = reinterpret_cast<char*>(&dhcp_identifier_type_);
+            bind_[2].is_unsigned = MLM_TRUE;
 
             // dhcp4_subnet_id : INT UNSIGNED NULL
             // Can't take an address of intermediate object, so let's store it
@@ -495,25 +479,14 @@ public:
     /// @return Host Pointer to a @ref HostPtr object holding a pointer to the
     /// @ref Host object returned.
     HostPtr retrieveHost() {
-
-        // Set the dhcp identifier type in a variable of the appropriate data type,
-        // which has been initialized with an arbitrary (but valid) value.
-        Host::IdentifierType type = Host::IDENT_HWADDR;
-
-        switch (dhcp_identifier_type_) {
-        case 0:
-            type = Host::IDENT_HWADDR;
-            break;
-
-        case 1:
-            type = Host::IDENT_DUID;
-            break;
-
-        default:
+        // Check if the identifier stored in the database is correct.
+        if (dhcp_identifier_type_ > MAX_IDENTIFIER_TYPE) {
             isc_throw(BadValue, "invalid dhcp identifier type returned: "
-                      << static_cast<int>(dhcp_identifier_type_)
-                      << ". Only 0 or 1 are supported.");
+                      << static_cast<int>(dhcp_identifier_type_));
         }
+        // Set the dhcp identifier type in a variable of the appropriate data type.
+        Host::IdentifierType type =
+            static_cast<Host::IdentifierType>(dhcp_identifier_type_);
 
         // Set DHCPv4 subnet ID to the value returned. If NULL returned, set to 0.
         SubnetID ipv4_subnet_id(0);

+ 51 - 108
src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc

@@ -26,7 +26,7 @@ GenericHostDataSourceTest::GenericHostDataSourceTest()
 GenericHostDataSourceTest::~GenericHostDataSourceTest() {
 }
 
-std::string
+std::vector<uint8_t>
 GenericHostDataSourceTest::generateHWAddr(const bool new_identifier) {
     /// @todo: Consider moving this somewhere to lib/testutils.
 
@@ -34,15 +34,6 @@ GenericHostDataSourceTest::generateHWAddr(const bool new_identifier) {
     // if you need to enter MySQL queries by hand.
     static uint8_t hwaddr[] = {65, 66, 67, 68, 69, 70};
 
-    stringstream tmp;
-    for (int i = 0; i < sizeof(hwaddr); ++i) {
-        if (i) {
-            tmp << ":";
-        }
-        tmp << std::setw(2) << std::hex << std::setfill('0')
-            << static_cast<unsigned int>(hwaddr[i]);
-    }
-
     if (new_identifier) {
         // Increase the address for the next time we use it.
         // This is primitive, but will work for 65k unique
@@ -52,46 +43,38 @@ GenericHostDataSourceTest::generateHWAddr(const bool new_identifier) {
             hwaddr[sizeof(hwaddr) - 2]++;
         }
     }
-    return (tmp.str());
+    return (std::vector<uint8_t>(hwaddr, hwaddr + sizeof(hwaddr)));
 }
 
-std::string
-GenericHostDataSourceTest::generateDuid(const bool new_identifier) {
+std::vector<uint8_t>
+GenericHostDataSourceTest::generateIdentifier(const bool new_identifier) {
     /// @todo: Consider moving this somewhere to lib/testutils.
 
     // Let's use something that is easily printable. That's convenient
     // if you need to enter MySQL queries by hand.
-    static uint8_t duid[] = { 65, 66, 67, 68, 69, 70, 71, 72, 73, 74 };
+    static uint8_t ident[] = { 65, 66, 67, 68, 69, 70, 71, 72, 73, 74 };
 
-    stringstream tmp;
-    for (int i = 0; i < sizeof(duid); ++i) {
-        tmp << std::setw(2) << std::hex << std::setfill('0')
-            << static_cast<unsigned int>(duid[i]);
-    }
-
-    // Increase the DUID for the next time we use it.
+    // Increase the identifier for the next time we use it.
     // This is primitive, but will work for 65k unique
-    // DUIDs.
+    // identifiers.
     if (new_identifier) {
-        duid[sizeof(duid) - 1]++;
-        if (duid[sizeof(duid) - 1] == 0) {
-            duid[sizeof(duid) - 2]++;
+        ident[sizeof(ident) - 1]++;
+        if (ident[sizeof(ident) - 1] == 0) {
+            ident[sizeof(ident) - 2]++;
         }
     }
-    return (tmp.str());
+    return (std::vector<uint8_t>(ident, ident + sizeof(ident)));
 }
 
-HostPtr GenericHostDataSourceTest::initializeHost4(std::string address,
-                                                   bool hwaddr) {
-    string ident;
-    string ident_type;
-
-    if (hwaddr) {
+HostPtr
+GenericHostDataSourceTest::initializeHost4(const std::string& address,
+                                           const Host::IdentifierType& id) {
+    std::vector<uint8_t> ident;
+    if (id == Host::IDENT_HWADDR) {
         ident = generateHWAddr();
-        ident_type = "hw-address";
+
     } else {
-        ident = generateDuid();
-        ident_type = "duid";
+        ident = generateIdentifier();
     }
 
     // Let's create ever increasing subnet-ids. Let's keep those different,
@@ -103,7 +86,7 @@ HostPtr GenericHostDataSourceTest::initializeHost4(std::string address,
     subnet6++;
 
     IOAddress addr(address);
-    HostPtr host(new Host(ident, ident_type, subnet4, subnet6, addr));
+    HostPtr host(new Host(&ident[0], ident.size(), id, subnet4, subnet6, addr));
 
     return (host);
 }
@@ -112,17 +95,13 @@ HostPtr GenericHostDataSourceTest::initializeHost6(std::string address,
                                                    Host::IdentifierType identifier,
                                                    bool prefix,
                                                    bool new_identifier) {
-    string ident;
-    string ident_type;
-
+    std::vector<uint8_t> ident;
     switch (identifier) {
     case Host::IDENT_HWADDR:
         ident = generateHWAddr(new_identifier);
-        ident_type = "hw-address";
         break;
     case Host::IDENT_DUID:
-        ident = generateDuid(new_identifier);
-        ident_type = "duid";
+        ident = generateIdentifier(new_identifier);
         break;
     default:
         ADD_FAILURE() << "Unknown IdType: " << identifier;
@@ -137,7 +116,8 @@ HostPtr GenericHostDataSourceTest::initializeHost6(std::string address,
     subnet4++;
     subnet6++;
 
-    HostPtr host(new Host(ident, ident_type, subnet4, subnet6, IOAddress("0.0.0.0")));
+    HostPtr host(new Host(&ident[0], ident.size(), identifier, subnet4,
+                          subnet6, IOAddress("0.0.0.0")));
 
     if (!prefix) {
         // Create IPv6 reservation (for an address)
@@ -333,12 +313,12 @@ GenericHostDataSourceTest::compareClientClasses(const ClientClasses& /*classes1*
     ///        This is part of the work for #4213.
 }
 
-void GenericHostDataSourceTest::testBasic4(bool hwaddr) {
+void GenericHostDataSourceTest::testBasic4(const Host::IdentifierType& id) {
     // Make sure we have the pointer to the host data source.
     ASSERT_TRUE(hdsptr_);
 
     // Create a host reservation.
-    HostPtr host = initializeHost4("192.0.2.1", hwaddr);
+    HostPtr host = initializeHost4("192.0.2.1", id);
     ASSERT_TRUE(host); // Make sure the host is generate properly.
     SubnetID subnet = host->getIPv4SubnetID();
 
@@ -358,15 +338,15 @@ void GenericHostDataSourceTest::testBasic4(bool hwaddr) {
 }
 
 
-void GenericHostDataSourceTest::testGetByIPv4(bool hwaddr) {
+void GenericHostDataSourceTest::testGetByIPv4(const Host::IdentifierType& id) {
     // Make sure we have a pointer to the host data source.
     ASSERT_TRUE(hdsptr_);
 
     // Let's create a couple of hosts...
-    HostPtr host1 = initializeHost4("192.0.2.1", hwaddr);
-    HostPtr host2 = initializeHost4("192.0.2.2", hwaddr);
-    HostPtr host3 = initializeHost4("192.0.2.3", hwaddr);
-    HostPtr host4 = initializeHost4("192.0.2.4", hwaddr);
+    HostPtr host1 = initializeHost4("192.0.2.1", id);
+    HostPtr host2 = initializeHost4("192.0.2.2", id);
+    HostPtr host3 = initializeHost4("192.0.2.3", id);
+    HostPtr host4 = initializeHost4("192.0.2.4", id);
 
     // ... and add them to the data source.
     ASSERT_NO_THROW(hdsptr_->add(host1));
@@ -402,52 +382,16 @@ void GenericHostDataSourceTest::testGetByIPv4(bool hwaddr) {
     EXPECT_FALSE(hdsptr_->get4(subnet1, IOAddress("192.0.1.5")));
 }
 
-void GenericHostDataSourceTest::testGet4ByHWAddr() {
-    // Make sure we have a pointer to the host data source.
-    ASSERT_TRUE(hdsptr_);
-
-    HostPtr host1 = initializeHost4("192.0.2.1", true);
-    HostPtr host2 = initializeHost4("192.0.2.2", true);
-
-    // Sanity check: make sure the hosts have different HW addresses.
-    ASSERT_TRUE(host1->getHWAddress());
-    ASSERT_TRUE(host2->getHWAddress());
-    compareHwaddrs(host1, host2, false);
-
-    // Try to add both of them to the host data source.
-    ASSERT_NO_THROW(hdsptr_->add(host1));
-    ASSERT_NO_THROW(hdsptr_->add(host2));
-
-    SubnetID subnet1 = host1->getIPv4SubnetID();
-    SubnetID subnet2 = host2->getIPv4SubnetID();
-
-    ConstHostPtr from_hds1 = hdsptr_->get4(subnet1,
-                                           Host::IDENT_HWADDR,
-                                           &host1->getIdentifier()[0],
-                                           host1->getIdentifier().size());
-    ConstHostPtr from_hds2 = hdsptr_->get4(subnet2,
-                                           Host::IDENT_HWADDR,
-                                           &host2->getIdentifier()[0],
-                                           host2->getIdentifier().size());
-
-    // Now let's check if we got what we expected.
-    ASSERT_TRUE(from_hds1);
-    ASSERT_TRUE(from_hds2);
-    compareHosts(host1, from_hds1);
-    compareHosts(host2, from_hds2);
-}
-
-void GenericHostDataSourceTest::testGet4ByClientId() {
+void
+GenericHostDataSourceTest::testGet4ByIdentifier(const Host::IdentifierType& identifier_type) {
     // Make sure we have a pointer to the host data source.
     ASSERT_TRUE(hdsptr_);
 
-    HostPtr host1 = initializeHost4("192.0.2.1", false);
-    HostPtr host2 = initializeHost4("192.0.2.2", false);
+    HostPtr host1 = initializeHost4("192.0.2.1", identifier_type);
+    HostPtr host2 = initializeHost4("192.0.2.2", identifier_type);
 
-    // Sanity check: make sure the hosts have different client-ids.
-    ASSERT_TRUE(host1->getDuid());
-    ASSERT_TRUE(host2->getDuid());
-    compareDuids(host1, host2, false);
+    // Sanity check: make sure the hosts have different identifiers..
+    ASSERT_FALSE(host1->getIdentifier() == host2->getIdentifier());
 
     // Try to add both of them to the host data source.
     ASSERT_NO_THROW(hdsptr_->add(host1));
@@ -457,12 +401,12 @@ void GenericHostDataSourceTest::testGet4ByClientId() {
     SubnetID subnet2 = host2->getIPv4SubnetID();
 
     ConstHostPtr from_hds1 = hdsptr_->get4(subnet1,
-                                           Host::IDENT_DUID,
+                                           identifier_type,
                                            &host1->getIdentifier()[0],
                                            host1->getIdentifier().size());
 
     ConstHostPtr from_hds2 = hdsptr_->get4(subnet2,
-                                           Host::IDENT_DUID,
+                                           identifier_type,
                                            &host2->getIdentifier()[0],
                                            host2->getIdentifier().size());
 
@@ -478,7 +422,7 @@ void GenericHostDataSourceTest::testHWAddrNotClientId() {
     ASSERT_TRUE(hdsptr_);
 
     // Create a host with HW address
-    HostPtr host = initializeHost4("192.0.2.1", true);
+    HostPtr host = initializeHost4("192.0.2.1", Host::IDENT_HWADDR);
     ASSERT_TRUE(host->getHWAddress());
     ASSERT_FALSE(host->getDuid());
 
@@ -509,7 +453,7 @@ void GenericHostDataSourceTest::testClientIdNotHWAddr() {
     ASSERT_TRUE(hdsptr_);
 
     // Create a host with client-id
-    HostPtr host = initializeHost4("192.0.2.1", false);
+    HostPtr host = initializeHost4("192.0.2.1", Host::IDENT_DUID);
     ASSERT_FALSE(host->getHWAddress());
     ASSERT_TRUE(host->getDuid());
 
@@ -553,7 +497,7 @@ GenericHostDataSourceTest::testHostname(std::string name, int num) {
 
         addr = IOAddress::increase(addr);
 
-        HostPtr host = initializeHost4(addr.toText(), false);
+        HostPtr host = initializeHost4(addr.toText(), Host::IDENT_DUID);
 
         stringstream hostname;
         hostname.str("");
@@ -589,12 +533,13 @@ GenericHostDataSourceTest::testHostname(std::string name, int num) {
 }
 
 void
-GenericHostDataSourceTest::testMultipleSubnets(int subnets, bool hwaddr) {
+GenericHostDataSourceTest::testMultipleSubnets(int subnets,
+                                               const Host::IdentifierType& id) {
 
     // Make sure we have a pointer to the host data source.
     ASSERT_TRUE(hdsptr_);
 
-    HostPtr host = initializeHost4("192.0.2.1", hwaddr);
+    HostPtr host = initializeHost4("192.0.2.1", id);
 
     for (int i = 0; i < subnets; ++i) {
         host->setIPv4SubnetID(i + 1000);
@@ -616,8 +561,7 @@ GenericHostDataSourceTest::testMultipleSubnets(int subnets, bool hwaddr) {
 
         // Try to retrieve the host by either HW address of client-id
         from_hds = hdsptr_->get4(i + 1000,
-                                 hwaddr ? Host::IDENT_HWADDR : Host::IDENT_DUID,
-                                 &host->getIdentifier()[0],
+                                 id, &host->getIdentifier()[0],
                                  host->getIdentifier().size());
         ASSERT_TRUE(from_hds);
         EXPECT_EQ(i + 1000, from_hds->getIPv4SubnetID());
@@ -637,8 +581,7 @@ GenericHostDataSourceTest::testMultipleSubnets(int subnets, bool hwaddr) {
 
     // Finally, check that the hosts can be retrived by HW address or DUID
     ConstHostCollection all_by_id =
-        hdsptr_->getAll(hwaddr ? Host::IDENT_HWADDR : Host::IDENT_DUID,
-                        &host->getIdentifier()[0],
+        hdsptr_->getAll(id, &host->getIdentifier()[0],
                         host->getIdentifier().size());
     ASSERT_EQ(subnets, all_by_id.size());
 
@@ -656,8 +599,8 @@ void GenericHostDataSourceTest::testGet6ByHWAddr() {
     ASSERT_TRUE(hdsptr_);
 
     // Create a host reservations.
-    HostPtr host1 = initializeHost6("2001:db8::1", Host::IDENT_HWADDR, true);
-    HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_HWADDR, true);
+    HostPtr host1 = initializeHost6("2001:db8::1", Host::IDENT_HWADDR, "hw-address");
+    HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_HWADDR, "hw-address");
 
     // Sanity check: make sure the hosts have different HW addresses.
     ASSERT_TRUE(host1->getHWAddress());
@@ -692,8 +635,8 @@ void GenericHostDataSourceTest::testGet6ByClientId() {
     ASSERT_TRUE(hdsptr_);
 
     // Create a host reservations.
-    HostPtr host1 = initializeHost6("2001:db8::1", Host::IDENT_DUID, true);
-    HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_DUID, true);
+    HostPtr host1 = initializeHost6("2001:db8::1", Host::IDENT_DUID, "hw-address");
+    HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_DUID, "hw-address");
 
     // Sanity check: make sure the hosts have different HW addresses.
     ASSERT_TRUE(host1->getDuid());
@@ -848,7 +791,7 @@ void GenericHostDataSourceTest::testAddDuplicate4() {
     ASSERT_TRUE(hdsptr_);
 
     // Create a host reservations.
-    HostPtr host = initializeHost4("192.0.2.1", false);
+    HostPtr host = initializeHost4("192.0.2.1", Host::IDENT_DUID);
 
     // Add this reservation once.
     ASSERT_NO_THROW(hdsptr_->add(host));

+ 17 - 21
src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h

@@ -39,10 +39,11 @@ public:
     /// @brief Creates a host reservation for specified IPv4 address.
     ///
     /// @param address IPv4 address to be set
-    /// @param hwaddr type of identifier (true = hwaddr, false = client-id)
+    /// @param id Identifier type.
     ///
     /// @return generated Host object
-    HostPtr initializeHost4(std::string address, bool hwaddr);
+    HostPtr initializeHost4(const std::string& address,
+                            const Host::IdentifierType& id);
 
     /// @brief Creates a host reservation for specified IPv6 address.
     ///
@@ -61,14 +62,14 @@ public:
     /// @param increase A boolean value indicating if new address (increased)
     /// must be generated or the same address as previously.
     /// @return HW address in textual form acceptable by Host constructor
-    std::string generateHWAddr(const bool new_identifier = true);
+    std::vector<uint8_t> generateHWAddr(const bool new_identifier = true);
 
-    /// @brief Generates a hardware address in text version.
+    /// @brief Generates a host identifier in a textual form..
     ///
-    /// @param increase A boolean value indicating if new DUID (increased)
-    /// must be generated or the same DUID as previously.
-    /// @return DUID in textual form acceptable by Host constructor
-    std::string generateDuid(const bool new_identifier = true);
+    /// @param increase A boolean value indicating if new identifier (increased)
+    /// must be generated or the same identifier as previously.
+    /// @return Identifier in textual form acceptable by Host constructor
+    std::vector<uint8_t> generateIdentifier(const bool new_identifier = true);
 
     /// @brief Compares hardware addresses of the two hosts.
     ///
@@ -130,25 +131,20 @@ public:
     ///        can be inserted and later retrieved.
     ///
     /// Uses gtest macros to report failures.
-    /// @param hwaddr true = use HW address as identifier, false = use client-id(DUID)
-    void testBasic4(bool hwaddr);
+    /// @param id Identifier type.
+    void testBasic4(const Host::IdentifierType& id);
 
     /// @brief Test inserts several hosts with unique IPv4 address and
     ///        checks that they can be retrieved properly.
     ///
     /// Uses gtest macros to report failures.
-    /// @param hwaddr true = use HW address as identifier, false = use client-id(DUID)
-    void testGetByIPv4(bool hwaddr);
-
-    /// @brief Test that hosts can be retrieved by hardware address.
-    ///
-    /// Uses gtest macros to report failures.
-    void testGet4ByHWAddr();
+    /// @param id Identifier type.
+    void testGetByIPv4(const Host::IdentifierType& id);
 
-    /// @brief Test that hosts can be retrieved by client-id
+    /// @brief Test that hosts can be retrieved by host identifier.
     ///
     /// Uses gtest macros to report failures.
-    void testGet4ByClientId();
+    void testGet4ByIdentifier(const Host::IdentifierType& identifier_type);
 
     /// @brief Test that clients with stored HW address can't be retrieved
     ///        by DUID with the same value.
@@ -184,8 +180,8 @@ public:
     /// Uses gtest macros to report failures.
     ///
     /// @param subnets number of subnets to test
-    /// @param hwaddr true = use HW address, false = use client-id
-    void testMultipleSubnets(int subnets, bool hwaddr);
+    /// @param id Host identifier type.
+    void testMultipleSubnets(int subnets, const Host::IdentifierType& id);
 
     /// @brief Test inserts several hosts with unique IPv6 addresses and
     ///        checks that they can be retrieved properly.

+ 328 - 93
src/lib/dhcpsrv/tests/host_unittest.cc

@@ -7,8 +7,12 @@
 #include <config.h>
 
 #include <dhcpsrv/host.h>
+#include <util/encode/hex.h>
+#include <util/range_utilities.h>
 #include <boost/scoped_ptr.hpp>
 #include <gtest/gtest.h>
+#include <cstdlib>
+#include <sstream>
 
 using namespace isc;
 using namespace isc::dhcp;
@@ -16,6 +20,11 @@ using namespace isc::asiolink;
 
 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;
+
 // This test verifies that it is possible to create IPv6 address
 // reservation.
 TEST(IPv6ResrvTest, constructorAddress) {
@@ -121,9 +130,52 @@ TEST(IPv6ResrvTest, equal) {
 
 }
 
+/// @brief Test fixture class for @c Host.
+class HostTest : public ::testing::Test {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// Re-initializes random number generator.
+    HostTest() {
+        srand(1);
+    }
+
+    /// @brief Checks if the reservation is in the range of reservations.
+    ///
+    /// @param resrv Reservation to be searched for.
+    /// @param range Range of reservations returned by the @c Host object
+    /// in which the reservation will be searched.
+    ///
+    /// @return true if reservation exists, false otherwise.
+    bool
+    reservationExists(const IPv6Resrv& resrv, const IPv6ResrvRange& range) {
+        for (IPv6ResrvIterator it = range.first; it != range.second;
+             ++it) {
+            if (resrv == it->second) {
+                return (true);
+            }
+        }
+        return (false);
+    }
+
+    /// @brief Returns upper bound of the supported identifier types.
+    ///
+    /// Some unit tests verify the @c Host class behavior for all
+    /// supported identifier types. The unit test needs to iterate
+    /// over all supported identifier types and thus it must be
+    /// aware of the upper bound of the @c Host::IdentifierType
+    /// enum. The upper bound is the numeric representation of the
+    /// last identifier type plus 1.
+    unsigned int
+    identifierTypeUpperBound()  const {
+        return (static_cast<unsigned int>(LAST_IDENTIFIER_TYPE) + 1);
+    }
+};
+
 // This test verfies that it is possible to create a Host object
 // using hardware address in the textual format.
-TEST(HostTest, createFromHWAddrString) {
+TEST_F(HostTest, createFromHWAddrString) {
     boost::scoped_ptr<Host> host;
     ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address",
                                         SubnetID(1), SubnetID(2),
@@ -155,7 +207,7 @@ TEST(HostTest, createFromHWAddrString) {
 
 // This test verifies that it is possible to create Host object using
 // a DUID in the textual format.
-TEST(HostTest, createFromDUIDString) {
+TEST_F(HostTest, createFromDUIDString) {
     boost::scoped_ptr<Host> host;
     ASSERT_NO_THROW(host.reset(new Host("a1:b2:c3:d4:e5:06", "duid",
                                         SubnetID(10), SubnetID(20),
@@ -189,7 +241,7 @@ TEST(HostTest, createFromDUIDString) {
 
 // This test verifies that it is possible to create Host object using
 // hardware address in the binary format.
-TEST(HostTest, createFromHWAddrBinary) {
+TEST_F(HostTest, createFromHWAddrBinary) {
     boost::scoped_ptr<Host> host;
     // Prepare the hardware address in binary format.
     const uint8_t hwaddr_data[] = {
@@ -217,7 +269,7 @@ TEST(HostTest, createFromHWAddrBinary) {
 
 // This test verifies that it is possible to create a Host object using
 // DUID in the binary format.
-TEST(HostTest, createFromDuidBinary) {
+TEST_F(HostTest, createFromDuidBinary) {
     boost::scoped_ptr<Host> host;
     // Prepare DUID binary.
     const uint8_t duid_data[] = {
@@ -243,103 +295,270 @@ TEST(HostTest, createFromDuidBinary) {
     EXPECT_EQ("me.example.org", host->getHostname());
 }
 
-// Test that it is possible to replace an identifier for a particular
-// Host instance (HW address -> DUID and vice versa) with a new
-// indentifier in the textual format.
-TEST(HostTest, setIdentifierString) {
+// This test verifies that it is possible create Host instance using all
+// supported identifiers in a binary format.
+TEST_F(HostTest, createFromIdentifierBinary) {
     boost::scoped_ptr<Host> host;
-    ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address",
-                                        SubnetID(1), SubnetID(2),
-                                        IOAddress("192.0.2.3"),
-                                        "me.example.com")));
-    // Initially, there should be a HW address, but not a DUID set.
-    ASSERT_TRUE(host->getHWAddress());
-    ASSERT_FALSE(host->getDuid());
-
-    // Now, use a DUID as identifier.
-    ASSERT_NO_THROW(host->setIdentifier("aabbccddee", "duid"));
-
-    // Verify that the DUID is correct.
-    DuidPtr duid = host->getDuid();
-    ASSERT_TRUE(duid);
-    EXPECT_EQ("aa:bb:cc:dd:ee", duid->toText());
-    // HW address should be not set.
-    EXPECT_FALSE(host->getHWAddress());
-
-    // Now, let's do another way around.
+    // Iterate over all supported identifier types.
+    for (unsigned int i = 0; i < identifierTypeUpperBound(); ++i) {
+        const Host::IdentifierType type = static_cast<Host::IdentifierType>(i);
+        // Create identifier of variable length and fill with random values.
+        std::vector<uint8_t> identifier(random() % 14 + 6);
+        util::fillRandom(identifier.begin(), identifier.end());
+
+        // Try to create a Host instance using this identifier.
+        ASSERT_NO_THROW(host.reset(new Host(&identifier[0], identifier.size(),
+                                            type, SubnetID(10), SubnetID(20),
+                                            IOAddress("192.0.2.5"),
+                                            "me.example.org")));
+
+        // Retrieve identifier from Host instance and check if it is correct.
+        const std::vector<uint8_t>& identifier_returned = host->getIdentifier();
+        EXPECT_TRUE(identifier_returned == identifier);
+        EXPECT_EQ(type, host->getIdentifierType());
+
+        EXPECT_EQ(10, host->getIPv4SubnetID());
+        EXPECT_EQ(20, host->getIPv6SubnetID());
+        EXPECT_EQ("192.0.2.5", host->getIPv4Reservation().toText());
+        EXPECT_EQ("me.example.org", host->getHostname());
+    }
+}
 
-    ASSERT_NO_THROW(host->setIdentifier("09:08:07:06:05:04", "hw-address"));
+// This test verifies that it is possible to create Host instance using
+// all supported identifiers in hexadecimal format.
+TEST_F(HostTest, createFromIdentifierHex) {
+    boost::scoped_ptr<Host> host;
+    // Iterate over all supported identifiers.
+    for (unsigned int i = 0; i < identifierTypeUpperBound(); ++i) {
+        const Host::IdentifierType type = static_cast<Host::IdentifierType>(i);
+        // Create identifier of a variable length.
+        std::vector<uint8_t> identifier(random() % 14 + 6);
+        util::fillRandom(identifier.begin(), identifier.end());
+
+        // HW address is a special case, because it must contain colons
+        // between consecutive octets.
+        HWAddrPtr hwaddr;
+        if (type == Host::IDENT_HWADDR) {
+            hwaddr.reset(new HWAddr(identifier, HTYPE_ETHER));
+        }
 
-    // Verify that HW address is correct.
-    HWAddrPtr hw_addr = host->getHWAddress();
-    ASSERT_TRUE(hw_addr);
-    EXPECT_EQ("hwtype=1 09:08:07:06:05:04", hw_addr->toText());
-    // DUID should be not set.
-    EXPECT_FALSE(host->getDuid());
+        // Convert identifier to hexadecimal representation.
+        const std::string identifier_hex = (hwaddr ?
+                                            hwaddr->toText(false) :
+                                            util::encode::encodeHex(identifier));
+        const std::string identifier_name = Host::getIdentifierName(type);
+
+        // Try to create Host instance.
+        ASSERT_NO_THROW(host.reset(new Host(identifier_hex, identifier_name,
+                                            SubnetID(10), SubnetID(20),
+                                            IOAddress("192.0.2.5"),
+                                            "me.example.org")))
+            << "test failed for " << identifier_name << "="
+            << identifier_hex;
+
+        // Retrieve the identifier from the Host instance and verify if it
+        // is correct.
+        const std::vector<uint8_t>& identifier_returned = host->getIdentifier();
+        EXPECT_TRUE(identifier_returned == identifier);
+        EXPECT_EQ(type, host->getIdentifierType());
+
+        EXPECT_EQ(10, host->getIPv4SubnetID());
+        EXPECT_EQ(20, host->getIPv6SubnetID());
+        EXPECT_EQ("192.0.2.5", host->getIPv4Reservation().toText());
+        EXPECT_EQ("me.example.org", host->getHostname());
+    }
 }
 
-// Test that it is possible to replace an identifier for a particular
-// Host instance (HW address -> DUID and vice versa) with the new
-// identifier in the binary format.
-TEST(HostTest, setIdentifierBinary) {
+// This test verifies that it is possible to create Host instance using
+// identifiers specified as text in quotes.
+TEST_F(HostTest, createFromIdentifierString) {
     boost::scoped_ptr<Host> host;
-    ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address",
-                                        SubnetID(1), SubnetID(2),
-                                        IOAddress("192.0.2.3"),
-                                        "me.example.com")));
-    // Initially, there should be a HW address, but not a DUID set.
-    ASSERT_TRUE(host->getHWAddress());
-    ASSERT_FALSE(host->getDuid());
-
-    // Now, use a DUID as identifier.
-    const uint8_t duid_data[] = {
-        0xaa, 0xbb, 0xcc, 0xdd, 0xee
-    };
-    ASSERT_NO_THROW(host->setIdentifier(duid_data, sizeof(duid_data),
-                                        Host::IDENT_DUID));
+    // It is not allowed to specify HW address or DUID as a string in quotes.
+    for (unsigned int i = 2; i < identifierTypeUpperBound(); ++i) {
+        const Host::IdentifierType type = static_cast<Host::IdentifierType>(i);
+        const std::string identifier_name = Host::getIdentifierName(type);
+
+        // Construct unique identifier for a host. This is a string
+        // consisting of a word "idenetifier", hyphen and the name of
+        // the identifier, e.g. "identifier-hw-address".
+        std::ostringstream identifier_without_quotes;
+        identifier_without_quotes << "identifier-" << identifier_name;
+
+        // Insert quotes to the identifier to indicate to the Host
+        // constructor that it is encoded as a text.
+        std::ostringstream identifier;
+        identifier << "'" << identifier_without_quotes.str() << "'";
+
+        ASSERT_NO_THROW(host.reset(new Host(identifier.str(), identifier_name,
+                                            SubnetID(10), SubnetID(20),
+                                            IOAddress("192.0.2.5"),
+                                            "me.example.org")))
+            << "test failed for " << identifier_name << "="
+            << identifier.str();
+
+        // Get the identifier from the Host and convert it back to the string
+        // format, so as it can be compared with the identifier used during
+        // Host object construction.
+        const std::vector<uint8_t>& identifier_returned = host->getIdentifier();
+        const std::string identifier_returned_str(identifier_returned.begin(),
+                                                  identifier_returned.end());
+        // Exclude quotes in comparison. Quotes should have been removed.
+        EXPECT_EQ(identifier_without_quotes.str(), identifier_returned_str);
+        EXPECT_EQ(type, host->getIdentifierType());
+
+        EXPECT_EQ(10, host->getIPv4SubnetID());
+        EXPECT_EQ(20, host->getIPv6SubnetID());
+        EXPECT_EQ("192.0.2.5", host->getIPv4Reservation().toText());
+        EXPECT_EQ("me.example.org", host->getHostname());
+    }
+}
 
-    // Verify that the DUID is correct.
-    DuidPtr duid = host->getDuid();
-    ASSERT_TRUE(duid);
-    EXPECT_EQ("aa:bb:cc:dd:ee", duid->toText());
-    // HW address should be not set.
-    EXPECT_FALSE(host->getHWAddress());
+// This test verifies that it is possible to override a host identifier
+// using setIdentifier method with an identifier specified in
+// hexadecimal format.
+TEST_F(HostTest, setIdentifierHex) {
+    boost::scoped_ptr<Host> host;
+    // Iterate over all supported identifiers.
+    for (unsigned int i = 0; i < identifierTypeUpperBound(); ++i) {
+
+        // In order to test that setIdentifier replaces an existing
+        // identifier we have to initialize Host with a different
+        // identifier first. We pick the next identifier after the
+        // one we want to set. If 'i' points to the last one, we
+        // use the first one.
+        unsigned int j = (i + 1) % identifierTypeUpperBound();
+
+        Host::IdentifierType type = static_cast<Host::IdentifierType>(j);
+        // Create identifier of a variable length.
+        std::vector<uint8_t> identifier(random() % 14 + 6);
+        util::fillRandom(identifier.begin(), identifier.end());
+
+        // HW address is a special case, because it must contain colons
+        // between consecutive octets.
+        HWAddrPtr hwaddr;
+        if (type == Host::IDENT_HWADDR) {
+            hwaddr.reset(new HWAddr(identifier, HTYPE_ETHER));
+        }
 
-    // Now, let's do another way around.
+        // Convert identifier to hexadecimal representation.
+        std::string identifier_hex = (hwaddr ?
+                                      hwaddr->toText(false) :
+                                      util::encode::encodeHex(identifier));
+        std::string identifier_name = Host::getIdentifierName(type);
+
+        // Try to create Host instance.
+        ASSERT_NO_THROW(host.reset(new Host(identifier_hex, identifier_name,
+                                            SubnetID(10), SubnetID(20),
+                                            IOAddress("192.0.2.5"),
+                                            "me.example.org")))
+            << "test failed for " << identifier_name << "="
+            << identifier_hex;
+
+        // Retrieve the identifier from the Host instance and verify if it
+        // is correct.
+        std::vector<uint8_t> identifier_returned = host->getIdentifier();
+        EXPECT_TRUE(identifier_returned == identifier);
+        EXPECT_EQ(type, host->getIdentifierType());
+
+        EXPECT_EQ(10, host->getIPv4SubnetID());
+        EXPECT_EQ(20, host->getIPv6SubnetID());
+        EXPECT_EQ("192.0.2.5", host->getIPv4Reservation().toText());
+        EXPECT_EQ("me.example.org", host->getHostname());
+
+        // Now use another identifier.
+        type = static_cast<Host::IdentifierType>(i);
+        // Create identifier of a variable length.
+        identifier.resize(random() % 14 + 6);
+        util::fillRandom(identifier.begin(), identifier.end());
+
+        hwaddr.reset();
+        if (type == Host::IDENT_HWADDR) {
+            hwaddr.reset(new HWAddr(identifier, HTYPE_ETHER));
+        }
 
-    const uint8_t hwaddr_data[] = {
-        9, 8, 7, 6, 5, 4
-    };
-    ASSERT_NO_THROW(host->setIdentifier(hwaddr_data, sizeof(hwaddr_data),
-                                        Host::IDENT_HWADDR));
-
-    // Verify that HW address is correct.
-    HWAddrPtr hw_addr = host->getHWAddress();
-    ASSERT_TRUE(hw_addr);
-    EXPECT_EQ("hwtype=1 09:08:07:06:05:04", hw_addr->toText());
-    // DUID should be not set.
-    EXPECT_FALSE(host->getDuid());
+        // Convert identifier to hexadecimal representation.
+        identifier_hex = (hwaddr ? hwaddr->toText(false) :
+                          util::encode::encodeHex(identifier));
+        identifier_name = Host::getIdentifierName(type);
+
+        // Try to replace identifier for a host.
+        ASSERT_NO_THROW(host->setIdentifier(identifier_hex, identifier_name))
+            << "test failed for " << identifier_name << "="
+            << identifier_hex;
+
+        // Retrieve the identifier from the Host instance and verify if it
+        // is correct.
+        identifier_returned = host->getIdentifier();
+        EXPECT_TRUE(identifier_returned == identifier);
+        EXPECT_EQ(type, host->getIdentifierType());
+
+        EXPECT_EQ(10, host->getIPv4SubnetID());
+        EXPECT_EQ(20, host->getIPv6SubnetID());
+        EXPECT_EQ("192.0.2.5", host->getIPv4Reservation().toText());
+        EXPECT_EQ("me.example.org", host->getHostname());
+    }
 }
 
-/// @brief Checks if the reservation is in the range of reservations.
-///
-/// @param resrv Reservation to be searched for.
-/// @param range Range of reservations returned by the @c Host object
-/// in which the reservation will be searched.
-bool
-reservationExists(const IPv6Resrv& resrv, const IPv6ResrvRange& range) {
-    for (IPv6ResrvIterator it = range.first; it != range.second;
-         ++it) {
-        if (resrv == it->second) {
-            return (true);
-        }
+// This test verifies that it is possible to override a host identifier
+// using setIdentifier method with an identifier specified in binary
+// format.
+TEST_F(HostTest, setIdentifierBinary) {
+    boost::scoped_ptr<Host> host;
+    // Iterate over all supported identifier types.
+    for (unsigned int i = 0; i < identifierTypeUpperBound(); ++i) {
+
+        // In order to test that setIdentifier replaces an existing
+        // identifier we have to initialize Host with a different
+        // identifier first. We pick the next identifier after the
+        // one we want to set. If 'i' points to the last one, we
+        // use the first one.
+        unsigned int j = (i + 1) % identifierTypeUpperBound();
+
+        Host::IdentifierType type = static_cast<Host::IdentifierType>(j);
+        // Create identifier of variable length and fill with random values.
+        std::vector<uint8_t> identifier(random() % 14 + 6);
+        util::fillRandom(identifier.begin(), identifier.end());
+
+        // Try to create a Host instance using this identifier.
+        ASSERT_NO_THROW(host.reset(new Host(&identifier[0], identifier.size(),
+                                            type, SubnetID(10), SubnetID(20),
+                                            IOAddress("192.0.2.5"),
+                                            "me.example.org")));
+
+        // Retrieve identifier from Host instance and check if it is correct.
+        std::vector<uint8_t> identifier_returned = host->getIdentifier();
+        EXPECT_TRUE(identifier_returned == identifier);
+        EXPECT_EQ(type, host->getIdentifierType());
+
+        EXPECT_EQ(10, host->getIPv4SubnetID());
+        EXPECT_EQ(20, host->getIPv6SubnetID());
+        EXPECT_EQ("192.0.2.5", host->getIPv4Reservation().toText());
+        EXPECT_EQ("me.example.org", host->getHostname());
+
+        type = static_cast<Host::IdentifierType>(i);
+        // Create identifier of variable length and fill with random values.
+        identifier.resize(random() % 14 + 6);
+        util::fillRandom(identifier.begin(), identifier.end());
+
+        // Try to set new identifier.
+        ASSERT_NO_THROW(host->setIdentifier(&identifier[0], identifier.size(),
+                                            type));
+
+        // Retrieve identifier from Host instance and check if it is correct.
+        identifier_returned = host->getIdentifier();
+        EXPECT_TRUE(identifier_returned == identifier);
+        EXPECT_EQ(type, host->getIdentifierType());
+
+        EXPECT_EQ(10, host->getIPv4SubnetID());
+        EXPECT_EQ(20, host->getIPv6SubnetID());
+        EXPECT_EQ("192.0.2.5", host->getIPv4Reservation().toText());
+        EXPECT_EQ("me.example.org", host->getHostname());
     }
-    return (false);
 }
 
 // This test verifies that the IPv6 reservations of a different type can
 // be added for the host.
-TEST(HostTest, addReservations) {
+TEST_F(HostTest, addReservations) {
     boost::scoped_ptr<Host> host;
     ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address",
                                         SubnetID(1), SubnetID(2),
@@ -397,7 +616,7 @@ TEST(HostTest, addReservations) {
 
 // This test checks that various modifiers may be used to replace the current
 // values of the Host class.
-TEST(HostTest, setValues) {
+TEST_F(HostTest, setValues) {
     boost::scoped_ptr<Host> host;
     ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address",
                                         SubnetID(1), SubnetID(2),
@@ -436,7 +655,7 @@ TEST(HostTest, setValues) {
 }
 
 // Test that Host constructors initialize client classes from string.
-TEST(HostTest, clientClassesFromConstructor) {
+TEST_F(HostTest, clientClassesFromConstructor) {
     boost::scoped_ptr<Host> host;
     // Prepare the hardware address in binary format.
     const uint8_t hwaddr_data[] = {
@@ -477,7 +696,7 @@ TEST(HostTest, clientClassesFromConstructor) {
 }
 
 // Test that new client classes can be added for the Host.
-TEST(HostTest, addClientClasses) {
+TEST_F(HostTest, addClientClasses) {
     boost::scoped_ptr<Host> host;
     ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address",
                                         SubnetID(1), SubnetID(2),
@@ -504,7 +723,7 @@ TEST(HostTest, addClientClasses) {
 }
 
 // This test checks that it is possible to add DHCPv4 options for a host.
-TEST(HostTest, addOptions4) {
+TEST_F(HostTest, addOptions4) {
     Host host("01:02:03:04:05:06", "hw-address", SubnetID(1), SubnetID(2),
               IOAddress("192.0.2.3"));
 
@@ -568,7 +787,7 @@ TEST(HostTest, addOptions4) {
 }
 
 // This test checks that it is possible to add DHCPv6 options for a host.
-TEST(HostTest, addOptions6) {
+TEST_F(HostTest, addOptions6) {
     Host host("01:02:03:04:05:06", "hw-address", SubnetID(1), SubnetID(2),
               IOAddress("192.0.2.3"));
 
@@ -633,22 +852,38 @@ TEST(HostTest, addOptions6) {
 
 // This test verifies that it is possible to retrieve a textual
 // representation of the host identifier.
-TEST(HostTest, getIdentifierAsText) {
+TEST_F(HostTest, getIdentifierAsText) {
+    // HW address
     Host host1("01:02:03:04:05:06", "hw-address",
                SubnetID(1), SubnetID(2),
                IOAddress("192.0.2.3"));
     EXPECT_EQ("hwaddr=010203040506", host1.getIdentifierAsText());
 
+    // DUID
     Host host2("0a:0b:0c:0d:0e:0f:ab:cd:ef", "duid",
                SubnetID(1), SubnetID(2),
                IOAddress("192.0.2.3"));
     EXPECT_EQ("duid=0A0B0C0D0E0FABCDEF",
               host2.getIdentifierAsText());
+
+    // Circuit id.
+    Host host3("'marcin's-home'", "circuit-id",
+               SubnetID(1), SubnetID(2),
+               IOAddress("192.0.2.3"));
+    EXPECT_EQ("circuit-id=6D617263696E27732D686F6D65",
+              host3.getIdentifierAsText());
+}
+
+// This test verifies that conversion of the identifier type to a
+// name works correctly.
+TEST_F(HostTest, getIdentifierName) {
+    EXPECT_EQ("hw-address", Host::getIdentifierName(Host::IDENT_HWADDR));
+
 }
 
 // This test checks that Host object is correctly described in the
 // textual format using the toText method.
-TEST(HostTest, toText) {
+TEST_F(HostTest, toText) {
     boost::scoped_ptr<Host> host;
     ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address",
                                         SubnetID(1), SubnetID(2),
@@ -720,7 +955,7 @@ TEST(HostTest, toText) {
 }
 
 // Test verifies if the host can store HostId properly.
-TEST(HostTest, hostId) {
+TEST_F(HostTest, hostId) {
     boost::scoped_ptr<Host> host;
     ASSERT_NO_THROW(host.reset(new Host("01:02:03:04:05:06", "hw-address",
                                         SubnetID(1), SubnetID(2),

+ 16 - 10
src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc

@@ -190,37 +190,43 @@ TEST(MySqlConnection, checkTimeConversion) {
 // Test verifies if a host reservation can be added and later retrieved by IPv4
 // address. Host uses hw address as identifier.
 TEST_F(MySqlHostDataSourceTest, basic4HWAddr) {
-    testBasic4(true);
+    testBasic4(Host::IDENT_HWADDR);
 }
 
 // Test verifies if a host reservation can be added and later retrieved by IPv4
 // address. Host uses client-id (DUID) as identifier.
 TEST_F(MySqlHostDataSourceTest, basic4ClientId) {
-    testBasic4(false);
+    testBasic4(Host::IDENT_DUID);
 }
 
 // Test verifies that multiple hosts can be added and later retrieved by their
 // reserved IPv4 address. This test uses HW addresses as identifiers.
 TEST_F(MySqlHostDataSourceTest, getByIPv4HWaddr) {
-    testGetByIPv4(true);
+    testGetByIPv4(Host::IDENT_HWADDR);
 }
 
 // Test verifies that multiple hosts can be added and later retrieved by their
 // reserved IPv4 address. This test uses client-id (DUID) as identifiers.
 TEST_F(MySqlHostDataSourceTest, getByIPv4ClientId) {
-    testGetByIPv4(false);
+    testGetByIPv4(Host::IDENT_DUID);
 }
 
 // Test verifies if a host reservation can be added and later retrieved by
 // hardware address.
 TEST_F(MySqlHostDataSourceTest, get4ByHWaddr) {
-    testGet4ByHWAddr();
+    testGet4ByIdentifier(Host::IDENT_HWADDR);
 }
 
 // Test verifies if a host reservation can be added and later retrieved by
-// client identifier.
-TEST_F(MySqlHostDataSourceTest, get4ByClientId) {
-    testGet4ByClientId();
+// DUID.
+TEST_F(MySqlHostDataSourceTest, get4ByDUID) {
+    testGet4ByIdentifier(Host::IDENT_DUID);
+}
+
+// Test verifies if a host reservation can be added and later retrieved by
+// circuit id.
+TEST_F(MySqlHostDataSourceTest, get4ByCircuitId) {
+    testGet4ByIdentifier(Host::IDENT_CIRCUIT_ID);
 }
 
 // Test verifies if hardware address and client identifier are not confused.
@@ -355,7 +361,7 @@ TEST_F(MySqlHostDataSourceTest, DISABLED_multipleClientClassesBoth) {
 // hardware address), but for different subnets (different subnet-ids).
 // Make sure that getAll() returns them all correctly.
 TEST_F(MySqlHostDataSourceTest, multipleSubnetsHWAddr) {
-    testMultipleSubnets(10, true);
+    testMultipleSubnets(10, Host::IDENT_HWADDR);
 }
 
 // Test if the same host can have reservations in different subnets (with the
@@ -365,7 +371,7 @@ TEST_F(MySqlHostDataSourceTest, multipleSubnetsHWAddr) {
 // client-identifier), but for different subnets (different subnet-ids).
 // Make sure that getAll() returns them correctly.
 TEST_F(MySqlHostDataSourceTest, multipleSubnetsClientId) {
-    testMultipleSubnets(10, false);
+    testMultipleSubnets(10, Host::IDENT_DUID);
 }
 
 // Test if host reservations made for different IPv6 subnets are handled correctly.

+ 19 - 1
src/lib/util/strutil.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-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
@@ -133,6 +133,24 @@ getToken(std::istringstream& iss) {
     return (token);
 }
 
+std::vector<uint8_t>
+quotedStringToBinary(const std::string& quoted_string) {
+    std::vector<uint8_t> binary;
+    // Remove whitespace before and after the quotes.
+    std::string trimmed_string = trim(quoted_string);
+
+    // We require two quote characters, so the length of the string must be
+    // equal to 2 at minimum, and it must start and end with quotes.
+    if ((trimmed_string.length() > 1) && ((trimmed_string[0] == '\'') &&
+        (trimmed_string[trimmed_string.length()-1] == '\''))) {
+        // Remove quotes and trim the text inside the quotes.
+        trimmed_string = trim(trimmed_string.substr(1, trimmed_string.length() - 2));
+        // Copy string contents into the vector.
+        binary.assign(trimmed_string.begin(), trimmed_string.end());
+    }
+    // Return resulting vector or empty vector.
+    return (binary);
+}
 
 } // namespace str
 } // namespace util

+ 20 - 1
src/lib/util/strutil.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-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
@@ -194,6 +194,25 @@ tokenToNum(const std::string& num_token) {
     return (num);
 }
 
+/// \brief Converts a string in quotes into vector.
+///
+/// A converted string is first trimmed. If a trimmed string is in
+/// quotes, the quotes are removed and the resulting string is copied
+/// into a vector. If the string is not in quotes, an empty vector is
+/// returned.
+///
+/// The resulting string is copied to a vector and returned.
+///
+/// This function is intended to be used by the server configuration
+/// parsers to convert string values surrounded with quotes into
+/// binary form.
+///
+/// \param quoted_string String to be converted.
+/// \return Vector containing converted string or empty string if
+/// input string didn't contain expected quote characters.
+std::vector<uint8_t>
+quotedStringToBinary(const std::string& quoted_string);
+
 } // namespace str
 } // namespace util
 } // namespace isc

+ 39 - 1
src/lib/util/tests/strutil_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-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
@@ -259,3 +259,41 @@ TEST(StringUtilTest, tokenToNum) {
                  isc::util::str::StringTokenError);
 
 }
+
+/// @brief Convenience function which calls quotedStringToBinary
+/// and converts returned vector back to string.
+///
+/// @param s Input string.
+/// @return String holding a copy of a vector returned by the
+/// quotedStringToBinary.
+std::string testQuoted(const std::string& s) {
+    std::vector<uint8_t> vec = str::quotedStringToBinary(s);
+    std::string s2(vec.begin(), vec.end());
+    return (s2);
+}
+
+TEST(StringUtilTest, quotedStringToBinary) {
+    // No opening or closing quote should result in empty string.
+    EXPECT_TRUE(str::quotedStringToBinary("'").empty());
+    EXPECT_TRUE(str::quotedStringToBinary("").empty());
+    EXPECT_TRUE(str::quotedStringToBinary("  ").empty());
+    EXPECT_TRUE(str::quotedStringToBinary("'circuit id").empty());
+    EXPECT_TRUE(str::quotedStringToBinary("circuit id'").empty());
+
+    // If there is only opening and closing quote, an empty
+    // vector should be returned.
+    EXPECT_TRUE(str::quotedStringToBinary("''").empty());
+
+    // Both opening and ending quote is present.
+    EXPECT_EQ("circuit id", testQuoted("'circuit id'"));
+    EXPECT_EQ("remote id", testQuoted("  ' remote id'"));
+    EXPECT_EQ("duid", testQuoted("  ' duid'"));
+    EXPECT_EQ("duid", testQuoted("'duid    '  "));
+    EXPECT_EQ("remote'id", testQuoted("  ' remote'id  '"));
+    EXPECT_EQ("remote id'", testQuoted("'remote id''"));
+    EXPECT_EQ("'remote id", testQuoted("''remote id'"));
+
+    // Multiple quotes.
+    EXPECT_EQ("'", testQuoted("'''"));
+    EXPECT_EQ("''", testQuoted("''''"));
+}