Repository navigation
fix: don't send display size to guacd while a VNC connection is still being established - #15
Closed
aerandir83 wants to merge 1 commit into
Closed
aerandir83 wants to merge 1 commit into
aerandir83 wants to merge 1 commit into
Conversation
The ResizeObserver sent a size instruction 150 ms after first layout, even though the connected handler already sends the initial size. For a VNC host whose handshake takes longer than that (macOS Screen Sharing takes about a second), guacd 1.6.0 received the size mid-connect and segfaulted in libvncclient, so the tab sat on "Negotiating VNC handshake" forever. Skip sendSize while isConnectingRef is set; the state 3 handler sends it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PsrdLuuh8bZNDnGMA3cG1U
|
AI review | Looks safe to merge Adds a guard to skip sending the initial display size while a VNC connection is establishing, preventing a known guacd segfault. The change is minimal, follows conventions, and introduces no security risks. Reviewed 1da920c. AI can be wrong, a maintainer still decides. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
GuacamoleDisplay's ResizeObserver callssendSize()as soon as the container has a non-zero size, including while the connection is still being established. For VNC hosts this reaches guacd while it is still dialing the server. With guacd 1.6.0 (libvncclient 0.9.15) this segfaults the VNC client process, and the tab stays on "Negotiating VNC handshake" forever with no error.I hit this against a macOS Screen Sharing host (RFB 003.889). The handshake and VNC auth succeed, but the session never renders because guacd crashes.
Fix
Skip the first
sendSize()whileisConnectingRefis set. The existing connected handler already sends the first size, so nothing is lost.Testing
Tested against a real macOS host in a local Termix 26.10 stack with Remote Desktop 1.0.2. Before the change, guacd segfaults and the tab hangs on the handshake. After it, the lock screen renders and there are no guacd crashes. Sending the size only after state 3 (connected) does not crash.
Not tested: RDP hosts. The guard only affects the first size sent during connect.