From: Francis Dupont Date: Fri, 14 Jun 2019 20:33:52 +0000 (+0200) Subject: [150-add-sub-option-classification] Added unit tests and fixed logs X-Git-Tag: Kea-1.6.0~41^2~103 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=212e163abe143765d518703bf134ec8a8538743a;p=thirdparty%2Fkea.git [150-add-sub-option-classification] Added unit tests and fixed logs --- diff --git a/src/lib/eval/tests/context_unittest.cc b/src/lib/eval/tests/context_unittest.cc index 170ff2a82e..1670487747 100644 --- a/src/lib/eval/tests/context_unittest.cc +++ b/src/lib/eval/tests/context_unittest.cc @@ -404,7 +404,7 @@ public: EXPECT_EQ(expected_client_class, member->getClientClass()); } - /// @rief This tests attempts to parse the expression then checks + /// @rief This tests attempts to parse the expression then checks /// if the number of tokens is correct and the TokenMember is as /// expected. /// @@ -679,6 +679,26 @@ public: EXPECT_EQ(field, vendor->getField()); } + /// @brief checks if the given token is a sub-option with the expected + /// parent option and sub-option codes and representation type + /// @param token token to be checked + /// @param expected_code expected option code + /// @param expected_sub_code expected sub-option code + /// @param expected_repr expected representation (text, hex, exists) + void checkTokenSubOption(const TokenPtr& token, + uint16_t expected_code, + uint16_t expected_sub_code, + TokenOption::RepresentationType expected_repr) { + ASSERT_TRUE(token); + boost::shared_ptr sub = + boost::dynamic_pointer_cast(token); + ASSERT_TRUE(sub); + + EXPECT_EQ(expected_code, sub->getCode()); + EXPECT_EQ(expected_sub_code, sub->getSubCode()); + EXPECT_EQ(expected_repr, sub->getRepresentation()); + } + Option::Universe universe_; ///< Universe (V4 or V6) bool parsed_; ///< Parsing status @@ -1544,6 +1564,11 @@ TEST_F(EvalContextTest, typeErrors) { // Member uses quotes around the client class name. checkError("member(foo)", ":1.8: Invalid character: f"); + + // sub-option by name is not supported. + checkError("option[123].option[host-name].exists", + ":1.20-28: syntax error, unexpected option name, " + "expecting integer"); } @@ -1637,6 +1662,46 @@ TEST_F(EvalContextTest, vendorClass6DataIndex) { testVendorClass("vendor-class[4491].data[3] == 0x1234", Option::V6, 4491, 3); } +// Test the parsing of a sub-option with perent by code. +TEST_F(EvalContextTest, subOptionWithCode) { + EvalContext eval(Option::V4); + + EXPECT_NO_THROW(parsed_ = eval.parseString("option[123].option[234].text == 'foo'")); + EXPECT_TRUE(parsed_); + ASSERT_EQ(3, eval.expression.size()); + checkTokenSubOption(eval.expression.at(0), 123, 234, TokenOption::TEXTUAL); +} + +// Test the parsing of a sub-option with perent by name. +TEST_F(EvalContextTest, subOptionWithName) { + EvalContext eval(Option::V4); + + EXPECT_NO_THROW(parsed_ = eval.parseString("option[host-name].option[123].text == 'foo'")); + EXPECT_TRUE(parsed_); + ASSERT_EQ(3, eval.expression.size()); + checkTokenSubOption(eval.expression.at(0), 12, 123, TokenOption::TEXTUAL); +} + +// Test the parsing of a sub-option existence +TEST_F(EvalContextTest, subOptionExists) { + EvalContext eval(Option::V4); + + EXPECT_NO_THROW(parsed_ = eval.parseString("option[100].option[200].exists")); + EXPECT_TRUE(parsed_); + ASSERT_EQ(1, eval.expression.size()); + checkTokenSubOption(eval.expression.at(0), 100, 200, TokenOption::EXISTS); +} + +// Test parsing of a sub-option represented as hexadecimal string. +TEST_F(EvalContextTest, subOptionHex) { + EvalContext eval(Option::V4); + + EXPECT_NO_THROW(parsed_ = eval.parseString("option[123].option[234].hex == 0x666F6F")); + EXPECT_TRUE(parsed_); + ASSERT_EQ(3, eval.expression.size()); + checkTokenSubOption(eval.expression.at(0), 123, 234, TokenOption::HEXADECIMAL); +} + // Checks if integer expressions can be parsed and checked for equality. TEST_F(EvalContextTest, integer1) { diff --git a/src/lib/eval/tests/token_unittest.cc b/src/lib/eval/tests/token_unittest.cc index 69191211e1..a038ee7c9b 100644 --- a/src/lib/eval/tests/token_unittest.cc +++ b/src/lib/eval/tests/token_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2015-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2019 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -2910,6 +2910,169 @@ TEST_F(TokenTest, vendorClass6DataIndex) { EXPECT_TRUE(checkFile()); } +// This test checks that the existing RAI -sunoption can be found. +TEST_F(TokenTest, subOption) { + + // Insert relay option with sub-options 1 and 13 + insertRelay4Option(); + + // Creating the token should be safe. + ASSERT_NO_THROW(t_.reset(new TokenSubOption(DHO_DHCP_AGENT_OPTIONS, + 13, TokenOption::TEXTUAL))); + + // We should be able to evaluate it. + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + + // we should have one value on the stack + ASSERT_EQ(1, values_.size()); + + // The option should be found and option[82].option[13] should evaluate + // to thecontent of that sub-option, i.e. "thirteen" + EXPECT_EQ("thirteen", values_.top()); + + // Check that the debug output was correct. Add the strings + // to the test vector in the class and then call checkFile + // for comparison + addString("EVAL_DEBUG_SUB_OPTION Pushing option 82 " + "sub-option 13 with value 'thirteen'"); + EXPECT_TRUE(checkFile()); +} + +// This test checks that the code properly handles cases when +// there is a RAI option, but there's no requested sub-option. +TEST_F(TokenTest, subOptionNoSubOption) { + + // Insert relay option with sub-options 1 and 13 + insertRelay4Option(); + + + // Creating the token should be safe. + ASSERT_NO_THROW(t_.reset(new TokenSubOption(DHO_DHCP_AGENT_OPTIONS, + 15, TokenOption::TEXTUAL))); + + // We should be able to evaluate it. + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + + // we should have one value on the stack + ASSERT_EQ(1, values_.size()); + + // The option should NOT be found (there is no sub-option 15), + // so the expression should evaluate to "" + EXPECT_EQ("", values_.top()); + + // Check that the debug output was correct. Add the strings + // to the test vector in the class and then call checkFile + // for comparison + addString("EVAL_DEBUG_SUB_OPTION Pushing option 82 " + "sub-option 15 with value ''"); + EXPECT_TRUE(checkFile()); +} + +// This test checks that the code properly handles cases when +// there's no RAI option at all. +TEST_F(TokenTest, subOptionNoOption) { + + // We didn't call insertRelay4Option(), so there's no RAI option. + + // Creating the token should be safe. + ASSERT_NO_THROW(t_.reset(new TokenSubOption(DHO_DHCP_AGENT_OPTIONS, + 13, TokenOption::TEXTUAL))); + + // We should be able to evaluate it. + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + + // we should have one value on the stack + ASSERT_EQ(1, values_.size()); + + // The option should NOT be found (there is option 82), + // so the expression should evaluate to "" + EXPECT_EQ("", values_.top()); + + // Check that the debug output was correct. Add the strings + // to the test vector in the class and then call checkFile + // for comparison + addString("EVAL_DEBUG_SUB_OPTION_NO_OPTION Requested option 82 " + "sub-option 13, but the parent option is not present, " + "pushing result ''"); + EXPECT_TRUE(checkFile()); +} + +// This test checks that only the requested parent is searched for +// the requested sub-option. +TEST_F(TokenTest, subOptionOptionOnly) { + + // Insert relay option with sub-options 1 and 13 + insertRelay4Option(); + + // Add options 13 and 70 to the packet. + OptionPtr opt13(new OptionString(Option::V4, 13, "THIRTEEN")); + OptionPtr opt70(new OptionString(Option::V4, 70, "SEVENTY")); + pkt4_->addOption(opt13); + pkt4_->addOption(opt70); + + // The situation is as follows: + // Packet: + // - option 13 (containing "THIRTEEN") + // - option 82 (rai) + // - option 1 (containing "one") + // - option 13 (containing "thirteen") + + // Let's try to get option 13. It should get the one from RAI + ASSERT_NO_THROW(t_.reset(new TokenSubOption(DHO_DHCP_AGENT_OPTIONS, + 13, TokenOption::TEXTUAL))); + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + ASSERT_EQ(1, values_.size()); + EXPECT_EQ("thirteen", values_.top()); + + // Try to get option 1. It should get the one from RAI + clearStack(); + ASSERT_NO_THROW(t_.reset(new TokenSubOption(DHO_DHCP_AGENT_OPTIONS, + 1, TokenOption::TEXTUAL))); + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + ASSERT_EQ(1, values_.size()); + EXPECT_EQ("one", values_.top()); + + // Try to get option 70. It should fail, as there's no such + // sub option in RAI. + clearStack(); + ASSERT_NO_THROW(t_.reset(new TokenSubOption(DHO_DHCP_AGENT_OPTIONS, + 70, TokenOption::TEXTUAL))); + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + ASSERT_EQ(1, values_.size()); + EXPECT_EQ("", values_.top()); + + // Try to check option 1. It should return "true" + clearStack(); + ASSERT_NO_THROW(t_.reset(new TokenSubOption(DHO_DHCP_AGENT_OPTIONS, + 1, TokenOption::EXISTS))); + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + ASSERT_EQ(1, values_.size()); + EXPECT_EQ("true", values_.top()); + + // Try to check option 70. It should return "false" + clearStack(); + ASSERT_NO_THROW(t_.reset(new TokenSubOption(DHO_DHCP_AGENT_OPTIONS, + 70, TokenOption::EXISTS))); + EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_)); + ASSERT_EQ(1, values_.size()); + EXPECT_EQ("false", values_.top()); + + // Check that the debug output was correct. Add the strings + // to the test vector in the class and then call checkFile + // for comparison + addString("EVAL_DEBUG_SUB_OPTION Pushing option 82 " + "sub-option 13 with value 'thirteen'"); + addString("EVAL_DEBUG_SUB_OPTION Pushing option 82 " + "sub-option 1 with value 'one'"); + addString("EVAL_DEBUG_SUB_OPTION Pushing option 82 " + "sub-option 70 with value ''"); + addString("EVAL_DEBUG_SUB_OPTION Pushing option 82 " + "sub-option 1 with value 'true'"); + addString("EVAL_DEBUG_SUB_OPTION Pushing option 82 " + "sub-option 70 with value 'false'"); + EXPECT_TRUE(checkFile()); +} + // Checks if various values can be represented as integer tokens TEST_F(TokenTest, integer) { testInteger(encode(0), 0); diff --git a/src/lib/eval/token.cc b/src/lib/eval/token.cc index f96dabe669..482a9cd597 100644 --- a/src/lib/eval/token.cc +++ b/src/lib/eval/token.cc @@ -995,50 +995,49 @@ void TokenSubOption::evaluate(Pkt& pkt, ValueStack& values) { OptionPtr parent = getOption(pkt); std::string txt; + isc::log::MessageID msgid = EVAL_DEBUG_SUB_OPTION; if (!parent) { - // There's no parent option, give up. - txt = pushFailure(values); - LOG_DEBUG(eval_logger, EVAL_DBG_STACK, EVAL_DEBUG_SUB_OPTION_NO_OPTION) - .arg(option_code_) - .arg(sub_option_code_) - .arg(txt); - return; - } - - OptionPtr sub = getSubOption(parent); - if (!sub) { - // Failed to find the sub-option - txt = pushFailure(values); - LOG_DEBUG(eval_logger, EVAL_DBG_STACK, EVAL_DEBUG_SUB_OPTION) - .arg(option_code_) - .arg(sub_option_code_) - .arg(txt); - return; - } - - if (representation_type_ == TEXTUAL) { - txt = sub->toString(); - } else if (representation_type_ == HEXADECIMAL) { - std::vector binary = sub->toBinary(); - txt.resize(binary.size()); - if (!binary.empty()) { - memmove(&txt[0], &binary[0], binary.size()); + // There's no parent option, notify that. + msgid = EVAL_DEBUG_SUB_OPTION_NO_OPTION; + if (representation_type_ == EXISTS) { + txt = "false"; } } else { - txt = "true"; + OptionPtr sub = getSubOption(parent); + if (!sub) { + // Failed to find the sub-option + if (representation_type_ == EXISTS) { + txt = "false"; + } + } else { + if (representation_type_ == TEXTUAL) { + txt = sub->toString(); + } else if (representation_type_ == HEXADECIMAL) { + std::vector binary = sub->toBinary(); + txt.resize(binary.size()); + if (!binary.empty()) { + memmove(&txt[0], &binary[0], binary.size()); + } + } else { + txt = "true"; + } + } } + + // Push value of the sub-option or empty string if there was no + // such parent option in the packet or sub-option in the parent. values.push(txt); // Log what we pushed, both exists and textual are simple text // and can be output directly. We also include the code numbers // of the requested parent option and sub-option. if (representation_type_ == HEXADECIMAL) { - LOG_DEBUG(eval_logger, EVAL_DBG_STACK, EVAL_DEBUG_SUB_OPTION) + LOG_DEBUG(eval_logger, EVAL_DBG_STACK, msgid) .arg(option_code_) .arg(sub_option_code_) .arg(toHex(txt)); } else { - LOG_DEBUG(eval_logger, EVAL_DBG_STACK, EVAL_DEBUG_SUB_OPTION) + LOG_DEBUG(eval_logger, EVAL_DBG_STACK, msgid) .arg(option_code_) .arg(sub_option_code_) .arg('\'' + txt + '\'');