Merge branch 'trac4090' Add support for OptionSubstring to the eval code

doc/guide/logging.xml

@@ -219,6 +219,11 @@
             <simpara><command>kea-dhcp4.dhcpsrv</command> - this is a base
             logger for the libdhcpsrv library.</simpara>
           </listitem>
+          <listitem>
+            <simpara><command>kea-dhcp4.eval</command> - this logger is used
+            to log messages relating to the client classification expression
+            evaluation code.</simpara>
+          </listitem>
           <listitem>
             <simpara><command>kea-dhcp4.hooks</command> - this logger is used
             to log messages related to management of hooks libraries, e.g.
@@ -302,6 +307,11 @@
             <simpara><command>kea-dhcp6.dhcpsrv</command> - this is a base
             logger for the libdhcpsrv library.</simpara>
           </listitem>
+          <listitem>
+            <simpara><command>kea-dhcp6.eval</command> - this logger is used
+            to log messages relating to the client classification expression
+            evaluation code.</simpara>
+          </listitem>
           <listitem>
             <simpara><command>kea-dhcp6.hooks</command> - this logger is used
             to log messages related to management of hooks libraries, e.g.

src/lib/eval/Makefile.am

@@ -12,14 +12,21 @@ AM_CXXFLAGS += $(WARNING_NO_MISSING_FIELD_INITIALIZERS_CFLAG)
 
 lib_LTLIBRARIES = libkea-eval.la
 libkea_eval_la_SOURCES  =
+libkea_eval_la_SOURCES += eval_log.cc eval_log.h
 libkea_eval_la_SOURCES += token.cc token.h
 
+nodist_libkea_eval_la_SOURCES = eval_messages.h eval_messages.cc
+
 libkea_eval_la_CXXFLAGS = $(AM_CXXFLAGS)
 libkea_eval_la_CPPFLAGS = $(AM_CPPFLAGS)
 libkea_eval_la_LIBADD   = $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
 libkea_eval_la_LIBADD  += $(top_builddir)/src/lib/dhcp/libkea-dhcp++.la
+libkea_eval_la_LIBADD  += $(top_builddir)/src/lib/log/libkea-log.la
+libkea_eval_la_LIBADD  += $(top_builddir)/src/lib/util/libkea-util.la
+libkea_eval_la_LIBADD  += $(LOG4CPLUS_LIBS) $(CRYPTO_LIBS)
+
 libkea_eval_la_LDFLAGS  = -no-undefined -version-info 3:0:0
-libkea_eval_la_LDFLAGS += $(LOG4CPLUS_LIBS) $(CRYPTO_LDFLAGS)
+libkea_eval_la_LDFLAGS += $(CRYPTO_LDFLAGS)
 
 EXTRA_DIST  = eval.dox
 EXTRA_DIST += eval_messages.mes
@@ -29,6 +36,7 @@ eval_messages.h eval_messages.cc: s-messages
 
 s-messages: eval_messages.mes
 	$(top_builddir)/src/lib/log/compiler/message $(top_srcdir)/src/lib/eval/eval_messages.mes
+	touch $@
 
 # Tell Automake that the eval_messages.{cc,h} source files are created in the
 # build process, so it must create these before doing anything else. Although
@@ -39,4 +47,4 @@ s-messages: eval_messages.mes
 # first.
 BUILT_SOURCES = eval_messages.h eval_messages.cc
 
-CLEANFILES = eval_messages.h eval_messages.cc
+CLEANFILES = eval_messages.h eval_messages.cc s-messages

src/lib/eval/eval_log.cc

@@ -0,0 +1,26 @@
+// Copyright (C) 2015  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+/// Defines the logger used by the Eval (classification) code
+
+#include <eval/eval_log.h>
+
+namespace isc {
+namespace dhcp {
+
+isc::log::Logger eval_logger("eval");
+
+} // namespace dhcp
+} // namespace isc
+

src/lib/eval/eval_log.h

