[
https://issues.apache.org/jira/browse/GUACAMOLE-1514?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17480975#comment-17480975
]
Gabriel Joseph Valcázar Berdofe commented on GUACAMOLE-1514:
------------------------------------------------------------
Hi Mike,
Apologies for the slight delay in my response. I've been looking into the
source of this issue and it seems to stem from how the RDP server is
implemented, rather than the RDP client.
The server is a slightly modified version of Weston 9.0.0 with FreeRDP 2.3.0
(although the modifications don't affect the RDP server component of Weston, so
int this context it can be considered a regular Weston 9.0.0) running on an
embedded system. It turns out Weston only calls *update->SurfaceBits* whenever
the output needs to be repainted (it also calls _update->SurfaceFrameMarker_ to
indicate that the frame has ended when not using RemoteFX or NSCodec). Since
neither of these hooks are implemented in guacamole-server, their default
versions are used instead: *gdi_surface_bits()* and
{*}gdi_surface_frame_marker(){*}, both implemented in libfreerdp.
This results in the gdi buffer being used, rather than the guacamole one. In
turn, this explains the behavior I'm seeing:
* For one, the sudden crash explained in this JIRA's description.
* Once the crash is fixed with the patch attached to this JIRA, I'm unable to
see the desktop in the web browser, although I _am_ able to interact with it
(if I click on the GUI's elements in the coordinates where they would normally
appear, the application reacts accordingly). I was initially convinced that
this was due to the fact that guacamole doesn't support RemoteFX, but now I'm
certain it's because the bitmap data is being copied to gdi's internal buffer,
rather than guacamole's.
Other RDP clients I've tried, such as KRDC and Remmina, seems to work fine even
if I explicitly disable the use of RemoteFX or any other bitmap codec. However,
neither of them register custom hooks for _SurfaceBits_ or
{_}SurfaceFrameMarker{_}, so I assume they just rely on libfreerdp's default
gdi implementation.
Is it feasible to implement hooks for these functions? Or is there any kind of
workaround that would allow the Weston RDP server to be usable via guacamole?
> Self-built guacd, Release 1.4.0, crashes when browser resolution is smaller
> than remote desktop resolution
> ----------------------------------------------------------------------------------------------------------
>
> Key: GUACAMOLE-1514
> URL: https://issues.apache.org/jira/browse/GUACAMOLE-1514
> Project: Guacamole
> Issue Type: Bug
> Components: guacamole-server, guacd, RDP
> Affects Versions: 1.4.0
> Reporter: Gabriel Joseph Valcázar Berdofe
> Priority: Major
> Attachments:
> 0001-rdp-gdi-resize-bitmap-buffer-to-match-server-s-displ.patch
>
>
> ||Component||Release||
> |guacd|1.4.0|
> |OS|Kubuntu 18.04|
> |FreeRDP|2.4.1|
> |RDP server|Weston 9.0.0 w/ RDP backend|
> I originally found this bug when testing a specific application which uses a
> rotated 800x1280 UI. Attempting to access the application via Guacamole
> always resulted in its RDP client crashing and resetting the connection with
> no additional information in the log, even with debug messages enabled. I
> then tried using different combinations of web browser and application sizes,
> and the client always crashes as long as the initial web browser size is
> smaller than the application's.
> Here's a guacd log using a small browser size and the 800x1280:
> {code:java}
> guacd[17516]: INFO: Connection ID is
> "$402f4aad-2d4a-4f07-86af-4fff9288e35c"
> guacd[17519]: DEBUG: Processing instruction: size
> guacd[17519]: DEBUG: Processing instruction: audio
> guacd[17519]: DEBUG: Processing instruction: video
> guacd[17519]: DEBUG: Processing instruction: image
> guacd[17519]: DEBUG: Processing instruction: timezone
> guacd[17519]: DEBUG: Parameter "console" omitted. Using default value of 0.
> guacd[17519]: DEBUG: Parameter "console-audio" omitted. Using default
> value of 0.
> guacd[17519]: DEBUG: Parameter "disable-auth" omitted. Using default value
> of 0.
> guacd[17519]: INFO: No security mode specified. Defaulting to security
> mode negotiation with server.
> guacd[17519]: DEBUG: User resolution is 808x452 at 96 DPI
> guacd[17519]: DEBUG: Parameter "dpi" omitted. Using default value of 96.
> guacd[17519]: DEBUG: Using resolution of 808x452 at 96 DPI
> guacd[17519]: DEBUG: Parameter "force-lossless" omitted. Using default
> value of 0.
> guacd[17519]: DEBUG: Parameter "read-only" omitted. Using default value of
> 0.
> guacd[17519]: DEBUG: Parameter "client-name" omitted. Using default value
> of "Guacamole RDP".
> guacd[17519]: DEBUG: Parameter "enable-theming" omitted. Using default
> value of 0.
> guacd[17519]: DEBUG: Parameter "enable-font-smoothing" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "enable-full-window-drag" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "enable-desktop-composition" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "enable-menu-animations" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "disable-bitmap-caching" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "disable-offscreen-caching" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "disable-glyph-caching" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Glyph caching is currently universally disabled,
> regardless of the value of the "disable-glyph-caching" parameter, as glyph
> caching support is not considered stable by FreeRDP as of the FreeRDP 2.0.0
> release. See: https://issues.apache.org/jira/browse/GUACAMOLE-1191
> guacd[17519]: DEBUG: Parameter "disable-audio" omitted. Using default
> value of 0.
> guacd[17519]: DEBUG: Parameter "enable-printing" omitted. Using default
> value of 0.
> guacd[17519]: DEBUG: Parameter "printer-name" omitted. Using default value
> of "Guacamole Printer".
> guacd[17519]: DEBUG: Parameter "enable-drive" omitted. Using default value
> of 0.
> guacd[17519]: DEBUG: Parameter "drive-name" omitted. Using default value
> of "Guacamole Filesystem".
> guacd[17519]: DEBUG: Parameter "drive-path" omitted. Using default value
> of "".
> guacd[17519]: DEBUG: Parameter "create-drive-path" omitted. Using default
> value of 0.
> guacd[17519]: DEBUG: Parameter "disable-download" omitted. Using default
> value of 0.
> guacd[17519]: DEBUG: Parameter "disable-upload" omitted. Using default
> value of 0.
> guacd[17519]: DEBUG: Parameter "timezone" omitted. Using default value of
> "Europe/Madrid".
> guacd[17519]: DEBUG: Parameter "enable-sftp" omitted. Using default value
> of 0.
> guacd[17519]: DEBUG: Parameter "sftp-hostname" omitted. Using default
> value of "10.101.2.164".
> guacd[17519]: DEBUG: Parameter "sftp-port" omitted. Using default value of
> "22".
> guacd[17519]: DEBUG: Parameter "sftp-username" omitted. Using default
> value of "".
> guacd[17519]: DEBUG: Parameter "sftp-password" omitted. Using default
> value of "".
> guacd[17519]: DEBUG: Parameter "sftp-passphrase" omitted. Using default
> value of "".
> guacd[17519]: DEBUG: Parameter "sftp-root-directory" omitted. Using
> default value of "/".
> guacd[17519]: DEBUG: Parameter "sftp-server-alive-interval" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "sftp-disable-download" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "sftp-disable-upload" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "recording-name" omitted. Using default
> value of "recording".
> guacd[17519]: DEBUG: Parameter "recording-exclude-output" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "recording-exclude-mouse" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "recording-exclude-touch" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "recording-include-keys" omitted. Using
> default value of 0.
> guacd[17519]: DEBUG: Parameter "create-recording-path" omitted. Using
> default value of 0.
> guacd[17519]: INFO: Resize method: none
> guacd[17519]: DEBUG: Parameter "enable-touch" omitted. Using default value
> of 0.
> guacd[17519]: DEBUG: Parameter "enable-audio-input" omitted. Using default
> value of 0.
> guacd[17519]: DEBUG: Parameter "gateway-port" omitted. Using default value
> of 443.
> guacd[17519]: DEBUG: Parameter "disable-copy" omitted. Using default value
> of 0.
> guacd[17519]: DEBUG: Parameter "disable-paste" omitted. Using default
> value of 0.
> guacd[17519]: INFO: No clipboard line-ending normalization specified.
> Defaulting to preserving the format of all line endings.
> guacd[17519]: DEBUG: Parameter "wol-send-packet" omitted. Using default
> value of 0.
> guacd[17519]: INFO: User "@367f8cf8-3120-42c1-8f48-9839a2a1d808" joined
> connection "$402f4aad-2d4a-4f07-86af-4fff9288e35c" (1 users now present)
> guacd[17519]: DEBUG: Client is using protocol version "VERSION_1_3_0"
> guacd[17519]: INFO: Loading keymap "base"
> guacd[17519]: INFO: Loading keymap "en-us-qwerty"
> guacd[17519]: DEBUG: freerdp_connect:freerdp_set_last_error_ex resetting
> error state
> guacd[17519]: DEBUG: Support for CLIPRDR (clipboard redirection)
> registered. Awaiting channel connection.
> guacd[17519]: DEBUG: Support for static channel "rdpdr" loaded.
> guacd[17519]: DEBUG: Support for static channel "rdpsnd" loaded.
> guacd[17519]: DEBUG: Local framebuffer format PIXEL_FORMAT_BGRX32
> guacd[17519]: DEBUG: Remote framebuffer format PIXEL_FORMAT_BGR24
> guacd[17519]: DEBUG: primitives autodetect, using optimized
> guacd[17519]: DEBUG:
> freerdp_tcp_is_hostname_resolvable:freerdp_set_last_error_ex resetting error
> state
> guacd[17519]: DEBUG: freerdp_tcp_connect:freerdp_set_last_error_ex
> resetting error state
> guacd[17519]: DEBUG: CLIPRDR (clipboard redirection) channel connected.
> guacd[17519]: DEBUG: SVC "rdpdr" connected.
> guacd[17519]: DEBUG: SVC "rdpsnd" connected.
> guacd[17519]: DEBUG: Server resized display to 800x1280
> guacd[17519]: DEBUG: tpdu length 0 > tpkt header length 0
> ...
> guacd[17519]: DEBUG: tpdu length 0 > tpkt header length 0
> guacd[17516]: INFO: Connection "$402f4aad-2d4a-4f07-86af-4fff9288e35c"
> removed. {code}
> After debugging FreeRDP, I found the root cause of the issue: the guacamole
> RDP component initializes FreeRDP's gdi bitmap buffer using the web browser's
> initial resolution (in the log above, "Using resolution of 808x452 at 96
> DPI"). Then, guacamole receives the server's desktop resolution ("Server
> resized display to 800x1280"), but the bitmap buffer is not resized
> accordingly. Once guacamole's FreeRDP receives the server's bitmap and copies
> it to gdi's buffer, one of three things can happen:
> * The browser and desktop resolutions are the same ->the bitmap fits
> perfectly
> * The browser is larger than the desktop -> the bitmap fits with room to
> spare
> * The browser is smaller than the desktop -> the bitmap {*}doesn't fit{*},
> and eventually, there is an *out of bounds* memcpy that causes
> FreeRDP/guacamole to crash.
> In my case, the crash happens in this part of the _freerdp_image_copy()_
> function in FreeRDP. As soon as the loop surpasses the web browser's height,
> guacamole crashes (in the case of the log above, since the browser has a
> height of {*}452{*}, the crash happens in iteration {*}453{*}):
> {code:java}
> for (y = 0; y < (INT32)nHeight; y++)
> {
> const BYTE* srcLine =
> &pSrcData[(y + nYSrc) * nSrcStep * srcVMultiplier +
> srcVOffset];
> BYTE* dstLine = &pDstData[(y + nYDst) * nDstStep *
> dstVMultiplier + dstVOffset];
> memcpy(&dstLine[xDstOffset], &srcLine[xSrcOffset],
> copyDstWidth);
> } {code}
> Thankfully, FreeRDP's gdi API has a function that allows to resize the bitmap
> buffer on the fly. By calling said function from guacamole when it receives
> the server's resolution ("Server resized display to 800x1280"), I was able to
> circumvent the issue. I'm not sure if this fix has other implications, but
> I'm attaching a patch for it.
> Also, I don't know if this issue is specific to my client/server combination,
> but I haven't seen the gdi buffer being resized anywhere else in the code, so
> we might as well do so if we have the chance.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
