More logging for attribute value == null in pCheckAttribute (user request, issue #1312)

CHANGES_NEXT_RELEASE

@@ -43,3 +43,4 @@
 * Issue #1307  Implemented notifications for entity deletion, for DELETE /entities/{entityId}
 * Issue #280   Implemented forwarding for entity deletion, for DELETE /entities/{entityId}
 * Issue #280   Implemented forwarding for Batch entity deletion, for POST /entityOperations/delete
+* Issue #1312  Better error handling for a specific use case about RHS being 'null'

src/lib/orionld/forwarding/distOpSend.cpp

@@ -546,7 +546,7 @@ bool distOpSend(DistOp* distOpP, const char* dateHeader, const char* xForwardedF
 
   if (tenant != NULL)
   {
-    char tenantHeader[64];
+    char tenantHeader[80];
     snprintf(tenantHeader, sizeof(tenantHeader), "NGSILD-Tenant: %s", tenant);
     headers = curl_slist_append(headers, tenantHeader);
   }

src/lib/orionld/legacyDriver/legacyPatchAttribute.cpp

@@ -790,7 +790,7 @@ bool legacyPatchAttribute(void)
   if (attrTypeInDb != NULL)
     attributeType = orionldAttributeType(attrTypeInDb);
 
-  if (pCheckAttribute(inAttribute, true, attributeType, true, contextItemP) == false)
+  if (pCheckAttribute(entityId, inAttribute, true, attributeType, true, contextItemP) == false)
     return false;
 
 

src/lib/orionld/legacyDriver/legacyPatchEntity.cpp

@@ -199,7 +199,7 @@ bool legacyPatchEntity(void)
     if (dbTypeP != NULL)
       attributeType = orionldAttributeType(dbTypeP->value.s);
 
-    if (pCheckAttribute(newAttrP, true, attributeType, true, contextItemP) == false)
+    if (pCheckAttribute(entityId, newAttrP, true, attributeType, true, contextItemP) == false)
     {
       //
       // A failure will set a 400 (probably) in orionldState.pd.status

src/lib/orionld/legacyDriver/legacyPostEntity.cpp

@@ -261,7 +261,7 @@ bool legacyPostEntity(void)
     OrionldContextItem*  contextItemP = NULL;
     char*                shortName    = attrP->name;
 
-    if (pCheckAttribute(attrP, true, NoAttributeType, false, contextItemP) == false)
+    if (pCheckAttribute(entityId, attrP, true, NoAttributeType, false, contextItemP) == false)
     {
       attributeNotUpdated(notUpdatedP, shortName, orionldState.pd.title, orionldState.pd.detail);
 

src/lib/orionld/payloadCheck/pCheckAttribute.cpp

@@ -190,6 +190,7 @@ static bool pCheckTypeFromContext(KjNode* attrP, OrionldContextItem* attrContext
 //
 inline bool pCheckAttributeString
 (
+  const char*           entityId,
   KjNode*               attrP,
   bool                  isAttribute,
   OrionldAttributeType  attrTypeFromDb,
@@ -237,6 +238,7 @@ inline bool pCheckAttributeString
 //
 inline bool pCheckAttributeInteger
 (
+  const char*           entityId,
   KjNode*               attrP,
   bool                  isAttribute,
   OrionldAttributeType  attrTypeFromDb,
@@ -259,6 +261,7 @@ inline bool pCheckAttributeInteger
 //
 inline bool pCheckAttributeFloat
 (
+  const char*           entityId,
   KjNode*               attrP,
   bool                  isAttribute,
   OrionldAttributeType  attrTypeFromDb,
@@ -281,6 +284,7 @@ inline bool pCheckAttributeFloat
 //
 inline bool pCheckAttributeBoolean
 (
+  const char*           entityId,
   KjNode*               attrP,
   bool                  isAttribute,
   OrionldAttributeType  attrTypeFromDb,
@@ -303,6 +307,7 @@ inline bool pCheckAttributeBoolean
 //
 inline bool pCheckAttributeArray
 (
+  const char*           entityId,
   KjNode*               attrP,
   bool                  isAttribute,
   OrionldAttributeType  attrTypeFromDb,
@@ -328,7 +333,7 @@ inline bool pCheckAttributeArray
 //
 // pCheckAttributeNull -
 //
-inline bool pCheckAttributeNull(KjNode* attrP)
+inline bool pCheckAttributeNull(const char* entityId, KjNode* attrP)
 {
 #if 0
   LM_W(("RHS for attribute '%s' is NULL - that is forbidden in the NGSI-LD API", attrP->name));
@@ -790,6 +795,7 @@ static bool isJsonLiteral(KjNode* attrP, KjNode* typeP)
 //
 static bool pCheckAttributeObject
 (
+  const char*           entityId,                // For log messages only, still important
   KjNode*               attrP,
   bool                  isAttribute,
   OrionldAttributeType  attrTypeFromDb,
@@ -963,7 +969,12 @@ static bool pCheckAttributeObject
     {
       if ((orionldState.serviceP->options & ORIONLD_SERVICE_OPTION_ACCEPT_JSONLD_NULL) == 0)
       {
-        orionldError(OrionldBadRequestData, "null is not allowed as RHS in a JSON-LD document", fieldP->name, 400);
+        char errorString[512];
+        snprintf(errorString, sizeof(errorString),
+                 "Got a JSON 'null' in RHS (not allowed in JSON-LD docs) for the entity '%s', attribute '%s', attribute field '%s'",
+                 entityId, attrP->name, fieldP->name);
+        LM_E(("%s", errorString));
+        orionldError(OrionldBadRequestData, "Bad Input", errorString, 400);
         return false;
       }
 
@@ -1045,7 +1056,7 @@ static bool pCheckAttributeObject
     }
     else
     {
-      if (pCheckAttribute(fieldP, false, NoAttributeType, false, NULL) == false)
+      if (pCheckAttribute(entityId, fieldP, false, NoAttributeType, false, NULL) == false)
         return false;
     }
 
@@ -1215,6 +1226,7 @@ static bool validAttrName(const char* attrName, bool isAttribute)
 //
 bool pCheckAttribute
 (
+  const char*             entityId,
   KjNode*                 attrP,
   bool                    isAttribute,
   OrionldAttributeType    attrTypeFromDb,
@@ -1268,7 +1280,7 @@ bool pCheckAttribute
         // => KjNode* datasetsP should be a parameter to this function
         //
         aInstanceP->name = attrP->name;
-        if (pCheckAttribute(aInstanceP, true, attrTypeFromDb, attrNameAlreadyExpanded, attrContextInfoP) == false)
+        if (pCheckAttribute(entityId, aInstanceP, true, attrTypeFromDb, attrNameAlreadyExpanded, attrContextInfoP) == false)
           return false;
       }
 
@@ -1293,13 +1305,13 @@ bool pCheckAttribute
   //   We don't know which sub-attributes are special until we know the type of the attribute (Property, Relationship, etc)
   //   Postponed to ... all simple Property functions - pCheckAttribute[String|Integer|Float|Boolean] + pCheckAttributeObject (Geo)
   //
-  if      (attrP->type == KjString)  return pCheckAttributeString(attrP,  isAttribute, attrTypeFromDb, attrContextInfoP);
-  else if (attrP->type == KjInt)     return pCheckAttributeInteger(attrP, isAttribute, attrTypeFromDb, attrContextInfoP);
-  else if (attrP->type == KjFloat)   return pCheckAttributeFloat(attrP,   isAttribute, attrTypeFromDb, attrContextInfoP);
-  else if (attrP->type == KjBoolean) return pCheckAttributeBoolean(attrP, isAttribute, attrTypeFromDb, attrContextInfoP);
-  else if (attrP->type == KjArray)   return pCheckAttributeArray(attrP,   isAttribute, attrTypeFromDb, attrContextInfoP);
-  else if (attrP->type == KjObject)  return pCheckAttributeObject(attrP,  isAttribute, attrTypeFromDb, attrContextInfoP);
-  else if (attrP->type == KjNull)    return pCheckAttributeNull(attrP);
+  if      (attrP->type == KjString)  return pCheckAttributeString(entityId,  attrP, isAttribute, attrTypeFromDb, attrContextInfoP);
+  else if (attrP->type == KjInt)     return pCheckAttributeInteger(entityId, attrP, isAttribute, attrTypeFromDb, attrContextInfoP);
+  else if (attrP->type == KjFloat)   return pCheckAttributeFloat(entityId,   attrP, isAttribute, attrTypeFromDb, attrContextInfoP);
+  else if (attrP->type == KjBoolean) return pCheckAttributeBoolean(entityId, attrP, isAttribute, attrTypeFromDb, attrContextInfoP);
+  else if (attrP->type == KjArray)   return pCheckAttributeArray(entityId,   attrP, isAttribute, attrTypeFromDb, attrContextInfoP);
+  else if (attrP->type == KjObject)  return pCheckAttributeObject(entityId,  attrP, isAttribute, attrTypeFromDb, attrContextInfoP);
+  else if (attrP->type == KjNull)    return pCheckAttributeNull(entityId,    attrP);
 
   // Invalid JSON type of the attribute - we should never reach this point
   orionldError(OrionldInternalError, "invalid value type for attribute", attrP->name, 500);

src/lib/orionld/payloadCheck/pCheckAttribute.h

@@ -48,6 +48,7 @@ extern "C"
 //
 extern bool pCheckAttribute
 (
+  const char*             entityId,
   KjNode*                 attrP,
   bool                    isAttribute,
   OrionldAttributeType    attrTypeFromDb,

src/lib/orionld/payloadCheck/pCheckEntity.cpp

@@ -171,8 +171,9 @@ bool pCheckEntity
   if ((nodeP = kjLookup(entityP, "modifiedAt")) != NULL)  kjChildRemove(entityP, nodeP);
 
   // Loop over attributes
-  KjNode* idP   = NULL;
-  KjNode* typeP = NULL;
+  KjNode* idP      = NULL;
+  KjNode* typeP    = NULL;
+  char*   entityId = (char*)  "urn:unknown:id";
 
   for (KjNode* attrP = entityP->value.firstChildP; attrP != NULL; attrP = attrP->next)
   {
@@ -191,6 +192,7 @@ bool pCheckEntity
         return false;
 
       idP = attrP;
+      entityId = idP->value.s;
       continue;
     }
 
@@ -233,7 +235,7 @@ bool pCheckEntity
       aTypeFromDb = attrTypeFromDb(dbAttrsP, attrName);
     }
 
-    if (pCheckAttribute(attrP, true, aTypeFromDb, true, contextItemP) == false)
+    if (pCheckAttribute(entityId, attrP, true, aTypeFromDb, true, contextItemP) == false)
       return false;
   }
 

src/lib/orionld/serviceRoutines/orionldPatchAttribute.cpp

@@ -350,7 +350,7 @@ bool orionldPatchAttribute(void)
   // Make sure the incoming attribute is valid
   //
   OrionldAttributeType attrType = orionldAttributeType(attrTypeInDb);
-  if (pCheckAttribute(orionldState.requestTree, true, attrType, true, NULL) == false)
+  if (pCheckAttribute(entityId, orionldState.requestTree, true, attrType, true, NULL) == false)
     return false;
 
   kjTreeLog(orionldState.requestTree, "TR: Right after pCheckAttribute");

src/lib/orionld/serviceRoutines/orionldPatchEntity.cpp

@@ -254,7 +254,7 @@ bool orionldPatchEntity(void)
     OrionldAttributeType attrTypeFromDb = orionldAttributeType(dbAttrTypeP->value.s);  // Crash if dbAttrTypeP == NULL ... OK somehow ...
     OrionldContextItem*  contextItemP   = NULL;
 
-    if (pCheckAttribute(inAttrP, true, attrTypeFromDb, true, contextItemP) == false)
+    if (pCheckAttribute(entityId, inAttrP, true, attrTypeFromDb, true, contextItemP) == false)
     {
       kjChildRemove(orionldState.requestTree, inAttrP);
       attributeNotUpdated(notUpdatedV, shortName, orionldState.pd.title, orionldState.pd.detail);

src/lib/orionld/serviceRoutines/orionldPostEntity.cpp

@@ -211,7 +211,7 @@ bool orionldPostEntity(void)
     //
     // Is the attribute valid?
     //
-    if (pCheckAttribute(attrP, true, NoAttributeType, false, contextItemP) == false)
+    if (pCheckAttribute(entityId, attrP, true, NoAttributeType, false, contextItemP) == false)
     {
       kjChildRemove(orionldState.requestTree, attrP);
       attributeNotUpdated(notUpdatedP, shortName, orionldState.pd.title, orionldState.pd.detail);

src/lib/orionld/serviceRoutines/orionldPostTemporalEntities.cpp

@@ -118,15 +118,15 @@ bool orionldPostTemporalEntities(void)
       {
         aInstanceP->name = attrP->name;  // Need to "inherit" the name for the Array
 
-        if (pCheckAttribute(aInstanceP, true, NoAttributeType, false, NULL) == false)
+        if (pCheckAttribute(entityId, aInstanceP, true, NoAttributeType, false, NULL) == false)
           return false;
 
         attrP->name = aInstanceP->name;  // It's been expanded ionside pCheckAttribute
       }
     }
     else  // KjObject
     {
-      if (pCheckAttribute(attrP, true, NoAttributeType, false, NULL) == false)
+      if (pCheckAttribute(entityId, attrP, true, NoAttributeType, false, NULL) == false)
         return false;
     }
   }

test/functionalTest/cases/0000_ngsild/ngsild_error_reject_entity_if_property_value_is_null.test

@@ -53,13 +53,13 @@ echo
 01. POST /ngsi-ld/v1/entities with an attribute with value NULL
 ===============================================================
 HTTP/1.1 400 Bad Request
-Content-Length: 137
+Content-Length: 265
 Content-Type: application/json
 Date: REGEX(.*)
 
 {
-    "detail": "value",
-    "title": "null is not allowed as RHS in a JSON-LD document",
+    "detail": "Got a JSON 'null' in RHS (not allowed in JSON-LD docs) for the entity 'urn:unknown:id', attribute 'https://uri.etsi.org/ngsi-ld/default-context/P1', attribute field 'value'",
+    "title": "Bad Input",
     "type": "https://uri.etsi.org/ngsi-ld/errors/BadRequestData"
 }
 

test/functionalTest/cases/0000_ngsild/ngsild_error_reject_entity_if_relationship_object_is_null.test

@@ -53,13 +53,13 @@ echo
 01. POST /ngsi-ld/v1/entities with a relationship attribute with object == null
 ===============================================================================
 HTTP/1.1 400 Bad Request
-Content-Length: 138
+Content-Length: 266
 Content-Type: application/json
 Date: REGEX(.*)
 
 {
-    "detail": "object",
-    "title": "null is not allowed as RHS in a JSON-LD document",
+    "detail": "Got a JSON 'null' in RHS (not allowed in JSON-LD docs) for the entity 'urn:unknown:id', attribute 'https://uri.etsi.org/ngsi-ld/default-context/R1', attribute field 'object'",
+    "title": "Bad Input",
     "type": "https://uri.etsi.org/ngsi-ld/errors/BadRequestData"
 }