search

FreeOpcUa / python-opcua

LGPL Pure Python OPC-UA Client and Server
http://freeopcua.github.io/
GNU Lesser General Public License v3.0
1.36k stars 658 forks source link

Client calling opcua methods without arguments throws exception #1352

Open mrkrizic opened 3 years ago

mrkrizic commented 3 years ago

Describe the bug
When calling a method without return values using call_method() or call_method_full the client throws and exception: 'NoneType' object is not iterable. When calling the same method using _call_method() no exception occurs. This is due to call_method() calling call_method_full() which in the last step iterates over the OutputArguments. Since there are none, the exception is thrown.

To Reproduce
Create a method without return type. In my case I was using a 4diac opcua server which uses the C opcua implementation.

Expected behavior
The method call would succeed and None or [] would be returned.

Screenshots
This is the method viewed using UA_Expert. It has no input or output parameters (Will be changed in the future). image

Possible solution Check if arguments are not None before collecting the Output Arguments in: https://github.com/FreeOpcUa/python-opcua/blob/3012acf9047aad3acc2674e9008f0d1eee680c25/opcua/common/methods.py#L41

Version
Python-Version: 2.7, but should give the same results on any version
python-opcua Version (e.g. master branch, 0.9): 0.98.13

zerox1212 commented 3 years ago

Did you check OPC UA spec if its allowed to have no return arguments? I can't remember.

AndreasHeine commented 3 years ago

It is allowed to have no args e.g. in Part 10 Programs the statechange method which have no args at all!

mrkrizic commented 3 years ago

As far as I understood it, you are allowed to have no output arguments.

According to the documentation, methods without output arguments should return an empty list.

AndreasHeine commented 3 years ago

long things shot could you make a wireshark trace of the method call so we can figure out if the server returns an empty list or not!?

mrkrizic commented 3 years ago

I will try, but I am currently not in the office so I'll need to come up with something

mrkrizic commented 3 years ago

I am not able to perform a wireshark trace, but I have found the server function block implementation for the method nodes in my example. It is taken from an amalgated source file of open62514 and is used in 4diac to provide an abstract opcua layer for function blocks. Sorry for the big code block, but I don't know which part might be interesting to you.

#ifdef UA_ENABLE_METHODCALLS

static const UA_NodeId hasproperty = {0, UA_NODEIDTYPE_NUMERIC, {UA_NS0ID_HASPROPERTY}};
static const UA_NodeId propertytype = {0, UA_NODEIDTYPE_NUMERIC, {UA_NS0ID_PROPERTYTYPE}};

