sibson / vncdotool

A command line VNC client and python library
Other
454 stars 122 forks source link

Fix error when buffer doesn't start with RFB #215

Closed liquidpele closed 1 year ago

liquidpele commented 2 years ago

Fixes this error:

[CRITICAL]  2022-03-20T06:41:29.696Z    ba5101fc-68db-4723-9ab2-c68cd21c6f72    Unhandled Error
Traceback (most recent call last):
  File "/var/task/twisted/python/log.py", line 103, in callWithLogger
    return callWithContext({"system": lp}, func, *args, **kw)
  File "/var/task/twisted/python/log.py", line 86, in callWithContext
    return context.call({ILogContext: newCtx}, func, *args, **kw)
  File "/var/task/twisted/python/context.py", line 122, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "/var/task/twisted/python/context.py", line 85, in callWithContext
    return func(*args,**kw)
--- <exception caught here> ---
  File "/var/task/twisted/internet/posixbase.py", line 614, in _doReadOrWrite
    why = selectable.doRead()
  File "/var/task/twisted/internet/tcp.py", line 243, in doRead
    return self._dataReceived(data)
  File "/var/task/twisted/internet/tcp.py", line 249, in _dataReceived
    rval = self.protocol.dataReceived(data)
  File "/var/task/vncdotool/rfb.py", line 527, in dataReceived
    self._handler()
  File "/var/task/vncdotool/rfb.py", line 151, in _handleInitial
    self._version_server = version_server
builtins.UnboundLocalError: local variable 'version_server' referenced before assignment

Our version is a bit outdated but the issue still exists in main.

sibson commented 2 years ago

What server is this running against? What string is the server returning? This seems like the server is not following the vnc protocol so I'd like to better understand if this is the right fix to accept.

liquidpele commented 2 years ago

So, we're using this as an app probe to determine if the port is actually vnc during a host network check, and the server this happened on vanished already. I can see if it returns and try to get that info for you, but I can't promise it will. That's why my PR is mostly just accounting for the reference before assignment error currently, with a log line to get that info. I'll let you know if I can dig up more for you.

sibson commented 2 years ago

Got it, it sounds like you connected to a server but it wasn't speaking VNC protocol. If that is the case, then I think the issue is that the exception raised is UnboundLocalError rather than cleanly handling the error with a more descriptive exception.

pmhahn commented 1 year ago

This PR no longer applies and is no longer necessary since the RFB version parsing and handling was rewritten by bc93eb97.