[5145b] Reverted trac5126 merge

configure.ac

@@ -201,21 +201,6 @@ for retry in "none" "--std=c++11" "--std=c++0x" "--std=c++1x" "fail"; do
 		[AC_MSG_RESULT([no])
 		 continue])
 
-	AC_MSG_CHECKING(template alias)
-	feature="template alias"
-	AC_COMPILE_IFELSE(
-		[AC_LANG_PROGRAM(
-			[template<int i>
-			 class I {
-			 public: int get() { return i; };
-			 };
-			 using Zero = I<0>;],
-			[Zero Z;
-			 return Z.get();])],
-		[AC_MSG_RESULT([yes])],
-		[AC_MSG_RESULT([no])
-		 continue])
-
 	AC_MSG_CHECKING(lambda support)
 	feature="lambda"
 	AC_COMPILE_IFELSE(
@@ -1482,11 +1467,11 @@ AC_ARG_ENABLE(generate_parser, [AC_HELP_STRING([--enable-generate-parser],
    enable_generate_parser=$enableval, enable_generate_parser=no)
 
 # Check if flex is avaible. Flex is not needed for building Kea sources,
-# unless you want to regenerate grammars
+# unless you want to regenerate grammar in src/lib/eval
 AC_PROG_LEX
 
 # Check if bison is available. Bison is not needed for building Kea sources,
-# unless you want to regenerate grammars
+# unless you want to regenerate grammar in src/lib/eval
 AC_PROG_YACC
 
 if test "x$enable_generate_parser" != "xno"; then
@@ -1500,7 +1485,7 @@ if test "x$enable_generate_parser" != "xno"; then
     fi
 
 # Ok, let's check if we have at least 3.0.0 version of the bison. The code used
-# to generate parsers is roughly based on bison 3.0 examples.
+# to generate src/lib/eval parser is roughly based on bison 3.0 examples.
    cat > bisontest.y << EOF
 %require "3.0.0"
 %token X
@@ -1614,28 +1599,20 @@ AM_COND_IF([HAVE_OPTRESET], [AC_DEFINE([HAVE_OPTRESET], [1], [Check for optreset
 
 AC_DEFINE([CONFIG_H_WAS_INCLUDED], [1], [config.h inclusion marker])
 
-AC_CONFIG_FILES([Makefile
-                 compatcheck/Makefile
+AC_CONFIG_FILES([compatcheck/Makefile
                  dns++.pc
-                 doc/Makefile
-                 doc/design/Makefile
                  doc/design/datasrc/Makefile
+                 doc/design/Makefile
                  doc/guide/Makefile
+                 doc/Makefile
                  doc/version.ent
-                 ext/Makefile
                  ext/coroutine/Makefile
                  ext/gtest/Makefile
+                 ext/Makefile
                  m4macros/Makefile
+                 Makefile
                  src/Makefile
                  src/bin/Makefile
-                 src/bin/admin/Makefile
-                 src/bin/admin/kea-admin
-                 src/bin/admin/tests/Makefile
-                 src/bin/admin/tests/cql_tests.sh
-                 src/bin/admin/tests/data/Makefile
-                 src/bin/admin/tests/memfile_tests.sh
-                 src/bin/admin/tests/mysql_tests.sh
-                 src/bin/admin/tests/pgsql_tests.sh
                  src/bin/agent/Makefile
                  src/bin/agent/tests/Makefile
                  src/bin/agent/tests/ca_process_tests.sh
@@ -1667,6 +1644,15 @@ AC_CONFIG_FILES([Makefile
                  src/bin/perfdhcp/Makefile
                  src/bin/perfdhcp/tests/Makefile
                  src/bin/perfdhcp/tests/testdata/Makefile
+                 src/bin/admin/Makefile
+                 src/bin/admin/kea-admin
+                 src/bin/admin/tests/Makefile
+                 src/bin/admin/tests/data/Makefile
+                 src/bin/admin/tests/memfile_tests.sh
+                 src/bin/admin/tests/mysql_tests.sh
+                 src/bin/admin/tests/pgsql_tests.sh
+                 src/bin/admin/tests/cql_tests.sh
+                 src/bin/agent/tests/test_libraries.h
                  src/hooks/Makefile
                  src/hooks/dhcp/Makefile
                  src/hooks/dhcp/user_chk/Makefile
@@ -1699,8 +1685,6 @@ AC_CONFIG_FILES([Makefile
                  src/lib/dns/gen-rdatacode.py
                  src/lib/dns/tests/Makefile
                  src/lib/dns/tests/testdata/Makefile
-                 src/lib/eval/Makefile
-                 src/lib/eval/tests/Makefile
                  src/lib/exceptions/Makefile
                  src/lib/exceptions/tests/Makefile
                  src/lib/hooks/Makefile
@@ -1726,10 +1710,10 @@ AC_CONFIG_FILES([Makefile
                  src/lib/process/spec_config.h.pre
                  src/lib/process/tests/Makefile
                  src/lib/process/testutils/Makefile
-                 src/lib/stats/Makefile
-                 src/lib/stats/tests/Makefile
                  src/lib/testutils/Makefile
                  src/lib/testutils/dhcp_test_lib.sh
+                 src/lib/stats/Makefile
+                 src/lib/stats/tests/Makefile
                  src/lib/util/Makefile
                  src/lib/util/io/Makefile
                  src/lib/util/python/Makefile
@@ -1739,10 +1723,11 @@ AC_CONFIG_FILES([Makefile
                  src/lib/util/threads/Makefile
                  src/lib/util/threads/tests/Makefile
                  src/lib/util/unittests/Makefile
+                 src/lib/eval/Makefile
+                 src/lib/eval/tests/Makefile
                  src/share/Makefile
                  src/share/database/Makefile
                  src/share/database/scripts/Makefile
-                 src/share/database/scripts/cql/Makefile
                  src/share/database/scripts/mysql/Makefile
                  src/share/database/scripts/mysql/upgrade_1.0_to_2.0.sh
                  src/share/database/scripts/mysql/upgrade_2.0_to_3.0.sh
@@ -1752,16 +1737,17 @@ AC_CONFIG_FILES([Makefile
                  src/share/database/scripts/pgsql/Makefile
                  src/share/database/scripts/pgsql/upgrade_1.0_to_2.0.sh
                  src/share/database/scripts/pgsql/upgrade_2.0_to_3.0.sh
+                 src/share/database/scripts/cql/Makefile
                  tools/Makefile
                  tools/path_replacer.sh
 ])
 
-AC_CONFIG_COMMANDS([permissions], [
-           chmod +x src/bin/admin/kea-admin
+ AC_CONFIG_COMMANDS([permissions], [
            chmod +x src/bin/dhcp4/tests/dhcp4_process_tests.sh
            chmod +x src/bin/dhcp6/tests/dhcp6_process_tests.sh
            chmod +x src/bin/keactrl/keactrl
            chmod +x src/bin/keactrl/tests/keactrl_tests.sh
+           chmod +x src/bin/admin/kea-admin
            chmod +x src/lib/dns/gen-rdatacode.py
            chmod +x src/lib/log/tests/console_test.sh
            chmod +x src/lib/log/tests/destination_test.sh

src/bin/dhcp4/dhcp4to6_ipc.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-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
@@ -35,7 +35,7 @@ Dhcp4to6Ipc& Dhcp4to6Ipc::instance() {
 }
 
 void Dhcp4to6Ipc::open() {
-    uint16_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port();
+    uint32_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port();
     if (port == 0) {
         Dhcp4o6IpcBase::close();
         return;
@@ -45,7 +45,8 @@ void Dhcp4to6Ipc::open() {
     }
 
     int old_fd = socket_fd_;
-    socket_fd_ = Dhcp4o6IpcBase::open(port, ENDPOINT_TYPE_V4);
+    socket_fd_ = Dhcp4o6IpcBase::open(static_cast<uint16_t>(port),
+                                      ENDPOINT_TYPE_V4);
     if ((old_fd == -1) && (socket_fd_ != old_fd)) {
         IfaceMgr::instance().addExternalSocket(socket_fd_,
                                                Dhcp4to6Ipc::handler);

src/bin/dhcp4/json_config_parser.cc

@@ -341,9 +341,24 @@ public:
         cfg->setDeclinePeriod(probation_period);
 
         // Set the DHCPv4-over-DHCPv6 interserver port.
-        uint16_t dhcp4o6_port = getUint16(global, "dhcp4o6-port");
+        // @todo Change for uint16_t
+        uint32_t dhcp4o6_port = getUint32(global, "dhcp4o6-port");
         cfg->setDhcp4o6Port(dhcp4o6_port);
     }
+
+private:
+
+    /// @brief Returns a value converted to uint32_t
+    ///
+    /// Instantiation of getIntType() to uint32_t
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return an uint32_t value
+    uint32_t getUint32(isc::data::ConstElementPtr scope,
+                       const std::string& name) {
+        return (getIntType<uint32_t>(scope, name));
+    }
 };
 
 } // anonymous namespace

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -4557,6 +4557,14 @@ TEST_F(Dhcp4ParserTest, invalidClientClassDictionary) {
         " } ] \n"
         "} \n";
 
+    ConstElementPtr json;
+    ASSERT_NO_THROW(json = parseJSON(config));
+
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 1);
+
     EXPECT_THROW(parseDHCP4(config), Dhcp4ParseError);
 }
 

src/bin/dhcp6/dhcp6to4_ipc.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-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
@@ -34,7 +34,7 @@ Dhcp6to4Ipc& Dhcp6to4Ipc::instance() {
 }
 
 void Dhcp6to4Ipc::open() {
-    uint16_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port();
+    uint32_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port();
     if (port == 0) {
         Dhcp4o6IpcBase::close();
         return;
@@ -44,7 +44,8 @@ void Dhcp6to4Ipc::open() {
     }
 
     int old_fd = socket_fd_;
-    socket_fd_ = Dhcp4o6IpcBase::open(port, ENDPOINT_TYPE_V6);
+    socket_fd_ = Dhcp4o6IpcBase::open(static_cast<uint16_t>(port),
+                                      ENDPOINT_TYPE_V6);
     if ((old_fd == -1) && (socket_fd_ != old_fd)) {
         IfaceMgr::instance().addExternalSocket(socket_fd_,
                                                Dhcp6to4Ipc::handler);

src/bin/dhcp6/json_config_parser.cc

@@ -214,6 +214,18 @@ public:
 
 private:
 
+    /// @brief Get an uint8_t value
+    ///
+    /// Instantiation of getIntType() to uint8_t
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return uint8_t value
+    /// @throw isc::dhcp::DhcpConfigError when it is not an uint8_t
+    uint8_t getUint8(ConstElementPtr scope, const std::string& name) {
+        return (getIntType<uint8_t>(scope, name));
+    }
+
     /// Pointer to the created pool object.
     isc::dhcp::Pool6Ptr pool_;
 
@@ -535,9 +547,24 @@ public:
         srv_config->setDeclinePeriod(probation_period);
 
         // Set the DHCPv4-over-DHCPv6 interserver port.
-        uint16_t dhcp4o6_port = getUint16(global, "dhcp4o6-port");
+        // @todo Change for uint16_t
+        uint32_t dhcp4o6_port = getUint32(global, "dhcp4o6-port");
         srv_config->setDhcp4o6Port(dhcp4o6_port);
     }
+
+private:
+
+    /// @brief Returns a value converted to uint32_t
+    ///
+    /// Instantiation of getIntType() to uint32_t
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return an uint32_t value
+    uint32_t getUint32(isc::data::ConstElementPtr scope,
+                       const std::string& name) {
+        return (getIntType<uint32_t>(scope, name));
+    }
 };
 
 } // anonymous namespace

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -4969,6 +4969,13 @@ TEST_F(Dhcp6ParserTest, invalidClientClassDictionary) {
         " } ] \n"
         "} \n";
 
+    ConstElementPtr json = parseJSON(config);
+
+    ConstElementPtr status;
+    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
+    ASSERT_TRUE(status);
+    checkResult(status, 1);
+
     EXPECT_THROW(parseDHCP6(config), Dhcp6ParseError);
 }
 

src/lib/cc/simple_parser.cc

@@ -74,7 +74,7 @@ SimpleParser::getPosition(const std::string& name, const data::ConstElementPtr p
     }
     ConstElementPtr elem = parent->get(name);
     if (!elem) {
-        return (parent->getPosition());
+        return (data::Element::ZERO_POSITION());
     }
     return (elem->getPosition());
 }

src/lib/cc/simple_parser.h

@@ -104,9 +104,9 @@ class SimpleParser {
 
     /// @brief Utility method that returns position of an element
     ///
-    /// It's mostly useful for logging. If the element is missing
-    /// the parent position is returned or ZERO_POSITION if parent
-    /// is null.
+    /// It's mostly useful for logging. When any necessary parameter is
+    /// missing (either parent is null or it doesn't contain specified
+    /// name) ZERO_POSITION is returned.
     ///
     /// @param name position of that element will be returned
     /// @param parent parent element (optional)
@@ -203,44 +203,6 @@ protected:
                       << "' (" << getPosition(name, scope) << ")");
         }
     }
-
-    /// @brief Returns a value converted to uint32_t
-    ///
-    /// Instantiation of getIntType() to uint32_t
-    ///
-    /// @param scope specified parameter will be extracted from this scope
-    /// @param name name of the parameter
-    /// @return an uint32_t value
-    /// @throw isc::dhcp::DhcpConfigError when it is not an uint32_t
-    uint32_t getUint32(isc::data::ConstElementPtr scope,
-                       const std::string& name) {
-        return (getIntType<uint32_t>(scope, name));
-    }
-
-    /// @brief Returns a value converted to uint16_t
-    ///
-    /// Instantiation of getIntType() to uint16_t
-    ///
-    /// @param scope specified parameter will be extracted from this scope
-    /// @param name name of the parameter
-    /// @return an uint16_t value
-    /// @throw isc::dhcp::DhcpConfigError when it is not an uint16_t
-    uint16_t getUint16(isc::data::ConstElementPtr scope,
-                       const std::string& name) {
-        return (getIntType<uint16_t>(scope, name));
-    }
-
-    /// @brief Get an uint8_t value
-    ///
-    /// Instantiation of getIntType() to uint8_t
-    ///
-    /// @param scope specified parameter will be extracted from this scope
-    /// @param name name of the parameter
-    /// @return uint8_t value
-    /// @throw isc::dhcp::DhcpConfigError when it is not an uint8_t
-    uint8_t getUint8(ConstElementPtr scope, const std::string& name) {
-        return (getIntType<uint8_t>(scope, name));
-    }
 };
 
 };

src/lib/cc/tests/simple_parser_unittest.cc

@@ -58,9 +58,14 @@ public:
 
 class SimpleParserClassTest : public SimpleParser {
 public:
-
-    /// Make getUint8 public
-    using SimpleParser::getUint8;
+    /// @brief Instantiation of getIntType for uint8_t
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter for error report
+    /// @return an uint8_t value
+    uint8_t getUint8(ConstElementPtr scope, const std::string& name) {
+        return (getIntType<uint8_t>(scope, name));
+    }
 
     /// @brief Instantiation of getAndConvert
     ///

src/lib/dhcpsrv/parsers/client_class_def_parser.cc

@@ -62,87 +62,86 @@ void
 ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
                             ConstElementPtr class_def_cfg,
                             uint16_t family) {
-    // name is now mandatory
-    std::string name = getString(class_def_cfg, "name");
-    if (name.empty()) {
-        isc_throw(DhcpConfigError,
-                  "not empty parameter 'name' is required "
-                  << getPosition("name", class_def_cfg) << ")");
-    }
 
-    // Parse matching expression
-    ExpressionPtr match_expr;
-    ConstElementPtr test = class_def_cfg->get("test");
-    if (test) {
-        ExpressionParser parser;
-        parser.parse(match_expr, test, family);
-    }
+    try {
+        std::string name;
+        std::string next_server_txt = "0.0.0.0";
+        std::string sname;
+        std::string filename;
+        ExpressionPtr match_expr;
+        CfgOptionPtr options(new CfgOption());
+
+        // Parse the elements that make up the client class definition.
+        BOOST_FOREACH(ConfigPair param, class_def_cfg->mapValue()) {
+            std::string entry(param.first);
+            ConstElementPtr value(param.second);
+
+            if (entry == "name") {
+                name = value->stringValue();
+
+            } else if (entry == "test") {
+                ExpressionParser parser;
+                parser.parse(match_expr, value, family);
+                
+            } else if (entry == "option-data") {
+                OptionDataListParser opts_parser(family);
+                opts_parser.parse(options, value);
+
+            } else if (entry == "next-server") {
+                next_server_txt = value->stringValue();
+
+            } else if (entry == "server-hostname") {
+                sname = value->stringValue();
+
+            } else if (entry == "boot-file-name") {
+                filename = value->stringValue();
+
+            } else {
+                isc_throw(DhcpConfigError, "invalid parameter '" << entry
+                          << "' (" << value->getPosition() << ")");
+            }
+        }
 
-    // Parse option data
-    CfgOptionPtr options(new CfgOption());
-    ConstElementPtr option_data = class_def_cfg->get("option-data");
-    if (option_data) {
-        OptionDataListParser opts_parser(family);
-        opts_parser.parse(options, option_data);
-    }
+        // name is now mandatory
+        if (name.empty()) {
+            isc_throw(DhcpConfigError,
+                      "not empty parameter 'name' is required");
+        }
 
-    // Let's try to parse the next-server field
-    IOAddress next_server("0.0.0.0");
-    if (class_def_cfg->contains("next-server")) {
-        std::string next_server_txt = getString(class_def_cfg, "next-server");
+        // Let's parse the next-server field
+        IOAddress next_server("0.0.0.0");
         try {
             next_server = IOAddress(next_server_txt);
         } catch (const IOError& ex) {
-            isc_throw(DhcpConfigError,
-                      "Invalid next-server value specified: '"
-                      << next_server_txt << "' ("
-                      << getPosition("next-server", class_def_cfg) << ")");
+            isc_throw(DhcpConfigError, "Invalid next-server value specified: '"
+                      << next_server_txt);
         }
 
         if (next_server.getFamily() != AF_INET) {
             isc_throw(DhcpConfigError, "Invalid next-server value: '"
-                      << next_server_txt
-                      << "', must be IPv4 address ("
-                      << getPosition("next-server", class_def_cfg) << ")");
+                      << next_server_txt << "', must be IPv4 address");
         }
 
         if (next_server.isV4Bcast()) {
             isc_throw(DhcpConfigError, "Invalid next-server value: '"
-                      << next_server_txt
-                      << "', must not be a broadcast ("
-                      << getPosition("next-server", class_def_cfg) << ")");
+                      << next_server_txt << "', must not be a broadcast");
         }
-    }
-
-    // Let's try to parse server-hostname
-    std::string sname;
-    if (class_def_cfg->contains("server-hostname")) {
-        sname = getString(class_def_cfg, "server-hostname");
 
+        // Let's try to parse server-hostname
         if (sname.length() >= Pkt4::MAX_SNAME_LEN) {
             isc_throw(DhcpConfigError, "server-hostname must be at most "
                       << Pkt4::MAX_SNAME_LEN - 1 << " bytes long, it is "
-                      << sname.length() << " ("
-                      << getPosition("server-hostname", class_def_cfg) << ")");
+                      << sname.length());
         }
-    }
-
-    // Let's try to parse boot-file-name
-    std::string filename;
-    if (class_def_cfg->contains("boot-file-name")) {
-        filename = getString(class_def_cfg, "boot-file-name");
 
+        // Let's try to parse boot-file-name
         if (filename.length() > Pkt4::MAX_FILE_LEN) {
             isc_throw(DhcpConfigError, "boot-file-name must be at most "
                       << Pkt4::MAX_FILE_LEN - 1 << " bytes long, it is "
-                      << filename.length() << " ("
-                      << getPosition("boot-file-name", class_def_cfg) << ")");
+                      << filename.length());
         }
 
-    }
-
-    // Add the client class definition
-    try {
+        // Add the client class definition
         class_dictionary->addClass(name, match_expr, options, next_server,
                                    sname, filename);
     } catch (const std::exception& ex) {

src/lib/dhcpsrv/parsers/dbaccess_parser.h

@@ -43,7 +43,7 @@ public:
     /// @brief Constructor
     ///
     /// @param db_type Specifies database type (lease or hosts)
-    explicit DbAccessParser(DBType db_type);
+    DbAccessParser(DBType db_type);
 
     /// The destructor.
     virtual ~DbAccessParser()

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -295,7 +295,10 @@ OptionDataParser::extractSpace(ConstElementPtr parent) const {
         }
 
     } catch (std::exception& ex) {
-        // Append position of the option space parameter.
+        // Append position of the option space parameter. Note, that in the case
+        // when 'space' was not specified a default value will be used and we
+        // should never get here. Therefore, it is ok to call getPosition for
+        // the space parameter here as this parameter will always be specified.
         isc_throw(DhcpConfigError, ex.what() << " ("
                   << getPosition("space", parent) << ")");
     }
@@ -597,31 +600,57 @@ RelayInfoParser::RelayInfoParser(const Option::Universe& family)
     : family_(family) {
 };
 
-// Can't use a constructor as a function
-namespace {
-IOAddress buildIOAddress(const std::string& str) { return (IOAddress(str)); }
-};
-
-IOAddress
-RelayInfoParser::getIOAddress(ConstElementPtr scope,
-                              const std::string& name) {
-    return (getAndConvert<IOAddress,
-            buildIOAddress>(scope, name, "address"));
-}
-
 void
 RelayInfoParser::parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg,
                        ConstElementPtr relay_info) {
-    // There is only one parameter which is mandatory
-    IOAddress ip = getIOAddress(relay_info, "ip-address");
-
-    // Check if the address family matches.
-    if ((ip.isV4() && family_ != Option::V4) ||
-        (ip.isV6() && family_ != Option::V6) ) {
-        isc_throw(DhcpConfigError, "ip-address field " << ip.toText()
-                  << " does not have IP address of expected family type: "
-                  << (family_ == Option::V4 ? "IPv4" : "IPv6")
-                  << " (" << getPosition("ip-address", relay_info) << ")");
+    // Let's start with some sanity checks.
+    if (!relay_info || !cfg) {
+        isc_throw(DhcpConfigError, "Logic error: RelayInfoParser::parse() called "
+                  "with at least one NULL parameter.");
+    }
+
+    if (relay_info->getType() != Element::map) {
+        isc_throw(DhcpConfigError, "Configuration error: RelayInfoParser::parse() "
+                  "called with non-map parameter");
+    }
+
+    // Now create the default value.
+    isc::asiolink::IOAddress ip(family_ == Option::V4 ? IOAddress::IPV4_ZERO_ADDRESS()
+                                : IOAddress::IPV6_ZERO_ADDRESS());
+
+    // Now iterate over all parameters. Currently there's only one supported
+    // parameter, so it should be an easy thing to check.
+    bool ip_address_specified = false;
+    BOOST_FOREACH(ConfigPair param, relay_info->mapValue()) {
+        if (param.first == "ip-address") {
+            ip_address_specified = true;
+
+            try {
+                ip = asiolink::IOAddress(param.second->stringValue());
+            } catch (...)  {
+                isc_throw(DhcpConfigError, "Failed to parse ip-address "
+                          "value: " << param.second
+                          << " (" << param.second->getPosition() << ")");
+            }
+
+            // Check if the address family matches.
+            if ( (ip.isV4() && family_ != Option::V4) ||
+                 (ip.isV6() && family_ != Option::V6) ) {
+                isc_throw(DhcpConfigError, "ip-address field " << ip.toText()
+                          << " does not have IP address of expected family type: "
+                          << (family_ == Option::V4 ? "IPv4" : "IPv6")
+                          << " (" << param.second->getPosition() << ")");
+            }
+        } else {
+            isc_throw(NotImplemented,
+                      "parser error: RelayInfoParser parameter not supported: "
+                      << param.second);
+        }
+    }
+
+    if (!ip_address_specified) {
+        isc_throw(DhcpConfigError, "'relay' specified, but mandatory 'ip-address' "
+                  "paramter in it is missing");
     }
 
     // Ok, we're done with parsing. Let's store the result in the structure
@@ -885,10 +914,17 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
     try {
         std::string hr_mode = getString(params, "reservation-mode");
         subnet_->setHostReservationMode(hrModeFromText(hr_mode));
-    } catch (const BadValue& ex) { 
-       isc_throw(DhcpConfigError, "Failed to process specified value "
+    } catch (const BadValue& ex) {
+        ConstElementPtr mode = params->get("reservation-mode");
+        string pos;
+        if (mode) {
+            pos = mode->getPosition().str();
+        } else {
+            pos = params->getPosition().str();
+        }
+        isc_throw(DhcpConfigError, "Failed to process specified value "
                   " of reservation-mode parameter: " << ex.what()
-                  << "(" << getPosition("reservation-mode", params) << ")");
+                  << "(" << pos << ")");
     }
 
     // Try setting up client class.
@@ -907,6 +943,17 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
 
 //**************************** D2ClientConfigParser **********************
 
+uint32_t
+D2ClientConfigParser::getUint32(ConstElementPtr scope,
+                                const std::string& name) {
+    return (getIntType<uint32_t>(scope, name));
+}
+
+// Can't use a constructor as a function
+namespace {
+IOAddress buildIOAddress(const std::string& str) { return (IOAddress(str)); }
+};
+
 IOAddress
 D2ClientConfigParser::getIOAddress(ConstElementPtr scope,
                                    const std::string& name) {

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -365,7 +365,7 @@ public:
     /// @brief Constructor.
     ///
     /// @param address_family Address family: @c AF_INET or @c AF_INET6.
-    explicit OptionDataParser(const uint16_t address_family);
+    OptionDataParser(const uint16_t address_family);
 
     /// @brief Parses ElementPtr containing option definition
     ///
@@ -477,7 +477,7 @@ public:
     /// @brief Constructor.
     ///
     /// @param address_family Address family: @c AF_INET or AF_INET6
-    explicit OptionDataListParser(const uint16_t address_family);
+    OptionDataListParser(const uint16_t address_family);
 
     /// @brief Parses a list of options, instantiates them and stores in cfg
     ///
@@ -620,7 +620,7 @@ public:
 
     /// @brief constructor
     /// @param family specifies protocol family (IPv4 or IPv6)
-    explicit RelayInfoParser(const isc::dhcp::Option::Universe& family);
+    RelayInfoParser(const isc::dhcp::Option::Universe& family);
 
     /// @brief parses the actual relay parameters
     ///
@@ -632,18 +632,7 @@ public:
     void parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg,
                isc::data::ConstElementPtr relay_info);
 
-private:
-
-    /// @brief Returns a value converted to IOAddress
-    ///
-    /// Instantiation of getAndConvert() to IOAddress
-    ///
-    /// @param scope specified parameter will be extracted from this scope
-    /// @param name name of the parameter
-    /// @return an IOAddress value
-    isc::asiolink::IOAddress
-    getIOAddress(isc::data::ConstElementPtr scope, const std::string& name);
-
+protected:
     /// Protocol family (IPv4 or IPv6)
     Option::Universe family_;
 };
@@ -675,7 +664,7 @@ public:
     /// @brief constructor
     ///
     /// @param family address family: @c AF_INET or @c AF_INET6
-    explicit SubnetConfigParser(uint16_t family);
+    SubnetConfigParser(uint16_t family);
 
     /// @brief virtual destructor (does nothing)
     virtual ~SubnetConfigParser() { }
@@ -787,6 +776,16 @@ public:
 
 private:
 
+    /// @brief Returns a value converted to uint32_t
+    ///
+    /// Instantiation of getIntType() to uint32_t
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return an uint32_t value
+    uint32_t
+    getUint32(isc::data::ConstElementPtr scope, const std::string& name);
+
     /// @brief Returns a value converted to IOAddress
     ///
     /// Instantiation of getAndConvert() to IOAddress

src/lib/dhcpsrv/parsers/duid_config_parser.cc

@@ -24,62 +24,107 @@ namespace isc {
 namespace dhcp {
 
 void
-DUIDConfigParser::parse(const CfgDUIDPtr& cfg,
-                        isc::data::ConstElementPtr duid_configuration) {
-
-    std::string param;
-    try {
-        param = "type";
-        std::string duid_type = getString(duid_configuration, "type");
-        // Map DUID type represented as text into numeric value.
-        DUID::DUIDType numeric_type = DUID::DUID_UNKNOWN;
-        if (duid_type == "LLT") {
-            numeric_type = DUID::DUID_LLT;
-        } else if (duid_type == "EN") {
-            numeric_type = DUID::DUID_EN;
-        } else if (duid_type == "LL") {
-            numeric_type = DUID::DUID_LL;
-        } else {
-            isc_throw(BadValue, "unsupported DUID type '"
-                      << duid_type << "'. Expected: LLT, EN or LL");
+DUIDConfigParser::parse(const CfgDUIDPtr& cfg, isc::data::ConstElementPtr duid_configuration) {
+    if (!cfg) {
+        isc_throw(DhcpConfigError, "Must provide valid pointer to cfg when parsing duid");
+    }
+
+    bool type_present = false;
+    BOOST_FOREACH(ConfigPair element, duid_configuration->mapValue()) {
+        try {
+            if (element.first == "type") {
+                type_present = true;
+                setType(cfg, element.second->stringValue());
+            } else if (element.first == "identifier") {
+                setIdentifier(cfg, element.second->stringValue());
+            } else if (element.first == "htype") {
+                setHType(cfg, element.second->intValue());
+            } else if (element.first == "time") {
+                setTime(cfg, element.second->intValue());
+            } else if (element.first == "enterprise-id") {
+                setEnterpriseId(cfg, element.second->intValue());
+            } else if (element.first == "persist") {
+                setPersist(cfg, element.second->boolValue());
+            } else {
+                isc_throw(DhcpConfigError, "unsupported configuration "
+                          "parameter '" << element.first << "'");
+            }
+        } catch (const std::exception& ex) {
+            // Append position.
+            isc_throw(DhcpConfigError, ex.what() << " ("
+                      << element.second->getPosition() << ")");
         }
+    }
 
-        cfg->setType(static_cast<DUID::DUIDType>(numeric_type));
+    // "type" is mandatory
+    if (!type_present) {
+        isc_throw(DhcpConfigError, "mandatory parameter \"type\" not specified"
+                  " for the DUID configuration ("
+                  << duid_configuration->getPosition() << ")");
+    }
 
-        param = "identifier";
-        if (duid_configuration->contains(param)) {
-            cfg->setIdentifier(getString(duid_configuration, param));
-        }
+    LOG_WARN(dhcpsrv_logger, DHCPSRV_CFGMGR_CONFIGURE_SERVERID);
+}
 
-        param = "htype";
-        if (duid_configuration->contains(param)) {
-            cfg->setHType(getUint16(duid_configuration, param));
-        }
+void
+DUIDConfigParser::setType(const CfgDUIDPtr& cfg, const std::string& duid_type) const {
+    // Map DUID type represented as text into numeric value.
+    DUID::DUIDType numeric_type = DUID::DUID_UNKNOWN;
+    if (duid_type == "LLT") {
+        numeric_type = DUID::DUID_LLT;
+    } else if (duid_type == "EN") {
+        numeric_type = DUID::DUID_EN;
+    } else if (duid_type == "LL") {
+        numeric_type = DUID::DUID_LL;
+    } else {
+        isc_throw(DhcpConfigError, "unsupported DUID type '"
+                  << duid_type << "'. Expected: LLT, EN or LL");
+    }
 
-        param = "time";
-        if (duid_configuration->contains(param)) {
-            cfg->setTime(getUint32(duid_configuration, param));
-        }
+    cfg->setType(static_cast<DUID::DUIDType>(numeric_type));
+}
 
-        param = "enterprise-id";
-        if (duid_configuration->contains(param)) {
-            cfg->setEnterpriseId(getUint32(duid_configuration, param));
-        }
+void
+DUIDConfigParser::setIdentifier(const CfgDUIDPtr& cfg, const std::string& identifier) const {
+    cfg->setIdentifier(identifier);
+}
 
-        param = "persist";
-        if (duid_configuration->contains(param)) {
-            cfg->setPersist(getBoolean(duid_configuration, param));
-        }
-    } catch (const DhcpConfigError&) {
-        throw;
-    } catch (const std::exception& ex) {
-        // Append position.
-        isc_throw(DhcpConfigError, ex.what() << " ("
-                  << getPosition(param, duid_configuration) << ")");
-    }
+void
+DUIDConfigParser::setHType(const CfgDUIDPtr& cfg, const int64_t htype) const {
+    checkRange<uint16_t>("htype", htype);
+    cfg->setHType(static_cast<uint16_t>(htype));
+}
 
-    LOG_WARN(dhcpsrv_logger, DHCPSRV_CFGMGR_CONFIGURE_SERVERID);
+void
+DUIDConfigParser::setTime(const CfgDUIDPtr& cfg, const int64_t new_time) const {
+    checkRange<uint32_t>("time", new_time);
+    cfg->setTime(static_cast<uint32_t>(new_time));
+}
+
+void
+DUIDConfigParser::setEnterpriseId(const CfgDUIDPtr& cfg, const int64_t enterprise_id) const {
+    checkRange<uint32_t>("enterprise-id", enterprise_id);
+    cfg->setEnterpriseId(static_cast<uint32_t>(enterprise_id));
 }
 
+void
+DUIDConfigParser::setPersist(const CfgDUIDPtr& cfg, const bool persist) {
+    cfg->setPersist(persist);
+}
+
+template<typename NumericType>
+void
+DUIDConfigParser::checkRange(const std::string& parameter_name,
+                             const int64_t parameter_value) const {
+    if ((parameter_value < 0) ||
+        (parameter_value > std::numeric_limits<NumericType>::max())) {
+        isc_throw(DhcpConfigError, "out of range value '" << parameter_value
+                  << "' specified for parameter '" << parameter_name
+                  << "'; expected value in range of [0.."
+                  << std::numeric_limits<NumericType>::max() << "]");
+    }
+}
+
+
 } // end of namespace isc::dhcp
 } // end of namespace isc

src/lib/dhcpsrv/parsers/duid_config_parser.h

@@ -33,6 +33,57 @@ public:
     ///
     /// @throw DhcpConfigError If the configuration is invalid.
     void parse(const CfgDUIDPtr& cfg, isc::data::ConstElementPtr duid_configuration);
+private:
+
+    /// @brief Validate and set DUID type.
+    ///
+    /// @param cfg parsed information will be stored here
+    /// @param duid_type DUID type in textual format.
+    void setType(const CfgDUIDPtr& cfg, const std::string& duid_type) const;
+
+    /// @brief Validate and set identifier.
+    ///
+    /// @param cfg parsed information will be stored here
+    /// @param identifier Identifier.
+    void setIdentifier(const CfgDUIDPtr& cfg, const std::string& identifier) const;
+
+    /// @brief Validate and set hardware type.
+    ///
+    /// @param cfg parsed information will be stored here
+    /// @param htype Hardware type.
+    void setHType(const CfgDUIDPtr& cfg, const int64_t htype) const;
+
+    /// @brief Validate and set time value.
+    ///
+    /// @param cfg parsed information will be stored here
+    /// @param new_time Time value to be used for DUID.
+    void setTime(const CfgDUIDPtr& cfg, const int64_t new_time) const;
+
+    /// @brief Validate and set enterprise id.
+    ///
+    /// @param cfg parsed information will be stored here
+    /// @param enterprise_id Enterprise id.
+    void setEnterpriseId(const CfgDUIDPtr& cfg, const int64_t enterprise_id) const;
+
+    /// @brief Set persistence flag.
+    ///
+    /// @param cfg parsed information will be stored here
+    /// @param persist A boolean value indicating if the server
+    /// identifier should be stored on the disk (if true) or
+    /// not (if false).
+    void setPersist(const CfgDUIDPtr& cfg, const bool persist);
+
+    /// @brief Verifies if the specified parameter is in range.
+    ///
+    /// Each numeric value must be in range of [0 .. max_value], where
+    /// max_value is a maximum value for the numeric type used for this
+    /// parameter.
+    ///
+    /// @param parameter_name Parameter name.
+    /// @tparam Numeric type of the specified parameter.
+    template<typename NumericType>
+    void checkRange(const std::string& parameter_name,
+                    const int64_t parameter_value) const;
 };
 
 }

src/lib/dhcpsrv/parsers/expiration_config_parser.cc

@@ -20,46 +20,43 @@ void
 ExpirationConfigParser::parse(ConstElementPtr expiration_config) {
     CfgExpirationPtr cfg = CfgMgr::instance().getStagingCfg()->getCfgExpiration();
 
-    std::string param;
+    BOOST_FOREACH(ConfigPair config_element, expiration_config->mapValue()) {
 
-    try {
-        param = "reclaim-timer-wait-time";
-        if (expiration_config->contains(param)) {
-            cfg->setReclaimTimerWaitTime(getInteger(expiration_config, param));
-        }
+        // Get parameter name and value.
+        std::string param_name = config_element.first;
+        ConstElementPtr param_value = config_element.second;
 
-        param = "flush-reclaimed-timer-wait-time";
-        if (expiration_config->contains(param)) {
-            cfg->setFlushReclaimedTimerWaitTime(getInteger(expiration_config,
-                                                           param));
-        }
+        try {
+            // Set configuration parameters.
+            if (param_name == "reclaim-timer-wait-time") {
+                cfg->setReclaimTimerWaitTime(param_value->intValue());
 
-        param = "hold-reclaimed-time";
-        if (expiration_config->contains(param)) {
-            cfg->setHoldReclaimedTime(getInteger(expiration_config, param));
-        }
+            } else if (param_name == "flush-reclaimed-timer-wait-time") {
+                cfg->setFlushReclaimedTimerWaitTime(param_value->intValue());
 
-        param = "max-reclaim-leases";
-        if (expiration_config->contains(param)) {
-            cfg->setMaxReclaimLeases(getInteger(expiration_config, param));
-        }                          
+            } else if (param_name == "hold-reclaimed-time") {
+                cfg->setHoldReclaimedTime(param_value->intValue());
 
-        param = "max-reclaim-time";
-        if (expiration_config->contains(param)) {
-            cfg->setMaxReclaimTime(getInteger(expiration_config, param));
-        }
+            } else if (param_name == "max-reclaim-leases") {
+                cfg->setMaxReclaimLeases(param_value->intValue());
+
+            } else if (param_name == "max-reclaim-time") {
+                cfg->setMaxReclaimTime(param_value->intValue());
+
+            } else if (param_name == "unwarned-reclaim-cycles") {
+                cfg->setUnwarnedReclaimCycles(param_value->intValue());
+
+            } else {
+                isc_throw(DhcpConfigError, "unsupported parameter '"
+                          << param_name << "'");
+            }
 
-        param = "unwarned-reclaim-cycles";
-        if (expiration_config->contains(param)) {
-            cfg->setUnwarnedReclaimCycles(
-                getInteger(expiration_config, param));
+        } catch (const std::exception& ex) {
+            // Append position of the configuration parameter to the error
+            // message.
+            isc_throw(DhcpConfigError, ex.what() << " ("
+                      << param_value->getPosition() << ")");
         }
-    } catch (const DhcpConfigError&) {
-        throw;
-    } catch (const std::exception& ex) {
-        // Append position of the configuration parameter to the error message.
-        isc_throw(DhcpConfigError, ex.what() << " ("
-                  << getPosition(param, expiration_config) << ")");
     }
 }
 

src/lib/dhcpsrv/parsers/ifaces_config_parser.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-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
@@ -29,7 +29,7 @@ public:
     /// @brief Constructor
     ///
     /// @param protocol AF_INET for DHCPv4 and AF_INET6 for DHCPv6.
-    explicit IfacesConfigParser(const uint16_t protocol);
+    IfacesConfigParser(const uint16_t protocol);
 
     /// @brief Parses content of the "interfaces-config".
     ///

src/lib/dhcpsrv/srv_config.h

@@ -507,7 +507,7 @@ public:
     /// this socket is bound and connected to this port and port + 1
     ///
     /// @param port port and port + 1 to use
-    void setDhcp4o6Port(uint16_t port) {
+    void setDhcp4o6Port(uint32_t port) {
         /// @todo: Port is supposed to be uint16_t, not uint32_t
         dhcp4o6_port_ = port;
     }
@@ -516,7 +516,7 @@ public:
     ///
     /// See @ref setDhcp4o6Port for brief discussion.
     /// @return value of DHCP4o6 IPC port
-    uint16_t getDhcp4o6Port() {
+    uint32_t getDhcp4o6Port() {
         return (dhcp4o6_port_);
     }
 
@@ -624,7 +624,7 @@ private:
     ///
     /// DHCPv4-over-DHCPv6 uses a UDP socket for interserver communication,
     /// this socket is bound and connected to this port and port + 1
-    uint16_t dhcp4o6_port_;
+    uint32_t dhcp4o6_port_;
 
     D2ClientConfigPtr d2_client_config_;
 };

src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc

@@ -459,6 +459,20 @@ TEST_F(ClientClassDefParserTest, blankClassName) {
                  DhcpConfigError);
 }
 
+
+// Verifies that a class with an unknown element, fails to parse.
+TEST_F(ClientClassDefParserTest, unknownElement) {
+    std::string cfg_text =
+        "{ \n"
+        "    \"name\": \"one\", \n"
+        "    \"bogus\": \"bad\" \n"
+        "} \n";
+
+    ClientClassDefPtr cclass;
+    ASSERT_THROW(cclass = parseClientClassDef(cfg_text, AF_INET),
+                 DhcpConfigError);
+}
+
 // Verifies that a class with an invalid expression, fails to parse.
 TEST_F(ClientClassDefParserTest, invalidExpression) {
     std::string cfg_text =
@@ -551,6 +565,22 @@ TEST_F(ClientClassDefListParserTest, duplicateClass) {
                  DhcpConfigError);
 }
 
+// Verifies that a class list containing an invalid class entry, fails to
+// parse.
+TEST_F(ClientClassDefListParserTest, invalidClass) {
+    std::string cfg_text =
+        "[ \n"
+        "   { \n"
+        "       \"name\": \"one\", \n"
+        "       \"bogus\": \"bad\" \n"
+        "   } \n"
+        "] \n";
+
+    ClientClassDictionaryPtr dictionary;
+    ASSERT_THROW(dictionary = parseClientClassDefList(cfg_text, AF_INET6),
+                 DhcpConfigError);
+}
+
 // Test verifies that without any class specified, the fixed fields have their
 // default, empty value.
 // @todo same with AF_INET6

src/lib/dhcpsrv/tests/expiration_config_parser_unittest.cc

@@ -221,6 +221,15 @@ TEST_F(ExpirationConfigParserTest, otherParameters) {
     EXPECT_EQ(20, cfg->getUnwarnedReclaimCycles());
 }
 
+// This test verifies that the exception is thrown if unsupported
+// parameter is specified.
+TEST_F(ExpirationConfigParserTest, invalidParameter) {
+   addParam("reclaim-timer-wait-time", 20);
+   addParam("invalid-parameter", 20);
+
+   EXPECT_THROW(renderConfig(), DhcpConfigError);
+}
+
 // This test verifies that negative parameter values are not allowed.
 TEST_F(ExpirationConfigParserTest, outOfRangeValues) {
     testOutOfRange("reclaim-timer-wait-time",