The second step of raw transfer is to transfer the format data itself.
This has been already implemented in XFreeRDP before, but several
tweaks are required for it to work correctly.
The idea of raw data transfer is to request for _FREERDP_RAW clipboard
format while putting the actual formatId into _FREERDP_CLIPRDR property
where the requested data is expected to arrive to. Then the clipboard
owner will check for the real formatId and deliver the expected data.
This stays true, but the check is performed in a more straightforward
way, and CF_RAW format (numerically equal to zero) is not considered
an unknown destination format when performing (identity) conversions
with wClipboard. This is not an issue because wClipboard will allow
only identity conversion for CF_RAW, it will fail if something else
is going to be converted into CF_RAW.
The first part of raw transfer sequence is to transfer the format
list of the session A into the session B. Then we will be able to
request/reply with raw data using proper format IDs.
xf_cliprdr_server_format_list() of the session A now exposes the
raw server format list. As soon as the list is received, it is
serialized and put into _FREERDP_CLIPRDR_FORMATS property.
xf_cliprdr_get_requested_targets() of the session B now checks
whether the clipboard owner is a FreeRDP session with enabled
raw transfer capability. If it is, the raw format list is simply
extracted from _FREERDP_CLIPRDR_FORMATS of the clipboard owner.
Otherwise, the format list is populated from the usual TARGETS
clipboard format.
Some time ago there was a property _FREERDP_CLIPRDR_ID which was indended
to indicate that an XFreeRDP window owns a clipboard. This was necessary
for raw transfers. This property was used by xf_cliprdr_is_self_owned()
function. However, raw transfer support was broken and the meaning of
xf_cliprdr_is_self_owned() gradually changed into checking whether
the *current* window owns the clipboard, not just any XFreeRDP window.
Thus _FREERDP_CLIPRDR_ID was removed in a4580923e7 (xfreerdp/clipr:
fix self owned test and hardening).
However, now we are going to fix raw transfers and we need that property.
This patch reintroduces a similar property "_FREERDP_CLIPRDR_RAW" which
indicates that a window is an XFreeRDP window with enabled raw transfer.
It is currently used by xf_cliprdr_server_format_data_request() to
correctly request format data from another XFreeRDP instance via raw
transfer protocol.
This property can be queried from the clipboard owner with the function
xf_cliprdr_is_raw_transfer_available() and can be enabled or disabled
on the current window by xf_cliprdr_set_raw_transfer_enabled().
Disabling raw transfers will be necesary to correctly implement file
transfers in the future. However, currently raw transfers are always
enabled.
xf_cliprdr_send_data_response() is consistently called with NULL
data pointer as a way to report errors, but it was not setting
the msgFlags field accordingly.
Sometimes Windows sends strings with excess null terminating bytes.
For example, when one copies digits from calc.exe. At the same time,
some local applications freak out when they encounter null bytes
(at least LibreOffice is known to be replacing them with '#').
According to the specification of UTF8_STRING format [1], the string
data must not contain any trailing null bytes. So they all should be
trimmed, not only the last one.
Also, if the trailing null byte is not present, the length should not
be adjusted. For example, Firefox is actually sending "HTML Format"
without a null byte while Internet Explorer adds one. The spec for
text/html format [2] says nothing about the teminating null byte, so
we are free to remove it, but at least we should not mistakingly
delete '>' character of "</html>" tag when it is the last character.
[1] http://www.pps.univ-paris-diderot.fr/~jch/software/UTF8_STRING/UTF8_STRING.text
[2] https://www.ietf.org/rfc/rfc2854.txt
Clipboard formats are identified by numerical IDs and literal names.
We can keep using arbitrary defined IDs for local clipboard formats
as we are sure that they have some fixed meaning, but the server can
and will be using its own IDs, which can be different from ours for
the named formats.
Therefore:
1) A correct way to compare a local format to a remote one is
to check the names first, and only then compare their IDs.
(Extra care should be taken to support short format names.)
2) Server IDs cannot be used with wClipboard directly when dealing
with named formats. Format name should be used to extract correct
local ID for the use with Clipboard{Set,Get}Data().
Also, I find the notion of 'alternate' format IDs to be confusing.
We either deal with a fixed ID format (declared in <winpr/user.h>),
or a format that was given an arbitrary fixed ID for local use
(defined in <freerdp/channels/cliprdr.h>), or a remote format
identified by a pair of an ID and a name. Format IDs can be local
and remote, but there are no 'alternates'.
So now:
1) A new function xf_cliprdr_formats_equal() is used to compare
formats correctly in xf_cliprdr_get_server_format_by_atom()
when searching for a server format corresponding to a local
one, and in xf_cliprdr_server_format_list() when constructing
a local TARGETS list from the server format list.
2) Correct local format IDs are used with wClipboard conversions
by xf_cliprdr_process_requested_data() and
xf_cliprdr_server_format_data_response().
3) We refer to formatId and formatName when doing requests,
and srcFormatId and dstFormatId when doing conversions,
instead of using formatId and altFormatId for both purposes.
4) Server format ID and name are used to identify cached clipboard
contents. The name is compared directly as a pointer because it
will be a pointer from the same clipboard->serverFormats array.
Also, the clipboard contents are invalidated when format list
arrives, so xf_cliprdr_server_format_list() now also clears
the format ID and name together with the data.
The functions now have appropriate names which tell what exactly
they are searching for:
xf_cliprdr_get_client_format_by_id()
Get a client-provided format by client-side ID.
xf_cliprdr_get_client_format_by_atom()
Get a client-provided format by client-side format name.
xf_cliprdr_get_server_format_by_atom()
Get a corresponding server format by client-side format name.
The return types of functions have been adjusted accordingly and
correct formats are now used everywhere without mixing them up:
client-side formats are used for client -> server data flow,
while server-side ones are used for server -> client tranfers.
This resolves the issue #1414 as, for some reason, xfreerdp required
server format list to be present to be able to provide its own client
formats. Actually, we need only client format list to provide these.
Also, CF_RAW special case is handled in a more elegant way: it is
assumed to be present in every server format list (which is true).
This patch is needed when wanting to install binaries/libraries to other
locations than data. The linux distro Exherbo installs binaries and
libraries to /usr/<chost>/bin, /usr/<chost>/lib respectively but
manpages should still go in /usr/share/man/ because they are
architecture independent, without this patch they go in
/usr/<chost>/share/man unconditionally.
cmake documentation states:
DATAROOTDIR - read-only architecture-independent data root (share)
So this patch makes it use that so that its configurable.
- fixed wrong calculation of xfc->fullscreenMonitors.[right|bottom]
- only use _NET_WM_FULLSCREEN_MONITORS if at least 2 monitors are involved
- call XMoveWindow before setting the _NET_WM_STATE_FULLSCREEN property
- xf_cliprdr_is_self_owned() lied if multiple xfreerdp instances were
running.
- fixed a few unchecked callocs
- added/modified and handled some return values in compliance with
the new hardened channel api
1. Remove all uses of "localWindowOffsetCorr" variables, they added an extra layer of complexity and they are not actually needed to handle coordination of window position/size between
the local coordinate system and the remote one. This logic was causing issues in the case where the window was moved off the left side of the screen.
2. Update the xf_setWindowVisibilityRects function to offset the visibility rects as necessary when the window is hanging off the left side of the screen.
3. Stop sending mouse events when doing keyboard moves/sizes(as desired), and stop sending two mouse events for non-keyboard moves/sizes
4. Move location of new UTF8_STRING variable from previous commit
5. Refresh window and window shape for any window position/size updates, this helps keep the local and server windows in sync and works around some race conditions
of existing after only looking at part of the information. For instance, window visibility
rects are part of the message with the new order and were being ignored.
The regions used to store and calculate the invalidRegion are exclusive
of the bottom and right edges, not inclusive.
Fixes "mouse droppings" in mspaint.exe when moving the mouse leftwards
across the canvas.
remove duplicate call to XStoreName when setting window title
expand WITH_XEXT #define for rail window rects as extra unecessary work was being done when WITH_XEXT was not defined
1. Make use of freerdp_set_last_error to set authentication failure without the helper functions
2. Rename ssl callback function
3. Break out AuthenticationOnly exit handling from bad connect handling
Don't abort the entire xf_rail_window_common function when the window is
already in the correct location.
To reproduce:
- move an application off the edge of the screen
- resize the window to cause a shape to be set
- move the application window fully on the screen
- resize the application window larger
- note lack of drawing in newly enlarged portion of window
Bug introduced in abf6d4f71e "xfreerdp:
prepare RAIL migration away from libfreerdp-rail" when
xf_rail_MoveWindow was copy-and-pasted into xf_rail_window_common
without noticing that the "return" would omit the rest of the combined
function, not just the portion that was pasted.
Since REGION16 uses unsigned values, when appWindow->x or appWindow->y
is negative, the region will have a very large left or top value.
Avoid this problem by clamping to 0 before casting to an unsigned value.
When connecting to windows 8.1 machines the remote RDP server
implementation sometimes sends invalid H264 data. To avoid client
disconnections ignore the broken updates.
Channels like EGFX need resources like the main window in order to
work correctly. Before the window, GDI,.. is freed it needs to be
ensured that all channels are stopped properly to prevent them to access
already freed resources. Disconnecting the channels first fixed a
possible race condition/SEGFAULT that could occur with remote initiated
disconnects.
winsock.h pulls in a lot of defines and dependencies that are not
required and partially unwanted in winpr's core (for parts that are not
related to network). In order to get rid of this dependency and have an
independent defines for extended winpr functions the WINPR_FD_* defines
are used internally (and for exposed functions). Where required, like in
WSAEventSelect, the FD_* is mapped to WINPR_FD_*.
Passing True to XSync() discards any pending X11 events. Occasionally
this caused ButtonRelease or KeyRelease to be lost and not forwarded
to the remote computed, leading to stuck keys and buttons.
This should resolve issue #2391
This patch contains:
* checks for malloc return value + treat callers;
* modified malloc() + ZeroMemory() to calloc();
* misc fixes of micro errors seen during the code audit:
** some invalid checks in gcc.c, also there were some possible
integer overflow. This is interesting because at the end the data are parsed
and freed directly, so it's a vulnerability in some kind of dead code (at least
useless);
** fixed usage of GetComputerNameExA with just one call, when 2 were used
in misc places. According to MSDN GetComputerNameA() is supposed to return
an error when called with NULL;
** there were a bug in the command line parsing of shadow;
** in freerdp_dynamic_channel_collection_add() the size of array was multiplied
by 4 instead of 2 on resize
* increase minimum required SDK to 14
* change functions from void to BOOL where required to have proper
return values
* update the JAVA classes and JNI accordingly
* add return value checks for *alloc and strdup
gcc 4.7.2
client/X11/xf_keyboard.c: In function ‘xf_keyboard_action_script_init’:
client/X11/xf_keyboard.c:48:6: warning: unused variable ‘exitCode’
[-Wunused-variable]
gcc 4.9
client/X11/xf_client.c: In function ‘xf_client_thread’:
client/X11/xf_client.c:1537:3: warning: ‘inputThread’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
When using software gdi (/gdi:sw) the rdp update PDU callback
gdi_palette_update() is used which writes the new colors to
the rdpContext's gdi->palette buffer.
The X11 functions however access xfc->palette which gets only
updated by xf_gdi_palette_update() which is the callback if
/gdi:hw is used.
This commit changes xfc->palette to a pointer which points to
xfc's private buffer with /gdi:hw or to the gdi->palette if
software gdi is used.
- HRESULT was unsigned which means that until now all usages of the
SUCCEDED(hr) and FAILED(hr) macros never detected any errors
- Also fixed the (unused) SCODE typedef
- Added new ctest TestTypes
- handle WAIT_TIMEOUT result as error in async transport thread
if an INFINITE timeout was specified in WaitForMultipleObjects
- fix mfreerdp's async transport handling to not use
freerdp_get_event_handles/freerdp_check_event_handles if async
transport is activated
1)
Added missing checks for CreateEvent which also required the
following related changes:
- changed freerdp_context_new API to BOOL
- changed freerdp_peer_context_new API to BOOL
- changed pRdpClientNew callback to BOOL
- changed pContextNew callback to BOOL
- changed psPeerAccepted callback to BOOL
- changed psPeerContextNew callback to BOOL
2)
Fixed lots of missing alloc and error checks in the
changed code's neighbourhood.
3)
Check freerdp_client_codecs_prepare result to avoid segfaults
caused by using non-initialized codecs.
4)
Fixed deadlocks in x11 caused by missing xf_unlock_x11() calls
in some error handlers
5)
Some fixes in thread pool:
- DEFAULT_POOL assignment did not match TP_POOL definition
- don't free the pool pointer if it points to the static DEFAULT_POOL
- added error handling and cleanup in InitializeThreadpool
- removed some unneeded null checks for free()
- fixed a memory leak in shadow_client
- removed rfx_compose_message_header from API
Changed the following functions to BOOL, check the result
where they are called and handle failures:
- rfx_compose_message
- rfx_compose_message_header
- rfx_write_tile
- rfx_write_message_tileset
- rfx_write_message_frame_begin
- rfx_write_message_region
- rfx_write_message_frame_end
- rfx_write_message
rfx_process_message:
- check memory allocation failures
- verify protocol-conform order of data messages to prevents memory
leaks caused by repeated allocations
- verify that header messages were parsed/received before the
data messages
- treat unknown rlgr mode as error
- fixed/added error handling
- fixed all callers to check/handle result
rfx_encode_message:
- fixed incorrect usage of realloc
- missing malloc check
- missing check of CreateThreadpoolWork
- correct cleanup on failure (threadpool, memory)
- check rfx_encode_message result
rfx_encode_messages:
- check rfx_split_message result
- correct cleanup on failure
- prevent memory leak on failure
rfx_write_message_context:
- fixed invalid channelId value (must be 0xFF for WBT_CONTEXT)
rfx_process_message_codec_versions:
- fixed invalid read size of codec_version (it is 16bit)
rfx_process_message_channels:
- verify protocol conform channelId value
rfx_process_message_region:
- replaced invalid reallocs with malloc
- read and verify regionType and numTileSets from stream
rfx_process_message_tileset:
- check allocation results
- fixed incorrect usages of realloc
setupWorkers:
- fixed incorrect usages of realloc
rfx_split_message:
- removed dead code
- missing malloc check
rfx_compose_message:
- fixed a memory leak
- check/handle rfx_encode_message result
