client/X11: cache original clipboard data for raw transfers

FreeRDP uses clipboard->data to cache the result of the Windows->X11
clipboard format conversion, and xf_cliprdr_process_selection_request()
immediately provides this result to local applications if they request
the same clipboard format again. This saves us a possibly costly
conversion in case where the user pastes data repeatedly.

However, this caching mechanism did not support raw clipboard transfers
where the unmodified data is passed between two FreeRDP clients. We use
the same XClipboard protocol for this, so the clipboard->data is in play.
We clear the cached value when we receive new data from the server, so
initially raw transfers are fine. But if some local application (e.g.,
a clipboard manager) asks for some data format before the data is pasted
into the second FreeRDP session then clipboard->data will contain the
*converted* data. And this converted cached data will be provided to
the second FreeRDP session as a part of the raw data transfer. Instead
we should have provided the original data.

In order to achieve this we are now caching the original data in the
same way as the converted one, and the original data is now correctly
provided when the second FreeRDP session asks for a raw data transfer.

diff --git a/client/X11/xf_cliprdr.c b/client/X11/xf_cliprdr.c
index e2af8cc..47068e9 100644 (file)
--- a/client/X11/xf_cliprdr.c
+++ b/client/X11/xf_cliprdr.c
@@ -81,10 +81,12 @@ struct xf_clipboard
        int requestedFormatId;
 
        BYTE* data;
+       BYTE* data_raw;
        BOOL data_raw_format;
        UINT32 data_format_id;
        const char* data_format_name;
        int data_length;
+       int data_raw_length;
        XEvent* respond;
 
        Window owner;
@@ -774,6 +776,23 @@ static BOOL xf_cliprdr_process_selection_notify(xfClipboard* clipboard,
        }
 }
 
+static void xf_cliprdr_clear_cached_data(xfClipboard* clipboard)
+{
+       if (clipboard->data)
+       {
+               free(clipboard->data);
+               clipboard->data = NULL;
+       }
+       clipboard->data_length = 0;
+
+       if (clipboard->data_raw)
+       {
+               free(clipboard->data_raw);
+               clipboard->data_raw = NULL;
+       }
+       clipboard->data_raw_length = 0;
+}
+
 static BOOL xf_cliprdr_process_selection_request(xfClipboard* clipboard,
         XEvent* xevent)
 {
@@ -785,6 +804,7 @@ static BOOL xf_cliprdr_process_selection_request(xfClipboard* clipboard,
        BYTE* data = NULL;
        BOOL delayRespond;
        BOOL rawTransfer;
+       BOOL matchingFormat;
        unsigned long length;
        unsigned long bytes_left;
        CLIPRDR_FORMAT* format;
@@ -847,14 +867,24 @@ static BOOL xf_cliprdr_process_selection_request(xfClipboard* clipboard,
                                }
                        }
 
-                       if ((clipboard->data != 0) && (formatId == clipboard->data_format_id)
-                           && (formatName == clipboard->data_format_name))
+                       /* We can compare format names by pointer value here as they are both
+                        * taken from the same clipboard->serverFormats array */
+                       matchingFormat = (formatId == clipboard->data_format_id)
+                               && (formatName == clipboard->data_format_name);
+
+                       if (matchingFormat && (clipboard->data != 0) && !rawTransfer)
                        {
-                               /* Cached clipboard data available. Send it now */
+                               /* Cached converted clipboard data available. Send it now */
                                respond->xselection.property = xevent->xselectionrequest.property;
                                xf_cliprdr_provide_data(clipboard, respond, clipboard->data,
                                                        clipboard->data_length);
                        }
+                       else if (matchingFormat && (clipboard->data_raw != 0) && rawTransfer)
+                       {
+                               /* Cached raw clipboard data available. Send it now */
+                               respond->xselection.property = xevent->xselectionrequest.property;
+                               xf_cliprdr_provide_data(clipboard, respond, clipboard->data_raw, clipboard->data_raw_length);
+                       }
                        else if (clipboard->respond)
                        {
                                /* duplicate request */
@@ -865,11 +895,7 @@ static BOOL xf_cliprdr_process_selection_request(xfClipboard* clipboard,
                                 * Send clipboard data request to the server.
                                 * Response will be postponed after receiving the data
                                 */
-                               if (clipboard->data)
-                               {
-                                       free(clipboard->data);
-                                       clipboard->data = NULL;
-                               }
+                               xf_cliprdr_clear_cached_data(clipboard);
 
                                respond->xselection.property = xevent->xselectionrequest.property;
                                clipboard->respond = respond;
@@ -1128,11 +1154,7 @@ static UINT xf_cliprdr_server_format_list(CliprdrClientContext* context,
        xfContext* xfc = clipboard->xfc;
        UINT ret;
 
-       if (clipboard->data)
-       {
-               free(clipboard->data);
-               clipboard->data = NULL;
-       }
+       xf_cliprdr_clear_cached_data(clipboard);
 
        clipboard->data_format_id = -1;
        clipboard->data_format_name = NULL;
@@ -1276,11 +1298,7 @@ static UINT xf_cliprdr_server_format_data_response(CliprdrClientContext*
        if (!clipboard->respond)
                return CHANNEL_RC_OK;
 
-       if (clipboard->data)
-       {
-               free(clipboard->data);
-               clipboard->data = NULL;
-       }
+       xf_cliprdr_clear_cached_data(clipboard);
 
        pDstData = NULL;
        DstSize = 0;
@@ -1345,8 +1363,25 @@ static UINT xf_cliprdr_server_format_data_response(CliprdrClientContext*
                }
        }
 
+       /* Cache converted and original data to avoid doing a possibly costly
+        * conversion again on subsequent requests */
        clipboard->data = pDstData;
        clipboard->data_length = DstSize;
+
+       /* We have to copy the original data again, as pSrcData is now owned
+        * by clipboard->system. Memory allocation failure is not fatal here
+        * as this is only a cached value. */
+       clipboard->data_raw = (BYTE*) malloc(size);
+       if (clipboard->data_raw)
+       {
+               CopyMemory(clipboard->data_raw, data, size);
+               clipboard->data_raw_length = size;
+       }
+       else
+       {
+               WLog_WARN(TAG, "failed to allocate %"PRIu32" bytes for a copy of raw clipboard data", size);
+       }
+
        xf_cliprdr_provide_data(clipboard, clipboard->respond, pDstData, DstSize);
        XSendEvent(xfc->display, clipboard->respond->xselection.requestor, 0, 0,
                   clipboard->respond);
@@ -1491,6 +1526,7 @@ void xf_clipboard_free(xfClipboard* clipboard)
 
        ClipboardDestroy(clipboard->system);
        free(clipboard->data);
+       free(clipboard->data_raw);
        free(clipboard->respond);
        free(clipboard->incr_data);
        free(clipboard);