FreeOpcUa / opcua-asyncio

OPC UA library for python >= 3.7
GNU Lesser General Public License v3.0
1.11k stars 358 forks source link

Browsing for HasTypeDefinition references is broken #1529

Open kevinherron opened 9 months ago

kevinherron commented 9 months ago

Browsing a server built with opcua-asyncio for HasTypeDefinition references is broken.

When specifying a NodeClassMask of only ObjectType or VariableType, no results are returned.

If the NodeClassMask is set to 0 (all NodeClasses should be returned), then 1 result is returned, however it incorrectly identifies the NodeClass as NodeClass.DataType. (I suspect this is the cause of the 0 length result when a mask is supplied).

This code written against Eclipse Milo dev/1.0 branch reproduces, though you should be able to reproduce from any OPC UA SDK: https://gist.github.com/kevinherron/f85ce3b0eebe84008e59a5192f6b404d

Results:

Browsing with NodeClass mask: 24
  No references found
Browsing with NodeClass mask: 0
  Reference[0]:
    NodeId: ExpandedNodeId{ns=0, id=2138, serverIndex=0}
    BrowseName: QualifiedName{name=ServerStatusType, namespaceIndex=0}
    DisplayName: LocalizedText{text=ServerStatusType, locale=null}
    NodeClass: DataType
    TypeDefinition: ExpandedNodeId{ns=0, id=0, serverIndex=0}

Wireshark capture: browse_has_type_definition.zip

kevinherron commented 7 months ago

I dug into this a little in hopes the additional information might nudge a maintainer to attempt to fix this.

For Nodes belonging to the standard OPC UA address space (ns0), the problem can be traced back to here: https://github.com/FreeOpcUa/opcua-asyncio/blob/d4d4c84366dea6bc0ede632fff0821b00f85d8eb/schemas/generate_address_space.py#L345-L359

where the target NodeClass of every ReferenceDescription created is hardcoded as NodeClass.DataType.

In custom address spaces it can be traced to here: https://github.com/FreeOpcUa/opcua-asyncio/blob/d4d4c84366dea6bc0ede632fff0821b00f85d8eb/asyncua/server/address_space.py#L382-L389

where again, the target NodeClass is hardcoded as NodeClass.DataType. Obviously wrong, because the target of a HasTypeDefinition reference is always going to be either an ObjectTypeNode or VariableTypeNode.

BruceGitHub2015 commented 4 months ago

It is just a typo. Change DataType to ObjectType will fix it.

kevinherron commented 4 months ago

It is just a typo. Change DataType to ObjectType will fix it.

No, it won't. The target NodeClass of a HasTypeDefinition reference can be either ObjectType or VariableType, depending on whether the source is an Object or Variable.

BruceGitHub2015 commented 4 months ago

It is just a typo. Change DataType to ObjectType will fix it.

No, it won't. The target NodeClass of a HasTypeDefinition reference can be either ObjectType or VariableType, depending on whether the source is an Object or Variable.

I was brought here by this post https://forum.inductiveautomation.com/t/error-calling-opc-ua-method/83234 I can verify that by changing the DataType to ObjectType, the error goes away.

kevinherron commented 4 months ago

Yes, understood. It would fix that particular issue.

It's not a general fix, though, because it just changes the hardcoded value from one incorrect NodeClass to another.

Whether that's acceptable or not is up to the maintainers of this project, I guess.

BruceGitHub2015 commented 4 months ago

Yes you are right. As other OPC UA clients can handle the original mistake without an issue, I guess it also has something to do with how a client tolerates such problems. As everything on the server is an Object, I thought maybe changing it to a more general ObjectType could make the mistake more tolerable. I barely know anything about ignition so I couldn't really explain how that change affects it.