Browse Source

[4204] TokenOption uses binary option format for evaluation.

This fixes a bug in the TokenOption class which used the hexadecimal
format of the option when comparing with TokenHexString. Note that
TokenHexString is automatically converted to binary format for
evaluation.
Marcin Siodelski 9 years ago
parent
commit
99d13c7de0

+ 11 - 5
src/lib/dhcp/option.cc

@@ -220,8 +220,8 @@ Option::toString() {
     return (toText(0));
     return (toText(0));
 }
 }
 
 
-std::string
-Option::toHexString(const bool include_header) {
+std::vector<uint8_t>
+Option::toBinary(const bool include_header) {
     OutputBuffer buf(len());
     OutputBuffer buf(len());
     try {
     try {
         // If the option is too long, exception will be thrown. We allow
         // If the option is too long, exception will be thrown. We allow
@@ -233,12 +233,18 @@ Option::toHexString(const bool include_header) {
                   " of option " << getType() << ": " << ex.what());
                   " of option " << getType() << ": " << ex.what());
     }
     }
     const uint8_t* option_data = static_cast<const uint8_t*>(buf.getData());
     const uint8_t* option_data = static_cast<const uint8_t*>(buf.getData());
-    std::vector<uint8_t> option_vec;
 
 
     // Assign option data to a vector, with or without option header depending
     // Assign option data to a vector, with or without option header depending
     // on the value of "include_header" flag.
     // on the value of "include_header" flag.
-    option_vec.assign(option_data + (include_header ? 0 : getHeaderLen()),
-                      option_data + buf.getLength());
+    std::vector<uint8_t> option_vec(option_data + (include_header ? 0 : getHeaderLen()),
+                                    option_data + buf.getLength());
+    return (option_vec);
+}
+
+std::string
+Option::toHexString(const bool include_header) {
+    // Prepare binary version of the option.
+    std::vector<uint8_t> option_vec = toBinary(include_header);
 
 
     // Return hexadecimal representation prepended with 0x or empty string
     // Return hexadecimal representation prepended with 0x or empty string
     // if option has no payload and the header fields are excluded.
     // if option has no payload and the header fields are excluded.

+ 9 - 0
src/lib/dhcp/option.h

@@ -216,6 +216,15 @@ public:
     /// @return string that represents the value of the option.
     /// @return string that represents the value of the option.
     virtual std::string toString();
     virtual std::string toString();
 
 
+    /// @brief Returns binary representation of the option.
+    ///
+    /// @param include_header Boolean flag which indicates if the output should
+    /// also contain header fields. The default is that it shouldn't include
+    /// header fields.
+    ///
+    /// @return Vector holding binary representation of the option.
+    virtual std::vector<uint8_t> toBinary(const bool include_header = false);
+
     /// @brief Returns string containing hexadecimal representation of option.
     /// @brief Returns string containing hexadecimal representation of option.
     ///
     ///
     /// @param include_header Boolean flag which indicates if the output should
     /// @param include_header Boolean flag which indicates if the output should

+ 1 - 1
src/lib/eval/tests/evaluate_unittest.cc

@@ -217,7 +217,7 @@ TEST_F(EvaluateTest, optionHex) {
 
 
     ASSERT_NO_THROW(toption.reset(new TokenOption(100, TokenOption::HEXADECIMAL)));
     ASSERT_NO_THROW(toption.reset(new TokenOption(100, TokenOption::HEXADECIMAL)));
     e_.push_back(toption);
     e_.push_back(toption);
-    ASSERT_NO_THROW(tstring.reset(new TokenString("0x68756E6472656434")));
+    ASSERT_NO_THROW(tstring.reset(new TokenString("hundred4")));
     e_.push_back(tstring);
     e_.push_back(tstring);
     ASSERT_NO_THROW(tequal.reset(new TokenEqual()));
     ASSERT_NO_THROW(tequal.reset(new TokenEqual()));
     e_.push_back(tequal);
     e_.push_back(tequal);

+ 4 - 4
src/lib/eval/tests/token_unittest.cc

@@ -349,7 +349,7 @@ TEST_F(TokenTest, optionHexString4) {
     values_.pop();
     values_.pop();
 
 
     // Then the content of the option 100.
     // Then the content of the option 100.
-    EXPECT_EQ("0x68756E6472656434", values_.top());
+    EXPECT_EQ("hundred4", values_.top());
 }
 }
 
 
 // This test checks if a token representing an option identified by name is
 // This test checks if a token representing an option identified by name is
@@ -371,7 +371,7 @@ TEST_F(TokenTest, optionWithNameHexString4) {
     ASSERT_EQ(1, values_.size());
     ASSERT_EQ(1, values_.size());
 
 
     // Then the content of the option.
     // Then the content of the option.
-    EXPECT_EQ("0x68756E6472656434", values_.top());
+    EXPECT_EQ("hundred4", values_.top());
 }
 }
 
 
 // This test checks if a token representing an option value is able to extract
 // This test checks if a token representing an option value is able to extract
@@ -451,7 +451,7 @@ TEST_F(TokenTest, optionHexString6) {
     values_.pop();
     values_.pop();
 
 
     // Then the content of the option 100.
     // Then the content of the option 100.
-    EXPECT_EQ("0x68756E6472656436", values_.top());
+    EXPECT_EQ("hundred6", values_.top());
 }
 }
 
 
 // This test checks if a token representing an option identified by name is
 // This test checks if a token representing an option identified by name is
@@ -473,7 +473,7 @@ TEST_F(TokenTest, optionWithNameHexString6) {
     ASSERT_EQ(1, values_.size());
     ASSERT_EQ(1, values_.size());
 
 
     // Then the content of the option.
     // Then the content of the option.
-    EXPECT_EQ("0x68756E6472656436", values_.top());
+    EXPECT_EQ("hundred6", values_.top());
 }
 }
 
 
 // This test checks if a token representing an == operator is able to
 // This test checks if a token representing an == operator is able to

+ 11 - 5
src/lib/eval/token.cc

@@ -87,13 +87,19 @@ TokenOption::TokenOption(const std::string& option_name,
 void
 void
 TokenOption::evaluate(const Pkt& pkt, ValueStack& values) {
 TokenOption::evaluate(const Pkt& pkt, ValueStack& values) {
     OptionPtr opt = pkt.getOption(option_code_);
     OptionPtr opt = pkt.getOption(option_code_);
+    std::string opt_str;
     if (opt) {
     if (opt) {
-        values.push(representation_type_ == TEXTUAL ? opt->toString()
-                    : opt->toHexString());
-    } else {
-        // Option not found, push empty string
-        values.push("");
+        if (representation_type_ == TEXTUAL) {
+            opt_str = opt->toString();
+        } else {
+            std::vector<uint8_t> binary = opt->toBinary();
+            opt_str.assign(binary.begin(), binary.end());
+        }
     }
     }
+
+    // Push value of the option or empty string if there was no such option
+    // in the packet.
+    values.push(opt_str);
 }
 }
 
 
 void
 void