@@ -0,0 +1,49 @@
+// Copyright (C) 2015  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef EVAL_LOG_H
+#define EVAL_LOG_H
+
+#include <log/macros.h>
+#include <eval/eval_messages.h>
+
+namespace isc {
+namespace dhcp {
+
+/// @brief Eval debug Logging levels
+///
+/// Defines the levels used to output debug messages in the eval (classification) code.
+/// Note that higher numbers equate to more verbose (and detailed) output.
+
+// The first level traces normal operations,
+const int EVAL_DBG_TRACE = DBGLVL_TRACE_BASIC;
+
+// The next level traces each call to hook code.
+const int EVAL_DBG_CALLS = DBGLVL_TRACE_BASIC_DATA;
+
+// Additional information on the calls.  Report each call to a callout (even
+// if there are multiple callouts on a hook) and each status return.
+const int EVAL_DBG_EXTENDED_CALLS = DBGLVL_TRACE_DETAIL_DATA;
+
+/// @brief Eval Logger
+///
+/// Define the logger used to log messages.  We could define it in multiple
+/// modules, but defining in a single module and linking to it saves time and
+/// space.
+extern isc::log::Logger eval_logger;
+
+} // namespace dhcp
+} // namespace isc
+
+#endif // EVAL_LOG_H

src/lib/eval/eval_messages.mes

@@ -18,3 +18,8 @@ $NAMESPACE isc::dhcp
 This debug message indicates that the expression has been evaluated
 to said value. This message is mostly useful during debugging of the
 client classification expressions.
+
+% EVAL_SUBSTRING_BAD_PARAM_CONVERSION starting %1, length %2
+This debug message indicates that the parameter for the starting postion
+or length of the substring couldn't be converted to an integer.  In this
+case the substring routine returns an empty string.

src/lib/eval/tests/Makefile.am

@@ -2,6 +2,8 @@ SUBDIRS = .
 
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)
+AM_CPPFLAGS += -DLOGGING_SPEC_FILE=\"$(abs_top_srcdir)/src/lib/dhcpsrv/logging.spec\"
+
 AM_CXXFLAGS = $(KEA_CXXFLAGS)
 
 # Some versions of GCC warn about some versions of Boost regarding

src/lib/eval/tests/token_unittest.cc

@@ -60,6 +60,40 @@ public:
     OptionPtr option_str4_; ///< A string option for DHCPv4
     OptionPtr option_str6_; ///< A string option for DHCPv6
 
+
+    /// @brief Verify that the substring eval works properly
+    ///
+    /// This function takes the parameters and sets up the value
+    /// stack then executes the eval and checks the results.
+    ///
+    /// @param test_string The string to operate on
+    /// @param test_start The postion to start when getting a substring
+    /// @param test_length The length of the substring to get
+    /// @param result_string The expected result of the eval
+    void verifySubstringEval(const std::string& test_string,
+                             const std::string& test_start,
+                             const std::string& test_length,
+                             const std::string& result_string) {
+
+        // create the token
+        ASSERT_NO_THROW(t_.reset(new TokenSubstring()));
+
+        // push values on stack
+        values_.push(test_string);
+        values_.push(test_start);
+        values_.push(test_length);
+
+        // evaluate the token
+        EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_));
+
+        // verify results
+        ASSERT_EQ(1, values_.size());
+        EXPECT_EQ(result_string, values_.top());
+
+        // remove result
+        values_.pop();
+    }
+
     /// @todo: Add more option types here
 };
 
@@ -197,3 +231,177 @@ TEST_F(TokenTest, optionEqualTrue) {
 }
 
 };
