[#229,!140] Class parser can now check if all parameters are recognized.

diff --git a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc
index 00b0d2becd5c220ecf443d2800eb88608f304047..49b871f2dd396eb75fcba834dd32410a89e96aaa 100644 (file)
--- a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc
+++ b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc
@@ -71,7 +71,7 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
                             ConstElementPtr class_def_cfg,
                             uint16_t family,
                             bool append_error_position) {
-    // name is now mandatory
+    // name is now mandatory, so let's deal with it first.
     std::string name = getString(class_def_cfg, "name");
     if (name.empty()) {
         isc_throw(DhcpConfigError,
@@ -219,6 +219,37 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
     }
 }
 
+void
+ClientClassDefParser::checkParametersSupported(const ConstElementPtr& class_def_cfg,
+                                               const uint16_t family) {
+    // Make sure that the client class definition is stored in a map.
+    if (!class_def_cfg || (class_def_cfg->getType() != Element::map)) {
+        isc_throw(DhcpConfigError, "client class definition is not a map");
+    }
+
+    // Common v4 and v6 parameters supported for the client class.
+    static std::set<std::string> supported_params = { "name", "test", "option-def",
+                                                      "option-data", "user-context",
+                                                      "only-if-required" };
+
+    // The v4 client class supports additional parmeters.
+    static std::set<std::string> supported_params_v4 = { "next-server", "server-hostname",
+                                                         "boot-file-name" };
+
+    // Iterate over the specified parameters and check if they are all supported.
+    for (auto name_value_pair : class_def_cfg->mapValue()) {
+        if ((supported_params.count(name_value_pair.first) > 0) ||
+            ((family == AF_INET) && (supported_params_v4.count(name_value_pair.first) > 0))) {
+            continue;
+
+        } else {
+            isc_throw(DhcpConfigError, "unsupported client class parameter '"
+                      << name_value_pair.first << "'");
+        }
+    }
+}
+
+
 // ****************** ClientClassDefListParser ************************
 
 ClientClassDictionaryPtr

diff --git a/src/lib/dhcpsrv/parsers/client_class_def_parser.h b/src/lib/dhcpsrv/parsers/client_class_def_parser.h
index 02c6e0613d800f8fe50009ca677010cb9b4230cc..221ba138501b348bf4344e66c9c45cdf666058ae 100644 (file)
--- a/src/lib/dhcpsrv/parsers/client_class_def_parser.h
+++ b/src/lib/dhcpsrv/parsers/client_class_def_parser.h
@@ -100,6 +100,18 @@ public:
                isc::data::ConstElementPtr client_class_def,
                uint16_t family,
                bool append_error_position = true);
+
+    /// @brief Iterates over class parameters and checks if they are supported.
+    ///
+    /// This method should be called by hooks libraries which do not use Bison
+    /// to validate class syntax prior to parsing the client class information.
+    ///
+    /// @param class_def_cfg class configuration entry.
+    /// @param family the address family of the client class.
+    ///
+    /// @throw DhcpConfigError if any of the parameters is not supported.
+    void checkParametersSupported(const isc::data::ConstElementPtr& class_def_cfg,
+                                  const uint16_t family);
 };
 
 /// @brief Defines a pointer to a ClientClassDefParser

diff --git a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
index 83324806fa817266a9496b35d7facc71e8f200d3..93c0f3ce6d814d0dc7ddccff84ba6749879cf5e7 100644 (file)
--- a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc
@@ -268,6 +268,105 @@ TEST_F(ExpressionParserTest, nameEmpty) {
                  DhcpConfigError);
 }
 
+// Verifies that the function checking if specified client class parameters
+// are supported does not throw if all specified DHCPv4 client class
+// parameters are recognized.
+TEST_F(ClientClassDefParserTest, checkAllSupported4) {
+    std::string cfg_text =
+        "{\n"
+        "    \"name\": \"foo\","
+        "    \"test\": \"member('ALL')\","
+        "    \"option-def\": [ ],\n"
+        "    \"option-data\": [ ],\n"
+        "    \"user-context\": { },\n"
+        "    \"only-if-required\": false,\n"
+        "    \"next-server\": \"192.0.2.3\",\n"
+        "    \"server-hostname\": \"myhost\",\n"
+        "    \"boot-file-name\": \"efi\""
+        "}\n";
+
+    ElementPtr config_element = Element::fromJSON(cfg_text);
+
+    ClientClassDefParser parser;
+    EXPECT_NO_THROW(parser.checkParametersSupported(config_element, AF_INET));
+}
+
+// Verifies that the function checking if specified client class parameters
+// are supported does not throw if all specified DHCPv6 client class
+// parameters are recognized.
+TEST_F(ClientClassDefParserTest, checkAllSupported6) {
+    std::string cfg_text =
+        "{\n"
+        "    \"name\": \"foo\","
+        "    \"test\": \"member('ALL')\","
+        "    \"option-def\": [ ],\n"
+        "    \"option-data\": [ ],\n"
+        "    \"user-context\": { },\n"
+        "    \"only-if-required\": false\n"
+        "}\n";
+
+    ElementPtr config_element = Element::fromJSON(cfg_text);
+
+    ClientClassDefParser parser;
+    EXPECT_NO_THROW(parser.checkParametersSupported(config_element, AF_INET6));
+}
+
+// Verifies that the function checking if specified client class parameters
+// are supported throws if DHCPv4 specific parameters are specified for the
+// DHCPv6 client class.
+TEST_F(ClientClassDefParserTest, checkParams4Unsupported6) {
+    std::string cfg_text =
+        "{\n"
+        "    \"name\": \"foo\","
+        "    \"test\": \"member('ALL')\","
+        "    \"option-def\": [ ],\n"
+        "    \"option-data\": [ ],\n"
+        "    \"user-context\": { },\n"
+        "    \"only-if-required\": false,\n"
+        "    \"next-server\": \"192.0.2.3\",\n"
+        "    \"server-hostname\": \"myhost\",\n"
+        "    \"boot-file-name\": \"efi\""
+        "}\n";
+
+    ElementPtr config_element = Element::fromJSON(cfg_text);
+
+    ClientClassDefParser parser;
+    EXPECT_THROW(parser.checkParametersSupported(config_element, AF_INET6),
+                 DhcpConfigError);
+}
+
+// Verifies that the function checking if specified DHCPv4 client class
+// parameters are supported throws if one of the parameters is not recognized.
+TEST_F(ClientClassDefParserTest, checkParams4Unsupported) {
+    std::string cfg_text =
+        "{\n"
+        "    \"name\": \"foo\","
+        "    \"unsupported\": \"member('ALL')\""
+        "}\n";
+
+    ElementPtr config_element = Element::fromJSON(cfg_text);
+
+    ClientClassDefParser parser;
+    EXPECT_THROW(parser.checkParametersSupported(config_element, AF_INET),
+                 DhcpConfigError);
+}
+
+// Verifies that the function checking if specified DHCPv6 client class
+// parameters are supported throws if one of the parameters is not recognized.
+TEST_F(ClientClassDefParserTest, checkParams6Unsupported) {
+    std::string cfg_text =
+        "{\n"
+        "    \"name\": \"foo\","
+        "    \"unsupported\": \"member('ALL')\""
+        "}\n";
+
+    ElementPtr config_element = Element::fromJSON(cfg_text);
+
+    ClientClassDefParser parser;
+    EXPECT_THROW(parser.checkParametersSupported(config_element, AF_INET6),
+                 DhcpConfigError);
+}
+
 // Verifies you can create a class with only a name
 // Whether that's useful or not, remains to be seen.
 // For now the class allows it.