Merge pull request #1756 from telefonicaid/hardening/1751_syntax_for_…

…uri_param_q_and_exists

Hardening/1751 syntax for uri param q and exists

CHANGES_NEXT_RELEASE

@@ -3,4 +3,6 @@
 - Fix: libmicrohttpd 0.9.48 included in contextBroker as static lib (previous Orion versions used 0.9.22 as dynamic library) (Issue #1675)
 - Fix: list of attribute names in URI param 'type' (Issue #1749)
 - Fix: long servicepath when getting in NGSIv2 (#1423)
-
+- Fix: syntax change in string query 'q' for exist and not-exist (Issue #1751)
+- Fix: sanity check for string query 'q' - detect 'left-hand-side missing' (Issue #1754)
+- Fix: more sanity checks for string query 'q' (q empty, parts of 'q' empty - parts of 'q' are separated by ';')

doc/apiary/v2/fiware-ngsiv2-reference.apib

@@ -24,7 +24,7 @@ context information including updates, queries, registrations and subscriptions.
 ## Acknowledgements
 
 The editors would like to express their gratitude to the following people who actively contributed to this specification:
-Juan José Hierro, Marcos Reyes, Ken Gunnar, Antonio López, ... 
+Juan José Hierro, Marcos Reyes, Ken Zangelin, Antonio López, ... 
 
 ## Status
 
@@ -506,12 +506,14 @@ The list of operators (and the format of the values they use) is as follows:
 In the case of equal or unequal, if the value to match include a `,`, you can use simple quote
 (`'`), e.g: `color=='light,green','deep,blue'`.
 
-Unary statements are composed of a unary operator (either `+`or `-`) and are used in two situations:
+Unary negatory statements use the unary operator `!`, while affirmative unary statements use no operator at all.
+The unary statements are used in two situations:
 
-+ To check for attribute existence. E.g. `+temperature`matches entities which have
-  a temperature attribute (no matter its value), while `-temperature` matches entities which do not have
++ To check for attribute existence. E.g. `temperature` matches entities that have
+  an attribute called 'temperature' (no matter its value), while `!temperature` matches entities that do not have
   a temperature attribute.
-+ To check for entity type existence, with the `type`keyword. E.g, `-type`   matches entities which do not have a type.
++ To check for entity type existence, with the `type` keyword. E.g, `!type` matches entities which do not have a type,
+  while `type` matches entities having a type.
   
  
 ## Geographical Queries

src/lib/mongoBackend/MongoGlobal.cpp

@@ -1378,18 +1378,32 @@ static bool addBsonFilter
 *   provide it in order to conform with function signature).
 *
 * I don't like too much to have a function with two quite parallel behaviours in the same logic. Probably a
-* smarter design will be to divide this function in two part: the one focused in string parsing and the one
-* focused in appliying filters to each different sytem (either in memory or as BSON).
+* smarter design would be to divide this function in two parts: one focused on string parsing and the other one
+* focused on applying filters to each different system (either in memory or as BSON).
 *
 */
 bool qStringFilters(const std::string& in, std::vector<BSONObj> &filters, ContextElementResponse* cerP)
 {
+  //
+  // Initial Sanity check (of the entire string)
+  // - Not empty
+  // - Not just a single ';'
+  // - Not two ';' in a row
+  //
+  if ((in == "")  || 
+      (in == ";") ||
+      (strstr(in.c_str(), ";;") != NULL))
+  {
+    return false;
+  }
+
   char* str         = strdup(in.c_str());
   char* toFree      = str;
   char* s;
   char* saver;
-
-  bool retval = true;
+  bool  rightHandSideMandatory = true;
+  bool  leftHandSideMandatory  = true;
+  bool  retval                 = true;
 
   while ((s = strtok_r(str, ";", &saver)) != NULL)
   {
@@ -1403,11 +1417,17 @@ bool qStringFilters(const std::string& in, std::vector<BSONObj> &filters, Contex
     s = wsStrip(s);
 
     //
-    // Rudimentary sanity checks
+    // Rudimentary sanity checks (of *this* part of 'q')
     //
-    // 1. If a range is present, the op MUST be either '==' OR '!=' 
+    // 1. If the 'q-part' is empty, error
+    // 2. If a range is present, the op MUST be either '==' OR '!=' 
     //
-    if (strstr(s, "..") != NULL)
+    if (*s == 0)
+    {
+      free(toFree);
+      return false;
+    }
+    else if (strstr(s, "..") != NULL)
     {
       if ((strstr(s, "==") == NULL) && (strstr(s, "!=") == NULL))
       {
@@ -1459,27 +1479,39 @@ bool qStringFilters(const std::string& in, std::vector<BSONObj> &filters, Contex
       right = &op[1];
       op    = (char*) ">";
     }
-    else if (s[0] == '-')
+    else if (s[0] == '!')
     {
       left  = (char*) "";
       op    = (char*) OPR_NOT_EXIST;
       right = wsStrip(&s[1]);
+      leftHandSideMandatory = false;
     }
-    else if (s[0] == '+')
+    else
     {
       left  = (char*) "";
       op    = (char*) OPR_EXIST;
-      right = wsStrip(&s[1]);
-    }
-    else
-    {
-      op    = (char*) "";
-      right = (char*) "";
+      right = wsStrip(s);
+      leftHandSideMandatory = false;
     }
 
     left  = wsStrip(left);
     right = wsStrip(right);
 
+    //
+    // Sanity check for left-hand and right-hand non-empty
+    //
+    if ((rightHandSideMandatory == true) && (*right == 0))
+    {
+      free(toFree);
+      return false;
+    }
+
+    if ((leftHandSideMandatory == true) && (*left == 0))
+    {
+      free(toFree);
+      return false;
+    }
+
     std::string opr = op;
 
     if ((opr == "==") || (opr == "!="))

src/lib/rest/rest.cpp

@@ -80,6 +80,7 @@ static unsigned int              maxConns;
 static unsigned int              threadPoolSize;
 
 
+
 /* ****************************************************************************
 *
 * uriArgumentGet - 
@@ -90,6 +91,14 @@ static int uriArgumentGet(void* cbDataP, MHD_ValueKind kind, const char* ckey, c
   std::string      key   = ckey;
   std::string      value = (val == NULL)? "" : val;
 
+  if (*val == 0)
+  {
+    OrionError error(SccBadRequest, std::string("Empty right-hand-side for URI param /") + ckey + "/");
+    ciP->httpStatusCode = SccBadRequest;
+    ciP->answer         = error.render(ciP, "");
+    return MHD_YES;
+  }
+
   if (key == URI_PARAM_NOTIFY_FORMAT)
   {
     if (strcasecmp(val, "xml") == 0)

test/functionalTest/cases/1316_create_subscription/post_subs_expr_q_exist_attr.test

@@ -31,15 +31,15 @@ accumulatorStart
 --SHELL--
 
 #
-# 01. Create subscription with q: +A
+# 01. Create subscription with q:A (attribute A exists)
 # 02. Create entity E1 with A (notif)
 # 03. Create entity E2 without A (not notif)
 # 04. Append attribute A to E2 (notif)
 # 05. Dump accumulator (2 notifications)
 #
 
-echo "01. Create subscription with q: +A"
-echo "=================================="
+echo "01. Create subscription with q:A (attribute A exists)"
+echo "====================================================="
 payload='
 {
     "subject": {
@@ -52,7 +52,7 @@ payload='
         "condition": {
             "attributes": [ ],
             "expression": {
-               "q": "+A"
+               "q": "A"
             }
          }
     },
@@ -112,8 +112,8 @@ echo
 
 
 --REGEXPECT--
-01. Create subscription with q: +A
-==================================
+01. Create subscription with q:A (attribute A exists)
+=====================================================
 HTTP/1.1 201 Created
 Content-Length: 0
 Location: /v2/subscriptions/REGEX([0-9a-f]{24})

test/functionalTest/cases/1316_create_subscription/post_subs_expr_q_not_exist_attr.test

@@ -31,14 +31,14 @@ accumulatorStart
 --SHELL--
 
 #
-# 01. Create subscription with q: -A
+# 01. Create subscription with q:!A (attribute A does not exist)
 # 02. Create entity E1 with A (not notif)
 # 03. Create entity E2 without A (notif)
 # 04. Dump accumulator (1 notifications)
 #
 
-echo "01. Create subscription with q: -A"
-echo "=================================="
+echo "01. Create subscription with q:!A (attribute A does not exist)"
+echo "=============================================================="
 payload='
 {
     "subject": {
@@ -51,7 +51,7 @@ payload='
         "condition": {
             "attributes": [ ],
             "expression": {
-               "q": "-A"
+               "q": "!A"
             }
          }
     },
@@ -101,8 +101,8 @@ echo
 
 
 --REGEXPECT--
-01. Create subscription with q: -A
-==================================
+01. Create subscription with q:!A (attribute A does not exist)
+==============================================================
 HTTP/1.1 201 Created
 Content-Length: 0
 Location: /v2/subscriptions/REGEX([0-9a-f]{24})