Include sub-references in parent type check

Add additional error output when adding nodes fails

Remove check for empty variable value. Since Specification 1.04 there
are many variables which contain no specific value.

src/server/ua_server_internal.h

@@ -267,12 +267,25 @@ isNodeInTree(UA_Nodestore *ns, const UA_NodeId *leafNode,
              size_t referenceTypeIdsSize);
 
 /* Returns an array with the hierarchy of type nodes. The returned array starts
- * at the leaf and continues "upwards" in the hierarchy based on the
+ * at the leaf and continues "upwards" or "downwards" in the hierarchy based on the
  * ``hasSubType`` references. Since multiple-inheritance is possible in general,
- * duplicate entries are removed. */
+ * duplicate entries are removed.
+ * The parameter `walkDownwards` indicates the direction of search.
+ * If set to TRUE it will get all the subtypes of the given
+ * leafType (including leafType).
+ * If set to FALSE it will get all the parent types of the given
+ * leafType (including leafType)*/
 UA_StatusCode
 getTypeHierarchy(UA_Nodestore *ns, const UA_NodeId *leafType,
-                 UA_NodeId **typeHierarchy, size_t *typeHierarchySize);
+                 UA_NodeId **typeHierarchy, size_t *typeHierarchySize,
+                 UA_Boolean walkDownwards);
+
+/* Same as getTypeHierarchy but takes multiple leafTypes as parameter and returns
+ * an combined list of all the found types for all the leaf types */
+UA_StatusCode
+getTypesHierarchy(UA_Nodestore *ns, const UA_NodeId *leafType, size_t leafTypeSize,
+                 UA_NodeId **typeHierarchy, size_t *typeHierarchySize,
+                 UA_Boolean walkDownwards);
 
 /* Returns the type node from the node on the stack top. The type node is pushed
  * on the stack and returned. */

src/server/ua_server_utils.c

