python-opcua
python-opcua copied to clipboard
FreeOpcUa
Crash if ApplicationDescription.DiscoveryUrls is None
I tried to establish a connection to a proprietary OPC UA server using the opcua-client-gui. The application crashes if you press the "Querry server capability" button in the ConnectionDialog. It seems that the application_to_strings() method cannot handle ApplicationDescription.DiscoveryUrls == None. In my case, the method application_to_strings was called implicitly by endpoint_to_strings ().
In python-opcua/opcua/tools.py:
def application_to_strings(app):
...
for url in app.DiscoveryUrls: # Crash here -> TypeError: 'NoneType' object is not iterable
result.append(('Discovery URL', url))
return result # ['{}: {}'.format(n, v) for (n, v) in result]
I assume that a server without at least one discovery url is generally allowed. (Unfortunately, I don't know my way around the point) Can someone look at this please?
Console output: uaclient.uaclient - INFO - Endpoint 1:') uawidgets.utils - ERROR - 'NoneType' object is not iterable') Traceback (most recent call last): File "C:_repo\opcua-client-gui-master\venv\lib\site-packages\uawidgets\utils.py", line 21, in wrapper result = func(self, *args) File "C:_repo\opcua-client-gui-master\uaclient\connection_dialog.py", line 34, in query endpoints = self.parent.uaclient.get_endpoints(self.uri) File "C:_repo\opcua-client-gui-master\uaclient\uaclient.py", line 48, in get_endpoints for (n, v) in endpoint_to_strings(ep): File "C:_repo\opcua-client-gui-master\venv\lib\site-packages\opcua\tools.py", line 448, in endpoint_to_strings result += application_to_strings(ep.Server) File "C:_repo\opcua-client-gui-master\venv\lib\site-packages\opcua\tools.py", line 423, in application_to_strings for url in app.DiscoveryUrls: TypeError: 'NoneType' object is not iterable uawidgets.utils - WARNING - Error class <uaclient.connection_dialog.ConnectionDialog object at 0x000001E0ED21E3A8>s no member show_error or error')
guess it should be an empty list not none and if its none it should be overwritten with an empty list!?
https://github.com/FreeOpcUa/python-opcua/blob/5c580f661d2237924a4d4ad853e7fe03075f39ce/opcua/tools.py#L408
but i think the the method is ok the question is why is app.DiscoveryUrls = "None" and not "[ ]"
guess it should be an empty list not none and if its none it should be overwritten with an empty list!?
https://github.com/FreeOpcUa/python-opcua/blob/5c580f661d2237924a4d4ad853e7fe03075f39ce/opcua/tools.py#L408
but i think the the method is ok the question is why is app.DiscoveryUrls = "None" and not "[ ]"
That is a very good point. DiscoveryUrls seems to be initialized correctly:
opcua/ua/uaprotocoll_auto.py ~ at line 3428
def __init__(self):
self.ApplicationUri = None
self.ProductUri = None
self.ApplicationName = LocalizedText()
self.ApplicationType = ApplicationType(0)
self.GatewayServerUri = None
self.DiscoveryProfileUri = None
self.DiscoveryUrls = []
self._freeze = True
But then it will be overwritten in ua_binary.py ~ at line 229:
def unpack_uatype_array(vtype, data):
length = Primitives.Int32.unpack(data)
if length == -1:
return None
At the end struct.unpack with format='<1i' and data=b'\xff\xff\xff\xff' returns -1.
class _Primivive1(object)
...
def unpack(self, data):
return struct.unpack(self.format, data.read(self.size))[0]
# return struct.unpack('<1i', b'\xff\xff\xff\xff')
This leads to return None in unpack_uatype_array (see first code block above). Now we are at a point where I don't know enough about it. b '\ xff \ xff \ xff \ xff' -> -1 is correct. But in this case DiscoveryUris with type=ListOfStrings should not lead to None, but to [].
Can someone help me please?
Perhaps like this? (See comment # NEW CODE FROM HERE) A general special treatment for lists. At this point the client still knows that it is a list type ... ... I don't know
def struct_from_binary(objtype, data):
"""
unpack an ua struct. Arguments are an objtype as Python class or string
"""
if isinstance(objtype, (unicode, str)):
objtype = getattr(ua, objtype)
if issubclass(objtype, Enum):
return objtype(Primitives.UInt32.unpack(data))
obj = objtype()
for name, uatype in obj.ua_types:
# if our member has a swtich and it is not set we skip it
if hasattr(obj, "ua_switches") and name in obj.ua_switches:
container_name, idx = obj.ua_switches[name]
val = getattr(obj, container_name)
if not test_bit(val, idx):
continue
val = from_binary(uatype, data)
# NEW CODE FROM HERE
if val == None and uatype[:6] =='ListOf':
val = []
# END OF NEW CODE
setattr(obj, name, val)
return obj
@AndreasHeine Unfortunately I didn't get to it earlier. I really wanted to do a few local tests before signing up for PR.
why not adding a test before that line then:
for url in app.DiscoveryUrls: # Crash here -> TypeError: 'NoneType' object is not iterable
why not adding a test before that line then:
for url in app.DiscoveryUrls: # Crash here -> TypeError: 'NoneType' object is not iterable
Why having two different states for an empty list? DiscoveryUrls is initialized as an empty list. And then server response changes this to None.
Add a test in opcua / tools.py is also fine for me. But to be correct, we also have to check every list from the server (on every point of use in the toolkit) against None then.
I would prefer to be able to iterate over any empty list without having to check every List against None first. This looks more like an universal (and pythonic) solution to me. What do you think?
Dear @oroulet, the problem still exists. Due to this error, the client-gui also crashes on servers that provide an empty DiscoveryUrls list.
If it is okay for you, I can create an RP for your proposed solution (according to your answer of 12 Oct 2020). I would make the change in both projects (python-opcua and opcua-asyncua).
What do you think?
With kind regards, CC
Lists can be none in ua so yes adding a test to not crash is a good idea. And a PR is welcome