+
+// This test checks if an a token representing a substring request
+// throws an exception if there aren't enough values on the stack.
+// The stack from the top is: length, start, string.
+// The actual packet is not used.
+TEST_F(TokenTest, substringNotEnoughValues) {
+    ASSERT_NO_THROW(t_.reset(new TokenSubstring()));
+
+    // Subsring requires three values on the stack, try
+    // with 0, 1 and 2 all should thorw an exception
+    EXPECT_THROW(t_->evaluate(*pkt4_, values_), EvalBadStack);
+
+    values_.push("");
+    EXPECT_THROW(t_->evaluate(*pkt4_, values_), EvalBadStack);
+
+    values_.push("0");
+    EXPECT_THROW(t_->evaluate(*pkt4_, values_), EvalBadStack);
+
+    // Three should work
+    values_.push("0");
+    EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_));
+
+    // As we had an empty string to start with we should have an empty
+    // one after the evaluate
+    ASSERT_EQ(1, values_.size());
+    EXPECT_EQ("", values_.top());
+}
+
+// Test getting the whole string in different ways
+TEST_F(TokenTest, substringWholeString) {
+    // Get the whole string
+    verifySubstringEval("foobar", "0", "6", "foobar");
+
+    // Get the whole string with "all"
+    verifySubstringEval("foobar", "0", "all", "foobar");
+
+    // Get the whole string with an extra long number
+    verifySubstringEval("foobar", "0", "123456", "foobar");
+
+    // Get the whole string counting from the back
+    verifySubstringEval("foobar", "-6", "all", "foobar");
+}
+
+// Test getting a suffix, in this case the last 3 characters
+TEST_F(TokenTest, substringTrailer) {
+    verifySubstringEval("foobar", "3", "3", "bar");
+    verifySubstringEval("foobar", "3", "all", "bar");
+    verifySubstringEval("foobar", "-3", "all", "bar");
+    verifySubstringEval("foobar", "-3", "123", "bar");
+}
+
+// Test getting the middle of the string in different ways
+TEST_F(TokenTest, substringMiddle) {
+    verifySubstringEval("foobar", "1", "4", "ooba");
+    verifySubstringEval("foobar", "-5", "4", "ooba");
+    verifySubstringEval("foobar", "-1", "-4", "ooba");
+    verifySubstringEval("foobar", "5", "-4", "ooba");
+}
+
+// Test getting the last letter in different ways
+TEST_F(TokenTest, substringLastLetter) {
+    verifySubstringEval("foobar", "5", "all", "r");
+    verifySubstringEval("foobar", "5", "1", "r");
+    verifySubstringEval("foobar", "5", "5", "r");
+    verifySubstringEval("foobar", "-1", "all", "r");
+    verifySubstringEval("foobar", "-1", "1", "r");
+    verifySubstringEval("foobar", "-1", "5", "r");
+}
+
+// Test we get only what is available if we ask for a longer string
+TEST_F(TokenTest, substringLength) {
+    // Test off the front
+    verifySubstringEval("foobar", "0", "-4", "");
+    verifySubstringEval("foobar", "1", "-4", "f");
+    verifySubstringEval("foobar", "2", "-4", "fo");
+    verifySubstringEval("foobar", "3", "-4", "foo");
+
+    // and the back
+    verifySubstringEval("foobar", "3", "4", "bar");
+    verifySubstringEval("foobar", "4", "4", "ar");
+    verifySubstringEval("foobar", "5", "4", "r");
+    verifySubstringEval("foobar", "6", "4", "");
+}
+
+// Test that we get nothing if the starting postion is out of the string
+TEST_F(TokenTest, substringStartingPosition) {
+    // Off the front
+    verifySubstringEval("foobar", "-7", "1", "");
+    verifySubstringEval("foobar", "-7", "-11", "");
+    verifySubstringEval("foobar", "-7", "all", "");
+
+    // and the back
+    verifySubstringEval("foobar", "6", "1", "");
+    verifySubstringEval("foobar", "6", "-11", "");
+    verifySubstringEval("foobar", "6", "all", "");
+}
+
+// Check what happens if we use strings that aren't numbers for start or length
+// We should return the empty string
+TEST_F(TokenTest, substringBadParams) {
+    verifySubstringEval("foobar", "0ick", "all", "");
+    verifySubstringEval("foobar", "ick0", "all", "");
+    verifySubstringEval("foobar", "ick", "all", "");
+    verifySubstringEval("foobar", "0", "ick", "");
+    verifySubstringEval("foobar", "0", "0ick", "");
+    verifySubstringEval("foobar", "0", "ick0", "");
+    verifySubstringEval("foobar", "0", "allaboard", "");
+}
+
+// lastly check that we don't get anything if the string is empty or
+// we don't ask for any characters from it.
+TEST_F(TokenTest, substringReturnEmpty) {
+    verifySubstringEval("", "0", "all", "");
+    verifySubstringEval("foobar", "0", "0", "");
+}
+
+// Check if we can use the substring and equal tokens together
+// We put the result on the stack first then the substring values
+// then evaluate the substring which should leave the original
+// result on the bottom with the substring result on next.
+// Evaulating the equals should produce true for the first
+// and false for the second.
+// throws an exception if there aren't enough values on the stack.
+// The stack from the top is: length, start, string.
+// The actual packet is not used.
+TEST_F(TokenTest, substringEquals) {
+    TokenPtr tequal;
+
+    ASSERT_NO_THROW(t_.reset(new TokenSubstring()));
+    ASSERT_NO_THROW(tequal.reset(new TokenEqual()));
+
+    // The final expected value
+    values_.push("ooba");
+
+    // The substring values
+    // Subsring requires three values on the stack, try
+    // with 0, 1 and 2 all should thorw an exception
+    values_.push("foobar");
+    values_.push("1");
+    values_.push("4");
+    EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_));
+
+    // we should have two values on the stack
+    ASSERT_EQ(2, values_.size());
+
+    // next the equals eval
+    EXPECT_NO_THROW(tequal->evaluate(*pkt4_, values_));
+    ASSERT_EQ(1, values_.size());
+    EXPECT_EQ("true", values_.top());
+
+    // get rid of the result
+    values_.pop();
+
+    // and try it again but with a bad final value
+    // The final expected value
+    values_.push("foob");
+
+    // The substring values
+    // Subsring requires three values on the stack, try
+    // with 0, 1 and 2 all should thorw an exception
+    values_.push("foobar");
+    values_.push("1");
+    values_.push("4");
+    EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_));
+
+    // we should have two values on the stack
+    ASSERT_EQ(2, values_.size());
+
+    // next the equals eval
+    EXPECT_NO_THROW(tequal->evaluate(*pkt4_, values_));
+    ASSERT_EQ(1, values_.size());
+    EXPECT_EQ("false", values_.top());
+
+}

