From: Francis Dupont Date: Wed, 2 Oct 2019 16:03:40 +0000 (+0200) Subject: [219-allow-an-option-value-to-be-set-from-an-expression] Checkpoint: code and test... X-Git-Tag: Kea-1.7.1~40 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b391eebae6aded6ca085b697b53ccf63c5673824;p=thirdparty%2Fkea.git [219-allow-an-option-value-to-be-set-from-an-expression] Checkpoint: code and test done, todo docs --- diff --git a/src/hooks/dhcp/flex_option/flex_option.cc b/src/hooks/dhcp/flex_option/flex_option.cc index c1f4703da7..1d8f1e53bc 100644 --- a/src/hooks/dhcp/flex_option/flex_option.cc +++ b/src/hooks/dhcp/flex_option/flex_option.cc @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -19,6 +20,7 @@ using namespace isc; using namespace isc::data; using namespace isc::dhcp; using namespace isc::eval; +using namespace isc::log; using namespace std; namespace isc { @@ -225,5 +227,46 @@ FlexOptionImpl::parseOptionConfig(ConstElementPtr option) { option_config_map_[code] = opt_cfg; } +void +FlexOptionImpl::logAction(Action action, uint16_t code, + const string& value) const { + if (action == NONE) { + return; + } + if (action == REMOVE) { + LOG_DEBUG(flex_option_logger, DBGLVL_TRACE_BASIC, + FLEX_OPTION_PROCESS_REMOVE) + .arg(code); + return; + } + bool printable = true; + for (const unsigned char& ch : value) { + if (isprint(ch) == 0) { + printable = false; + break; + } + } + ostringstream repr; + if (printable) { + repr << "'" << value << "'"; + } else { + repr << "0x" << hex; + for (const unsigned char& ch : value) { + repr << setw(2) << setfill('0') << static_cast(ch); + } + } + if (action == SUPERSEDE) { + LOG_DEBUG(flex_option_logger, DBGLVL_TRACE_BASIC, + FLEX_OPTION_PROCESS_SUPERSEDE) + .arg(code) + .arg(repr.str()); + } else { + LOG_DEBUG(flex_option_logger, DBGLVL_TRACE_BASIC, + FLEX_OPTION_PROCESS_ADD) + .arg(code) + .arg(repr.str()); + } +} + } // end of namespace flex_option } // end of namespace isc diff --git a/src/hooks/dhcp/flex_option/flex_option.h b/src/hooks/dhcp/flex_option/flex_option.h index fb820d312b..681f40fcd1 100644 --- a/src/hooks/dhcp/flex_option/flex_option.h +++ b/src/hooks/dhcp/flex_option/flex_option.h @@ -163,6 +163,7 @@ public: opt.reset(new isc::dhcp::Option(universe, opt_cfg->getCode(), buffer)); response->addOption(opt); + logAction(ADD, opt_cfg->getCode(), value); break; case SUPERSEDE: value = isc::dhcp::evaluateString(*opt_cfg->getExpr(), *query); @@ -177,6 +178,7 @@ public: opt.reset(new isc::dhcp::Option(universe, opt_cfg->getCode(), buffer)); response->addOption(opt); + logAction(SUPERSEDE, opt_cfg->getCode(), value); break; case REMOVE: if (!opt) { @@ -189,11 +191,19 @@ public: response->delOption(opt_cfg->getCode()); opt = response->getOption(opt_cfg->getCode()); } + logAction(REMOVE, opt_cfg->getCode(), ""); break; } } } + /// @brief Log the action. + /// + /// @param action The action. + /// @param code The option code. + /// @param value The option value ("" for remove). + void logAction(Action action, uint16_t code, const std::string& value) const; + protected: /// @brief Get a mutable reference to the option config map /// diff --git a/src/hooks/dhcp/flex_option/flex_option_callouts.cc b/src/hooks/dhcp/flex_option/flex_option_callouts.cc index 63626836f2..2122de6a72 100644 --- a/src/hooks/dhcp/flex_option/flex_option_callouts.cc +++ b/src/hooks/dhcp/flex_option/flex_option_callouts.cc @@ -41,6 +41,11 @@ extern "C" { /// /// @return 0 upon success, non-zero otherwise int pkt4_send(CalloutHandle& handle) { + // Sanity. + if (!impl) { + return (0); + } + // Get the parameters. Pkt4Ptr query; Pkt4Ptr response; @@ -68,6 +73,11 @@ int pkt4_send(CalloutHandle& handle) { /// /// @return 0 upon success, non-zero otherwise int pkt6_send(CalloutHandle& handle) { + // Sanity. + if (!impl) { + return (0); + } + // Get the parameters. Pkt6Ptr query; Pkt6Ptr response; diff --git a/src/hooks/dhcp/flex_option/flex_option_log.h b/src/hooks/dhcp/flex_option/flex_option_log.h index 9fc3361b67..d8c55695fe 100644 --- a/src/hooks/dhcp/flex_option/flex_option_log.h +++ b/src/hooks/dhcp/flex_option/flex_option_log.h @@ -9,6 +9,7 @@ #include #include +#include #include namespace isc { diff --git a/src/hooks/dhcp/flex_option/flex_option_messages.cc b/src/hooks/dhcp/flex_option/flex_option_messages.cc index 3dcb13015c..78bebd4d75 100644 --- a/src/hooks/dhcp/flex_option/flex_option_messages.cc +++ b/src/hooks/dhcp/flex_option/flex_option_messages.cc @@ -1,18 +1,24 @@ -// File created from ../../../../src/hooks/dhcp/flex_option/flex_option_messages.mes on Tue Oct 01 2019 14:08 +// File created from ../../../../src/hooks/dhcp/flex_option/flex_option_messages.mes on Wed Oct 02 2019 17:43 #include #include #include extern const isc::log::MessageID FLEX_OPTION_LOAD_ERROR = "FLEX_OPTION_LOAD_ERROR"; +extern const isc::log::MessageID FLEX_OPTION_PROCESS_ADD = "FLEX_OPTION_PROCESS_ADD"; extern const isc::log::MessageID FLEX_OPTION_PROCESS_ERROR = "FLEX_OPTION_PROCESS_ERROR"; +extern const isc::log::MessageID FLEX_OPTION_PROCESS_REMOVE = "FLEX_OPTION_PROCESS_REMOVE"; +extern const isc::log::MessageID FLEX_OPTION_PROCESS_SUPERSEDE = "FLEX_OPTION_PROCESS_SUPERSEDE"; extern const isc::log::MessageID FLEX_OPTION_UNLOAD = "FLEX_OPTION_UNLOAD"; namespace { const char* values[] = { "FLEX_OPTION_LOAD_ERROR", "loading Flex Option hooks library failed: %1", + "FLEX_OPTION_PROCESS_ADD", "Added the option code %1 value by %2", "FLEX_OPTION_PROCESS_ERROR", "An error occurred processing query %1: %2", + "FLEX_OPTION_PROCESS_REMOVE", "Removed option code %1", + "FLEX_OPTION_PROCESS_SUPERSEDE", "Supersedes the value of option code %1 by %2", "FLEX_OPTION_UNLOAD", "Flex Option hooks library has been unloaded", NULL }; diff --git a/src/hooks/dhcp/flex_option/flex_option_messages.h b/src/hooks/dhcp/flex_option/flex_option_messages.h index 224936e03e..ad20a896b5 100644 --- a/src/hooks/dhcp/flex_option/flex_option_messages.h +++ b/src/hooks/dhcp/flex_option/flex_option_messages.h @@ -1,4 +1,4 @@ -// File created from ../../../../src/hooks/dhcp/flex_option/flex_option_messages.mes on Tue Oct 01 2019 14:08 +// File created from ../../../../src/hooks/dhcp/flex_option/flex_option_messages.mes on Wed Oct 02 2019 17:43 #ifndef FLEX_OPTION_MESSAGES_H #define FLEX_OPTION_MESSAGES_H @@ -6,7 +6,10 @@ #include extern const isc::log::MessageID FLEX_OPTION_LOAD_ERROR; +extern const isc::log::MessageID FLEX_OPTION_PROCESS_ADD; extern const isc::log::MessageID FLEX_OPTION_PROCESS_ERROR; +extern const isc::log::MessageID FLEX_OPTION_PROCESS_REMOVE; +extern const isc::log::MessageID FLEX_OPTION_PROCESS_SUPERSEDE; extern const isc::log::MessageID FLEX_OPTION_UNLOAD; #endif // FLEX_OPTION_MESSAGES_H diff --git a/src/hooks/dhcp/flex_option/flex_option_messages.mes b/src/hooks/dhcp/flex_option/flex_option_messages.mes index 5b3b1617e7..d2f1cdf6d3 100644 --- a/src/hooks/dhcp/flex_option/flex_option_messages.mes +++ b/src/hooks/dhcp/flex_option/flex_option_messages.mes @@ -5,12 +5,26 @@ This error message indicates an error during loading the Flex Option hooks library. The details of the error are provided as argument of the log message. +% FLEX_OPTION_PROCESS_ADD Added the option code %1 value by %2 +This debug message is printed when an option was added into the response +packet. The option code and the value (between quotes if printable, in +hexadecimal is not) are provided. + % FLEX_OPTION_PROCESS_ERROR An error occurred processing query %1: %2 This error message indicates an error during processing of a query by the Flex Option hooks library. The client identification information from the query and the details of the error are provided as arguments of the log message. +% FLEX_OPTION_PROCESS_REMOVE Removed option code %1 +This debug message is printed when an option was removed from the response +packet. The option code is provided. + +% FLEX_OPTION_PROCESS_SUPERSEDE Supersedes the value of option code %1 by %2 +This debug message is printed when an option was superseded into the response +packet. The option code and the value (between quotes if printable, in +hexadecimal is not) are provided. + % FLEX_OPTION_UNLOAD Flex Option hooks library has been unloaded This info message indicates that the Flex Option hooks library has been unloaded. diff --git a/src/hooks/dhcp/flex_option/tests/flex_option_unittests.cc b/src/hooks/dhcp/flex_option/tests/flex_option_unittests.cc index 5c69b5d274..c3a0360d67 100644 --- a/src/hooks/dhcp/flex_option/tests/flex_option_unittests.cc +++ b/src/hooks/dhcp/flex_option/tests/flex_option_unittests.cc @@ -674,7 +674,7 @@ TEST_F(FlexOptionTest, optionConfigMultipleAction) { errmsg.str(""); errmsg << "multiple actions: " << option->str(); EXPECT_EQ(errmsg.str(), impl_->getErrMsg()); - + // add and remove. option->remove("supersede"); option->set("add", add); @@ -860,7 +860,7 @@ TEST_F(FlexOptionTest, processSupersedeExisting) { options->add(option); ElementPtr code = Element::create(D6O_BOOTFILE_URL); option->set("code", code); - ElementPtr supersede = Element::create(string("'abc'")); + ElementPtr supersede = Element::create(string("0xabcdef")); option->set("supersede", supersede); EXPECT_NO_THROW(impl_->testConfigure(options)); EXPECT_TRUE(impl_->getErrMsg().empty()); @@ -877,7 +877,8 @@ TEST_F(FlexOptionTest, processSupersedeExisting) { EXPECT_EQ(D6O_BOOTFILE_URL, opt->getType()); const OptionBuffer& buffer = opt->getData(); ASSERT_EQ(3, buffer.size()); - EXPECT_EQ(0, memcmp(&buffer[0], "abc", 3)); + uint8_t expected[] = { 0xab, 0xcd, 0xef }; + EXPECT_EQ(0, memcmp(&buffer[0], expected, 3)); } // Verify that SUPERSEDE action does not supersede an empty value.