You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "necouchman (via GitHub)" <gi...@apache.org> on 2025/08/22 15:59:40 UTC
[PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
necouchman opened a new pull request, #611:
URL: https://github.com/apache/guacamole-server/pull/611
This is likely the first of a couple of changes to correct issues with RAIL support in FreeRDP3 and in Guacamole's support of GFX. I finally stumbled upon the reason that the FreeRDP3 changes had broken RAIL support - the RAIL plugin was being loaded _only_ the `PreConnect` callback (`rdp_freerdp_pre_connect()`), and not in the `LoadChannels` callback (`rdp_freerdp_load_channels()`). The change in FreeRDP (https://github.com/FreeRDP/FreeRDP/commit/ef8c9e48e8acf61e16cb9fd547799067c87b20da) that broke this unified the reloading of channels into a single function that disconnected and closed all of the channels and then used the `LoadChannels` callback to reload all of them. Since RAIL was not being processed through that callback, it would get dropped but never reconnected. This moves the processing of the RAIL plugin, along with any user-defined static plugins, to the `LoadChannels` callback, which keeps it connected in recent FreeRDP 3.x versions.
I think these changes are solid and ready to be merged - it's pretty simple, and seems to fix at least the RAIL plugin connection issue. That said, I'm still seeing some issues with RAIL when GFX support is enabled (hangs/disconnects/freezes) that don't occur when you disable GFX support, so I think there may be additional items to address related to the intersection of GFX + RAIL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #611:
URL: https://github.com/apache/guacamole-server/pull/611#discussion_r2387190900
##########
src/protocols/rdp/settings.c:
##########
@@ -1683,7 +1683,8 @@ void guac_rdp_push_settings(guac_client* client,
if (guac_settings->remote_app != NULL) {
freerdp_settings_set_bool(rdp_settings, FreeRDP_Workarea, TRUE);
freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteApplicationMode, TRUE);
- freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, TRUE);
+ freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, FALSE);
Review Comment:
I think we should revert this unless things do not work without it disabled.
IIRC, the language bar is the UI component of Windows that allows switching between input methods, and would be needed for languages like Chinese and Japanese.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on PR #611:
URL: https://github.com/apache/guacamole-server/pull/611#issuecomment-3257376428
NICE!!! Looking through this now...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #611:
URL: https://github.com/apache/guacamole-server/pull/611#issuecomment-3348768806
> > That said, I'm still seeing some issues with RAIL when GFX support is enabled (hangs/disconnects/freezes) that don't occur when you disable GFX support, so I think there may be additional items to address related to the intersection of GFX + RAIL.
>
> I wonder if this might have something to do with:
>
> ```c
> freerdp_settings_set_bool(rdp_settings, FreeRDP_HiDefRemoteApp, guac_settings->enable_gfx);
> ```
>
> (https://github.com/necouchman/guacamole-server/blob/10f2ccf3050f4c720b94d3a07a4753083556b80d/src/protocols/rdp/settings.c#L1687)
>
> I'm not sure why that would cause issues, but it is at least one direct connection between RemoteApp and whether GFX is enabled.
I did try playing with and toggling that setting, to no apparent avail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper merged PR #611:
URL: https://github.com/apache/guacamole-server/pull/611
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on PR #611:
URL: https://github.com/apache/guacamole-server/pull/611#issuecomment-3348709500
> That said, I'm still seeing some issues with RAIL when GFX support is enabled (hangs/disconnects/freezes) that don't occur when you disable GFX support, so I think there may be additional items to address related to the intersection of GFX + RAIL.
I wonder if this might have something to do with:
```c
freerdp_settings_set_bool(rdp_settings, FreeRDP_HiDefRemoteApp, guac_settings->enable_gfx);
```
(https://github.com/necouchman/guacamole-server/blob/10f2ccf3050f4c720b94d3a07a4753083556b80d/src/protocols/rdp/settings.c#L1687)
I'm not sure why that would cause issues, but it is at least one direct connection between RemoteApp and whether GFX is enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #611:
URL: https://github.com/apache/guacamole-server/pull/611#discussion_r2387624266
##########
src/protocols/rdp/settings.c:
##########
@@ -1683,7 +1683,8 @@ void guac_rdp_push_settings(guac_client* client,
if (guac_settings->remote_app != NULL) {
freerdp_settings_set_bool(rdp_settings, FreeRDP_Workarea, TRUE);
freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteApplicationMode, TRUE);
- freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, TRUE);
+ freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, FALSE);
Review Comment:
Okay, this should be reverted, now, via rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #611:
URL: https://github.com/apache/guacamole-server/pull/611#discussion_r2325009951
##########
src/protocols/rdp/channels/rail.c:
##########
@@ -64,24 +64,6 @@ static UINT guac_rdp_rail_complete_handshake(RailClientContext* rail) {
guac_client* client = (guac_client*) rail->custom;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
- RAIL_HANDSHAKE_ORDER handshake = {
-
- /* Build number 7600 (0x1DB0) apparently represents Windows 7 and
- * compatibility with RDP 7.0. As of this writing, this is the same
- * build number sent for RAIL connections by xfreerdp. */
- .buildNumber = 7600
-
- };
-
- /* Send client handshake response */
- guac_client_log(client, GUAC_LOG_TRACE, "Sending RAIL handshake.");
- pthread_mutex_lock(&(rdp_client->message_lock));
- status = rail->ClientHandshake(rail, &handshake);
Review Comment:
Yes, see: https://github.com/FreeRDP/FreeRDP/issues/11785#issuecomment-3197896683
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #611:
URL: https://github.com/apache/guacamole-server/pull/611#discussion_r2325011574
##########
src/protocols/rdp/settings.c:
##########
@@ -1683,7 +1683,8 @@ void guac_rdp_push_settings(guac_client* client,
if (guac_settings->remote_app != NULL) {
freerdp_settings_set_bool(rdp_settings, FreeRDP_Workarea, TRUE);
freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteApplicationMode, TRUE);
- freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, TRUE);
+ freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, FALSE);
Review Comment:
I think I changed it as part of the debugging I was doing, I can definitely flip that back...although, I'm wondering, do we actually support the language bar? I've never seen it pop up in my use of RAIL support, so not sure if we should be saying we support that or not?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #611:
URL: https://github.com/apache/guacamole-server/pull/611#discussion_r2324348171
##########
src/protocols/rdp/channels/rail.c:
##########
@@ -64,24 +64,6 @@ static UINT guac_rdp_rail_complete_handshake(RailClientContext* rail) {
guac_client* client = (guac_client*) rail->custom;
guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
- RAIL_HANDSHAKE_ORDER handshake = {
-
- /* Build number 7600 (0x1DB0) apparently represents Windows 7 and
- * compatibility with RDP 7.0. As of this writing, this is the same
- * build number sent for RAIL connections by xfreerdp. */
- .buildNumber = 7600
-
- };
-
- /* Send client handshake response */
- guac_client_log(client, GUAC_LOG_TRACE, "Sending RAIL handshake.");
- pthread_mutex_lock(&(rdp_client->message_lock));
- status = rail->ClientHandshake(rail, &handshake);
Review Comment:
Is this handled automatically by FreeRDP?
##########
src/protocols/rdp/settings.c:
##########
@@ -1683,7 +1683,8 @@ void guac_rdp_push_settings(guac_client* client,
if (guac_settings->remote_app != NULL) {
freerdp_settings_set_bool(rdp_settings, FreeRDP_Workarea, TRUE);
freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteApplicationMode, TRUE);
- freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, TRUE);
+ freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, FALSE);
Review Comment:
Why is this changing to `FALSE`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2072: RAIL plugin should be loaded in the LoadPlugins callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #611:
URL: https://github.com/apache/guacamole-server/pull/611#discussion_r2384226643
##########
src/protocols/rdp/settings.c:
##########
@@ -1683,7 +1683,8 @@ void guac_rdp_push_settings(guac_client* client,
if (guac_settings->remote_app != NULL) {
freerdp_settings_set_bool(rdp_settings, FreeRDP_Workarea, TRUE);
freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteApplicationMode, TRUE);
- freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, TRUE);
+ freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, FALSE);
Review Comment:
@mike-jumper Any further thoughts on reverting this or leaving disabled?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org