src/lib/eval/token.cc

@@ -13,6 +13,8 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <eval/token.h>
+#include <eval/eval_log.h>
+#include <boost/lexical_cast.hpp>
 #include <string>
 
 using namespace isc::dhcp;
@@ -53,3 +55,76 @@ TokenEqual::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
     else
         values.push("false");
 }
+
+void
+TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
+
+    if (values.size() < 3) {
+        isc_throw(EvalBadStack, "Incorrect stack order. Expected at least "
+                  "3 values for substring operator, got " << values.size());
+    }
+
+    string len_str = values.top();
+    values.pop();
+    string start_str = values.top();
+    values.pop();
+    string string_str = values.top();
+    values.pop();
+
+    // If we have no string to start with we push an empty string and leave
+    if (string_str.empty()) {
+        values.push("");
+        return;
+    }
+
+    // Convert the starting position and length from strings to numbers
+    // the length may also be "all" in which case simply make it the
+    // length of the string.
+    // If we have a problem push an empty string and leave
+    int start_pos;
+    int length;
+    try {
+        start_pos = boost::lexical_cast<int>(start_str);
+        if (len_str == "all") {
+            length = string_str.length();
+        } else {
+            length = boost::lexical_cast<int>(len_str);
+        }
+    } catch (const boost::bad_lexical_cast&) {
+        LOG_DEBUG(eval_logger, EVAL_DBG_TRACE,
+                  EVAL_SUBSTRING_BAD_PARAM_CONVERSION)
+            .arg(start_str)
+            .arg(len_str);
+
+        values.push("");
+        return;
+    }
+
+    const int string_length = string_str.length();
+    // If the starting postion is outside of the string push an
+    // empty string and leave
+    if ((start_pos < -string_length) || (start_pos >= string_length)) {
+        values.push("");
+        return;
+    }
+
+    // Adjust the values to be something for substr.  We first figure out
+    // the starting postion, then update it and the length to get the
+    // characters before or after it depending on the sign of length
+    if (start_pos < 0) {
+        start_pos = string_length + start_pos;
+    }
+
+    if (length < 0) {
+        length = -length;
+        if (length <=  start_pos){
+            start_pos -= length;
+        } else {
+            length = start_pos;
+            start_pos = 0;
+        }
+    }
+
+    // and finally get the substring
+    values.push(string_str.substr(start_pos, length));
+}

