FreeRDP 2.0.0 will automatically free all settings strings when the
settings structure is freed. As we will also do the same for our own
settings strings, the FreeRDP settings must be kept independent. There
is no guarantee that the FreeRDP settings will be pushed before an error
causes the connection to abort, nor that the FreeRDP settings will not
need to be pushed multiple times due to an automatic reconnect.
The intent of the previous version of the SFTP directory listing code
was to break the JSON transfer at blob boundaries, waiting for an ack
before sending the next blob, however the ordering of the "blob_written"
and directory read checks could result in a directory entry being
skipped at the boundary of each blob.
The proper order would be to check the "blob_written" flag first,
however the "blob_written" flag is unnecessary. It's simpler and more
correct to just break out of the loop once the desired blob has been
flushed.
This commit effectively reverts commit 9855d875c7.
With relaxed order checks enabled, FreeRDP will indeed invoke the
OpaqueRect and PatBlt handlers (even though we do not announce support
for those orders) as long as handlers are provided.
FreeRDP version 2.0.0-rc0 and older will automatically free whatever
entry_points->pInterface is set to when the channel plugin is unloaded.
This doesn't happen in later versions, but will result in a double-free
upon disconnect for 2.0.0-rc0 in our case. As we don't need pInterface,
we can safely set this to NULL and avoid the issue entirely.
In FreeRDP 2.0.0-rc0 and earlier, Bitmap_Free(bitmap) invokes the free
handler of the given bitmap, frees bitmap->data, and then frees the
bitmap. The implementation-specific free handler needs to be aware only
of the implementation's own concerns.
After FreeRDP 2.0.0-rc0, Bitmap_Free(bitmap) only invokes the
implementation-specific free handler, and it's on the implementation to
know that bitmap->data must be manually freed with _aligned_free() and
bitmap must be freed with free(). The implementation-specific free
handler must be aware of the internals of the library.
See commit 8dda26a.
If the admin intentionally disables both copy and paste, then there is
no need to log a warning that this is the case; it was intentional. A
warning will likely set off alarm bells within production log monitoring
systems.
On some platforms, the libguacai-client.so plugin for FreeRDP reports an
unlinked symbol:
undefined symbol: guac_freerdp_dynamic_channel_collection_add (/usr/local/lib/freerdp2/libguacai-client.so)
This symbol is actually unused within the plugin, but may be referenced
due to being defined within a function in a common piece of source
shared between the plugin and the RDP support.
Separating the actual common components such that they can be included
by both the RDP support and the libguacai-client.so plugin removes the
potential for unused pieces being flagged as missing.
Without a typecast, errors like the following are generated by the
compiler:
channels/cliprdr.c: In function 'guac_rdp_cliprdr_channel_connected':
channels/cliprdr.c:477:27: error: assignment from incompatible pointer type [-Werror]
cliprdr->MonitorReady = guac_rdp_cliprdr_monitor_ready;
^
channels/cliprdr.c:478:31: error: assignment from incompatible pointer type [-Werror]
cliprdr->ServerFormatList = guac_rdp_cliprdr_format_list;
^
channels/cliprdr.c:479:38: error: assignment from incompatible pointer type [-Werror]
cliprdr->ServerFormatDataRequest = guac_rdp_cliprdr_format_data_request;
^
channels/cliprdr.c:480:39: error: assignment from incompatible pointer type [-Werror]
cliprdr->ServerFormatDataResponse = guac_rdp_cliprdr_format_data_response;
^
cc1: all warnings being treated as errors
This is because FreeRDP commit 65812bd added const to the pointer
argument of each of these handlers, wheras older versions of FreeRDP
lack const here. Our implementations of these functions declare const
and thus do not match the older prototype, though they are compatible
with it.
The latter expects color input to be in an intermediate representation
resulting from using ReadColor(), and produces color output which cannot
be used until converted back with WriteColor().
Though deferred creation of buffers is already intended, creation was
not actually being deferred in practice as the act of initializing the
buffer with a solid rect of color was causing the buffer to be realized,
even if that initialization process is the only drawing operation that
will ever occur to that buffer.
We set the values within the OrderSupport array to match which RDP
messages we have implemented within guac_rdp_push_settings(). This array
is reset to its default values prior to the preconnect handler being
invoked, thus guac_rdp_push_settings() MUST be invoked within the
preconnect handler for its settings to have the intended effect.
Without proper values within OrderSupport, the initial RDP negotiation
process will claim support for messages that we don't actually support,
resulting in graphical artifacts and unexpected behavior.
