Bug/2763 bug fix not refactor

CHANGES_NEXT_RELEASE

@@ -15,6 +15,9 @@
 - Fix: bug when tenants are over 44 bytes long (#2811)
 - Add: Support for null values in string filters (#2359)
 - Fix: Partial sanity check for custon url, only if the url contains no replacements using ${xxx} (#2280, #2279)
+- Fix: if entity::type is given but as an empty string, in payload of POST /v2/subscriptions or PATCH /v2/subscriptions/{sub}, an error is returned (#1985)
+- Fix: admin requests (such as GET /version) with errors now return '400 Bad Request', not '200 OK'
+- Fix: attempt to create an entity with an error in service path (and probably more types of errors) now returns '400 Bad Request', not '200 OK' (#2763)
 - Fix: not calling regfree when regcomp fails. Potential fix for a crash (#2769)
 - Fix: wrongly overlogging metadata abscense in csub docs as Runtime Error (#2796)
 - Fix: if HTTP header Content-Length contains a value above the maximum payload size, an error is returned and the message is not read (#2761)

doc/manuals/user/ngsiv2_implementation_notes.md

@@ -144,7 +144,7 @@ The particular validations that Orion implements on NGSIv2 subscription payloads
         * **id** or **idPattern**: one of them is mandatory (but both at the same time is not allowed). id
             must follow NGSIv2 restrictions for IDs. idPattern must be not empty and a valid regex.
         * **type** or **typePattern**: optional (but both at the same time is not allowed). type must 
-            follow NGSIv2 restrictions for IDs. typePattern must be not empty and a valid regex.
+            follow NGSIv2 restrictions for IDs. type must not be empty. typePattern must be a valid regex, and non-empty.
     * **condition**: optional (but if present it must have a content, i.e. `{}` is not allowed)
         * **attrs**: optional (but if present it must be a list; empty list is allowed)
         * **expression**: optional (but if present it must have a content, i.e. `{}` is not allowed)

src/lib/apiTypesV2/Entity.cpp

@@ -216,8 +216,8 @@ std::string Entity::check(ApiVersion apiVersion, RequestType requestType)
   {
     if (forbiddenIdChars(apiVersion, id.c_str()))
     {
-      alarmMgr.badInput(clientIp, "found a forbidden character in the id of an entity");
-      return "Invalid characters in entity id";
+      alarmMgr.badInput(clientIp, ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTID);
+      return ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTID;
     }
   }
 
@@ -244,8 +244,8 @@ std::string Entity::check(ApiVersion apiVersion, RequestType requestType)
   {
     if (forbiddenIdChars(apiVersion, type.c_str()))
     {
-      alarmMgr.badInput(clientIp, "found a forbidden character in the type of an entity");
-      return "Invalid characters in entity type";
+      alarmMgr.badInput(clientIp, ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTTYPE);
+      return ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTTYPE;
     }
   }
 

src/lib/common/errorMessages.h

@@ -56,6 +56,14 @@
 #define ERROR_DESC_BAD_REQUEST_EMPTY_ENTITY_TYPE      "entity type length: 0, min length supported: "    STR(MIN_ID_LEN)
 #define ERROR_DESC_BAD_REQUEST_EMPTY_PAYLOAD          "empty payload"
 #define ERROR_DESC_BAD_REQUEST_EMPTY_ENTITIES_VECTOR  "empty entities vector"
+#define ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTID     "Invalid characters in entity id"
+#define ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTTYPE   "Invalid characters in entity type"
+#define ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTID    "Invalid JSON type for entity id"
+#define ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTTYPE  "Invalid JSON type for entity type"
+#define ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTIDPATTERN     "Invalid JSON type for entity idPattern"
+#define ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTTYPEPATTERN   "Invalid JSON type for entity typePattern"
+#define ERROR_DESC_BAD_REQUEST_INVALID_REGEX_ENTIDPATTERN     "Invalid regex for entity idPattern"
+#define ERROR_DESC_BAD_REQUEST_INVALID_REGEX_ENTTYPEPATTERN   "Invalid regex for entity typePattern"
 
 #define ERROR_NOT_FOUND                               "NotFound"
 #define ERROR_DESC_NOT_FOUND_ENTITY                   "The requested entity has not been found. Check type and id"
@@ -67,6 +75,6 @@
 #define ERROR_TOO_MANY                                "TooManyResults"
 #define ERROR_DESC_TOO_MANY_ENTITIES                  "More than one matching entity. Please refine your query"
 
-#define ERROR_DESC_BAD_VERB                       "method not allowed"
+#define ERROR_DESC_BAD_VERB                           "method not allowed"
 
 #endif // SRC_LIB_COMMON_ERRORMESSAGES_H

src/lib/jsonParseV2/parseEntity.cpp

@@ -32,6 +32,7 @@
 #include "jsonParseV2/jsonParseTypeNames.h"
 #include "jsonParseV2/parseEntity.h"
 #include "jsonParseV2/parseContextAttribute.h"
+#include "parse/forbiddenChars.h"
 #include "alarmMgr/alarmMgr.h"
 
 using namespace rapidjson;
@@ -139,34 +140,68 @@ std::string parseEntity(ConnectionInfo* ciP, Entity* eP, bool eidInURL)
       {
         if (type != "String")
         {
-          alarmMgr.badInput(clientIp, "invalid JSON type for entity id");
-          ciP->httpStatusCode = SccBadRequest;;
-          OrionError oe(SccBadRequest, "invalid JSON type for entity id", "BadRequest");
+          alarmMgr.badInput(clientIp, ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTID);
+          ciP->httpStatusCode = SccBadRequest;
+          OrionError oe(SccBadRequest, ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTID, "BadRequest");
+
           return oe.toJson();
         }
 
         eP->id = iter->value.GetString();
+
+        if (forbiddenIdChars(ciP->apiVersion, eP->id.c_str(), ""))
+        {
+          alarmMgr.badInput(clientIp, ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTID);
+          ciP->httpStatusCode = SccBadRequest;
+          OrionError oe(SccBadRequest, ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTID, "BadRequest");
+
+          return oe.toJson();
+        }
       }
       else  // "id" is present in payload for /v2/entities/<eid> - not a valid payload
       {
-        alarmMgr.badInput(clientIp, "'id' is not a valid attribute");
-        ciP->httpStatusCode = SccBadRequest;;
-        OrionError oe(SccBadRequest, "invalid input, 'id' as attribute", "BadRequest");
+        const char* errorText = "invalid input, 'id' as attribute";
+
+        alarmMgr.badInput(clientIp, errorText);
+        ciP->httpStatusCode = SccBadRequest;
+        OrionError oe(SccBadRequest, errorText, "BadRequest");
+
         return oe.toJson();
       }
     }
     else if (name == "type")
     {
       if (type != "String")
       {
-        alarmMgr.badInput(clientIp, "invalid JSON type for entity type");
+        alarmMgr.badInput(clientIp, ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTTYPE);
         ciP->httpStatusCode = SccBadRequest;
-        OrionError oe(SccBadRequest, "invalid JSON type for entity type", "BadRequest");
+        OrionError oe(SccBadRequest, ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTTYPE, "BadRequest");
+
         return oe.toJson();
       }
 
       eP->type      = iter->value.GetString();
       eP->typeGiven = true;
+
+      if (eP->type.empty())
+      {
+        const char* errorText = "entity type length: 0, min length supported: 1";
+
+        alarmMgr.badInput(clientIp, errorText);
+        ciP->httpStatusCode = SccBadRequest;
+        OrionError oe(SccBadRequest, errorText, "BadRequest");
+
+        return oe.toJson();
+      }
+
+      if (forbiddenIdChars(ciP->apiVersion, eP->type.c_str(), ""))
+      {
+        alarmMgr.badInput(clientIp, ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTTYPE);
+        ciP->httpStatusCode = SccBadRequest;
+        OrionError oe(SccBadRequest, ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTTYPE, "BadRequest");
+
+        return oe.toJson();
+      }
     }
     else  // attribute
     {

src/lib/jsonParseV2/parseEntityObject.cpp

@@ -24,6 +24,7 @@
 */
 #include "rapidjson/document.h"
 
+#include "common/errorMessages.h"
 #include "rest/ConnectionInfo.h"
 #include "ngsi/ParseData.h"
 #include "ngsi/Request.h"
@@ -68,27 +69,27 @@ std::string parseEntityObject(ConnectionInfo* ciP, Value::ConstValueIterator val
     {
       if (type != "String")
       {
-        return "invalid JSON type for entity id";
+        return ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTID;
       }
 
       eP->id = iter->value.GetString();
 
-      if (forbiddenChars(eP->id.c_str(), ""))
+      if (forbiddenIdChars(ciP->apiVersion, eP->id.c_str(), ""))
       {
-        return "Invalid characters in entity id";
+        return ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTID;
       }
     }
     else if (name == "idPattern")
     {
       if (type != "String")
       {
-        return "invalid JSON type for entity idPattern";
+        return ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTIDPATTERN;
       }
 
       regex_t re;
       if (regcomp(&re, iter->value.GetString(), REG_EXTENDED) != 0)
       {
-        return "invalid regex for entity id pattern";
+        return ERROR_DESC_BAD_REQUEST_INVALID_REGEX_ENTIDPATTERN;
       }
       regfree(&re);  // If regcomp fails it frees up itself (see glibc sources for details)
 
@@ -99,28 +100,33 @@ std::string parseEntityObject(ConnectionInfo* ciP, Value::ConstValueIterator val
     {
       if (type != "String")
       {
-        return "invalid JSON type for entity type";
+        return ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTTYPE;
       }
 
       eP->type      = iter->value.GetString();
       eP->typeGiven = true;
 
-      if (forbiddenChars(eP->type.c_str(), ""))
+      if (eP->type.empty())
       {
-        return "Invalid characters in entity type";
+        return "entity type length: 0, min length supported: 1";
+      }
+
+      if (forbiddenIdChars(ciP->apiVersion, eP->type.c_str(), ""))
+      {
+        return ERROR_DESC_BAD_REQUEST_INVALID_CHAR_ENTTYPE;
       }
     }
     else if (name == "typePattern")
     {
       if (type != "String")
       {
-        return "invalid JSON type for entity typePattern";
+        return ERROR_DESC_BAD_REQUEST_INVALID_JTYPE_ENTTYPEPATTERN;
       }
 
       regex_t re;
       if (regcomp(&re, iter->value.GetString(), REG_EXTENDED) != 0)
       {
-        return "invalid regex for entity type pattern";
+        return ERROR_DESC_BAD_REQUEST_INVALID_REGEX_ENTTYPEPATTERN;
       }
       regfree(&re);  // If regcomp fails it frees up itself (see glibc sources for details)
 

src/lib/jsonParseV2/parseSubscription.cpp

@@ -362,7 +362,7 @@ static std::string parseEntitiesVector(ConnectionInfo* ciP, std::vector<EntID>*
         regex_t re;
         if (regcomp(&re, idPattern.c_str(), REG_EXTENDED) != 0)
         {
-          return badInput(ciP, "Invalid regex for entity id pattern");
+          return badInput(ciP, ERROR_DESC_BAD_REQUEST_INVALID_REGEX_ENTIDPATTERN);
         }
         regfree(&re);  // If regcomp fails it frees up itself
       }
@@ -384,6 +384,10 @@ static std::string parseEntitiesVector(ConnectionInfo* ciP, std::vector<EntID>*
         {
           return badInput(ciP, "max type length exceeded");
         }
+        if (typeOpt.value.empty())
+        {
+          return badInput(ciP, "entity type length: 0, min length supported: 1");
+        }
         type = typeOpt.value;
       }
     }
@@ -407,7 +411,7 @@ static std::string parseEntitiesVector(ConnectionInfo* ciP, std::vector<EntID>*
         regex_t re;
         if (regcomp(&re, typePattern.c_str(), REG_EXTENDED) != 0)
         {
-          return badInput(ciP, "Invalid regex for entity id pattern");
+          return badInput(ciP, ERROR_DESC_BAD_REQUEST_INVALID_REGEX_ENTTYPEPATTERN);
         }
         regfree(&re);  // If regcomp fails it frees up itself
       }

src/lib/rest/OrionError.cpp

@@ -109,7 +109,7 @@ std::string OrionError::smartRender(ApiVersion apiVersion)
 */
 std::string OrionError::setStatusCodeAndSmartRender(ApiVersion apiVersion, HttpStatusCode* scP)
 {
-  if (apiVersion == V2)
+  if ((apiVersion == V2) || (apiVersion == ADMIN_API))
   {
     *scP = code;
   }

src/lib/serviceRoutinesV2/postEntities.cpp

@@ -100,13 +100,22 @@ std::string postEntities
   // 02. Call standard op postUpdateContext
   postUpdateContext(ciP, components, compV, parseDataP, NGSIV2_FLAVOUR_ONCREATE);
 
-  // 03. Check error
+  //
+  // 03. Check error - 3 different ways to get an error from postUpdateContext ... :-(
+  //     FIXME P4: make postUpdateContext have ONE way to return errors. See github issue #2763
+  //
   std::string  answer = "";
-  if (parseDataP->upcrs.res.oe.code != SccNone )
+  if (parseDataP->upcrs.res.oe.code != SccNone)
   {
     TIMED_RENDER(answer = parseDataP->upcrs.res.oe.toJson());
     ciP->httpStatusCode = parseDataP->upcrs.res.oe.code;
   }
+  else if (parseDataP->upcrs.res.errorCode.code != SccOk)
+  {
+    ciP->httpStatusCode = parseDataP->upcrs.res.errorCode.code;
+    TIMED_RENDER(answer = parseDataP->upcrs.res.errorCode.toJson(true));
+    ciP->answer         = answer;
+  }
   else
   {
     // Prepare HTTP headers

test/functionalTest/cases/0970_create_entity/errors_at_creation.test

@@ -185,7 +185,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)
 
 {
-    "description": "invalid JSON type for entity id", 
+    "description": "Invalid JSON type for entity id", 
     "error": "BadRequest"
 }
 
@@ -199,7 +199,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)
 
 {
-    "description": "invalid JSON type for entity id", 
+    "description": "Invalid JSON type for entity id", 
     "error": "BadRequest"
 }
 
@@ -213,7 +213,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)
 
 {
-    "description": "invalid JSON type for entity type", 
+    "description": "Invalid JSON type for entity type", 
     "error": "BadRequest"
 }
 
@@ -227,7 +227,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)
 
 {
-    "description": "invalid JSON type for entity type", 
+    "description": "Invalid JSON type for entity type", 
     "error": "BadRequest"
 }
 

test/functionalTest/cases/1108_non_string_fields/non_string_fields.test

@@ -120,7 +120,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)
 
 {
-    "description": "invalid JSON type for entity id",
+    "description": "Invalid JSON type for entity id",
     "error": "BadRequest"
 }
 
@@ -134,7 +134,7 @@ Fiware-Correlator: REGEX([0-9a-f\-]{36})
 Date: REGEX(.*)
 
 {
-    "description": "invalid JSON type for entity type",
+    "description": "Invalid JSON type for entity type",
     "error": "BadRequest"
 }
 

test/functionalTest/cases/1728_bug_forbidden_chars/restrict_chars_entity.test → test/functionalTest/cases/1728_bug_forbidden_chars/forbidden_chars_entity.test

@@ -21,7 +21,7 @@
 # VALGRIND_READY - to mark the test ready for valgrindTestSuite.sh
 
 --NAME--
-Restict chars in id (old set)
+Forbidden chars in id (old set)
 
 --SHELL-INIT--
 dbInit CB