GUACAMOLE-1196: Implement support for VNC display size updates

[necouchman]

This PR takes a crack at implementing the recently-added libvncclient support for SendExtDesktopSize(), which should allow Guacamole as a VNC client to send its size to VNC servers that support such functionality and have the display dynamically resized.

I'm creating as a draft for the moment since I have not actually tested the functionality. Among the things I'm not sure of:

• The RDP protocol contains a resolution setting which specifies the DPI resolution of the screen. I don't see any reason to try to mirror this into the VNC protocol, as I don't think VNC actually does anything with it (I think the DPI is fixed), but if I'm wrong let me know and I can add that support and try to update the code accordingly.
• In the Jira issue (https://issues.apache.org/jira/browse/GUACAMOLE-1196), @mike-jumper mentioned a way of possibly sending the resize command/message using WriteToRFBServer() - I'm wondering if it's worth trying to implement the display update functionality with that in cases where libvncclient doesn't contain the SendExtDesktopSize() function?
• I added the message_lock mutex for sending this message, as in the RDP protocol, but I don't know if there are other places where this lock should be used (authentication, for example), or if it's even required in this scenario?
• I moved a few bits from the RDP protocol to the common/display.h file - among them are the minimum and maximum display size. It wasn't clear to me from the comments in the code whether those minimum and maximum sizes are arbitrary or if there's some particular reason those values are used? Not sure if there's something specific to RDP that has those bounds?

[necouchman]

Okay, did some initial testing, and the desktop size messages are being sent correctly through to the VNC server. Unfortunately I'm hitting an issue with the VNC server (tigervnc) where it is rejecting the size updates due to missing modelines for Xvnc. Obviously having to load all the possible modelines for all of the possible resolutions that Guacamole might request is a bit silly, so I'm thinking that I might need to find another VNC server that doesn't have this requirement.

Good news, though, is that the changes on the Guacamole side seem sound.

[DrummyFloyd]

Hi , any update regarding this wonderfull and waited PR ? =D

[necouchman]

Hi , any update regarding this wonderfull and waited PR ? =D

Looks like Mike has started reviewing it. Any chance you'd be able to test it out? I've been unable to get access to a VNC server that actually supports it...

[DrummyFloyd]

Hi , any update regarding this wonderfull and waited PR ? =D

Looks like Mike has started reviewing it. Any chance you'd be able to test it out? I've been unable to get access to a VNC server that actually supports it...

tbh, i only know tigerVnc, i can try the latest one , because it seems taht the options is presnet in latest tigerVNC + vncserver =>-AcceptSetDesktopSize

[necouchman]

Hi , any update regarding this wonderfull and waited PR ? =D

Looks like Mike has started reviewing it. Any chance you'd be able to test it out? I've been unable to get access to a VNC server that actually supports it...

tbh, i only know tigerVnc, i can try the latest one , because it seems taht the options is presnet in latest tigerVNC + vncserver =>-AcceptSetDesktopSize

If you're able to give it a try with the changes from this PR, that'd be great. I'm also using TigerVNC, and, according to the Xvnc man page, AcceptSetDesktopSize is enabled by default, so it should be working, but I was getting errors indicating that any resolution I wanted to set needed a modeline in the X configuration, which just isn't tenable.

[necouchman]

Okay, I was able to test it and am successfully sending the SendExtDesktopSize() message size, but immediately after the resize it's failing on the HandleRFBServerMessage() call, here:

https://github.com/necouchman/guacamole-server/blob/12c98e331c1dc85b36f2c194fc067ba7fbd29cfe/src/protocols/vnc/vnc.c#L473-L479

So, something about the resize is not working correctly - I'm not sure if there's something that needs to be updated in the rfbClient when the resize is sent, or it's a bug, or...?

[x-7]

Hi , any update regarding this wonderfull and waited PR ? =D

Looks like Mike has started reviewing it. Any chance you'd be able to test it out? I've been unable to get access to a VNC server that actually supports it...

tbh, i only know tigerVnc, i can try the latest one , because it seems taht the options is presnet in latest tigerVNC + vncserver =>-AcceptSetDesktopSize

If you're able to give it a try with the changes from this PR, that'd be great. I'm also using TigerVNC, and, according to the Xvnc man page, AcceptSetDesktopSize is enabled by default, so it should be working, but I was getting errors indicating that any resolution I wanted to set needed a modeline in the X configuration, which just isn't tenable.

[necouchman]

@x-7 Thank you for testing! I'm glad to hear that it works for you - maybe the error that I'm currently seeing is actually a bug in the VNC Server, and not something with how I've implemented it. I'll try to find some additional VNC servers to test with - maybe even grab an eval copy of RealVNC or something like that and see if that works.

[x-7]

@x-7 Thank you for testing! I'm glad to hear that it works for you - maybe the error that I'm currently seeing is actually a bug in the VNC Server, and not something with how I've implemented it. I'll try to find some additional VNC servers to test with - maybe even grab an eval copy of RealVNC or something like that and see if that works.

@x-7 Thank you for testing! I'm glad to hear that it works for you - maybe the error that I'm currently seeing is actually a bug in the VNC Server, and not something with how I've implemented it. I'll try to find some additional VNC servers to test with - maybe even grab an eval copy of RealVNC or something like that and see if that works.

yes,I used two clients for testing: next-terminal(https://github.com/dushixiang/next-terminal) and gucalmole-client.

1. next-terminal "autoresize" is ok, but when the mouse is placed on the edge of the window, the mouse style does not change to the zoom style.

2. gucalmole-client client "autoresize" is also ok

[necouchman]

Well, finally tracked down what's going on, here, and it appears that there is a bug in libvncclient, and not all of the required data is initialized to send a screen update. In particular, the RFB protocol specifies that a the screen update message should include the screen id, x and y offset, width and height, and (unused but initialized) flags. libvncclient currently only initializes and sends the x and y offset, resulting in bogus data being sent for the id, offset, and flags. While some VNC servers may be forgiving of this (or ignore it), TigerVNC does not, and this results in its refusal to resize correctly.

I've put in a pull request in the LibVNC/libvncserver repo to correct this, and it appears to work. However, until they accept that and release a new version, this effectively will not work. The options, then, are:

• Don't merge this code until libvncserver/libvncclient has a release that will actually make use of it.
• Merge this code, anyway, with a warning that it will probably not work with most VNC servers.
• Write a function that uses the WriteToRFBServer function and generate our own resize message, with the expected data, and then update Guacamole to use the corrected/built-in method when it gets fixed.

I'm tempted to go with the last option so that this feature can be used, so I'll see what I can come up with.

[necouchman]

Okay - one more change I've tried to make is to set the initial display size as soon as the connection is active; however, I'm having trouble making this work correctly, as the places where I could do it (as part of the user join handler, or before the main client loop for the VNC protocol) don't actually work because the display isn't fully initialized, yet. If I put it inside the main CLIENT_RUNNING loop, the callback runs repeatedly, which isn't the end of the world, but also isn't ideal - it really only needs to run as soon as the VNC connection is fully established. I'm open to any hints anyone has on better placement...