src/lib/eval/token.h

@@ -56,7 +56,7 @@ public:
 /// - option[123] (a token that extracts value of option 123)
 /// - == (an operator that compares two other tokens)
 /// - substring(a,b,c) (an operator that takes three arguments: a string,
-///   first and last character)
+///   first character and length)
 class Token {
 public:
 
@@ -150,10 +150,65 @@ public:
     /// either "true" or "false". It requires at least two parameters to be
     /// present on stack.
     ///
-    /// @throw EvalBadStack if there's less than 2 values on stack
+    /// @throw EvalBadStack if there are less than 2 values on stack
     ///
-    /// @brief pkt (unused)
+    /// @param pkt (unused)
-    /// @brief values - stack of values (2 arguments will be poped, 1 result
+    /// @param values - stack of values (2 arguments will be popped, 1 result
+    ///        will be pushed)
+    void evaluate(const Pkt& pkt, ValueStack& values);
+};
+
+/// @brief Token that represents the substring operator (returns a portion
+/// of the supplied string)
+///
+/// This token represents substring(str, start, len)  An operator that takes three
+/// arguments: a string, the first character and the length.
+class TokenSubstring : public Token {
+public:
+    /// @brief Constructor (does nothing)
+    TokenSubstring() {}
+
+    /// @brief Extract a substring from a string
+    ///
+    /// Evaluation does not use packet information.  It requires at least
+    /// three values to be present on the stack.  It will consume the top
+    /// three values on the stack as parameters and push the resulting substring
+    /// onto the stack.  From the top it expects the values on the stack as:
+    /// -  len
+    /// -  start
+    /// -  str
+    ///
+    /// str is the string to extract a substring from.  If it is empty, an empty
+    /// string is pushed onto the value stack.
+    ///
+    /// start is the postion from which the code starts extracting the substring.
+    /// 0 is the first character and a negative number starts from the end, with
+    /// -1 being the last character.  If the starting point is outside of the
+    /// original string an empty string is pushed onto the value stack.
+    ///
+    /// length is the number of characters from the string to extract.
+    /// "all" means all remaining characters from start to the end of string.
+    /// A negative number means to go from start towards the beginning of
+    /// the string, but doesn't include start.
+    /// If length is longer than the remaining portion of string
+    /// then the entire remaining portion is placed on the value stack.
+    ///
+    /// The following examples all use the base string "foobar", the first number
+    /// is the starting position and the second is the length.  Note that
+    /// a negative length only selects which characters to extract it does not
+    /// indicate an attempt to reverse the string.
+    /// -  0, all => "foobar"
+    /// -  0,  6  => "foobar"
+    /// -  0,  4  => "foob"
+    /// -  2, all => "obar"
+    /// -  2,  6  => "obar"
+    /// - -1, all => "r"
+    /// - -1, -4  => "ooba"
+    ///
+    /// @throw EvalBadStack if there are less than 3 values on stack
+    ///
+    /// @param pkt (unused)
+    /// @param values - stack of values (3 arguments will be popped, 1 result
     ///        will be pushed)
     void evaluate(const Pkt& pkt, ValueStack& values);
 };