[4552] Addressed review comments.

4 files changed, 68 insertions, 11 deletions

diff --git a/src/lib/dhcpsrv/host.cc b/src/lib/dhcpsrv/host.cc
index 70211e903b..8b3bbb3e1e 100644
--- a/src/lib/dhcpsrv/host.cc
+++ b/src/lib/dhcpsrv/host.cc
@@ -5,6 +5,7 @@
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 #include <config.h>
+#include <dhcp/pkt4.h>
 #include <dhcpsrv/host.h>
 #include <util/encode/hex.h>
 #include <util/strutil.h>
@@ -365,7 +366,7 @@ Host::setNextServer(const asiolink::IOAddress& next_server) {
     if (!next_server.isV4()) {
         isc_throw(isc::BadValue, "next server address '" << next_server
                   << "' is not a valid IPv4 address");
-    } else if (next_server.isV4Zero() || next_server.isV4Bcast()) {
+    } else if (next_server.isV4Bcast()) {
         isc_throw(isc::BadValue, "invalid next server address '"
                   << next_server << "'");
     }
@@ -373,6 +374,24 @@ Host::setNextServer(const asiolink::IOAddress& next_server) {
     next_server_ = next_server;
 }
 
+void
+Host::setServerHostname(const std::string& server_host_name) {
+    if (server_host_name.size() > Pkt4::MAX_SNAME_LEN - 1) {
+        isc_throw(isc::BadValue, "server hostname length must not exceed "
+                  << (Pkt4::MAX_SNAME_LEN - 1));
+    }
+    server_host_name_ = server_host_name;
+}
+
+void
+Host::setBootFileName(const std::string& boot_file_name) {
+    if (boot_file_name.size() > Pkt4::MAX_FILE_LEN - 1) {
+        isc_throw(isc::BadValue, "boot file length must not exceed "
+                  << (Pkt4::MAX_FILE_LEN - 1));
+    }
+    boot_file_name_ = boot_file_name;
+}
+
 std::string
 Host::toText() const {
     std::ostringstream s;
diff --git a/src/lib/dhcpsrv/host.h b/src/lib/dhcpsrv/host.h
index 5a912346d1..fa080e67ce 100644
--- a/src/lib/dhcpsrv/host.h
+++ b/src/lib/dhcpsrv/host.h
@@ -466,7 +466,7 @@ public:
     /// @param next_server New address of a next server.
     ///
     /// @throw isc::BadValue if the provided address is not an IPv4 address,
-    /// is a 0 address or broadcast address.
+    /// is broadcast address.
     void setNextServer(const asiolink::IOAddress& next_server);
 
     /// @brief Returns value of next server field (siaddr).
@@ -477,9 +477,9 @@ public:
     /// @brief Sets new value for server hostname (sname).
     ///
     /// @param server_host_name New value for server hostname.
-    void setServerHostname(const std::string& server_host_name) {
-        server_host_name_ = server_host_name;
-    }
+    ///
+    /// @throw BadValue if hostname is longer than 63 bytes.
+    void setServerHostname(const std::string& server_host_name);
 
     /// @brief Returns value of server hostname (sname).
     const std::string& getServerHostname() const {
@@ -489,9 +489,9 @@ public:
     /// @brief Sets new value for boot file name (file).
     ///
     /// @param boot_file_name New value of boot file name.
-    void setBootFileName(const std::string& boot_file_name) {
-        boot_file_name_ = boot_file_name;
-    }
+    ///
+    /// @throw BadValue if boot file name is longer than 128 bytes.
+    void setBootFileName(const std::string& boot_file_name);
 
     /// @brief Returns value of boot file name (file).
     const std::string& getBootFileName() const {
diff --git a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc
index c90e309912..f083299927 100644
--- a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc
@@ -20,6 +20,7 @@
 #include <boost/pointer_cast.hpp>
 #include <gtest/gtest.h>
 #include <iterator>
+#include <sstream>
 #include <string>
 #include <vector>
 
@@ -327,6 +328,43 @@ TEST_F(HostReservationParserTest, dhcp4MessageFields) {
     EXPECT_EQ("/tmp/some-file.efi", hosts[0]->getBootFileName());
 }
 
+// This test verifies that the invalid value of the next server is rejected.
+TEST_F(HostReservationParserTest, invalidNextServer) {
+    // Invalid IPv4 address.
+    std::string config = "{ \"hw-address\": \"1:2:3:4:5:6\","
+        "\"next-server\": \"192.0.2.foo\" }";
+    testInvalidConfig<HostReservationParser4>(config);
+
+    // Broadcast address.
+    config = "{ \"hw-address\": \"1:2:3:4:5:6\","
+        "\"next-server\": \"255.255.255.255\" }";
+    testInvalidConfig<HostReservationParser4>(config);
+
+    // IPv6 address.
+    config = "{ \"hw-address\": \"1:2:3:4:5:6\","
+        "\"next-server\": \"2001:db8:1::1\" }";
+    testInvalidConfig<HostReservationParser4>(config);
+}
+
+// This test verifies that the invalid server hostname is rejected.
+TEST_F(HostReservationParserTest, invalidServerHostname) {
+    std::ostringstream config;
+    config << "{ \"hw-address\": \"1:2:3:4:5:6\","
+        "\"server-hostname\": \"";
+    config << std::string(64, 'a');
+    config << "\" }";
+    testInvalidConfig<HostReservationParser4>(config.str());
+}
+
+// This test verifies that the invalid boot file name is rejected.
+TEST_F(HostReservationParserTest, invalidBootFileName) {
+    std::ostringstream config;
+    config << "{ \"hw-address\": \"1:2:3:4:5:6\","
+        "\"boot-file-name\": \"";
+    config << std::string(128, 'a');
+    config << "\" }";
+    testInvalidConfig<HostReservationParser4>(config.str());
+}
 
 // This test verifies that the configuration parser for host reservations
 // throws an exception when IPv6 address is specified for IPv4 address
diff --git a/src/lib/dhcpsrv/tests/host_unittest.cc b/src/lib/dhcpsrv/tests/host_unittest.cc
index 195b7766cd..e51733bbb1 100644
--- a/src/lib/dhcpsrv/tests/host_unittest.cc
+++ b/src/lib/dhcpsrv/tests/host_unittest.cc
@@ -686,11 +686,11 @@ TEST_F(HostTest, setValues) {
     EXPECT_THROW(host->setIPv4Reservation(IOAddress::IPV4_BCAST_ADDRESS()),
                  isc::BadValue);
 
-    // Zero or broadcast are invalid addresses for next server.
-    EXPECT_THROW(host->setNextServer(asiolink::IOAddress::IPV4_ZERO_ADDRESS()),
-                 isc::BadValue);
+    // Broadcast and IPv6 are invalid addresses for next server.
     EXPECT_THROW(host->setNextServer(asiolink::IOAddress::IPV4_BCAST_ADDRESS()),
                                      isc::BadValue);
+    EXPECT_THROW(host->setNextServer(IOAddress("2001:db8:1::1")),
+                                     isc::BadValue);
 }
 
 // Test that Host constructors initialize client classes from string.