From: Thomas Markwalder Date: Wed, 16 Apr 2014 14:52:30 +0000 (-0400) Subject: [3121] Moved validation logic to build method for TSIGKeyInfo parsing X-Git-Tag: trac3434_base~46^2~4 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=199dbab6f3fe70e708e34446a5c75e7d5ca59f45;p=thirdparty%2Fkea.git [3121] Moved validation logic to build method for TSIGKeyInfo parsing Modified TSIGKeyInfoParser, TSIGKeyInfoListParser, and unit tests --- diff --git a/src/bin/d2/d2_config.cc b/src/bin/d2/d2_config.cc index 2442372ec8..435cc982cf 100644 --- a/src/bin/d2/d2_config.cc +++ b/src/bin/d2/d2_config.cc @@ -201,30 +201,7 @@ TSIGKeyInfoParser::build(isc::data::ConstElementPtr key_config) { parser->build(config_pair.second); parser->commit(); } -} - -isc::dhcp::ParserPtr -TSIGKeyInfoParser::createConfigParser(const std::string& config_id) { - DhcpConfigParser* parser = NULL; - // Based on the configuration id of the element, create the appropriate - // parser. Scalars are set to use the parser's local scalar storage. - if ((config_id == "name") || - (config_id == "algorithm") || - (config_id == "secret")) { - parser = new isc::dhcp::StringParser(config_id, - local_scalars_.getStringStorage()); - } else { - isc_throw(NotImplemented, - "parser error: TSIGKeyInfo parameter not supported: " - << config_id); - } - - // Return the new parser instance. - return (isc::dhcp::ParserPtr(parser)); -} -void -TSIGKeyInfoParser::commit() { std::string name; std::string algorithm; std::string secret; @@ -266,6 +243,32 @@ TSIGKeyInfoParser::commit() { (*keys_)[name]=key_info; } +isc::dhcp::ParserPtr +TSIGKeyInfoParser::createConfigParser(const std::string& config_id) { + DhcpConfigParser* parser = NULL; + // Based on the configuration id of the element, create the appropriate + // parser. Scalars are set to use the parser's local scalar storage. + if ((config_id == "name") || + (config_id == "algorithm") || + (config_id == "secret")) { + parser = new isc::dhcp::StringParser(config_id, + local_scalars_.getStringStorage()); + } else { + isc_throw(NotImplemented, + "parser error: TSIGKeyInfo parameter not supported: " + << config_id); + } + + // Return the new parser instance. + return (isc::dhcp::ParserPtr(parser)); +} + +void +TSIGKeyInfoParser::commit() { + /// @todo if at some point TSIG keys need some form of resource + /// initialization do that here +} + // *********************** TSIGKeyInfoListParser ************************* TSIGKeyInfoListParser::TSIGKeyInfoListParser(const std::string& list_name, diff --git a/src/bin/d2/d2_config.h b/src/bin/d2/d2_config.h index 9212ee2e07..e460e2258b 100644 --- a/src/bin/d2/d2_config.h +++ b/src/bin/d2/d2_config.h @@ -540,8 +540,9 @@ public: /// @brief Performs the actual parsing of the given "tsig_key" element. /// - /// The results of the parsing are retained internally for use during - /// commit. + /// Parses a configuration for the elements needed to instantiate a + /// TSIGKeyInfo, validates those entries, creates a TSIGKeyInfo instance + /// then attempts to add to a list of keys /// /// @param key_config is the "tsig_key" configuration to parse virtual void build(isc::data::ConstElementPtr key_config); @@ -559,9 +560,8 @@ public: /// @return returns a pointer to newly created parser. virtual isc::dhcp::ParserPtr createConfigParser(const std::string& config_id); - - /// @brief Instantiates a DnsServerInfo from internal data values - /// saves it to the storage area pointed to by servers_. + /// @brief commits the TSIGKeyInfo configuration + /// At the moment this method is a NOP. virtual void commit(); private: @@ -629,7 +629,7 @@ private: /// the list of newly created TSIGKeyInfo instances. This is given to us /// as a constructor argument by an upper level. TSIGKeyInfoMapPtr keys_; - + /// @brief Local storage area to which individual key parsers commit. TSIGKeyInfoMapPtr local_keys_; diff --git a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc index 75790f6eba..52ff2b5bba 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -261,9 +261,8 @@ TEST_F(TSIGKeyInfoTest, invalidEntry) { "}"; ASSERT_TRUE(fromJSON(config)); - // Verify that build succeeds but commit fails on blank name. - EXPECT_NO_THROW(parser_->build(config_set_)); - EXPECT_THROW(parser_->commit(), D2CfgError); + // Verify that build fails on blank name. + EXPECT_THROW(parser_->build(config_set_), D2CfgError); // Config with a blank algorithm entry. config = "{" @@ -274,9 +273,8 @@ TEST_F(TSIGKeyInfoTest, invalidEntry) { ASSERT_TRUE(fromJSON(config)); - // Verify that build succeeds but commit fails on blank algorithm. - EXPECT_NO_THROW(parser_->build(config_set_)); - EXPECT_THROW(parser_->commit(), D2CfgError); + // Verify that build fails on blank algorithm. + EXPECT_THROW(parser_->build(config_set_), D2CfgError); // Config with a blank secret entry. config = "{" @@ -287,9 +285,8 @@ TEST_F(TSIGKeyInfoTest, invalidEntry) { ASSERT_TRUE(fromJSON(config)); - // Verify that build succeeds but commit fails on blank secret. - EXPECT_NO_THROW(parser_->build(config_set_)); - EXPECT_THROW(parser_->commit(), D2CfgError); + // Verify that build fails blank secret + EXPECT_THROW(parser_->build(config_set_), D2CfgError); } /// @brief Verifies that TSIGKeyInfo parsing creates a proper TSIGKeyInfo @@ -347,10 +344,7 @@ TEST_F(TSIGKeyInfoTest, invalidTSIGKeyList) { ASSERT_NO_THROW(parser.reset(new TSIGKeyInfoListParser("test", keys_))); // Verify that the list builds without errors. - ASSERT_NO_THROW(parser->build(config_set_)); - - // Verify that the list commit fails. - EXPECT_THROW(parser->commit(), D2CfgError); + EXPECT_THROW(parser->build(config_set_), D2CfgError); } /// @brief Verifies that attempting to parse an invalid list of TSIGKeyInfo @@ -380,10 +374,7 @@ TEST_F(TSIGKeyInfoTest, duplicateTSIGKey) { ASSERT_NO_THROW(parser.reset(new TSIGKeyInfoListParser("test", keys_))); // Verify that the list builds without errors. - ASSERT_NO_THROW(parser->build(config_set_)); - - // Verify that the list commit fails. - EXPECT_THROW(parser->commit(), D2CfgError); + EXPECT_THROW(parser->build(config_set_), D2CfgError); } /// @brief Verifies a valid list of TSIG Keys parses correctly. @@ -1013,7 +1004,7 @@ TEST_F(D2CfgMgrTest, fullConfig) { EXPECT_TRUE(cfg_mgr_->reverseUpdatesEnabled()); // Verify that parsing the exact same configuration a second time - // does not cause a duplicate value errors. + // does not cause a duplicate value errors. answer_ = cfg_mgr_->parseConfig(config_set_); ASSERT_TRUE(checkAnswer(0)); }