- Add test for illegal range queries.

- Improve error handling so we will not access uninitialized memory.

2 files changed, 58 insertions, 36 deletions

diff --git a/searchlib/src/tests/query/streaming_query_test.cpp b/searchlib/src/tests/query/streaming_query_test.cpp
index 306456518b7..08705fa837b 100644
--- a/searchlib/src/tests/query/streaming_query_test.cpp
+++ b/searchlib/src/tests/query/streaming_query_test.cpp
@@ -522,6 +522,7 @@ void assertInt64Range(const std::string &term, bool expAdjusted, int64_t expLow,
     EXPECT_EQ(expHigh, (int64_t)res.high);
 }
 
+
 TEST(StreamingQueryTest, require_that_int8_limits_are_enforced)
 {
     //std::numeric_limits<int8_t>::min() -> -128
@@ -607,6 +608,20 @@ TEST(StreamingQueryTest, require_that_we_can_take_floating_point_values_in_range
     assertInt64Range("[1.7976931348623157E308;-1.7976931348623157E308]", false, std::numeric_limits<int64_t>::max(), std::numeric_limits<int64_t>::min());
 }
 
+void assertIllegalRangeQueries(const QueryTermSimple & qt) {
+    QueryTermSimple::RangeResult<int64_t> ires = qt.getRange<int64_t>();
+    EXPECT_EQ(false, ires.valid);
+    QueryTermSimple::RangeResult<double> fres = qt.getRange<double>();
+    EXPECT_EQ(false, fres.valid);
+}
+
+TEST(StreamingQueryTest, require_safe_parsing_of_illegal_ranges) {
+    // The 2 below are created when naively splitting numeric terms by dot.
+    // T=A.B => T EQUIV PHRASE(A, B)
+    assertIllegalRangeQueries(QueryTermSimple("[1", TermType::WORD));
+    assertIllegalRangeQueries(QueryTermSimple(".1;2.1]", TermType::WORD));
+}
+
 TEST(StreamingQueryTest, require_that_we_handle_empty_range_as_expected)
 {
     assertInt64Range("[1;1]", false, 1, 1);
diff --git a/searchlib/src/vespa/searchlib/query/query_term_simple.cpp b/searchlib/src/vespa/searchlib/query/query_term_simple.cpp
index b7a1719fb5f..8df7764c183 100644
--- a/searchlib/src/vespa/searchlib/query/query_term_simple.cpp
+++ b/searchlib/src/vespa/searchlib/query/query_term_simple.cpp
@@ -259,46 +259,53 @@ template <typename T, typename D>
 bool
 QueryTermSimple::getAsNumericTerm(T & lower, T & upper, D d) const
 {
-    bool valid(empty());
+    if (empty()) return false;
+
     size_t sz(_term.size());
-    if (sz) {
-        char *err(nullptr);
-        T low(lower);
-        T high(upper);
-        const char * q = _term.c_str();
-        const char first(q[0]);
-        const char last(q[sz-1]);
-        q += ((first == '<') || (first == '>') || (first == '[')) ? 1 : 0;
-        T ll = d.fromstr(q, &err);
-        valid = isValid() && ((*err == 0) || (*err == ';'));
-        if (valid) {
-            if (first == '<' && (*err == 0)) {
-                high = d.nearestDownwd(ll, lower);
-            } else if (first == '>' && (*err == 0)) {
-                low = d.nearestUpward(ll, upper);
-            } else if ((first == '[') || (first == '<')) {
-                if (q != err) {
-                    low = (first == '[') ? ll : d.nearestUpward(ll, upper);
-                }
-                q = err + 1;
-                T hh = d.fromstr(q, &err);
-                bool hasUpperLimit(q != err);
-                if (*err == ';') {
-                    err = const_cast<char *>(_term.end() - 1);
-                }
-                valid = (*err == last) && ((last == ']') || (last == '>'));
-                if (hasUpperLimit) {
-                    high = (last == ']') ? hh : d.nearestDownwd(hh, lower);
-                }
-            } else {
-                low = high = ll;
-            }
+    char *err(nullptr);
+    T low(lower);
+    T high(upper);
+    const char * q = _term.c_str();
+    const char first(q[0]);
+    const char last(q[sz-1]);
+    bool isRange = (first == '<') || (first == '>') || (first == '[');
+    q += isRange ? 1 : 0;
+    T ll = d.fromstr(q, &err);
+    bool valid = isValid() && ((*err == 0) || (*err == ';'));
+    if (!valid) return false;
+
+    if (*err == 0) {
+        if (first == '<') {
+            high = d.nearestDownwd(ll, lower);
+        } else if (first == '>') {
+            low = d.nearestUpward(ll, upper);
+        } else {
+            low = high = ll;
+            valid = ! isRange;
         }
-        if (valid) {
-            lower = low;
-            upper = high;
+    } else {
+        if ((first == '[') || (first == '<')) {
+            if (q != err) {
+                low = (first == '[') ? ll : d.nearestUpward(ll, upper);
+            }
+            q = err + 1;
+            T hh = d.fromstr(q, &err);
+            bool hasUpperLimit(q != err);
+            if (*err == ';') {
+                err = const_cast<char *>(_term.end() - 1);
+            }
+            valid = (*err == last) && ((last == ']') || (last == '>'));
+            if (hasUpperLimit) {
+                high = (last == ']') ? hh : d.nearestDownwd(hh, lower);
+            }
+        } else {
+            valid = false;
         }
     }
+    if (valid) {
+        lower = low;
+        upper = high;
+    }
     return valid;
 }