@@ -168,12 +168,14 @@ static const UA_NodeId hasSubtypeNodeId =
 
 static UA_StatusCode
 getTypeHierarchyFromNode(UA_NodeId **results_ptr, size_t *results_count,
-                         size_t *results_size, const UA_Node *node) {
+                         size_t *results_size, const UA_Node *node,
+                         UA_Boolean walkDownwards) {
     UA_NodeId *results = *results_ptr;
     for(size_t i = 0; i < node->referencesSize; ++i) {
         /* Is the reference kind relevant? */
         UA_NodeReferenceKind *refs = &node->references[i];
-        if(!refs->isInverse)
+        // if downwards, we do not want inverse, if upwards we want inverse
+        if (walkDownwards == refs->isInverse)
             continue;
         if(!UA_NodeId_equal(&hasSubtypeNodeId, &refs->referenceTypeId))
             continue;
@@ -219,7 +221,8 @@ getTypeHierarchyFromNode(UA_NodeId **results_ptr, size_t *results_count,
 
 UA_StatusCode
 getTypeHierarchy(UA_Nodestore *ns, const UA_NodeId *leafType,
-                 UA_NodeId **typeHierarchy, size_t *typeHierarchySize) {
+                 UA_NodeId **typeHierarchy, size_t *typeHierarchySize,
+                 UA_Boolean walkDownwards) {
     /* Allocate the results array. Probably too big, but saves mallocs. */
     size_t results_size = 20;
     UA_NodeId *results = (UA_NodeId*)UA_malloc(sizeof(UA_NodeId) * results_size);
@@ -249,7 +252,7 @@ getTypeHierarchy(UA_Nodestore *ns, const UA_NodeId *leafType,
 
         /* Add references from the current node to the end of the array */
         retval = getTypeHierarchyFromNode(&results, &results_count,
-                                          &results_size, node);
+                                          &results_size, node, walkDownwards);
 
         /* Release the node */
         ns->releaseNode(ns->context, node);
@@ -271,6 +274,40 @@ getTypeHierarchy(UA_Nodestore *ns, const UA_NodeId *leafType,
     return UA_STATUSCODE_GOOD;
 }
 
+
+UA_StatusCode
+getTypesHierarchy(UA_Nodestore *ns, const UA_NodeId *leafType, size_t leafTypeSize,
+                 UA_NodeId **typeHierarchy, size_t *typeHierarchySize,
+                 UA_Boolean walkDownwards) {
+    UA_NodeId *results = NULL;
+    size_t results_count = 0;
+    for (size_t i=0; i<leafTypeSize; i++) {
+        UA_NodeId *tmpResults = NULL;
+        size_t tmpResults_size = 0;
+        UA_StatusCode retval = getTypeHierarchy(ns, &leafType[i], &tmpResults, &tmpResults_size, walkDownwards);
+        if(retval != UA_STATUSCODE_GOOD) {
+            UA_Array_delete(results, results_count, &UA_TYPES[UA_TYPES_NODEID]);
+            return retval;
+        }
+        if (tmpResults_size == 0 || tmpResults == NULL )
+            continue;
+        size_t new_size = sizeof(UA_NodeId) * (results_count + tmpResults_size);
+        UA_NodeId *new_results = (UA_NodeId*)UA_realloc(results, new_size);
+        if(!new_results) {
+            UA_Array_delete(results, results_count, &UA_TYPES[UA_TYPES_NODEID]);
+            return UA_STATUSCODE_BADOUTOFMEMORY;
+        }
+        results = new_results;
+        memcpy(&results[results_count],tmpResults, sizeof(UA_NodeId)*tmpResults_size);
+        /* do not use UA_Array_delete since we still need the content of the nodes */
+        UA_free(tmpResults);
+        results_count = results_count + tmpResults_size;
+    }
+    *typeHierarchy = results;
+    *typeHierarchySize = results_count;
+    return UA_STATUSCODE_GOOD;
+}
+
 /* For mulithreading: make a copy of the node, edit and replace.
  * For singlethreading: edit the original */
 UA_StatusCode

src/server/ua_services_attribute.c

@@ -774,9 +774,12 @@ compatibleValue(UA_Server *server, UA_Session *session, const UA_NodeId *targetD
         }
 
         UA_LOG_INFO(server->config.logger, UA_LOGCATEGORY_SERVER,
-                    "Only Variables with data type BaseDataType may contain "
+                    "Only Variables with data type BaseDataType should contain "
                     "a null (empty) value");
-        return false;
+        /* we allow addition of the node anyways since existing information models may have
+           variables with no value, e.g. OldValues - ns=0;i=3024.
+           See also #1889, https://github.com/open62541/open62541/pull/1889#issuecomment-403506538 */
+        return true;
     }
 
     /* Has the value a subtype of the required type? BaseDataType (Variant) can

src/server/ua_services_nodemanagement.c

@@ -19,6 +19,14 @@
 #include "ua_server_internal.h"
 #include "ua_services.h"
 
+#define UA_LOG_NODEID_WRAP(NODEID, LOG) { \
+    UA_String nodeIdStr = UA_STRING_NULL; \
+    UA_NodeId_toString(NODEID, &nodeIdStr); \
+    LOG; \
+    UA_String_deleteMembers(&nodeIdStr); \
+}
+
+
 /*********************/
 /* Edit Node Context */
 /*********************/
@@ -70,15 +78,16 @@ static UA_StatusCode
 checkParentReference(UA_Server *server, UA_Session *session, UA_NodeClass nodeClass,
                      const UA_NodeId *parentNodeId, const UA_NodeId *referenceTypeId) {
     /* Objects do not need a parent (e.g. mandatory/optional modellingrules) */
-    if(nodeClass == UA_NODECLASS_OBJECT && UA_NodeId_isNull(parentNodeId) &&
-       UA_NodeId_isNull(referenceTypeId))
+    /* Also, there are some variables which do not have parents, e.g. EnumStrings, EnumValues */
+    if((nodeClass == UA_NODECLASS_OBJECT || nodeClass == UA_NODECLASS_VARIABLE)
+       && UA_NodeId_isNull(parentNodeId) && UA_NodeId_isNull(referenceTypeId))
         return UA_STATUSCODE_GOOD;
 
     /* See if the parent exists */
     const UA_Node *parent = UA_Nodestore_get(server, parentNodeId);
     if(!parent) {
-        UA_LOG_INFO_SESSION(server->config.logger, session,
-                            "AddNodes: Parent node not found");
+        UA_LOG_NODEID_WRAP(parentNodeId, UA_LOG_INFO_SESSION(server->config.logger, session,
+                            "AddNodes: Parent node %.*s not found", (int)nodeIdStr.length, nodeIdStr.data));
         return UA_STATUSCODE_BADPARENTNODEIDINVALID;
     }
 
@@ -581,7 +590,7 @@ addChildren(UA_Server *server, UA_Session *session,
     UA_NodeId *hierarchy = NULL;
     size_t hierarchySize = 0;
     UA_StatusCode retval = getTypeHierarchy(&server->config.nodestore, &type->nodeId,
-                                            &hierarchy, &hierarchySize);
+                                            &hierarchy, &hierarchySize, UA_FALSE);
     if(retval != UA_STATUSCODE_GOOD)
         return retval;
 
@@ -689,9 +698,11 @@ AddNode_addRefs(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
     UA_StatusCode retval = checkParentReference(server, session, node->nodeClass,
                                                 parentNodeId, referenceTypeId);
     if(retval != UA_STATUSCODE_GOOD) {
-        UA_LOG_INFO_SESSION(server->config.logger, session,
-                            "AddNodes: The parent reference is invalid "
-                            "with status code %s", UA_StatusCode_name(retval));
+        UA_LOG_NODEID_WRAP(nodeId, UA_LOG_INFO_SESSION(server->config.logger, session,
+                            "AddNodes: The parent reference for %.*s is invalid "
+                            "with status code %s",
+                            (int)nodeIdStr.length, nodeIdStr.data,
+                            UA_StatusCode_name(retval)));
         goto cleanup;
     }
 
@@ -699,9 +710,10 @@ AddNode_addRefs(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
     if((node->nodeClass == UA_NODECLASS_VARIABLE ||
         node->nodeClass == UA_NODECLASS_OBJECT) &&
        UA_NodeId_isNull(typeDefinitionId)) {
-        UA_LOG_INFO_SESSION(server->config.logger, session,
-                            "AddNodes: No TypeDefinition; Use the default "
-                            "TypeDefinition for the Variable/Object");
+        UA_LOG_NODEID_WRAP(nodeId, UA_LOG_INFO_SESSION(server->config.logger, session,
+                            "AddNodes: No TypeDefinition for %.*s; Use the default "
+                            "TypeDefinition for the Variable/Object",
+                            (int)nodeIdStr.length, nodeIdStr.data));
         if(node->nodeClass == UA_NODECLASS_VARIABLE)
             typeDefinitionId = &baseDataVariableType;
         else
@@ -714,8 +726,9 @@ AddNode_addRefs(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
         /* Get the type node */
         type = UA_Nodestore_get(server, typeDefinitionId);
         if(!type) {
-            UA_LOG_INFO_SESSION(server->config.logger, session,
-                                "AddNodes: Node type not found");
+            UA_LOG_NODEID_WRAP(typeDefinitionId, UA_LOG_INFO_SESSION(server->config.logger, session,
+                                "AddNodes: Node type %.*s not found",
+                                (int)nodeIdStr.length, nodeIdStr.data));
             retval = UA_STATUSCODE_BADTYPEDEFINITIONINVALID;
             goto cleanup;
         }
@@ -750,8 +763,9 @@ AddNode_addRefs(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
                 typeOk = UA_FALSE;
         }
         if(!typeOk) {
-            UA_LOG_INFO_SESSION(server->config.logger, session,
-                                "AddNodes: Type does not match node class");
+            UA_LOG_NODEID_WRAP(nodeId, UA_LOG_INFO_SESSION(server->config.logger, session,
+                                "AddNodes: Type for %.*s does not match node class",
+                                (int)nodeIdStr.length, nodeIdStr.data));
             retval = UA_STATUSCODE_BADTYPEDEFINITIONINVALID;
             goto cleanup;
         }
@@ -760,35 +774,50 @@ AddNode_addRefs(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
          * that type has the same nodeClass from checkParentReference. */
         if(node->nodeClass == UA_NODECLASS_VARIABLE) {
             if(((const UA_VariableTypeNode*)type)->isAbstract) {
+                /* Get subtypes of the parent reference types */
+                UA_NodeId *parentTypeHierachy = NULL;
+                size_t parentTypeHierachySize = 0;
+                getTypesHierarchy(&server->config.nodestore, parentReferences,UA_PARENT_REFERENCES_COUNT,
+                                  &parentTypeHierachy, &parentTypeHierachySize, UA_TRUE);
                 /* Abstract variable is allowed if parent is a children of a base data variable */
                 const UA_NodeId variableTypes = UA_NODEID_NUMERIC(0, UA_NS0ID_BASEDATAVARIABLETYPE);
                 /* A variable may be of an object type which again is below BaseObjectType */
                 const UA_NodeId objectTypes = UA_NODEID_NUMERIC(0, UA_NS0ID_BASEOBJECTTYPE);
-                // TODO handle subtypes of parent reference types
                 if(!isNodeInTree(&server->config.nodestore, parentNodeId, &variableTypes,
-                                 parentReferences, UA_PARENT_REFERENCES_COUNT) &&
+                                 parentTypeHierachy, parentTypeHierachySize) &&
                    !isNodeInTree(&server->config.nodestore, parentNodeId, &objectTypes,
-                                 parentReferences, UA_PARENT_REFERENCES_COUNT)) {
-                    UA_LOG_INFO_SESSION(server->config.logger, session,
-                                        "AddNodes: Type of variable node must "
-                                        "be VariableType and not cannot be abstract");
+                                 parentTypeHierachy,parentTypeHierachySize)) {
+                    UA_Array_delete(parentTypeHierachy, parentTypeHierachySize, &UA_TYPES[UA_TYPES_NODEID]);
+                    UA_LOG_NODEID_WRAP(nodeId, UA_LOG_INFO_SESSION(server->config.logger, session,
+                                        "AddNodes: Type of variable node %.*s must "
+                                        "be VariableType and not cannot be abstract",
+                                        (int)nodeIdStr.length, nodeIdStr.data));
                     retval = UA_STATUSCODE_BADTYPEDEFINITIONINVALID;
                     goto cleanup;
                 }
+                UA_Array_delete(parentTypeHierachy, parentTypeHierachySize, &UA_TYPES[UA_TYPES_NODEID]);
             }
         }
 
         if(node->nodeClass == UA_NODECLASS_OBJECT) {
             if(((const UA_ObjectTypeNode*)type)->isAbstract) {
+                /* Get subtypes of the parent reference types */
+                UA_NodeId *parentTypeHierachy = NULL;
+                size_t parentTypeHierachySize = 0;
+                getTypesHierarchy(&server->config.nodestore, parentReferences,UA_PARENT_REFERENCES_COUNT,
+                                  &parentTypeHierachy, &parentTypeHierachySize, UA_TRUE);
                 /* Object node created of an abstract ObjectType. Only allowed
                  * if within BaseObjectType folder */
                 const UA_NodeId objectTypes = UA_NODEID_NUMERIC(0, UA_NS0ID_BASEOBJECTTYPE);
-                // TODO handle subtypes of parent reference types
-                if(!isNodeInTree(&server->config.nodestore, parentNodeId, &objectTypes,
-                                 parentReferences, UA_PARENT_REFERENCES_COUNT)) {
-                    UA_LOG_INFO_SESSION(server->config.logger, session,
-                                        "AddNodes: Type of object node must "
-                                        "be ObjectType and not be abstract");
+                UA_Boolean isInBaseObjectType = isNodeInTree(&server->config.nodestore, parentNodeId, &objectTypes,
+                                                             parentTypeHierachy, parentTypeHierachySize);
+
+                UA_Array_delete(parentTypeHierachy, parentTypeHierachySize, &UA_TYPES[UA_TYPES_NODEID]);
+                if(!isInBaseObjectType) {
+                    UA_LOG_NODEID_WRAP(nodeId, UA_LOG_INFO_SESSION(server->config.logger, session,
+                                        "AddNodes: Type of object node %.*s must "
+                                        "be ObjectType and not be abstract",
+                                        (int)nodeIdStr.length, nodeIdStr.data));
                     retval = UA_STATUSCODE_BADTYPEDEFINITIONINVALID;
                     goto cleanup;
                 }
@@ -799,16 +828,18 @@ AddNode_addRefs(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
     /* Add reference to the parent */
     if(!UA_NodeId_isNull(parentNodeId)) {
         if(UA_NodeId_isNull(referenceTypeId)) {
-            UA_LOG_INFO_SESSION(server->config.logger, session,
-                                "AddNodes: Reference to parent cannot be null");
+            UA_LOG_NODEID_WRAP(nodeId, UA_LOG_INFO_SESSION(server->config.logger, session,
+                                "AddNodes: Reference to parent of %.*s cannot be null",
+                                (int)nodeIdStr.length, nodeIdStr.data));
             retval = UA_STATUSCODE_BADTYPEDEFINITIONINVALID;
             goto cleanup;
         }
 
         retval = addRef(server, session, &node->nodeId, referenceTypeId, parentNodeId, false);
         if(retval != UA_STATUSCODE_GOOD) {
-            UA_LOG_INFO_SESSION(server->config.logger, session,
-                                "AddNodes: Adding reference to parent failed");
+            UA_LOG_NODEID_WRAP(nodeId, UA_LOG_INFO_SESSION(server->config.logger, session,
+                                "AddNodes: Adding reference to parent of %.*s failed",
+                                (int)nodeIdStr.length, nodeIdStr.data));
             goto cleanup;
         }
     }
@@ -818,11 +849,13 @@ AddNode_addRefs(UA_Server *server, UA_Session *session, const UA_NodeId *nodeId,
        node->nodeClass == UA_NODECLASS_OBJECT) {
         UA_assert(type != NULL); /* see above */
         retval = addRef(server, session, &node->nodeId, &hasTypeDefinition, &type->nodeId, true);
-        if(retval != UA_STATUSCODE_GOOD)
-            UA_LOG_INFO_SESSION(server->config.logger, session,
+        if(retval != UA_STATUSCODE_GOOD) {
+            UA_LOG_NODEID_WRAP(nodeId, UA_LOG_INFO_SESSION(server->config.logger, session,
                                 "AddNodes: Adding a reference to the type "
-                                "definition failed with error code %s",
-                                UA_StatusCode_name(retval));
+                                "definition of %.*s failed with error code %s",
+                                (int)nodeIdStr.length, nodeIdStr.data,
+                                UA_StatusCode_name(retval)));
+        }
     }
 
  cleanup:

src/server/ua_subscription_events.c

@@ -373,7 +373,13 @@ UA_StatusCode
 UA_Server_triggerEvent(UA_Server *server, const UA_NodeId eventNodeId, const UA_NodeId origin,
                        UA_ByteString *outEventId, const UA_Boolean deleteEventNode) {
     /* Make sure the origin is in the ObjectsFolder (TODO: or in the ViewsFolder) */
-    if(!isNodeInTree(&server->config.nodestore, &origin, &objectsFolderId, parentReferences_events, 2)) {
+    UA_NodeId *parentTypeHierachy = NULL;
+    size_t parentTypeHierachySize = 0;
+    getTypesHierarchy(&server->config.nodestore, parentReferences_events, 2,
+                      &parentTypeHierachy, &parentTypeHierachySize, UA_TRUE);
+    UA_Boolean isInObjectsFolder = isNodeInTree(&server->config.nodestore, &origin, &objectsFolderId, parentTypeHierachy, parentTypeHierachySize);
+    UA_Array_delete(parentTypeHierachy, parentTypeHierachySize, &UA_TYPES[UA_TYPES_NODEID]);
+    if (!isInObjectsFolder) {
         UA_LOG_ERROR(server->config.logger, UA_LOGCATEGORY_USERLAND,
                      "Node for event must be in ObjectsFolder!");
         return UA_STATUSCODE_BADINVALIDARGUMENT;