From: Francis Dupont Date: Fri, 13 Jan 2017 22:10:40 +0000 (+0100) Subject: [5033] Refactored D2ClientConfigParser X-Git-Tag: trac5112_base~6^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f09449b3ebb92507344392f9ddd4d7b75d649eb8;p=thirdparty%2Fkea.git [5033] Refactored D2ClientConfigParser --- diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index 40fc493d8c..ed49a47db1 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -1238,25 +1238,81 @@ SubnetConfigParser::getOptionalParam(const std::string& name) { //**************************** D2ClientConfigParser ********************** +namespace { + +template int_type +getInt(const std::string& name, ConstElementPtr value) { + int64_t val_int = value->intValue(); + if ((val_int < std::numeric_limits::min()) || + (val_int > std::numeric_limits::max())) { + isc_throw(DhcpConfigError, "out of range value (" << val_int + << ") specified for parameter '" << name + << "' (" << value->getPosition() << ")"); + } + return (static_cast(val_int)); +} + +uint32_t +getUint32(const std::string& name, ConstElementPtr value) { + return (getInt(name, value)); +} + +IOAddress +getIOAddress(const std::string& name, ConstElementPtr value) { + std::string str = value->stringValue(); + try { + return (IOAddress(str)); + } catch (const std::exception& ex) { + isc_throw(DhcpConfigError, "invalid address (" << str + << ") specified for parameter '" << name + << "' (" << value->getPosition() << ")"); + } +} + +dhcp_ddns::NameChangeProtocol +getProtocol(const std::string& name, ConstElementPtr value) { + std::string str = value->stringValue(); + try { + return (dhcp_ddns::stringToNcrProtocol(str)); + } catch (const std::exception& ex) { + isc_throw(DhcpConfigError, + "invalid NameChangeRequest protocol (" << str + << ") specified for parameter '" << name + << "' (" << value->getPosition() << ")"); + } +} + +dhcp_ddns::NameChangeFormat +getFormat(const std::string& name, ConstElementPtr value) { + std::string str = value->stringValue(); + try { + return (dhcp_ddns::stringToNcrFormat(str)); + } catch (const std::exception& ex) { + isc_throw(DhcpConfigError, + "invalid NameChangeRequest format (" << str + << ") specified for parameter '" << name + << "' (" << value->getPosition() << ")"); + } +} + +D2ClientConfig::ReplaceClientNameMode +getMode(const std::string& name, ConstElementPtr value) { + std::string str = value->stringValue(); + try { + return (D2ClientConfig::stringToReplaceClientNameMode(str)); + } catch (const std::exception& ex) { + isc_throw(DhcpConfigError, + "invalid ReplaceClientName mode (" << str + << ") specified for parameter '" << name + << "' (" << value->getPosition() << ")"); + } +} + +}; + D2ClientConfigPtr D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { D2ClientConfigPtr new_config; - bool enable_updates; - IOAddress server_ip(0); - uint32_t server_port; - IOAddress sender_ip(0); - uint32_t sender_port; - uint32_t max_queue_size; - dhcp_ddns::NameChangeProtocol ncr_protocol; - dhcp_ddns::NameChangeFormat ncr_format; - bool always_include_fqdn; - bool override_no_update; - bool override_client_update; - D2ClientConfig::ReplaceClientNameMode replace_client_name_mode; - std::string generated_prefix; - std::string qualifying_suffix; - std::string current_param; - if (isShortCutDisabled(client_config)) { // If enable-updates is the only parameter and it is false then @@ -1267,82 +1323,96 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { return (new_config); } - // Get all parameters that are needed to create the D2ClientConfig. - // We fetch all the parameters inside their own try clause so we - // spit out an error with an accurate text position. Use the - // local, current_param, to keep track of the parameter involved. - try { - current_param = "enable-updates"; - enable_updates = getBoolean(client_config, current_param); - - current_param = "server-ip"; - server_ip = IOAddress(getString(client_config, (current_param))); - - current_param = "server-port"; - server_port = getInteger(client_config, current_param); - - current_param = "sender-ip"; - std::string sender_ip_str(getString(client_config, current_param)); - if (sender_ip_str.empty()) { - // The default sender IP depends on the server IP family - sender_ip = (server_ip.isV4() ? IOAddress::IPV4_ZERO_ADDRESS() : - IOAddress::IPV6_ZERO_ADDRESS()); - } - else { - sender_ip = IOAddress(sender_ip_str); - } - - current_param = "sender-port"; - sender_port = getInteger(client_config, current_param); - - current_param = "max-queue-size"; - max_queue_size = getInteger(client_config, current_param); + // As isShortCutDisabled() was called this cannot fail + bool enable_updates = client_config->get("enable-updates")->boolValue(); - current_param = "ncr-protocol"; - ncr_protocol = dhcp_ddns::stringToNcrProtocol(getString(client_config, - current_param)); - current_param = "ncr-format"; - ncr_format = dhcp_ddns::stringToNcrFormat(getString(client_config, - current_param)); - - current_param = "always-include-fqdn"; - always_include_fqdn = getBoolean(client_config, current_param); - - current_param = "override-no-update"; - override_no_update = getBoolean(client_config, current_param); - - current_param = "override-client-update"; - override_client_update = getBoolean(client_config, current_param); + // Get all parameters that are needed to create the D2ClientConfig. + std::string qualifying_suffix; + bool found_qualifying_suffix = false; + IOAddress server_ip(0); + uint32_t server_port; + std::string sender_ip_str; + uint32_t sender_port; + uint32_t max_queue_size; + dhcp_ddns::NameChangeProtocol ncr_protocol; + dhcp_ddns::NameChangeFormat ncr_format; + bool always_include_fqdn; + bool allow_client_update; + bool override_no_update; + bool override_client_update; + D2ClientConfig::ReplaceClientNameMode replace_client_name_mode; + std::string generated_prefix; - // Formerly, replace-client-name was boolean, so for now we'll support - // boolean values by mapping them to the appropriate mode - current_param = "replace-client-name"; - std::string mode_str = getString(client_config, current_param); - if (boost::iequals(mode_str, "false")) { - // @todo add a debug log - replace_client_name_mode = D2ClientConfig::RCM_NEVER; - } - else if (boost::iequals(mode_str, "true")) { - // @todo add a debug log - replace_client_name_mode = D2ClientConfig::RCM_WHEN_PRESENT; - } else { - replace_client_name_mode = D2ClientConfig:: - stringToReplaceClientNameMode(mode_str); + BOOST_FOREACH(ConfigPair param, client_config->mapValue()) { + std::string entry(param.first); + ConstElementPtr value(param.second); + try { + if (entry == "enable-updates") { + // already done. + } else if (entry == "qualifying-suffix") { + qualifying_suffix = value->stringValue(); + found_qualifying_suffix = true; + } else if (entry == "server-ip") { + server_ip = getIOAddress("server-ip", value); + } else if (entry == "server-port") { + server_port = getUint32("server-port", value); + } else if (entry == "sender-ip") { + sender_ip_str = value->stringValue(); + } else if (entry == "sender-port") { + sender_port = getUint32("sender-port", value); + } else if (entry == "max-queue-size") { + max_queue_size = getUint32("max-queue-size", value); + } else if (entry == "ncr-protocol") { + ncr_protocol = getProtocol("ncr-protocol", value); + } else if (entry == "ncr-format") { + ncr_format = getFormat("ncr-format", value); + } else if (entry == "always-include-fqdn") { + always_include_fqdn = value->boolValue(); + } else if (entry == "allow-client-update") { + allow_client_update = value->boolValue(); + // currently unused + (void)allow_client_update; + } else if (entry == "override-no-update") { + override_no_update = value->boolValue(); + } else if (entry == "override-client-update") { + override_client_update = value->boolValue(); + } else if (entry == "replace-client-name") { + replace_client_name_mode = getMode("replace-client-name", value); + } else if (entry == "generated-prefix") { + generated_prefix = value->stringValue(); + } else { + isc_throw(DhcpConfigError, + "unsupported parameter '" << entry + << " (" << value->getPosition() << ")"); + } + } catch (const isc::data::TypeError&) { + isc_throw(DhcpConfigError, + "invalid value type specified for parameter '" << entry + << " (" << value->getPosition() << ")"); } + } - current_param = "generated-prefix"; - generated_prefix = getString(client_config, current_param); + // Qualifying-suffix is required when updates are enabled + if (enable_updates && !found_qualifying_suffix) { + isc_throw(DhcpConfigError, + "parameter 'qualifying-suffix' is required when " + "updates are enabled (" + << client_config->getPosition() << ")"); + } - // temporary fix + IOAddress sender_ip(0); + if (sender_ip_str.empty()) { + // The default sender IP depends on the server IP family + sender_ip = (server_ip.isV4() ? IOAddress::IPV4_ZERO_ADDRESS() : + IOAddress::IPV6_ZERO_ADDRESS()); + } else { try { - current_param = "qualifying-suffix"; - qualifying_suffix = getString(client_config, current_param); - } catch (const std::exception&) { - if (enable_updates) throw; + sender_ip = IOAddress(sender_ip_str); + } catch (const std::exception& ex) { + isc_throw(DhcpConfigError, "invalid address (" << sender_ip_str + << ") specified for parameter 'sender-ip' (" + << getPosition("sender-ip", client_config) << ")"); } - } catch (const std::exception& ex) { - isc_throw(D2ClientError, "D2ClientConfig error: " << ex.what() - << " (" << getPosition(current_param, client_config) << ")"); } // Now we check for logical errors. This repeats what is done in @@ -1363,7 +1433,8 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { } if (sender_ip.getFamily() != server_ip.getFamily()) { - isc_throw(D2ClientError, "D2ClientConfig error: address family mismatch: " + isc_throw(D2ClientError, + "D2ClientConfig error: address family mismatch: " << "server-ip: " << server_ip.toText() << " is: " << (server_ip.isV4() ? "IPv4" : "IPv6") << " while sender-ip: " << sender_ip.toText() @@ -1372,7 +1443,8 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) { } if (server_ip == sender_ip && server_port == sender_port) { - isc_throw(D2ClientError, "D2ClientConfig error: server and sender cannot" + isc_throw(D2ClientError, + "D2ClientConfig error: server and sender cannot" " share the exact same IP address/port: " << server_ip.toText() << "/" << server_port << " (" << getPosition("sender-ip", client_config) << ")"); diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index 7bd69ddde0..e90fe27a48 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -1860,14 +1860,12 @@ TEST_F(ParseConfigTest, invalidD2Config) { } std::string invalid_configs[] = { -#if simple_parser_get_position_got_fixed // Must supply qualifying-suffix when updates are enabled "{ \"dhcp-ddns\" :" " {" " \"enable-updates\" : true" " }" "}", -#endif // Invalid server ip value "{ \"dhcp-ddns\" :" " {"