Browse Source

[4088fd] Assume the parser produces only well typed expressions

Francis Dupont 9 years ago
parent
commit
29282dfa0f

+ 0 - 5
src/lib/eval/eval_messages.mes

@@ -18,8 +18,3 @@ $NAMESPACE isc::dhcp
 This debug message indicates that the expression has been evaluated
 This debug message indicates that the expression has been evaluated
 to said value. This message is mostly useful during debugging of the
 to said value. This message is mostly useful during debugging of the
 client classification expressions.
 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.

+ 23 - 16
src/lib/eval/tests/token_unittest.cc

@@ -72,10 +72,12 @@ public:
     /// @param test_start The postion to start when getting a substring
     /// @param test_start The postion to start when getting a substring
     /// @param test_length The length of the substring to get
     /// @param test_length The length of the substring to get
     /// @param result_string The expected result of the eval
     /// @param result_string The expected result of the eval
+    /// @param should_throw The eval will throw
     void verifySubstringEval(const std::string& test_string,
     void verifySubstringEval(const std::string& test_string,
                              const std::string& test_start,
                              const std::string& test_start,
                              const std::string& test_length,
                              const std::string& test_length,
-                             const std::string& result_string) {
+                             const std::string& result_string,
+                             bool should_throw = false) {
 
 
         // create the token
         // create the token
         ASSERT_NO_THROW(t_.reset(new TokenSubstring()));
         ASSERT_NO_THROW(t_.reset(new TokenSubstring()));
@@ -86,14 +88,19 @@ public:
         values_.push(test_length);
         values_.push(test_length);
 
 
         // evaluate the token
         // evaluate the token
-        EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_));
+        if (should_throw) {
-
+            EXPECT_THROW(t_->evaluate(*pkt4_, values_), EvalTypeError);
-        // verify results
+            ASSERT_EQ(0, values_.size());
-        ASSERT_EQ(1, values_.size());
+        } else {
-        EXPECT_EQ(result_string, values_.top());
+            EXPECT_NO_THROW(t_->evaluate(*pkt4_, values_));
-
+
-        // remove result
+            // verify results
-        values_.pop();
+            ASSERT_EQ(1, values_.size());
+            EXPECT_EQ(result_string, values_.top());
+
+            // remove result
+            values_.pop();
+        }
     }
     }
 
 
     /// @todo: Add more option types here
     /// @todo: Add more option types here
@@ -443,13 +450,13 @@ TEST_F(TokenTest, substringStartingPosition) {
 // Check what happens if we use strings that aren't numbers for start or length
 // Check what happens if we use strings that aren't numbers for start or length
 // We should return the empty string
 // We should return the empty string
 TEST_F(TokenTest, substringBadParams) {
 TEST_F(TokenTest, substringBadParams) {
-    verifySubstringEval("foobar", "0ick", "all", "");
+    verifySubstringEval("foobar", "0ick", "all", "", true);
-    verifySubstringEval("foobar", "ick0", "all", "");
+    verifySubstringEval("foobar", "ick0", "all", "", true);
-    verifySubstringEval("foobar", "ick", "all", "");
+    verifySubstringEval("foobar", "ick", "all", "", true);
-    verifySubstringEval("foobar", "0", "ick", "");
+    verifySubstringEval("foobar", "0", "ick", "", true);
-    verifySubstringEval("foobar", "0", "0ick", "");
+    verifySubstringEval("foobar", "0", "0ick", "", true);
-    verifySubstringEval("foobar", "0", "ick0", "");
+    verifySubstringEval("foobar", "0", "ick0", "", true);
-    verifySubstringEval("foobar", "0", "allaboard", "");
+    verifySubstringEval("foobar", "0", "allaboard", "", true);
 }
 }
 
 
 // lastly check that we don't get anything if the string is empty or
 // lastly check that we don't get anything if the string is empty or

+ 9 - 7
src/lib/eval/token.cc

@@ -121,19 +121,21 @@ TokenSubstring::evaluate(const Pkt& /*pkt*/, ValueStack& values) {
     int length;
     int length;
     try {
     try {
         start_pos = boost::lexical_cast<int>(start_str);
         start_pos = boost::lexical_cast<int>(start_str);
+    } catch (const boost::bad_lexical_cast&) {
+        isc_throw(EvalTypeError, "the parameter '" << start_str
+                  << "' for the starting postion of the substring "
+                  << "couldn't be converted to an integer.");
+    }
+    try {
         if (len_str == "all") {
         if (len_str == "all") {
             length = string_str.length();
             length = string_str.length();
         } else {
         } else {
             length = boost::lexical_cast<int>(len_str);
             length = boost::lexical_cast<int>(len_str);
         }
         }
     } catch (const boost::bad_lexical_cast&) {
     } catch (const boost::bad_lexical_cast&) {
-        LOG_DEBUG(eval_logger, EVAL_DBG_TRACE,
+        isc_throw(EvalTypeError, "the parameter '" << len_str
-                  EVAL_SUBSTRING_BAD_PARAM_CONVERSION)
+                  << "' for the length of the substring "
-            .arg(start_str)
+                  << "couldn't be converted to an integer.");
-            .arg(len_str);
-
-        values.push("");
-        return;
     }
     }
 
 
     const int string_length = string_str.length();
     const int string_length = string_str.length();

+ 11 - 1
src/lib/eval/token.h

@@ -38,7 +38,7 @@ typedef std::vector<TokenPtr> Expression;
 /// Evaluated values are stored as a stack of strings
 /// Evaluated values are stored as a stack of strings
 typedef std::stack<std::string> ValueStack;
 typedef std::stack<std::string> ValueStack;
 
 
-/// @brief EvalStackError is thrown when more or less parameters are on the
+/// @brief EvalBadStack is thrown when more or less parameters are on the
 ///        stack than expected.
 ///        stack than expected.
 class EvalBadStack : public Exception {
 class EvalBadStack : public Exception {
 public:
 public:
@@ -46,6 +46,14 @@ public:
         isc::Exception(file, line, what) { };
         isc::Exception(file, line, what) { };
 };
 };
 
 
+/// @brief EvalTypeError is thrown when a value on the stack has a content
+///        with an unexpected type.
+class EvalTypeError : public Exception {
+public:
+    EvalTypeError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
 /// @brief Base class for all tokens
 /// @brief Base class for all tokens
 ///
 ///
 /// It provides an interface for all tokens and storage for string representation
 /// It provides an interface for all tokens and storage for string representation
@@ -241,6 +249,8 @@ public:
     /// - -1, -4  => "ooba"
     /// - -1, -4  => "ooba"
     ///
     ///
     /// @throw EvalBadStack if there are less than 3 values on stack
     /// @throw EvalBadStack if there are less than 3 values on stack
+    /// @throw EvalTypeError if start is not a number or length a number or
+    ///        the special value "all".
     ///
     ///
     /// @param pkt (unused)
     /// @param pkt (unused)
     /// @param values - stack of values (3 arguments will be popped, 1 result
     /// @param values - stack of values (3 arguments will be popped, 1 result