static UA_StatusCode
UA_Server_addMethodNodeEx_finish(UA_Server *server, const UA_NodeId nodeId,
                                 UA_MethodCallback method,
                                 const size_t inputArgumentsSize, const UA_Argument *inputArguments,
                                 const UA_NodeId inputArgumentsRequestedNewNodeId,
                                 UA_NodeId *inputArgumentsOutNewNodeId,
                                 const size_t outputArgumentsSize, const UA_Argument *outputArguments,
                                 const UA_NodeId outputArgumentsRequestedNewNodeId,
                                 UA_NodeId *outputArgumentsOutNewNodeId) {
    /* Browse to see which argument nodes exist */
    UA_BrowseDescription bd;
    UA_BrowseDescription_init(&bd);
    bd.nodeId = nodeId;
    bd.referenceTypeId = UA_NODEID_NUMERIC(0, UA_NS0ID_HASPROPERTY);
    bd.includeSubtypes = false;
    bd.browseDirection = UA_BROWSEDIRECTION_FORWARD;
    bd.nodeClassMask = UA_NODECLASS_VARIABLE;
    bd.resultMask = UA_BROWSERESULTMASK_BROWSENAME;

    UA_BrowseResult br;
    UA_BrowseResult_init(&br);
    UA_UInt32 maxrefs = 0;
    Operation_Browse(server, &server->adminSession, &maxrefs, &bd, &br);

    UA_StatusCode retval = br.statusCode;
    if(retval != UA_STATUSCODE_GOOD) {
        UA_Server_deleteNode(server, nodeId, true);
        UA_BrowseResult_deleteMembers(&br);
        return retval;
    }

    /* Filter out the argument nodes */
    UA_NodeId inputArgsId = UA_NODEID_NULL;
    UA_NodeId outputArgsId = UA_NODEID_NULL;
    const UA_QualifiedName inputArgsName = UA_QUALIFIEDNAME(0, "InputArguments");
    const UA_QualifiedName outputArgsName = UA_QUALIFIEDNAME(0, "OutputArguments");
    for(size_t i = 0; i < br.referencesSize; i++) {
        UA_ReferenceDescription *rd = &br.references[i];
        if(rd->browseName.namespaceIndex == 0 &&
           UA_String_equal(&rd->browseName.name, &inputArgsName.name))
            inputArgsId = rd->nodeId.nodeId;
        else if(rd->browseName.namespaceIndex == 0 &&
                UA_String_equal(&rd->browseName.name, &outputArgsName.name))
            outputArgsId = rd->nodeId.nodeId;
    }

    /* Add the Input Arguments VariableNode */
    if(inputArgumentsSize > 0 && UA_NodeId_isNull(&inputArgsId)) {
        UA_VariableAttributes attr = UA_VariableAttributes_default;
        char *name = "InputArguments";
        attr.displayName = UA_LOCALIZEDTEXT("", name);
        attr.dataType = UA_TYPES[UA_TYPES_ARGUMENT].typeId;
        attr.valueRank = UA_VALUERANK_ONE_DIMENSION;
        UA_UInt32 inputArgsSize32 = (UA_UInt32)inputArgumentsSize;
        attr.arrayDimensions = &inputArgsSize32;
        attr.arrayDimensionsSize = 1;
        UA_Variant_setArray(&attr.value, (void*)(uintptr_t) inputArguments,
                            inputArgumentsSize, &UA_TYPES[UA_TYPES_ARGUMENT]);
        retval |= UA_Server_addVariableNode(server, inputArgumentsRequestedNewNodeId, nodeId,
                                            hasproperty, UA_QUALIFIEDNAME(0, name),
                                            propertytype, attr, NULL, &inputArgsId);
    }

    /* Add the Output Arguments VariableNode */
    if(outputArgumentsSize > 0 && UA_NodeId_isNull(&outputArgsId)) {
        UA_VariableAttributes attr = UA_VariableAttributes_default;
        char *name = "OutputArguments";
        attr.displayName = UA_LOCALIZEDTEXT("", name);
        attr.dataType = UA_TYPES[UA_TYPES_ARGUMENT].typeId;
        attr.valueRank = UA_VALUERANK_ONE_DIMENSION;
        UA_UInt32 outputArgsSize32 = (UA_UInt32)outputArgumentsSize;
        attr.arrayDimensions = &outputArgsSize32;
        attr.arrayDimensionsSize = 1;
        UA_Variant_setArray(&attr.value, (void*)(uintptr_t) outputArguments,
                            outputArgumentsSize, &UA_TYPES[UA_TYPES_ARGUMENT]);
        retval |= UA_Server_addVariableNode(server, outputArgumentsRequestedNewNodeId, nodeId,
                                            hasproperty, UA_QUALIFIEDNAME(0, name),
                                            propertytype, attr, NULL, &outputArgsId);
    }

    retval |= UA_Server_setMethodNode_callback(server, nodeId, method);

    /* Call finish to add the parent reference */
    retval |= Operation_addNode_finish(server, &server->adminSession, &nodeId);

    if(retval != UA_STATUSCODE_GOOD) {
        UA_Server_deleteNode(server, nodeId, true);
        UA_Server_deleteNode(server, inputArgsId, true);
        UA_Server_deleteNode(server, outputArgsId, true);
    } else {
        if(inputArgumentsOutNewNodeId != NULL) {
            UA_NodeId_copy(&inputArgsId, inputArgumentsOutNewNodeId);
    }
        if(outputArgumentsOutNewNodeId != NULL) {
            UA_NodeId_copy(&outputArgsId, outputArgumentsOutNewNodeId);
        }
    }
    UA_BrowseResult_deleteMembers(&br);
    return retval;
}

