[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 170ff2a82e2809277f97a6148108886d6667d2dc..1670487747e1e0bb32fb62ae14f4598836571164 100644 (file)
--- 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<TokenSubOption> sub =
+            boost::dynamic_pointer_cast<TokenSubOption>(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)", "<string>:1.8: Invalid character: f");
+
+    // sub-option by name is not supported.
+    checkError("option[123].option[host-name].exists",
+               "<string>: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 69191211e1726ed69270957f11bebdbbf356cda3..a038ee7c9b1cf56b965708f4605746be24b396b5 100644 (file)
--- 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 f96dabe6692665708a7418ef076a309d9db72988..482a9cd597bea4ee1223a3df0bda55e311c1564c 100644 (file)
--- 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<uint8_t> 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<uint8_t> 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 + '\'');