From: Francis Dupont Date: Fri, 10 Feb 2017 13:55:13 +0000 (+0100) Subject: [5126] Flatten ClientClassDefParser X-Git-Tag: trac5124a_base~26^2~4 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=fc8e4b883d6d8b596d5d3e2115ab3642628ea3dd;p=thirdparty%2Fkea.git [5126] Flatten ClientClassDefParser --- diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 6d2bfcc868..82143943ff 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -4559,14 +4559,6 @@ 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); } diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 34aaa6a500..352f47c308 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -4971,13 +4971,6 @@ 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); } diff --git a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc index 2194cffc31..747909a926 100644 --- a/src/lib/dhcpsrv/parsers/client_class_def_parser.cc +++ b/src/lib/dhcpsrv/parsers/client_class_def_parser.cc @@ -62,86 +62,89 @@ 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) << ")"); + } - 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 matching expression + ExpressionPtr match_expr; + ConstElementPtr test_cfg = class_def_cfg->get("test"); + if (test_cfg) { + ExpressionParser parser; + parser.parse(match_expr, test_cfg, family); + } - // name is now mandatory - if (name.empty()) { - isc_throw(DhcpConfigError, - "not empty parameter 'name' is required"); - } + // Parse option data + CfgOptionPtr options(new CfgOption()); + ConstElementPtr options_cfg = class_def_cfg->get("option-data"); + if (options_cfg) { + OptionDataListParser opts_parser(family); + opts_parser.parse(options, options_cfg); + } - // Let's parse the next-server field - IOAddress next_server("0.0.0.0"); + // Let's try to parse the next-server field + IOAddress next_server("0.0.0.0"); + ConstElementPtr next_server_cfg = class_def_cfg->get("next-server"); + if (next_server_cfg) { try { - next_server = IOAddress(next_server_txt); + next_server = IOAddress(getString(class_def_cfg, "next-server")); } catch (const IOError& ex) { - isc_throw(DhcpConfigError, "Invalid next-server value specified: '" - << next_server_txt); + isc_throw(DhcpConfigError, + "Invalid next-server value specified: '" + << next_server_cfg->stringValue() << "' (" + << next_server_cfg->getPosition() << ")"); } if (next_server.getFamily() != AF_INET) { isc_throw(DhcpConfigError, "Invalid next-server value: '" - << next_server_txt << "', must be IPv4 address"); + << next_server_cfg->stringValue() + << "', must be IPv4 address (" + << next_server_cfg->getPosition() << ")"); } if (next_server.isV4Bcast()) { isc_throw(DhcpConfigError, "Invalid next-server value: '" - << next_server_txt << "', must not be a broadcast"); + << next_server_cfg->stringValue() + << "', must not be a broadcast (" + << next_server_cfg->getPosition() << ")"); } + } + + // Let's try to parse server-hostname + std::string sname; + ConstElementPtr sname_cfg = class_def_cfg->get("server-hostname"); + if (sname_cfg) { + 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()); + << sname.length() << " (" + << sname_cfg->getPosition() << ")"); } + } + + // Let's try to parse boot-file-name + std::string filename; + ConstElementPtr filename_cfg = class_def_cfg->get("boot-file-name"); + if (filename_cfg) { + 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()); + << filename.length() << " (" + << filename_cfg->getPosition() << ")"); } - // Add the client class definition + } + + // Add the client class definition + try { class_dictionary->addClass(name, match_expr, options, next_server, sname, filename); } catch (const std::exception& ex) { 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 17eeb9a97f..13c0217350 100644 --- a/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc @@ -459,20 +459,6 @@ 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 = @@ -565,22 +551,6 @@ 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