UA_StatusCode
UA_Server_addMethodNode_finish(UA_Server *server, const UA_NodeId nodeId,
                               UA_MethodCallback method,
                               size_t inputArgumentsSize, const UA_Argument* inputArguments,
                               size_t outputArgumentsSize, const UA_Argument* outputArguments) {
    return UA_Server_addMethodNodeEx_finish(server, nodeId, method,
                                            inputArgumentsSize, inputArguments, UA_NODEID_NULL, NULL,
                                            outputArgumentsSize, outputArguments, UA_NODEID_NULL, NULL);
}

UA_StatusCode
UA_Server_addMethodNodeEx(UA_Server *server, const UA_NodeId requestedNewNodeId,
                          const UA_NodeId parentNodeId,
                          const UA_NodeId referenceTypeId,
                          const UA_QualifiedName browseName,
                          const UA_MethodAttributes attr, UA_MethodCallback method,
                          size_t inputArgumentsSize, const UA_Argument *inputArguments,
                          const UA_NodeId inputArgumentsRequestedNewNodeId,
                          UA_NodeId *inputArgumentsOutNewNodeId,
                          size_t outputArgumentsSize, const UA_Argument *outputArguments,
                          const UA_NodeId outputArgumentsRequestedNewNodeId,
                          UA_NodeId *outputArgumentsOutNewNodeId,
                          void *nodeContext, UA_NodeId *outNewNodeId) {
    UA_AddNodesItem item;
    UA_AddNodesItem_init(&item);
    item.nodeClass = UA_NODECLASS_METHOD;
    item.requestedNewNodeId.nodeId = requestedNewNodeId;
    item.browseName = browseName;
    item.nodeAttributes.encoding = UA_EXTENSIONOBJECT_DECODED_NODELETE;
    item.nodeAttributes.content.decoded.data = (void*)(uintptr_t)&attr;
    item.nodeAttributes.content.decoded.type = &UA_TYPES[UA_TYPES_METHODATTRIBUTES];

    UA_NodeId newId;
    if(!outNewNodeId) {
        UA_NodeId_init(&newId);
        outNewNodeId = &newId;
    }

    UA_StatusCode retval = Operation_addNode_begin(server, &server->adminSession, nodeContext,
                                                   &item, &parentNodeId, &referenceTypeId, outNewNodeId);
    if(retval != UA_STATUSCODE_GOOD)
        return retval;

    retval = UA_Server_addMethodNodeEx_finish(server, *outNewNodeId, method,
                                              inputArgumentsSize, inputArguments,
                                              inputArgumentsRequestedNewNodeId,
                                              inputArgumentsOutNewNodeId,
                                              outputArgumentsSize, outputArguments,
                                              outputArgumentsRequestedNewNodeId,
                                              outputArgumentsOutNewNodeId);

    if(outNewNodeId == &newId)
        UA_NodeId_deleteMembers(&newId);
    return retval;
}

static UA_StatusCode
editMethodCallback(UA_Server *server, UA_Session* session,
                   UA_Node* node, void* handle) {
    if(node->nodeClass != UA_NODECLASS_METHOD)
        return UA_STATUSCODE_BADNODECLASSINVALID;
    UA_MethodNode *mnode = (UA_MethodNode*) node;
    mnode->method = (UA_MethodCallback)(uintptr_t)handle;
    return UA_STATUSCODE_GOOD;
}

UA_StatusCode
UA_Server_setMethodNode_callback(UA_Server *server,
                                 const UA_NodeId methodNodeId,
                                 UA_MethodCallback methodCallback) {
    return UA_Server_editNode(server, &server->adminSession, &methodNodeId,
                              (UA_EditNodeCallback)editMethodCallback,
                              (void*)(uintptr_t)methodCallback);
}

#endif

There is no output argument refeference node in the first place, since the definition of an outputArgument Node is optional if I understand the reference correctly. Since there is no outputArgument, the check elif len(result.OutputArguments) == 1: fails. Therefore nothing was returned, which would leave the value uninitialized to None and the client throws an exception.

UaExpert is able to handle this case and from the documentation it seems to me that this is a valid implementation.

Please correct me if I'm wrong.

mrkrizic commented 3 years ago

This seems related: https://www.gitmemory.com/issue/eclipse/milo/838/839512557 And here: https://github.com/eclipse/milo/issues/815