Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
1.4.0 (UNRELEASED)
----------------------
- Fix black screen captures from servers that announce DesktopSize before sending pixel data, e.g. TightVNC (@sibson, #90)
- Fix the dead protocol reference in the published ``rfb`` module documentation, which pointed at a RealVNC PDF that has been 403 for years; RFC 6143 and the rfbproto community document replace it (@sibson)
- Declare python_requires >=3.10, matching the versions CI tests and the development requirements. 3.9 was advertised but neither tested nor able to install the dev environment (@sibson, #357)
- [BREAKING] vncdo exit codes now say what went wrong: single digits for bad input, including 3 for authentication, and tens grouped by cause out on the wire, 10s connection, 20s protocol, 30s command, 40s timeout, documented in docs/usage.rst. Scripts reading the exit code see new values: authentication failure is now 3 and a session cut short 11, both of which used to be 0; an unknown action is now 2, previously 1 (@sibson, #345)
Expand Down
50 changes: 50 additions & 0 deletions tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,56 @@ def test_commitUpdate(self) -> None:

self.deferred.callback.assert_called_once_with(self.client)

# A framebuffer update whose only rectangle is the DesktopSize
# pseudo-encoding carries no pixel data.
MSG_FBU_DESKTOP_SIZE_ONLY = (
b"\x00" # FRAMEBUFFER_UPDATE
b"\x00" # padding
b"\x00\x01" # number-of-rectangles
b"\x00\x00\x00\x00\x07\x80\x04\xb0" # x=0 y=0 w=1920 h=1200
b"\xff\xff\xff\x21" # PSEUDO_DESKTOP_SIZE (-223)
)
MSG_FBU_ONE_PIXEL = (
b"\x00" # FRAMEBUFFER_UPDATE
b"\x00" # padding
b"\x00\x01" # number-of-rectangles
b"\x00\x00\x00\x00\x00\x01\x00\x01" # x=0 y=0 w=1 h=1
b"\x00\x00\x00\x00" # Encoding.RAW
b"\xff\x00\x00\x00" # one RGBX pixel
)

def _connect(self) -> None:
self.client._packet = bytearray(self.MSG_HANDSHAKE)
self.client._handleInitial()
self.client._handleServerInit(self.MSG_INIT)

def test_desktop_size_only_update_rerequests_instead_of_completing(self) -> None:
cli = self.client
self._connect()
d = cli.refreshScreen()
fired: list = []
d.addCallback(fired.append)
cli.framebufferUpdateRequest.reset_mock()

cli.dataReceived(self.MSG_FBU_DESKTOP_SIZE_ONLY)

self.assertEqual(fired, [])
cli.framebufferUpdateRequest.assert_called_once_with()

def test_refresh_completes_once_pixel_data_arrives(self) -> None:
cli = self.client
self._connect()
d = cli.refreshScreen()
fired: list = []
d.addCallback(fired.append)

cli.dataReceived(self.MSG_FBU_DESKTOP_SIZE_ONLY)
cli.dataReceived(self.MSG_FBU_ONE_PIXEL)

self.assertEqual(fired, [cli])
assert cli.screen is not None
self.assertEqual(cli.screen.size, (1920, 1200))

def test_vncRequestPassword_attribute(self):
cli = self.client
cli.sendPassword = mock.Mock()
Expand Down
62 changes: 0 additions & 62 deletions tests/unit/test_issue_90.py

This file was deleted.

5 changes: 5 additions & 0 deletions vncdotool/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,11 @@ def updateRectangle(

def commitUpdate(self, rectangles: list[rfb.Rect] | None = None) -> None:
if self.deferred:
if not rectangles:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should commitUpdate even get called if we are properly deleting the pseudo rectangle

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude Code: Still needs to be called — it's the extension point, not incidental plumbing. Proof: the base class already routes a genuinely-empty update (server sends rect count 0) through the same commitUpdate(self.rectanglePos) call in _doConnection, so "zero rectangles delivered" already meant "call commitUpdate anyway" before this fix existed. Moving the retry policy down into rfb.py instead would only patch the DesktopSize case and leave the QEMU-pseudo-rect-only case and a literal empty update still completing on nothing.

What is fixable: the append-then-delete on rectanglePos for the two pseudo-encodings is backwards — skip the append instead of adding and undoing it. Saving that as a follow-up rather than folding it into this PR.

# No rectangle in this update painted self.screen; wait for
# one that does before completing the refresh.
self.framebufferUpdateRequest()
return
d = self.deferred
self.deferred = None
d.callback(self)
Expand Down
1 change: 1 addition & 0 deletions vncdotool/rfb.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ def _handleRectangle(self, block: bytes) -> None:
length += ((width + 7) // 8) * height
self.expect(self._handleDecodePsuedoCursor, length, x, y, width, height)
elif encoding == Encoding.PSEUDO_DESKTOP_SIZE:
del self.rectanglePos[-1] # undo append as this carries no pixel data
self._handleDecodeDesktopSize(width, height)
elif encoding == Encoding.PSEUDO_QEMU_EXTENDED_KEY_EVENT:
self.negotiated_encodings.add(Encoding.PSEUDO_QEMU_EXTENDED_KEY_EVENT)
Expand Down
Loading